From bda02f4be84cae12085a91e012fc24e603e7320d Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Fri, 29 May 2026 22:28:13 +0530 Subject: [PATCH] fix(agents): scope cli binding clears --- src/agents/cli-runner.runtime.ts | 2 +- .../reply/agent-runner-cli-dispatch.ts | 4 +- .../reply/agent-runner-execution.test.ts | 50 +++++++++++++++++++ ...run.cron-model-override-forwarding.test.ts | 40 +++++++++++++++ src/cron/isolated-agent/run.test-harness.ts | 3 ++ src/cron/isolated-agent/run.ts | 5 +- 6 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/agents/cli-runner.runtime.ts b/src/agents/cli-runner.runtime.ts index 14143ee491b..8f2e1876fdc 100644 --- a/src/agents/cli-runner.runtime.ts +++ b/src/agents/cli-runner.runtime.ts @@ -1,2 +1,2 @@ export { runCliAgent } from "./cli-runner.js"; -export { getCliSessionId, setCliSessionId } from "./cli-session.js"; +export { clearCliSession, getCliSessionId, setCliSessionId } from "./cli-session.js"; diff --git a/src/auto-reply/reply/agent-runner-cli-dispatch.ts b/src/auto-reply/reply/agent-runner-cli-dispatch.ts index 98199802c4a..f9b96082acd 100644 --- a/src/auto-reply/reply/agent-runner-cli-dispatch.ts +++ b/src/auto-reply/reply/agent-runner-cli-dispatch.ts @@ -95,10 +95,11 @@ export function keepCliSessionBindingOnlyWhenReused(params: { const existingSessionId = normalizeOptionalString(params.existingSessionId); const agentMeta = params.result.meta.agentMeta; const returnedSessionId = normalizeOptionalString(agentMeta?.cliSessionBinding?.sessionId); + const shouldClearStoredSession = agentMeta?.clearCliSessionBinding === true; if (agentMeta === undefined || (existingSessionId && returnedSessionId === existingSessionId)) { return params.result; } - if (returnedSessionId) { + if (returnedSessionId || shouldClearStoredSession) { params.onDroppedReplacement?.(); } return { @@ -109,6 +110,7 @@ export function keepCliSessionBindingOnlyWhenReused(params: { ...agentMeta, sessionId: "", cliSessionBinding: undefined, + clearCliSessionBinding: undefined, }, }, }; diff --git a/src/auto-reply/reply/agent-runner-execution.test.ts b/src/auto-reply/reply/agent-runner-execution.test.ts index 0311a5ccb88..790545e4fb7 100644 --- a/src/auto-reply/reply/agent-runner-execution.test.ts +++ b/src/auto-reply/reply/agent-runner-execution.test.ts @@ -1950,6 +1950,7 @@ describe("runAgentTurnWithFallback", () => { sessionId: "transient-cli-session", authProfileId: "profile", }, + clearCliSessionBinding: true, }, }, }); @@ -1985,6 +1986,7 @@ describe("runAgentTurnWithFallback", () => { } expect(result.runResult.meta?.agentMeta?.sessionId).toBe(""); expect(result.runResult.meta?.agentMeta?.cliSessionBinding).toBeUndefined(); + expect(result.runResult.meta?.agentMeta?.clearCliSessionBinding).toBeUndefined(); expect(activeSessionStore.main.cliSessionBindings?.["codex-cli"]).toBeUndefined(); }); @@ -2036,6 +2038,54 @@ describe("runAgentTurnWithFallback", () => { }); }); + it("clears room-event CLI bindings when an unflushed replacement is dropped", async () => { + state.isCliProviderMock.mockReturnValue(true); + state.runWithModelFallbackMock.mockImplementationOnce(async (params: FallbackRunnerParams) => ({ + result: await params.run("codex-cli", "gpt-5.4"), + provider: "codex-cli", + model: "gpt-5.4", + attempts: [], + })); + state.runCliAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "handled" }], + meta: { + agentMeta: { + sessionId: "", + provider: "codex-cli", + model: "gpt-5.4", + clearCliSessionBinding: true, + }, + }, + }); + + const runAgentTurnWithFallback = await getRunAgentTurnWithFallback(); + const followupRun = createFollowupRun(); + followupRun.currentInboundEventKind = "room_event"; + followupRun.run.provider = "codex-cli"; + followupRun.run.model = "gpt-5.4"; + const sessionEntry = { + cliSessionBindings: { + "codex-cli": { sessionId: "existing-cli-session" }, + }, + } as unknown as SessionEntry; + const activeSessionStore = { main: sessionEntry }; + + const result = await runAgentTurnWithFallback({ + ...createMinimalRunAgentTurnParams({ followupRun }), + activeSessionStore, + getActiveSessionEntry: () => sessionEntry, + }); + + expect(result.kind).toBe("success"); + if (result.kind !== "success") { + throw new Error("expected success"); + } + expect(result.runResult.meta?.agentMeta?.sessionId).toBe(""); + expect(result.runResult.meta?.agentMeta?.cliSessionBinding).toBeUndefined(); + expect(result.runResult.meta?.agentMeta?.clearCliSessionBinding).toBeUndefined(); + expect(activeSessionStore.main.cliSessionBindings?.["codex-cli"]).toBeUndefined(); + }); + it("bridges CLI assistant agent events into onPartialReply for live preview (#76869)", async () => { state.isCliProviderMock.mockReturnValue(true); state.runWithModelFallbackMock.mockImplementationOnce(async (params: FallbackRunnerParams) => ({ diff --git a/src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts b/src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts index 517feea211f..384bc562ab1 100644 --- a/src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts +++ b/src/cron/isolated-agent/run.cron-model-override-forwarding.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { + clearCliSessionMock, clearFastTestEnv, getCliSessionIdMock, isCliProviderMock, @@ -284,6 +285,45 @@ describe("runCronIsolatedAgentTurn — cron model override forwarding (#58065)", ).toBe(true); }); + it("clears stale CLI bindings when cron CLI replacement is unflushed", async () => { + isCliProviderMock.mockReturnValue(true); + runWithModelFallbackMock.mockImplementation(async ({ provider, model, run }) => { + const result = await run(provider, model); + return { result, provider, model, attempts: [] }; + }); + const cronSession = makeCronSession({ + sessionEntry: makeCronSessionEntry({ + cliSessionBindings: { + "claude-cli": { sessionId: "stale-cli-session" }, + "codex-cli": { sessionId: "codex-session" }, + }, + }), + isNewSession: false, + }); + resolveCronSessionMock.mockReturnValue(cronSession); + runCliAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "summary done" }], + meta: { + agentMeta: { + provider: "claude-cli", + model: "claude-opus-4-6", + sessionId: "", + clearCliSessionBinding: true, + usage: { input: 10, output: 20 }, + }, + }, + }); + + const result = await runCronIsolatedAgentTurn( + makeParams({ + job: makeJob({ sessionTarget: "session:existing-cron-session" }), + }), + ); + + expect(result.status).toBe("ok"); + expect(clearCliSessionMock).toHaveBeenCalledWith(cronSession.sessionEntry, "claude-cli"); + }); + it("validates cron thinking with catalog reasoning metadata", async () => { resolveAllowedModelRefMock.mockImplementation(() => ({ ref: { provider: "ollama", model: "qwen3:0.6b" }, diff --git a/src/cron/isolated-agent/run.test-harness.ts b/src/cron/isolated-agent/run.test-harness.ts index 7d57c2bf4c8..a8155bdbe7a 100644 --- a/src/cron/isolated-agent/run.test-harness.ts +++ b/src/cron/isolated-agent/run.test-harness.ts @@ -56,6 +56,7 @@ export const runEmbeddedAgentMock = createMock(); export const runCliAgentMock = createMock(); export const lookupContextTokensMock = createMock(); export const getCliSessionIdMock = createMock(); +export const clearCliSessionMock = createMock(); export const updateSessionStoreMock = createMock(); export const resolveCronSessionMock = createMock(); export const logWarnMock = createMock(); @@ -283,6 +284,7 @@ vi.mock("./run-subagent-registry.runtime.js", () => ({ })); vi.mock("../../agents/cli-runner.runtime.js", () => ({ + clearCliSession: clearCliSessionMock, setCliSessionId: vi.fn(), })); @@ -487,6 +489,7 @@ function resetRunExecutionMocks(): void { runEmbeddedAgentMock.mockReset(); runEmbeddedAgentMock.mockResolvedValue(makeDefaultEmbeddedResult()); runCliAgentMock.mockReset(); + clearCliSessionMock.mockReset(); getCliSessionIdMock.mockReturnValue(undefined); countActiveDescendantRunsMock.mockReset(); countActiveDescendantRunsMock.mockReturnValue(0); diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index d6deee25525..54ffb0bc64c 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -944,7 +944,10 @@ async function finalizeCronRun(params: { prepared.cronSession.sessionEntry.contextTokens = contextTokens; if (isCliProvider(providerUsed, prepared.cfgWithAgentDefaults)) { const cliSessionId = finalRunResult.meta?.agentMeta?.sessionId?.trim(); - if (cliSessionId) { + if (finalRunResult.meta?.agentMeta?.clearCliSessionBinding === true) { + const { clearCliSession } = await loadCliRunnerRuntime(); + clearCliSession(prepared.cronSession.sessionEntry, providerUsed); + } else if (cliSessionId) { const { setCliSessionId } = await loadCliRunnerRuntime(); setCliSessionId(prepared.cronSession.sessionEntry, providerUsed, cliSessionId); }