fix(claude): report a timed-out first-party rollback as incomplete by lidge-jun · Pull Request #7781 · lidge-jun/opencodex

lidge-jun · 2026-10-27T00:37:50Z

Summary

After #7777, the three Claude Desktop first-party rollback sites wait for overlapping ACL work through withConfigDirHardeningSettled. When that wait hits its deadline it throws, so the rollback skipped the existing "first-party settings rollback did not complete" response and fell into the generic error path. In the management route that was a 400 with an unrelated message. The settings left on disk were the same either way; only the reported error was wrong.

rollbackDesktopFirstPartySettled in src/claude/desktop-first-party.ts now returns false on a settle timeout without running the rollback, and rethrows any other error. It is used at all three sites:

src/server/management/agent-settings-routes.ts (POST /api/claude-desktop/apply)
src/server/management/native-integration-routes.ts (PUT /api/native-integrations/claude-desktop)
src/cli/claude-desktop.ts (offline ocx claude-desktop apply)

The model-defaults POST barrier test also gains a barrier-entry handshake, so it proves the request is held at the barrier rather than relying on a single macrotask.

Both items come from CodeRabbit on #7777 (review 5660614984 and thread r4383732541). They were moved here so #7777 could merge at its reviewed head for the v2.93.0 release gate. This PR lands after v2.93.0 is published on main.

Verification

Pinned Bun 1.4.4, OCX_TEST_NO_QUEUE=1:

bun test --isolate --timeout 60000 tests/claude-integration/claude-intercept-acl-barrier.test.ts: 30 pass, 0 fail. This includes the new test settled first-party rollback reports a settle timeout as incomplete without running, which also covers rethrowing a non-timeout error and the normal path.
bun node_modules/typescript/bin/tsc --noEmit: pass.
git diff --check: pass.

NOT RUN: full suite, lane=all, a route-level replay of a timed-out rollback. Hosted PR CI pending.

Checklist

Scope stays focused and avoids unrelated cleanup.
Docs or release notes were updated when needed.
Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

Bug Fixes
- Claude Desktop settings rollback now waits for configuration security changes to settle before proceeding. If that wait times out, rollback reports failure without running; other errors continue to be reported. This makes rollback outcomes more predictable when security changes are still in progress.

A single microtask yield depended on the current await count; draining queued microtasks with setImmediate keeps the assertions meaningful if later code adds awaits (CodeRabbit review on #7777).

Review r2 F3: startup migration and Desktop first-party removals rewrite the credential-bearing Claude settings file synchronously after an await. A CLAUDE_CONFIG_DIR nested in the opencodex home shares the config-root ACL flight, so gate the migration and every removal through withConfigDirHardeningSettled on the Claude settings home (removeDesktopFirstPartySettled maps a settle timeout to the existing unreadable refusal without writing). Regressions: nested-home migration and settled removal, red without the barrier, green with it.

Review r3 F4: POST /api/claude-code/model-defaults rewrites Claude settings, its staged file and recovery backups synchronously after the body await; gate the whole operation through withConfigDirHardeningSettled(claudeConfigDir()) inside the existing failure mapping. N1: the gateway-apply cleanup refusal no longer claims a JSON parse failure. Also asserts that the startup migration actually rewrites the stale proxy URL (CodeRabbit). Regression: model-defaults POST with a held parent flight, red without the barrier.

Rollback admission after #7777 threw WindowsSecretAclSettleTimeoutError on a settle timeout, escaping the existing 'rollback did not complete' responses. rollbackDesktopFirstPartySettled returns false instead. Also adds a barrier-entry handshake to the model-defaults POST test.

…-rollback-timeout # Conflicts: # src/claude/desktop-first-party.ts # src/cli/claude-desktop.ts # src/server/management/agent-settings-routes.ts # src/server/management/native-integration-routes.ts # tests/claude-integration/claude-intercept-acl-barrier.test.ts

lidge-jun · 2026-10-27T00:37:56Z

Post-release: this lands after v2.93.0 is published on main (release freeze). Do not merge before then.

chatgpt-codex-connector · 2026-10-27T00:38:01Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-27T00:39:55.355055Z	`8df35b0`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

coderabbitai · 2026-10-27T00:38:18Z

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

Source mode: Auto
Repository: lidge-jun/opencodex
Branch: fix/claude-first-party-rollback-timeout
File: .coderabbit.yaml
Commit: aa6e468
Layers: Repository YAML
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 355b40ba-7cfc-45bc-8835-78bcd9e85424

Commits

Reviewing files that changed from the base of the PR and between 8df35b0 and aa6e468.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: lidge-jun/opencodex
Branch: fix/claude-first-party-rollback-timeout
File: .coderabbit.yaml
Commit: 8df35b0
Layers: Repository YAML
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 07e18c4a-054c-43eb-b0f3-3c75083e9f8a

Commits

Reviewing files that changed from the base of the PR and between 4111e93 and 8df35b0.

Files selected for processing: 5

src/claude/desktop-first-party.ts
src/cli/claude-desktop.ts
src/server/management/agent-settings-routes.ts
src/server/management/native-integration-routes.ts
tests/claude-integration/claude-intercept-acl-barrier.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The change adds rollbackDesktopFirstPartySettled and uses it in three Claude Desktop rollback paths. Tests cover timeout handling, error propagation, successful rollback admission, and arrival at the hardening barrier.

Changes: Claude Desktop rollback

Layer / File(s)	Summary
Settled rollback helper `src/claude/desktop-first-party.ts` (lines 245–257)	Adds a helper that returns the rollback result, returns `false` on `WindowsSecretAclSettleTimeoutError`, and rethrows other errors.
Rollback paths and tests `src/cli/claude-desktop.ts` (lines 23, 437), `src/server/management/agent-settings-routes.ts` (lines 1065, 1108, 1127), `src/server/management/native-integration-routes.ts` (lines 34, 53, 771), `tests/claude-integration/claude-intercept-acl-barrier.test.ts` (lines 23, 540–570)	The CLI and two management routes use the helper for rollback. Tests cover timeout, error propagation, successful admission, and waiting for entry into the hardening barrier.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: luvs01

Merge Risk: Minimal · previous review 8df35

No code issue identified prevents merge after normal checks. Honor the stated v2.93.0 release sequencing.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: reporting a timed-out Claude Desktop first-party rollback as incomplete.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-10-27T00:42:50Z

✅ Deterministic PR hygiene checks passed.

…-rollback-timeout

lidge-jun · 2026-10-27T08:13:13Z

Maintainer integration decision: MERGE (squash) into dev

v2.93.0 is published on main (release captain confirmed; gh release view v2.93.0 shows a non-draft release at 88aaa75), so this post-release follow-up to #7777 can land.

Head aa6e468 = 8df35b0 + dev 736fa50 (clean merge). dev has not moved since.

Review. Independent sol review rel293-lb-review-7781-r1b: PASS, 0 blocking. It traced all three callers on a rollback admission timeout:

POST /api/claude-desktop/apply: 409 with the rollback warning.
PUT /api/native-integrations/claude-desktop: 500 write_failed.
The CLI: ok:false with the warning and exit 1.

None of these saves the mode marker, and each releases its lock. No other rollback or restore caller has the same problem. The reviewer showed both tests fail when either fix is removed.

CI on the exact head: PR CI all 27 checks green; lane=all workflow_dispatch run 40468314480, 28 jobs green (Windows 1–9/9, macOS, Linux).

Local (pinned Bun 1.4.4): claude-intercept-acl-barrier.test.ts 30/30, typecheck clean. NOT RUN: full suite; claude-desktop-first-party.test.ts locally (known host-baseline timeouts; covered by lane=all).

lidge-jun added 7 commits October 27, 2026 06:53

fix(claude): drain parent ACL flights before intercept secret writes

fdcec8e

fix(claude): bound ACL admission and guard picker cleanup steps

32720b9

test(claude): yield a macrotask before barrier ordering assertions

af22d2e

A single microtask yield depended on the current await count; draining queued microtasks with setImmediate keeps the assertions meaningful if later code adds awaits (CodeRabbit review on #7777).

lidge-jun requested a review from Ingwannu as a code owner October 27, 2026 00:37

github-actions Bot added the bug Something isn't working label Oct 27, 2026

coderabbitai Bot mentioned this pull request Oct 27, 2026

fix(windows): preserve bounded ACL failure diagnostics #7773

Merged

3 of 4 tasks

Merge remote-tracking branch 'origin/dev' into fix/claude-first-party…

aa6e468

…-rollback-timeout

lidge-jun merged commit dd8bf1d into dev Oct 27, 2026
62 checks passed

lidge-jun deleted the fix/claude-first-party-rollback-timeout branch October 27, 2026 08:13

lidge-jun mentioned this pull request Oct 27, 2026

feat(responses): preserve structural tool turns #7561

Merged

7 of 11 tasks

This was referenced Oct 27, 2026

Windows: writers under the config root can overlap the optional config-root ACL flight #7815

Open

fix(windows): gate config and Desktop writer phases on config-root ACL settlement #7826

Merged

Navigation Menu

Repository navigation

fix(claude): report a timed-out first-party rollback as incomplete - #7781

fix(claude): report a timed-out first-party rollback as incomplete#7781
lidge-jun merged 8 commits into
devfrom
fix/claude-first-party-rollback-timeout

lidge-jun commented Oct 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

lidge-jun commented Oct 27, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 27, 2026 •

edited

Loading

Review skipped

Uh oh!

github-actions Bot commented Oct 27, 2026 •

edited

Loading

Uh oh!

lidge-jun commented Oct 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lidge-jun commented Oct 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Checklist

Summary by CodeRabbit

Uh oh!

lidge-jun commented Oct 27, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

coderabbitai Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lidge-jun commented Oct 27, 2026

Maintainer integration decision: MERGE (squash) into dev

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lidge-jun commented Oct 27, 2026 •

edited by coderabbitai Bot

Loading

chatgpt-codex-connector Bot commented Oct 27, 2026 •

edited

Loading

coderabbitai Bot commented Oct 27, 2026 •

edited

Loading

github-actions Bot commented Oct 27, 2026 •

edited

Loading