-
Notifications
You must be signed in to change notification settings - Fork 868
fix(integrations): honor OFF for Claude Desktop drift and Grok ensure (#2250 rebased) #2259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0dba81f
fix(integrations): honor OFF for Claude Desktop drift and Grok ensure
lilinxiong bff2c5f
test(integrations): cover desired-off gateway drift status
lilinxiong 8e4b8dd
fix(integrations): re-read desired state before ensure mutations
lilinxiong 1419ba2
fix(integrations): close ensure desired-state race
lidge-jun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| /** | ||
| * Align Grok and Claude Desktop files with the durable switches during `ocx ensure`. | ||
| * | ||
| * handleEnsure used to load config once, then health-probe / model-sync / spawn, | ||
| * and only afterwards mutate ~/.grok/config.toml and the Desktop library from | ||
| * that snapshot. An OFF→ON flip in that window stripped a freshly enabled fence | ||
| * or deleted a freshly applied Desktop profile; ON→OFF rewrote the files the | ||
| * user had just turned off. Re-read persisted desired state immediately before | ||
| * each external-file mutation, and use that current config for sync inputs. | ||
| */ | ||
| import { loadConfig } from "../config"; | ||
| import { stripGrokConfig, type GrokInjectResult } from "../grok/inject"; | ||
| import { removeDesktop3pStandardPivot } from "../claude/desktop-3p"; | ||
| import { | ||
| claudeDesktopIntegrationEnabled, | ||
| shouldSyncGrokOnStart, | ||
| } from "../codex/desired-state"; | ||
| import type { OcxConfig } from "../types"; | ||
|
|
||
| export function grokSyncFailureMessage(err: unknown): string { | ||
| const detail = err instanceof Error ? err.message : String(err); | ||
| return `Grok Build config sync failed: ${detail}. ` | ||
| + "~/.grok/config.toml may still point at a previous proxy port — " | ||
| + "run 'ocx ensure' (or apply from the dashboard's Grok page) to repoint it."; | ||
| } | ||
|
|
||
| export interface EnsureDesiredIntegrationsDeps { | ||
| loadConfig: () => OcxConfig; | ||
| stripGrokConfig: typeof stripGrokConfig; | ||
| syncGrokConfig: ( | ||
| port: number, | ||
| config: OcxConfig, | ||
| opts?: { hostname?: string }, | ||
| ) => Promise<GrokInjectResult>; | ||
| removeDesktop3pStandardPivot: typeof removeDesktop3pStandardPivot; | ||
| log?: (message: string) => void; | ||
| error?: (message: string) => void; | ||
| } | ||
|
|
||
| async function defaultSyncGrokConfig( | ||
| port: number, | ||
| config: OcxConfig, | ||
| opts: { hostname?: string } = {}, | ||
| ): Promise<GrokInjectResult> { | ||
| const { syncGrokConfig } = await import("../grok/sync"); | ||
| return syncGrokConfig(port, config, opts); | ||
| } | ||
|
|
||
| const productionDeps: EnsureDesiredIntegrationsDeps = { | ||
| loadConfig, | ||
| stripGrokConfig, | ||
| syncGrokConfig: defaultSyncGrokConfig, | ||
| removeDesktop3pStandardPivot, | ||
| }; | ||
|
|
||
| function io(deps: EnsureDesiredIntegrationsDeps): { | ||
| log: (message: string) => void; | ||
| error: (message: string) => void; | ||
| } { | ||
| return { | ||
| log: deps.log ?? (message => console.log(message)), | ||
| error: deps.error ?? (message => console.error(message)), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Keep ~/.grok/config.toml aligned with the durable Grok switch. | ||
| * | ||
| * `handleStart` already gates inject on `shouldSyncGrokOnStart`. `ocx ensure` | ||
| * used to call `syncGrokConfig` unconditionally, so a dashboard/update/restart | ||
| * path that lands in ensure rewrote the fence while the switch stayed OFF. | ||
| * When the switch is OFF, strip any leftover managed block instead of injecting. | ||
| */ | ||
| export async function ensureGrokFenceMatchesDesired( | ||
| port: number, | ||
| opts: { hostname?: string } = {}, | ||
| deps: EnsureDesiredIntegrationsDeps = productionDeps, | ||
| ): Promise<void> { | ||
| const config = deps.loadConfig(); | ||
| const { log, error } = io(deps); | ||
| if (!shouldSyncGrokOnStart(config)) { | ||
| try { | ||
| const grok = deps.stripGrokConfig(); | ||
| if (grok.changed) log(` ↩️ ${grok.message}`); | ||
| else if (!grok.ok) error(`⚠️ ${grok.message}`); | ||
| } catch (err) { | ||
| error(`⚠️ ${grokSyncFailureMessage(err)}`); | ||
| } | ||
| return; | ||
| } | ||
| try { | ||
| const hostname = opts.hostname ?? config.hostname; | ||
| const g = await deps.syncGrokConfig( | ||
| port, | ||
| config, | ||
| hostname !== undefined ? { hostname } : {}, | ||
| ); | ||
| if (g.changed) log(" + Grok Build config updated (~/.grok/config.toml)"); | ||
| else if (!g.ok) error(`⚠️ ${g.message}`); | ||
| } catch (err) { | ||
| error(`⚠️ ${grokSyncFailureMessage(err)}`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * When Claude Desktop is durably OFF, clear any leftover owned gateway profile. | ||
| * ensure/update used to leave Claude-3p residue in place after a failed disable | ||
| * (drifted fingerprint), so the Integrations card kept looking applied/stale. | ||
| */ | ||
| export function ensureClaudeDesktopMatchesDesired( | ||
| deps: EnsureDesiredIntegrationsDeps = productionDeps, | ||
| ): void { | ||
| const config = deps.loadConfig(); | ||
| const { log, error } = io(deps); | ||
| if (claudeDesktopIntegrationEnabled(config)) return; | ||
| try { | ||
| const removed = deps.removeDesktop3pStandardPivot({ | ||
| appliedFingerprint: config.claudeCode?.desktopProfile?.appliedFingerprint ?? null, | ||
| }); | ||
| if (removed.ok && removed.changed) { | ||
| log(" ↩️ Claude Desktop integration residue removed."); | ||
| } else if (!removed.ok) { | ||
| error(`⚠️ Claude Desktop cleanup skipped: ${removed.reason ?? removed.kind}.`); | ||
| } | ||
| } catch (err) { | ||
| const detail = err instanceof Error ? err.message : String(err); | ||
| error(`⚠️ Claude Desktop cleanup failed: ${detail}.`); | ||
| } | ||
| } | ||
|
|
||
| export type EnsureDesiredIntegrationsBranch = | ||
| | { kind: "live"; hostname?: string } | ||
| | { kind: "spawned" }; | ||
|
|
||
| /** | ||
| * Reconcile the two external integration files after either ensure race window. | ||
| * Only the live proxy's observed bind host crosses this boundary; persisted | ||
| * config is deliberately loaded inside each mutation helper. | ||
| */ | ||
| export async function reconcileEnsureDesiredIntegrations( | ||
| port: number, | ||
| branch: EnsureDesiredIntegrationsBranch, | ||
| deps: EnsureDesiredIntegrationsDeps = productionDeps, | ||
| ): Promise<void> { | ||
| const liveHost = branch.kind === "live" ? branch.hostname : undefined; | ||
| await ensureGrokFenceMatchesDesired( | ||
| port, | ||
| liveHost ? { hostname: liveHost } : {}, | ||
| deps, | ||
| ); | ||
| ensureClaudeDesktopMatchesDesired(deps); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Grok is ON and
syncGrokConfigis awaitingfetchAllModels(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 👍 / 👎.