Skip to content

Commit e4c3df5

Browse files
committed
fix(link): fence sync commits during signal shutdown
1 parent 165b689 commit e4c3df5

12 files changed

Lines changed: 273 additions & 24 deletions

‎src/client/connect.ts‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import {
1818
withClientLifecycle, withClientLifecycleSync,
1919
type ClientLifecycleHeld, type ClientLifecycleLockDeps,
2020
} from "./lifecycle-lock";
21-
import { invalidateCodexModelsCache } from "../codex/catalog/sync";
21+
import { invalidateCodexModelsCache, invalidateCodexModelsCacheWithPermit } from "../codex/catalog/sync";
22+
import { withCatalogWriteSerialization } from "../codex/catalog-write-serialization";
2223
import {
2324
injectCodexConfig,
2425
currentExternalCodexModelProvider,
@@ -30,7 +31,7 @@ import {
3031
journalOwner,
3132
restoreJournalState,
3233
} from "../codex/journal";
33-
import { DEFAULT_CATALOG_PATH } from "../codex/paths";
34+
import { DEFAULT_CATALOG_PATH, getCodexHome } from "../codex/paths";
3435
import {
3536
readServiceApiTokenState,
3637
readTokenBackupState,
@@ -95,6 +96,7 @@ import {
9596
import { hubStateCachePath } from "./hub-state";
9697
import { ClientLinkStateError, clearClientLinkState, readClientLinkState } from "./link-state";
9798
import { isLinkPort } from "../link/ports";
99+
import { clientSyncRuntimeGuard, ClientSyncRuntimeError } from "./sync-runtime";
98100

99101
class RotationRecoveryRequiredError extends Error {
100102
constructor(message: string, options?: ErrorOptions) {
@@ -901,7 +903,15 @@ async function syncConnectedClientUnlocked(
901903
if (state.kind !== "connected" || state.value.pendingOperation) throw new Error("client_sync_unavailable");
902904
const token = readServiceApiTokenState();
903905
if (token.kind !== "present" || token.fingerprint !== state.value.tokenFingerprint) throw new Error("client_token_changed");
904-
return { connection: state.value, token };
906+
let assertRuntimeCurrent = () => {};
907+
try { if (isLinkConnection(state.value)) assertRuntimeCurrent = clientSyncRuntimeGuard(); }
908+
catch (error) {
909+
if (options.expect && error instanceof ClientSyncRuntimeError && error.code === "client_runtime_stopping") {
910+
throw new ClientSyncExpectationError(options.expect, "pending_operation");
911+
}
912+
throw error;
913+
}
914+
return { connection: state.value, token, assertRuntimeCurrent };
905915
}), deps.lifecycleLockDeps);
906916
let downloaded: Awaited<ReturnType<typeof downloadClientCatalog>> | undefined;
907917
let stale = false;
@@ -918,6 +928,7 @@ async function syncConnectedClientUnlocked(
918928
// they were, which is the known-good state.
919929
if (downloaded) assertClientCatalogCompatible(downloaded.body, deps.catalogCompatibility);
920930
const next = withClientLifecycleSync(() => withConfigMutationLockSync(() => {
931+
initial.assertRuntimeCurrent();
921932
assertClientConnectionUnchanged(initial.connection);
922933
const token = readServiceApiTokenState();
923934
if (token.kind !== "present" || token.fingerprint !== initial.token.fingerprint) throw new Error("client_token_changed");
@@ -933,10 +944,12 @@ async function syncConnectedClientUnlocked(
933944
}), deps.lifecycleLockDeps);
934945
// Read-only: injection invokes this while N/C are held. Acquiring L here would invert C→L.
935946
const beforeClientWrite = () => {
947+
initial.assertRuntimeCurrent();
936948
assertClientConnectionUnchanged(next);
937949
const token = readServiceApiTokenState();
938950
if (next.pendingOperation || token.kind !== "present" || token.fingerprint !== next.tokenFingerprint) throw new Error("client_token_changed");
939951
};
952+
const withClientWrite = <T>(write: () => T): T => withConfigMutationLockSync(() => { beforeClientWrite(); return write(); });
940953
let injected = false;
941954
let homeSettings: HomeCodexSettingsSource | undefined;
942955
let appliedRecord: HomeCodexSettingsRecord | undefined;
@@ -945,12 +958,15 @@ async function syncConnectedClientUnlocked(
945958
let config = loadConfig();
946959
// #6957: a link Child applies the Home's OpenCodex-owned Codex settings with the catalog.
947960
if (isLinkConnection(next)) {
948-
homeSettings = await resolveHomeCodexSettings(next, initial.token.token, deps.linkHome);
961+
homeSettings = await resolveHomeCodexSettings(next, initial.token.token, { ...deps.linkHome, withWriteGuard: withClientWrite });
949962
if (homeSettings.source !== "local") config = overlayHomeCodexSettings(config, homeSettings.document.settings);
950963
// About to write different settings: until their record commits, report no applied digest
951964
// rather than the previous one (OCX-6). A failed injection then reads "unknown", never a
952965
// digest of values Codex no longer has or never got.
953-
if (homeSettings.source === "home" && next.link) withdrawChangingHomeCodexSettingsRecord(next.link.linkId, homeSettings.document);
966+
if (homeSettings.source === "home" && next.link) {
967+
const { linkId } = next.link, { document } = homeSettings;
968+
withClientWrite(() => withdrawChangingHomeCodexSettingsRecord(linkId, document));
969+
}
954970
}
955971
const result = await injectCodexConfig(config.port, { ...config, syncResumeHistory: false }, {
956972
routingTarget: routingTarget(next.serverUrl, isLinkConnection(next) ? config.port : undefined, config), catalogPath: DEFAULT_CATALOG_PATH,
@@ -962,23 +978,24 @@ async function syncConnectedClientUnlocked(
962978
const linkId = next.link.linkId;
963979
const settings = homeSettings;
964980
try {
965-
appliedRecord = withClientLifecycleSync(() => {
966-
beforeClientWrite();
967-
return recordHomeCodexSettings(linkId, settings, deps.now) ?? readHomeCodexSettingsRecord(linkId) ?? undefined;
968-
}, deps.lifecycleLockDeps);
981+
appliedRecord = withClientLifecycleSync(() => withClientWrite(() =>
982+
recordHomeCodexSettings(linkId, settings, deps.now) ?? readHomeCodexSettingsRecord(linkId) ?? undefined), deps.lifecycleLockDeps);
969983
} catch (error) {
970984
// An ownership change is not a disk problem: report it as itself.
971-
if (error instanceof Error && (error.message === "client_token_changed" || error.message === "client_connection_changed")) throw error;
985+
if (error instanceof ClientSyncRuntimeError || (error instanceof Error && (error.message === "client_token_changed" || error.message === "client_connection_changed"))) throw error;
972986
// The injection committed, so the cache refresh below still runs; the caller reports this.
973-
try { clearHomeCodexSettingsRecord(); } catch { /* status already reads no recorded apply or reports the write */ }
987+
try { withClientWrite(clearHomeCodexSettingsRecord); } catch { /* status already reads no recorded apply or reports the write */ }
974988
applyRecordFailed = true;
975989
}
976990
}
977991
}
978992
const cacheSynced = withClientLifecycleSync(() => {
979-
beforeClientWrite();
980-
// Cache invalidation acquires K itself (N -> K -> C); never call it while C is held.
981-
return invalidateCodexModelsCache({ allowWhenDesiredDisabled: true });
993+
// Acquire K before the guarded C commit; never acquire K from inside C.
994+
const owningHome = getCodexHome();
995+
const result = withCatalogWriteSerialization(owningHome, permit => withClientWrite(() =>
996+
invalidateCodexModelsCacheWithPermit(permit, owningHome, { allowWhenDesiredDisabled: true })),
997+
{ intent: "cache", writer: "cache-invalidate" });
998+
return result.kind === "completed" && result.value;
982999
}, deps.lifecycleLockDeps);
9831000
return {
9841001
catalogWritten: downloaded !== undefined, cacheSynced, injected, stale,

‎src/client/link-home.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ export type HomeCodexSettingsSource =
299299
export async function resolveHomeCodexSettings(
300300
connection: LinkHomeConnection,
301301
admissionKey: string,
302-
deps: LinkHomeDeps & { now?: () => Date } = {},
302+
deps: LinkHomeDeps & { now?: () => Date; withWriteGuard?: <T>(write: () => T) => T } = {},
303303
): Promise<HomeCodexSettingsSource> {
304304
const linkId = connection.link?.linkId;
305305
if (!linkId) return { source: "local", reason: "not a Remote Link Child" };
@@ -308,7 +308,8 @@ export async function resolveHomeCodexSettings(
308308
return { source: "home", document: fetched.document, fetchedAt: (deps.now ?? (() => new Date()))().toISOString() };
309309
}
310310
if (fetched.kind === "unsupported") {
311-
clearHomeCodexSettingsRecord();
311+
if (deps.withWriteGuard) deps.withWriteGuard(clearHomeCodexSettingsRecord);
312+
else clearHomeCodexSettingsRecord();
312313
return { source: "local", reason: "the Home does not publish Codex settings; upgrade the Home" };
313314
}
314315
const cached = readHomeCodexSettingsRecord(linkId);

‎src/client/runtime.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { withClientLifecycleSync } from "./lifecycle-lock";
3232
import { clientRestartRollbackAction, clientRestartSupervisor, createClientRestartController, type ClientRestartController } from "./runtime-restart";
3333
import { createLinkParityReporter, type LinkParityReporter } from "./link-parity-reporter";
3434
import { restoreLinkClientRouting } from "./stop-routing";
35+
import { markClientRuntimeStopping } from "./sync-runtime";
3536

3637
let activeServer: Server<unknown> | null = null;
3738
let activePort: number | null = null;
@@ -369,7 +370,7 @@ export async function startClientRuntime(
369370
supervisor?.start();
370371
credentialObservation ??= observeLocalCredentials();
371372
writePid(process.pid);
372-
writeRuntimePort(clientRuntimeRecord(process.pid, boundPort, attestationSecret));
373+
writeRuntimePort({ ...clientRuntimeRecord(process.pid, boundPort, attestationSecret), ...(clientStopping ? { clientStopping: true as const } : {}) });
373374
if (linkMode && state.value.link && linkKey) {
374375
activeReporter = createLinkParityReporter({ connection: state.value, linkKey, runtimeStartedAt: CLIENT_RUNTIME_STARTED_AT });
375376
activeReporter.start();
@@ -466,11 +467,19 @@ export async function startClientRuntime(
466467
shuttingDown = true;
467468
clientStopping = true;
468469
if (recycleTimer) { clearTimeout(recycleTimer); recycleTimer = undefined; }
470+
const fenceSync = () => {
471+
if (!linkMode) return true;
472+
try { markClientRuntimeStopping({ pid: process.pid, port: servedPort, attestationSecret }); return true; }
473+
catch { return false; }
474+
};
475+
let syncFenced = fenceSync();
469476
void (async () => {
470477
// An accepted restart owns its replacement and rollback. A successful handoff exits
471478
// inside that flight; a rollback finishes before this signal tears the runtime down.
472479
if (restartController.hasHandoff()) await restartController.settled().catch(() => {});
473480
if (recycleFlight) await recycleFlight.catch(() => {});
481+
// A rollback may have republished its runtime record while shutdown joined it.
482+
syncFenced = fenceSync();
474483
credentialObservation?.stop();
475484
credentialObservation = null;
476485
activeReporter?.stop();
@@ -481,15 +490,15 @@ export async function startClientRuntime(
481490
console.warn(`[client] link supervisor stop failed: ${error instanceof Error ? error.message : String(error)}`);
482491
}
483492
activeSupervisor = null;
484-
const restored = linkMode ? restoreLinkClientRouting(state.value) : null;
493+
const restored = linkMode ? restoreLinkClientRouting(state.value, { syncFenced }) : null;
485494
if (restored && !restored.success) console.error(`[client] ${restored.message}`);
486495
try {
487496
activeServer?.stop(true);
488497
} catch (error) {
489498
console.warn(`[client] listener stop failed: ${error instanceof Error ? error.message : String(error)}`);
490499
} finally {
491-
cleanup();
492-
await flushLinkDiagnostics();
500+
// Keep stopping admission visible through the last await. Cleanup and exit are adjacent.
501+
try { await flushLinkDiagnostics(); } finally { cleanup(); }
493502
// launchd relaunches only unsuccessful exits; an external signal must still count.
494503
if (restored?.success === false) process.exit(1);
495504
else process.exit(handledSignalExitCode(signal));

‎src/client/stop-routing.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@ const refusal = (reason: string): LinkRoutingRestoreResult => ({
2222
});
2323

2424
/** Unsupervised signal cleanup; explicit parent stop and restart retain their own restoration. */
25-
export function restoreLinkClientRouting(expected: OcxClientConnectionConfig): LinkRoutingRestoreResult {
25+
export function restoreLinkClientRouting(expected: OcxClientConnectionConfig, options: { syncFenced?: boolean } = {}): LinkRoutingRestoreResult {
2626
if (siblingOfLivePort() !== null) return { success: true, restored: false, message: siblingSkipMessage() };
2727
if (!isLinkConnection(expected)) {
2828
return { success: true, restored: false, message: "Hub connection-owned integrations preserved." };
2929
}
3030
const preserve = () => process.env.OCX_SERVICE === "1" || isDesktopSupervised()
3131
|| pendingTeardownOutstanding();
3232
if (preserve()) return { success: true, restored: false, message: "Supervised restart or pending teardown owns the client integrations." };
33+
if (options.syncFenced === false) return refusal("the runtime sync fence could not be verified");
3334
const guard = stopRestoreWriteGuard(expected);
3435
const withWriteGuard = <T>(write: () => T): T => guard.run(() => {
3536
if (preserve()) throw new Error("restart or teardown ownership changed");

‎src/client/sync-runtime.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/** Cross-process admission for Link sync while its local runtime shuts down. */
2+
import { lstatSync } from "node:fs";
3+
import { withConfigMutationLockSync } from "../config";
4+
import { getRuntimePortPath, isValidRuntimePortState, writeRuntimePort, type RuntimePortState } from "../config/process-state";
5+
import { readBoundedRegularFileSync } from "../lib/bounded-file-read";
6+
7+
export class ClientSyncRuntimeError extends Error {
8+
constructor(readonly code: "client_runtime_stopping" | "client_runtime_changed" | "client_runtime_unverified") {
9+
super(`${code}: retry sync after the Child has started; ocx start/stop can reconcile a stale runtime record`);
10+
}
11+
}
12+
13+
function readSyncRuntime(): RuntimePortState | null {
14+
const path = getRuntimePortPath();
15+
try { lstatSync(path); }
16+
catch (error) {
17+
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
18+
throw new ClientSyncRuntimeError("client_runtime_unverified");
19+
}
20+
try {
21+
const value: unknown = JSON.parse(readBoundedRegularFileSync(path, 16 * 1024).toString("utf8"));
22+
if (!isValidRuntimePortState(value)) throw new Error("invalid runtime record");
23+
return value;
24+
} catch { throw new ClientSyncRuntimeError("client_runtime_unverified"); }
25+
}
26+
27+
/** Capture and recheck under C. Proven no-runtime sync keeps its existing offline behavior. */
28+
export function clientSyncRuntimeGuard(): () => void {
29+
const expected = readSyncRuntime();
30+
if (expected?.clientStopping) throw new ClientSyncRuntimeError("client_runtime_stopping");
31+
return () => {
32+
const current = readSyncRuntime();
33+
if (current?.clientStopping) throw new ClientSyncRuntimeError("client_runtime_stopping");
34+
if (!Bun.deepEquals(current, expected, true)) throw new ClientSyncRuntimeError("client_runtime_changed");
35+
};
36+
}
37+
38+
/** The same C transaction excludes every sync commit; never mark a replacement runtime. */
39+
export function markClientRuntimeStopping(expected: Required<Pick<RuntimePortState, "pid" | "port" | "attestationSecret">>): void {
40+
withConfigMutationLockSync(() => {
41+
const current = readSyncRuntime();
42+
if (!current || current.pid !== expected.pid || current.port !== expected.port
43+
|| current.attestationSecret !== expected.attestationSecret) {
44+
throw new ClientSyncRuntimeError("client_runtime_changed");
45+
}
46+
if (!current.clientStopping) writeRuntimePort({ ...current, clientStopping: true });
47+
});
48+
}

‎src/config/process-state.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export type RuntimePortState = {
3737
hostname?: string;
3838
/** Per-process proof key; protected by the config directory and never served. */
3939
attestationSecret?: string;
40+
/** A Link runtime is draining; new and in-flight sync commits must stand down. */
41+
clientStopping?: true;
4042
/**
4143
* The live proxy's port when this runtime is a sibling instance started beside it
4244
* (`src/codex/sibling-start.ts`). `ocx stop` reads it to leave shared client routing alone.
@@ -59,6 +61,7 @@ export function isValidRuntimePortState(value: unknown): value is RuntimePortSta
5961
&& Number(state.port) <= 65535
6062
&& hostnameOk
6163
&& attestationOk
64+
&& (state.clientStopping === undefined || state.clientStopping === true)
6265
&& siblingOk;
6366
}
6467

‎structure/remote-link.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Remote Link
22

3+
`src/client/sync-runtime.ts` fences Link sync against the local runtime record. Signal shutdown marks its own PID/port/attestation generation as `clientStopping` under C before restoration, and retains that mark through its last await. Sync captures the record at admission and rechecks it under C before catalog, injection, settings-record and cache commits. A stopping, replaced, removed or unreadable record refuses the write; a stale operation cannot reinject after the Child exits. Failure to mark the runtime refuses restoration without overriding service, Desktop, sibling or parent-receipt preservation. No-runtime sync remains an explicit offline operation; after a normal stop, start the Child before syncing its routing. A stale stopping record is reconciled by the existing start/stop recovery paths, not by guessing PID ownership. The separate-process catalog/settings-response holds in `tests/clients/client-link-stop.test.ts` cover late commits, fresh sync during shutdown and the completed-sync control.
4+
35
Orderly signals in an unsupervised Link Child call `src/client/stop-routing.ts` before releasing the listener. It reuses native Codex config/catalog/history restoration and managed Grok removal; the saved connection and credentials remain. `src/client/restore-ownership.ts` optionally binds each write under C to the immutable served connection, including selection and rotation state. Only `catalogFingerprint` and `catalogSyncedAt`, updated by normal catalog sync, are excluded from this restore-specific comparison; `priorCatalog` remains guarded and the shared full-snapshot CAS for asynchronous connection work is unchanged. A changed owner, pending transition or unverified journal refuses remaining writes. Service/desktop supervision, sibling ownership, accepted restart/recycle handoff and outstanding parent teardown receipts preserve integrations for their existing owners. Signal shutdown waits for an accepted restart to hand off or finish rolling back; a pending admission owns no routing. It refuses new or pending restart admission with `409 runtime_stopping`; normal bound restart remains unchanged. After an ordinary unsupervised stop, start and run `ocx sync --restart-codex`; start alone does not reapply routing. Coverage: `tests/clients/client-link-stop.test.ts`, `tests/clients/client-runtime-restart.test.ts`.
46

57
`src/link/` owns the building blocks for linking OpenCodex machines over SSH. The pure modules start no process, open no socket and schedule no timer on import.

0 commit comments

Comments
 (0)