fix(integrations): honor OFF for Claude Desktop drift and Grok ensure (#2250 rebased) - #2259
Conversation
Claude Desktop disable refused owned gateway_drifted profiles (including a missing appliedFingerprint), so leftover Claude-3p configs could not be turned off and looked like they needed an update. Grok ensure still called syncGrokConfig unconditionally, rewriting ~/.grok after updates even when the durable switch stayed off. Gate ensure on desired state, clear Desktop residue when OFF, and stop treating OFF leftovers as stale applies.
Keep leftover owned drift and assert the status route reports residue, not a stale apply.
A toggle during ensure's probe/start window could make the stale snapshot strip a freshly enabled Grok fence or delete a freshly applied Desktop profile. Re-read persisted desired state immediately before each file write, and show leftover desired-off Desktop gateways as stale/cleanup-pending instead of absent.
|
Warning Review limit reached
Next review available in: 8 minutes Limit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
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. |
⏳ DRAFT
What to do
Automatic draft conversion failed (token cannot change draft status). Please convert this pull request to a draft manually. The required |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1419ba2fdf
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const g = await deps.syncGrokConfig( | ||
| port, | ||
| config, | ||
| hostname !== undefined ? { hostname } : {}, | ||
| ); |
There was a problem hiding this comment.
Recheck Grok intent after catalog loading
When Grok is ON and syncGrokConfig is awaiting fetchAllModels (src/grok/sync.ts), a concurrent dashboard toggle can persist OFF and strip the fence, after which this call resumes and injects it again from the stale config loaded at line 79. This leaves the durable switch OFF while Grok remains routed through OpenCodex, recreating the race this change is intended to close; recheck the desired state after model discovery and immediately before the synchronous file write.
Useful? React with 👍 / 👎.
| // responds — align here too so `ocx ensure` never returns with a stale ON/OFF mismatch. | ||
| // Persisted state is loaded inside each mutation after waitForProxy, so a | ||
| // toggle while the child starts wins over the pre-spawn snapshot. | ||
| await reconcileEnsureDesiredIntegrations(port, { kind: "spawned" }); |
There was a problem hiding this comment.
Use the spawned proxy's observed hostname
When config.hostname changes after the child reads its startup configuration but before waitForProxy returns, the spawned branch reloads the new configured hostname instead of using the hostname of the process that actually became live. Because syncGrokConfig derives both the generated URL and its loopback/non-loopback policy from that value, it can strip a valid fence or write one for an address the running proxy did not bind; retain the LiveProxy returned by waitForProxy and pass its observed hostname as the live branch already does.
Useful? React with 👍 / 👎.
Summary
Lands PR #2250 by @lilinxiong (integrations honor OFF for Claude Desktop drift and Grok ensure), rebased onto current dev plus the two reviewer blockers fixed:
Verification
Checklist