Repository navigation
fix(cli): name the reviewed recovery for Aside profiles that read off (stale) - #6825
Conversation
… (stale) Sync refreshes only profiles whose preference is on, so an "off (stale)" profile left "ocx integration client sync --client aside" printing a bare no-op. The empty result now names the status command and the preview-then- enable commands, and status (list and --profile) prints them with the real ID for each off profile beside a stale block. Nothing is re-enabled and no request is added; --json output is unchanged. A malformed asideProfileSync still falls back to all profiles off, but the config load now says so once without echoing the value. Closes #6757
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (17)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe change records N5 review and closeout procedures. It adds CLI guidance for recovering disabled stale Aside profiles and warns when a persisted ChangesN5 workflow and Aside recovery
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change provides manual recovery instructions and a malformed-configuration warning without automatically enabling profiles. No concrete merge-blocking issue is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)✅ Passed checks (4 passed)Full details: Out of Scope Changes checkExplanation The functional and documentation changes support issue
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
|
Maintainer integration record (MAINTAINERS.md,
|
Summary
off (stale), breaking sidecar sync (no auto-recovery,client syncno-ops) #6757):ocx integration client status --client asidecan show profiles asoff (stale).ocx integration client sync --client asidethen printed "No eligible Aside profiles to synchronize. Check integration status and profile sync preferences." and gave no next step. Sync refreshes only profiles whose sync preference is on, which is correct and unchanged.ocx integration client status --client aside, and gives the preview-then-enable commands with a<N>placeholder. The enable step says to run it "if the preview permits the change and you accept it", because a refused preview can still exit 0.--profile Nviews print those commands with the real ID for each profile that is off beside a stale managed block. Conflict, unsafe, current and enabled profiles get no hint.asideProfileSync(for example, one non-boolean entry) still falls back to{ allProfiles: false }, which turns every profile off. The config load now warns about this once and does not echo the value. This is the one silent path we found that turns all profiles off.--jsonoutput stay the same. The sync response does not carry profile IDs on purpose: reading profile state runs integration maintenance (snapshot pruning), and an empty sync currently writes nothing.trueintofalse. The candidates are the schema fallback above, a lost legacy-derived default (no policy, and the root ownership record no longer matching), an explicit disable or Undo, or a differentOPENCODEX_HOME. Telling them apart needs the affected runtime'sasideProfileSyncand ownership records.devlog/_plan/261009_n5_small_sweep/(N5 sweep) is included.Closes #6757
Verification
With a temporary HOME, on this head:
bun test tests/cli/cli-aside-sync.test.ts tests/config/config-load-degrade.test.ts: 64 pass, 0 fail.bun test tests/cli/cli-headless-parity.test.ts -t Aside: 15 pass, 0 fail.bun test tests/ci-workflows/file-size-ratchet.test.ts tests/test-layout.test.ts: 11 pass, 0 fail.bun run typecheck,bun run structure:check,bun run privacy:scan: all passed.src/changes reverted, the four new behavior tests fail: the sync guidance, the status list, the single-profile status, and the config warning.Checklist
docs-siteintegrations guide,structure/cli-management.md,structure/config.md,structure/clients/integrations.md.Summary by CodeRabbit
off (stale), including commands to preview and conditionally re-enable each affected profile.