Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/cli/ensure-desired-integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export interface EnsureDesiredIntegrationsDeps {
syncGrokConfig: (
port: number,
config: OcxConfig,
opts?: { hostname?: string; probeHostname?: string },
opts?: { hostname?: string; probeHostname?: string; admit?: () => boolean },
) => Promise<GrokInjectResult>;
removeDesktop3pStandardPivot: typeof removeDesktop3pStandardPivot;
removeDesktopFirstParty?: typeof removeDesktopFirstParty;
Expand All @@ -67,7 +67,7 @@ export interface EnsureDesiredIntegrationsDeps {
async function defaultSyncGrokConfig(
port: number,
config: OcxConfig,
opts: { hostname?: string; probeHostname?: string } = {},
opts: { hostname?: string; probeHostname?: string; admit?: () => boolean } = {},
): Promise<GrokInjectResult> {
const { syncGrokConfig } = await import("../grok/sync");
return syncGrokConfig(port, config, opts);
Expand Down Expand Up @@ -133,7 +133,8 @@ export async function ensureGrokFenceMatchesDesired(
port,
config,
{ ...(hostname !== undefined ? { hostname } : {}),
...(opts.probeHostname !== undefined ? { probeHostname: opts.probeHostname } : {}) },
...(opts.probeHostname !== undefined ? { probeHostname: opts.probeHostname } : {}),
admit: () => shouldSyncGrokOnStart(deps.loadConfig()) },
);
if (g.changed) log(" + Grok Build config updated (~/.grok/config.toml)");
else if (!g.ok) error(`⚠️ ${g.message}`);
Expand Down
5 changes: 4 additions & 1 deletion src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,10 @@ async function handleStart(options: { block?: boolean } = {}) {
// one restart, because the toggle removed the fence and start wrote it back.
if (shouldSyncGrokOnStart(config)) try {
const { syncGrokConfig } = await import("../grok/sync");
const r = await syncGrokConfig(port, config, config.hostname ? { hostname: config.hostname } : {});
const r = await syncGrokConfig(port, config, {
...(config.hostname ? { hostname: config.hostname } : {}),
admit: () => !shuttingDown && !cleaned && shouldSyncGrokOnStart(loadConfig()),
});
if (r.changed) console.log(" + Grok Build config updated (~/.grok/config.toml)");
else if (!r.ok) console.error(`⚠️ ${r.message}`);
} catch (err) {
Expand Down
11 changes: 10 additions & 1 deletion src/grok/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ async function defaultFetchAllModels(config: OcxConfig): Promise<CatalogModel[]>
export async function syncGrokConfig(
port: number,
config: OcxConfig,
opts: { hostname?: string; probeHostname?: string; grokHome?: string; refreshOnly?: { admit: () => boolean } } = {},
opts: { hostname?: string; probeHostname?: string; grokHome?: string; refreshOnly?: { admit: () => boolean };
/** Recheck caller-owned desired state/lifecycle after asynchronous catalog work. */
admit?: () => boolean } = {},
deps: GrokSyncDeps = { fetchAllModels: defaultFetchAllModels, injectGrokConfig },
): Promise<GrokInjectResult> {
const skipped = (): GrokInjectResult => ({
ok: true, changed: false, message: "Grok config sync skipped: caller admission is no longer current.",
});
if (opts.admit && !opts.admit()) return skipped();
let projection: ReturnType<typeof projectGrokCatalog>;
try {
const allRouted = await deps.fetchAllModels(config);
Expand All @@ -54,6 +60,9 @@ export async function syncGrokConfig(
unauthenticatedLoopbackListener: config.unauthenticatedLoopbackListener,
});
const targetUrl = new URL(target.baseUrl);
// Admission and the synchronous writer share one event-loop turn. A toggle or
// shutdown that landed while catalog discovery awaited must win over this snapshot.
if (opts.admit && !opts.admit()) return skipped();
return deps.injectGrokConfig(Number(targetUrl.port), projection.models, {
hostname: target.requiresAdmissionToken
? (opts.hostname ?? config.hostname)
Expand Down
2 changes: 1 addition & 1 deletion structure/runtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ uninstall still restore. `src/service/cli.ts` removes the service token on unins
`src/cli/tray-proxy.ts` coordinates `ocx restart` and tray restart. A second start requires proof the prior attempt never launched or its child exited, plus confirmed absence under a fresh bounded discovery window; a missing health response alone is insufficient. After an accepted restart, polling continues through uncertain observations until its deadline and requires a new runtime PID on the same port. If the old proxy then disappears, explicit recovery starts it regardless of the Codex autostart preference; a skipped recovery fails rather than reporting success. The Windows tray startup-health probe in `src/tray/windows-tray.ps1` requests termination at its 30-second deadline without waiting on the UI thread. It retains the process through later ticks until actual exit and both redirected readers settle; replacements cannot start while that reference is held. Launch setup failures follow the same cleanup. Tray shutdown alone waits at most 500 ms before disposal.

`ocx restart` also restarts a connected client runtime, which serves the same bound `POST /api/system/restart` from its client listener; the CLI sends that request only while the client's 90-second restart budget still fits before replacement observation ends (`CLIENT_RESTART_REQUEST_MIN_REMAINING_MS`), and reports a supervised client's `409 restart_supervised` as "not performed, runtime kept". See [Remote Link](remote-link.md) for the rollback.
The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. `src/cli/start-owner-fence.ts` holds the ownership mutation lease through recovery for ordinary and supervised starts. After an early negative probe, `start` and `ensure` recheck with `START_OWNERSHIP_LIVENESS` under that lease through `observeProxyLiveness` in `src/server/proxy-liveness.ts`, probing both available loopback families concurrently for every loopback or wildcard spelling classified after the same startup normalization (including `localhost.` and case variants) within the existing per-family retry ceilings and shared deadline. Only definitive absence permits PID deletion and journal reconciliation: no alive recorded PID unless fresh readable command-line evidence confirms it is foreign, no answering runtime owner, and only unanimous aggregate connection refusals or clean foreign identities at probed endpoints. Port zero names no existing listener. Timeouts, resets, unreadable/non-200 responses, PID mismatches and exhausted deadlines remain unknown; startup refuses with a fixed message while preserving PID, journal and shared config. Unreadable/empty PID command lines and any OpenCodex invocation remain unknown; a confirmed foreign PID is never signalled. A discovered live owner preserves both records and its runtime/config bind scope in `LiveProxy.hostname`; omitted or blank hostnames become explicit `127.0.0.1` in both early and recovery discovery, so a later config change cannot replace a default loopback bind. Explicit wildcard scope remains intact, and the concrete responding address is separate `probeHostname` metadata. Integration policy uses the bind scope, so loopback discovery of a wildcard listener does not authorize tokenless Grok registration. Ensure carries `probeHostname` separately through `src/cli/ensure-desired-integrations.ts` and `src/grok/sync.ts` to `src/grok/inject.ts` for the written destination; an explicit secondary loopback listener keeps its own address and port; after lease release, start applies `decideStartWithLiveOwner` and ensure follows its existing live-owner path. Cross-home sibling marking still precedes journal recovery. The bind/publication fence remains separate, and supervised recovery checks durable service ownership before liveness or mutation. `tests/cli/cli-start-journal-order.test.ts` covers an early miss; `tests/cli/cli-start-owner-absence.test.ts` covers consecutive misses and dead-owner recovery; `tests/server/proxy-liveness-recovery.test.ts` binds classification and existing budgets; `tests/server/proxy-liveness-bind-scope.test.ts` follows a missed early probe through desired-state reconciliation and the real Grok writer for runtime, orphan and configured-port discovery; `tests/server/proxy-liveness-owner-evidence.test.ts` and `tests/cli/cli-start-owner-evidence.test.ts` cover aggregate children, both listener families, shared deadlines and foreign-PID recovery.
The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. `src/cli/start-owner-fence.ts` holds the ownership mutation lease through recovery for ordinary and supervised starts. After an early negative probe, `start` and `ensure` recheck with `START_OWNERSHIP_LIVENESS` under that lease through `observeProxyLiveness` in `src/server/proxy-liveness.ts`, probing both available loopback families concurrently for every loopback or wildcard spelling classified after the same startup normalization (including `localhost.` and case variants) within the existing per-family retry ceilings and shared deadline. Only definitive absence permits PID deletion and journal reconciliation: no alive recorded PID unless fresh readable command-line evidence confirms it is foreign, no answering runtime owner, and only unanimous aggregate connection refusals or clean foreign identities at probed endpoints. Port zero names no existing listener. Timeouts, resets, unreadable/non-200 responses, PID mismatches and exhausted deadlines remain unknown; startup refuses with a fixed message while preserving PID, journal and shared config. Unreadable/empty PID command lines and any OpenCodex invocation remain unknown; a confirmed foreign PID is never signalled. A discovered live owner preserves both records and its runtime/config bind scope in `LiveProxy.hostname`; omitted or blank hostnames become explicit `127.0.0.1` in both early and recovery discovery, so a later config change cannot replace a default loopback bind. Explicit wildcard scope remains intact, and the concrete responding address is separate `probeHostname` metadata. Integration policy uses the bind scope, so loopback discovery of a wildcard listener does not authorize tokenless Grok registration. Grok start and ensure/restart synchronization also recheck current desired-state admission after catalog discovery, immediately before the synchronous writer in `src/grok/sync.ts`; start additionally rejects work after shutdown begins or cleanup runs. This closes the same-process await gap without turning the desired-state and Grok files into a cross-process transaction. Ensure carries `probeHostname` separately through `src/cli/ensure-desired-integrations.ts` and `src/grok/sync.ts` to `src/grok/inject.ts` for the written destination; an explicit secondary loopback listener keeps its own address and port; after lease release, start applies `decideStartWithLiveOwner` and ensure follows its existing live-owner path. Cross-home sibling marking still precedes journal recovery. The bind/publication fence remains separate, and supervised recovery checks durable service ownership before liveness or mutation. `tests/cli/cli-start-journal-order.test.ts` covers an early miss; `tests/cli/cli-start-owner-absence.test.ts` covers consecutive misses and dead-owner recovery; `tests/server/proxy-liveness-recovery.test.ts` binds classification and existing budgets; `tests/server/proxy-liveness-bind-scope.test.ts` follows a missed early probe through desired-state reconciliation and the real Grok writer for runtime, orphan and configured-port discovery; `tests/server/proxy-liveness-owner-evidence.test.ts` and `tests/cli/cli-start-owner-evidence.test.ts` cover aggregate children, both listener families, shared deadlines and foreign-PID recovery.

A busy preferred port is never resolved by starting somewhere else. Both questions a start asks
about an existing proxy — the pre-bind owner check and the port-is-busy check in `src/cli/index.ts`
Expand Down
19 changes: 19 additions & 0 deletions tests/cli/ensure-desired-integrations-race.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { describe, expect, test } from "bun:test";
import {
ensureGrokFenceMatchesDesired,
reconcileEnsureDesiredIntegrations,
type EnsureDesiredIntegrationsDeps,
} from "../../src/cli/ensure-desired-integrations";
import type { OcxConfig } from "../../src/types";
import type { GrokInjectResult } from "../../src/grok/inject";
import type { Desktop3pRemovalResult } from "../../src/claude/desktop-3p";
import { syncGrokConfig } from "../../src/grok/sync";
import type { CatalogModel } from "../../src/codex/catalog";

function config(overrides: {
grok?: boolean;
Expand Down Expand Up @@ -159,6 +162,22 @@ test("CLI intent with disabled intercept retains the env through Desktop-off cle
});

describe("ensure desired-state races", () => {
test("Grok disabled during catalog fetch cannot be reinjected by ensure", async () => {
const h = harness(config());
let finishCatalog!: (models: CatalogModel[]) => void;
const catalog = new Promise<CatalogModel[]>(resolve => { finishCatalog = resolve; });
let injectCalls = 0;
h.deps.syncGrokConfig = (port, current, opts) => syncGrokConfig(port, current, opts, {
fetchAllModels: () => catalog,
injectGrokConfig: () => { injectCalls++; return okGrok(true); },
});
const pending = ensureGrokFenceMatchesDesired(10100, {}, h.deps);
h.flip(config({ grok: false }));
finishCatalog([]);
await pending;
expect(injectCalls).toBe(0);
});

test("live-proxy OFF→ON uses the current ON snapshot instead of stripping", async () => {
const staleOff = config({ grok: false, desktop: false, hostname: "stale-host", fingerprint: "fp-off" });
const currentOn = config({ hostname: "fresh-host", fingerprint: "fp-on" });
Expand Down
Loading
Loading