mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-05 00:00:22 +00:00
fix: address Codex harness review regressions
This commit is contained in:
@@ -282,6 +282,13 @@ export function resetSharedCodexAppServerClientForTests(): void {
|
|||||||
sharedClientPromise = undefined;
|
sharedClientPromise = undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function clearSharedCodexAppServerClient(): void {
|
||||||
|
const client = sharedClient;
|
||||||
|
sharedClient = undefined;
|
||||||
|
sharedClientPromise = undefined;
|
||||||
|
client?.close();
|
||||||
|
}
|
||||||
|
|
||||||
function clearSharedClientIfCurrent(client: CodexAppServerClient): void {
|
function clearSharedClientIfCurrent(client: CodexAppServerClient): void {
|
||||||
if (sharedClient !== client) {
|
if (sharedClient !== client) {
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import {
|
|||||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
import type { CodexServerNotification } from "./protocol.js";
|
import type { CodexServerNotification } from "./protocol.js";
|
||||||
import { runCodexAppServerAttempt, __testing } from "./run-attempt.js";
|
import { runCodexAppServerAttempt, __testing } from "./run-attempt.js";
|
||||||
|
import { writeCodexAppServerBinding } from "./session-binding.js";
|
||||||
|
|
||||||
let tempDir: string;
|
let tempDir: string;
|
||||||
|
|
||||||
@@ -226,4 +227,79 @@ describe("runCodexAppServerAttempt", () => {
|
|||||||
timedOut: false,
|
timedOut: false,
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("times out app-server startup before thread setup can hang forever", async () => {
|
||||||
|
__testing.setCodexAppServerClientFactoryForTests(() => new Promise<never>(() => undefined));
|
||||||
|
const params = createParams(
|
||||||
|
path.join(tempDir, "session.jsonl"),
|
||||||
|
path.join(tempDir, "workspace"),
|
||||||
|
);
|
||||||
|
params.timeoutMs = 1;
|
||||||
|
|
||||||
|
await expect(runCodexAppServerAttempt(params)).rejects.toThrow(
|
||||||
|
"codex app-server startup timed out",
|
||||||
|
);
|
||||||
|
expect(queueAgentHarnessMessage("session-1", "after timeout")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps extended history enabled when resuming a bound Codex thread", async () => {
|
||||||
|
const sessionFile = path.join(tempDir, "session.jsonl");
|
||||||
|
const workspaceDir = path.join(tempDir, "workspace");
|
||||||
|
await writeCodexAppServerBinding(sessionFile, {
|
||||||
|
threadId: "thread-existing",
|
||||||
|
cwd: workspaceDir,
|
||||||
|
model: "gpt-5.4-codex",
|
||||||
|
modelProvider: "openai",
|
||||||
|
dynamicToolsFingerprint: "[]",
|
||||||
|
});
|
||||||
|
const requests: Array<{ method: string; params: unknown }> = [];
|
||||||
|
let notify: (notification: CodexServerNotification) => Promise<void> = async () => undefined;
|
||||||
|
const request = vi.fn(async (method: string, params?: unknown) => {
|
||||||
|
requests.push({ method, params });
|
||||||
|
if (method === "thread/resume") {
|
||||||
|
return { thread: { id: "thread-existing" }, modelProvider: "openai" };
|
||||||
|
}
|
||||||
|
if (method === "turn/start") {
|
||||||
|
return { turn: { id: "turn-1", status: "inProgress" } };
|
||||||
|
}
|
||||||
|
return {};
|
||||||
|
});
|
||||||
|
__testing.setCodexAppServerClientFactoryForTests(
|
||||||
|
async () =>
|
||||||
|
({
|
||||||
|
request,
|
||||||
|
addNotificationHandler: (handler: typeof notify) => {
|
||||||
|
notify = handler;
|
||||||
|
return () => undefined;
|
||||||
|
},
|
||||||
|
addRequestHandler: () => () => undefined,
|
||||||
|
}) as never,
|
||||||
|
);
|
||||||
|
|
||||||
|
const run = runCodexAppServerAttempt(createParams(sessionFile, workspaceDir));
|
||||||
|
await vi.waitFor(() =>
|
||||||
|
expect(requests.some((entry) => entry.method === "turn/start")).toBe(true),
|
||||||
|
);
|
||||||
|
await notify({
|
||||||
|
method: "turn/completed",
|
||||||
|
params: {
|
||||||
|
threadId: "thread-existing",
|
||||||
|
turnId: "turn-1",
|
||||||
|
turn: { id: "turn-1", status: "completed" },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
await run;
|
||||||
|
|
||||||
|
expect(requests).toEqual(
|
||||||
|
expect.arrayContaining([
|
||||||
|
{
|
||||||
|
method: "thread/resume",
|
||||||
|
params: {
|
||||||
|
threadId: "thread-existing",
|
||||||
|
persistExtendedHistory: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]),
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import {
|
|||||||
} from "openclaw/plugin-sdk/agent-harness";
|
} from "openclaw/plugin-sdk/agent-harness";
|
||||||
import { handleCodexAppServerApprovalRequest } from "./approval-bridge.js";
|
import { handleCodexAppServerApprovalRequest } from "./approval-bridge.js";
|
||||||
import {
|
import {
|
||||||
|
clearSharedCodexAppServerClient,
|
||||||
getSharedCodexAppServerClient,
|
getSharedCodexAppServerClient,
|
||||||
isCodexAppServerApprovalRequest,
|
isCodexAppServerApprovalRequest,
|
||||||
type CodexAppServerClient,
|
type CodexAppServerClient,
|
||||||
@@ -98,13 +99,28 @@ export async function runCodexAppServerAttempt(
|
|||||||
tools,
|
tools,
|
||||||
signal: runAbortController.signal,
|
signal: runAbortController.signal,
|
||||||
});
|
});
|
||||||
const client = await clientFactory();
|
let client: CodexAppServerClient;
|
||||||
const thread = await startOrResumeThread({
|
let thread: CodexAppServerThreadBinding;
|
||||||
client,
|
try {
|
||||||
params,
|
({ client, thread } = await withCodexStartupTimeout({
|
||||||
cwd: effectiveWorkspace,
|
timeoutMs: params.timeoutMs,
|
||||||
dynamicTools: toolBridge.specs,
|
signal: runAbortController.signal,
|
||||||
});
|
operation: async () => {
|
||||||
|
const startupClient = await clientFactory();
|
||||||
|
const startupThread = await startOrResumeThread({
|
||||||
|
client: startupClient,
|
||||||
|
params,
|
||||||
|
cwd: effectiveWorkspace,
|
||||||
|
dynamicTools: toolBridge.specs,
|
||||||
|
});
|
||||||
|
return { client: startupClient, thread: startupThread };
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
} catch (error) {
|
||||||
|
clearSharedCodexAppServerClient();
|
||||||
|
params.abortSignal?.removeEventListener("abort", abortFromUpstream);
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
|
||||||
let projector: CodexAppServerEventProjector | undefined;
|
let projector: CodexAppServerEventProjector | undefined;
|
||||||
let turnId: string | undefined;
|
let turnId: string | undefined;
|
||||||
@@ -335,6 +351,44 @@ async function buildDynamicTools(input: DynamicToolBuildParams) {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function withCodexStartupTimeout<T>(params: {
|
||||||
|
timeoutMs: number;
|
||||||
|
signal: AbortSignal;
|
||||||
|
operation: () => Promise<T>;
|
||||||
|
}): Promise<T> {
|
||||||
|
if (params.signal.aborted) {
|
||||||
|
throw new Error("codex app-server startup aborted");
|
||||||
|
}
|
||||||
|
let timeout: NodeJS.Timeout | undefined;
|
||||||
|
let abortCleanup: (() => void) | undefined;
|
||||||
|
try {
|
||||||
|
return await Promise.race([
|
||||||
|
params.operation(),
|
||||||
|
new Promise<never>((_, reject) => {
|
||||||
|
const rejectOnce = (error: Error) => {
|
||||||
|
if (timeout) {
|
||||||
|
clearTimeout(timeout);
|
||||||
|
timeout = undefined;
|
||||||
|
}
|
||||||
|
reject(error);
|
||||||
|
};
|
||||||
|
const timeoutMs = Math.max(100, params.timeoutMs);
|
||||||
|
timeout = setTimeout(() => {
|
||||||
|
rejectOnce(new Error("codex app-server startup timed out"));
|
||||||
|
}, timeoutMs);
|
||||||
|
const abortListener = () => rejectOnce(new Error("codex app-server startup aborted"));
|
||||||
|
params.signal.addEventListener("abort", abortListener, { once: true });
|
||||||
|
abortCleanup = () => params.signal.removeEventListener("abort", abortListener);
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
} finally {
|
||||||
|
if (timeout) {
|
||||||
|
clearTimeout(timeout);
|
||||||
|
}
|
||||||
|
abortCleanup?.();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async function startOrResumeThread(params: {
|
async function startOrResumeThread(params: {
|
||||||
client: CodexAppServerClient;
|
client: CodexAppServerClient;
|
||||||
params: EmbeddedRunAttemptParams;
|
params: EmbeddedRunAttemptParams;
|
||||||
@@ -356,6 +410,7 @@ async function startOrResumeThread(params: {
|
|||||||
try {
|
try {
|
||||||
const response = await params.client.request<CodexThreadResumeResponse>("thread/resume", {
|
const response = await params.client.request<CodexThreadResumeResponse>("thread/resume", {
|
||||||
threadId: binding.threadId,
|
threadId: binding.threadId,
|
||||||
|
persistExtendedHistory: true,
|
||||||
});
|
});
|
||||||
await writeCodexAppServerBinding(params.params.sessionFile, {
|
await writeCodexAppServerBinding(params.params.sessionFile, {
|
||||||
threadId: response.thread.id,
|
threadId: response.thread.id,
|
||||||
|
|||||||
@@ -4,10 +4,6 @@
|
|||||||
"name": "Codex",
|
"name": "Codex",
|
||||||
"description": "Codex app-server harness and Codex-managed GPT model catalog.",
|
"description": "Codex app-server harness and Codex-managed GPT model catalog.",
|
||||||
"providers": ["codex"],
|
"providers": ["codex"],
|
||||||
"modelSupport": {
|
|
||||||
"providerIds": ["codex"],
|
|
||||||
"modelPrefixes": ["gpt-", "o1", "o3", "o4", "arcanine"]
|
|
||||||
},
|
|
||||||
"configSchema": {
|
"configSchema": {
|
||||||
"type": "object",
|
"type": "object",
|
||||||
"additionalProperties": false,
|
"additionalProperties": false,
|
||||||
|
|||||||
@@ -63,7 +63,8 @@ async function prewarmConfiguredPrimaryModel(params: {
|
|||||||
if (isCliProvider(provider, params.cfg)) {
|
if (isCliProvider(provider, params.cfg)) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (resolveEmbeddedAgentRuntime() !== "auto") {
|
const runtime = resolveEmbeddedAgentRuntime();
|
||||||
|
if (runtime !== "auto" && runtime !== "pi") {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (selectAgentHarness({ provider, modelId: model }).id !== "pi") {
|
if (selectAgentHarness({ provider, modelId: model }).id !== "pi") {
|
||||||
|
|||||||
@@ -167,4 +167,29 @@ describe("gateway startup primary model warmup", () => {
|
|||||||
expect(ensureOpenClawModelsJsonMock).not.toHaveBeenCalled();
|
expect(ensureOpenClawModelsJsonMock).not.toHaveBeenCalled();
|
||||||
expect(resolveModelMock).not.toHaveBeenCalled();
|
expect(resolveModelMock).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("keeps PI static warmup when the PI agent runtime is forced", async () => {
|
||||||
|
resolveEmbeddedAgentRuntimeMock.mockReturnValue("pi");
|
||||||
|
const cfg = {
|
||||||
|
agents: {
|
||||||
|
defaults: {
|
||||||
|
model: {
|
||||||
|
primary: "openai-codex/gpt-5.4",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} as OpenClawConfig;
|
||||||
|
|
||||||
|
await prewarmConfiguredPrimaryModel({
|
||||||
|
cfg,
|
||||||
|
log: { warn: vi.fn() },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(selectAgentHarnessMock).toHaveBeenCalledWith({
|
||||||
|
provider: "openai-codex",
|
||||||
|
modelId: "gpt-5.4",
|
||||||
|
});
|
||||||
|
expect(ensureOpenClawModelsJsonMock).toHaveBeenCalledWith(cfg, "/tmp/agent");
|
||||||
|
expect(resolveModelMock).toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user