Skip to content

Commit 7f7eca1

Browse files
steipetesteipete
authored
fix(codex): preserve shared app-server after startup app errors (#87399)
* fix(codex): preserve shared app-server after startup app errors * fix(codex): align startup cleanup tests with current types * test(config): isolate installed plugin ledger cache
1 parent 87944c0 commit 7f7eca1

3 files changed

Lines changed: 221 additions & 11 deletions

File tree

‎extensions/codex/src/app-server/attempt-startup.test.ts‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
import type {
2+
CodexBundleMcpThreadConfig,
3+
EmbeddedRunAttemptParams,
4+
} from "openclaw/plugin-sdk/agent-harness-runtime";
5+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
6+
import { startCodexAttemptThread } from "./attempt-startup.js";
7+
import { defaultLeasedCodexAppServerClientFactory } from "./client-factory.js";
8+
import { CodexAppServerClient } from "./client.js";
9+
import { type CodexPluginConfig, resolveCodexAppServerRuntimeOptions } from "./config.js";
10+
import { clearSharedCodexAppServerClient } from "./shared-client.js";
11+
import { createClientHarness, createCodexTestModel } from "./test-support.js";
12+
13+
type ClientHarness = ReturnType<typeof createClientHarness>;
14+
15+
function createAttemptParams(): EmbeddedRunAttemptParams {
16+
return {
17+
prompt: "hello",
18+
sessionId: "session-1",
19+
sessionKey: "agent:main:session-1",
20+
sessionFile: "/tmp/session.jsonl",
21+
workspaceDir: "/tmp",
22+
runId: "run-1",
23+
provider: "codex",
24+
modelId: "gpt-5.4-codex",
25+
model: createCodexTestModel("codex"),
26+
thinkLevel: "medium",
27+
disableTools: true,
28+
timeoutMs: 5_000,
29+
authStorage: {} as never,
30+
authProfileStore: { version: 1, profiles: {} },
31+
modelRegistry: {} as never,
32+
} as EmbeddedRunAttemptParams;
33+
}
34+
35+
const pluginConfig: CodexPluginConfig = {
36+
appServer: { command: "codex" },
37+
};
38+
39+
const bundleMcpThreadConfig = {
40+
configPatch: undefined,
41+
diagnostics: [],
42+
evaluated: false,
43+
fingerprint: undefined,
44+
} satisfies CodexBundleMcpThreadConfig;
45+
46+
function readHarnessMessages(writes: string[]): Array<{ id?: number; method?: string }> {
47+
return writes.map((write) => JSON.parse(write) as { id?: number; method?: string });
48+
}
49+
50+
function startThreadWithHarness(
51+
startupTimeoutMs: number,
52+
signal = new AbortController().signal,
53+
overrides?: { pluginConfig?: CodexPluginConfig },
54+
) {
55+
const harness = createClientHarness();
56+
vi.spyOn(CodexAppServerClient, "start").mockReturnValue(harness.client);
57+
const effectivePluginConfig = overrides?.pluginConfig ?? pluginConfig;
58+
59+
const run = startCodexAttemptThread({
60+
attemptClientFactory: defaultLeasedCodexAppServerClientFactory,
61+
appServer: resolveCodexAppServerRuntimeOptions({ pluginConfig: effectivePluginConfig }),
62+
pluginConfig: effectivePluginConfig,
63+
computerUseConfig: effectivePluginConfig.computerUse ?? { enabled: false },
64+
startupAuthProfileId: undefined,
65+
startupAuthAccountCacheKey: undefined,
66+
startupEnvApiKeyCacheKey: undefined,
67+
agentDir: "/tmp/agent",
68+
config: undefined,
69+
buildAttemptParams: createAttemptParams,
70+
sessionAgentId: "agent-1",
71+
effectiveWorkspace: "/tmp",
72+
dynamicTools: [],
73+
developerInstructions: undefined,
74+
finalConfigPatch: undefined,
75+
bundleMcpThreadConfig,
76+
nativeToolSurfaceEnabled: true,
77+
sandboxExecServerEnabled: false,
78+
sandbox: null,
79+
contextEngineProjection: undefined,
80+
startupTimeoutMs,
81+
signal,
82+
onStartupTimeout: vi.fn(),
83+
spawnedBy: undefined,
84+
});
85+
86+
return { harness, run };
87+
}
88+
89+
async function answerInitialize(harness: ClientHarness): Promise<void> {
90+
await vi.waitFor(() => expect(harness.writes.length).toBeGreaterThanOrEqual(1), {
91+
interval: 1,
92+
timeout: 5_000,
93+
});
94+
const initialize = JSON.parse(harness.writes[0] ?? "{}") as { id?: number };
95+
harness.send({ id: initialize.id, result: { userAgent: "openclaw/0.125.0 (macOS; test)" } });
96+
}
97+
98+
async function waitForRequest(
99+
harness: ClientHarness,
100+
method: string,
101+
): Promise<{ id?: number; method?: string }> {
102+
await vi.waitFor(
103+
() =>
104+
expect(readHarnessMessages(harness.writes).some((write) => write.method === method)).toBe(
105+
true,
106+
),
107+
{ interval: 1, timeout: 5_000 },
108+
);
109+
const request = readHarnessMessages(harness.writes).find((write) => write.method === method);
110+
if (!request) {
111+
throw new Error(`${method} request was not written`);
112+
}
113+
return request;
114+
}
115+
116+
async function waitForThreadStart(harness: ClientHarness): Promise<{ id?: number }> {
117+
return waitForRequest(harness, "thread/start");
118+
}
119+
120+
describe("startCodexAttemptThread", () => {
121+
beforeEach(() => {
122+
vi.stubEnv("CODEX_API_KEY", "");
123+
vi.stubEnv("OPENAI_API_KEY", "");
124+
clearSharedCodexAppServerClient();
125+
});
126+
127+
afterEach(() => {
128+
clearSharedCodexAppServerClient();
129+
vi.restoreAllMocks();
130+
vi.unstubAllEnvs();
131+
});
132+
133+
it("keeps the shared app-server when thread startup fails with an app error", async () => {
134+
const { harness, run } = startThreadWithHarness(5_000);
135+
await answerInitialize(harness);
136+
const threadStart = await waitForThreadStart(harness);
137+
harness.send({
138+
id: threadStart.id,
139+
error: { code: -32000, message: "401 authentication_error: Invalid bearer token" },
140+
});
141+
142+
await expect(run).rejects.toThrow("Invalid bearer token");
143+
expect(harness.process.stdin.destroyed).toBe(false);
144+
145+
clearSharedCodexAppServerClient();
146+
expect(harness.process.stdin.destroyed).toBe(true);
147+
});
148+
149+
it("clears the shared app-server when startup abandons an in-flight thread request", async () => {
150+
const { harness, run } = startThreadWithHarness(2_000);
151+
const runError = run.then(
152+
() => undefined,
153+
(error: unknown) => error,
154+
);
155+
await answerInitialize(harness);
156+
await waitForThreadStart(harness);
157+
158+
const error = await runError;
159+
expect(error).toBeInstanceOf(Error);
160+
expect((error as Error).message).toBe("codex app-server startup timed out");
161+
expect(harness.process.stdin.destroyed).toBe(true);
162+
});
163+
164+
it("clears the shared app-server when cancellation abandons an in-flight thread request", async () => {
165+
const abortController = new AbortController();
166+
const { harness, run } = startThreadWithHarness(5_000, abortController.signal);
167+
const runError = run.then(
168+
() => undefined,
169+
(error: unknown) => error,
170+
);
171+
await answerInitialize(harness);
172+
await waitForThreadStart(harness);
173+
174+
abortController.abort();
175+
176+
const error = await runError;
177+
expect(error).toBeInstanceOf(Error);
178+
expect((error as Error).message).toBe("codex app-server startup aborted");
179+
expect(harness.process.stdin.destroyed).toBe(true);
180+
});
181+
182+
it("clears the shared app-server when a startup RPC times out", async () => {
183+
const perRpcTimeoutPluginConfig = {
184+
...pluginConfig,
185+
appServer: { command: "codex", requestTimeoutMs: 100 },
186+
computerUse: { enabled: true, marketplaceDiscoveryTimeoutMs: 1 },
187+
} satisfies CodexPluginConfig;
188+
const { harness, run } = startThreadWithHarness(5_000, new AbortController().signal, {
189+
pluginConfig: perRpcTimeoutPluginConfig,
190+
});
191+
const runError = run.then(
192+
() => undefined,
193+
(error: unknown) => error,
194+
);
195+
await answerInitialize(harness);
196+
await waitForRequest(harness, "plugin/list");
197+
198+
const error = await runError;
199+
expect(error).toBeInstanceOf(Error);
200+
expect((error as Error).message).toBe("plugin/list timed out");
201+
expect(harness.process.stdin.destroyed).toBe(true);
202+
});
203+
});

‎extensions/codex/src/app-server/attempt-startup.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ import {
4949
} from "./shared-client.js";
5050
import {
5151
startOrResumeThread,
52-
isCodexThreadStartRequestError,
5352
type CodexAppServerThreadLifecycleBinding,
5453
type CodexContextEngineThreadBootstrapProjection,
5554
} from "./thread-lifecycle.js";
@@ -100,7 +99,7 @@ export async function startCodexAttemptThread(params: {
10099
}): Promise<StartCodexAttemptThreadResult> {
101100
let pluginAppServer = params.appServer;
102101
let releaseSharedClientLease: (() => void) | undefined;
103-
let startupClientForCleanup: CodexAppServerClient | undefined;
102+
let startupClientForAbandonedRequestCleanup: CodexAppServerClient | undefined;
104103
let releaseStartupResourcesOnTimeout: (() => Promise<void>) | undefined;
105104
try {
106105
const startupResult = await withCodexStartupTimeout({
@@ -185,7 +184,7 @@ export async function startCodexAttemptThread(params: {
185184
};
186185
releaseSharedClientLease = startupClientLease;
187186
attemptedClient = startupClient;
188-
startupClientForCleanup = startupClient;
187+
startupClientForAbandonedRequestCleanup = startupClient;
189188
await ensureCodexComputerUse({
190189
client: startupClient,
191190
pluginConfig: params.pluginConfig,
@@ -334,8 +333,8 @@ export async function startCodexAttemptThread(params: {
334333
}
335334
const failedClient = attemptedClient;
336335
const clearedSharedClient = clearSharedCodexAppServerClientIfCurrent(failedClient);
337-
if (startupClientForCleanup === failedClient) {
338-
startupClientForCleanup = undefined;
336+
if (startupClientForAbandonedRequestCleanup === failedClient) {
337+
startupClientForAbandonedRequestCleanup = undefined;
339338
}
340339
attemptedClient = undefined;
341340
if (attempt >= CODEX_APP_SERVER_STARTUP_CONNECTION_CLOSE_MAX_ATTEMPTS) {
@@ -365,7 +364,7 @@ export async function startCodexAttemptThread(params: {
365364
throw new Error("codex app-server startup retry loop exited unexpectedly");
366365
},
367366
});
368-
startupClientForCleanup = undefined;
367+
startupClientForAbandonedRequestCleanup = undefined;
369368
if (!releaseSharedClientLease) {
370369
throw new Error("codex app-server startup succeeded without a shared client lease");
371370
}
@@ -375,12 +374,18 @@ export async function startCodexAttemptThread(params: {
375374
releaseSharedClientLease,
376375
};
377376
} catch (error) {
378-
const transportPoisoned = isCodexAppServerConnectionClosedError(error);
379-
const preserveSharedClientForSpawnedThreadStartError =
380-
Boolean(params.spawnedBy?.trim()) && isCodexThreadStartRequestError(error);
381-
if (transportPoisoned || !preserveSharedClientForSpawnedThreadStartError) {
382-
clearSharedCodexAppServerClientIfCurrent(startupClientForCleanup);
377+
if (params.signal.aborted || shouldClearSharedClientAfterStartupRace(error)) {
378+
clearSharedCodexAppServerClientIfCurrent(startupClientForAbandonedRequestCleanup);
383379
}
384380
throw error;
385381
}
386382
}
383+
384+
function shouldClearSharedClientAfterStartupRace(error: unknown): boolean {
385+
return (
386+
error instanceof Error &&
387+
(error.message === "codex app-server startup timed out" ||
388+
error.message === "codex app-server startup aborted" ||
389+
error.message.endsWith(" timed out"))
390+
);
391+
}

‎src/config/config.plugin-validation.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,7 @@ describe("config plugin validation", () => {
834834
it("uses persisted installed-plugin records as stale channel evidence", async () => {
835835
const installedPluginIndexPath = path.join(suiteHome, ".openclaw", "plugins", "installs.json");
836836
await mkdirSafe(path.dirname(installedPluginIndexPath));
837+
clearLoadInstalledPluginIndexInstallRecordsCache();
837838
await fs.writeFile(
838839
installedPluginIndexPath,
839840
JSON.stringify(
@@ -872,6 +873,7 @@ describe("config plugin validation", () => {
872873
});
873874
} finally {
874875
await fs.rm(installedPluginIndexPath, { force: true });
876+
clearLoadInstalledPluginIndexInstallRecordsCache();
875877
}
876878
});
877879

0 commit comments

Comments
 (0)