Fix updater cleanup for stale OpenClaw install metadata#186
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesClawRouter Metadata Cleanup with Supporting Documentation and Formatting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 936-945: The newestCurrentPackage() function currently selects the
package candidate with the most recent modification time (mtimeMs), which can
incorrectly choose a stale project-scoped or npm-layout copy if it was touched
more recently than the active extension install. Change the selection logic to
prioritize the active extension install location
(~/.openclaw/extensions/clawrouter) first before falling back to other
candidates. Modify the comparison that updates the best variable to check if the
candidate is the verified active install and prefer it, only using mtimeMs as a
secondary comparison when choosing between non-active candidates or candidates
in the same category.
🪄 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: a0a948ba-016d-48f3-b45d-500bfd284fd1
📒 Files selected for processing (1)
scripts/update.sh
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/proxy.reasoning-header.test.ts (1)
54-56: 💤 Low valueConsider reverting to the simpler regex approach.
The new
Array.from(reasoning).some((ch) => (ch.codePointAt(0) ?? 0) > 0x7f)check is functionally equivalent to the previous regex/[^\x00-\x7F]/but significantly more verbose. For this test's purpose—verifying that the reasoning string contains at least one non-ASCII character—the regex is clearer and more idiomatic. The?? 0fallback is also unnecessary sincecodePointAt(0)on a character fromArray.fromalways returns a number.♻️ Simpler approach
- expect( - Array.from(reasoning).some((ch) => (ch.codePointAt(0) ?? 0) > 0x7f), - ).toBe(true); + expect(reasoning).toMatch(/[^\x00-\x7F]/);🤖 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 `@src/proxy.reasoning-header.test.ts` around lines 54 - 56, Replace the verbose Array.from(reasoning).some((ch) => (ch.codePointAt(0) ?? 0) > 0x7f) check in the expect statement with a simpler regex approach. Use the regex pattern /[^\x00-\x7F]/ to test if the reasoning string contains at least one non-ASCII character by calling test() on the regex against the reasoning variable, which is clearer and more idiomatic for this purpose.
🤖 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.
Nitpick comments:
In `@src/proxy.reasoning-header.test.ts`:
- Around line 54-56: Replace the verbose Array.from(reasoning).some((ch) =>
(ch.codePointAt(0) ?? 0) > 0x7f) check in the expect statement with a simpler
regex approach. Use the regex pattern /[^\x00-\x7F]/ to test if the reasoning
string contains at least one non-ASCII character by calling test() on the regex
against the reasoning variable, which is clearer and more idiomatic for this
purpose.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: da536eed-59cf-41d2-82ab-1894e73b80af
📒 Files selected for processing (2)
scripts/update.shsrc/proxy.reasoning-header.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/update.sh
1 participant
Summary
Testing
Summary by CodeRabbit
NO_PROXY/loopback handling) whenBLOCKRUN_UPSTREAM_PROXYis unset.