fix(management): harden native toggle request handling

[luvs01]

Summary

• reject top-level null, arrays, and other non-object JSON bodies across the Claude, Codex, Grok, and Claude Desktop native-toggle routes;
• require a boolean enabled field through one shared type guard before any desired-state or artifact mutation;
• restore the live Claude config snapshot when persistence fails, so a lock refusal cannot leave memory ahead of disk or make a same-object retry look idempotent;
• add route-level regressions for all four native toggles and a real-lock same-object retry regression.

Why

The original Claude route parsed JSON into an object-shaped TypeScript variable and immediately read body.enabled. JSON null therefore bypassed malformed-JSON handling and raised a TypeError. The same runtime assumption remained in the Codex, Grok, and Claude Desktop sibling routes.

A deeper review found a second Claude-specific failure mode. The route changed config.claudeCode before calling the persistence seam. If the config lock returned 409/500, the live object stayed changed even though disk did not. Retrying with that same server snapshot then hit the “already current” branch and skipped persistence entirely.

The shared guard now rejects non-object bodies before every native-toggle mutation. Claude persistence remains the commit point: any thrown save restores the exact prior property state before returning or rethrowing.

Verification

• Bun 1.3.14: four native-toggle suites 51/51 passed.
• Bun 1.4.0-canary.1 (b22e0e6d0): the same four suites 51/51 passed.
• The real config-lock regression retries the same live object after 409 and confirms the second request persists the change.
• bun x tsc --noEmit: passed.
• bun scripts/privacy-scan.ts: passed.
• git diff --check: passed.
• Two independent exact-diff reviews found no remaining P0–P3 issues.
• Windows full suite: attempted, not passed and not counted as verification. Bun 1.3.14 terminated after 264.9 seconds with its known --isolate internal assertion while entering the storage-policy tests; no test failure was reported before the runtime crash.

Checklist

• Invalid bodies are rejected before desired-state, config, or artifact mutation.
• A failed Claude save restores the live snapshot and the same object can retry.
• Existing successful toggle semantics remain covered.

Draft pending exact-head upstream CI and fresh maintainer review.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes
  • Improved validation for native integration toggle requests, rejecting invalid or null request bodies with clear HTTP 400 errors.
  • Prevented invalid toggle requests from changing or persisting configuration.
  • Restored the previous Claude integration state when configuration saving fails, allowing subsequent retries to succeed.
• Tests
  • Added regression coverage for invalid requests and failed configuration persistence across supported integrations.

[coderabbitai]

📝 Walkthrough

Walkthrough

Native integration toggle routes now reject null, array, and primitive request bodies. Claude toggle persistence now restores the prior in-memory state when saving fails. Tests cover validation, state preservation, and retry behavior.

Changes

Native toggle integrity

Layer / File(s)	Summary
Request validation across native toggles src/server/management/native-integration-routes.ts, tests/native-claude-code-toggle.test.ts, tests/native-claude-desktop-toggle.test.ts, tests/native-codex-toggle.test.ts, tests/native-grok-toggle.test.ts	A shared guard requires a non-null, non-array object with a boolean enabled property. Codex, Grok, Claude Desktop, and Claude routes use the guard. Tests verify HTTP 400 responses and unchanged configuration or intent.
Claude persistence rollback src/server/management/native-integration-routes.ts, tests/native-claude-code-toggle.test.ts	The Claude route snapshots the previous claudeCode state before mutation. If persistence fails, the route restores the previous value or removes the property. Lock-contention coverage verifies that a retry succeeds.

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

Possibly related PRs

• lidge-jun/opencodex#1106: Introduced the native integration routes and toggle tests updated by this change.
• lidge-jun/opencodex#1291: Updated validation and persistence behavior for the native integration routes.

Suggested reviewers: lidge-jun, wibias

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: stronger request validation for native integration toggle routes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Ingwannu]

Reviewed exact head c85d792d. The route now rejects null/non-object/array JSON before reading enabled, preserving the existing 400 contract instead of throwing. Focused coverage passed locally (11 tests), git diff --check passed, and typecheck passed. The substantive cross-platform shards, gates, privacy scan, keyring jobs, and platform tests are green; the macOS npm-global packaging job is still waiting for a runner, so do not merge until that final job completes successfully.

[Ingwannu]

Re-reviewed rebased exact head a1cf0d9c. The PR diff is byte-equivalent in scope to the previously approved patch and still contains only the null-body guard plus its regression. Exact-head focused tests passed 11/11 and typecheck passed. Do not merge until the newly triggered exact-head CI is green.

[github-actions]

⏳ DRAFT

• review readiness checklist open (0/4 boxes ticked).

What to do

• Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

• ⬜ All CI tests are green on my local testing.
• ⬜ I pushed my PR to the latest dev commit.
• ⬜ I resolved all correct Codex and CodeRabbit findings.
• ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

[luvs01]

Updated exact head ca0f4a05 onto current dev 8f26e986 (ahead 2, behind 0).

The original null-body fix remains, and review of the surrounding route found two additional issues that are now covered in the same focused native-toggle patch:

• Codex, Grok, and Claude Desktop now use the same non-object/body guard, so null, arrays, and primitives return the deterministic 400 envelope before desired-state or artifact writes.
• Claude now restores the live claudeCode snapshot if persistence throws. The real-lock regression retries the exact same config object after 409 and proves the retry still acquires the lock and saves.

Verification at the published tree:

• Bun 1.3.14: four native-toggle suites 51/51 passed.
• Bun 1.4.0-canary.1: 51/51 passed.
• Typecheck, privacy scan, and git diff --check passed.
• Two independent exact-diff reviews found no remaining P0–P3 issues.
• The Windows full suite was attempted but is not counted as green: Bun 1.3.14 crashed after 264.9 seconds with the known --isolate internal assertion and no preceding test failure.

Fresh exact-head CI runs are awaiting maintainer approval, and the PR remains Draft pending CI and a new exact-head review.

[luvs01]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[lidge-jun]

Approved the pending Cross-platform CI run at this exact head, and it came back green.

Worth explaining the silence beforehand: contributor PRs queue their workflow runs as action_required until a maintainer releases them, and gh pr checks does not show that state — so from your side it looks like nothing is happening rather than like something is waiting on us. That was ours to clear, not yours.

To be precise about what this does and does not do: it only unblocks CI. It does not make this mergeable. The four-box readiness checklist in the description is your attestation, and the gate keeps the PR in draft until you complete it. With CI now green at your head, box 1 is provable.

If anything in the run looks wrong to you, say so and I will dig into it rather than leaving you to guess.

[lidge-jun]

Landed on dev as d3da4f4, rebased onto the current head with your authorship preserved.

Independent security review passed clean. Verified isNativeToggleBody rejects null, arrays, primitives, missing enabled, and non-boolean values before any side effect on all four toggle routes, and that the Claude rollback restores prior absence as well as a prior value — the reviewer injected lock contention into a config with no claudeCode block and confirmed 409 plus own-property absence restored. Full suite green on the rebased head (10189 pass, 0 fail).

Thanks — extending the guard to the sibling routes rather than patching only the reported one was the right instinct.