feat(toggles): Codex restore truth, Claude Desktop switch, composed acceptance (WP13 resume) - #1106
Conversation
…tating today, full sync-caller gating, workstation-only 030 leaves #1048 open)
…hantom coordinator; OFF path stays async
…alog joins the restore envelope
The commit-path OFF check runs under the first catalog permit; the models_cache rewrite reacquires K after release, so a disable landing in the gap could still publish a routed cache. Re-read intent under the second permit too.
…ee-client contract
…set, transport-liveness test restored
…status The inspector now carries ownedProfileActive (ID-match tri-state, null when metadata or appliedId is absent/unreadable) so the status route stops flattening undeterminable into false.
The toggle field names the API wire id (claude-desktop) which is not an OverviewClientId; widen the union explicitly. The status parser narrowed on fields absent from its readOptional shape.
…sts:write Pre-existing red on origin/dev (run 31078958589 and a clean dev worktree both fail it); the permissions gate now matches the migrated workflow.
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
This campaign is high-value, but the current head must be rebased before I can approve it.
The important boundaries are in the right place: OFF intent is persisted before mutation; config/history/catalog/cache writers re-read intent inside their serialization boundary; restore returns artifact-level truth instead of folding history failure into success; Claude Desktop removal pivots to a credential-free standard profile and refuses owned-but-drifted state; and the composed suite drives real CLI/HTTP entry points rather than only helpers. Those are meaningful correctness improvements, not cosmetic toggle work.
Current blocker: head 39f452ac is 8 commits behind dev and GitHub reports it conflicting. I reproduced the merge against a5b37827; the only conflict is tests/ci-workflows.test.ts. Current dev intentionally accepts either copilot-requests: write or the migration-era models: read, while this branch re-tightens the assertion to Copilot only. Please rebase onto current dev and keep the current dev assertion unless the workflow itself is being changed in this PR. The source/runtime changes merge without conflict.
Local current-dev merge validation after resolving that test conflict with the dev version:
- 12 focused Codex/Claude Desktop toggle, restore, sync, lock, and composed suites: 100 passed; one
native-codex-togglecase exceeded its 5 s timeout under the concurrent CPU-limited run and caused 3 cascading failures - isolated rerun of
tests/native-codex-toggle.test.ts: 6 passed, 0 failed (the timed case completed in 180 ms) bun run typecheck: passed
Because this touches credential-bearing Desktop profiles, persistent desired state, cross-process locks, management writers, and GUI controls, the rebased exact head needs the full required CI and GUI gates before approval. Please rebase rather than merge dev into the branch again, rerun the suite, and request review on the new SHA.
# Conflicts: # tests/ci-workflows.test.ts
The route persisted desired ON, then saved the whole long-lived server snapshot -- whose clientIntegrations still said OFF -- back over it, so its post-await guard refused the apply it had just been asked to perform. Persist only the desktopProfile field under the config-mutation lock, and route the writer through the deps seam its sibling path already uses.
setIntegrationEnabled writes disk only, and the server reuses one config object per request, so the native GET reported OFF right after a successful apply and the profile PUT wrote that stale OFF back. Mirror the transition onto the snapshot, and surface an unavailable profile mutation instead of dropping it.
…ccess The route already reported saved:false with a warning when the applied marker failed to persist, but both front ends discarded it and announced a clean save+apply. GUI shows a warn-tone notice (six locales), CLI prints the warning after a successful apply.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
src/server/management/agent-settings-routes.ts (1)
177-208: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse field-scoped persistence for auto-apply bookkeeping.
saveConfigPreservingClaudeCode(current)writes the completecurrentsnapshot after the Desktop artifact write. A concurrent process can changeclientIntegrationsor another config field after Line 189. This save can restore the stale value.Use
persistDesktopProfileFieldto store onlyclaudeCode.desktopProfilewith the new fingerprint and timestamp. That helper rebases against current disk state under the config mutation lock.🤖 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/server/management/agent-settings-routes.ts` around lines 177 - 208, Replace the full-snapshot saveConfigPreservingClaudeCode call in the result.written && result.fingerprint block with persistDesktopProfileField, passing the updated claudeCode.desktopProfile fingerprint and timestamp. Preserve the existing bookkeeping values while ensuring persistence is scoped only to claudeCode.desktopProfile.
🤖 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 `@src/server/management/agent-settings-routes.ts`:
- Around line 177-208: Replace the full-snapshot saveConfigPreservingClaudeCode
call in the result.written && result.fingerprint block with
persistDesktopProfileField, passing the updated claudeCode.desktopProfile
fingerprint and timestamp. Preserve the existing bookkeeping values while
ensuring persistence is scoped only to claudeCode.desktopProfile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 766ba33f-3b44-4830-b844-084abb919520
📒 Files selected for processing (15)
gui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/ClaudeDesktop.tsxgui/src/ui.tsxsrc/cli/claude-desktop.tssrc/config.tssrc/server/management/agent-settings-routes.tssrc/types.tstests/ci-workflows.test.tstests/claude-messages-endpoint.test.tstests/native-claude-desktop-toggle.test.ts
…-margin Restores dev to green after #1106: the contended-restore case needed a watchdog wide enough for production's own retry budget. Test-only.
…ggles Merging the WP13/WP14 toggle resume after an independent completeness audit (4 rounds) closed three real defects: the self-cancelling /apply enable, in-process snapshot incoherence, and an unconsumed partial-success envelope. Issue lidge-jun#1048 stays open for the deferred disposable-host census rows.
…-watchdog-margin Restores dev to green after lidge-jun#1106: the contended-restore case needed a watchdog wide enough for production's own retry budget. Test-only.
2 participants
Summary
Resumes the paused WP13/WP14 tail of the Codex write-substrate campaign: the Codex CLI toggle gets artifact-level restore truth, Claude Desktop gets a real ON/OFF toggle with a documented standard-mode pivot, and a composed acceptance suite proves the production entry points reach the substrate instead of writing around it.
devlog/_plan/260803_codex_desktop_toggle/040as amended by260806_wp13_toggles_resume/010):ocx restore/ejectpersist desired OFF before mutating (crash-durable),restore back/eject backpersist ON;restoreNativeCodexAsyncreturns a per-artifact{config, catalog, history}envelope where history failure carries abusy | permissiondiscriminator instead of being folded into success; everysyncModelsToCodexcaller (startup, ensure, sync, restore dispatch, models, provider,POST /api/sync, toggle enable) branches on a discriminatedapplied/skipped/refusedstatus; desired state is re-read inside each artifact serializer (write lock, history serialization, catalog commit, cache reacquisition) so a lost race becomes a typed skip, never a stale write.050as amended by020):claude-desktopjoinsclientIntegrationsand the native route union; a read-only inspector classifiesnot_installed/standard/gateway_ours/gateway_drifted/foreign/no_owned_state/broken/unsafewith an ownership boundary (owned-but-drifted refuses as unsafe; reads never create files); OFF pivots to a credential-free{}standard profile before removing the opencodex profile, its backup, and its metadata row; all four writer paths (auto-apply, native enable,POST /apply, no-daemon CLI apply) re-read persisted intent after their awaits;desiredEnabledis required on post-commit refusals for every client; GUI switch with six-locale copy.030):tests/codex-composed-acceptance.test.tsruns six scenarios through real spawned CLI children and a real HTTP server on temp homes — entry-to-funnel manifests, lost transition via HTTP, foreign-home zero-artifact refusal, same-user cross-home single lock, Grok OFF surviving a real restart, and restore truth via the newocx restore --json. Building it exposed four real production holes (OFF paths creating artifacts inCODEX_HOME,POST /api/synctrusting the server-captured config, foreign homes accepted over HTTP, opaque busy results), all fixed here. The disposable-host service class (P09/P10/P18/P34-P36) and remaining census rows stay deferred; WP13: composed acceptance at the production boundary #1048 remains open (referenced, not closed).Verification
bun x tsc --noEmit— clean at every commit.bun scripts/test.ts(full suite) — 9242 pass / 8 skip / 1 fail; the single failure (issue-quality workflow rejects workflow_dispatch pull request numbers before mutationintests/ci-workflows.test.ts) is pre-existing onorigin/dev(reproduced on a clean detached worktree ate496e9bfe) and untouched by this branch (git diff origin/dev...HEADtouches no workflow files).bun run lint:gui,bun run privacy:scan,bun run build:gui— green.PUT /api/native-integrations/claude-desktopOFF on anot_installedhome returns an idempotent no-op with zero filesystem footprint and persistedclientIntegrations["claude-desktop"]: false; GET lists all four clients.GUI change screenshot (Integrations overview with the Claude Desktop switch and the four native rows):
Related: #1048 (stays open — workstation subset landed here, service-class scenarios still deferred).
Checklist
260806_wp13_toggles_resume/documents the roadmap, amendments, and deferred rows).Summary by CodeRabbit
New Features
Bug Fixes
Tests