diff --git a/extensions/codex/src/app-server/attempt-client-cleanup.ts b/extensions/codex/src/app-server/attempt-client-cleanup.ts index d48ede326d0a..a53cc0ece33d 100644 --- a/extensions/codex/src/app-server/attempt-client-cleanup.ts +++ b/extensions/codex/src/app-server/attempt-client-cleanup.ts @@ -2,6 +2,7 @@ * Best-effort cleanup helpers for Codex app-server startup attempts and turns. */ import { embeddedAgentLog } from "openclaw/plugin-sdk/agent-harness-runtime"; +import { unsubscribeCodexAppServerLiveThread } from "./client-runtime.js"; import { CodexAppServerRpcError, type CodexAppServerClient } from "./client.js"; import { retireSharedCodexAppServerClientIfCurrent } from "./shared-client.js"; import { getCodexAppServerTurnRouter } from "./turn-router.js"; @@ -146,11 +147,7 @@ export async function unsubscribeCodexThreadBestEffort( }, ): Promise { try { - await client.request( - "thread/unsubscribe", - { threadId: params.threadId }, - { timeoutMs: params.timeoutMs }, - ); + await unsubscribeCodexAppServerLiveThread(client, params.threadId, params.timeoutMs); return true; } catch (error) { embeddedAgentLog.debug("codex app-server thread unsubscribe cleanup failed", { diff --git a/extensions/codex/src/app-server/client-runtime.test.ts b/extensions/codex/src/app-server/client-runtime.test.ts index db212d0bd2c9..ecef22ef7dce 100644 --- a/extensions/codex/src/app-server/client-runtime.test.ts +++ b/extensions/codex/src/app-server/client-runtime.test.ts @@ -19,11 +19,14 @@ vi.mock("./rate-limit-cache.js", () => ({ })); const { + claimCodexAppServerLiveThread, consumeCodexAppServerLiveThread, ensureCodexAppServerClientRuntime, + isCodexAppServerLiveThreadClaimed, protectCodexAppServerLiveThread, releaseCodexAppServerLiveThread, retainCodexAppServerLiveThread, + unsubscribeCodexAppServerLiveThread, } = await import("./client-runtime.js"); describe("Codex app-server client runtime", () => { @@ -132,6 +135,235 @@ describe("Codex app-server client runtime", () => { ).resolves.toBeUndefined(); }); + it("claims a fresh auto-subscribed child without exposing or evicting an idle owner", async () => { + const request = vi.fn(async () => ({})); + const client = { + request, + addCloseHandler: vi.fn(), + addNotificationHandler: vi.fn(), + addRequestHandler: vi.fn(), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: "/tmp/agent" }); + const idleRelease = vi.fn(async () => undefined); + for (let index = 0; index < EXPECTED_MAX_IDLE_LIVE_THREADS; index += 1) { + await retainCodexAppServerLiveThread(client, `thread-idle-${index}`, idleRelease); + } + + const ownership = await claimCodexAppServerLiveThread(client, "thread-fresh-child"); + + expect(ownership).toEqual(expect.objectContaining({ release: expect.any(Function) })); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-fresh-child")).toBe(true); + expect(idleRelease).not.toHaveBeenCalled(); + await expect( + claimCodexAppServerLiveThread(client, "thread-fresh-child"), + ).resolves.toBeUndefined(); + await expect(retainCodexAppServerLiveThread(client, "thread-fresh-child")).resolves.toBe(false); + await expect( + consumeCodexAppServerLiveThread(client, "thread-fresh-child"), + ).resolves.toBeUndefined(); + await ownership?.release("thread-fresh-child"); + + expect(request).toHaveBeenCalledExactlyOnceWith( + "thread/unsubscribe", + { threadId: "thread-fresh-child" }, + { timeoutMs: 5_000 }, + ); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-fresh-child")).toBe(false); + expect(idleRelease).not.toHaveBeenCalled(); + }); + + it("keeps an active claim until its exact ownership is successfully released", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + const release = vi + .fn<(threadId: string) => Promise>() + .mockRejectedValueOnce(new Error("unsubscribe unavailable")) + .mockResolvedValueOnce(undefined); + await retainCodexAppServerLiveThread(harness.client, "thread-claimed", release); + const ownership = await consumeCodexAppServerLiveThread(harness.client, "thread-claimed"); + + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-claimed")).toBe(true); + await expect(ownership?.release("thread-claimed")).rejects.toThrow("unsubscribe unavailable"); + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-claimed")).toBe(true); + await expect(ownership?.release("thread-claimed")).resolves.toBeUndefined(); + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-claimed")).toBe(false); + }); + + it("rejects unproven or stale ownership before transferring an active claim", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + await retainCodexAppServerLiveThread(harness.client, "thread-owned"); + const current = await consumeCodexAppServerLiveThread(harness.client, "thread-owned"); + + await expect(retainCodexAppServerLiveThread(harness.client, "thread-owned")).resolves.toBe( + false, + ); + await expect( + retainCodexAppServerLiveThread(harness.client, "thread-owned", async () => undefined), + ).resolves.toBe(false); + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-owned")).toBe(true); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-owned"), + ).resolves.toBeUndefined(); + await expect( + retainCodexAppServerLiveThread(harness.client, "thread-owned", current?.release), + ).resolves.toBe(true); + const successor = await consumeCodexAppServerLiveThread(harness.client, "thread-owned"); + + await expect( + retainCodexAppServerLiveThread(harness.client, "thread-owned", current?.release), + ).resolves.toBe(false); + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-owned")).toBe(true); + await expect( + retainCodexAppServerLiveThread(harness.client, "thread-owned", successor?.release), + ).resolves.toBe(true); + }); + + it("does not let an older release erase a newly claimed thread generation", async () => { + const request = vi.fn(async () => ({})); + const client = { + request, + addCloseHandler: vi.fn(), + addNotificationHandler: vi.fn(), + addRequestHandler: vi.fn(), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: "/tmp/agent" }); + await retainCodexAppServerLiveThread(client, "thread-reclaimed"); + const first = await consumeCodexAppServerLiveThread(client, "thread-reclaimed"); + await retainCodexAppServerLiveThread(client, "thread-reclaimed", first?.release); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-reclaimed")).toBe(false); + const second = await consumeCodexAppServerLiveThread(client, "thread-reclaimed"); + + await first?.release("thread-reclaimed"); + + expect(request).not.toHaveBeenCalled(); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-reclaimed")).toBe(true); + await second?.release("thread-reclaimed"); + expect(request).toHaveBeenCalledExactlyOnceWith( + "thread/unsubscribe", + { threadId: "thread-reclaimed" }, + { timeoutMs: 5_000 }, + ); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-reclaimed")).toBe(false); + }); + + it("blocks same-thread replacement until its claimed unsubscribe is acknowledged", async () => { + let acknowledgeUnsubscribe: (() => void) | undefined; + const unsubscribeAcknowledged = new Promise((resolve) => { + acknowledgeUnsubscribe = resolve; + }); + const request = vi.fn(async (method: string) => { + if (method === "thread/unsubscribe") { + await unsubscribeAcknowledged; + } + return {}; + }); + const client = { + request, + addCloseHandler: vi.fn(), + addNotificationHandler: vi.fn(), + addRequestHandler: vi.fn(), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: "/tmp/agent" }); + await retainCodexAppServerLiveThread(client, "thread-transition"); + const previous = await consumeCodexAppServerLiveThread(client, "thread-transition"); + const releasing = previous?.release("thread-transition"); + const duplicateRelease = previous?.release("thread-transition"); + await vi.waitFor(() => expect(request).toHaveBeenCalledOnce()); + const overlappingPhysicalRelease = unsubscribeCodexAppServerLiveThread( + client, + "thread-transition", + 5_000, + ); + await Promise.resolve(); + expect(request).toHaveBeenCalledOnce(); + const replacement = retainCodexAppServerLiveThread( + client, + "thread-transition", + previous?.release, + ); + const blockedClaim = consumeCodexAppServerLiveThread(client, "thread-transition"); + let replacementPublished = false; + void replacement.then(() => { + replacementPublished = true; + }); + await Promise.resolve(); + + expect(replacementPublished).toBe(false); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-transition")).toBe(true); + acknowledgeUnsubscribe?.(); + + await expect(releasing).resolves.toBeUndefined(); + await expect(duplicateRelease).resolves.toBeUndefined(); + await expect(overlappingPhysicalRelease).resolves.toBeUndefined(); + await expect(replacement).resolves.toBe(false); + await expect(blockedClaim).resolves.toBeUndefined(); + await client.request("thread/resume", { threadId: "thread-transition" }, { timeoutMs: 5_000 }); + await expect( + retainCodexAppServerLiveThread(client, "thread-transition", previous?.release), + ).resolves.toBe(true); + const successor = await consumeCodexAppServerLiveThread(client, "thread-transition"); + await successor?.release("thread-transition"); + expect(request.mock.calls.map(([method]) => method)).toEqual([ + "thread/unsubscribe", + "thread/resume", + "thread/unsubscribe", + ]); + }); + + it("finishes a claimed thread when its direct physical unsubscribe succeeds", async () => { + const request = vi.fn(async () => ({})); + request.mockRejectedValueOnce(new Error("unsubscribe unavailable")); + const client = { + request, + addCloseHandler: vi.fn(), + addNotificationHandler: vi.fn(), + addRequestHandler: vi.fn(), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: "/tmp/agent" }); + await retainCodexAppServerLiveThread(client, "thread-direct"); + await consumeCodexAppServerLiveThread(client, "thread-direct"); + + const failedRelease = unsubscribeCodexAppServerLiveThread(client, "thread-direct", 5_000); + const failedJoin = unsubscribeCodexAppServerLiveThread(client, "thread-direct", 5_000); + await expect(Promise.all([failedRelease, failedJoin])).rejects.toThrow( + "unsubscribe unavailable", + ); + expect(request).toHaveBeenCalledOnce(); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-direct")).toBe(true); + await unsubscribeCodexAppServerLiveThread(client, "thread-direct", 5_000); + + expect(request).toHaveBeenLastCalledWith( + "thread/unsubscribe", + { threadId: "thread-direct" }, + { timeoutMs: 5_000 }, + ); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-direct")).toBe(false); + }); + + it("clears claimed ownership when Codex closes the thread or its physical client", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + await retainCodexAppServerLiveThread(harness.client, "thread-closed"); + await consumeCodexAppServerLiveThread(harness.client, "thread-closed"); + + harness.send({ method: "thread/closed", params: { threadId: "thread-closed" } }); + + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-closed")).toBe(false), + ); + await retainCodexAppServerLiveThread(harness.client, "thread-client-closed"); + await consumeCodexAppServerLiveThread(harness.client, "thread-client-closed"); + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-client-closed")).toBe(true); + + harness.client.close(); + + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-client-closed")).toBe(false); + }); + it("blocks only the exact thread whose subscription is being released", async () => { const harness = createClientHarness(); clients.push(harness.client); @@ -154,7 +386,7 @@ describe("Codex app-server client runtime", () => { await expect(sameThreadAcquisition).resolves.toBeUndefined(); }); - it("does not re-expose a failed release or discard an unrelated conversation", async () => { + it("preserves a failed idle release and its unrelated conversation for retry", async () => { const harness = createClientHarness(); clients.push(harness.client); ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); @@ -166,9 +398,9 @@ describe("Codex app-server client runtime", () => { await expect(releaseCodexAppServerLiveThread(harness.client, "thread-a")).rejects.toThrow( "unsubscribe unavailable", ); - await expect( - consumeCodexAppServerLiveThread(harness.client, "thread-a"), - ).resolves.toBeUndefined(); + await expect(consumeCodexAppServerLiveThread(harness.client, "thread-a")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); await expect(consumeCodexAppServerLiveThread(harness.client, "thread-b")).resolves.toEqual( expect.objectContaining({ release: expect.any(Function) }), ); @@ -206,9 +438,66 @@ describe("Codex app-server client runtime", () => { } expect(release).toHaveBeenCalledExactlyOnceWith("thread-0"); - await expect(consumeCodexAppServerLiveThread(harness.client, "thread-1")).resolves.toEqual( - expect.objectContaining({ release }), + const retained = await consumeCodexAppServerLiveThread(harness.client, "thread-1"); + expect(retained).toEqual(expect.objectContaining({ release: expect.any(Function) })); + await retained?.release("thread-1"); + expect(release).toHaveBeenLastCalledWith("thread-1"); + }); + + it("rolls back the new owner when capacity eviction cannot release its oldest thread", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + const failingRelease = vi + .fn<(threadId: string) => Promise>() + .mockRejectedValueOnce(new Error("oldest unsubscribe failed")) + .mockResolvedValueOnce(undefined); + await retainCodexAppServerLiveThread(harness.client, "thread-oldest", failingRelease); + for (let index = 1; index < EXPECTED_MAX_IDLE_LIVE_THREADS; index += 1) { + await retainCodexAppServerLiveThread(harness.client, `thread-${index}`); + } + + await expect(retainCodexAppServerLiveThread(harness.client, "thread-overflow")).resolves.toBe( + false, ); + expect(failingRelease).toHaveBeenCalledOnce(); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-overflow"), + ).resolves.toBeUndefined(); + await expect(consumeCodexAppServerLiveThread(harness.client, "thread-1")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + await expect(releaseCodexAppServerLiveThread(harness.client, "thread-oldest")).resolves.toBe( + true, + ); + expect(failingRelease).toHaveBeenCalledTimes(2); + }); + + it("cannot resurrect an overflow owner after the physical client closes during eviction", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + let finishRelease: (() => void) | undefined; + const pendingRelease = new Promise((resolve) => { + finishRelease = resolve; + }); + await retainCodexAppServerLiveThread( + harness.client, + "thread-oldest", + async () => pendingRelease, + ); + for (let index = 1; index < EXPECTED_MAX_IDLE_LIVE_THREADS; index += 1) { + await retainCodexAppServerLiveThread(harness.client, `thread-${index}`); + } + const retain = retainCodexAppServerLiveThread(harness.client, "thread-overflow"); + + harness.client.close(); + finishRelease?.(); + + await expect(retain).resolves.toBe(false); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-overflow"), + ).resolves.toBeUndefined(); }); it("expires an idle subscription without keeping the process alive", async () => { @@ -226,6 +515,60 @@ describe("Codex app-server client runtime", () => { expect(release).toHaveBeenCalledExactlyOnceWith("thread-expired"); }); + it("renews a failed expiry instead of spinning and retries the same native owner", async () => { + vi.useFakeTimers(); + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + const release = vi + .fn<(threadId: string) => Promise>() + .mockRejectedValueOnce(new Error("temporary unsubscribe failure")) + .mockResolvedValueOnce(undefined); + await retainCodexAppServerLiveThread(harness.client, "thread-expiry-retry", release); + + await vi.advanceTimersByTimeAsync(EXPECTED_LIVE_THREAD_IDLE_TIMEOUT_MS); + + expect(release).toHaveBeenCalledExactlyOnceWith("thread-expiry-retry"); + await vi.advanceTimersByTimeAsync(EXPECTED_LIVE_THREAD_IDLE_TIMEOUT_MS - 1); + expect(release).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + + expect(release).toHaveBeenCalledTimes(2); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-expiry-retry"), + ).resolves.toBeUndefined(); + }); + + it("never resurrects a natively closed thread after its in-flight unsubscribe fails", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + let rejectUnsubscribe: ((error: Error) => void) | undefined; + const failedUnsubscribe = new Promise((_resolve, reject) => { + rejectUnsubscribe = reject; + }); + const release = vi.fn(async () => await failedUnsubscribe); + await retainCodexAppServerLiveThread(harness.client, "thread-terminal", release); + const notificationObserved = new Promise((resolve) => { + harness.client.addNotificationHandler((notification) => { + if (notification.method === "thread/closed") { + resolve(); + } + }); + }); + const releasing = releaseCodexAppServerLiveThread(harness.client, "thread-terminal"); + await vi.waitFor(() => expect(release).toHaveBeenCalledOnce()); + + harness.send({ method: "thread/closed", params: { threadId: "thread-terminal" } }); + await notificationObserved; + rejectUnsubscribe?.(new Error("client closed before unsubscribe completed")); + + await expect(releasing).rejects.toThrow("client closed before unsubscribe completed"); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-terminal"), + ).resolves.toBeUndefined(); + }); + it("protects native-child parents and renews their idle clock after the final child", async () => { vi.useFakeTimers(); const harness = createClientHarness(); @@ -268,6 +611,38 @@ describe("Codex app-server client runtime", () => { expect(release).toHaveBeenNthCalledWith(2, "conversation-b"); }); + it("keeps a failed unpin eviction owned until its original subscription can be retried", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + const release = vi + .fn<(threadId: string) => Promise>() + .mockRejectedValueOnce(new Error("temporary unpin unsubscribe failure")) + .mockResolvedValueOnce(undefined); + await retainCodexAppServerLiveThread(harness.client, "thread-oldest", release); + for (let index = 1; index < EXPECTED_MAX_IDLE_LIVE_THREADS; index += 1) { + await retainCodexAppServerLiveThread(harness.client, `thread-sibling-${index}`); + } + const unprotect = protectCodexAppServerLiveThread(harness.client, "thread-parent"); + await retainCodexAppServerLiveThread(harness.client, "thread-parent"); + + unprotect(); + + await vi.waitFor(() => expect(release).toHaveBeenCalledOnce()); + await vi.waitFor(async () => { + await expect(releaseCodexAppServerLiveThread(harness.client, "thread-oldest")).resolves.toBe( + true, + ); + }); + expect(release).toHaveBeenCalledTimes(2); + await expect(consumeCodexAppServerLiveThread(harness.client, "thread-parent")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-sibling-1"), + ).resolves.toEqual(expect.objectContaining({ release: expect.any(Function) })); + }); + it.each(["thread/archived", "thread/deleted", "thread/closed"])( "discards only the exact thread after %s", async (method) => { @@ -311,4 +686,45 @@ describe("Codex app-server client runtime", () => { consumeCodexAppServerLiveThread(harness.client, "thread-closed"), ).resolves.toBeUndefined(); }); + + it("never publishes new live ownership after its physical client closes", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + + harness.client.close(); + + await expect(retainCodexAppServerLiveThread(harness.client, "thread-stale")).resolves.toBe( + false, + ); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-stale"), + ).resolves.toBeUndefined(); + }); + + it("cannot resurrect thread ownership when its client closes during release", async () => { + const harness = createClientHarness(); + clients.push(harness.client); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: "/tmp/agent" }); + let finishRelease: (() => void) | undefined; + const pendingRelease = new Promise((resolve) => { + finishRelease = resolve; + }); + await retainCodexAppServerLiveThread( + harness.client, + "thread-stale", + async () => pendingRelease, + ); + const release = releaseCodexAppServerLiveThread(harness.client, "thread-stale"); + const retain = retainCodexAppServerLiveThread(harness.client, "thread-stale"); + + harness.client.close(); + finishRelease?.(); + + await expect(release).resolves.toBe(true); + await expect(retain).resolves.toBe(false); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-stale"), + ).resolves.toBeUndefined(); + }); }); diff --git a/extensions/codex/src/app-server/client-runtime.ts b/extensions/codex/src/app-server/client-runtime.ts index 7b31a7e59491..1fcb59d9f023 100644 --- a/extensions/codex/src/app-server/client-runtime.ts +++ b/extensions/codex/src/app-server/client-runtime.ts @@ -1,4 +1,5 @@ /** Client-scoped Codex auth and account observers. */ +import { embeddedAgentLog, formatErrorMessage } from "openclaw/plugin-sdk/agent-harness-runtime"; import { refreshCodexAppServerAuthTokens } from "./auth-bridge.js"; import type { CodexAppServerClient } from "./client.js"; import type { CodexServiceTier, JsonValue } from "./protocol.js"; @@ -12,8 +13,10 @@ type ClientRuntimeContext = Omit & type ClientRuntime = { context: ClientRuntimeContext; + closed: boolean; retainedThreads: Map; - releasingThreads: Map>; + claimedThreads: Map; + releasingThreads: Map; protectedThreads: Map; evictionTimer?: ReturnType; }; @@ -25,6 +28,12 @@ type RetainedLiveThread = { release: (threadId: string) => Promise; }; +type ThreadReleaseTransition = { + completion: Promise; + physicalRelease?: Promise; + invalidated?: boolean; +}; + export type CodexAppServerLiveThreadOwnership = { configFingerprint?: string; serviceTier?: CodexServiceTier | null; @@ -37,6 +46,20 @@ const CODEX_APP_SERVER_LIVE_THREAD_IDLE_TIMEOUT_MS = 30 * 60_000; const CODEX_APP_SERVER_LIVE_THREAD_MAX_IDLE = 64; const configuredClients = new WeakMap(); +const physicalThreadReleases = new WeakMap< + CodexAppServerLiveThreadOwnership["release"], + CodexAppServerLiveThreadOwnership["release"] +>(); +const claimedThreadReleaseTokens = new WeakMap< + CodexAppServerLiveThreadOwnership["release"], + symbol +>(); + +/** Only an initialized, still-open physical client can own retained native subscriptions. */ +export function isCodexAppServerClientRuntimeLive(client: CodexAppServerClient): boolean { + const runtime = configuredClients.get(client); + return runtime !== undefined && !runtime.closed; +} /** Installs one auth-refresh handler and one rate-limit observer per physical client. */ export function ensureCodexAppServerClientRuntime( @@ -45,6 +68,9 @@ export function ensureCodexAppServerClientRuntime( ): void { const existing = configuredClients.get(client); if (existing) { + if (existing.closed) { + return; + } // Shared-client keys already isolate agent/auth identity. Keep config fresh // without installing another physical-client handler set. existing.context = context; @@ -52,17 +78,23 @@ export function ensureCodexAppServerClientRuntime( } const runtime: ClientRuntime = { context, + closed: false, retainedThreads: new Map(), + claimedThreads: new Map(), releasingThreads: new Map(), protectedThreads: new Map(), }; configuredClients.set(client, runtime); client.addCloseHandler(() => { + // Pending releases may settle after close; their continuations must never + // resurrect subscriptions or eviction timers on a dead physical client. + runtime.closed = true; if (runtime.evictionTimer) { clearTimeout(runtime.evictionTimer); runtime.evictionTimer = undefined; } runtime.retainedThreads.clear(); + runtime.claimedThreads.clear(); runtime.protectedThreads.clear(); }); client.addRequestHandler(async (request) => { @@ -94,8 +126,13 @@ export function ensureCodexAppServerClientRuntime( const threadId = (notification.params as { threadId?: unknown } | undefined)?.threadId; if (typeof threadId === "string") { // Codex already removed server-side ownership; unsubscribing again can - // race a replacement, so only discard this exact local idle entry. + // race a replacement, so only discard this exact local ownership. + const releasing = runtime.releasingThreads.get(threadId); + if (releasing) { + releasing.invalidated = true; + } runtime.retainedThreads.delete(threadId); + runtime.claimedThreads.delete(threadId); scheduleRetainedThreadEviction(client, runtime); } } @@ -110,6 +147,9 @@ function scheduleRetainedThreadEviction( clearTimeout(runtime.evictionTimer); runtime.evictionTimer = undefined; } + if (runtime.closed) { + return; + } let expiresAt = Number.POSITIVE_INFINITY; for (const [threadId, thread] of runtime.retainedThreads) { if (!runtime.protectedThreads.has(threadId)) { @@ -122,7 +162,13 @@ function scheduleRetainedThreadEviction( runtime.evictionTimer = setTimeout( () => { runtime.evictionTimer = undefined; - void evictExpiredRetainedThreads(client, runtime).catch(() => client.close()); + void evictExpiredRetainedThreads(client, runtime).catch((error: unknown) => { + // The subscription owner already chose safe shared-client retirement; + // force-closing here would abort unrelated still-leased conversations. + embeddedAgentLog.warn("codex retained thread expiry failed", { + reason: formatErrorMessage(error), + }); + }); }, Math.max(0, expiresAt - Date.now()), ); @@ -136,26 +182,59 @@ async function releaseRetainedThread( ): Promise { const pendingRelease = runtime.releasingThreads.get(threadId); if (pendingRelease) { - await pendingRelease; + await pendingRelease.completion; return false; } const retained = runtime.retainedThreads.get(threadId); if (!retained) { return false; } + const olderThreadIds = new Set(); + for (const candidateThreadId of runtime.retainedThreads.keys()) { + if (candidateThreadId === threadId) { + break; + } + olderThreadIds.add(candidateThreadId); + } runtime.retainedThreads.delete(threadId); scheduleRetainedThreadEviction(client, runtime); // Keep release ownership addressable until unsubscribe settles. Unrelated // conversations must stay reusable while only this thread transitions. - const release = retained.release(threadId); - runtime.releasingThreads.set(threadId, release); + const transition: ThreadReleaseTransition = { + completion: Promise.resolve().then(() => retained.release(threadId)), + }; + runtime.releasingThreads.set(threadId, transition); try { - await release; + await transition.completion; return true; + } catch (error) { + if ( + !runtime.closed && + !transition.invalidated && + runtime.releasingThreads.get(threadId) === transition && + !runtime.retainedThreads.has(threadId) && + !runtime.claimedThreads.has(threadId) + ) { + // A failed unsubscribe leaves the native subscription alive. Restore its + // exact callback and LRU position; renewing TTL prevents a zero-delay retry spin. + retained.expiresAt = Date.now() + CODEX_APP_SERVER_LIVE_THREAD_IDLE_TIMEOUT_MS; + const newerThreads = [...runtime.retainedThreads.entries()].filter( + ([candidateThreadId]) => !olderThreadIds.has(candidateThreadId), + ); + for (const [candidateThreadId] of newerThreads) { + runtime.retainedThreads.delete(candidateThreadId); + } + runtime.retainedThreads.set(threadId, retained); + for (const [candidateThreadId, newerThread] of newerThreads) { + runtime.retainedThreads.set(candidateThreadId, newerThread); + } + } + throw error; } finally { - if (runtime.releasingThreads.get(threadId) === release) { + if (runtime.releasingThreads.get(threadId) === transition) { runtime.releasingThreads.delete(threadId); } + scheduleRetainedThreadEviction(client, runtime); } } @@ -196,32 +275,68 @@ export async function retainCodexAppServerLiveThread( serviceTier?: CodexServiceTier | null, ): Promise { const runtime = configuredClients.get(client); - if (!runtime) { + if (!runtime || runtime.closed) { + return false; + } + const claimed = runtime.claimedThreads.get(threadId); + if ( + claimed !== undefined && + (releaseThread === undefined || claimedThreadReleaseTokens.get(releaseThread) !== claimed) + ) { + // Only the active generation's branded ownership handle may republish it; + // manual resume or another turn must never steal an in-flight subscription. return false; } const pendingRelease = runtime.releasingThreads.get(threadId); if (pendingRelease) { - await pendingRelease; + await pendingRelease.completion; + if ( + runtime.closed || + (claimed !== undefined && runtime.claimedThreads.get(threadId) !== claimed) + ) { + // The pending operation unsubscribed this exact active generation; + // publishing it again without thread/resume would invent ownership. + return false; + } } runtime.retainedThreads.delete(threadId); - runtime.retainedThreads.set(threadId, { + const retained: RetainedLiveThread = { configFingerprint, serviceTier, expiresAt: Date.now() + CODEX_APP_SERVER_LIVE_THREAD_IDLE_TIMEOUT_MS, release: - releaseThread ?? + (releaseThread ? (physicalThreadReleases.get(releaseThread) ?? releaseThread) : undefined) ?? (async (releasedThreadId) => { - await client.request( - "thread/unsubscribe", - { threadId: releasedThreadId }, - { timeoutMs: 5_000 }, - ); + await unsubscribeCodexAppServerLiveThread(client, releasedThreadId, 5_000); }), - }); + }; + runtime.retainedThreads.set(threadId, retained); // Map insertion order is the LRU. Active turns are claimed out of this map, // and detached native-child parents are pinned until their final child settles. - await evictExcessIdleThreads(client, runtime); + try { + await evictExcessIdleThreads(client, runtime); + } catch (error) { + // Capacity eviction can retire the physical client. Never leave the new + // thread published when its caller must instead release active ownership. + if (runtime.retainedThreads.get(threadId) === retained) { + runtime.retainedThreads.delete(threadId); + } + scheduleRetainedThreadEviction(client, runtime); + embeddedAgentLog.warn("codex retained thread capacity eviction failed", { + threadId, + reason: formatErrorMessage(error), + }); + return false; + } + if (runtime.closed) { + return false; + } + // A turn owns its claimed subscription until its idle replacement actually + // survives capacity eviction; a failed publish must remain fail-closed. + if (claimed !== undefined && runtime.claimedThreads.get(threadId) === claimed) { + runtime.claimedThreads.delete(threadId); + } scheduleRetainedThreadEviction(client, runtime); return true; } @@ -233,12 +348,12 @@ export async function consumeCodexAppServerLiveThread( configFingerprint?: string, ): Promise { const runtime = configuredClients.get(client); - if (!runtime) { + if (!runtime || runtime.closed) { return undefined; } const pendingRelease = runtime.releasingThreads.get(threadId); if (pendingRelease) { - await pendingRelease; + await pendingRelease.completion; return undefined; } const retained = runtime.retainedThreads.get(threadId); @@ -248,15 +363,139 @@ export async function consumeCodexAppServerLiveThread( ) { return undefined; } + return claimCodexAppServerThreadOwnership(client, runtime, threadId, retained); +} + +/** Claims an observed Codex auto-subscription without exposing a temporarily idle owner. */ +export async function claimCodexAppServerLiveThread( + client: CodexAppServerClient, + threadId: string, +): Promise { + const runtime = configuredClients.get(client); + if (!runtime || runtime.closed || runtime.claimedThreads.has(threadId)) { + return undefined; + } + const pendingRelease = runtime.releasingThreads.get(threadId); + if (pendingRelease) { + // A pending unsubscribe can invalidate the observed subscription; it must + // never be resurrected after its physical connection acknowledges release. + await pendingRelease.completion; + return undefined; + } + const retained = runtime.retainedThreads.get(threadId) ?? { + expiresAt: Date.now() + CODEX_APP_SERVER_LIVE_THREAD_IDLE_TIMEOUT_MS, + release: async (releasedThreadId: string) => { + await unsubscribeCodexAppServerLiveThread(client, releasedThreadId, 5_000); + }, + }; + return claimCodexAppServerThreadOwnership(client, runtime, threadId, retained); +} + +function claimCodexAppServerThreadOwnership( + client: CodexAppServerClient, + runtime: ClientRuntime, + threadId: string, + retained: RetainedLiveThread, +): CodexAppServerLiveThreadOwnership { runtime.retainedThreads.delete(threadId); + const claimed = Symbol(threadId); + runtime.claimedThreads.set(threadId, claimed); scheduleRetainedThreadEviction(client, runtime); + const release = async (releasedThreadId: string): Promise => { + // Codex subscriptions have no generation identifier. An obsolete owner + // must be rejected before it can unsubscribe a replacement's live turn. + if (releasedThreadId !== threadId || runtime.claimedThreads.get(threadId) !== claimed) { + return; + } + const pendingRelease = runtime.releasingThreads.get(threadId); + if (pendingRelease) { + await pendingRelease.completion; + return; + } + // Publish the transition before invoking the physical callback so a new + // retain/claim cannot slip in while Codex acknowledges unsubscribe. + const transition: ThreadReleaseTransition = { + completion: Promise.resolve().then(async () => { + if (runtime.closed || runtime.claimedThreads.get(threadId) !== claimed) { + return; + } + await retained.release(releasedThreadId); + }), + }; + runtime.releasingThreads.set(threadId, transition); + try { + await transition.completion; + if (runtime.claimedThreads.get(threadId) === claimed) { + runtime.claimedThreads.delete(threadId); + } + } finally { + if (runtime.releasingThreads.get(threadId) === transition) { + runtime.releasingThreads.delete(threadId); + } + } + }; + // Compaction and bound turns transfer this callback back into idle storage; + // the successor must inherit its raw release, never the obsolete token guard. + physicalThreadReleases.set(release, retained.release); + claimedThreadReleaseTokens.set(release, claimed); return { configFingerprint: retained.configFingerprint, serviceTier: retained.serviceTier, - release: retained.release, + release, }; } +/** Distinguish active claimed ownership from an already-evicted idle subscription. */ +export function isCodexAppServerLiveThreadClaimed( + client: CodexAppServerClient, + threadId: string, +): boolean { + const runtime = configuredClients.get(client); + return runtime !== undefined && !runtime.closed && runtime.claimedThreads.has(threadId); +} + +/** Release the exact physical subscription and finish only its observed claim generation. */ +export async function unsubscribeCodexAppServerLiveThread( + client: CodexAppServerClient, + threadId: string, + timeoutMs: number, +): Promise { + const runtime = configuredClients.get(client); + const claimed = runtime?.claimedThreads.get(threadId); + let transition = runtime?.releasingThreads.get(threadId); + if (transition?.physicalRelease) { + await transition.physicalRelease; + return; + } + const physicalRelease = Promise.resolve().then(async () => { + if (claimed !== undefined && runtime?.claimedThreads.get(threadId) !== claimed) { + return; + } + await client.request("thread/unsubscribe", { threadId }, { timeoutMs }); + }); + const ownsTransition = runtime !== undefined && transition === undefined; + if (transition) { + // Idle/claimed owners may invoke this helper from inside their own + // transition; join its one RPC slot instead of joining ourselves. + transition.physicalRelease = physicalRelease; + } else if (runtime) { + transition = { completion: physicalRelease, physicalRelease }; + runtime.releasingThreads.set(threadId, transition); + } + try { + await physicalRelease; + // Warm/compaction/native-child cleanup can release directly. Their exact + // successful RPC ends only the claim generation observed before it began. + if (claimed !== undefined && runtime?.claimedThreads.get(threadId) === claimed) { + runtime.claimedThreads.delete(threadId); + } + } finally { + if (ownsTransition && runtime.releasingThreads.get(threadId) === transition) { + runtime.releasingThreads.delete(threadId); + } + } +} + /** Reset/end owns the exact thread; failed generation retirement must never release its successor. */ export async function releaseCodexAppServerLiveThread( client: CodexAppServerClient, @@ -272,7 +511,7 @@ export function protectCodexAppServerLiveThread( threadId: string, ): () => void { const runtime = configuredClients.get(client); - if (!runtime) { + if (!runtime || runtime.closed) { return () => undefined; } runtime.protectedThreads.set(threadId, (runtime.protectedThreads.get(threadId) ?? 0) + 1); @@ -283,6 +522,9 @@ export function protectCodexAppServerLiveThread( return; } protectedThread = false; + if (runtime.closed) { + return; + } const count = runtime.protectedThreads.get(threadId) ?? 0; if (count <= 1) { runtime.protectedThreads.delete(threadId); @@ -300,6 +542,11 @@ export function protectCodexAppServerLiveThread( runtime.protectedThreads.set(threadId, count - 1); } scheduleRetainedThreadEviction(client, runtime); - void evictExcessIdleThreads(client, runtime).catch(() => client.close()); + void evictExcessIdleThreads(client, runtime).catch((error: unknown) => { + embeddedAgentLog.warn("codex retained thread unpin eviction failed", { + threadId, + reason: formatErrorMessage(error), + }); + }); }; } diff --git a/extensions/codex/src/app-server/compact.test.ts b/extensions/codex/src/app-server/compact.test.ts index b5090d245ee4..e4ace2f6f71b 100644 --- a/extensions/codex/src/app-server/compact.test.ts +++ b/extensions/codex/src/app-server/compact.test.ts @@ -265,6 +265,37 @@ describe("maybeCompactCodexAppServerSession", () => { ).resolves.toEqual(expect.objectContaining({ configFingerprint: "config-thread-2" })); }); + it("releases an obsolete physical owner when compaction migrates the same native thread", async () => { + const fake = createFakeCodexClient({ autoCompleteCompaction: false }); + setCodexAppServerClientFactoryForTest(async () => fake.client); + const sessionFile = await writeTestBinding({ clientId: "client-before-compaction" }); + const pending = startCompaction(sessionFile); + await vi.waitFor(() => { + expect(fake.request).toHaveBeenCalledWith("thread/compact/start", { threadId: "thread-1" }); + }); + + seedCodexTestBinding(sessionFile, { + threadId: "thread-1", + clientId: "client-after-compaction", + cwd: tempDir, + }); + fake.completeCompaction(); + + await expect(pending).resolves.toMatchObject({ ok: true, compacted: true }); + expect(fake.request.mock.calls.filter(([method]) => method === "thread/unsubscribe")).toEqual([ + [ + "thread/unsubscribe", + { threadId: "thread-1" }, + expect.objectContaining({ timeoutMs: expect.any(Number) }), + ], + ]); + await expect(consumeCodexAppServerLiveThread(fake.client, "thread-1")).resolves.toBeUndefined(); + await expect(readCodexAppServerBinding(sessionFile)).resolves.toMatchObject({ + threadId: "thread-1", + clientId: "client-after-compaction", + }); + }); + it("preserves an incognito thread's separately owned live subscription", async () => { const fake = createFakeCodexClient({ retainedThreadId: null, @@ -538,6 +569,70 @@ describe("maybeCompactCodexAppServerSession", () => { ).toBeUndefined(); }); + it("clears bootstrap projection before manual native compaction rewrites thread history", async () => { + const fake = createFakeCodexClient(); + setCodexAppServerClientFactoryForTest(async () => fake.client); + const sessionFile = await writeTestBinding({ + contextEngine: { + schemaVersion: 1, + engineId: "lossless-claw", + policyFingerprint: "policy-1", + projection: { + schemaVersion: 1, + mode: "thread_bootstrap", + epoch: "epoch-1", + fingerprint: "fingerprint-1", + }, + }, + }); + + await expect(startCompaction(sessionFile)).resolves.toMatchObject({ + ok: true, + compacted: true, + }); + expect( + (await readCodexAppServerBinding(sessionFile))?.contextEngine?.projection, + ).toBeUndefined(); + }); + + it("preserves projected context and warm ownership when native compaction is rejected", async () => { + const fake = createFakeCodexClient(); + fake.request.mockRejectedValueOnce( + new CodexAppServerRpcError( + { code: -32_600, message: "compaction temporarily unavailable" }, + "thread/compact/start", + ), + ); + setCodexAppServerClientFactoryForTest(async () => fake.client); + const projection = { + schemaVersion: 1 as const, + mode: "thread_bootstrap" as const, + epoch: "epoch-1", + fingerprint: "fingerprint-1", + }; + const sessionFile = await writeTestBinding({ + contextEngine: { + schemaVersion: 1, + engineId: "lossless-claw", + policyFingerprint: "policy-1", + projection, + }, + }); + + await expect(startCompaction(sessionFile)).resolves.toMatchObject({ + ok: false, + compacted: false, + reason: "compaction temporarily unavailable", + }); + expect((await readCodexAppServerBinding(sessionFile))?.contextEngine?.projection).toEqual( + projection, + ); + expect(fake.request.mock.calls.map(([method]) => method)).toEqual(["thread/compact/start"]); + await expect(consumeCodexAppServerLiveThread(fake.client, "thread-1")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + }); + it("preserves projection when aborted before guarded native compaction", async () => { const fake = createFakeCodexClient(); setCodexAppServerClientFactoryForTest(async () => fake.client); @@ -1715,31 +1810,39 @@ describe("maybeCompactCodexAppServerSession", () => { async function runWithIgnoredOverrides(index: number): Promise { const agentId = `cap-${index}`; - await maybeCompactCodexAppServerSession({ - sessionId: `session-${index}`, - sessionKey: `agent:${agentId}:session-${index}`, - sessionFile: path.join(tempDir, `${agentId}.jsonl`), - workspaceDir: tempDir, - trigger: "budget", - config: { - agents: { - list: [ - { - id: agentId, - compaction: { - model: "openai/gpt-5.4-mini", - provider: "custom-summary", + await maybeCompactCodexAppServerSessionImpl( + { + sessionId: `session-${index}`, + sessionKey: `agent:${agentId}:session-${index}`, + sessionFile: path.join(tempDir, `${agentId}.jsonl`), + workspaceDir: tempDir, + trigger: "budget", + config: { + agents: { + list: [ + { + id: agentId, + compaction: { + model: "openai/gpt-5.4-mini", + provider: "custom-summary", + }, }, - }, - ], + ], + }, }, }, - }); + { bindingStore: testCodexAppServerBindingStore }, + ); } // Fill exactly the advertised 4,096-entry cap: every distinct key warns once. for (let index = 0; index < 4_096; index += 1) { await runWithIgnoredOverrides(index); + if ((index + 1) % 256 === 0) { + // Thousands of resolved promises otherwise starve Vitest timeout and + // progress callbacks without increasing real LRU-cap coverage. + await flushAsyncTasks(1); + } } expect(warn).toHaveBeenCalledTimes(4_096); diff --git a/extensions/codex/src/app-server/compact.ts b/extensions/codex/src/app-server/compact.ts index 6b03fdb0fbb6..d020459e3b0a 100644 --- a/extensions/codex/src/app-server/compact.ts +++ b/extensions/codex/src/app-server/compact.ts @@ -1,6 +1,7 @@ /** * Native Codex app-server compaction bridge for bound OpenClaw sessions. */ +import { AsyncLocalStorage } from "node:async_hooks"; import { embeddedAgentLog, resolveCompactionTimeoutMs, @@ -45,6 +46,7 @@ import { releaseLeasedSharedCodexAppServerClient, type CodexAppServerClientFactory, } from "./shared-client.js"; +import { isSameCodexAppServerThreadOwner } from "./thread-ownership.js"; import { resumeCodexAppServerThread } from "./thread-resume.js"; // ttlMs: 0 retains keys until the 4,096-entry LRU cap evicts them, after which a @@ -77,6 +79,7 @@ function watchCodexNativeCompactionCompletion(params: { retireUnconfirmedRequest: (reason: string) => Promise; cancel: () => void; } { + const runOutsideBindingLease = AsyncLocalStorage.snapshot(); let settled = false; let requestStarted = false; let abortRequested = false; @@ -111,8 +114,9 @@ function watchCodexNativeCompactionCompletion(params: { return; } retirementStarted = true; - void params - .retireUnconfirmed() + // Timers started under the short-lived binding lease inherit its async + // owner. Remote retirement must not reuse that already-released token. + void runOutsideBindingLease(() => params.retireUnconfirmed()) .then(() => finish({ completed: false, reason })) .catch((error: unknown) => { embeddedAgentLog.error("failed to retire unconfirmed codex app-server compaction", { @@ -552,6 +556,7 @@ async function compactCodexNativeThread( let releaseThreadSubscription: (() => Promise) | undefined; let retainedThreadOwnership: CodexAppServerLiveThreadOwnership | undefined; let compactionSucceeded = false; + let compactionRequestDefinitelyRejected = false; const releaseCompactionThread = async (threadId: string) => { if ( await unsubscribeCodexThreadBestEffort(client, { @@ -651,84 +656,85 @@ async function compactCodexNativeThread( } }; try { - if (options.allowNonManualNativeRequest) { - const guardedResult = await options.bindingStore.withLease( - bindingIdentity, - async () => { - const currentBinding = await options.bindingStore.read(bindingIdentity); - if (params.abortSignal?.aborted) { - return { - started: false as const, - result: skippedCodexNativeCompactionResult(params, { - reason: "codex app-server compaction aborted before native compaction", - code: "aborted_before_native_compaction", - expectedThreadId: binding.threadId, - currentThreadId: currentBinding?.threadId, - }), - }; - } - if (!currentBinding || !isSameNativeCompactionBinding(currentBinding, binding)) { - embeddedAgentLog.warn( - "skipping codex app-server compaction because the thread binding changed", - { - sessionId: params.sessionId, - sessionKey: params.sessionKey, - expectedThreadId: binding.threadId, - currentThreadId: currentBinding?.threadId, - }, - ); - return { - started: false as const, - result: skippedCodexNativeCompactionResult(params, { + const guardedResult = await options.bindingStore.withLease(bindingIdentity, async () => { + const currentBinding = await options.bindingStore.read(bindingIdentity); + if (params.abortSignal?.aborted) { + if (!options.allowNonManualNativeRequest) { + params.abortSignal.throwIfAborted(); + } + return { + started: false as const, + result: skippedCodexNativeCompactionResult(params, { + reason: "codex app-server compaction aborted before native compaction", + code: "aborted_before_native_compaction", + expectedThreadId: binding.threadId, + currentThreadId: currentBinding?.threadId, + }), + }; + } + if (!currentBinding || !isSameNativeCompactionBinding(currentBinding, binding)) { + embeddedAgentLog.warn( + "skipping codex app-server compaction because the thread binding changed", + { + sessionId: params.sessionId, + sessionKey: params.sessionKey, + expectedThreadId: binding.threadId, + currentThreadId: currentBinding?.threadId, + }, + ); + return { + started: false as const, + result: options.allowNonManualNativeRequest + ? skippedCodexNativeCompactionResult(params, { reason: "codex app-server binding changed before native compaction", code: "binding_changed_before_native_compaction", expectedThreadId: binding.threadId, currentThreadId: currentBinding?.threadId, + }) + : failedCodexThreadBindingCompactionResult(params, { + threadId: binding.threadId, + reason: "codex app-server binding changed before native compaction", + recovery: "stale_thread_binding", }), - }; - } - binding = currentBinding; - const guardedRequestTimeoutMs = Math.min( + }; + } + binding = currentBinding; + const guardedRequestTimeoutMs = options.allowNonManualNativeRequest + ? Math.min( appServer.requestTimeoutMs, CODEX_APP_SERVER_BINDING_GUARDED_REQUEST_TIMEOUT_MS, - ); - await acquireThreadSubscription(guardedRequestTimeoutMs); - await clearContextEngineProjectionBeforeNativeCompaction({ - sessionId: params.sessionId, - bindingStore: options.bindingStore, - identity: bindingIdentity, + ) + : undefined; + await acquireThreadSubscription(guardedRequestTimeoutMs); + await clearContextEngineProjectionBeforeNativeCompaction({ + sessionId: params.sessionId, + bindingStore: options.bindingStore, + identity: bindingIdentity, + binding, + }); + try { + await beginNativeCompactionRequest(guardedRequestTimeoutMs); + return { started: true as const, accepted: true as const }; + } catch (error) { + if (error instanceof CodexAppServerRpcError) { + // A server rejection proves native history was untouched; an + // ambiguous transport failure must never restore stale context. + await options.bindingStore.mutate(bindingIdentity, { + kind: "set", binding, }); - try { - await beginNativeCompactionRequest(guardedRequestTimeoutMs); - return { started: true as const, accepted: true as const }; - } catch (error) { - await options.bindingStore.mutate(bindingIdentity, { - kind: "set", - binding, - }); - // Retire outside the binding lock: remote detach acquires this - // same lock and would otherwise deadlock the failure path. - return { started: true as const, accepted: false as const, error }; - } - }, - ); - if (!guardedResult.started) { - return guardedResult.result; - } - if (!guardedResult.accepted) { - await settleNativeCompactionRequestError(guardedResult.error); - throw guardedResult.error; - } - } else { - params.abortSignal?.throwIfAborted(); - await acquireThreadSubscription(); - try { - await beginNativeCompactionRequest(); - } catch (error) { - await settleNativeCompactionRequestError(error); - throw error; + compactionRequestDefinitelyRejected = !isCodexThreadNotFoundError(error); + } + // Retirement can acquire this same generation lease. + return { started: true as const, accepted: false as const, error }; } + }); + if (!guardedResult.started) { + return guardedResult.result; + } + if (!guardedResult.accepted) { + await settleNativeCompactionRequestError(guardedResult.error); + throw guardedResult.error; } embeddedAgentLog.info("started codex app-server compaction", { sessionId: params.sessionId, @@ -765,16 +771,19 @@ async function compactCodexNativeThread( } finally { completionWatch.cancel(); try { - if (compactionSucceeded && retainedThreadOwnership) { + if ( + (compactionSucceeded || compactionRequestDefinitelyRejected) && + retainedThreadOwnership + ) { const ownership = retainedThreadOwnership; const currentBinding = await options.bindingStore.read(bindingIdentity); // Reset uses this same generation lease; without it compaction // could return an obsolete subscription after its owner ended. const retained = - currentBinding?.threadId === binding.threadId && + isSameCodexAppServerThreadOwner(currentBinding, binding) && (await options.bindingStore.withLease(bindingIdentity, async () => { const leasedBinding = await options.bindingStore.read(bindingIdentity); - if (leasedBinding?.threadId !== binding.threadId) { + if (!isSameCodexAppServerThreadOwner(leasedBinding, binding)) { return false; } return await retainCodexAppServerLiveThread( @@ -935,7 +944,7 @@ function isSameNativeCompactionBinding( expected: CodexAppServerThreadBinding, ): boolean { return ( - current.threadId === expected.threadId && + isSameCodexAppServerThreadOwner(current, expected) && current.authProfileId === expected.authProfileId && current.contextEngine?.engineId === expected.contextEngine?.engineId && current.contextEngine?.policyFingerprint === expected.contextEngine?.policyFingerprint && diff --git a/extensions/codex/src/app-server/native-subagent-monitor.test.ts b/extensions/codex/src/app-server/native-subagent-monitor.test.ts index c2964f26f143..482fb5392772 100644 --- a/extensions/codex/src/app-server/native-subagent-monitor.test.ts +++ b/extensions/codex/src/app-server/native-subagent-monitor.test.ts @@ -5,6 +5,14 @@ import type { AgentHarnessTaskRuntimeScope, } from "openclaw/plugin-sdk/agent-harness-task-runtime"; import { describe, expect, it, vi } from "vitest"; +import { + claimCodexAppServerLiveThread, + consumeCodexAppServerLiveThread, + ensureCodexAppServerClientRuntime, + isCodexAppServerLiveThreadClaimed, + releaseCodexAppServerLiveThread, + retainCodexAppServerLiveThread, +} from "./client-runtime.js"; import { createFakeCodexAppServerClient } from "./codex-app-server.test-fixtures.js"; import { codexNativeSubagentMonitorRuntime } from "./native-subagent-monitor.js"; import type { @@ -72,6 +80,7 @@ function createClient() { threadTurns.set(childThreadId, response); }, addNotificationHandler: fixture.client.addNotificationHandler.bind(fixture.client), + addRequestHandler: fixture.client.addRequestHandler.bind(fixture.client), addCloseHandler: fixture.client.addCloseHandler.bind(fixture.client), notify: (notification: CodexServerNotification) => fixture.notify(notification), close: () => fixture.close(), @@ -212,6 +221,31 @@ function nativeCompletionNotification( }; } +function closeAgentNotification(params: { + method: "item/started" | "item/completed"; + childThreadId?: string; + previousStatus?: "completed" | "running"; +}): CodexServerNotification { + const childThreadId = params.childThreadId ?? "child-thread"; + return { + method: params.method, + params: { + threadId: "parent-thread", + item: { + type: "collabAgentToolCall", + tool: "closeAgent", + status: params.method === "item/started" ? "inProgress" : "completed", + senderThreadId: "parent-thread", + receiverThreadIds: [childThreadId], + agentsStates: + params.method === "item/completed" + ? { [childThreadId]: { status: params.previousStatus ?? "completed" } } + : {}, + }, + }, + }; +} + function childTurnCompletedNotification(params: { status: "completed" | "failed" | "interrupted"; error?: string; @@ -372,6 +406,118 @@ describe("CodexNativeSubagentMonitor", () => { expect(releaseParentThread).toHaveBeenCalledOnce(); }); + it("retains completed-open children in the bounded owner and reclaims them for follow-up", async () => { + const client = createClient(); + const runtime = createRuntime(); + const claimChildThread = vi.fn(async () => undefined); + const retainChildThread = vi.fn(async () => true); + const retainParentThread = vi.fn(() => vi.fn()); + const monitor = new CodexNativeSubagentMonitor(client as never, runtime, { + claimChildThread, + retainChildThread, + retainParentThread, + }); + registerParent(monitor); + + await notifyChildStarted(client); + await client.notify(nativeCompletionNotification()); + + expect(claimChildThread).toHaveBeenCalledExactlyOnceWith("child-thread"); + expect(retainChildThread).toHaveBeenCalledExactlyOnceWith("child-thread"); + + await client.notify({ + method: "item/started", + params: { + threadId: "parent-thread", + item: { + type: "collabAgentToolCall", + tool: "sendInput", + senderThreadId: "parent-thread", + receiverThreadIds: ["child-thread"], + }, + }, + }); + + expect(claimChildThread).toHaveBeenCalledTimes(2); + expect(retainParentThread).toHaveBeenCalledTimes(2); + monitor.dispose(); + }); + + it("does not resurrect completed children or repin parents when closeAgent runs", async () => { + const client = createClient(); + const runtime = createRuntime(); + const releaseParentThread = vi.fn(); + const retainParentThread = vi.fn(() => releaseParentThread); + const retainChildThread = vi.fn(async () => true); + const releaseChildThread = vi.fn(async () => true); + const monitor = new CodexNativeSubagentMonitor(client as never, runtime, { + retainParentThread, + retainChildThread, + releaseChildThread, + }); + registerParent(monitor); + + await notifyChildStarted(client); + await client.notify(nativeCompletionNotification()); + expect(releaseParentThread).toHaveBeenCalledOnce(); + + await client.notify(closeAgentNotification({ method: "item/started" })); + await client.notify(closeAgentNotification({ method: "item/completed" })); + + expect(retainParentThread).toHaveBeenCalledExactlyOnceWith("parent-thread"); + expect(releaseParentThread).toHaveBeenCalledOnce(); + expect(retainChildThread).toHaveBeenCalledExactlyOnceWith("child-thread"); + expect(releaseChildThread).toHaveBeenCalledExactlyOnceWith("child-thread"); + expect(runtime.deliverAgentHarnessTaskCompletion).toHaveBeenCalledOnce(); + monitor.dispose(); + }); + + it("cancels running children and releases their parent pin when closeAgent completes", async () => { + const client = createClient(); + const runtime = createRuntime(); + const releaseParentThread = vi.fn(); + const monitor = new CodexNativeSubagentMonitor(client as never, runtime, { + retainParentThread: () => releaseParentThread, + }); + registerParent(monitor); + + await notifyChildStarted(client); + await client.notify( + closeAgentNotification({ method: "item/completed", previousStatus: "running" }), + ); + await client.notify(nativeCompletionNotification()); + + expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith( + expect.objectContaining({ runId: "codex-thread:child-thread", status: "cancelled" }), + ); + expect(releaseParentThread).toHaveBeenCalledOnce(); + expect(runtime.deliverAgentHarnessTaskCompletion).not.toHaveBeenCalled(); + monitor.dispose(); + }); + + it("retires parent generations idempotently and fences late child completions", async () => { + const client = createClient(); + const runtime = createRuntime(); + const releaseParentThread = vi.fn(); + const monitor = new CodexNativeSubagentMonitor(client as never, runtime, { + retainParentThread: () => releaseParentThread, + }); + const parent = registerParent(monitor); + await notifyChildStarted(client); + + monitor.retireParent("parent-thread"); + monitor.retireParent("parent-thread"); + parent.unregister(); + await client.notify(nativeCompletionNotification()); + + expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ runId: "codex-thread:child-thread", status: "cancelled" }), + ); + expect(releaseParentThread).toHaveBeenCalledOnce(); + expect(runtime.deliverAgentHarnessTaskCompletion).not.toHaveBeenCalled(); + monitor.dispose(); + }); + it("keeps native subagent task mirroring on the shared client", async () => { const client = createClient(); const runtime = createRuntime(); @@ -1801,6 +1947,9 @@ describe("CodexNativeSubagentMonitor", () => { it("ref-counts shared parent registrations", async () => { const client = createClient(); const runtime = createRuntime(); + const childRelease = vi.fn(async () => undefined); + ensureCodexAppServerClientRuntime(client as never, { agentDir: "/tmp/agent" }); + await retainCodexAppServerLiveThread(client as never, "child-thread", childRelease); const first = registerCodexNativeSubagentMonitor({ client: client as never, parentThreadId: "parent-thread", @@ -1817,7 +1966,17 @@ describe("CodexNativeSubagentMonitor", () => { }); first.unregister(); await notifyChildStarted(client); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(true), + ); await client.notify(nativeCompletionNotification()); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(false), + ); + const reusedChild = await consumeCodexAppServerLiveThread(client as never, "child-thread"); + expect(reusedChild).toEqual(expect.objectContaining({ release: expect.any(Function) })); + await reusedChild?.release("child-thread"); + expect(childRelease).toHaveBeenCalledOnce(); expect(runtime.createRunningTaskRun).toHaveBeenCalledTimes(1); expect(runtime.deliverAgentHarnessTaskCompletion).toHaveBeenCalledTimes(1); @@ -1827,6 +1986,209 @@ describe("CodexNativeSubagentMonitor", () => { client.close(); }); + it("claims a fresh auto-subscribed child until completion transfers its exact owner", async () => { + const client = createClient(); + const runtime = createRuntime(); + client.request.mockImplementation(async (method) => { + if (method === "thread/unsubscribe") { + return {} as never; + } + throw new Error(`unexpected request: ${method}`); + }); + ensureCodexAppServerClientRuntime(client as never, { agentDir: "/tmp/agent" }); + const parent = registerCodexNativeSubagentMonitor({ + client: client as never, + parentThreadId: "parent-thread", + requesterSessionKey: "agent:main:main", + taskRuntimeScope: createTaskScope("agent:main:main"), + runtime, + }); + + await notifyChildStarted(client); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(true), + ); + await expect(retainCodexAppServerLiveThread(client as never, "child-thread")).resolves.toBe( + false, + ); + await expect( + consumeCodexAppServerLiveThread(client as never, "child-thread"), + ).resolves.toBeUndefined(); + expect(client.request).not.toHaveBeenCalled(); + + await client.notify(nativeCompletionNotification()); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(false), + ); + const completed = await consumeCodexAppServerLiveThread(client as never, "child-thread"); + expect(completed).toEqual(expect.objectContaining({ release: expect.any(Function) })); + await completed?.release("child-thread"); + + expect(client.request).toHaveBeenCalledExactlyOnceWith( + "thread/unsubscribe", + { threadId: "child-thread" }, + { timeoutMs: 5_000 }, + ); + parent.unregister(); + client.close(); + }); + + it("releases a completed native child when its full idle pool cannot evict its oldest owner", async () => { + const client = createClient(); + const runtime = createRuntime(); + client.request.mockImplementation(async (method) => { + if (method === "thread/unsubscribe") { + return {} as never; + } + throw new Error(`unexpected request: ${method}`); + }); + ensureCodexAppServerClientRuntime(client as never, { agentDir: "/tmp/agent" }); + const oldestRelease = vi + .fn<(threadId: string) => Promise>() + .mockRejectedValueOnce(new Error("oldest native subscription could not be released")) + .mockResolvedValueOnce(undefined); + await retainCodexAppServerLiveThread(client as never, "thread-oldest", oldestRelease); + for (let index = 1; index < 64; index += 1) { + await retainCodexAppServerLiveThread(client as never, `thread-sibling-${index}`); + } + const releaseParentThread = vi.fn(); + const parent = registerCodexNativeSubagentMonitor({ + client: client as never, + parentThreadId: "parent-thread", + requesterSessionKey: "agent:main:main", + taskRuntimeScope: createTaskScope("agent:main:main"), + runtime, + retainParentThread: () => releaseParentThread, + }); + + await notifyChildStarted(client); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(true), + ); + await client.notify(nativeCompletionNotification()); + + await vi.waitFor(() => + expect(client.request).toHaveBeenCalledExactlyOnceWith( + "thread/unsubscribe", + { threadId: "child-thread" }, + { timeoutMs: 5_000 }, + ), + ); + expect(oldestRelease).toHaveBeenCalledExactlyOnceWith("thread-oldest"); + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(false); + await expect( + consumeCodexAppServerLiveThread(client as never, "child-thread"), + ).resolves.toBeUndefined(); + const oldest = await consumeCodexAppServerLiveThread(client as never, "thread-oldest"); + expect(oldest).toEqual(expect.objectContaining({ release: expect.any(Function) })); + await expect( + retainCodexAppServerLiveThread(client as never, "thread-oldest", oldest?.release), + ).resolves.toBe(true); + await expect( + consumeCodexAppServerLiveThread(client as never, "thread-sibling-1"), + ).resolves.toEqual(expect.objectContaining({ release: expect.any(Function) })); + expect(releaseParentThread).toHaveBeenCalledOnce(); + + parent.unregister(); + client.close(); + }); + + it("releases the exact retained completed child when its original parent closes it", async () => { + const client = createClient(); + const runtime = createRuntime(); + client.request.mockImplementation(async (method) => { + if (method === "thread/unsubscribe") { + return {} as never; + } + throw new Error(`unexpected request: ${method}`); + }); + ensureCodexAppServerClientRuntime(client as never, { agentDir: "/tmp/agent" }); + const parent = registerCodexNativeSubagentMonitor({ + client: client as never, + parentThreadId: "parent-thread", + requesterSessionKey: "agent:main:main", + taskRuntimeScope: createTaskScope("agent:main:main"), + runtime, + }); + + await notifyChildStarted(client); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(true), + ); + await client.notify(nativeCompletionNotification()); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(false), + ); + + await client.notify(closeAgentNotification({ method: "item/completed" })); + await vi.waitFor(() => + expect(client.request).toHaveBeenCalledExactlyOnceWith( + "thread/unsubscribe", + { threadId: "child-thread" }, + { timeoutMs: 5_000 }, + ), + ); + await expect( + consumeCodexAppServerLiveThread(client as never, "child-thread"), + ).resolves.toBeUndefined(); + + parent.unregister(); + client.close(); + }); + + it("fences a stale child close after eviction and same-client replacement ownership", async () => { + const client = createClient(); + const runtime = createRuntime(); + client.request.mockImplementation(async (method) => { + if (method === "thread/unsubscribe" || method === "thread/resume") { + return {} as never; + } + throw new Error(`unexpected request: ${method}`); + }); + ensureCodexAppServerClientRuntime(client as never, { agentDir: "/tmp/agent" }); + const parent = registerCodexNativeSubagentMonitor({ + client: client as never, + parentThreadId: "parent-thread", + requesterSessionKey: "agent:main:main", + taskRuntimeScope: createTaskScope("agent:main:main"), + runtime, + }); + + await notifyChildStarted(client); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(true), + ); + await client.notify(nativeCompletionNotification()); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(false), + ); + await expect(releaseCodexAppServerLiveThread(client as never, "child-thread")).resolves.toBe( + true, + ); + expect(client.request).toHaveBeenCalledOnce(); + + await client.request("thread/resume", { threadId: "child-thread" }); + const replacement = await claimCodexAppServerLiveThread(client as never, "child-thread"); + expect(replacement).toEqual(expect.objectContaining({ release: expect.any(Function) })); + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(true); + + await client.notify(closeAgentNotification({ method: "item/completed" })); + expect(client.request.mock.calls.map(([method]) => method)).toEqual([ + "thread/unsubscribe", + "thread/resume", + ]); + expect(isCodexAppServerLiveThreadClaimed(client as never, "child-thread")).toBe(true); + + await replacement?.release("child-thread"); + expect(client.request.mock.calls.map(([method]) => method)).toEqual([ + "thread/unsubscribe", + "thread/resume", + "thread/unsubscribe", + ]); + parent.unregister(); + client.close(); + }); + it("clears child recovery timers when the app-server client closes", async () => { vi.useFakeTimers(); try { diff --git a/extensions/codex/src/app-server/native-subagent-monitor.ts b/extensions/codex/src/app-server/native-subagent-monitor.ts index 9dab292dbc29..3cc5037051d1 100644 --- a/extensions/codex/src/app-server/native-subagent-monitor.ts +++ b/extensions/codex/src/app-server/native-subagent-monitor.ts @@ -11,7 +11,14 @@ import { type AgentHarnessTaskRuntime, type AgentHarnessTaskRuntimeScope, } from "openclaw/plugin-sdk/agent-harness-task-runtime"; +import { KeyedAsyncQueue } from "openclaw/plugin-sdk/keyed-async-queue"; import { asFiniteNumber, normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { + claimCodexAppServerLiveThread, + releaseCodexAppServerLiveThread, + retainCodexAppServerLiveThread, + type CodexAppServerLiveThreadOwnership, +} from "./client-runtime.js"; import type { CodexAppServerClient } from "./client.js"; import { codexNativeSubagentNotifications as nativeSubagentNotifications, @@ -94,6 +101,7 @@ type ThreadStatusRevision = { }; type TaskRecoveryCandidate = { + parentState: ParentState; childThreadId: string; recoveryAttempt: number; requesterSessionKey: string; @@ -109,6 +117,9 @@ type MonitorOptions = { now?: () => number; retainClient?: () => (() => void) | undefined; retainParentThread?: (threadId: string) => (() => void) | undefined; + claimChildThread?: (threadId: string) => Promise; + retainChildThread?: (threadId: string) => Promise; + releaseChildThread?: (threadId: string) => Promise; }; const DEFAULT_RECOVERY_POLL_DELAYS_MS = [ @@ -158,9 +169,58 @@ function registerMonitor(params: { }): { unregister: () => void } { let monitor = monitors.get(params.client); if (!monitor) { + // Native start/completion can race; serialize each child so only its + // original claim handle may publish or release the same subscription. + const childThreadOwnership = new Map(); + const childThreadTransitions = new KeyedAsyncQueue(); monitor = new Monitor(params.client, params.runtime ?? defaultRuntime, { retainClient: params.retainClient, retainParentThread: params.retainParentThread, + claimChildThread: (threadId) => + childThreadTransitions.enqueue(threadId, async () => { + // Codex subscribes fresh children before thread/started; they have + // no idle entry yet but must already be fenced from manual adoption. + const ownership = await claimCodexAppServerLiveThread(params.client, threadId); + if (ownership) { + childThreadOwnership.set(threadId, ownership); + } + return ownership; + }), + retainChildThread: (threadId) => + childThreadTransitions.enqueue(threadId, async () => { + const ownership = childThreadOwnership.get(threadId); + let retained = false; + try { + retained = await retainCodexAppServerLiveThread( + params.client, + threadId, + ownership?.release, + ); + return retained; + } finally { + // A full idle pool can reject terminal child ownership. Release + // its exact branded claim before the monitor forgets that child. + if (!retained && ownership) { + await ownership.release(threadId); + } + if (childThreadOwnership.get(threadId) === ownership) { + childThreadOwnership.delete(threadId); + } + } + }), + releaseChildThread: (threadId) => + childThreadTransitions.enqueue(threadId, async () => { + const ownership = childThreadOwnership.get(threadId); + if (ownership) { + await ownership.release(threadId); + if (childThreadOwnership.get(threadId) === ownership) { + childThreadOwnership.delete(threadId); + } + } else { + // A bare closeAgent thread id cannot authorize a successor's subscription. + await releaseCodexAppServerLiveThread(params.client, threadId); + } + }), }); monitors.set(params.client, monitor); } @@ -174,6 +234,7 @@ function registerMonitor(params: { class Monitor { private readonly parentStates = new Map(); + private readonly retiredParentStates = new WeakSet(); private readonly childStates = new Map(); private readonly childThreadIdsByAgentPath = new Map(); private readonly taskReconciliations = new Map>(); @@ -187,6 +248,9 @@ class Monitor { private readonly removeCloseHandler: () => void; private readonly retainClient?: () => (() => void) | undefined; private readonly retainParentThread?: (threadId: string) => (() => void) | undefined; + private readonly claimChildThread?: (threadId: string) => Promise; + private readonly retainChildThread?: (threadId: string) => Promise; + private readonly releaseChildThread?: (threadId: string) => Promise; private readonly parentThreadRetentions = new Map void>(); private releaseClientRetention?: () => void; private disposed = false; @@ -204,6 +268,9 @@ class Monitor { this.now = options.now ?? Date.now; this.retainClient = options.retainClient; this.retainParentThread = options.retainParentThread; + this.claimChildThread = options.claimChildThread; + this.retainChildThread = options.retainChildThread; + this.releaseChildThread = options.releaseChildThread; this.removeNotificationHandler = client.addNotificationHandler(async (notification) => { if (!NATIVE_SUBAGENT_NOTIFICATION_METHODS.has(notification.method)) { return; @@ -304,7 +371,7 @@ class Monitor { } registered = false; const current = this.parentStates.get(parentThreadId); - if (current) { + if (current === registeredState) { current.ownerCount -= 1; this.pruneParentIfUnused(current); } @@ -312,6 +379,26 @@ class Monitor { }; } + retireParent(parentThreadIdInput: string): void { + const parentThreadId = parentThreadIdInput.trim(); + const state = this.parentStates.get(parentThreadId); + if (!state) { + return; + } + // Reset invalidates this exact registration generation. Pending recovery + // must not recreate its parent or deliver old children into a replacement. + this.retiredParentStates.add(state); + state.ownerCount = 0; + for (const childState of Array.from(this.childStates.values())) { + if (childState.parentThreadId === parentThreadId) { + this.retireChild(state, childState, "Codex native subagent parent session ended."); + } + } + if (this.parentStates.get(parentThreadId) === state) { + this.parentStates.delete(parentThreadId); + } + } + private prepareParentTaskRuntime(state: ParentState): void { if (!state.requesterSessionKey || !state.taskRuntimeScope) { return; @@ -372,6 +459,9 @@ class Monitor { }); } } + if (state) { + this.handleClosedChild(notification, state); + } const childState = threadId ? this.childStates.get(threadId) : undefined; if (notification.method === "turn/started" && childState) { this.resumeChild(childState); @@ -639,6 +729,11 @@ class Monitor { return state; } const isSpawnAgentTool = normalizeIdentifier(readString(item, "tool")) === "spawnagent"; + if (normalizeIdentifier(readString(item, "tool")) === "closeagent") { + // closeAgent names an existing child before shutdown; treating its + // receiver as discovery resurrects completed tasks and repins parents. + return state; + } const childThreadIds = isSpawnAgentTool ? new Set([ ...readStringArray(item?.receiverThreadIds), @@ -658,6 +753,63 @@ class Monitor { return undefined; } + private handleClosedChild(notification: CodexServerNotification, state: ParentState): void { + if (notification.method !== "item/completed") { + return; + } + const params = isJsonObject(notification.params) ? notification.params : undefined; + const item = isJsonObject(params?.item) ? params.item : undefined; + if ( + readString(item, "type") !== "collabAgentToolCall" || + normalizeIdentifier(readString(item, "tool")) !== "closeagent" || + normalizeIdentifier(readString(item, "status")) !== "completed" + ) { + return; + } + const childThreadIds = new Set([ + ...readStringArray(item?.receiverThreadIds), + ...readObjectStringKeys(item?.agentsStates), + ]); + for (const childThreadId of childThreadIds) { + const childState = this.childStates.get(childThreadId); + if (childState && childState.parentThreadId !== state.parentThreadId) { + continue; + } + if (childState) { + this.retireChild(state, childState, "Codex native subagent was closed."); + } else { + this.updateChildThreadOwnership("release", childThreadId, this.releaseChildThread); + } + } + } + + private retireChild(state: ParentState, childState: ChildState, summary: string): void { + if (!childState.terminal) { + childState.terminal = true; + const eventAt = this.now(); + state.mirror?.markAuthoritativeCompletion(childState.childThreadId); + state.taskRuntime?.finalizeTaskRunByRunId({ + runId: codexNativeSubagentRunId(childState.childThreadId), + status: "cancelled", + endedAt: eventAt, + lastEventAt: eventAt, + error: summary, + progressSummary: summary, + terminalSummary: summary, + }); + } + if (childState.pendingCompletion) { + childState.pendingCompletion = undefined; + state.taskRuntime?.setDetachedTaskDeliveryStatusByRunId({ + runId: codexNativeSubagentRunId(childState.childThreadId), + deliveryStatus: "failed", + error: summary, + }); + } + this.unregisterChild(childState, { retainSubscription: false }); + this.updateChildThreadOwnership("release", childState.childThreadId, this.releaseChildThread); + } + private async handleCompletionNotification(notification: CodexServerNotification): Promise { const params = isJsonObject(notification.params) ? notification.params : undefined; const parentThreadId = params ? readString(params, "threadId")?.trim() : undefined; @@ -918,6 +1070,12 @@ class Monitor { replyInstruction: "Use the Codex native subagent result to continue or wrap up the parent task. If this is a Discord/channel session, send the visible response with the message tool instead of only writing a transcript final answer. Reply in your normal assistant voice and do not expose internal notification markup.", }); + if ( + this.childStates.get(childState.childThreadId) !== childState || + this.parentStates.get(state.parentThreadId) !== state + ) { + return; + } if (isDurableAgentHarnessCompletionDelivery(delivery)) { childState.pendingCompletion = undefined; childState.completionDeliveryAttempt = 0; @@ -936,6 +1094,12 @@ class Monitor { }); this.scheduleCompletionDeliveryRetry(childState, error); } catch (error) { + if ( + this.childStates.get(childState.childThreadId) !== childState || + this.parentStates.get(state.parentThreadId) !== state + ) { + return; + } const message = formatErrorMessage(error); state.taskRuntime?.setDetachedTaskDeliveryStatusByRunId({ runId: codexNativeSubagentRunId(completion.childThreadId), @@ -1008,6 +1172,7 @@ class Monitor { return undefined; } if (!childState) { + this.updateChildThreadOwnership("claim", childThreadId, this.claimChildThread); this.releaseClientRetention ??= this.retainClient?.(); if (!this.parentThreadRetentions.has(parentThreadId)) { const releaseParentThread = this.retainParentThread?.(parentThreadId); @@ -1062,7 +1227,15 @@ class Monitor { childState.agentPathKeys.add(key); } - private unregisterChild(childState: ChildState): void { + private unregisterChild( + childState: ChildState, + options: { retainSubscription?: boolean } = {}, + ): void { + if (childState.terminal && options.retainSubscription !== false && !this.disposed) { + // Completed Codex children intentionally remain reusable. Transfer their + // auto-subscription into the shared bounded warm-thread owner, not oblivion. + this.updateChildThreadOwnership("retain", childState.childThreadId, this.retainChildThread); + } this.clearRecoveryTimers(childState); if (childState.completionDeliveryTimer) { clearTimeout(childState.completionDeliveryTimer); @@ -1100,6 +1273,23 @@ class Monitor { } } + private updateChildThreadOwnership( + operation: "claim" | "retain" | "release", + childThreadId: string, + update: ((threadId: string) => Promise) | undefined, + ): void { + if (!update) { + return; + } + void update(childThreadId).catch((error: unknown) => { + embeddedAgentLog.warn("Failed to update Codex native subagent thread ownership", { + operation, + childThreadId, + error: formatErrorMessage(error), + }); + }); + } + private releaseClientRetentionIfIdle(): void { if ( [...this.childStates.values()].some( @@ -1287,6 +1477,7 @@ class Monitor { } const childThreadId = task.runId!.slice(CODEX_NATIVE_SUBAGENT_RUN_ID_PREFIX.length).trim(); candidates.set(childThreadId, { + parentState: state, requesterSessionKey: state.requesterSessionKey, childThreadId, recoveryAttempt: 0, @@ -1329,6 +1520,7 @@ class Monitor { const key = `${candidate.requesterSessionKey}\0${candidate.childThreadId}`; if ( this.disposed || + this.retiredParentStates.has(candidate.parentState) || this.recoveryPollDelaysMs.length === 0 || this.taskReconciliationTimers.has(key) ) { @@ -1347,6 +1539,9 @@ class Monitor { } private async reconcileTaskCandidateOnce(candidate: TaskRecoveryCandidate): Promise { + if (this.retiredParentStates.has(candidate.parentState)) { + return; + } const runId = codexNativeSubagentRunId(candidate.childThreadId); const task = candidate.taskRuntime.listTaskRecords().find((record) => record.runId === runId); if ( @@ -1367,6 +1562,9 @@ class Monitor { this.scheduleTaskCandidateReconciliation(candidate); return; } + if (this.retiredParentStates.has(candidate.parentState)) { + return; + } if ( !statusRead.isCurrent() || this.childStates.get(candidate.childThreadId) !== childBeforeRead @@ -1457,7 +1655,13 @@ class Monitor { } } -export const codexNativeSubagentMonitorRuntime = { Monitor, register: registerMonitor }; +export const codexNativeSubagentMonitorRuntime = { + Monitor, + register: registerMonitor, + retireParent: (client: CodexAppServerClient, parentThreadId: string): void => { + monitors.get(client)?.retireParent(parentThreadId); + }, +}; function readThreadTurnRecovery( thread: JsonObject, diff --git a/extensions/codex/src/app-server/native-subagent-task-mirror.test.ts b/extensions/codex/src/app-server/native-subagent-task-mirror.test.ts index ec76e131ca2e..9dd0d0772bf9 100644 --- a/extensions/codex/src/app-server/native-subagent-task-mirror.test.ts +++ b/extensions/codex/src/app-server/native-subagent-task-mirror.test.ts @@ -406,15 +406,12 @@ describe("CodexNativeSubagentTaskMirror", () => { lastEventAt: 41_000, progressSummary: "Codex native subagent received more input.", }); - expect(runtime.finalizeTaskRunByRunId).toHaveBeenCalledWith({ + expect(runtime.recordTaskRunProgressByRunId).toHaveBeenCalledWith({ runId: "codex-thread:child-v2", - status: "cancelled", - endedAt: 41_000, lastEventAt: 41_000, - error: "Codex native subagent was interrupted.", progressSummary: "Codex native subagent was interrupted.", - terminalSummary: "Codex native subagent was interrupted.", }); + expect(runtime.finalizeTaskRunByRunId).not.toHaveBeenCalled(); }); it("uses the notification thread id when collab agent items omit sender thread id", () => { diff --git a/extensions/codex/src/app-server/native-subagent-task-mirror.ts b/extensions/codex/src/app-server/native-subagent-task-mirror.ts index 2e7fb7356191..497f231a37ac 100644 --- a/extensions/codex/src/app-server/native-subagent-task-mirror.ts +++ b/extensions/codex/src/app-server/native-subagent-task-mirror.ts @@ -358,7 +358,9 @@ export class CodexNativeSubagentTaskMirror { return; } const eventAt = this.now(); - if (normalizedStatus === "pendingInit" || normalizedStatus === "running") { + if (isNonTerminalAgentStateStatus(normalizedStatus)) { + // Codex interrupted agents remain open and can resume; finalizing here + // makes cancellation sticky and discards their later successful result. this.runtime.recordTaskRunProgressByRunId({ runId, lastEventAt: eventAt, @@ -366,7 +368,9 @@ export class CodexNativeSubagentTaskMirror { trimOptional(message) ?? (normalizedStatus === "pendingInit" ? "Codex native subagent is initializing." - : "Codex native subagent is running."), + : normalizedStatus === "interrupted" + ? "Codex native subagent was interrupted." + : "Codex native subagent is running."), }); return; } @@ -409,10 +413,7 @@ export class CodexNativeSubagentTaskMirror { this.terminalRunIds.add(runId); this.runtime.finalizeTaskRunByRunId({ runId, - status: - normalizedStatus === "interrupted" || normalizedStatus === "shutdown" - ? "cancelled" - : "failed", + status: normalizedStatus === "shutdown" ? "cancelled" : "failed", endedAt: eventAt, lastEventAt: eventAt, error: trimOptional(message) ?? `Codex native subagent status: ${normalizedStatus}`, @@ -544,7 +545,7 @@ function isBlockedOrFailedCollabToolCallStatus(value: string | undefined): boole } function isNonTerminalAgentStateStatus(value: string | undefined): boolean { - return value === "pendingInit" || value === "running"; + return value === "pendingInit" || value === "running" || value === "interrupted"; } function isTerminalAgentStateStatus(value: string | undefined): boolean { diff --git a/extensions/codex/src/app-server/plugin-thread-config.ts b/extensions/codex/src/app-server/plugin-thread-config.ts index fccfc74929d9..85c1b9db3230 100644 --- a/extensions/codex/src/app-server/plugin-thread-config.ts +++ b/extensions/codex/src/app-server/plugin-thread-config.ts @@ -509,7 +509,7 @@ function emptyPluginThreadConfig(params: { }; } -function buildDisabledAppsConfigPatch(): JsonObject { +export function buildDisabledAppsConfigPatch(): JsonObject { return { apps: { _default: { diff --git a/extensions/codex/src/app-server/request.ts b/extensions/codex/src/app-server/request.ts index 5dca487c4d30..617f947c9e36 100644 --- a/extensions/codex/src/app-server/request.ts +++ b/extensions/codex/src/app-server/request.ts @@ -217,7 +217,7 @@ export async function withCodexAppServerJsonClient( // to the conservative graceful/force-kill window used elsewhere. isolatedShutdown?: { exitTimeoutMs?: number; forceKillDelayMs?: number }; }, - run: (request: CodexAppServerScopedRequest) => Promise, + run: (request: CodexAppServerScopedRequest, client: CodexAppServerClient) => Promise, ): Promise { const timeoutMs = params.timeoutMs ?? 60_000; const timeoutMessage = params.timeoutMessage ?? "codex app-server request timed out"; @@ -273,7 +273,7 @@ export async function withCodexAppServerJsonClient( signal: timeoutController.signal, }); }; - return await run(scopedRequest); + return await run(scopedRequest, client); } catch (error) { if (!isCodexAppServerStartSelectionChangedError(error) || attempt > 0) { throw error; diff --git a/extensions/codex/src/app-server/run-attempt-cleanup.ts b/extensions/codex/src/app-server/run-attempt-cleanup.ts index 94ce6a46b0fb..5f623817c1a5 100644 --- a/extensions/codex/src/app-server/run-attempt-cleanup.ts +++ b/extensions/codex/src/app-server/run-attempt-cleanup.ts @@ -89,13 +89,11 @@ export async function cleanupCodexAttempt( terminalState.turnSucceeded && !isIncognitoSessionKey(params.sessionKey) && params.cleanupBundleMcpOnRunEnd !== true && - !connection.activeContextEngine && resourceState.thread.liveThreadConfigFingerprint !== undefined && resourceState.thread.clientId === resolveCodexAppServerClientInstanceId(resourceState.client) && resourceState.thread.preserveNativeModel !== true && resourceState.thread.connectionScope !== "supervision" && - !resourceState.thread.ringZeroConfigFingerprint && - !resourceState.thread.contextEngine + !resourceState.thread.ringZeroConfigFingerprint ? (await bindingStore.read(bindingIdentity))?.threadId === resourceState.thread.threadId && (await bindingStore.withLease(bindingIdentity, async () => { // Reset/end uses this same generation lease. Never publish an old @@ -108,18 +106,19 @@ export async function cleanupCodexAttempt( return await retainCodexAppServerLiveThread( resourceState.client, resourceState.thread.threadId, - async (threadId) => { - const released = await unsubscribeCodexThreadBestEffort(resourceState.client, { - threadId, - timeoutMs: CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, - }); - if (!released) { - await closeCodexStartupClientBestEffort(resourceState.client); - throw new CodexAppServerUnsafeSubscriptionError( - `Codex retained thread subscription could not be released: ${threadId}`, - ); - } - }, + resourceState.thread.liveThreadOwnership?.release ?? + (async (threadId) => { + const released = await unsubscribeCodexThreadBestEffort(resourceState.client, { + threadId, + timeoutMs: CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, + }); + if (!released) { + await closeCodexStartupClientBestEffort(resourceState.client); + throw new CodexAppServerUnsafeSubscriptionError( + `Codex retained thread subscription could not be released: ${threadId}`, + ); + } + }), resourceState.thread.liveThreadConfigFingerprint, connection.mutable.pluginAppServer.serviceTier, ); diff --git a/extensions/codex/src/app-server/run-attempt.context-engine.test.ts b/extensions/codex/src/app-server/run-attempt.context-engine.test.ts index adcd616102e6..fbe6b62402a2 100644 --- a/extensions/codex/src/app-server/run-attempt.context-engine.test.ts +++ b/extensions/codex/src/app-server/run-attempt.context-engine.test.ts @@ -577,20 +577,19 @@ describe("runCodexAppServerAttempt context-engine lifecycle", () => { fingerprint: undefined, }); - const secondHarness = createStartedThreadHarness(async (method) => { - if (method === "thread/resume") { - return threadStartResult("thread-1"); - } - return undefined; - }); const secondRun = runCodexAppServerAttempt(firstParams); - await secondHarness.waitForMethod("turn/start"); + await vi.waitFor(() => { + expect( + firstHarness.requests.filter((request) => request.method === "turn/start"), + ).toHaveLength(2); + }); - expect(secondHarness.requests.map((request) => request.method)).toEqual([ - "thread/resume", + expect(firstHarness.requests.map((request) => request.method)).toEqual([ + "thread/start", + "turn/start", "turn/start", ]); - const secondInputText = getRequestInputText(secondHarness); + const secondInputText = getRequestInputTextAt(firstHarness, 1); expect(secondInputText).not.toContain("OpenClaw assembled context for this turn:"); expect(secondInputText).not.toContain("bootstrap-only context"); expect(secondInputText).toBe("hello"); @@ -626,7 +625,7 @@ describe("runCodexAppServerAttempt context-engine lifecycle", () => { ], ]); - await secondHarness.completeTurn(); + await firstHarness.completeTurn(); await secondRun; }); diff --git a/extensions/codex/src/app-server/run-attempt.test.ts b/extensions/codex/src/app-server/run-attempt.test.ts index 070d549567a4..f6414c72a330 100644 --- a/extensions/codex/src/app-server/run-attempt.test.ts +++ b/extensions/codex/src/app-server/run-attempt.test.ts @@ -38,7 +38,11 @@ import { readAttemptTerminal } from "./attempt-terminal.test-helper.js"; import { withCodexStartupTimeout } from "./attempt-timeouts.js"; import { prepareCodexAppServerAuthBinding } from "./auth-binding.js"; import { resolveCodexAppServerFallbackApiKeyCacheKey } from "./auth-bridge.js"; -import { CodexAppServerRpcError } from "./client.js"; +import { + consumeCodexAppServerLiveThread, + retainCodexAppServerLiveThread, +} from "./client-runtime.js"; +import { CodexAppServerRpcError, type CodexAppServerClient } from "./client.js"; import { readCodexPluginConfig, resolveCodexAppServerRuntimeOptions, @@ -834,6 +838,7 @@ async function runSharedClientRestartTest(closeCount: number) { const { sessionFile, workspaceDir } = createRunPaths(); await writeExistingBinding(sessionFile, workspaceDir, { dynamicToolsFingerprint: "[]" }); const requests: string[][] = []; + const clients: CodexAppServerClient[] = []; let starts = 0; const state: { notify: (notification: CodexServerNotification) => Promise; @@ -842,7 +847,7 @@ async function runSharedClientRestartTest(closeCount: number) { const startIndex = starts++; const methods: string[] = []; requests.push(methods); - return { + const client = { ...mockClientRuntimeMethods(), request: vi.fn(async (method: string) => { methods.push(method); @@ -862,7 +867,9 @@ async function runSharedClientRestartTest(closeCount: number) { return () => undefined; }, addRequestHandler: () => () => undefined, - } as never; + } as unknown as CodexAppServerClient; + clients.push(client); + return client; }); const run = runCodexAppServerAttempt(createParams(sessionFile, workspaceDir)); await vi.waitFor(() => expect(requests[closeCount]).toContain("turn/start"), fastWait); @@ -874,7 +881,23 @@ async function runSharedClientRestartTest(closeCount: number) { turn: { id: "turn-1", status: "completed" }, }, }); - return { result: await run, requests }; + return { result: await run, requests, client: clients[closeCount]! }; +} + +async function expectRetainedSuccessfulThread(client: CodexAppServerClient, threadId: string) { + const ownership = await consumeCodexAppServerLiveThread(client, threadId); + expect(ownership).toEqual(expect.objectContaining({ release: expect.any(Function) })); + // Restore the exact branded owner so this assertion itself cannot orphan + // the persistent subscription or alter later cleanup in the same test. + await expect( + retainCodexAppServerLiveThread( + client, + threadId, + ownership?.release, + ownership?.configFingerprint, + ownership?.serviceTier, + ), + ).resolves.toBe(true); } async function createSandboxReleaseFixture( @@ -3999,8 +4022,8 @@ describe("runCodexAppServerAttempt", () => { "thread/resume", "turn/start", "turn/start", - "thread/unsubscribe", ]); + await expectRetainedSuccessfulThread(harness.client, "thread-existing"); }); it("waits for the exact active native turn before starting a resumed thread turn", async () => { @@ -4060,8 +4083,8 @@ describe("runCodexAppServerAttempt", () => { expect(harness.requests.map((request) => request.method)).toEqual([ "thread/resume", "turn/start", - "thread/unsubscribe", ]); + await expectRetainedSuccessfulThread(harness.client, "thread-existing"); }); it("does not retry turn/start for non-compact active turns", async () => { const { sessionFile, workspaceDir } = createRunPaths(); @@ -5002,22 +5025,21 @@ describe("runCodexAppServerAttempt", () => { expect(savedBinding?.threadId).toBe("thread-1"); }); it("restarts the app-server once when a shared client closes during startup", async () => { - const { result, requests } = await runSharedClientRestartTest(1); + const { result, requests, client } = await runSharedClientRestartTest(1); expect(readAttemptTerminal(result).aborted).toBe(false); - expect(requests).toEqual([ - ["thread/resume"], - ["thread/resume", "turn/start", "thread/unsubscribe"], - ]); + expect(requests).toEqual([["thread/resume"], ["thread/resume", "turn/start"]]); + await expectRetainedSuccessfulThread(client, "thread-existing"); }); it("tolerates a second app-server close while retrying startup", async () => { - const { result, requests } = await runSharedClientRestartTest(2); + const { result, requests, client } = await runSharedClientRestartTest(2); expect(readAttemptTerminal(result).aborted).toBe(false); expect(requests).toEqual([ ["thread/resume"], ["thread/resume"], - ["thread/resume", "turn/start", "thread/unsubscribe"], + ["thread/resume", "turn/start"], ]); + await expectRetainedSuccessfulThread(client, "thread-existing"); }); it("does not retire the shared Codex client when a spawned helper run fails with a logical thread/start error", async () => { const { retireSpy, state } = installFailingThreadStartClient(() => { diff --git a/extensions/codex/src/app-server/session-retirement.ts b/extensions/codex/src/app-server/session-retirement.ts index af6c3b84fbb6..dbe4abcc7bfc 100644 --- a/extensions/codex/src/app-server/session-retirement.ts +++ b/extensions/codex/src/app-server/session-retirement.ts @@ -6,6 +6,7 @@ import { unsubscribeCodexThreadBestEffort, } from "./attempt-client-cleanup.js"; import { releaseCodexAppServerLiveThread } from "./client-runtime.js"; +import { codexNativeSubagentMonitorRuntime } from "./native-subagent-monitor.js"; import type { CodexAppServerBindingIdentity, CodexAppServerBindingStore, @@ -46,6 +47,9 @@ export async function retireCodexAppServerSessionGeneration(params: { return result; } try { + // Reset retires native-child ownership before unsubscribing its parent; + // late child completions must never reach a replacement session generation. + codexNativeSubagentMonitorRuntime.retireParent(clientLease.client, binding.threadId); const released = await releaseCodexAppServerLiveThread(clientLease.client, binding.threadId); if (!released && isIncognitoSessionKey(params.identity.sessionKey)) { // Ephemeral threads have no rollout to resume, so they intentionally diff --git a/extensions/codex/src/app-server/thread-lifecycle-io.ts b/extensions/codex/src/app-server/thread-lifecycle-io.ts index ec6dfa2cecde..2d49e0d622b3 100644 --- a/extensions/codex/src/app-server/thread-lifecycle-io.ts +++ b/extensions/codex/src/app-server/thread-lifecycle-io.ts @@ -246,6 +246,9 @@ export async function resumeExistingCodexThread( ? params.mcpServersFingerprint : resumeBinding.mcpServersFingerprint; const resumePatch = { + // Resume moves native subscription ownership to this physical client. + // Keeping its previous client id disables warm reuse after every restart. + clientId: resolveCodexAppServerClientInstanceId(params.client), cwd: params.cwd, rolloutPath: resolveCodexThreadRolloutPath(response.thread) ?? resumeBinding.rolloutPath, authProfileId: boundAuthProfileId, diff --git a/extensions/codex/src/app-server/thread-lifecycle-run.ts b/extensions/codex/src/app-server/thread-lifecycle-run.ts index 0e966b05ad5d..b5ada919a567 100644 --- a/extensions/codex/src/app-server/thread-lifecycle-run.ts +++ b/extensions/codex/src/app-server/thread-lifecycle-run.ts @@ -449,19 +449,6 @@ export async function startOrResumeThread( await clearCurrentBinding("rotating a stale thread binding"); binding = undefined; } - if ( - binding?.threadId && - binding.environmentSelectionFingerprint !== environmentSelectionFingerprint - ) { - embeddedAgentLog.debug( - "codex app-server environment selection changed; starting a new thread", - { - threadId: binding.threadId, - }, - ); - await clearCurrentBinding("rotating a stale thread binding"); - binding = undefined; - } if ( binding?.threadId && (binding.networkProxyConfigFingerprint !== networkProxyConfigFingerprint || @@ -587,8 +574,8 @@ export async function startOrResumeThread( binding, bindingIdentity, clientId, - contextEngineBinding, dynamicToolsFingerprint, + environmentSelectionFingerprint, hostSystemAgentActive, lifecycleTiming, nativeSkillIsolation, diff --git a/extensions/codex/src/app-server/thread-lifecycle-types.ts b/extensions/codex/src/app-server/thread-lifecycle-types.ts index b3d1eea59e52..1edd06a464d8 100644 --- a/extensions/codex/src/app-server/thread-lifecycle-types.ts +++ b/extensions/codex/src/app-server/thread-lifecycle-types.ts @@ -1,4 +1,5 @@ import type { EmbeddedRunAttemptParams } from "openclaw/plugin-sdk/agent-harness-runtime"; +import type { CodexAppServerLiveThreadOwnership } from "./client-runtime.js"; import type { CodexAppServerClient } from "./client.js"; import type { CodexAppServerRuntimeOptions } from "./config.js"; import type { CodexPluginThreadConfig } from "./plugin-thread-config.js"; @@ -17,6 +18,8 @@ type CodexAppServerThreadLifecycle = { export type CodexAppServerThreadLifecycleBinding = CodexAppServerThreadBinding & { lifecycle: CodexAppServerThreadLifecycle; liveThreadConfigFingerprint?: string; + /** Process-local claim proof; never write this callback into durable binding state. */ + liveThreadOwnership?: CodexAppServerLiveThreadOwnership; clearInheritedServiceTier?: true; }; diff --git a/extensions/codex/src/app-server/thread-lifecycle-warm.ts b/extensions/codex/src/app-server/thread-lifecycle-warm.ts index a2ac33d899f9..e4c1450e8632 100644 --- a/extensions/codex/src/app-server/thread-lifecycle-warm.ts +++ b/extensions/codex/src/app-server/thread-lifecycle-warm.ts @@ -14,7 +14,6 @@ import { import type { JsonObject } from "./protocol.js"; import type { CodexAppServerBindingIdentity, - CodexAppServerContextEngineBinding, CodexAppServerThreadBinding, } from "./session-binding.js"; import { fingerprintCodexThreadConfig } from "./thread-fingerprints.js"; @@ -37,8 +36,8 @@ type CodexWarmThreadReuseParams = { binding: CodexAppServerThreadBinding; bindingIdentity: CodexAppServerBindingIdentity; clientId?: string; - contextEngineBinding?: CodexAppServerContextEngineBinding; dynamicToolsFingerprint: string; + environmentSelectionFingerprint?: string; hostSystemAgentActive: boolean; lifecycleTiming: CodexThreadLifecycleTimingTracker; nativeSkillIsolation?: Parameters[1]; @@ -121,8 +120,8 @@ export async function tryReuseCodexLiveThread( binding, bindingIdentity, clientId, - contextEngineBinding, dynamicToolsFingerprint, + environmentSelectionFingerprint, hostSystemAgentActive, lifecycleTiming, nativeSkillIsolation, @@ -140,12 +139,14 @@ export async function tryReuseCodexLiveThread( binding.clientId !== clientId || binding.preserveNativeModel === true || binding.connectionScope === "supervision" || - ringZeroActive || - contextEngineBinding + ringZeroActive ) { return {}; } + // Engine identity, projection epoch, and policy were checked by the owner + // before this call; compatible bootstrap threads must keep their session. + const prebuiltFinalConfigPatch = params.buildFinalConfigPatch?.({ action: "resume", binding, @@ -202,6 +203,15 @@ export async function tryReuseCodexLiveThread( liveThreadConfigFingerprint, ); if (!retainedThread) { + const incompatibleOwnership = await consumeCodexAppServerLiveThread( + params.client, + binding.threadId, + ); + if (incompatibleOwnership) { + // Codex ignores resume overrides while any subscription remains. Manual + // external adoption has no verified fingerprint, so release before resume. + await incompatibleOwnership.release(binding.threadId); + } return { prebuiltFinalConfigPatch }; } @@ -216,7 +226,14 @@ export async function tryReuseCodexLiveThread( params.bindingStore.mutate(bindingIdentity, { kind: "patch", threadId: binding.threadId, - patch: { cwd: params.cwd, model, nativeHookRelayGeneration }, + // Environment selection is sticky turn/start state, like cwd/model; + // recording its new value must not recreate the approval-bearing thread. + patch: { + cwd: params.cwd, + model, + nativeHookRelayGeneration, + environmentSelectionFingerprint, + }, }), ); if (!committed) { @@ -237,7 +254,9 @@ export async function tryReuseCodexLiveThread( cwd: params.cwd, model, nativeHookRelayGeneration, + environmentSelectionFingerprint, liveThreadConfigFingerprint, + liveThreadOwnership: retainedThread, ...(retainedThread.serviceTier && resumeParams.serviceTier === undefined ? { clearInheritedServiceTier: true } : {}), diff --git a/extensions/codex/src/app-server/thread-lifecycle.binding.test.ts b/extensions/codex/src/app-server/thread-lifecycle.binding.test.ts index b621b1f0fab3..90eee67c59a3 100644 --- a/extensions/codex/src/app-server/thread-lifecycle.binding.test.ts +++ b/extensions/codex/src/app-server/thread-lifecycle.binding.test.ts @@ -20,6 +20,7 @@ import { testCodexAppServerBindingStore, writeCodexAppServerBinding as writeRawCodexAppServerBinding, } from "./session-binding.test-helpers.js"; +import { fingerprintEnvironmentSelection } from "./thread-fingerprints.js"; import { buildThreadResumeParams, startOrResumeThread as startOrResumeThreadImpl, @@ -397,6 +398,174 @@ describe("Codex app-server thread lifecycle bindings", () => { }); }); + it("keeps a warm native session across sticky environment selection changes", async () => { + const sessionFile = path.join(tempDir, "environment-session.jsonl"); + const workspaceDir = path.join(tempDir, "environment-workspace"); + const request = vi.fn(async (method: string) => { + if (method === "thread/start") { + return threadStartResult("thread-environments"); + } + throw new Error(`unexpected method: ${method}`); + }); + const client = { + getInstanceId: () => "client-environments", + request, + addNotificationHandler: () => () => undefined, + addRequestHandler: () => () => undefined, + addCloseHandler: () => () => undefined, + } as never; + ensureCodexAppServerClientRuntime(client, { agentDir: workspaceDir }); + const common = { + client, + params: createParams(sessionFile, workspaceDir), + cwd: workspaceDir, + dynamicTools: [], + appServer: createThreadLifecycleAppServerOptions(), + userMcpServersEnabled: false, + }; + const firstSelection = [{ environmentId: "environment-a", cwd: workspaceDir }]; + const secondSelection = [{ environmentId: "environment-b", cwd: workspaceDir }]; + + const started = await startOrResumeThread({ + ...common, + environmentSelection: firstSelection, + }); + await expect( + retainCodexAppServerLiveThread( + client, + started.threadId, + undefined, + started.liveThreadConfigFingerprint, + ), + ).resolves.toBe(true); + const switched = await startOrResumeThread({ + ...common, + environmentSelection: secondSelection, + }); + await expect(readCodexAppServerBinding(sessionFile)).resolves.toMatchObject({ + threadId: "thread-environments", + environmentSelectionFingerprint: fingerprintEnvironmentSelection(secondSelection), + }); + await expect( + retainCodexAppServerLiveThread( + client, + switched.threadId, + switched.liveThreadOwnership?.release, + switched.liveThreadConfigFingerprint, + ), + ).resolves.toBe(true); + const restored = await startOrResumeThread({ + ...common, + environmentSelection: firstSelection, + }); + + expect(switched.threadId).toBe(started.threadId); + expect(restored.threadId).toBe(started.threadId); + expect(request.mock.calls.map(([method]) => method)).toEqual(["thread/start"]); + await expect(readCodexAppServerBinding(sessionFile)).resolves.toMatchObject({ + environmentSelectionFingerprint: fingerprintEnvironmentSelection(firstSelection), + }); + }); + + it("rebinds a resumed thread to its replacement physical client before warm reuse", async () => { + const sessionFile = path.join(tempDir, "replacement-client-session.jsonl"); + const workspaceDir = path.join(tempDir, "replacement-client-workspace"); + await writeCodexAppServerBinding(sessionFile, { + threadId: "thread-reused", + clientId: "client-before-restart", + cwd: workspaceDir, + dynamicToolsFingerprint: "[]", + }); + const request = vi.fn(async (method: string) => { + if (method === "thread/resume") { + return threadStartResult("thread-reused"); + } + throw new Error(`unexpected method: ${method}`); + }); + const client = { + getInstanceId: () => "client-after-restart", + request, + addNotificationHandler: () => () => undefined, + addRequestHandler: () => () => undefined, + addCloseHandler: () => () => undefined, + } as never; + ensureCodexAppServerClientRuntime(client, { agentDir: workspaceDir }); + const common = { + client, + params: createParams(sessionFile, workspaceDir), + cwd: workspaceDir, + dynamicTools: [], + appServer: createThreadLifecycleAppServerOptions(), + userMcpServersEnabled: false, + }; + + const resumed = await startOrResumeThread(common); + + expect(resumed.clientId).toBe("client-after-restart"); + await expect(readCodexAppServerBinding(sessionFile)).resolves.toMatchObject({ + threadId: "thread-reused", + clientId: "client-after-restart", + }); + await retainCodexAppServerLiveThread( + client, + resumed.threadId, + undefined, + resumed.liveThreadConfigFingerprint, + ); + await expect(startOrResumeThread(common)).resolves.toMatchObject({ + threadId: "thread-reused", + clientId: "client-after-restart", + }); + expect(request.mock.calls.map(([method]) => method)).toEqual(["thread/resume"]); + }); + + it("releases an unverified manual-resume owner before applying canonical harness overrides", async () => { + const sessionFile = path.join(tempDir, "manual-resume-session.jsonl"); + const workspaceDir = path.join(tempDir, "manual-resume-workspace"); + await writeCodexAppServerBinding(sessionFile, { + threadId: "thread-manual-resume", + clientId: "client-manual-resume", + cwd: workspaceDir, + }); + const request = vi.fn(async (method: string) => { + if (method === "thread/unsubscribe") { + return {}; + } + if (method === "thread/resume") { + return threadStartResult("thread-manual-resume"); + } + throw new Error(`unexpected method: ${method}`); + }); + const client = { + getInstanceId: () => "client-manual-resume", + request, + addNotificationHandler: () => () => undefined, + addRequestHandler: () => () => undefined, + addCloseHandler: () => () => undefined, + } as never; + ensureCodexAppServerClientRuntime(client, { agentDir: workspaceDir }); + await retainCodexAppServerLiveThread(client, "thread-manual-resume"); + + const resumed = await startOrResumeThread({ + client, + params: createParams(sessionFile, workspaceDir), + cwd: workspaceDir, + dynamicTools: [], + appServer: createThreadLifecycleAppServerOptions(), + userMcpServersEnabled: false, + }); + + expect(resumed).toMatchObject({ + threadId: "thread-manual-resume", + clientId: "client-manual-resume", + lifecycle: { action: "resumed" }, + }); + expect(request.mock.calls.map(([method]) => method)).toEqual([ + "thread/unsubscribe", + "thread/resume", + ]); + }); + it("reuses an isolated retained thread without dropping native skill isolation", async () => { vi.stubEnv("HOME", tempDir); vi.stubEnv("OPENCLAW_STATE_DIR", path.join(tempDir, "isolated-state")); diff --git a/extensions/codex/src/app-server/thread-ownership.ts b/extensions/codex/src/app-server/thread-ownership.ts new file mode 100644 index 000000000000..aa4c737e9170 --- /dev/null +++ b/extensions/codex/src/app-server/thread-ownership.ts @@ -0,0 +1,214 @@ +import { KeyedAsyncQueue } from "openclaw/plugin-sdk/keyed-async-queue"; +import { + CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, + closeCodexStartupClientBestEffort, + CodexAppServerUnsafeSubscriptionError, + unsubscribeCodexThreadBestEffort, +} from "./attempt-client-cleanup.js"; +import { + isCodexAppServerLiveThreadClaimed, + releaseCodexAppServerLiveThread, + retainCodexAppServerLiveThread, + type CodexAppServerLiveThreadOwnership, +} from "./client-runtime.js"; +import type { CodexAppServerClient } from "./client.js"; +import type { + CodexAppServerBindingIdentity, + CodexAppServerBindingStore, + CodexAppServerThreadBinding, +} from "./session-binding.js"; +import { retainSharedCodexAppServerClientByInstanceId } from "./shared-client.js"; + +const nativeThreadOwners = new KeyedAsyncQueue(); + +/** Codex subscriptions belong to a physical connection, not the native thread ID alone. */ +export function isSameCodexAppServerThreadOwner( + current: Pick | undefined, + expected: Pick | undefined, +): boolean { + return ( + current !== undefined && + expected !== undefined && + current.threadId === expected.threadId && + current.clientId === expected.clientId + ); +} + +/** Fences native subscription and commit together; Codex subscriptions are not reference-counted. */ +export async function withExclusiveCodexAppServerThread(params: { + bindingStore: CodexAppServerBindingStore; + identity: CodexAppServerBindingIdentity; + threadId: string; + run: () => Promise; +}): Promise { + return await nativeThreadOwners.enqueue(`thread:${params.threadId}`, async () => { + if (await params.bindingStore.hasOtherThreadOwner(params.threadId, params.identity)) { + throw new Error( + `Codex thread ${params.threadId} is owned by another OpenClaw session or conversation.`, + ); + } + return await params.run(); + }); +} + +/** Serializes bound turns and retirement so detach cannot unsubscribe an active turn. */ +export async function withCodexConversationThreadActivity( + bindingId: string, + run: () => Promise, +): Promise { + return await nativeThreadOwners.enqueue(`conversation:${bindingId}`, run); +} + +/** Publishes one owned persistent subscription into the shared bounded idle registry. */ +export async function retainCodexAppServerBindingSubscription( + client: CodexAppServerClient, + threadId: string, + ownership?: CodexAppServerLiveThreadOwnership, +): Promise { + return await retainCodexAppServerLiveThread( + client, + threadId, + ownership?.release ?? + (async (releasedThreadId) => { + const unsubscribed = await unsubscribeCodexThreadBestEffort(client, { + threadId: releasedThreadId, + timeoutMs: CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, + }); + if (!unsubscribed) { + await closeCodexStartupClientBestEffort(client); + throw new CodexAppServerUnsafeSubscriptionError( + `Codex thread subscription could not be released: ${releasedThreadId}`, + ); + } + }), + ownership?.configFingerprint, + ownership?.serviceTier, + ); +} + +/** Rolls back the exact subscription Codex created before its binding was committed. */ +export async function rollbackCodexAppServerBindingSubscription( + client: CodexAppServerClient, + threadId: string, + retained: boolean, +): Promise { + if (retained && (await releaseCodexAppServerLiveThread(client, threadId))) { + return; + } + // Failed retention can mean another generation already owns this exact + // subscription; resume did not create a second connection-scoped listener. + if (isCodexAppServerLiveThreadClaimed(client, threadId)) { + return; + } + // Start/resume subscribes before its response; failed retention has no + // registry owner, so the responding physical client must unsubscribe it. + if ( + !(await unsubscribeCodexThreadBestEffort(client, { + threadId, + timeoutMs: CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, + })) + ) { + await closeCodexStartupClientBestEffort(client); + } +} + +/** Releases only the physical client and native thread recorded by the displaced binding owner. */ +export async function releaseCodexAppServerBindingSubscription( + binding: Pick, + options: { allowUntracked?: boolean } = {}, +): Promise { + const clientLease = retainSharedCodexAppServerClientByInstanceId(binding.clientId); + if (!clientLease) { + return; + } + try { + if (await releaseCodexAppServerLiveThread(clientLease.client, binding.threadId)) { + return; + } + // Evicted idle owners also disappear from the registry. Only an explicit + // claimed generation proves an active turn still owns its subscription. + if (isCodexAppServerLiveThreadClaimed(clientLease.client, binding.threadId)) { + throw new Error( + `Codex thread ${binding.threadId} has an active run; stop it before changing its owner.`, + ); + } + if (!options.allowUntracked) { + return; + } + const unsubscribed = await unsubscribeCodexThreadBestEffort(clientLease.client, { + threadId: binding.threadId, + timeoutMs: CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, + }); + if (!unsubscribed) { + await closeCodexStartupClientBestEffort(clientLease.client); + throw new CodexAppServerUnsafeSubscriptionError( + `Codex retired thread subscription could not be released: ${binding.threadId}`, + ); + } + } finally { + clientLease.release(); + } +} + +/** Clears and releases one exact conversation generation without touching its replacement. */ +export async function retireCodexConversationThreadBinding(params: { + bindingStore: CodexAppServerBindingStore; + identity: Extract; + expectedThreadId?: string; + expectedStartId?: string; + allowUntracked?: boolean; + afterClear?: () => Promise; +}): Promise { + const expected = await params.bindingStore.read(params.identity); + if (!expected || (params.expectedThreadId && expected.threadId !== params.expectedThreadId)) { + return false; + } + return await params.bindingStore.withLease(params.identity, async () => { + const current = await params.bindingStore.read(params.identity); + if ( + current?.threadId !== expected.threadId || + (params.expectedStartId && current.conversationStartId !== params.expectedStartId) + ) { + return false; + } + // Keep the old row authoritative through unsubscribe; Codex has one + // subscription per physical client, so clearing first races a new owner. + await releaseCodexAppServerBindingSubscription(current, { + allowUntracked: params.allowUntracked, + }); + const cleared = await params.bindingStore.mutate(params.identity, { + kind: "clear", + threadId: current.threadId, + }); + if (!cleared || !params.afterClear) { + return cleared; + } + try { + await params.afterClear(); + return true; + } catch (error) { + try { + // Public binding storage commits separately. Restore its exact native + // owner on failure without ever overwriting a replacement generation. + const restored = await params.bindingStore.mutate(params.identity, { + kind: "set", + binding: current, + if: { kind: "absent" }, + }); + if (!restored) { + throw new Error("the previous Codex binding generation could not be restored", { + cause: error, + }); + } + } catch (restorationError) { + const recoveryError = new AggregateError( + [error, restorationError], + `Codex conversation detachment failed and native thread ${current.threadId} could not be restored; run /codex resume ${current.threadId} to recover it`, + { cause: restorationError }, + ); + throw recoveryError; + } + throw error; + } + }); +} diff --git a/extensions/codex/src/app-server/transcript-history-projection.ts b/extensions/codex/src/app-server/transcript-history-projection.ts index 0eb734e2916b..b140275595f8 100644 --- a/extensions/codex/src/app-server/transcript-history-projection.ts +++ b/extensions/codex/src/app-server/transcript-history-projection.ts @@ -1,6 +1,7 @@ import { Buffer } from "node:buffer"; import type { AgentMessage } from "openclaw/plugin-sdk/agent-harness-runtime"; import type { AssistantMessage, Usage } from "openclaw/plugin-sdk/llm"; +import type { SessionTranscriptMessageEntry } from "openclaw/plugin-sdk/session-transcript-runtime"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import type { CodexThread, JsonValue } from "./protocol.js"; import { attachCodexMirrorIdentity } from "./upstream-prompt-provenance.js"; @@ -267,3 +268,54 @@ export function projectBoundedCodexThreadHistory(params: { transcriptMessages: selected.map(({ message }) => message), }; } + +/** Projects only visible local user/assistant messages through the same bounded history policy. */ +export function projectBoundedCodexVisibleSessionHistory( + entries: readonly SessionTranscriptMessageEntry[], +): JsonValue[] { + const projected: ProjectedCodexHistoryMessage[] = []; + for (const entry of entries) { + if ((entry.role !== "user" && entry.role !== "assistant") || !("content" in entry.message)) { + continue; + } + if ( + entry.role === "assistant" && + "stopReason" in entry.message && + (entry.message.stopReason === "aborted" || entry.message.stopReason === "error") + ) { + continue; + } + const content = entry.message.content; + const text = normalizeImportedHistoryText( + typeof content === "string" + ? content + : Array.isArray(content) + ? content + .flatMap((part) => + part && typeof part === "object" && "text" in part && typeof part.text === "string" + ? [part.text] + : [], + ) + .join("\n") + : undefined, + ); + if (!text) { + continue; + } + projected.push({ + message: entry.message, + responseItem: { + type: "message", + role: entry.role, + content: [ + { + type: entry.role === "assistant" ? "output_text" : "input_text", + text, + }, + ], + }, + textBytes: Buffer.byteLength(text, "utf8"), + }); + } + return selectBoundedCodexHistoryTail(projected).map(({ responseItem }) => responseItem); +} diff --git a/extensions/codex/src/command-handler-bindings.ts b/extensions/codex/src/command-handler-bindings.ts index 7e0cacb0551d..4b6ca4c65f4a 100644 --- a/extensions/codex/src/command-handler-bindings.ts +++ b/extensions/codex/src/command-handler-bindings.ts @@ -6,6 +6,8 @@ import { import type { PluginCommandContext, PluginCommandResult } from "openclaw/plugin-sdk/plugin-entry"; import { getSessionEntry, resolveStorePath } from "openclaw/plugin-sdk/session-store-runtime"; import { resolveCodexAppServerAuthProfileIdForAgent } from "./app-server/auth-bridge.js"; +import { consumeCodexAppServerLiveThread } from "./app-server/client-runtime.js"; +import type { CodexAppServerClient } from "./app-server/client.js"; import { isCodexFastServiceTier } from "./app-server/config.js"; import { assertCodexThreadResumeResponse } from "./app-server/protocol-validators.js"; import { @@ -15,6 +17,15 @@ import { reclaimCurrentCodexSessionGeneration, sessionBindingIdentity, } from "./app-server/session-binding.js"; +import { + isSameCodexAppServerThreadOwner, + releaseCodexAppServerBindingSubscription, + retainCodexAppServerBindingSubscription, + retireCodexConversationThreadBinding, + rollbackCodexAppServerBindingSubscription, + withCodexConversationThreadActivity, + withExclusiveCodexAppServerThread, +} from "./app-server/thread-ownership.js"; import { formatCodexDisplayText, formatThreads } from "./command-formatters.js"; import { parseBindArgs, @@ -34,6 +45,7 @@ import { readCodexConversationBindingData, } from "./conversation-binding-data.js"; import { formatPermissionsMode } from "./conversation-control.js"; +import { isIncognitoSessionKey } from "./incognito-session.js"; import { formatCodexCliSessions } from "./node-cli-sessions.js"; export function isCurrentSessionModelSelectionLocked(ctx: PluginCommandContext): boolean { @@ -151,17 +163,53 @@ export async function detachConversation( } const current = await ctx.getCurrentConversationBinding(); const data = readCodexConversationBindingData(current); + const identity = + data?.kind === "codex-app-server-session" + ? conversationBindingIdentity(data.bindingId) + : undefined; + const sourceSessionKey = + data?.kind === "codex-app-server-session" ? data.source?.sessionKey : undefined; + let expectedThreadId: string | undefined; + let expectedStartId: string | undefined; if (data?.kind === "codex-app-server-session") { - const binding = await deps.bindingStore.read(conversationBindingIdentity(data.bindingId)); + const binding = await deps.bindingStore.read(identity!); assertCodexBindingMayBeReplaced(binding, "detaching its conversation binding"); + if (deps.readCodexConversationActiveTurn(identity!)) { + return "This Codex conversation has an active run; use /codex stop before detaching it."; + } + expectedThreadId = binding?.threadId; + expectedStartId = binding?.conversationStartId; } - const detached = await ctx.detachConversationBinding(); - if (data?.kind === "codex-app-server-session") { - await deps.bindingStore.mutate(conversationBindingIdentity(data.bindingId), { kind: "clear" }); + const detachPublicConversation = async () => { + const detached = await ctx.detachConversationBinding(); + return detached.removed + ? "Detached this conversation from Codex." + : "No Codex conversation binding was attached."; + }; + if (identity && expectedThreadId) { + return await withCodexConversationThreadActivity(identity.bindingId, async () => { + let detachedPublicConversation: string | undefined; + const retired = await retireCodexConversationThreadBinding({ + bindingStore: deps.bindingStore, + identity, + expectedThreadId, + ...(expectedStartId ? { expectedStartId } : {}), + // The source session owns ephemeral tracking; destination channel + // session keys do not describe how this subscription was created. + ...(isIncognitoSessionKey(sourceSessionKey) ? { allowUntracked: true } : {}), + afterClear: async () => { + // The owner restores the exact native row if public detach fails; + // an attached conversation then resumes its original thread. + detachedPublicConversation = await detachPublicConversation(); + }, + }); + if (!retired) { + return "This Codex conversation binding changed while detaching; try again."; + } + return detachedPublicConversation!; + }); } - return detached.removed - ? "Detached this conversation from Codex." - : "No Codex conversation binding was attached."; + return await detachPublicConversation(); } export async function describeConversationBinding( @@ -266,74 +314,125 @@ export async function resumeThread( agentId: scope.agentId, config: ctx.config, }); - return await deps.bindingStore.withLease(identity, async () => { - const reclaimed = await reclaimCurrentCodexSessionGeneration({ - bindingStore: deps.bindingStore, - identity, - config: ctx.config, - }); - if (!reclaimed) { - throw createCodexSessionGenerationSupersededError(identity.sessionId); - } - const currentBinding = await deps.bindingStore.read(identity); - assertCodexBindingMayBeReplaced(currentBinding, "attaching a different resumed thread"); - const authProfileId = resolveCodexAppServerAuthProfileIdForAgent({ - authProfileId: currentBinding?.authProfileId, - agentDir: scope.agentDir, - config: ctx.config, - }); - const response = assertCodexThreadResumeResponse( - await deps.codexControlRequest( - pluginConfig, - CODEX_CONTROL_METHODS.resumeThread, - { - threadId: normalizedThreadId, - excludeTurns: true, - }, - { + return await withExclusiveCodexAppServerThread({ + bindingStore: deps.bindingStore, + identity, + threadId: normalizedThreadId, + run: async () => + await deps.bindingStore.withLease(identity, async () => { + const reclaimed = await reclaimCurrentCodexSessionGeneration({ + bindingStore: deps.bindingStore, + identity, config: ctx.config, + }); + if (!reclaimed) { + throw createCodexSessionGenerationSupersededError(identity.sessionId); + } + const currentBinding = await deps.bindingStore.read(identity); + assertCodexBindingMayBeReplaced(currentBinding, "attaching a different resumed thread"); + const authProfileId = resolveCodexAppServerAuthProfileIdForAgent({ + authProfileId: currentBinding?.authProfileId, agentDir: scope.agentDir, - authProfileId, - sessionKey: ctx.sessionKey, - sessionId: ctx.sessionId, - }, - ), - ); - const effectiveThreadId = response.thread.id; - if (effectiveThreadId !== normalizedThreadId) { - throw new Error( - `Codex thread/resume returned ${effectiveThreadId} for ${normalizedThreadId}`, - ); - } - const resumedCwd = response.thread.cwd; - if (typeof resumedCwd !== "string") { - throw new Error(`Codex thread/resume returned no cwd for ${normalizedThreadId}`); - } - const modelProvider = normalizeCodexAppServerBindingModelProvider({ - authProfileId, - modelProvider: response.modelProvider ?? undefined, - agentDir: scope.agentDir, - config: ctx.config, - }); - const bindingBeforeCommit = await deps.bindingStore.read(identity); - assertCodexBindingMayBeReplaced(bindingBeforeCommit, "committing a different resumed thread"); - const committed = await deps.bindingStore.mutate(identity, { - kind: "set", - binding: { - threadId: effectiveThreadId, - cwd: resumedCwd, - authProfileId, - model: response.model, - modelProvider, - historyCoveredThrough: new Date().toISOString(), - }, - }); - if (!committed) { - throw new Error("Codex thread binding changed while attaching the resumed thread."); - } - return `Attached this OpenClaw session to Codex thread ${formatCodexDisplayText( - effectiveThreadId, - )}.`; + config: ctx.config, + }); + let committedResponse = false; + const commitResumedThread = async (value: unknown, client?: CodexAppServerClient) => { + const response = assertCodexThreadResumeResponse(value); + const effectiveThreadId = response.thread.id; + if (effectiveThreadId !== normalizedThreadId) { + throw new Error( + `Codex thread/resume returned ${effectiveThreadId} for ${normalizedThreadId}`, + ); + } + const resumedCwd = response.thread.cwd; + if (typeof resumedCwd !== "string") { + throw new Error(`Codex thread/resume returned no cwd for ${normalizedThreadId}`); + } + const modelProvider = normalizeCodexAppServerBindingModelProvider({ + authProfileId, + modelProvider: response.modelProvider ?? undefined, + agentDir: scope.agentDir, + config: ctx.config, + }); + const bindingBeforeCommit = await deps.bindingStore.read(identity); + assertCodexBindingMayBeReplaced( + bindingBeforeCommit, + "committing a different resumed thread", + ); + const clientId = client?.getInstanceId(); + const sameOwner = client + ? isSameCodexAppServerThreadOwner(bindingBeforeCommit, { + threadId: effectiveThreadId, + clientId, + }) + : bindingBeforeCommit?.threadId === effectiveThreadId; + let retained = false; + try { + if (client) { + const knownOwnership = sameOwner + ? await consumeCodexAppServerLiveThread(client, effectiveThreadId) + : undefined; + retained = await retainCodexAppServerBindingSubscription( + client, + effectiveThreadId, + knownOwnership, + ); + if (!retained) { + throw new Error("Codex resumed thread lost its native subscription owner."); + } + } + if (bindingBeforeCommit && !sameOwner) { + // The old row must remain authoritative until its subscription + // is gone; otherwise another session can claim and lose it. + await releaseCodexAppServerBindingSubscription(bindingBeforeCommit); + } + const committed = await deps.bindingStore.mutate(identity, { + kind: "set", + binding: { + ...(bindingBeforeCommit?.threadId === effectiveThreadId ? bindingBeforeCommit : {}), + threadId: effectiveThreadId, + ...(clientId ? { clientId } : {}), + cwd: resumedCwd, + authProfileId, + model: response.model, + modelProvider, + historyCoveredThrough: new Date().toISOString(), + }, + }); + if (!committed) { + throw new Error("Codex thread binding changed while attaching the resumed thread."); + } + } catch (error) { + if (client && !sameOwner) { + await rollbackCodexAppServerBindingSubscription(client, effectiveThreadId, retained); + } + throw error; + } + committedResponse = true; + }; + const response = await deps.codexControlRequest( + pluginConfig, + CODEX_CONTROL_METHODS.resumeThread, + { + threadId: normalizedThreadId, + excludeTurns: true, + }, + { + config: ctx.config, + agentDir: scope.agentDir, + authProfileId, + sessionKey: ctx.sessionKey, + sessionId: ctx.sessionId, + onResponse: commitResumedThread, + }, + ); + if (!committedResponse) { + await commitResumedThread(response); + } + return `Attached this OpenClaw session to Codex thread ${formatCodexDisplayText( + normalizedThreadId, + )}.`; + }), }); } diff --git a/extensions/codex/src/command-handler-scope.ts b/extensions/codex/src/command-handler-scope.ts index 0629d8e59ec2..9d7310059224 100644 --- a/extensions/codex/src/command-handler-scope.ts +++ b/extensions/codex/src/command-handler-scope.ts @@ -84,7 +84,9 @@ export async function resolveCommandAppServerScope( }; } -export function conversationBindingIdentity(bindingId: string): CodexAppServerBindingIdentity { +export function conversationBindingIdentity( + bindingId: string, +): Extract { return { kind: "conversation", bindingId }; } diff --git a/extensions/codex/src/command-rpc.ts b/extensions/codex/src/command-rpc.ts index 5eeb8687ba75..eaaf8ba73aad 100644 --- a/extensions/codex/src/command-rpc.ts +++ b/extensions/codex/src/command-rpc.ts @@ -5,6 +5,7 @@ import { describeControlFailure, type CodexControlMethod, } from "./app-server/capabilities.js"; +import type { CodexAppServerClient } from "./app-server/client.js"; import { resolveCodexAppServerRuntimeOptions, resolveCodexSupervisionAppServerRuntimeOptions, @@ -17,7 +18,7 @@ import type { CodexAppServerRequestResult, JsonValue, } from "./app-server/protocol.js"; -import { requestCodexAppServerJson } from "./app-server/request.js"; +import { requestCodexAppServerJson, withCodexAppServerJsonClient } from "./app-server/request.js"; export type SafeValue = { ok: true; value: T } | { ok: false; error: string }; @@ -34,6 +35,7 @@ export type CodexControlRequestOptions = { isolated?: boolean; startOptions?: CodexAppServerStartOptions; timeoutMs?: number; + onResponse?: (response: unknown, client: CodexAppServerClient) => Promise; }; export function requestOptions( @@ -76,9 +78,7 @@ export async function codexControlRequest( const runtime = options.startOptions ? resolveCodexSupervisionAppServerRuntimeOptions({ pluginConfig }) : resolveCodexAppServerRuntimeOptions({ pluginConfig }); - return await requestCodexAppServerJson({ - method, - requestParams, + const controlRequestOptions = { timeoutMs: options.timeoutMs ?? runtime.requestTimeoutMs, startOptions: options.startOptions ?? runtime.start, config: options.config, @@ -87,7 +87,17 @@ export async function codexControlRequest( authProfileId: options.authProfileId, agentDir: options.agentDir, isolated: options.isolated, - }); + }; + if (options.onResponse) { + return await withCodexAppServerJsonClient(controlRequestOptions, async (request, client) => { + const response = await request({ method, requestParams }); + // Subscription-producing control requests must publish their exact + // physical-client ownership before this shared lease can be released. + await options.onResponse!(response, client); + return response; + }); + } + return await requestCodexAppServerJson({ method, requestParams, ...controlRequestOptions }); } export function safeCodexControlRequest( diff --git a/extensions/codex/src/commands.test.ts b/extensions/codex/src/commands.test.ts index 1a297aa44e15..56b3e062827a 100644 --- a/extensions/codex/src/commands.test.ts +++ b/extensions/codex/src/commands.test.ts @@ -7,6 +7,7 @@ import { resolveDefaultAgentDir, type AuthProfileStore, } from "openclaw/plugin-sdk/agent-runtime"; +import { getSessionBindingService } from "openclaw/plugin-sdk/conversation-binding-runtime"; import { MODEL_SELECTION_LOCKED_MESSAGE } from "openclaw/plugin-sdk/model-session-runtime"; import type { PluginCommandContext, PluginCommandResult } from "openclaw/plugin-sdk/plugin-entry"; import { upsertSessionEntry } from "openclaw/plugin-sdk/session-store-runtime"; @@ -14,8 +15,16 @@ import { upsertSessionEntry } from "openclaw/plugin-sdk/session-store-runtime"; import { createRequireRecord } from "openclaw/plugin-sdk/test-fixtures"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { CODEX_CONTROL_METHODS } from "./app-server/capabilities.js"; +import { + consumeCodexAppServerLiveThread, + ensureCodexAppServerClientRuntime, + isCodexAppServerLiveThreadClaimed, + retainCodexAppServerLiveThread, +} from "./app-server/client-runtime.js"; +import type { CodexAppServerClient } from "./app-server/client.js"; import type { CodexComputerUseStatus } from "./app-server/computer-use.js"; import type { CodexAppServerStartOptions } from "./app-server/config.js"; +import { codexNativeSubagentMonitorRuntime } from "./app-server/native-subagent-monitor.js"; import type { JsonValue } from "./app-server/protocol.js"; import type { CodexAppServerThreadBinding } from "./app-server/session-binding.js"; import { @@ -24,6 +33,7 @@ import { testCodexAppServerBindingStore, } from "./app-server/session-binding.test-helpers.js"; import { resetSharedCodexAppServerClientForTests } from "./app-server/shared-client.js"; +import { createClientHarness } from "./app-server/test-support.js"; import { CODEX_APP_SERVER_VERSION } from "./app-server/version.js"; import { codexDiagnosticsFeedbackState } from "./command-diagnostics-state.js"; import { handleCodexCommand as dispatchCodexCommand } from "./command-dispatch.js"; @@ -32,6 +42,7 @@ import type { CodexPluginsConfigBlock, CodexPluginsManagementIO, } from "./command-plugins-management.js"; +import { codexConversationBindingRuntime } from "./conversation-binding.js"; type CodexPluginConfigEntry = NonNullable[string]; @@ -503,6 +514,376 @@ describe("codex command", () => { }); }); + it("publishes manual resume ownership on its responding physical client before returning", async () => { + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const response = createThreadResumeResponse({ threadId: "thread-owned-resume" }); + const codexControlRequest = vi.fn( + async ( + _pluginConfig: unknown, + _method: string, + _params: unknown, + options?: { + onResponse?: (value: unknown, client: CodexAppServerClient) => Promise; + }, + ) => { + await options?.onResponse?.(response, harness.client); + return response; + }, + ); + + try { + const result = await runCommand("resume thread-owned-resume", { codexControlRequest }); + + expect(result.text).toContain("Attached this OpenClaw session"); + await expect( + testCodexAppServerBindingStore.read({ + kind: "session", + agentId: "main", + sessionId: "session-1", + }), + ).resolves.toMatchObject({ + threadId: "thread-owned-resume", + clientId: harness.client.getInstanceId(), + }); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-owned-resume"), + ).resolves.toEqual(expect.objectContaining({ release: expect.any(Function) })); + } finally { + harness.client.close(); + } + }); + + it("unsubscribes a manually resumed thread when its idle owner cannot be published", async () => { + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const request = vi.spyOn(harness.client, "request").mockResolvedValue({} as never); + const oldestRelease = vi.fn(async () => { + throw new Error("oldest thread could not be unsubscribed"); + }); + await retainCodexAppServerLiveThread(harness.client, "thread-oldest", oldestRelease); + for (let index = 1; index < 63; index += 1) { + await retainCodexAppServerLiveThread(harness.client, `thread-idle-${index}`); + } + await retainCodexAppServerLiveThread(harness.client, "thread-existing"); + const identity = { + kind: "session" as const, + agentId: "main", + sessionId: "session-1", + }; + await writeTestBinding(identity, { + threadId: "thread-existing", + clientId: harness.client.getInstanceId(), + cwd: "/repo", + }); + const response = createThreadResumeResponse({ threadId: "thread-overflow" }); + const codexControlRequest = vi.fn( + async ( + _pluginConfig: unknown, + _method: string, + _params: unknown, + options?: { + onResponse?: (value: unknown, client: CodexAppServerClient) => Promise; + }, + ) => { + await options?.onResponse?.(response, harness.client); + return response; + }, + ); + + try { + const result = await runCommand("resume thread-overflow", { codexControlRequest }); + + expect(result.text).toContain("lost its native subscription owner"); + expect(oldestRelease).toHaveBeenCalledOnce(); + expect(request).toHaveBeenCalledExactlyOnceWith( + "thread/unsubscribe", + { threadId: "thread-overflow" }, + expect.objectContaining({ timeoutMs: expect.any(Number) }), + ); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + threadId: "thread-existing", + }); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-existing"), + ).resolves.toEqual(expect.objectContaining({ release: expect.any(Function) })); + } finally { + harness.client.close(); + } + }); + + it.each([false, true])( + "migrates manual resume ownership across physical clients (old release fails: %s)", + async (rejectOldRelease) => { + const previous = createClientHarness(); + const replacement = createClientHarness(); + ensureCodexAppServerClientRuntime(previous.client, { agentDir: tempDir }); + ensureCodexAppServerClientRuntime(replacement.client, { agentDir: tempDir }); + const identity = { + kind: "session" as const, + agentId: "main", + sessionId: "session-1", + }; + await writeTestBinding(identity, { + threadId: "thread-manual-migration", + clientId: previous.client.getInstanceId(), + cwd: "/repo", + }); + const operations: string[] = []; + const ownerDuringRelease: Array = []; + vi.spyOn(previous.client, "request").mockImplementation(async (method) => { + operations.push(`previous:${method}`); + ownerDuringRelease.push((await testCodexAppServerBindingStore.read(identity))?.clientId); + if (rejectOldRelease) { + throw new Error("previous manual owner unsubscribe failed"); + } + return {} as never; + }); + vi.spyOn(replacement.client, "request").mockImplementation(async (method) => { + operations.push(`replacement:${method}`); + return {} as never; + }); + await expect( + retainCodexAppServerLiveThread(previous.client, "thread-manual-migration"), + ).resolves.toBe(true); + const sharedClientRuntime = await import("./app-server/shared-client.js"); + const retainPreviousClient = vi + .spyOn(sharedClientRuntime, "retainSharedCodexAppServerClientByInstanceId") + .mockImplementation((clientId) => + clientId === previous.client.getInstanceId() + ? { client: previous.client, release: vi.fn() } + : undefined, + ); + const response = createThreadResumeResponse({ threadId: "thread-manual-migration" }); + const codexControlRequest = vi.fn( + async ( + _pluginConfig: unknown, + _method: string, + _params: unknown, + options?: { + onResponse?: (value: unknown, client: CodexAppServerClient) => Promise; + }, + ) => { + await options?.onResponse?.(response, replacement.client); + return response; + }, + ); + + try { + const result = await runCommand("resume thread-manual-migration", { codexControlRequest }); + + expect(result.text).toContain( + rejectOldRelease + ? "previous manual owner unsubscribe failed" + : "Attached this OpenClaw session", + ); + expect(operations).toEqual( + rejectOldRelease + ? ["previous:thread/unsubscribe", "replacement:thread/unsubscribe"] + : ["previous:thread/unsubscribe"], + ); + expect(ownerDuringRelease).toEqual([previous.client.getInstanceId()]); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + threadId: "thread-manual-migration", + clientId: rejectOldRelease + ? previous.client.getInstanceId() + : replacement.client.getInstanceId(), + }); + const survivingClient = rejectOldRelease ? previous.client : replacement.client; + const obsoleteClient = rejectOldRelease ? replacement.client : previous.client; + await expect( + consumeCodexAppServerLiveThread(survivingClient, "thread-manual-migration"), + ).resolves.toEqual(expect.objectContaining({ release: expect.any(Function) })); + await expect( + consumeCodexAppServerLiveThread(obsoleteClient, "thread-manual-migration"), + ).resolves.toBeUndefined(); + } finally { + retainPreviousClient.mockRestore(); + previous.client.close(); + replacement.client.close(); + } + }, + ); + + it("preserves known native config ownership when manually resuming the same thread", async () => { + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const identity = { + kind: "session" as const, + agentId: "main", + sessionId: "session-1", + }; + await writeTestBinding(identity, { + threadId: "thread-known-resume", + clientId: harness.client.getInstanceId(), + cwd: "/repo", + dynamicToolsFingerprint: "known-dynamic-tools", + pluginAppsFingerprint: "known-plugin-apps", + }); + const release = vi.fn(async () => undefined); + await retainCodexAppServerLiveThread( + harness.client, + "thread-known-resume", + release, + "known-native-config", + "priority", + ); + const response = createThreadResumeResponse({ threadId: "thread-known-resume" }); + const codexControlRequest = vi.fn( + async ( + _pluginConfig: unknown, + _method: string, + _params: unknown, + options?: { + onResponse?: (value: unknown, client: CodexAppServerClient) => Promise; + }, + ) => { + await options?.onResponse?.(response, harness.client); + return response; + }, + ); + + try { + await expect( + runCommand("resume thread-known-resume", { codexControlRequest }), + ).resolves.toMatchObject({ + text: "Attached this OpenClaw session to Codex thread thread-known-resume.", + }); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + dynamicToolsFingerprint: "known-dynamic-tools", + pluginAppsFingerprint: "known-plugin-apps", + }); + await expect( + consumeCodexAppServerLiveThread( + harness.client, + "thread-known-resume", + "known-native-config", + ), + ).resolves.toEqual( + expect.objectContaining({ + configFingerprint: "known-native-config", + serviceTier: "priority", + }), + ); + expect(release).not.toHaveBeenCalled(); + } finally { + harness.client.close(); + } + }); + + it.each([ + { label: "the same", existingThreadId: "thread-active-resume" }, + { label: "a different", existingThreadId: "thread-existing-resume" }, + ])( + "refuses an active native child while the source session owns $label thread", + async ({ existingThreadId }) => { + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const response = createThreadResumeResponse({ threadId: "thread-active-resume" }); + const request = vi.spyOn(harness.client, "request").mockImplementation(async (method) => { + if (method === "thread/resume") { + return response as never; + } + if (method === "thread/unsubscribe") { + return {} as never; + } + throw new Error(`unexpected Codex method ${method}`); + }); + const parent = codexNativeSubagentMonitorRuntime.register({ + client: harness.client, + parentThreadId: "thread-parent", + }); + const identity = { + kind: "session" as const, + agentId: "main", + sessionId: "session-1", + }; + await writeTestBinding(identity, { + threadId: existingThreadId, + clientId: harness.client.getInstanceId(), + cwd: "/repo", + }); + if (existingThreadId !== "thread-active-resume") { + await retainCodexAppServerLiveThread(harness.client, existingThreadId); + } + harness.send({ + method: "thread/started", + params: { + thread: { + id: "thread-active-resume", + parentThreadId: "thread-parent", + source: { + subAgent: { + thread_spawn: { + parent_thread_id: "thread-parent", + depth: 1, + agent_path: "thread-active-resume", + }, + }, + }, + }, + }, + }); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-active-resume")).toBe( + true, + ), + ); + const codexControlRequest = vi.fn( + async ( + _pluginConfig: unknown, + _method: string, + _params: unknown, + options?: { + onResponse?: (value: unknown, client: CodexAppServerClient) => Promise; + }, + ) => { + const resumed = await harness.client.request("thread/resume", { + threadId: "thread-active-resume", + excludeTurns: true, + }); + await options?.onResponse?.(resumed, harness.client); + return response; + }, + ); + + try { + const result = await runCommand("resume thread-active-resume", { codexControlRequest }); + + expect(result.text).toContain("lost its native subscription owner"); + expect(request.mock.calls.map(([method]) => method)).toEqual(["thread/resume"]); + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-active-resume")).toBe( + true, + ); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + threadId: existingThreadId, + }); + await expect( + retainCodexAppServerLiveThread(harness.client, "thread-active-resume"), + ).resolves.toBe(false); + if (existingThreadId !== "thread-active-resume") { + const previousOwnership = await consumeCodexAppServerLiveThread( + harness.client, + existingThreadId, + ); + expect(previousOwnership).toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + await expect( + retainCodexAppServerLiveThread( + harness.client, + existingThreadId, + previousOwnership?.release, + ), + ).resolves.toBe(true); + } + } finally { + parent.unregister(); + harness.client.close(); + } + }, + ); + it("serializes manual resume with other session binding owners", async () => { const identity = { kind: "session" as const, @@ -540,6 +921,26 @@ describe("codex command", () => { expect(order).toEqual(["resume-start", "resume-done", "competing-owner"]); }); + it("rejects manual resume of a thread owned by another OpenClaw session", async () => { + const otherIdentity = { + kind: "session" as const, + agentId: "main", + sessionId: "session-other", + }; + await writeTestBinding(otherIdentity, { threadId: "thread-owned", cwd: "/other" }); + const codexControlRequest = vi.fn(async () => + createThreadResumeResponse({ threadId: "thread-owned" }), + ); + + const result = await runCommand("resume thread-owned", { codexControlRequest }); + + expect(result.text).toContain("owned by another OpenClaw session"); + expect(codexControlRequest).not.toHaveBeenCalled(); + await expect(testCodexAppServerBindingStore.read(otherIdentity)).resolves.toMatchObject({ + threadId: "thread-owned", + }); + }); + it("reclaims an unloaded plugin's stale generation before attaching a thread", async () => { const sessionKey = "agent:main:test:chat-1"; const storePath = path.join(tempDir, "sessions.json"); @@ -4661,12 +5062,207 @@ describe("codex command", () => { it("detaches the current conversation and clears the Codex app-server thread binding", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); - const clearBinding = vi.fn(async () => true); - const detachConversationBinding = vi.fn(async () => ({ removed: true })); + const ownershipOrder: string[] = []; + const identity = { kind: "conversation" as const, bindingId: "binding-data-1" }; + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const releaseNativeThread = vi + .spyOn(harness.client, "request") + .mockImplementation(async (method) => { + if (method !== "thread/unsubscribe") { + throw new Error(`unexpected Codex method ${method}`); + } + ownershipOrder.push("native-release"); + return {} as never; + }); + await retainCodexAppServerLiveThread(harness.client, "thread-detached"); + const clearBinding = vi.fn( + async (...args: Parameters) => { + ownershipOrder.push("native-clear"); + return await testCodexAppServerBindingStore.mutate(...args); + }, + ); + const detachConversationBinding = vi.fn(async () => { + ownershipOrder.push("public"); + return { removed: true }; + }); + await writeTestBinding(identity, { + threadId: "thread-detached", + clientId: harness.client.getInstanceId(), + cwd: "/repo", + }); + const sharedClientRuntime = await import("./app-server/shared-client.js"); + const retainClient = vi + .spyOn(sharedClientRuntime, "retainSharedCodexAppServerClientByInstanceId") + .mockReturnValue({ client: harness.client, release: vi.fn() }); - await expect( - handleCodexCommand( - createContext("detach", sessionFile, { + try { + await expect( + handleCodexCommand( + createContext("detach", sessionFile, { + detachConversationBinding, + getCurrentConversationBinding: async () => ({ + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: "/plugin", + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + data: { + kind: "codex-app-server-session", + version: 2, + bindingId: "binding-data-1", + workspaceDir: "/repo", + }, + }), + }), + { + deps: createDeps({ + bindingStore: { ...testCodexAppServerBindingStore, mutate: clearBinding }, + }), + }, + ), + ).resolves.toEqual({ + text: "Detached this conversation from Codex.", + }); + expect(detachConversationBinding).toHaveBeenCalled(); + expect(clearBinding).toHaveBeenCalledWith(identity, { + kind: "clear", + threadId: "thread-detached", + }); + expect(releaseNativeThread).toHaveBeenCalledWith( + "thread/unsubscribe", + { threadId: "thread-detached" }, + expect.objectContaining({ timeoutMs: expect.any(Number) }), + ); + expect(ownershipOrder).toEqual(["native-release", "native-clear", "public"]); + } finally { + retainClient.mockRestore(); + harness.client.close(); + } + }); + + it.each([ + { + label: "an incognito source bound into an ordinary destination", + sourceSessionKey: "agent:main:dashboard:incognito-source", + destinationSessionKey: "agent:main:discord:ordinary-destination", + unsubscribes: true, + }, + { + label: "an ordinary source bound into an incognito destination", + sourceSessionKey: "agent:main:discord:ordinary-source", + destinationSessionKey: "agent:main:dashboard:incognito-destination", + unsubscribes: false, + }, + { + label: "a missing source bound into an incognito destination", + sourceSessionKey: undefined, + destinationSessionKey: "agent:main:dashboard:incognito-destination", + unsubscribes: false, + }, + ])( + "retires untracked detach ownership from $label using its source session", + async ({ sourceSessionKey, destinationSessionKey, unsubscribes }) => { + const identity = { kind: "conversation" as const, bindingId: "binding-mixed-session" }; + await writeTestBinding(identity, { + threadId: "thread-mixed-session", + clientId: "client-mixed-session", + cwd: "/repo", + }); + const request = vi.fn(async () => ({})); + const releaseClient = vi.fn(); + const sharedClientRuntime = await import("./app-server/shared-client.js"); + const retainClient = vi + .spyOn(sharedClientRuntime, "retainSharedCodexAppServerClientByInstanceId") + .mockReturnValue({ + client: { request } as unknown as CodexAppServerClient, + release: releaseClient, + }); + const detachConversationBinding = vi.fn(async () => ({ removed: true })); + + try { + await expect( + handleCodexCommand( + createContext("detach", undefined, { + sessionKey: destinationSessionKey, + detachConversationBinding, + getCurrentConversationBinding: async () => ({ + bindingId: "binding-public", + pluginId: "codex", + pluginRoot: "/plugin", + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + data: { + kind: "codex-app-server-session", + version: 2, + bindingId: identity.bindingId, + workspaceDir: "/repo", + ...(sourceSessionKey + ? { + source: { + agentId: "main", + sessionId: "session-source", + sessionKey: sourceSessionKey, + threadId: "thread-source", + }, + } + : {}), + }, + }), + }), + { deps: createDeps() }, + ), + ).resolves.toEqual({ text: "Detached this conversation from Codex." }); + + if (unsubscribes) { + expect(request).toHaveBeenCalledExactlyOnceWith( + "thread/unsubscribe", + { threadId: "thread-mixed-session" }, + expect.objectContaining({ timeoutMs: expect.any(Number) }), + ); + } else { + expect(request).not.toHaveBeenCalled(); + } + expect(releaseClient).toHaveBeenCalledOnce(); + expect(detachConversationBinding).toHaveBeenCalledOnce(); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toBeUndefined(); + } finally { + retainClient.mockRestore(); + } + }, + ); + + it("preserves the public conversation binding when native retirement fails", async () => { + const identity = { kind: "conversation" as const, bindingId: "binding-data-1" }; + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + await writeTestBinding(identity, { + threadId: "thread-detached", + clientId: harness.client.getInstanceId(), + cwd: "/repo", + }); + const detachConversationBinding = vi.fn(async () => ({ removed: true })); + const releaseNativeThread = vi + .spyOn(harness.client, "request") + .mockImplementation(async (method) => { + if (method !== "thread/unsubscribe") { + throw new Error(`unexpected Codex method ${method}`); + } + throw new Error("Codex native thread subscription could not be released"); + }); + await retainCodexAppServerLiveThread(harness.client, "thread-detached"); + const sharedClientRuntime = await import("./app-server/shared-client.js"); + const retainClient = vi + .spyOn(sharedClientRuntime, "retainSharedCodexAppServerClientByInstanceId") + .mockReturnValue({ client: harness.client, release: vi.fn() }); + + try { + const result = await handleCodexCommand( + createContext("detach", undefined, { detachConversationBinding, getCurrentConversationBinding: async () => ({ bindingId: "binding-1", @@ -4679,25 +5275,330 @@ describe("codex command", () => { data: { kind: "codex-app-server-session", version: 2, - bindingId: "binding-data-1", + bindingId: identity.bindingId, workspaceDir: "/repo", }, }), }), + { deps: createDeps() }, + ); + + expect(result.text).toContain("native thread subscription could not be released"); + expect(releaseNativeThread).toHaveBeenCalledOnce(); + expect(detachConversationBinding).not.toHaveBeenCalled(); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + threadId: "thread-detached", + clientId: harness.client.getInstanceId(), + cwd: "/repo", + }); + await expect( + consumeCodexAppServerLiveThread(harness.client, "thread-detached"), + ).resolves.toEqual(expect.objectContaining({ release: expect.any(Function) })); + } finally { + retainClient.mockRestore(); + harness.client.close(); + } + }); + + it.each([ + { + label: "returns false", + throws: false, + message: "changed while detaching", + }, + { + label: "throws", + throws: true, + message: "native durable binding clear failed", + }, + ])( + "preserves the public conversation when durable native clear $label", + async ({ throws, message }) => { + const identity = { kind: "conversation" as const, bindingId: "binding-clear-failure" }; + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const releaseNativeThread = vi + .spyOn(harness.client, "request") + .mockResolvedValue({} as never); + await retainCodexAppServerLiveThread(harness.client, "thread-clear-failure"); + const originalBinding = { + threadId: "thread-clear-failure", + clientId: harness.client.getInstanceId(), + cwd: tempDir, + conversationStartId: "start-clear-failure", + } satisfies CodexAppServerThreadBinding; + await writeTestBinding(identity, originalBinding); + const mutate = vi.fn( + async (...args: Parameters) => { + if (args[1].kind === "clear") { + if (throws) { + throw new Error("native durable binding clear failed"); + } + return false; + } + return await testCodexAppServerBindingStore.mutate(...args); + }, + ); + const detachConversationBinding = vi.fn(async () => ({ removed: true })); + const sharedClientRuntime = await import("./app-server/shared-client.js"); + const retainClient = vi + .spyOn(sharedClientRuntime, "retainSharedCodexAppServerClientByInstanceId") + .mockReturnValue({ client: harness.client, release: vi.fn() }); + + try { + const result = await handleCodexCommand( + createContext("detach", undefined, { + detachConversationBinding, + getCurrentConversationBinding: async () => ({ + bindingId: "binding-public-clear-failure", + pluginId: "codex", + pluginRoot: tempDir, + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + data: { + kind: "codex-app-server-session", + version: 2, + bindingId: identity.bindingId, + workspaceDir: tempDir, + start: { id: originalBinding.conversationStartId }, + }, + }), + }), + { + deps: createDeps({ + bindingStore: { ...testCodexAppServerBindingStore, mutate }, + }), + }, + ); + + expect(result.text).toContain(message); + expect(releaseNativeThread).toHaveBeenCalledOnce(); + expect(mutate).toHaveBeenCalledOnce(); + expect(detachConversationBinding).not.toHaveBeenCalled(); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject( + originalBinding, + ); + } finally { + retainClient.mockRestore(); + harness.client.close(); + } + }, + ); + + it("resumes the original native thread after public conversation detachment fails", async () => { + const identity = { kind: "conversation" as const, bindingId: "binding-detach-recovery" }; + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const operations: string[] = []; + const request = vi + .spyOn(harness.client, "request") + .mockImplementation(async (method, params) => { + operations.push(method); + if (method === "thread/unsubscribe") { + return {} as never; + } + if (method === "thread/resume") { + return createThreadResumeResponse({ + threadId: "thread-original-context", + cwd: tempDir, + }) as never; + } + if (method === "turn/start") { + queueMicrotask(() => { + harness.send({ + method: "turn/completed", + params: { + threadId: "thread-original-context", + turn: { + id: "turn-original-context", + status: "completed", + items: [{ type: "agentMessage", id: "answer", text: "Original context kept" }], + }, + }, + }); + }); + return { turn: { id: "turn-original-context" } } as never; + } + throw new Error(`unexpected Codex method ${method}: ${JSON.stringify(params)}`); + }); + await retainCodexAppServerLiveThread(harness.client, "thread-original-context"); + const originalBinding = { + threadId: "thread-original-context", + clientId: harness.client.getInstanceId(), + cwd: tempDir, + conversationStartId: "start-original-context", + historyCoveredThrough: "2026-01-01T00:00:00.000Z", + } satisfies CodexAppServerThreadBinding; + await writeTestBinding(identity, originalBinding); + const publicBinding = { + bindingId: "binding-public-original-context", + pluginId: "codex", + pluginRoot: tempDir, + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + data: { + kind: "codex-app-server-session" as const, + version: 2 as const, + bindingId: identity.bindingId, + workspaceDir: tempDir, + start: { id: originalBinding.conversationStartId }, + }, + }; + const detachConversationBinding = vi.fn(async () => { + throw new Error("public conversation binding store write failed"); + }); + const mutate = vi.fn( + async (...args: Parameters) => + await testCodexAppServerBindingStore.mutate(...args), + ); + const sharedClientRuntime = await import("./app-server/shared-client.js"); + const retainClient = vi + .spyOn(sharedClientRuntime, "retainSharedCodexAppServerClientByInstanceId") + .mockReturnValue({ client: harness.client, release: vi.fn() }); + const acquireClient = vi + .spyOn(sharedClientRuntime, "getLeasedSharedCodexAppServerClient") + .mockResolvedValue(harness.client); + const resolvePublic = vi + .spyOn(getSessionBindingService(), "resolveByConversation") + .mockReturnValue({ bindingId: publicBinding.bindingId } as never); + + try { + const result = await handleCodexCommand( + createContext("detach", undefined, { + detachConversationBinding, + getCurrentConversationBinding: async () => publicBinding, + }), + { + deps: createDeps({ + bindingStore: { ...testCodexAppServerBindingStore, mutate }, + }), + }, + ); + + expect(result.text).toContain("public conversation binding store write failed"); + expect(operations).toEqual(["thread/unsubscribe"]); + expect(mutate.mock.calls.map(([, mutation]) => mutation.kind)).toEqual(["clear", "set"]); + expect(mutate).toHaveBeenLastCalledWith(identity, { + kind: "set", + binding: originalBinding, + if: { kind: "absent" }, + }); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject( + originalBinding, + ); + + await expect( + codexConversationBindingRuntime.handleInboundClaim( + { + content: "continue original task", + bodyForAgent: "continue original task", + channel: "test", + isGroup: false, + commandAuthorized: true, + senderIsOwner: true, + }, + { channelId: "test", pluginBinding: publicBinding }, + { bindingStore: testCodexAppServerBindingStore, timeoutMs: 500 }, + ), + ).resolves.toEqual({ + handled: true, + reply: { text: "Original context kept" }, + }); + expect(operations).toEqual(["thread/unsubscribe", "thread/resume", "turn/start"]); + expect(request.mock.calls.find(([method]) => method === "thread/resume")?.[1]).toMatchObject({ + threadId: originalBinding.threadId, + }); + expect(request.mock.calls.find(([method]) => method === "turn/start")?.[1]).toMatchObject({ + threadId: originalBinding.threadId, + cwd: originalBinding.cwd, + }); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + threadId: originalBinding.threadId, + conversationStartId: originalBinding.conversationStartId, + historyCoveredThrough: originalBinding.historyCoveredThrough, + }); + } finally { + resolvePublic.mockRestore(); + acquireClient.mockRestore(); + retainClient.mockRestore(); + harness.client.close(); + } + }); + + it.each([ + { label: "returns false", throws: false }, + { label: "throws", throws: true }, + ])("shows actionable thread recovery when public detach rollback $label", async ({ throws }) => { + const identity = { kind: "conversation" as const, bindingId: "binding-rollback-failure" }; + const harness = createClientHarness(); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const releaseNativeThread = vi.spyOn(harness.client, "request").mockResolvedValue({} as never); + await retainCodexAppServerLiveThread(harness.client, "thread-rollback-failure"); + await writeTestBinding(identity, { + threadId: "thread-rollback-failure", + clientId: harness.client.getInstanceId(), + cwd: tempDir, + }); + const mutate = vi.fn( + async (...args: Parameters) => { + if (args[1].kind === "set") { + if (throws) { + throw new Error("native durable binding restore failed"); + } + return false; + } + return await testCodexAppServerBindingStore.mutate(...args); + }, + ); + const detachConversationBinding = vi.fn(async () => { + throw new Error("public conversation binding store write failed"); + }); + const sharedClientRuntime = await import("./app-server/shared-client.js"); + const retainClient = vi + .spyOn(sharedClientRuntime, "retainSharedCodexAppServerClientByInstanceId") + .mockReturnValue({ client: harness.client, release: vi.fn() }); + + try { + const result = await handleCodexCommand( + createContext("detach", undefined, { + detachConversationBinding, + getCurrentConversationBinding: async () => ({ + bindingId: "binding-public-rollback-failure", + pluginId: "codex", + pluginRoot: tempDir, + channel: "test", + accountId: "default", + conversationId: "conversation", + boundAt: 1, + data: { + kind: "codex-app-server-session", + version: 2, + bindingId: identity.bindingId, + workspaceDir: tempDir, + }, + }), + }), { deps: createDeps({ - bindingStore: { ...testCodexAppServerBindingStore, mutate: clearBinding }, + bindingStore: { ...testCodexAppServerBindingStore, mutate }, }), }, - ), - ).resolves.toEqual({ - text: "Detached this conversation from Codex.", - }); - expect(detachConversationBinding).toHaveBeenCalled(); - expect(clearBinding).toHaveBeenCalledWith( - { kind: "conversation", bindingId: "binding-data-1" }, - { kind: "clear" }, - ); + ); + + expect(result.text).toContain("native thread thread-rollback-failure could not be restored"); + expect(result.text).toContain("/codex resume thread-rollback-failure"); + expect(releaseNativeThread).toHaveBeenCalledOnce(); + expect(detachConversationBinding).toHaveBeenCalledOnce(); + expect(mutate.mock.calls.map(([, mutation]) => mutation.kind)).toEqual(["clear", "set"]); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toBeUndefined(); + } finally { + retainClient.mockRestore(); + harness.client.close(); + } }); it("rejects malformed detach commands before clearing bindings", async () => { diff --git a/extensions/codex/src/conversation-binding.test.ts b/extensions/codex/src/conversation-binding.test.ts index 8aaad15dbdb4..8279ac91f454 100644 --- a/extensions/codex/src/conversation-binding.test.ts +++ b/extensions/codex/src/conversation-binding.test.ts @@ -2,12 +2,21 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { + clearActiveEmbeddedRun, + setActiveEmbeddedRun, +} from "openclaw/plugin-sdk/agent-harness-runtime"; import type { ExecApprovalsFile } from "openclaw/plugin-sdk/exec-approvals-runtime"; +import type { PluginConversationBinding } from "openclaw/plugin-sdk/plugin-entry"; import { upsertSessionEntry } from "openclaw/plugin-sdk/session-store-runtime"; +import { appendSessionTranscriptMessageByIdentity } from "openclaw/plugin-sdk/session-transcript-runtime"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const sharedClientMocks = vi.hoisted(() => ({ getSharedCodexAppServerClient: vi.fn(), + retainSharedCodexAppServerClientByInstanceId: vi.fn( + (_clientId?: string): { client: unknown; release: () => void } | undefined => undefined, + ), clearSharedCodexAppServerClientIfCurrentAndUnclaimed: vi.fn((_client: unknown) => ({ found: false, closed: false, @@ -18,6 +27,12 @@ const sharedClientMocks = vi.hoisted(() => ({ clearSharedCodexAppServerClientIfCurrent: vi.fn((_client: unknown) => false), })); +const publicBindingMocks = vi.hoisted(() => ({ + resolveByConversation: vi.fn((_conversation: unknown): { bindingId: string } | null => ({ + bindingId: "binding-1", + })), +})); + const execApprovalsRuntimeMocks = vi.hoisted(() => ({ loadExecApprovals: vi.fn<() => ExecApprovalsFile>(() => ({ version: 1, agents: {} })), })); @@ -63,6 +78,17 @@ vi.mock("openclaw/plugin-sdk/agent-harness-runtime", async (importOriginal) => { }; }); +vi.mock("openclaw/plugin-sdk/conversation-binding-runtime", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getSessionBindingService: () => ({ + resolveByConversation: publicBindingMocks.resolveByConversation, + }), + }; +}); + vi.mock("./app-server/shared-client.js", () => ({ ...sharedClientMocks, isCodexAppServerStartSelectionChangedError: (error: unknown) => @@ -97,7 +123,15 @@ vi.mock("openclaw/plugin-sdk/exec-approvals-runtime", async (importOriginal) => }); vi.mock("openclaw/plugin-sdk/agent-runtime", () => agentRuntimeMocks); +import { + consumeCodexAppServerLiveThread, + ensureCodexAppServerClientRuntime, + isCodexAppServerLiveThreadClaimed, + retainCodexAppServerLiveThread, +} from "./app-server/client-runtime.js"; +import type { CodexAppServerClient } from "./app-server/client.js"; import { resolveCodexAppServerRuntimeOptions } from "./app-server/config.js"; +import { codexNativeSubagentMonitorRuntime } from "./app-server/native-subagent-monitor.js"; import type { JsonValue } from "./app-server/protocol.js"; import { readCodexAppServerBinding, @@ -108,6 +142,7 @@ import { writeCodexAppServerBinding, } from "./app-server/session-binding.test-helpers.js"; import { createClientHarness } from "./app-server/test-support.js"; +import { withCodexConversationThreadActivity } from "./app-server/thread-ownership.js"; import { getCodexAppServerTurnRouter } from "./app-server/turn-router.js"; import { legacyCodexConversationBindingId } from "./conversation-binding-data.js"; import { codexConversationBindingRuntime } from "./conversation-binding.js"; @@ -140,6 +175,21 @@ async function readTestConversationBinding(sessionFile: string) { } function boundConversationClaim(sessionFile: string, sessionKey?: string) { + const pluginBinding: PluginConversationBinding = { + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: tempDir, + channel: "telegram", + accountId: "default", + conversationId: "5185575566", + boundAt: Date.now(), + data: { + kind: "codex-app-server-session" as const, + version: 1 as const, + sessionFile, + workspaceDir: tempDir, + }, + }; return { event: { content: "continue", @@ -152,25 +202,97 @@ function boundConversationClaim(sessionFile: string, sessionKey?: string) { ctx: { channelId: "telegram", ...(sessionKey ? { sessionKey } : {}), - pluginBinding: { - bindingId: "binding-1", - pluginId: "codex", - pluginRoot: tempDir, - channel: "telegram", - accountId: "default", - conversationId: "5185575566", - boundAt: Date.now(), - data: { - kind: "codex-app-server-session" as const, - version: 1 as const, - sessionFile, - workspaceDir: tempDir, - }, - }, + pluginBinding, }, }; } +async function createSameThreadClientMigrationFixture( + sessionFile: string, + options: { rejectOldRelease: boolean; owner: "session" | "conversation" }, +) { + const binding = { + threadId: "thread-migrated", + clientId: "client-before-migration", + cwd: tempDir, + }; + const readOwner = + options.owner === "session" + ? async () => readCodexAppServerBinding(sessionFile) + : async () => readTestConversationBinding(sessionFile); + if (options.owner === "session") { + await writeCodexAppServerBinding(sessionFile, binding); + } else { + await writeTestConversationBinding(sessionFile, binding); + } + const operations: string[] = []; + const ownerDuringRelease: Array = []; + const notificationHandlers = new Set<(notification: unknown) => void>(); + const previousClient = { + getInstanceId: () => "client-before-migration", + request: vi.fn(async (method: string) => { + operations.push(`previous:${method}`); + ownerDuringRelease.push((await readOwner())?.clientId); + if (options.rejectOldRelease) { + throw new Error("previous physical client unsubscribe failed"); + } + return {}; + }), + addNotificationHandler: vi.fn(() => () => undefined), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + const replacementClient = { + getInstanceId: () => "client-after-migration", + request: vi.fn(async (method: string) => { + operations.push(`replacement:${method}`); + if (method === "thread/resume") { + return conversationThreadStartResult("thread-migrated"); + } + if (method === "thread/unsubscribe") { + return {}; + } + if (method === "turn/start") { + queueMicrotask(() => { + for (const handler of notificationHandlers) { + handler({ + method: "turn/completed", + params: { + threadId: "thread-migrated", + turn: { + id: "turn-migrated", + status: "completed", + items: [{ type: "agentMessage", id: "answer", text: "Migrated reply" }], + }, + }, + }); + } + }); + return { turn: { id: "turn-migrated" } }; + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn((handler: (notification: unknown) => void) => { + notificationHandlers.add(handler); + return () => notificationHandlers.delete(handler); + }), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(previousClient, { agentDir: tempDir }); + ensureCodexAppServerClientRuntime(replacementClient, { agentDir: tempDir }); + await expect(retainCodexAppServerLiveThread(previousClient, "thread-migrated")).resolves.toBe( + true, + ); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(replacementClient); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockImplementation((clientId) => + clientId === previousClient.getInstanceId() + ? { client: previousClient, release: vi.fn() } + : undefined, + ); + return { previousClient, replacementClient, operations, ownerDuringRelease, readOwner }; +} + function handleCodexConversationInboundClaim( event: Parameters[0], ctx: Parameters[1], @@ -268,7 +390,11 @@ describe("codex conversation binding", () => { }); afterEach(async () => { + publicBindingMocks.resolveByConversation.mockReset(); + publicBindingMocks.resolveByConversation.mockReturnValue({ bindingId: "binding-1" }); sharedClientMocks.getSharedCodexAppServerClient.mockReset(); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReset(); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReturnValue(undefined); sharedClientMocks.clearSharedCodexAppServerClientIfCurrentAndUnclaimed.mockReset(); sharedClientMocks.clearSharedCodexAppServerClientIfCurrentAndUnclaimed.mockReturnValue({ found: false, @@ -395,6 +521,169 @@ describe("codex conversation binding", () => { } }); + it("keeps queued bound turns ahead of retirement on their shared owner lane", async () => { + const sessionFile = path.join(tempDir, "queued-session.jsonl"); + await writeTestConversationBinding(sessionFile, { threadId: "bound-thread", cwd: tempDir }); + const notificationHandlers = new Set<(notification: unknown) => void>(); + const order: string[] = []; + const client = { + request: vi.fn(async (method: string) => { + if (method !== "turn/start") { + throw new Error(`unexpected method: ${method}`); + } + const turnId = `turn-${order.filter((entry) => entry.startsWith("turn-")).length + 1}`; + order.push(turnId); + return { turn: { id: turnId } }; + }), + addNotificationHandler: vi.fn((handler: (notification: unknown) => void) => { + notificationHandlers.add(handler); + return () => notificationHandlers.delete(handler); + }), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + }; + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(client); + const { event, ctx } = boundConversationClaim(sessionFile); + const completeTurn = (turnId: string) => { + for (const handler of notificationHandlers) { + handler({ + method: "turn/completed", + params: { + threadId: "bound-thread", + turn: { + id: turnId, + status: "completed", + items: [{ type: "agentMessage", id: `${turnId}-answer`, text: turnId }], + }, + }, + }); + } + }; + + const firstTurn = handleCodexConversationInboundClaim(event, ctx); + await vi.waitFor(() => expect(client.request).toHaveBeenCalledOnce()); + const secondTurn = handleCodexConversationInboundClaim(event, ctx); + const retirement = withCodexConversationThreadActivity( + legacyCodexConversationBindingId(sessionFile), + async () => { + order.push("retired"); + }, + ); + + completeTurn("turn-1"); + await vi.waitFor(() => expect(client.request).toHaveBeenCalledTimes(2)); + expect(order).toEqual(["turn-1", "turn-2"]); + completeTurn("turn-2"); + + await expect(firstTurn).resolves.toMatchObject({ reply: { text: "turn-1" } }); + await expect(secondTurn).resolves.toMatchObject({ reply: { text: "turn-2" } }); + await retirement; + expect(order).toEqual(["turn-1", "turn-2", "retired"]); + }); + + it.each(["detached", "replaced", "cleared-before-capture"] as const)( + "does not recreate a %s conversation from an inbound claim queued behind retirement", + async (outcome) => { + const bindingId = "binding-retiring"; + const identity = { kind: "conversation" as const, bindingId }; + if (outcome === "detached" || outcome === "cleared-before-capture") { + await testCodexAppServerBindingStore.mutate(identity, { + kind: "set", + binding: { + threadId: "thread-retiring", + cwd: tempDir, + conversationStartId: "start-original", + }, + }); + } + let retireConversation: (() => void) | undefined; + const retirementReady = new Promise((resolve) => { + retireConversation = resolve; + }); + let retirementStarted: (() => void) | undefined; + const enteredRetirement = new Promise((resolve) => { + retirementStarted = resolve; + }); + const retirement = withCodexConversationThreadActivity(bindingId, async () => { + if (outcome === "cleared-before-capture") { + await testCodexAppServerBindingStore.mutate(identity, { + kind: "clear", + threadId: "thread-retiring", + }); + } + retirementStarted?.(); + await retirementReady; + if (outcome === "detached") { + await testCodexAppServerBindingStore.mutate(identity, { + kind: "clear", + threadId: "thread-retiring", + }); + } else if (outcome === "replaced") { + await testCodexAppServerBindingStore.mutate(identity, { + kind: "set", + binding: { + threadId: "thread-replacement", + cwd: tempDir, + conversationStartId: "start-replacement", + }, + }); + } + publicBindingMocks.resolveByConversation.mockReturnValue( + outcome === "replaced" ? { bindingId: "binding-replacement" } : null, + ); + }); + await enteredRetirement; + let capturedOwner: (() => void) | undefined; + const ownerCaptured = new Promise((resolve) => { + capturedOwner = resolve; + }); + const bindingStore = { + ...testCodexAppServerBindingStore, + read: async ( + requestedIdentity: Parameters[0], + ) => { + const binding = await testCodexAppServerBindingStore.read(requestedIdentity); + capturedOwner?.(); + return binding; + }, + }; + const { event, ctx } = boundConversationClaim(path.join(tempDir, "retiring-session.jsonl")); + ctx.pluginBinding.data = { + kind: "codex-app-server-session" as const, + version: 2 as const, + bindingId, + workspaceDir: tempDir, + start: { id: "start-original" }, + }; + const queued = handleCodexConversationInboundClaimImpl( + { senderIsOwner: true, ...event }, + ctx, + { + bindingStore, + }, + ); + await ownerCaptured; + + retireConversation?.(); + + await retirement; + await expect(queued).resolves.toEqual({ + handled: true, + reply: { + text: "This Codex conversation was detached or changed before its message could run.", + }, + }); + expect(sharedClientMocks.getSharedCodexAppServerClient).not.toHaveBeenCalled(); + if (outcome !== "replaced") { + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toBeUndefined(); + } else { + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + threadId: "thread-replacement", + }); + } + }, + ); + it("uses the default Codex auth profile and omits the public OpenAI provider for new binds", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const sessionKey = "agent:main:dashboard:incognito-native-bind"; @@ -454,6 +743,184 @@ describe("codex conversation binding", () => { }); }); + it.each([ + { + label: "an incognito source bound to an ordinary destination", + sourceSessionKey: "agent:main:dashboard:incognito-source", + destinationSessionKey: "agent:main:telegram:ordinary-destination", + ephemeral: true, + turnFails: false, + }, + { + label: "an ordinary source bound to an incognito destination", + sourceSessionKey: "agent:main:telegram:ordinary-source", + destinationSessionKey: "agent:main:dashboard:incognito-destination", + ephemeral: false, + turnFails: false, + }, + { + label: "a source without a session key bound to an incognito destination", + sourceSessionKey: undefined, + destinationSessionKey: "agent:main:dashboard:incognito-destination", + ephemeral: false, + turnFails: false, + }, + { + label: "a failing incognito source bound to an ordinary destination", + sourceSessionKey: "agent:main:dashboard:incognito-source", + destinationSessionKey: "agent:main:telegram:ordinary-destination", + ephemeral: true, + turnFails: true, + }, + { + label: "a failing ordinary source bound to an incognito destination", + sourceSessionKey: "agent:main:telegram:ordinary-source", + destinationSessionKey: "agent:main:dashboard:incognito-destination", + ephemeral: false, + turnFails: true, + }, + ])( + "uses the persisted source lifecycle for $label", + async ({ sourceSessionKey, destinationSessionKey, ephemeral, turnFails }) => { + const sessionFile = path.join(tempDir, "mixed-source-lifecycle.jsonl"); + const bindingId = "binding-mixed-source-lifecycle"; + const operations: Array<{ method: string; params: Record }> = []; + const notificationHandlers = new Set<(notification: unknown) => void>(); + const client = { + getInstanceId: () => "client-mixed-source-lifecycle", + request: vi.fn(async (method: string, params: Record) => { + operations.push({ method, params }); + if (method === "thread/start") { + return conversationThreadStartResult("thread-mixed-source-lifecycle"); + } + if (method === "turn/start") { + if (turnFails) { + throw new Error("mixed source lifecycle turn failed"); + } + queueMicrotask(() => { + for (const handler of notificationHandlers) { + handler({ + method: "turn/completed", + params: { + threadId: "thread-mixed-source-lifecycle", + turn: { + id: "turn-mixed-source-lifecycle", + status: "completed", + items: [{ type: "agentMessage", id: "answer", text: "Bound reply" }], + }, + }, + }); + } + }); + return { turn: { id: "turn-mixed-source-lifecycle" } }; + } + if (method === "thread/unsubscribe") { + return {}; + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn((handler: (notification: unknown) => void) => { + notificationHandlers.add(handler); + return () => notificationHandlers.delete(handler); + }), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: tempDir }); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(client); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReturnValue({ + client, + release: vi.fn(), + }); + const { event, ctx } = boundConversationClaim(sessionFile, destinationSessionKey); + const data = { + kind: "codex-app-server-session" as const, + version: 2 as const, + bindingId, + workspaceDir: tempDir, + source: { + agentId: "main", + sessionId: "source-mixed-lifecycle", + threadId: "thread-source-mixed-lifecycle", + ...(sourceSessionKey ? { sessionKey: sourceSessionKey } : {}), + }, + start: { id: "start-mixed-source-lifecycle" }, + }; + ctx.pluginBinding.data = data; + + await expect(handleCodexConversationInboundClaim(event, ctx)).resolves.toMatchObject({ + handled: true, + reply: { + text: turnFails + ? "Codex app-server turn failed: mixed source lifecycle turn failed" + : "Bound reply", + }, + }); + + expect(operations.map(({ method }) => method)).toEqual( + turnFails + ? ["thread/start", "turn/start", "thread/unsubscribe"] + : ["thread/start", "turn/start"], + ); + if (ephemeral) { + expect(operations[0]?.params.ephemeral).toBe(true); + await expect( + consumeCodexAppServerLiveThread(client, "thread-mixed-source-lifecycle"), + ).resolves.toBeUndefined(); + } else { + expect(operations[0]?.params).not.toHaveProperty("ephemeral"); + if (turnFails) { + await expect( + consumeCodexAppServerLiveThread(client, "thread-mixed-source-lifecycle"), + ).resolves.toBeUndefined(); + } else { + const ownership = await consumeCodexAppServerLiveThread( + client, + "thread-mixed-source-lifecycle", + ); + expect(ownership).toEqual(expect.objectContaining({ release: expect.any(Function) })); + await expect( + retainCodexAppServerLiveThread( + client, + "thread-mixed-source-lifecycle", + ownership?.release, + ownership?.configFingerprint, + ownership?.serviceTier, + ), + ).resolves.toBe(true); + } + } + + if (turnFails && ephemeral) { + await expect( + testCodexAppServerBindingStore.read({ kind: "conversation", bindingId }), + ).resolves.toBeUndefined(); + return; + } + + await handleCodexConversationBindingResolved({ + status: "denied", + decision: "deny", + request: { + data, + conversation: { + channel: "telegram", + accountId: "default", + conversationId: "5185575566", + }, + }, + }); + + expect(operations.at(-1)).toEqual({ + method: "thread/unsubscribe", + params: { threadId: "thread-mixed-source-lifecycle" }, + }); + await expect( + testCodexAppServerBindingStore.read({ kind: "conversation", bindingId }), + ).resolves.toBeUndefined(); + }, + ); + it("selects Codex network-proxy permissions through app-server bind thread config", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const requests: Array<{ method: string; params: Record }> = []; @@ -514,6 +981,193 @@ describe("codex conversation binding", () => { expect(bindingAfterStart?.networkProxyConfigFingerprint).toBe(NETWORK_PROXY_CONFIG_FINGERPRINT); }); + it("drops a retained native child before applying bound-only apps and sandbox policy", async () => { + const sessionFile = path.join(tempDir, "retained-child-session.jsonl"); + const requests: Array<{ method: string; params: Record }> = []; + const client = { + getInstanceId: () => "client-native-child", + request: vi.fn(async (method: string, requestParams: Record) => { + requests.push({ method, params: requestParams }); + if (method === "thread/unsubscribe") { + return {}; + } + if (method === "thread/resume") { + return conversationThreadStartResult("thread-native-child"); + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn(() => () => undefined), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: tempDir }); + await retainCodexAppServerLiveThread(client, "thread-native-child"); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(client); + + await startCodexConversationThread({ + sessionFile, + threadId: "thread-native-child", + workspaceDir: tempDir, + sandbox: "read-only", + }); + + expect(requests.map(({ method }) => method)).toEqual(["thread/unsubscribe", "thread/resume"]); + expect(requests[0]?.params).toEqual({ threadId: "thread-native-child" }); + expect(requests[1]?.params).toMatchObject({ + threadId: "thread-native-child", + sandbox: "read-only", + config: { apps: { _default: { enabled: false } }, "features.apps": false }, + }); + await expect(consumeCodexAppServerLiveThread(client, "thread-native-child")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + }); + + it("never resumes or unsubscribes an actively claimed native child for a conversation", async () => { + const sessionFile = path.join(tempDir, "active-child-session.jsonl"); + const harness = createClientHarness(); + const request = vi + .spyOn(harness.client, "request") + .mockResolvedValue(conversationThreadStartResult("thread-active-child") as never); + ensureCodexAppServerClientRuntime(harness.client, { agentDir: tempDir }); + const parent = codexNativeSubagentMonitorRuntime.register({ + client: harness.client, + parentThreadId: "thread-parent", + }); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(harness.client); + + try { + harness.send({ + method: "thread/started", + params: { + thread: { + id: "thread-active-child", + parentThreadId: "thread-parent", + source: { + subAgent: { + thread_spawn: { + parent_thread_id: "thread-parent", + depth: 1, + agent_path: "thread-active-child", + }, + }, + }, + }, + }, + }); + await vi.waitFor(() => + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-active-child")).toBe(true), + ); + + await expect( + startCodexConversationThread({ + sessionFile, + threadId: "thread-active-child", + workspaceDir: tempDir, + }), + ).rejects.toThrow("active run"); + + expect(request).not.toHaveBeenCalled(); + expect(isCodexAppServerLiveThreadClaimed(harness.client, "thread-active-child")).toBe(true); + } finally { + parent.unregister(); + harness.client.close(); + } + }); + + it("keeps a retained native child owned when its pre-resume unsubscribe fails", async () => { + const sessionFile = path.join(tempDir, "failed-child-session.jsonl"); + const request = vi.fn(async (method: string) => { + if (method === "thread/unsubscribe") { + throw new Error("native child unsubscribe failed"); + } + throw new Error(`unexpected method: ${method}`); + }); + const client = { + getInstanceId: () => "client-failed-child", + request, + addNotificationHandler: vi.fn(() => () => undefined), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: tempDir }); + await retainCodexAppServerLiveThread(client, "thread-failed-child"); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(client); + + await expect( + startCodexConversationThread({ + sessionFile, + threadId: "thread-failed-child", + workspaceDir: tempDir, + }), + ).rejects.toThrow("native child unsubscribe failed"); + + expect(request.mock.calls.map(([method]) => method)).toEqual(["thread/unsubscribe"]); + await expect(consumeCodexAppServerLiveThread(client, "thread-failed-child")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + }); + + it.each([ + { path: "binding" as const, rejectOldRelease: false }, + { path: "binding" as const, rejectOldRelease: true }, + { path: "bound turn" as const, rejectOldRelease: false }, + { path: "bound turn" as const, rejectOldRelease: true }, + ])( + "transfers exact physical ownership during $path migration (old release fails: $rejectOldRelease)", + async ({ path: migrationPath, rejectOldRelease }) => { + const sessionFile = path.join(tempDir, "same-thread-client-migration.jsonl"); + const { previousClient, replacementClient, operations, ownerDuringRelease, readOwner } = + await createSameThreadClientMigrationFixture(sessionFile, { + rejectOldRelease, + owner: migrationPath === "binding" ? "session" : "conversation", + }); + + if (migrationPath === "binding") { + const binding = startCodexConversationThread({ + sessionFile, + threadId: "thread-migrated", + workspaceDir: tempDir, + }); + if (rejectOldRelease) { + await expect(binding).rejects.toThrow("previous physical client unsubscribe failed"); + } else { + await expect(binding).resolves.toMatchObject({ + kind: "codex-app-server-session", + source: { threadId: "thread-migrated" }, + }); + } + } else { + const { event, ctx } = boundConversationClaim(sessionFile); + const result = await handleCodexConversationInboundClaim(event, ctx, { timeoutMs: 500 }); + expect(result?.reply?.text).toContain( + rejectOldRelease ? "previous physical client unsubscribe failed" : "Migrated reply", + ); + } + + const expectedOperations = ["replacement:thread/resume", "previous:thread/unsubscribe"]; + if (rejectOldRelease) { + expectedOperations.push("replacement:thread/unsubscribe"); + } else if (migrationPath === "bound turn") { + expectedOperations.push("replacement:turn/start"); + } + expect(operations).toEqual(expectedOperations); + expect(ownerDuringRelease).toEqual(["client-before-migration"]); + await expect(readOwner()).resolves.toMatchObject({ + threadId: "thread-migrated", + clientId: rejectOldRelease ? "client-before-migration" : "client-after-migration", + }); + const survivingClient = rejectOldRelease ? previousClient : replacementClient; + const obsoleteClient = rejectOldRelease ? replacementClient : previousClient; + await expect( + consumeCodexAppServerLiveThread(survivingClient, "thread-migrated"), + ).resolves.toEqual(expect.objectContaining({ release: expect.any(Function) })); + await expect( + consumeCodexAppServerLiveThread(obsoleteClient, "thread-migrated"), + ).resolves.toBeUndefined(); + }, + ); + it("starts a new bind thread when no model override is provided", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const requests: Array<{ method: string; params: Record }> = []; @@ -805,6 +1459,173 @@ describe("codex conversation binding", () => { await expect(readTestConversationBinding(sessionFile)).resolves.toBeUndefined(); }); + it("retires an already-evicted conversation without disturbing its live client siblings", async () => { + const identity = { kind: "conversation" as const, bindingId: "binding-evicted" }; + const client = { + request: vi.fn(async () => ({})), + addCloseHandler: vi.fn(), + addNotificationHandler: vi.fn(), + addRequestHandler: vi.fn(), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: tempDir }); + const releaseEvicted = vi.fn(async (_threadId: string) => undefined); + await retainCodexAppServerLiveThread(client, "thread-evicted", releaseEvicted); + for (let index = 0; index < 64; index += 1) { + await retainCodexAppServerLiveThread( + client, + `thread-sibling-${index}`, + async () => undefined, + ); + } + expect(releaseEvicted).toHaveBeenCalledExactlyOnceWith("thread-evicted"); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReturnValue({ + client, + release: vi.fn(), + }); + await testCodexAppServerBindingStore.mutate(identity, { + kind: "set", + binding: { + threadId: "thread-evicted", + clientId: "client-with-siblings", + cwd: tempDir, + conversationStartId: "start-evicted", + }, + }); + + await handleCodexConversationBindingResolved({ + status: "denied", + decision: "deny", + request: { + data: { + kind: "codex-app-server-session", + version: 2, + bindingId: identity.bindingId, + workspaceDir: tempDir, + start: { id: "start-evicted" }, + }, + conversation: { + channel: "discord", + accountId: "default", + conversationId: "channel:evicted", + }, + }, + }); + + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toBeUndefined(); + await expect(consumeCodexAppServerLiveThread(client, "thread-sibling-0")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + }); + + it("unsubscribes an untracked incognito conversation when its binding is denied", async () => { + const identity = { kind: "conversation" as const, bindingId: "binding-incognito" }; + const request = vi.fn(async () => ({})); + const release = vi.fn(); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReturnValue({ + client: { request }, + release, + }); + await testCodexAppServerBindingStore.mutate(identity, { + kind: "set", + binding: { + threadId: "thread-incognito", + clientId: "client-incognito", + cwd: tempDir, + conversationStartId: "start-incognito", + }, + }); + + await handleCodexConversationBindingResolved({ + status: "denied", + decision: "deny", + request: { + data: { + kind: "codex-app-server-session", + version: 2, + bindingId: identity.bindingId, + workspaceDir: tempDir, + source: { + agentId: "main", + sessionId: "session-incognito", + threadId: "thread-source", + sessionKey: "agent:main:dashboard:incognito-native-bind", + }, + start: { id: "start-incognito" }, + }, + conversation: { + channel: "discord", + accountId: "default", + conversationId: "channel:incognito", + }, + }, + }); + + expect(request).toHaveBeenCalledWith( + "thread/unsubscribe", + { threadId: "thread-incognito" }, + expect.objectContaining({ timeoutMs: expect.any(Number) }), + ); + expect(release).toHaveBeenCalledOnce(); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toBeUndefined(); + }); + + it("keeps a denied conversation binding when native unsubscribe fails", async () => { + const identity = { kind: "conversation" as const, bindingId: "binding-failed-denial" }; + const request = vi.fn(async () => { + throw new Error("native unsubscribe failed"); + }); + const close = vi.fn(); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReturnValue({ + client: { request, close }, + release: vi.fn(), + }); + await testCodexAppServerBindingStore.mutate(identity, { + kind: "set", + binding: { + threadId: "thread-failed-denial", + clientId: "client-failed-denial", + cwd: tempDir, + conversationStartId: "start-failed-denial", + }, + }); + + await expect( + handleCodexConversationBindingResolved({ + status: "denied", + decision: "deny", + request: { + data: { + kind: "codex-app-server-session", + version: 2, + bindingId: identity.bindingId, + workspaceDir: tempDir, + source: { + agentId: "main", + sessionId: "session-incognito", + threadId: "thread-source", + sessionKey: "agent:main:dashboard:incognito-native-bind", + }, + start: { id: "start-failed-denial" }, + }, + conversation: { + channel: "discord", + accountId: "default", + conversationId: "channel:incognito", + }, + }, + }), + ).rejects.toThrow("subscription could not be released"); + + expect(request).toHaveBeenCalledWith( + "thread/unsubscribe", + { threadId: "thread-failed-denial" }, + expect.objectContaining({ timeoutMs: expect.any(Number) }), + ); + await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ + threadId: "thread-failed-denial", + }); + }); + it("preserves the live conversation generation when a replacement bind is denied", async () => { const identity = { kind: "conversation" as const, bindingId: "binding-data-1" }; await testCodexAppServerBindingStore.mutate(identity, { @@ -841,6 +1662,32 @@ describe("codex conversation binding", () => { }); }); + it("rejects attaching a conversation to another session's owned thread", async () => { + const otherIdentity = { + kind: "session" as const, + agentId: "main", + sessionId: "session-other", + }; + await testCodexAppServerBindingStore.mutate(otherIdentity, { + kind: "set", + binding: { threadId: "thread-owned", cwd: tempDir }, + }); + const request = vi.fn(async () => conversationThreadStartResult("thread-owned")); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue({ request }); + + await expect( + startCodexConversationThread({ + sessionFile: path.join(tempDir, "session.jsonl"), + workspaceDir: tempDir, + threadId: "thread-owned", + }), + ).rejects.toThrow("owned by another OpenClaw session"); + expect(request).not.toHaveBeenCalled(); + await expect(testCodexAppServerBindingStore.read(otherIdentity)).resolves.toMatchObject({ + threadId: "thread-owned", + }); + }); + it("consumes inbound bound messages when command authorization is absent", async () => { const result = await handleCodexConversationInboundClaim( { @@ -1569,13 +2416,220 @@ describe("codex conversation binding", () => { expect(result).toEqual({ handled: true, reply: { text: "rebound" } }); expect(requests.map((request) => request.method)).toEqual(["thread/resume", "turn/start"]); expect(requests[0]?.params.threadId).toBe("thread-target"); + expect(requests[1]?.params.cwd).toBe("/new-repo"); await expect(testCodexAppServerBindingStore.read(identity)).resolves.toMatchObject({ threadId: "thread-target", - cwd: tempDir, + cwd: "/new-repo", conversationStartId: "start-new", }); }); + it("moves bounded visible session history into an isolated owned conversation thread", async () => { + const source = { + agentId: "main", + sessionId: "source-session", + sessionKey: "agent:main:source-session", + threadId: "thread-source", + }; + const storePath = path.join(tempDir, "source.sqlite"); + await upsertSessionEntry({ + ...source, + storePath, + entry: { sessionId: source.sessionId, updatedAt: Date.now() }, + }); + await appendSessionTranscriptMessageByIdentity({ + ...source, + storePath, + message: { role: "user", content: "Earlier question", timestamp: 1 }, + }); + await appendSessionTranscriptMessageByIdentity({ + ...source, + storePath, + message: { role: "assistant", content: "Earlier answer", timestamp: 2 }, + }); + const sourceIdentity = { + kind: "session" as const, + agentId: source.agentId, + sessionId: source.sessionId, + sessionKey: source.sessionKey, + }; + await testCodexAppServerBindingStore.mutate(sourceIdentity, { + kind: "set", + binding: { + threadId: source.threadId, + clientId: "source-client", + cwd: tempDir, + dynamicToolsFingerprint: "harness-only-tools", + }, + }); + const requests: Array<{ method: string; params: Record }> = []; + const notificationHandlers = new Set<(notification: unknown) => void>(); + const releaseSource = vi.fn(async () => undefined); + const client = { + getInstanceId: () => "source-client", + request: vi.fn(async (method: string, params: Record) => { + requests.push({ method, params }); + if (method === "thread/start") { + return conversationThreadStartResult("thread-bound"); + } + if (method === "thread/inject_items") { + return {}; + } + if (method === "turn/start") { + queueMicrotask(() => { + for (const handler of notificationHandlers) { + handler({ + method: "turn/completed", + params: { + threadId: "thread-bound", + turn: { + id: "turn-bound", + status: "completed", + items: [{ type: "agentMessage", id: "answer", text: "Bound reply" }], + }, + }, + }); + } + }); + return { turn: { id: "turn-bound" } }; + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn((handler: (notification: unknown) => void) => { + notificationHandlers.add(handler); + return () => notificationHandlers.delete(handler); + }), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: tempDir }); + await retainCodexAppServerLiveThread(client, source.threadId, releaseSource); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(client); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReturnValue({ + client, + release: vi.fn(), + }); + const { event, ctx } = boundConversationClaim(path.join(tempDir, "session.jsonl")); + ctx.pluginBinding.data = { + kind: "codex-app-server-session" as const, + version: 2 as const, + bindingId: "binding-source-transfer", + workspaceDir: tempDir, + source, + start: { id: "start-source-transfer" }, + }; + + await expect( + handleCodexConversationInboundClaim(event, ctx, { + config: { session: { store: storePath } }, + timeoutMs: 500, + }), + ).resolves.toEqual({ handled: true, reply: { text: "Bound reply" } }); + + expect(requests.map(({ method }) => method)).toEqual([ + "thread/start", + "thread/inject_items", + "turn/start", + ]); + expect(requests[0]?.params).toMatchObject({ + developerInstructions: expect.stringContaining("bound to an OpenClaw conversation"), + config: { apps: { _default: { enabled: false } }, "features.apps": false }, + }); + expect(requests[0]?.params).not.toHaveProperty("dynamicTools"); + expect(requests[1]?.params.items).toMatchObject([ + { role: "user", content: [{ text: "Earlier question" }] }, + { role: "assistant", content: [{ text: "Earlier answer" }] }, + ]); + expect(releaseSource).toHaveBeenCalledExactlyOnceWith("thread-source"); + await expect(testCodexAppServerBindingStore.read(sourceIdentity)).resolves.toBeUndefined(); + await expect(consumeCodexAppServerLiveThread(client, "thread-bound")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + }); + + it.each([ + { label: "a registered source run after its client was retired", exposeClient: false }, + { label: "a live client whose source subscription is claimed", exposeClient: true }, + ])("does not transfer $label into a bound conversation", async ({ exposeClient }) => { + const source = { + agentId: "main", + sessionId: "active-source-session", + sessionKey: "agent:main:active-source-session", + threadId: "thread-active-source", + }; + const sourceIdentity = { + kind: "session" as const, + agentId: source.agentId, + sessionId: source.sessionId, + sessionKey: source.sessionKey, + }; + await testCodexAppServerBindingStore.mutate(sourceIdentity, { + kind: "set", + binding: { threadId: source.threadId, clientId: "active-client", cwd: tempDir }, + }); + const request = vi.fn(async (method: string) => { + if (method === "thread/start") { + return conversationThreadStartResult("thread-active-target"); + } + throw new Error(`unexpected method: ${method}`); + }); + const client = { + getInstanceId: () => "active-client", + request, + addNotificationHandler: vi.fn(() => () => undefined), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: tempDir }); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(client); + if (exposeClient) { + await retainCodexAppServerLiveThread(client, source.threadId, async () => undefined); + await consumeCodexAppServerLiveThread(client, source.threadId); + sharedClientMocks.retainSharedCodexAppServerClientByInstanceId.mockReturnValue({ + client, + release: vi.fn(), + }); + } + const activeRun = { + queueMessage: async () => undefined, + isStreaming: () => true, + isCompacting: () => false, + abort: vi.fn(), + }; + if (!exposeClient) { + setActiveEmbeddedRun(source.sessionId, activeRun, source.sessionKey); + } + const { event, ctx } = boundConversationClaim(path.join(tempDir, "active-source.jsonl")); + ctx.pluginBinding.data = { + kind: "codex-app-server-session" as const, + version: 2 as const, + bindingId: "binding-active-source", + workspaceDir: tempDir, + source, + start: { id: "start-active-source" }, + }; + + try { + const result = await handleCodexConversationInboundClaim(event, ctx, { timeoutMs: 500 }); + + expect(result?.reply?.text).toContain("active run"); + expect(request.mock.calls.map(([method]) => method)).toEqual(["thread/start"]); + await expect(testCodexAppServerBindingStore.read(sourceIdentity)).resolves.toMatchObject({ + threadId: source.threadId, + }); + await expect( + testCodexAppServerBindingStore.read({ + kind: "conversation", + bindingId: "binding-active-source", + }), + ).resolves.not.toHaveProperty("conversationSourceTransferComplete", true); + } finally { + if (!exposeClient) { + clearActiveEmbeddedRun(source.sessionId, activeRun, source.sessionKey); + } + } + }); + it("recreates a missing bound thread with the stored binding agent runtime policy", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); await writeTestConversationBinding(sessionFile, { @@ -2896,6 +3950,66 @@ describe("codex conversation binding", () => { expect(bindingAfterRefresh?.conversationSourceTransferComplete).toBe(true); }); + it("restores the old owner and rolls back a network replacement when unsubscribe fails", async () => { + const sessionFile = path.join(tempDir, "network-rotation-release-failure.jsonl"); + await writeTestConversationBinding(sessionFile, { + threadId: "thread-old", + clientId: "client-network-rotation", + cwd: tempDir, + networkProxyProfileName: "openclaw-network-stale", + networkProxyConfigFingerprint: "stale-proxy-config", + conversationStartId: "start-1", + }); + const requests: Array<{ method: string; params: Record }> = []; + const client = { + getInstanceId: () => "client-network-rotation", + request: vi.fn(async (method: string, params: Record) => { + requests.push({ method, params }); + if (method === "thread/start") { + return conversationThreadStartResult("thread-new"); + } + if (method === "thread/unsubscribe" && params.threadId === "thread-old") { + throw new Error("old thread unsubscribe failed"); + } + if (method === "thread/unsubscribe" && params.threadId === "thread-new") { + return {}; + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn(() => () => undefined), + addRequestHandler: vi.fn(() => () => undefined), + addCloseHandler: vi.fn(() => () => undefined), + } as unknown as CodexAppServerClient; + ensureCodexAppServerClientRuntime(client, { agentDir: tempDir }); + await expect( + retainCodexAppServerLiveThread(client, "thread-old", async (threadId) => { + await client.request("thread/unsubscribe", { threadId }); + }), + ).resolves.toBe(true); + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue(client); + const { event, ctx } = boundConversationClaim(sessionFile); + + const result = await handleCodexConversationInboundClaim(event, ctx, { + pluginConfig: NETWORK_PROXY_PLUGIN_CONFIG, + }); + + expect(result?.reply?.text).toContain("old thread unsubscribe failed"); + expect(requests).toEqual([ + { method: "thread/start", params: expect.any(Object) }, + { method: "thread/unsubscribe", params: { threadId: "thread-old" } }, + { method: "thread/unsubscribe", params: { threadId: "thread-new" } }, + ]); + await expect(readTestConversationBinding(sessionFile)).resolves.toMatchObject({ + clientId: "client-network-rotation", + threadId: "thread-old", + }); + expect(isCodexAppServerLiveThreadClaimed(client, "thread-old")).toBe(false); + await expect(consumeCodexAppServerLiveThread(client, "thread-old")).resolves.toEqual( + expect.objectContaining({ release: expect.any(Function) }), + ); + await expect(consumeCodexAppServerLiveThread(client, "thread-new")).resolves.toBeUndefined(); + }); + it("blocks Guardian-mode bound turns with stale no-approval policy on custom model providers", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); await writeTestConversationBinding(sessionFile, { diff --git a/extensions/codex/src/conversation-binding.ts b/extensions/codex/src/conversation-binding.ts index 09e9b18db52e..7373aab59748 100644 --- a/extensions/codex/src/conversation-binding.ts +++ b/extensions/codex/src/conversation-binding.ts @@ -1,9 +1,12 @@ // Codex plugin module implements conversation binding behavior. import { + embeddedAgentLog, formatErrorMessage, + resolveActiveEmbeddedRunSessionId, resolveSandboxContext, } from "openclaw/plugin-sdk/agent-harness-runtime"; import { resolveSessionAgentIds } from "openclaw/plugin-sdk/agent-runtime"; +import { getSessionBindingService } from "openclaw/plugin-sdk/conversation-binding-runtime"; import { loadExecApprovals } from "openclaw/plugin-sdk/exec-approvals-runtime"; import { KeyedAsyncQueue } from "openclaw/plugin-sdk/keyed-async-queue"; import type { @@ -12,7 +15,12 @@ import type { PluginHookInboundClaimEvent, } from "openclaw/plugin-sdk/plugin-entry"; import type { ReplyPayload } from "openclaw/plugin-sdk/reply-payload"; -import { getSessionEntry, resolveStorePath } from "openclaw/plugin-sdk/session-store-runtime"; +import { + getSessionEntry, + resolveStorePath, + resolveTranscriptSessionKeyBySessionId, +} from "openclaw/plugin-sdk/session-store-runtime"; +import { readVisibleSessionTranscriptMessageEntries } from "openclaw/plugin-sdk/session-transcript-runtime"; import { resolveCodexAppServerForModelProvider } from "./app-server/app-server-policy.js"; import { CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, @@ -23,6 +31,13 @@ import { } from "./app-server/attempt-client-cleanup.js"; import { resolveCodexAppServerAuthProfileIdForAgent } from "./app-server/auth-bridge.js"; import { CODEX_CONTROL_METHODS } from "./app-server/capabilities.js"; +import { + consumeCodexAppServerLiveThread, + isCodexAppServerClientRuntimeLive, + isCodexAppServerLiveThreadClaimed, + releaseCodexAppServerLiveThread, + type CodexAppServerLiveThreadOwnership, +} from "./app-server/client-runtime.js"; import { isCodexAppServerIndeterminateRequestCancellationError, type CodexAppServerClient, @@ -36,6 +51,10 @@ import { type CodexAppServerSandboxMode, type OpenClawExecPolicyForCodexAppServer, } from "./app-server/config.js"; +import { + buildDisabledAppsConfigPatch, + mergeCodexThreadConfigs, +} from "./app-server/plugin-thread-config.js"; import { assertCodexThreadStartResponse } from "./app-server/protocol-validators.js"; import type { CodexServiceTier, @@ -59,6 +78,7 @@ import { } from "./app-server/session-binding.js"; import { getLeasedSharedCodexAppServerClient, + retainSharedCodexAppServerClientByInstanceId, releaseCodexAppServerClientLease, withLeasedCodexAppServerClientStartSelectionRetry, type CodexAppServerClientLease, @@ -68,7 +88,17 @@ import { CODEX_NATIVE_PERSONALITY_NONE, resolveCodexAppServerRequestModelSelection, } from "./app-server/thread-lifecycle.js"; +import { + isSameCodexAppServerThreadOwner, + releaseCodexAppServerBindingSubscription, + retainCodexAppServerBindingSubscription, + retireCodexConversationThreadBinding, + rollbackCodexAppServerBindingSubscription, + withCodexConversationThreadActivity, + withExclusiveCodexAppServerThread, +} from "./app-server/thread-ownership.js"; import { resumeCodexAppServerThread } from "./app-server/thread-resume.js"; +import { projectBoundedCodexVisibleSessionHistory } from "./app-server/transcript-history-projection.js"; import { getCodexAppServerTurnRouter, type CodexThreadRouteReservation, @@ -212,6 +242,7 @@ async function startCodexConversationThread( authProfileId: params.authProfileId ?? existingBinding?.authProfileId, ...agentLookup, }); + const incognito = isIncognitoSessionKey(params.sessionKey); if (params.threadId?.trim()) { await attachExistingThread({ pluginConfig: params.pluginConfig, @@ -228,6 +259,7 @@ async function startCodexConversationThread( serviceTier: params.serviceTier, config: params.config, sessionKey: params.sessionKey, + incognito, agentId: params.agentId, }); } else { @@ -245,6 +277,7 @@ async function startCodexConversationThread( serviceTier: params.serviceTier, config: params.config, sessionKey: params.sessionKey, + incognito, agentId: params.agentId, }); } @@ -270,8 +303,9 @@ async function handleCodexConversationInboundClaim( ctx: PluginHookInboundClaimContext, options: CodexConversationRunOptions, ): Promise<{ handled: boolean; reply?: ReplyPayload } | undefined> { - const data = readCodexConversationBindingData(ctx.pluginBinding); - if (!data) { + const publicBinding = ctx.pluginBinding; + const data = readCodexConversationBindingData(publicBinding); + if (!data || !publicBinding) { return undefined; } if (event.commandAuthorized !== true) { @@ -332,18 +366,55 @@ async function handleCodexConversationInboundClaim( } } try { - const result = await enqueueBoundTurn(data.bindingId, () => - runBoundTurnWithMissingThreadRecovery({ + const identity = conversationBindingIdentity(data); + const sessionKey = event.sessionKey ?? ctx.sessionKey; + // Native ephemeral ownership follows the persisted source, not the + // destination channel. Shipped v1 bindings have no source to consult. + const incognito = isIncognitoSessionKey( + data.source?.sessionKey ?? (data.legacyBinding ? sessionKey : undefined), + ); + // Start the snapshot before enqueueing, but do not await: yielding here + // would let detach overtake an already-arrived bound message. + const queuedOwner = options.bindingStore.read(identity); + const result = await withCodexConversationThreadActivity(data.bindingId, async () => { + const currentPublicBinding = getSessionBindingService().resolveByConversation({ + channel: publicBinding.channel, + accountId: publicBinding.accountId, + conversationId: publicBinding.conversationId, + ...(publicBinding.parentConversationId + ? { parentConversationId: publicBinding.parentConversationId } + : {}), + }); + const expected = await queuedOwner; + const current = await options.bindingStore.read(identity); + if ( + currentPublicBinding?.bindingId !== publicBinding.bindingId || + (expected && + (!current || + current.threadId !== expected.threadId || + current.conversationStartId !== expected.conversationStartId)) || + (!expected && current && data.start?.id && current.conversationStartId !== data.start.id) + ) { + // Public hooks capture binding data before entering this owner lane; + // a later detach must never let that stale message recreate its thread. + return { + reply: { + text: "This Codex conversation was detached or changed before its message could run.", + }, + }; + } + return await runBoundTurnWithMissingThreadRecovery({ bindingStore: options.bindingStore, data, prompt, event, config: options.config, - sessionKey: event.sessionKey ?? ctx.sessionKey, + sessionKey, + incognito, pluginConfig: options.pluginConfig, timeoutMs: options.timeoutMs, - }), - ); + }); + }); return { handled: true, reply: result.reply }; } catch (error) { return { @@ -369,8 +440,16 @@ async function handleCodexConversationBindingResolved( const identity = conversationBindingIdentity(data); const binding = await options.bindingStore.read(identity); assertCodexBindingMayBeReplaced(binding, "clearing a denied conversation binding"); - if (!data.start?.id || binding?.conversationStartId === data.start.id) { - await options.bindingStore.mutate(identity, { kind: "clear" }); + if (binding && (!data.start?.id || binding.conversationStartId === data.start.id)) { + await withCodexConversationThreadActivity(identity.bindingId, () => + retireCodexConversationThreadBinding({ + bindingStore: options.bindingStore, + identity, + expectedThreadId: binding.threadId, + ...(data.start?.id ? { expectedStartId: data.start.id } : {}), + ...(isIncognitoSessionKey(data.source?.sessionKey) ? { allowUntracked: true } : {}), + }), + ); } } @@ -389,6 +468,7 @@ type CodexThreadBindingParams = { config?: CodexAppServerAuthProfileLookup["config"]; agentId?: string; sessionKey?: string; + incognito: boolean; }; type ConversationAppServerRuntime = Awaited>; @@ -505,12 +585,19 @@ function codexConversationSandboxOrPermissions( config?: JsonObject; } { const networkProxy = runtime.networkProxy; + // Bound conversations have no native app approval/tool bridge. Disable + // globally configured Codex apps even when a network profile adds config. + // Per-app user config overrides apps._default, so the feature kill switch + // is the only authoritative boundary for this handlerless runtime. + const disabledApps = mergeCodexThreadConfigs(buildDisabledAppsConfigPatch(), { + "features.apps": false, + })!; if (networkProxy) { return { - config: networkProxy.configPatch, + config: mergeCodexThreadConfigs(networkProxy.configPatch, disabledApps), }; } - return { sandbox }; + return { sandbox, config: disabledApps }; } async function requestNewConversationBindingThread( @@ -531,7 +618,7 @@ async function requestNewConversationBindingThread( ...buildThreadRequestRuntimeOptions(params, resolved), developerInstructions: CODEX_CONVERSATION_THREAD_DEVELOPER_INSTRUCTIONS, experimentalRawEvents: true, - ...(isIncognitoSessionKey(params.sessionKey) ? { ephemeral: true } : {}), + ...(params.incognito ? { ephemeral: true } : {}), }, requestOptions, ), @@ -552,32 +639,62 @@ async function writeThreadBindingFromResponse( typeof resolved.runtime.approvalPolicy === "string" ? resolved.runtime.approvalPolicy : undefined; - const committed = await params.bindingStore.mutate(params.identity, { - kind: "set", - binding: { - threadId: response.thread.id, - clientId: resolved.client.getInstanceId(), - cwd: response.thread.cwd ?? params.workspaceDir, - authProfileId: params.authProfileId, - model: response.model ?? resolved.model ?? params.model, - modelProvider: normalizeCodexAppServerBindingModelProvider({ - authProfileId: params.authProfileId, - modelProvider: response.modelProvider ?? resolved.modelProvider ?? params.modelProvider, - ...resolved.agentLookup, - }), - approvalPolicy: resolved.execPolicy?.touched - ? runtimeApprovalPolicy - : (params.approvalPolicy ?? runtimeApprovalPolicy), - sandbox: resolved.execPolicy?.touched - ? resolved.runtime.sandbox - : (params.sandbox ?? resolved.runtime.sandbox), - serviceTier: params.serviceTier ?? resolved.runtime.serviceTier ?? undefined, - networkProxyProfileName: resolved.runtime.networkProxy?.profileName, - networkProxyConfigFingerprint: resolved.runtime.networkProxy?.configFingerprint, - }, + const trackSubscription = !params.incognito && isCodexAppServerClientRuntimeLive(resolved.client); + const sameOwner = isSameCodexAppServerThreadOwner(current, { + threadId: response.thread.id, + clientId: resolved.client.getInstanceId(), }); - if (!committed) { - throw new Error("Codex conversation binding changed while storing its thread."); + let retained = false; + try { + if (trackSubscription) { + retained = await retainCodexAppServerBindingSubscription(resolved.client, response.thread.id); + if (!retained) { + throw new Error("Codex conversation thread lost its native subscription owner."); + } + } + if (current && !sameOwner) { + // Keep the old identity visible until its sole native subscription is + // released; a concurrent owner must not adopt it between clear and cleanup. + await releaseCodexAppServerBindingSubscription(current); + } + const committed = await params.bindingStore.mutate(params.identity, { + kind: "set", + binding: { + threadId: response.thread.id, + clientId: resolved.client.getInstanceId(), + cwd: params.workspaceDir, + authProfileId: params.authProfileId, + model: response.model ?? resolved.model ?? params.model, + modelProvider: normalizeCodexAppServerBindingModelProvider({ + authProfileId: params.authProfileId, + modelProvider: response.modelProvider ?? resolved.modelProvider ?? params.modelProvider, + ...resolved.agentLookup, + }), + approvalPolicy: resolved.execPolicy?.touched + ? runtimeApprovalPolicy + : (params.approvalPolicy ?? runtimeApprovalPolicy), + sandbox: resolved.execPolicy?.touched + ? resolved.runtime.sandbox + : (params.sandbox ?? resolved.runtime.sandbox), + serviceTier: params.serviceTier ?? resolved.runtime.serviceTier ?? undefined, + networkProxyProfileName: resolved.runtime.networkProxy?.profileName, + networkProxyConfigFingerprint: resolved.runtime.networkProxy?.configFingerprint, + }, + }); + if (!committed) { + throw new Error("Codex conversation binding changed while storing its thread."); + } + } catch (error) { + // A newly started/resumed Codex thread is already subscribed before its + // response arrives; failed ownership commits must release that exact thread. + if (trackSubscription && !sameOwner) { + await rollbackCodexAppServerBindingSubscription( + resolved.client, + response.thread.id, + retained, + ); + } + throw error; } } @@ -586,38 +703,64 @@ async function attachExistingThread( threadId: string; }, ): Promise { - const current = await params.bindingStore.read(params.identity); - assertCodexBindingMayBeReplaced(current, "attaching a conversation-bound Codex thread"); - const resolved = await resolveThreadBindingRuntime(params); - try { - // Codex applies network-proxy permission profiles at thread/start. Resuming - // an arbitrary existing thread cannot prove that profile is active. - const response: CodexThreadResumeResponse | CodexThreadStartResponse = resolved.runtime - .networkProxy - ? await requestNewConversationBindingThread(params, resolved) - : await withLeasedCodexAppServerClientStartSelectionRetry({ - lease: resolved.clientLease, - options: resolved.clientOptions, - run: async (client, requestOptions) => - await client.request( - CODEX_CONTROL_METHODS.resumeThread, - { - threadId: params.threadId, - ...(resolved.model ? { model: resolved.model } : {}), - ...(resolved.modelProvider ? { modelProvider: resolved.modelProvider } : {}), - personality: CODEX_NATIVE_PERSONALITY_NONE, - ...buildThreadRequestRuntimeOptions(params, resolved), + await withExclusiveCodexAppServerThread({ + bindingStore: params.bindingStore, + identity: params.identity, + threadId: params.threadId, + run: async () => { + const current = await params.bindingStore.read(params.identity); + assertCodexBindingMayBeReplaced(current, "attaching a conversation-bound Codex thread"); + const resolved = await resolveThreadBindingRuntime(params); + try { + // Codex applies network-proxy permission profiles at thread/start. Resuming + // an arbitrary existing thread cannot prove that profile is active. + const response: CodexThreadResumeResponse | CodexThreadStartResponse = resolved.runtime + .networkProxy + ? await requestNewConversationBindingThread(params, resolved) + : await withLeasedCodexAppServerClientStartSelectionRetry({ + lease: resolved.clientLease, + options: resolved.clientOptions, + run: async (client, requestOptions) => { + if (isCodexAppServerLiveThreadClaimed(client, params.threadId)) { + throw new Error( + `Codex thread ${params.threadId} has an active run; stop it before binding its conversation.`, + ); + } + // Codex ignores resume config while any connection is still + // subscribed; completed native children otherwise inherit app permissions. + await releaseCodexAppServerLiveThread(client, params.threadId); + if (isCodexAppServerLiveThreadClaimed(client, params.threadId)) { + throw new Error( + `Codex thread ${params.threadId} has an active run; stop it before binding its conversation.`, + ); + } + return await client.request( + CODEX_CONTROL_METHODS.resumeThread, + { + threadId: params.threadId, + ...(resolved.model ? { model: resolved.model } : {}), + ...(resolved.modelProvider ? { modelProvider: resolved.modelProvider } : {}), + personality: CODEX_NATIVE_PERSONALITY_NONE, + ...buildThreadRequestRuntimeOptions(params, resolved), + }, + requestOptions, + ); }, - requestOptions, - ), - onClientChange: (client) => { - resolved.client = client; - }, - }); - await writeThreadBindingFromResponse(params, resolved, response); - } finally { - releaseCodexAppServerClientLease(resolved.clientLease); - } + onClientChange: (client) => { + resolved.client = client; + }, + }); + if (!resolved.runtime.networkProxy && response.thread.id !== params.threadId) { + throw new Error( + `Codex conversation resume returned ${response.thread.id} for ${params.threadId}.`, + ); + } + await writeThreadBindingFromResponse(params, resolved, response); + } finally { + releaseCodexAppServerClientLease(resolved.clientLease); + } + }, + }); } async function createThread(params: CodexThreadBindingParams): Promise { @@ -640,6 +783,7 @@ async function runBoundTurn(params: { pluginConfig?: unknown; config?: CodexConversationConfig; sessionKey?: string; + incognito: boolean; timeoutMs?: number; }): Promise { const agentLookup = buildAgentLookup({ agentDir: params.data.agentDir, config: params.config }); @@ -726,7 +870,23 @@ async function runBoundTurn(params: { let activeTurnCleanup: () => void = () => undefined; let retiredUnsafeClient: CodexAppServerClient | undefined; let turnRoute: CodexThreadRouteReservation | undefined; + let liveThreadOwnership: + | { + client: CodexAppServerClient; + threadId: string; + ownership: CodexAppServerLiveThreadOwnership; + } + | undefined; + let ownsNativeSubscription = false; + let turnSucceeded = false; try { + if (!params.incognito && isCodexAppServerClientRuntimeLive(client)) { + const ownership = await consumeCodexAppServerLiveThread(client, threadId); + if (ownership) { + liveThreadOwnership = { client, threadId, ownership }; + ownsNativeSubscription = true; + } + } if (networkProxyBindingChanged) { const response = assertCodexThreadStartResponse( await withLeasedCodexAppServerClientStartSelectionRetry({ @@ -744,13 +904,11 @@ async function runBoundTurn(params: { personality: CODEX_NATIVE_PERSONALITY_NONE, approvalPolicy, approvalsReviewer: modelScopedRuntime.approvalsReviewer, - ...(modelScopedRuntime.networkProxy - ? { config: modelScopedRuntime.networkProxy.configPatch } - : { sandbox }), + ...codexConversationSandboxOrPermissions(modelScopedRuntime, sandbox), ...(serviceTier ? { serviceTier } : {}), developerInstructions: CODEX_CONVERSATION_THREAD_DEVELOPER_INSTRUCTIONS, experimentalRawEvents: true, - ...(isIncognitoSessionKey(params.sessionKey) ? { ephemeral: true } : {}), + ...(params.incognito ? { ephemeral: true } : {}), }, requestOptions, ), @@ -760,6 +918,34 @@ async function runBoundTurn(params: { }), ); threadId = response.thread.id; + ownsNativeSubscription = true; + if ( + liveThreadOwnership && + (liveThreadOwnership.threadId !== threadId || liveThreadOwnership.client !== client) + ) { + const previousOwnership = liveThreadOwnership; + try { + await previousOwnership.ownership.release(previousOwnership.threadId); + } catch (error) { + // A failed unsubscribe leaves the old subscription alive. Restore + // its exact branded owner before rolling back the new native thread. + const restored = + isCodexAppServerClientRuntimeLive(previousOwnership.client) && + (await retainCodexAppServerBindingSubscription( + previousOwnership.client, + previousOwnership.threadId, + previousOwnership.ownership, + ).catch(() => false)); + if (!restored) { + await closeCodexStartupClientBestEffort(previousOwnership.client); + } + liveThreadOwnership = undefined; + throw error; + } + liveThreadOwnership = undefined; + } else if (binding.threadId !== threadId) { + await releaseCodexAppServerBindingSubscription(binding); + } const committed = await params.bindingStore.mutate(identity, { kind: "set", binding: { @@ -788,7 +974,10 @@ async function runBoundTurn(params: { throw new Error("Codex conversation binding changed while rotating its thread."); } useStickyNetworkProfile = modelScopedRuntime.networkProxy !== undefined; - } else if (binding.clientId !== client.getInstanceId()) { + } else if ( + binding.clientId !== client.getInstanceId() || + (isCodexAppServerClientRuntimeLive(client) && !params.incognito && !liveThreadOwnership) + ) { const response = await withLeasedCodexAppServerClientStartSelectionRetry({ lease: clientLease, options: clientOptions, @@ -820,6 +1009,17 @@ async function runBoundTurn(params: { }, }); threadId = response.thread.id; + ownsNativeSubscription = true; + if ( + !isSameCodexAppServerThreadOwner(binding, { + threadId, + clientId: client.getInstanceId(), + }) + ) { + // Keep the old physical owner authoritative until unsubscribe succeeds; + // failed migration then rolls back only the newly resumed connection. + await releaseCodexAppServerBindingSubscription(binding); + } const committed = await params.bindingStore.mutate(identity, { kind: "patch", threadId: binding.threadId, @@ -880,6 +1080,7 @@ async function runBoundTurn(params: { timeoutMs: params.timeoutMs ?? DEFAULT_BOUND_TURN_TIMEOUT_MS, }); const replyText = completion.replyText.trim(); + turnSucceeded = true; return { reply: { text: replyText || "Codex completed without a text reply.", @@ -903,7 +1104,7 @@ async function runBoundTurn(params: { await retireUnsafeCodexTurnClientBestEffort(client, "turn interrupt"); } } - if (isIncognitoSessionKey(params.sessionKey)) { + if (params.incognito) { const bindingReleased = await params.bindingStore.mutate(identity, { kind: "clear", threadId, @@ -922,7 +1123,56 @@ async function runBoundTurn(params: { } finally { activeTurnCleanup(); turnRoute?.release(); - releaseCodexAppServerClientLease(clientLease); + try { + if ( + ownsNativeSubscription && + retiredUnsafeClient !== client && + !params.incognito && + isCodexAppServerClientRuntimeLive(client) + ) { + // Ownership callbacks are branded to one physical client and native + // thread; an old generation must never clean up its replacement. + const currentLiveThreadOwnership = + liveThreadOwnership?.client === client && liveThreadOwnership.threadId === threadId + ? liveThreadOwnership.ownership + : undefined; + let retained = false; + if (turnSucceeded) { + retained = await params.bindingStore.withLease(identity, async () => { + const current = await params.bindingStore.read(identity); + if (current?.threadId !== threadId || current.clientId !== client.getInstanceId()) { + return false; + } + // Claim before turn/start and republish only its unchanged owner; + // TTL/LRU eviction must never detach an active conversation turn. + return await retainCodexAppServerBindingSubscription( + client, + threadId, + currentLiveThreadOwnership, + ); + }); + } + if (!retained) { + const released = currentLiveThreadOwnership + ? await currentLiveThreadOwnership.release(threadId).then(() => true) + : await unsubscribeCodexThreadBestEffort(client, { + threadId, + timeoutMs: CODEX_APP_SERVER_UNSUBSCRIBE_TIMEOUT_MS, + }); + if (!released) { + await closeCodexStartupClientBestEffort(client); + } + } + } + } catch (error) { + embeddedAgentLog.warn("codex conversation subscription cleanup failed", { + threadId, + reason: formatErrorMessage(error), + }); + await closeCodexStartupClientBestEffort(client); + } finally { + releaseCodexAppServerClientLease(clientLease); + } } } @@ -949,6 +1199,7 @@ async function runBoundTurnWithMissingThreadRecovery(params: { pluginConfig?: unknown; config?: CodexConversationConfig; sessionKey?: string; + incognito: boolean; timeoutMs?: number; }): Promise { await prepareConversationBinding(params); @@ -970,6 +1221,7 @@ async function prepareConversationBinding( pluginConfig?: unknown; config?: CodexConversationConfig; sessionKey?: string; + incognito: boolean; }, options: { forceNew?: boolean } = {}, ): Promise { @@ -1022,9 +1274,12 @@ async function prepareConversationBinding( serviceTier: inherited?.serviceTier, config: params.config, sessionKey: params.sessionKey, + incognito: params.incognito, agentId: params.data.agentId, }; - const threadId = requested?.threadId ?? (!current ? params.data.source?.threadId : undefined); + // Harness threads retain immutable tools, developer instructions, and app + // policy. Transfer bounded visible history into a fresh bound-only thread. + const threadId = requested?.threadId; if (threadId && !options.forceNew) { await attachExistingThread({ ...bindingParams, threadId }); } else { @@ -1038,6 +1293,27 @@ async function prepareConversationBinding( await params.bindingStore.withLease(sourceIdentity, async () => { const source = await params.bindingStore.read(sourceIdentity); if (source && source.threadId === params.data.source?.threadId) { + const sourceSessionKey = + sourceIdentity.sessionKey ?? + resolveTranscriptSessionKeyBySessionId({ + agentId: sourceIdentity.agentId, + sessionId: sourceIdentity.sessionId, + storePath: resolveStorePath(params.config?.session?.store, { + agentId: sourceIdentity.agentId, + }), + }); + if ( + sourceSessionKey && + resolveActiveEmbeddedRunSessionId(sourceSessionKey) === sourceIdentity.sessionId + ) { + throw new Error( + "Codex source session has an active run; stop it before binding this conversation.", + ); + } + if (source.threadId !== stored.threadId) { + await releaseCodexAppServerBindingSubscription(source); + await projectConversationSourceHistory(params.data.source, stored, params.config); + } await params.bindingStore.mutate(sourceIdentity, { kind: "clear", threadId: source.threadId, @@ -1059,6 +1335,48 @@ async function prepareConversationBinding( }); } +async function projectConversationSourceHistory( + source: { agentId: string; sessionId: string; sessionKey?: string; threadId: string }, + target: { threadId: string; clientId?: string }, + config?: CodexConversationConfig, +): Promise { + const storePath = resolveStorePath(config?.session?.store, { agentId: source.agentId }); + const sessionKey = + source.sessionKey ?? + resolveTranscriptSessionKeyBySessionId({ + agentId: source.agentId, + sessionId: source.sessionId, + storePath, + }); + if (!sessionKey) { + return; + } + // Local visible transcripts remain readable for ephemeral and paginated + // Codex threads, both of which reject native includeTurns history reads. + const entries = await readVisibleSessionTranscriptMessageEntries({ + agentId: source.agentId, + sessionId: source.sessionId, + sessionKey, + storePath, + }); + const history = projectBoundedCodexVisibleSessionHistory(entries); + if (history.length === 0) { + return; + } + const clientLease = retainSharedCodexAppServerClientByInstanceId(target.clientId); + if (!clientLease) { + throw new Error("Codex conversation source history lost its bound client owner."); + } + try { + await clientLease.client.request("thread/inject_items", { + threadId: target.threadId, + items: history, + }); + } finally { + clientLease.release(); + } +} + function resolveConversationExecPolicy(params: { config?: CodexConversationConfig; agentId?: string; @@ -1260,7 +1578,7 @@ function buildAgentLookup(params: { function conversationBindingIdentity( data: Pick, -): CodexAppServerBindingIdentity { +): Extract { return { kind: "conversation", bindingId: data.bindingId }; } diff --git a/extensions/codex/src/conversation-control.test.ts b/extensions/codex/src/conversation-control.test.ts index 6afc26abbd8e..52e75da85329 100644 --- a/extensions/codex/src/conversation-control.test.ts +++ b/extensions/codex/src/conversation-control.test.ts @@ -401,6 +401,48 @@ describe("codex conversation controls", () => { expect(sharedClientMocks.getSharedCodexAppServerClient).not.toHaveBeenCalled(); }); + it("drops an incompatible pinned auth profile when selecting another provider", async () => { + const sessionKey = "agent:main:model-provider-switch"; + const sessionId = "session-provider-switch"; + const identity = { kind: "session" as const, agentId: "main", sessionId, sessionKey }; + const storePath = resolveStorePath(undefined, { agentId: "main" }); + await upsertSessionEntry({ + agentId: "main", + storePath, + sessionKey, + entry: { + sessionId, + updatedAt: Date.now(), + authProfileOverride: "lmstudio:work", + authProfileOverrideSource: "user", + }, + }); + await testCodexAppServerBindingStore.mutate(identity, { + kind: "set", + binding: { + threadId: "thread-provider-switch", + cwd: tempDir, + model: "local-model", + modelProvider: "lmstudio", + }, + }); + + await expect( + setCodexConversationModelImpl({ + identity, + bindingStore: testCodexAppServerBindingStore, + model: "openai/gpt-5.5", + }), + ).resolves.toBe("Codex model set to gpt-5.5."); + + expect(getSessionEntry({ storePath, sessionKey })).toMatchObject({ + providerOverride: "openai", + modelOverride: "gpt-5.5", + liveModelSwitchPending: true, + }); + expect(getSessionEntry({ storePath, sessionKey })?.authProfileOverride).toBeUndefined(); + }); + it("escapes requested model names before chat display", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); await writeCodexAppServerBinding(sessionFile, { diff --git a/extensions/codex/src/conversation-control.ts b/extensions/codex/src/conversation-control.ts index 4f4e2afe192c..0a1fbd3a6f46 100644 --- a/extensions/codex/src/conversation-control.ts +++ b/extensions/codex/src/conversation-control.ts @@ -1,12 +1,10 @@ +import { resolveAgentDir } from "openclaw/plugin-sdk/agent-runtime"; // Codex plugin module implements conversation control behavior. import { - applyModelOverrideToSessionEntry, + applyModelOverrideWithAuthProfileCompatibility, ModelSelectionLockedError, } from "openclaw/plugin-sdk/model-session-runtime"; -import { - resolveStorePath, - updateSessionStoreEntry, -} from "openclaw/plugin-sdk/session-store-runtime"; +import { patchSessionEntry, resolveStorePath } from "openclaw/plugin-sdk/session-store-runtime"; import { resolveCodexBindingAppServerConnection } from "./app-server/binding-connection.js"; import type { CodexAppServerClient } from "./app-server/client.js"; import { @@ -230,18 +228,24 @@ export async function setCodexConversationModel(params: { } : undefined); if (session) { - const updated = await updateSessionStoreEntry({ + const updated = await patchSessionEntry({ + agentId: session.agentId, storePath: resolveStorePath(params.config?.session?.store, { agentId: session.agentId }), sessionKey: session.sessionKey, requireWriteSuccess: true, + // Model override helpers delete stale credentials and model metadata; + // replacing the snapshot is required because partial patches merge fields. + replaceEntry: true, update: (entry) => { if (entry.sessionId !== session.sessionId) { - return null; + throw new Error("Codex session changed while applying the model selection."); } - applyModelOverrideToSessionEntry({ + applyModelOverrideWithAuthProfileCompatibility({ + cfg: params.config ?? {}, + agentDir: params.agentDir ?? resolveAgentDir(params.config ?? {}, session.agentId), entry, + currentProvider: binding.modelProvider ?? "openai", selection: { provider: nextModelProvider ?? "openai", model: nextModel }, - preserveAuthProfileOverride: true, markLiveSwitchPending: true, }); return entry; diff --git a/scripts/test-live-codex-harness-docker.sh b/scripts/test-live-codex-harness-docker.sh index 3082096409d8..0fcadce498ec 100644 --- a/scripts/test-live-codex-harness-docker.sh +++ b/scripts/test-live-codex-harness-docker.sh @@ -283,6 +283,7 @@ echo "==> Docker run timeout: $CODEX_HARNESS_DOCKER_RUN_TIMEOUT" echo "==> Chat image probe: ${OPENCLAW_LIVE_CODEX_HARNESS_CHAT_IMAGE_PROBE:-0}" echo "==> Image probe: ${OPENCLAW_LIVE_CODEX_HARNESS_IMAGE_PROBE:-1}" echo "==> MCP probe: ${OPENCLAW_LIVE_CODEX_HARNESS_MCP_PROBE:-1}" +echo "==> Multi-session probe: ${OPENCLAW_LIVE_CODEX_HARNESS_MULTI_SESSION_PROBE:-0}" echo "==> Subagent probe: ${OPENCLAW_LIVE_CODEX_HARNESS_SUBAGENT_PROBE:-1}" echo "==> Subagent count: ${OPENCLAW_LIVE_CODEX_HARNESS_SUBAGENT_COUNT:-1}" echo "==> Subagent-only fast path: ${OPENCLAW_LIVE_CODEX_HARNESS_SUBAGENT_ONLY:-auto}" @@ -333,6 +334,7 @@ DOCKER_RUN_ARGS+=(--rm -t \ -e OPENCLAW_LIVE_CODEX_HARNESS_IMAGE_PROBE="${OPENCLAW_LIVE_CODEX_HARNESS_IMAGE_PROBE:-1}" \ -e OPENCLAW_LIVE_CODEX_HARNESS_LARGE_OUTPUT_BYTES="${OPENCLAW_LIVE_CODEX_HARNESS_LARGE_OUTPUT_BYTES:-300000}" \ -e OPENCLAW_LIVE_CODEX_HARNESS_MCP_PROBE="${OPENCLAW_LIVE_CODEX_HARNESS_MCP_PROBE:-1}" \ + -e OPENCLAW_LIVE_CODEX_HARNESS_MULTI_SESSION_PROBE="${OPENCLAW_LIVE_CODEX_HARNESS_MULTI_SESSION_PROBE:-0}" \ -e OPENCLAW_LIVE_CODEX_HARNESS_MODEL="${OPENCLAW_LIVE_CODEX_HARNESS_MODEL:-openai/gpt-5.6-luna}" \ -e OPENCLAW_LIVE_CODEX_HARNESS_TARGETS="${OPENCLAW_LIVE_CODEX_HARNESS_TARGETS:-}" \ -e OPENCLAW_LIVE_CODEX_HARNESS_THINKING="${OPENCLAW_LIVE_CODEX_HARNESS_THINKING:-low}" \ diff --git a/test/scripts/test-live-codex-harness-docker.test.ts b/test/scripts/test-live-codex-harness-docker.test.ts index 8cf7d26624bf..1b2b70025545 100644 --- a/test/scripts/test-live-codex-harness-docker.test.ts +++ b/test/scripts/test-live-codex-harness-docker.test.ts @@ -118,6 +118,9 @@ describe("scripts/test-live-codex-harness-docker.sh", () => { it("forwards bounded resume stress controls into Docker", () => { const script = fs.readFileSync(SCRIPT_PATH, "utf8"); + expect(script).toContain( + '-e OPENCLAW_LIVE_CODEX_HARNESS_MULTI_SESSION_PROBE="${OPENCLAW_LIVE_CODEX_HARNESS_MULTI_SESSION_PROBE:-0}"', + ); expect(script).toContain( '-e OPENCLAW_LIVE_CODEX_HARNESS_RESUME_STRESS="${OPENCLAW_LIVE_CODEX_HARNESS_RESUME_STRESS:-0}"', );