fix(update): recover from OpenClaw size-drop rejection#170
Merged
1bcMax merged 6 commits intoMay 24, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughscripts/update.sh captures OpenClaw install output to a temp log, detects timeouts and "Config write rejected: ... size-drop:" failures, applies a Node-based scoped-model trim from the rejected payload when appropriate, sets OPENCLAW_INSTALL_RECOVERABLE to defer rollback, and—if needed—force-installs the plugin from npm and clears the trap afterward. ChangesInstall Resilience Improvements
Documentation formatting (non-functional)
Minor code formatting and tests (non-functional)
Sequence Diagram(s)sequenceDiagram
participant UpdateScript as UpdateScript
participant OpenClaw as openclaw
participant LogFile as OPENCLAW_INSTALL_LOG
participant NodeHelper as apply_scoped_model_trim
participant Npm as npm
participant Disk as Filesystem
UpdateScript->>OpenClaw: run "openclaw plugins install --force `@blockrun/clawrouter`" (captured via timeout|tee -> OPENCLAW_INSTALL_LOG)
OpenClaw->>LogFile: emit install output (may include "Config write rejected: ... size-drop:")
UpdateScript->>NodeHelper: on size-drop, extract rejected payload path and invoke apply_scoped_model_trim
NodeHelper->>Disk: read active openclaw.json and rejected payload
NodeHelper->>NodeHelper: validate keys, sections, ordering, and byte/delta thresholds
NodeHelper->>Disk: write updated openclaw.json (models.providers.blockrun.models)
UpdateScript->>Npm: if recoverable and plugin missing, resolve version and force-install from npm
Npm->>Disk: install package (package.json present)
UpdateScript->>UpdateScript: clear rollback trap after recovery path
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/update.sh`:
- Around line 456-468: The recoverable-install branch uses an elif so a
size-drop path can skip the npm fallback while still clearing the rollback trap;
change the logic so when OPENCLAW_INSTALL_RECOVERABLE=1 you always attempt the
npm fallback via force_install_from_npm and then only run the trap - EXIT INT
TERM after confirming INSTALLED_VER (from the node check of
$PLUGIN_DIR/package.json) indicates a successful install (not "?" or empty);
locate the block referencing OPENCLAW_INSTALL_RECOVERABLE,
force_install_from_npm, the node-based INSTALLED_VER check and the trap - EXIT
INT TERM and refactor them into an unconditional-if for the fallback plus a
guarded trap-clear that runs only on successful INSTALLED_VER.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c89363a-eeee-443b-87f8-1291fcb55a44
📒 Files selected for processing (1)
scripts/update.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/update.sh`:
- Around line 421-425: The timeout handler that checks exit_code==124 should not
unconditionally treat the install as successful; change the block in
scripts/update.sh to detect whether the plugin actually completed (for example
by checking for the expected package.json or the installed plugin artifact)
before returning 0, and only clear/avoid setting OPENCLAW_INSTALL_RECOVERABLE
when that verification passes; if the verification fails, propagate failure (do
not return 0) so the npm fallback runs and rollback remains possible. Ensure you
reference exit_code, OPENCLAW_INSTALL_RECOVERABLE, and the presence of
package.json (or the specific install success indicator used elsewhere) when
implementing the check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ae61885-6151-416a-b326-2cedd4bf92da
📒 Files selected for processing (1)
scripts/update.sh
prettier reinterpreted the bare `*` chars in `/v1/phone/*, /v1/voice/*` as italic markers and rewrote them to `_`. Escape with `\*` so the literal asterisks survive future prettier passes.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/update.sh (1)
373-405:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify the rejected BlockRun list is a true shrink before applying it.
Lines 379-405 only check counts and byte deltas. They never prove that
rejected.models.providers.blockrun.modelsis actually a subset of the active list, so a payload that swaps or edits model entries can still pass and overwrite the live config. Please require every rejected model to match an existing active model entry, and ideally preserve relative order, before Line 408 copies it over.🔧 Minimal guard
const activeModels = getBlockrunModels(active); const rejectedModels = getBlockrunModels(rejected); if (!Array.isArray(activeModels) || !Array.isArray(rejectedModels)) { fail('blockrun model list is missing or invalid'); } + +const canonical = (model) => JSON.stringify(model); +const activeSet = new Set(activeModels.map(canonical)); +for (const model of rejectedModels) { + if (!activeSet.has(canonical(model))) { + fail('rejected model list is not a pure shrink of the active list'); + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/update.sh` around lines 373 - 405, Ensure the rejected BlockRun list is a true shrink by verifying each entry in rejectedModels actually exists in activeModels (and optionally in the same relative order) before accepting the change: iterate rejectedModels (from rejected.models.providers.blockrun.models / rejectedModels as returned by getBlockrunModels) and check each item matches an entry in activeModels (or that the sequence of indexes in activeModels is strictly increasing to preserve order); if any rejected model is not found or ordering is violated, call fail with a clear message and abort instead of proceeding to copy active.models -> rejected.models.
♻️ Duplicate comments (1)
scripts/update.sh (1)
557-578:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep rollback armed until the recoverable npm install is actually confirmed.
Because the generic
package.jsonbranch at Line 557 runs before the recoverable branch at Line 566, a size-drop recovery can skipforce_install_from_npmentirely and still clear the rollback trap at Lines 576-578. That puts the script back in the state this PR is trying to avoid: rollback is disabled without a confirmed successful fallback install.🔧 Safer control flow
-if [ -d "$PLUGIN_DIR" ] && [ -f "$PLUGIN_DIR/package.json" ]; then +if [ "$OPENCLAW_INSTALL_RECOVERABLE" = "1" ]; then + LATEST_VER=$(npm view `@blockrun/clawrouter`@latest version 2>/dev/null || echo "") + if [ -z "$LATEST_VER" ]; then + echo " ✗ Could not resolve latest ClawRouter version from npm" + exit 1 + fi + force_install_from_npm "$LATEST_VER" + INSTALLED_VER=$(node -e "try{const p=require('$PLUGIN_DIR/package.json');console.log(p.version);}catch{console.log('');}" 2>/dev/null || echo "") + if [ -z "$INSTALLED_VER" ]; then + echo " ✗ Recoverable install did not produce a valid package.json" + exit 1 + fi + echo " ✓ ClawRouter v${INSTALLED_VER} installed" + trap - EXIT INT TERM +elif [ -d "$PLUGIN_DIR" ] && [ -f "$PLUGIN_DIR/package.json" ]; then INSTALLED_VER=$(node -e "try{const p=require('$PLUGIN_DIR/package.json');console.log(p.version);}catch{console.log('');}" 2>/dev/null || echo "") LATEST_VER=$(npm view `@blockrun/clawrouter`@latest version 2>/dev/null || echo "") if [ -n "$LATEST_VER" ] && [ -n "$INSTALLED_VER" ] && [ "$INSTALLED_VER" != "$LATEST_VER" ]; then echo " ⚠️ openclaw installed v${INSTALLED_VER} (cached) but latest is v${LATEST_VER}" force_install_from_npm "$LATEST_VER" || true fi INSTALLED_VER=$(node -e "try{const p=require('$PLUGIN_DIR/package.json');console.log(p.version);}catch{console.log('?');}" 2>/dev/null || echo "?") echo " ✓ ClawRouter v${INSTALLED_VER} installed" -fi -if [ "$OPENCLAW_INSTALL_RECOVERABLE" = "1" ]; then - trap - EXIT INT TERM fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/update.sh` around lines 557 - 578, The current flow clears the rollback trap unconditionally at the end, which can disable rollback when the generic package.json branch ran but the recoverable npm fallback hasn't completed; modify the control flow so that the trap (trap - EXIT INT TERM) is only cleared after a confirmed successful recoverable install: keep OPENCLAW_INSTALL_RECOVERABLE set while running force_install_from_npm and only run trap - EXIT INT TERM after force_install_from_npm succeeds (and after verifying INSTALLED_VER via node require), or clear the trap inside the branch that runs when OPENCLAW_INSTALL_RECOVERABLE=1 but only after a successful force_install_from_npm and verification; reference PLUGIN_DIR, OPENCLAW_INSTALL_RECOVERABLE, force_install_from_npm, INSTALLED_VER and the trap command to locate where to change the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@scripts/update.sh`:
- Around line 373-405: Ensure the rejected BlockRun list is a true shrink by
verifying each entry in rejectedModels actually exists in activeModels (and
optionally in the same relative order) before accepting the change: iterate
rejectedModels (from rejected.models.providers.blockrun.models / rejectedModels
as returned by getBlockrunModels) and check each item matches an entry in
activeModels (or that the sequence of indexes in activeModels is strictly
increasing to preserve order); if any rejected model is not found or ordering is
violated, call fail with a clear message and abort instead of proceeding to copy
active.models -> rejected.models.
---
Duplicate comments:
In `@scripts/update.sh`:
- Around line 557-578: The current flow clears the rollback trap unconditionally
at the end, which can disable rollback when the generic package.json branch ran
but the recoverable npm fallback hasn't completed; modify the control flow so
that the trap (trap - EXIT INT TERM) is only cleared after a confirmed
successful recoverable install: keep OPENCLAW_INSTALL_RECOVERABLE set while
running force_install_from_npm and only run trap - EXIT INT TERM after
force_install_from_npm succeeds (and after verifying INSTALLED_VER via node
require), or clear the trap inside the branch that runs when
OPENCLAW_INSTALL_RECOVERABLE=1 but only after a successful
force_install_from_npm and verification; reference PLUGIN_DIR,
OPENCLAW_INSTALL_RECOVERABLE, force_install_from_npm, INSTALLED_VER and the trap
command to locate where to change the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 645664cd-1967-43bc-8689-e154e0a9d432
📒 Files selected for processing (2)
CHANGELOG.mdscripts/update.sh
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Context
Older ClawRouter versions wrote the full BlockRun model catalog into openclaw.json. Newer versions trim that provider model list to the curated visible picker set, so upgrades from older installs can legitimately shrink the config by a large amount. OpenClaw correctly rejects that as a potential data-loss size drop, but the updater should not roll back the whole ClawRouter update in that recoverable case.
The scoped config update is only applied after validating that required top-level sections are still present and the size reduction is primarily from models.providers.blockrun.models. If validation fails, the updater preserves the existing config and still uses the npm fallback instead of forcing the rejected payload.
Testing
Summary by CodeRabbit
Bug Fixes
Chores
Documentation