fix(codex): prevent startup hangs after binding migration (#103281)

* fix(codex): archive imported orphan binding sidecars

* fix(codex): harden binding sidecar migration

Co-authored-by: Benjamin Badejo <ben@benbadejo.com>

* chore: keep release notes out of contributor PRs

---------

Co-authored-by: Benjamin Badejo <ben@benbadejo.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Ben Badejo
2026-07-10 12:38:35 +03:00
committed by GitHub
parent 7eacd78c01
commit 1f3ea6faaa
3 changed files with 787 additions and 193 deletions
+529 -99
View File
@@ -20,21 +20,104 @@ import {
bindingStoreKey,
CODEX_APP_SERVER_BINDING_MAX_ENTRIES,
CODEX_APP_SERVER_BINDING_NAMESPACE,
createStoredCodexAppServerBinding,
type StoredCodexAppServerBinding,
} from "./src/app-server/session-binding.js";
import { legacyCodexConversationBindingId } from "./src/conversation-binding-data.js";
function createDoctorContext(env: NodeJS.ProcessEnv): PluginDoctorStateMigrationContext {
function createDoctorContext(
env: NodeJS.ProcessEnv,
afterRegister?: () => Promise<void>,
): PluginDoctorStateMigrationContext {
return {
openPluginStateKeyedStore<T>(options: OpenKeyedStoreOptions) {
return createPluginStateKeyedStoreForTests<T>("codex", {
const store = createPluginStateKeyedStoreForTests<T>("codex", {
...options,
env: options.env ?? env,
});
return afterRegister
? {
...store,
async registerIfAbsent(...args: Parameters<typeof store.registerIfAbsent>) {
const registered = await store.registerIfAbsent(...args);
await afterRegister();
return registered;
},
}
: store;
},
};
}
function openBindingStore(env: NodeJS.ProcessEnv) {
return createDoctorContext(env).openPluginStateKeyedStore<StoredCodexAppServerBinding>({
namespace: CODEX_APP_SERVER_BINDING_NAMESPACE,
maxEntries: CODEX_APP_SERVER_BINDING_MAX_ENTRIES,
overflowPolicy: "reject-new",
});
}
async function createBindingMigrationFixture(options: {
binding?: Record<string, unknown>;
name: string;
sessionIndex?: Record<string, unknown>;
threadId: string;
}) {
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-doctor-"));
const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir };
const sessionsDir = path.join(stateDir, "agents", "main", "sessions");
const transcriptPath = path.join(sessionsDir, `${options.name}.jsonl`);
const sidecarPath = `${transcriptPath}.codex-app-server.json`;
await fs.mkdir(sessionsDir, { recursive: true });
await fs.writeFile(
transcriptPath,
`${JSON.stringify({ type: "session", id: options.name })}\n`,
"utf8",
);
if (options.sessionIndex !== undefined) {
await fs.writeFile(
path.join(sessionsDir, "sessions.json"),
JSON.stringify(options.sessionIndex),
"utf8",
);
}
await fs.writeFile(
sidecarPath,
JSON.stringify({
schemaVersion: 2,
threadId: options.threadId,
sessionFile: transcriptPath,
updatedAt: "2026-01-01T00:00:00.000Z",
pluginAppPolicyContext: {
fingerprint: "policy-1",
apps: {},
pluginAppIds: {},
},
...options.binding,
}),
"utf8",
);
const migration = stateMigrations[0];
if (!migration) {
throw new Error("missing Codex binding migration");
}
return {
env,
migration,
params: {
config: {},
env,
stateDir,
oauthDir: path.join(stateDir, "oauth"),
context: createDoctorContext(env),
},
sessionsDir,
sidecarPath,
stateDir,
transcriptPath,
};
}
afterEach(() => {
resetPluginStateStoreForTests();
});
@@ -127,31 +210,17 @@ describe("codex doctor contract", () => {
});
it("imports and archives shipped binding sidecars", async () => {
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-doctor-"));
const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir };
const sessionsDir = path.join(stateDir, "agents", "main", "sessions");
const transcriptPath = path.join(sessionsDir, "session-current.jsonl");
const sidecarPath = `${transcriptPath}.codex-app-server.json`;
await fs.mkdir(sessionsDir, { recursive: true });
await fs.writeFile(transcriptPath, '{"type":"session","id":"session-current"}\n', "utf8");
await fs.writeFile(
path.join(sessionsDir, "sessions.json"),
JSON.stringify({
const fixture = await createBindingMigrationFixture({
name: "session-current",
sessionIndex: {
"agent:main:session-1": {
sessionId: "session-current",
sessionFile: "session-current.jsonl",
updatedAt: Date.now(),
updatedAt: 1,
},
}),
"utf8",
);
await fs.writeFile(
sidecarPath,
JSON.stringify({
schemaVersion: 2,
threadId: "thread-1",
sessionFile: transcriptPath,
updatedAt: "2026-01-01T00:00:00.000Z",
},
threadId: "thread-1",
binding: {
pluginAppPolicyContext: {
fingerprint: "policy-1",
apps: {
@@ -166,34 +235,18 @@ describe("codex doctor contract", () => {
},
pluginAppIds: {},
},
}),
"utf8",
);
const params = {
config: {},
env,
stateDir,
oauthDir: path.join(stateDir, "oauth"),
context: createDoctorContext(env),
};
const migration = stateMigrations[0];
if (!migration) {
throw new Error("missing Codex binding migration");
}
},
});
await expect(migration.detectLegacyState(params)).resolves.toMatchObject({
await expect(fixture.migration.detectLegacyState(fixture.params)).resolves.toMatchObject({
preview: [expect.stringContaining("legacy sidecar")],
});
await expect(migration.migrateLegacyState(params)).resolves.toMatchObject({
await expect(fixture.migration.migrateLegacyState(fixture.params)).resolves.toMatchObject({
changes: [expect.stringContaining("Migrated 1")],
warnings: [],
});
const store = createDoctorContext(env).openPluginStateKeyedStore<StoredCodexAppServerBinding>({
namespace: CODEX_APP_SERVER_BINDING_NAMESPACE,
maxEntries: CODEX_APP_SERVER_BINDING_MAX_ENTRIES,
overflowPolicy: "reject-new",
});
const store = openBindingStore(fixture.env);
await expect(
store.lookup(
bindingStoreKey({
@@ -217,98 +270,472 @@ describe("codex doctor contract", () => {
store.lookup(
bindingStoreKey({
kind: "conversation",
bindingId: legacyCodexConversationBindingId(transcriptPath),
bindingId: legacyCodexConversationBindingId(fixture.transcriptPath),
}),
),
).resolves.toMatchObject({ state: "active", binding: { threadId: "thread-1" } });
await expect(fs.access(`${sidecarPath}.migrated`)).resolves.toBeUndefined();
await expect(fs.access(`${fixture.sidecarPath}.migrated`)).resolves.toBeUndefined();
await expect(
fs.readFile(path.join(sessionsDir, "sessions.json"), "utf8").then(JSON.parse),
fs.readFile(path.join(fixture.sessionsDir, "sessions.json"), "utf8").then(JSON.parse),
).resolves.toMatchObject({
"agent:main:session-1": { sessionId: "session-current", agentHarnessId: "codex" },
});
await fs.rm(stateDir, { recursive: true, force: true });
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it("reports unresolved-owner binding sidecars as notices after importing conversation binding", async () => {
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-doctor-"));
const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir };
const sessionsDir = path.join(stateDir, "agents", "main", "sessions");
const transcriptPath = path.join(sessionsDir, "orphan.jsonl");
const sidecarPath = `${transcriptPath}.codex-app-server.json`;
await fs.mkdir(sessionsDir, { recursive: true });
it("matches an owner through the contained fallback for a stale session file locator", async () => {
const sessionKey = "agent:main:stale-locator";
const fixture = await createBindingMigrationFixture({
name: "stale-locator",
sessionIndex: {
[sessionKey]: {
sessionId: "stale-locator",
sessionFile: "../outside.jsonl",
},
},
threadId: "thread-stale-locator",
});
const result = await fixture.migration.migrateLegacyState(fixture.params);
expect(result.warnings).toEqual([]);
await expect(fs.access(`${fixture.sidecarPath}.migrated`)).resolves.toBeUndefined();
await expect(
fs.readFile(path.join(fixture.sessionsDir, "sessions.json"), "utf8").then(JSON.parse),
).resolves.toMatchObject({ [sessionKey]: { agentHarnessId: "codex" } });
await expect(
openBindingStore(fixture.env).lookup(
bindingStoreKey({
kind: "session",
agentId: "main",
sessionId: "stale-locator",
sessionKey,
}),
),
).resolves.toMatchObject({ state: "active", binding: { threadId: "thread-stale-locator" } });
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it("deduplicates session-store aliases before classifying binding ownership", async () => {
const fixture = await createBindingMigrationFixture({
name: "aliased-store",
sessionIndex: {
"agent:main:aliased-store": {
sessionId: "aliased-store",
sessionFile: "aliased-store.jsonl",
},
},
threadId: "thread-aliased-store",
});
await fs.writeFile(
sidecarPath,
path.join(fixture.sessionsDir, "sessions.json"),
JSON.stringify({
schemaVersion: 2,
threadId: "thread-orphan",
sessionFile: transcriptPath,
updatedAt: "2026-01-01T00:00:00.000Z",
pluginAppPolicyContext: {
fingerprint: "policy-1",
apps: {},
pluginAppIds: {},
"agent:main:aliased-store": {
sessionId: "aliased-store",
sessionFile: fixture.transcriptPath,
},
}),
"utf8",
);
const params = {
config: {},
env,
stateDir,
oauthDir: path.join(stateDir, "oauth"),
context: createDoctorContext(env),
};
const migration = stateMigrations[0];
if (!migration) {
throw new Error("missing Codex binding migration");
}
const storeAlias = path.join(fixture.stateDir, "sessions-alias.json");
await fs.symlink(path.join(fixture.sessionsDir, "sessions.json"), storeAlias);
const result = await migration.migrateLegacyState(params);
const canonicalSidecarPath = await fs.realpath(sidecarPath);
const result = await fixture.migration.migrateLegacyState({
...fixture.params,
config: { session: { store: storeAlias } },
});
expect(result.warnings).toStrictEqual([]);
expect(result.notices).toStrictEqual([
`Left Codex binding sidecar in place after importing its conversation binding because its session owner could not be resolved: ${canonicalSidecarPath}`,
]);
expect(result.changes).toContain(
"Migrated 1 safe Codex app-server binding row(s) to plugin state; retained legacy sidecars needing review",
expect(result.warnings).toEqual([]);
await expect(fs.access(`${fixture.sidecarPath}.migrated`)).resolves.toBeUndefined();
const configuredIndex = JSON.parse(await fs.readFile(storeAlias, "utf8")) as Record<
string,
Record<string, unknown>
>;
const targetIndex = JSON.parse(
await fs.readFile(path.join(fixture.sessionsDir, "sessions.json"), "utf8"),
) as Record<string, Record<string, unknown>>;
expect(configuredIndex["agent:main:aliased-store"]).toMatchObject({
agentHarnessId: "codex",
});
expect(targetIndex["agent:main:aliased-store"]).not.toHaveProperty("agentHarnessId");
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it("resolves relative session files from a symlinked store path", async () => {
const sessionKey = "agent:main:symlinked-store";
const fixture = await createBindingMigrationFixture({
name: "symlinked-store",
sessionIndex: {
[sessionKey]: {
sessionId: "symlinked-store",
sessionFile: "symlinked-store.jsonl",
},
},
threadId: "thread-symlinked-store",
});
const configuredDir = path.join(fixture.stateDir, "configured-sessions");
const configuredStore = path.join(configuredDir, "sessions.json");
const configuredTranscript = path.join(configuredDir, "symlinked-store.jsonl");
const configuredSidecar = `${configuredTranscript}.codex-app-server.json`;
await fs.mkdir(configuredDir, { recursive: true });
await fs.rename(fixture.transcriptPath, configuredTranscript);
await fs.rename(fixture.sidecarPath, configuredSidecar);
const sidecar = JSON.parse(await fs.readFile(configuredSidecar, "utf8")) as Record<
string,
unknown
>;
await fs.writeFile(
configuredSidecar,
JSON.stringify({ ...sidecar, sessionFile: configuredTranscript }),
"utf8",
);
await expect(fs.access(sidecarPath)).resolves.toBeUndefined();
const store = createDoctorContext(env).openPluginStateKeyedStore<StoredCodexAppServerBinding>({
await fs.symlink(path.join(fixture.sessionsDir, "sessions.json"), configuredStore);
const result = await fixture.migration.migrateLegacyState({
...fixture.params,
config: { session: { store: configuredStore } },
});
expect(result.warnings).toEqual([]);
await expect(fs.access(`${configuredSidecar}.migrated`)).resolves.toBeUndefined();
await expect(fs.readFile(configuredStore, "utf8").then(JSON.parse)).resolves.toMatchObject({
[sessionKey]: { agentHarnessId: "codex" },
});
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it.each([
{ label: "new", preexisting: false },
{ label: "pre-existing", preexisting: true },
])(
"retires a $label session row when its owner rebinds during migration",
async ({ preexisting }) => {
const sessionKey = "agent:main:session-1";
const fixture = await createBindingMigrationFixture({
name: "session-current",
sessionIndex: {
[sessionKey]: {
sessionId: "session-current",
sessionFile: "session-current.jsonl",
lifecycleRevision: "rev-1",
},
},
threadId: "thread-1",
});
const sessionBindingKey = bindingStoreKey({
kind: "session",
agentId: "main",
sessionId: "session-current",
sessionKey,
});
const imported = createStoredCodexAppServerBinding(
JSON.parse(await fs.readFile(fixture.sidecarPath, "utf8")),
);
if (!imported) {
throw new Error("missing imported Codex binding");
}
const store = openBindingStore(fixture.env);
if (preexisting) {
await store.register(sessionBindingKey, { ...imported, sessionId: "session-current" });
}
let rebound = false;
const context = createDoctorContext(fixture.env, async () => {
if (rebound) {
return;
}
rebound = true;
await fs.writeFile(
path.join(fixture.sessionsDir, "sessions.json"),
JSON.stringify({
[sessionKey]: {
sessionId: "session-current",
sessionFile: "replacement.jsonl",
lifecycleRevision: "rev-2",
},
}),
);
});
const result = await fixture.migration.migrateLegacyState({ ...fixture.params, context });
expect(result.warnings).toEqual([
expect.stringContaining("session owner changed before Codex ownership could be recorded"),
]);
await expect(fs.access(fixture.sidecarPath)).resolves.toBeUndefined();
await expect(fs.access(`${fixture.sidecarPath}.migrated`)).rejects.toThrow();
await expect(
fs.readFile(path.join(fixture.sessionsDir, "sessions.json"), "utf8").then(JSON.parse),
).resolves.not.toHaveProperty(`${sessionKey}.agentHarnessId`);
await expect(store.lookup(sessionBindingKey)).resolves.toMatchObject({
version: 1,
state: "cleared",
sessionId: "session-current",
retired: true,
});
await fs.rm(fixture.stateDir, { recursive: true, force: true });
},
);
it("does not resurrect a retired session generation from its legacy sidecar", async () => {
const sessionKey = "agent:main:retired";
const fixture = await createBindingMigrationFixture({
name: "retired",
sessionIndex: {
[sessionKey]: {
sessionId: "retired",
sessionFile: "retired.jsonl",
},
},
threadId: "thread-retired",
});
const store = openBindingStore(fixture.env);
const active = createStoredCodexAppServerBinding(
JSON.parse(await fs.readFile(fixture.sidecarPath, "utf8")),
);
if (!active) {
throw new Error("missing imported Codex binding");
}
await store.register(
bindingStoreKey({
kind: "conversation",
bindingId: legacyCodexConversationBindingId(fixture.transcriptPath),
}),
active,
);
const sessionBindingKey = bindingStoreKey({
kind: "session",
agentId: "main",
sessionId: "retired",
sessionKey,
});
const retired: StoredCodexAppServerBinding = {
version: 1,
state: "cleared",
sessionId: "retired",
retired: true,
};
await store.register(sessionBindingKey, retired);
const result = await fixture.migration.migrateLegacyState(fixture.params);
expect(result.changes).toEqual([]);
expect(result.warnings).toEqual([
expect.stringContaining(`canonical plugin state changed at ${sessionBindingKey}`),
]);
await expect(fs.access(fixture.sidecarPath)).resolves.toBeUndefined();
await expect(store.lookup(sessionBindingKey)).resolves.toEqual(retired);
await expect(
fs.readFile(path.join(fixture.sessionsDir, "sessions.json"), "utf8").then(JSON.parse),
).resolves.not.toHaveProperty(`${sessionKey}.agentHarnessId`);
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it.each(["active", "cleared"] as const)(
"archives zero-owner sidecars without changing imported $state conversation state",
async (state) => {
const fixture = await createBindingMigrationFixture({
name: `orphan-${state}`,
threadId: "thread-orphan",
});
const bindingKey = bindingStoreKey({
kind: "conversation",
bindingId: legacyCodexConversationBindingId(fixture.transcriptPath),
});
const active = createStoredCodexAppServerBinding(
JSON.parse(await fs.readFile(fixture.sidecarPath, "utf8")),
);
if (!active) {
throw new Error("missing imported Codex binding");
}
const existing: StoredCodexAppServerBinding =
state === "active" ? active : { version: 1, state: "cleared", retired: true };
const store = openBindingStore(fixture.env);
await store.register(bindingKey, existing);
await expect(fixture.migration.migrateLegacyState(fixture.params)).resolves.toEqual({
changes: [
"Migrated 1 Codex app-server binding sidecar(s) to plugin state and archived the legacy sources",
],
warnings: [],
});
await expect(fs.access(fixture.sidecarPath)).rejects.toThrow();
await expect(fs.access(`${fixture.sidecarPath}.migrated`)).resolves.toBeUndefined();
await expect(store.lookup(bindingKey)).resolves.toEqual(existing);
await expect(fixture.migration.detectLegacyState(fixture.params)).resolves.toBeNull();
await expect(fixture.migration.migrateLegacyState(fixture.params)).resolves.toEqual({
changes: [],
warnings: [],
});
await fs.rm(fixture.stateDir, { recursive: true, force: true });
},
);
it("retains a zero-owner sidecar when canonical plugin state is malformed", async () => {
const fixture = await createBindingMigrationFixture({
name: "orphan-invalid-state",
threadId: "thread-orphan",
});
const bindingKey = bindingStoreKey({
kind: "conversation",
bindingId: legacyCodexConversationBindingId(fixture.transcriptPath),
});
const store = createDoctorContext(fixture.env).openPluginStateKeyedStore<unknown>({
namespace: CODEX_APP_SERVER_BINDING_NAMESPACE,
maxEntries: CODEX_APP_SERVER_BINDING_MAX_ENTRIES,
overflowPolicy: "reject-new",
});
await expect(
store.lookup(
bindingStoreKey({
kind: "conversation",
bindingId: legacyCodexConversationBindingId(transcriptPath),
}),
),
).resolves.toMatchObject({ state: "active", binding: { threadId: "thread-orphan" } });
const malformed = { version: 1, state: "active" };
await store.register(bindingKey, malformed);
await fs.rm(stateDir, { recursive: true, force: true });
const result = await fixture.migration.migrateLegacyState(fixture.params);
expect(result.changes).toEqual([]);
expect(result.warnings).toEqual([
expect.stringContaining(`canonical plugin state is invalid at ${bindingKey}`),
]);
await expect(fs.access(fixture.sidecarPath)).resolves.toBeUndefined();
await expect(store.lookup(bindingKey)).resolves.toEqual(malformed);
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it("does not scan above stateDir when a session store sits at its parent", async () => {
it("retains mixed Codex and foreign ambiguous binding owners", async () => {
const fixture = await createBindingMigrationFixture({
name: "shared",
sessionIndex: {
"agent:main:first": {
sessionId: "first",
sessionFile: "shared.jsonl",
agentHarnessId: "codex",
},
"agent:main:second": {
sessionId: "second",
sessionFile: "shared.jsonl",
agentHarnessId: "pi",
},
},
threadId: "thread-shared",
});
const result = await fixture.migration.migrateLegacyState(fixture.params);
expect(result.changes).toEqual([]);
expect(result.warnings).toHaveLength(1);
expect(result.warnings[0]).toContain("2 matching session owners make ownership ambiguous");
await expect(fs.access(fixture.sidecarPath)).resolves.toBeUndefined();
await expect(openBindingStore(fixture.env).entries()).resolves.toEqual([]);
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it("retains a sidecar owned by a foreign harness without importing plugin state", async () => {
const fixture = await createBindingMigrationFixture({
name: "foreign",
sessionIndex: {
"agent:main:foreign": {
sessionId: "foreign",
sessionFile: "foreign.jsonl",
agentHarnessId: "pi",
},
},
threadId: "thread-foreign",
});
const result = await fixture.migration.migrateLegacyState(fixture.params);
expect(result.changes).toEqual([]);
expect(result.warnings).toEqual([expect.stringContaining("owned by agent harness pi")]);
await expect(fs.access(fixture.sidecarPath)).resolves.toBeUndefined();
await expect(openBindingStore(fixture.env).entries()).resolves.toEqual([]);
await fs.rm(fixture.stateDir, { recursive: true, force: true });
});
it.each([
{ contents: "{", detail: "invalid JSON", label: "invalid JSON" },
{
contents: JSON.stringify({
"agent:main:invalid": { sessionId: "invalid", agentHarnessId: 42 },
}),
detail: "invalid entries",
label: "malformed harness metadata",
},
{
contents: JSON.stringify({
"agent:main:unsafe": { sessionId: "../unsafe", sessionFile: "unsafe.jsonl" },
}),
detail: "invalid entries",
label: "unsafe session id",
},
])("retains binding sidecars for an indeterminate $label index", async ({ contents, detail }) => {
const fixture = await createBindingMigrationFixture({
name: "unknown-owner",
threadId: "thread-unknown-owner",
});
const externalDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-store-"));
const externalStore = path.join(externalDir, "sessions.json");
await fs.writeFile(externalStore, contents, "utf8");
const params = {
...fixture.params,
config: { session: { store: externalStore } },
};
const result = await fixture.migration.migrateLegacyState(params);
expect(result.changes).toEqual([]);
expect(result.warnings).toHaveLength(1);
expect(result.warnings[0]).toContain("session index");
expect(result.warnings[0]).toContain(detail);
await expect(fs.access(fixture.sidecarPath)).resolves.toBeUndefined();
await expect(fs.access(`${fixture.sidecarPath}.migrated`)).rejects.toThrow();
await expect(openBindingStore(fixture.env).entries()).resolves.toEqual([]);
await Promise.all([
fs.rm(fixture.stateDir, { recursive: true, force: true }),
fs.rm(externalDir, { recursive: true, force: true }),
]);
});
it("does not scan above stateDir or follow escaped external store locators", async () => {
const outerDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-doctor-outer-"));
const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-doctor-outside-"));
const stateDir = path.join(outerDir, "state");
await fs.mkdir(stateDir, { recursive: true });
const strayDir = path.join(outerDir, "unrelated");
await fs.mkdir(strayDir, { recursive: true });
const externalStore = path.join(outerDir, "sessions.json");
await fs.writeFile(
path.join(strayDir, "foreign.jsonl.codex-app-server.json"),
JSON.stringify({ schemaVersion: 2, threadId: "thread-foreign" }),
"utf8",
);
await fs.writeFile(
path.join(outsideDir, "foreign.jsonl.codex-app-server.json"),
JSON.stringify({ schemaVersion: 2, threadId: "thread-escaped" }),
"utf8",
);
await fs.writeFile(
externalStore,
JSON.stringify({
"agent:main:foreign": {
sessionId: "foreign",
sessionFile: path.join(outsideDir, "foreign.jsonl"),
},
}),
"utf8",
);
const env = { ...process.env, OPENCLAW_STATE_DIR: stateDir };
const params = {
// The store dir is exactly the parent of stateDir; doctor must treat it
// as an external store (indexed reads only), not a scannable state root.
config: { session: { store: path.join(outerDir, "sessions.json") } },
// The store directory is exactly stateDir's parent. It stays indexed-only,
// and its explicit locator cannot escape that directory.
config: { session: { store: externalStore } },
env,
stateDir,
oauthDir: path.join(stateDir, "oauth"),
@@ -321,7 +748,10 @@ describe("codex doctor contract", () => {
await expect(migration.detectLegacyState(params)).resolves.toBeNull();
await fs.rm(outerDir, { recursive: true, force: true });
await Promise.all([
fs.rm(outerDir, { recursive: true, force: true }),
fs.rm(outsideDir, { recursive: true, force: true }),
]);
});
it("renames old approval-routed destructive plugin policy values", () => {
@@ -379,7 +379,7 @@ export function createCodexAppServerBindingStore(
try {
let renewed = false;
const stored = update(key, (raw) => {
const current = readStoredBinding(raw);
const current = readStoredCodexAppServerBinding(raw);
if (raw !== undefined && !current) {
throw new Error(`Invalid Codex app-server binding row: ${key}`);
}
@@ -426,7 +426,7 @@ export function createCodexAppServerBindingStore(
update(
key,
(raw) => {
const current = readStoredBinding(raw);
const current = readStoredCodexAppServerBinding(raw);
if (raw !== undefined && !current) {
throw new Error(`Invalid Codex app-server binding row: ${key}`);
}
@@ -470,7 +470,7 @@ export function createCodexAppServerBindingStore(
async read(identity) {
const key = bindingStoreKey(identity);
const raw = state.lookup(key);
const stored = readStoredBinding(raw);
const stored = readStoredCodexAppServerBinding(raw);
if (raw !== undefined && !stored) {
throw new Error(`Invalid Codex app-server binding row: ${key}`);
}
@@ -482,7 +482,7 @@ export function createCodexAppServerBindingStore(
async prepareSessionGenerationReclaim(identity) {
const key = bindingStoreKey(identity);
const raw = state.lookup(key);
const current = readStoredBinding(raw);
const current = readStoredCodexAppServerBinding(raw);
if (raw !== undefined && !current) {
throw new Error(`Invalid Codex app-server binding row: ${key}`);
}
@@ -723,7 +723,7 @@ export function createCodexAppServerBindingStore(
raw: unknown,
matches: (current: StoredCodexAppServerBinding) => boolean,
) => {
const current = readStoredBinding(raw);
const current = readStoredCodexAppServerBinding(raw);
if (!current || !matches(current) || current.lease?.token !== token) {
return undefined;
}
@@ -789,7 +789,9 @@ export function bindingStoreKey(identity: CodexAppServerBindingIdentity): string
return `conversation:${bindingId}`;
}
function readStoredBinding(value: unknown): StoredCodexAppServerBinding | undefined {
export function readStoredCodexAppServerBinding(
value: unknown,
): StoredCodexAppServerBinding | undefined {
const result = storedBindingSchema.safeParse(value);
return result.success
? (stripUndefinedValue(result.data) as StoredCodexAppServerBinding)
@@ -50,14 +50,26 @@ type LegacyBindingOwner = {
sessionKey: string;
storePath: string;
transcriptPath: string;
lifecycleRevision?: string;
agentHarnessId?: string;
};
type LegacySessionIndexEntry = {
sessionId: string;
sessionFile?: string;
lifecycleRevision?: string;
agentHarnessId?: string;
};
type BindingOwnerCollection = {
owners: Map<string, LegacyBindingOwner[]>;
failures: string[];
};
type SourceMigrationResult = {
archived: boolean;
importedKeys: number;
warning?: string;
notice?: string;
};
// Keep the doctor contract graph independent from the full Codex runtime.
@@ -88,6 +100,8 @@ async function collectSessionSurfaces(params: MigrationEnvironment): Promise<Ses
agentIds: new Set<string>(),
};
surface.scan ||= scan;
// A store's configured path defines how relative sessionFile locators are
// resolved. Keep it intact; canonicalize only when deduplicating aliases.
surface.storePaths.add(path.resolve(storePath));
surface.agentIds.add(agentId);
surfaces.set(canonicalRoot, surface);
@@ -159,22 +173,80 @@ async function collectLegacyBindingSources(
};
}
async function readLegacySessionIndex(
storePath: string,
): Promise<
{ entries: Array<{ sessionKey: string; entry: LegacySessionIndexEntry }> } | { failure: string }
> {
let contents: string;
try {
contents = await fs.readFile(storePath, "utf8");
} catch (error) {
const code = (error as NodeJS.ErrnoException).code;
return code === "ENOENT"
? { entries: [] }
: { failure: `session index ${storePath} could not be read${code ? ` (${code})` : ""}` };
}
let raw: unknown;
try {
raw = JSON.parse(contents);
} catch {
return { failure: `session index ${storePath} could not be read (invalid JSON)` };
}
if (!isRecord(raw)) {
return { failure: `session index ${storePath} has invalid entries` };
}
let normalizedEntries: ReturnType<typeof listSessionEntries>;
try {
normalizedEntries = listSessionEntries({ storePath, hydrateSkillPromptRefs: false });
} catch {
return { failure: `session index ${storePath} could not be normalized` };
}
const normalizedByKey = new Map(
normalizedEntries.map(({ sessionKey, entry }) => [sessionKey, entry] as const),
);
const entries: Array<{ sessionKey: string; entry: LegacySessionIndexEntry }> = [];
for (const [sessionKey, value] of Object.entries(raw)) {
if (!isRecord(value)) {
return { failure: `session index ${storePath} has invalid entries` };
}
const rawSessionId = typeof value.sessionId === "string" ? value.sessionId.trim() : "";
const sessionId = normalizedByKey.get(sessionKey)?.sessionId?.trim() ?? "";
const sessionFile = value.sessionFile;
const lifecycleRevision = value.lifecycleRevision;
const agentHarnessId = value.agentHarnessId;
if (
!sessionId ||
sessionId !== rawSessionId ||
(sessionFile !== undefined && typeof sessionFile !== "string") ||
(lifecycleRevision !== undefined && typeof lifecycleRevision !== "string") ||
(agentHarnessId !== undefined && typeof agentHarnessId !== "string")
) {
return { failure: `session index ${storePath} has invalid entries` };
}
entries.push({
sessionKey,
entry: {
sessionId,
...(typeof sessionFile === "string" ? { sessionFile } : {}),
...(typeof lifecycleRevision === "string" ? { lifecycleRevision } : {}),
...(typeof agentHarnessId === "string" ? { agentHarnessId } : {}),
},
});
}
return { entries };
}
async function* iterateIndexedSidecars(
surface: SessionSurface,
params: MigrationEnvironment,
): AsyncGenerator<string> {
for (const storePath of surface.storePaths) {
let entries: ReturnType<typeof listSessionEntries>;
try {
entries = listSessionEntries({ storePath, hydrateSkillPromptRefs: false });
} catch {
const index = await readLegacySessionIndex(storePath);
if ("failure" in index) {
continue;
}
for (const { sessionKey, entry } of entries) {
const sessionId = entry.sessionId?.trim();
if (!sessionId) {
continue;
}
for (const { sessionKey, entry } of index.entries) {
const agentId = resolveLegacyBindingOwnerAgentId({
sessionKey,
config: params.config,
@@ -182,7 +254,7 @@ async function* iterateIndexedSidecars(
});
let transcriptPath: string;
try {
transcriptPath = resolveSessionFilePath(sessionId, entry, {
transcriptPath = resolveSessionFilePath(entry.sessionId, entry, {
sessionsDir: path.dirname(storePath),
agentId,
});
@@ -221,7 +293,7 @@ async function collectBindingOwners(
sources: LegacyBindingSource[],
surfaces: SessionSurface[],
params: MigrationEnvironment,
): Promise<Map<string, LegacyBindingOwner[]>> {
): Promise<BindingOwnerCollection> {
const sourcePaths = new Set(
await Promise.all(sources.map((source) => canonicalizePath(source.transcriptPath))),
);
@@ -237,34 +309,33 @@ async function collectBindingOwners(
storeAgentIds.set(storePath, agents);
}
}
const failures: string[] = [];
for (const storePath of storePaths) {
let entries: ReturnType<typeof listSessionEntries>;
try {
entries = listSessionEntries({ storePath, hydrateSkillPromptRefs: false });
} catch {
const canonicalStorePath = await canonicalizePath(storePath);
const index = await readLegacySessionIndex(storePath);
if ("failure" in index) {
failures.push(index.failure);
continue;
}
const sessionsDir = path.dirname(storePath);
for (const { sessionKey, entry } of entries) {
const sessionId = entry.sessionId?.trim();
if (!sessionId) {
continue;
}
for (const { sessionKey, entry } of index.entries) {
const sessionId = entry.sessionId;
const agentId = resolveLegacyBindingOwnerAgentId({
sessionKey,
config: params.config,
storeAgentIds: storeAgentIds.get(storePath),
});
let transcriptPath: string;
let legacyTranscriptPath: string;
let effectiveTranscriptPath: string;
try {
legacyTranscriptPath = resolveLegacySessionFileLocator(sessionsDir, entry, sessionId);
transcriptPath = await canonicalizePath(
resolveSessionFilePath(sessionId, entry, { sessionsDir, agentId }),
);
effectiveTranscriptPath = resolveSessionFilePath(sessionId, entry, {
sessionsDir,
agentId,
});
} catch {
failures.push(`session index ${storePath} has an invalid locator for ${sessionKey}`);
continue;
}
const transcriptPath = await canonicalizePath(effectiveTranscriptPath);
if (!sourcePaths.has(transcriptPath)) {
continue;
}
@@ -273,24 +344,28 @@ async function collectBindingOwners(
sessionId,
sessionKey,
storePath,
transcriptPath: legacyTranscriptPath,
transcriptPath: effectiveTranscriptPath,
...(entry.lifecycleRevision ? { lifecycleRevision: entry.lifecycleRevision } : {}),
...(entry.agentHarnessId?.trim() ? { agentHarnessId: entry.agentHarnessId.trim() } : {}),
};
const candidates = owners.get(transcriptPath) ?? new Map<string, LegacyBindingOwner>();
candidates.set(`${agentId}\0${sessionId}\0${sessionKey}\0${storePath}`, owner);
const ownerKey = `${agentId}\0${sessionId}\0${sessionKey}\0${canonicalStorePath}`;
const configuredStorePath = resolveStorePath(params.config.session?.store, {
agentId,
env: params.env,
});
// The same physical store can appear through a configured symlink and a
// discovered real path. Mutate through the path the runtime itself owns.
if (!candidates.has(ownerKey) || storePath === configuredStorePath) {
candidates.set(ownerKey, owner);
}
owners.set(transcriptPath, candidates);
}
}
return new Map([...owners].map(([key, values]) => [key, [...values.values()]]));
}
function resolveLegacySessionFileLocator(
sessionsDir: string,
entry: { sessionFile?: string },
sessionId: string,
): string {
const sessionFile = entry.sessionFile?.trim();
return path.resolve(sessionsDir, sessionFile || `${sessionId}.jsonl`);
return {
owners: new Map([...owners].map(([key, values]) => [key, [...values.values()]])),
failures,
};
}
function resolveLegacyBindingOwnerAgentId(params: {
@@ -325,7 +400,7 @@ function copyBindingForSession(stored: MigratedBindingRow, sessionId: string): M
async function migrateSource(
source: LegacyBindingSource,
owner: LegacyBindingOwner | undefined,
candidates: LegacyBindingOwner[],
params: MigrationParams,
store: PluginStateKeyedStore<MigratedBindingRow>,
): Promise<SourceMigrationResult> {
@@ -335,6 +410,7 @@ async function migrateSource(
importedKeys,
warning: `Left Codex binding sidecar in place because ${reason}: ${source.sidecarPath}`,
});
const owner = candidates.length === 1 ? candidates[0] : undefined;
try {
return await withFileLock(source.sidecarPath, LEGACY_BINDING_LOCK_OPTIONS, async () => {
const [contents, stat] = await Promise.all([
@@ -343,7 +419,7 @@ async function migrateSource(
]);
const raw = JSON.parse(contents) as Record<string, unknown>;
const [
{ bindingStoreKey, createStoredCodexAppServerBinding, readCodexAppServerThreadBinding },
{ bindingStoreKey, createStoredCodexAppServerBinding, readStoredCodexAppServerBinding },
{ legacyCodexConversationBindingId },
] = await Promise.all([
import("../app-server/session-binding.js"),
@@ -361,10 +437,14 @@ async function migrateSource(
if (!baseStored) {
return retain("its binding is invalid");
}
if (candidates.length > 1) {
// The legacy writer keyed one sidecar to one active session file. Multiple
// current owners are indeterminate, so preserve the source without writes.
return retain(`${candidates.length} matching session owners make ownership ambiguous`);
}
if (owner?.agentHarnessId && owner.agentHarnessId !== CODEX_AGENT_HARNESS_ID) {
return retain(`its session is owned by agent harness ${owner.agentHarnessId}`);
}
const sourceSessionFile =
typeof raw.sessionFile === "string" && raw.sessionFile.trim()
? raw.sessionFile
@@ -384,12 +464,17 @@ async function migrateSource(
);
let currentConversation: MigratedBindingRow | undefined;
for (const key of conversationKeys) {
currentConversation ??= await store.lookup(key);
const current = await store.lookup(key);
if (current === undefined) {
continue;
}
const parsed = readStoredCodexAppServerBinding(current);
if (!parsed) {
return retain(`canonical plugin state is invalid at ${key}`);
}
currentConversation ??= parsed;
}
const stored = currentConversation ?? baseStored;
if (stored.state !== "active" && stored.state !== "cleared") {
return retain(`canonical plugin state changed at ${conversationKeys[0]}`);
}
const sessionKey = owner
? bindingStoreKey({
kind: "session",
@@ -398,21 +483,25 @@ async function migrateSource(
sessionKey: owner.sessionKey,
})
: undefined;
const entries = [
...conversationKeys.map((key) => ({ key, value: stored })),
...(owner && sessionKey
? [{ key: sessionKey, value: copyBindingForSession(stored, owner.sessionId) }]
: []),
];
const hasExpected = (value: MigratedBindingRow | undefined, expected: MigratedBindingRow) =>
expected.state === "cleared"
? value?.state === "cleared" &&
value.sessionId === expected.sessionId &&
value.retired === expected.retired
: value?.state === "active" &&
value.sessionId === expected.sessionId &&
isDeepStrictEqual(readCodexAppServerThreadBinding(value.binding), expected.binding);
const conversationEntries = conversationKeys.map((key) => ({ key, value: stored }));
const sessionEntry =
owner && sessionKey
? { key: sessionKey, value: copyBindingForSession(stored, owner.sessionId) }
: undefined;
const entries = [...conversationEntries, ...(sessionEntry ? [sessionEntry] : [])];
const hasExpected = (value: MigratedBindingRow | undefined, target: MigratedBindingRow) => {
const parsed = readStoredCodexAppServerBinding(value);
if (!parsed) {
return false;
}
return target.state === "cleared"
? parsed.state === "cleared" &&
parsed.sessionId === target.sessionId &&
parsed.retired === target.retired
: parsed.state === "active" &&
parsed.sessionId === target.sessionId &&
isDeepStrictEqual(parsed.binding, target.binding);
};
for (const entry of entries) {
const current = await store.lookup(entry.key);
if (current !== undefined && !hasExpected(current, entry.value)) {
@@ -427,45 +516,98 @@ async function migrateSource(
return retain(`canonical plugin state changed at ${entry.key}`);
}
}
if (!owner) {
return {
archived: false,
importedKeys,
notice: `Left Codex binding sidecar in place after importing its conversation binding because its session owner could not be resolved: ${source.sidecarPath}`,
};
}
const ownershipWarning = await recordSessionOwner(owner);
if (ownershipWarning) {
return retain(ownershipWarning);
}
for (const entry of entries) {
if (!hasExpected(await store.lookup(entry.key), entry.value)) {
return retain(`canonical plugin state changed at ${entry.key}`);
if (owner) {
const ownershipWarning = await recordSessionOwner(owner);
if (ownershipWarning) {
if (sessionEntry?.value.state === "active") {
const update = store.update;
if (!update) {
return retain(`${ownershipWarning}; its stale session binding could not be retired`);
}
await update(sessionEntry.key, (current) => {
const parsed = readStoredCodexAppServerBinding(current);
if (parsed?.lease && parsed.lease.expiresAt > Date.now()) {
return undefined;
}
if (!hasExpected(current, sessionEntry.value)) {
// Atomic no-op: a concurrent runtime owner replaced or removed this row.
return undefined;
}
return {
version: 1,
state: "cleared",
sessionId: owner.sessionId,
retired: true,
};
});
if (hasExpected(await store.lookup(sessionEntry.key), sessionEntry.value)) {
return retain(`${ownershipWarning}; its stale session binding could not be retired`);
}
}
return retain(ownershipWarning);
}
for (const entry of entries) {
if (!hasExpected(await store.lookup(entry.key), entry.value)) {
return retain(`canonical plugin state changed at ${entry.key}`);
}
}
}
// Legacy writers only created sidecars for an existing session file. Once
// unique ownership is recorded, or zero ownership is proven, it is safe to archive.
await archiveBindingSidecar(source.sidecarPath);
return { archived: true, importedKeys };
});
} catch (error) {
// Parallel doctor runs can both discover a source before the first archives it.
if (
(error as NodeJS.ErrnoException).code === "ENOENT" &&
!(await pathExists(source.sidecarPath))
) {
return { archived: true, importedKeys };
}
return retain(`migration or archiving failed: ${String(error)}`);
}
}
async function recordSessionOwner(owner: LegacyBindingOwner): Promise<string | undefined> {
let observedForeignHarness: string | undefined;
const updated = await updateSessionStoreEntry({
storePath: owner.storePath,
sessionKey: owner.sessionKey,
skipMaintenance: true,
requireWriteSuccess: true,
update: (entry) => {
if (entry.sessionId.trim() !== owner.sessionId) {
const transcriptPath = resolveOwnerTranscriptPath(owner, entry);
if (
entry.sessionId.trim() !== owner.sessionId ||
transcriptPath !== owner.transcriptPath ||
entry.lifecycleRevision !== owner.lifecycleRevision
) {
return null;
}
const harnessId = entry.agentHarnessId?.trim();
return harnessId ? null : { agentHarnessId: CODEX_AGENT_HARNESS_ID };
const harnessId =
typeof entry.agentHarnessId === "string" ? entry.agentHarnessId.trim() : undefined;
if (entry.agentHarnessId !== undefined && harnessId === undefined) {
return null;
}
if (harnessId && harnessId !== CODEX_AGENT_HARNESS_ID) {
observedForeignHarness = harnessId;
return null;
}
return { agentHarnessId: CODEX_AGENT_HARNESS_ID };
},
});
if (!updated || updated.sessionId.trim() !== owner.sessionId) {
if (!updated) {
return observedForeignHarness
? `its session is owned by agent harness ${observedForeignHarness}`
: "its session owner changed before Codex ownership could be recorded";
}
const transcriptPath = resolveOwnerTranscriptPath(owner, updated);
if (
updated.sessionId.trim() !== owner.sessionId ||
transcriptPath !== owner.transcriptPath ||
updated.lifecycleRevision !== owner.lifecycleRevision
) {
return "its session owner changed before Codex ownership could be recorded";
}
const harnessId = updated.agentHarnessId?.trim();
@@ -476,6 +618,20 @@ async function recordSessionOwner(owner: LegacyBindingOwner): Promise<string | u
: "Codex harness ownership could not be recorded on its session";
}
function resolveOwnerTranscriptPath(
owner: LegacyBindingOwner,
entry: { sessionFile?: string; sessionId: string },
): string | undefined {
try {
return resolveSessionFilePath(entry.sessionId, entry, {
sessionsDir: path.dirname(owner.storePath),
agentId: owner.agentId,
});
} catch {
return undefined;
}
}
async function readDirectoryEntries(directory: string) {
try {
return await fs.readdir(directory, { withFileTypes: true });
@@ -497,6 +653,10 @@ async function isRegularFile(filePath: string): Promise<boolean> {
}
}
function isRecord(value: unknown): value is Record<string, unknown> {
return value !== null && typeof value === "object" && !Array.isArray(value);
}
function isPathWithin(root: string, candidate: string): boolean {
const relative = path.relative(root, candidate);
// Bare ".." (candidate is root's parent) must stay outside; treating it as
@@ -567,12 +727,17 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
async migrateLegacyState(params) {
const changes: string[] = [];
const warnings: string[] = [];
const notices: string[] = [];
const { sources, surfaces } = await collectLegacyBindingSources(params);
if (sources.length === 0) {
return { changes, warnings };
}
const owners = await collectBindingOwners(sources, surfaces, params);
const ownerCollection = await collectBindingOwners(sources, surfaces, params);
if (ownerCollection.failures.length > 0) {
warnings.push(
`Left ${sources.length} Codex binding sidecar(s) in place because session ownership is indeterminate: ${ownerCollection.failures.join("; ")}`,
);
return { changes, warnings };
}
const store = params.context.openPluginStateKeyedStore<MigratedBindingRow>({
namespace: CODEX_APP_SERVER_BINDING_NAMESPACE,
maxEntries: CODEX_APP_SERVER_BINDING_MAX_ENTRIES,
@@ -581,15 +746,12 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
let migrated = 0;
let partialImports = 0;
for (const source of sources) {
const candidates = owners.get(await canonicalizePath(source.transcriptPath)) ?? [];
const owner = candidates.length === 1 ? candidates[0] : undefined;
const result = await migrateSource(source, owner, params, store);
const candidates =
ownerCollection.owners.get(await canonicalizePath(source.transcriptPath)) ?? [];
const result = await migrateSource(source, candidates, params, store);
if (result.warning) {
warnings.push(result.warning);
}
if (result.notice) {
notices.push(result.notice);
}
if (result.archived) {
migrated++;
} else {
@@ -606,7 +768,7 @@ export const stateMigrations: PluginDoctorStateMigration[] = [
`Migrated ${partialImports} safe Codex app-server binding row(s) to plugin state; retained legacy sidecars needing review`,
);
}
return notices.length > 0 ? { changes, warnings, notices } : { changes, warnings };
return { changes, warnings };
},
},
];