From b65231616534d25d1cc28f696017b7a36b7338db Mon Sep 17 00:00:00 2001 From: Mahmoud Mabrouk Date: Fri, 24 Jul 2026 23:48:05 +0200 Subject: [PATCH 1/2] fix(runner): stop replaying the current prompt as a prior turn The runner persists the inbound user message before it starts the engine, and acquiring a sandbox takes seconds, so by the time reconstruction reads the record log the current turn is already in it. Reconstruction returned that record as a prior turn and the inbound message was appended on top, sending the prompt to the model twice. It reproduced on every first turn of every conversation, including with the frontend flag off, because a first turn always carries exactly one message and the `messages.length > 1` guard does not stop it. Drop the current turn's records by `turn_id` before folding, and replace the message-count guard with a shared `carriesMinimalHistory` predicate that both this seam and the keep-alive check can agree on. The count alone also let reconstruction run for an empty array and for a lone assistant message. Claude-Session: https://claude.ai/code/session_01KM69J7uHafgciiN5zfG7qR --- .../sandbox_agent/reconstruct-history.ts | 32 ++++++++----- .../engines/sandbox_agent/session-identity.ts | 14 ++++++ .../unit/session-reconstruct-history.test.ts | 45 +++++++++++++++++++ 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/services/runner/src/engines/sandbox_agent/reconstruct-history.ts b/services/runner/src/engines/sandbox_agent/reconstruct-history.ts index 1a3e0a4766..ba2e0c25bd 100644 --- a/services/runner/src/engines/sandbox_agent/reconstruct-history.ts +++ b/services/runner/src/engines/sandbox_agent/reconstruct-history.ts @@ -3,14 +3,19 @@ * trusting a full inbound history — the server side of "client sends only the last message". * * Flag-gated (`AGENTA_SESSIONS_RECONSTRUCT`) and a strict no-op until BOTH the flag is on AND the - * client actually sent a minimal history: when the client still sends the whole conversation - * (`messages.length > 1`), reconstruction is skipped and behaviour is unchanged. Best-effort — any - * miss (no session, no records, fetch failure) leaves the inbound history untouched. + * client actually sent a minimal history (`carriesMinimalHistory`). Best-effort — any miss (no + * session, no records, fetch failure) leaves the inbound history untouched. + * + * The record log already contains the CURRENT turn by the time this runs: the runner persists the + * inbound user message before it starts the engine, and acquiring a sandbox takes seconds. Its + * records are therefore dropped by `turn_id` here, or the current prompt would be reconstructed + * as a prior turn and then appended again from the inbound history. */ import type { AgentRunRequest } from "../../protocol.ts"; import { fetchSessionRecords } from "../../sessions/records-query.ts"; import { reconstructMessages } from "../../sessions/reconstruct.ts"; +import { carriesMinimalHistory } from "./session-identity.ts"; function reconstructEnabled(): boolean { return ( @@ -21,9 +26,6 @@ function reconstructEnabled(): boolean { /** * Returns a request whose `messages` are `[...reconstructed prior turns, ...inbound]` when * reconstruction applies, else `null` to keep the inbound history as-is. - * - * MUST be called before the current turn's user message is persisted, so the record log holds - * only prior turns (no duplication of the incoming prompt). */ export async function reconstructHistoryIfNeeded( request: AgentRunRequest, @@ -33,18 +35,26 @@ export async function reconstructHistoryIfNeeded( ): Promise { if (!reconstructEnabled() || !sessionId) return null; const inbound = request.messages ?? []; - // The client already sent the conversation — nothing to rebuild. - if (inbound.length > 1) return null; + // The client still asserts the conversation itself — nothing to rebuild. + if (!carriesMinimalHistory(request)) return null; const records = await fetchSessionRecords(sessionId, auth); - if (!records || records.length === 0) return null; + if (!records) return null; + + // Drop this turn's own records: the inbound message already carries the current prompt. + const currentTurnId = request.turnId?.trim(); + const prior = currentTurnId + ? records.filter((row) => row.turn_id !== currentTurnId) + : records; + if (prior.length === 0) return null; - const reconstructed = reconstructMessages(records); + const reconstructed = reconstructMessages(prior); if (reconstructed.length === 0) return null; log?.( `[reconstruct] session=${sessionId} records=${records.length} ` + - `priorMessages=${reconstructed.length} inbound=${inbound.length}`, + `prior=${prior.length} priorMessages=${reconstructed.length} ` + + `inbound=${inbound.length}`, ); return { ...request, messages: [...reconstructed, ...inbound] }; } diff --git a/services/runner/src/engines/sandbox_agent/session-identity.ts b/services/runner/src/engines/sandbox_agent/session-identity.ts index 27a4a90f39..dd6bf5b225 100644 --- a/services/runner/src/engines/sandbox_agent/session-identity.ts +++ b/services/runner/src/engines/sandbox_agent/session-identity.ts @@ -290,6 +290,20 @@ export function approvalDecisionForToolCall( return undefined; } +/** + * True when the request carries exactly its own fresh user turn and no prior conversation — + * what a last-message-only client sends. The single predicate both sides agree on: the runner + * reconstructs prior turns only for such a request, and the keep-alive check skips its history + * comparison for one, because the client is no longer asserting the conversation at all. + * + * A message count alone is NOT enough: turn one of any conversation is also a single message, + * and a lone assistant message or an empty array are neither a fresh turn nor a full history. + */ +export function carriesMinimalHistory(request: AgentRunRequest): boolean { + const messages = request.messages ?? []; + return messages.length === 1 && tailIsFreshUserMessage(request); +} + /** * True when the request's tail is a fresh user message with text and NOT an approval envelope. * A continuation only takes the live path for a plain new user turn; an approval reply (a diff --git a/services/runner/tests/unit/session-reconstruct-history.test.ts b/services/runner/tests/unit/session-reconstruct-history.test.ts index 08c877037e..c8369832ed 100644 --- a/services/runner/tests/unit/session-reconstruct-history.test.ts +++ b/services/runner/tests/unit/session-reconstruct-history.test.ts @@ -88,4 +88,49 @@ describe("reconstructHistoryIfNeeded", () => { // Other request fields are preserved. assert.equal((out as { harness?: string }).harness, "pi"); }); + + it("no-op when the request carries no messages at all", async () => { + vi.stubEnv("AGENTA_SESSIONS_RECONSTRUCT", "true"); + const req = { messages: [] } as never; + const out = await reconstructHistoryIfNeeded(req, "sess-1", auth); + assert.equal(out, null); + assert.equal(fetchCalls, 0); + }); + + it("no-op when the single inbound message is not a fresh user turn", async () => { + vi.stubEnv("AGENTA_SESSIONS_RECONSTRUCT", "true"); + const req = { messages: [{ role: "assistant", content: "a1" }] } as never; + const out = await reconstructHistoryIfNeeded(req, "sess-1", auth); + assert.equal(out, null); + assert.equal(fetchCalls, 0); + }); + + it("drops the current turn's own records so the prompt is not replayed twice", async () => { + vi.stubEnv("AGENTA_SESSIONS_RECONSTRUCT", "true"); + // The runner persists the inbound prompt BEFORE the engine starts, so by the time this + // runs the log already holds turn-2's own user record. + recordsToReturn = [ + { turn_id: "turn-1", record_source: "user", attributes: { type: "message", text: "q1" } }, + { turn_id: "turn-1", record_source: "agent", attributes: { type: "message", text: "a1" } }, + { turn_id: "turn-2", record_source: "user", attributes: { type: "message", text: "hi again" } }, + ]; + const req = { messages: [userTurn], turnId: "turn-2" } as never; + const out = await reconstructHistoryIfNeeded(req, "sess-1", auth); + assert.ok(out); + assert.deepEqual(out!.messages, [ + { role: "user", content: "q1" }, + { role: "assistant", content: "a1" }, + userTurn, + ]); + }); + + it("no-op when the only records belong to the current turn (first turn of a session)", async () => { + vi.stubEnv("AGENTA_SESSIONS_RECONSTRUCT", "true"); + recordsToReturn = [ + { turn_id: "turn-1", record_source: "user", attributes: { type: "message", text: "hi again" } }, + ]; + const req = { messages: [userTurn], turnId: "turn-1" } as never; + const out = await reconstructHistoryIfNeeded(req, "sess-1", auth); + assert.equal(out, null); + }); }); From 66e70b6926115ab5a80fc7cdc064604fceb0fc73 Mon Sep 17 00:00:00 2001 From: Mahmoud Mabrouk Date: Fri, 24 Jul 2026 23:49:56 +0200 Subject: [PATCH 2/2] fix(runner): keep the warm session when the client sends a minimal history The keep-alive check fingerprints the prior conversation the client sent and compares it against what the previous turn stored. A last-message-only client sends no prior conversation, so the fingerprint is of an empty array and can never match. Every conversation was evicted to cold on every turn after the first. Measured on a three-turn run, turn 2 went from 1570ms to 4623ms while the suite still reported a pass, because a cold turn is still faster than the first. Skip the history comparison when the request carries only its own fresh user turn. The session id already binds the request to the conversation; a minimal-history client simply no longer asserts it, so there is nothing to compare. Requests that do send a history are still checked exactly as before, including the approval-resume path, which always sends the full history. Claude-Session: https://claude.ai/code/session_01KM69J7uHafgciiN5zfG7qR --- services/runner/src/server.ts | 9 ++++++++- .../tests/unit/session-keepalive-dispatch.test.ts | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/services/runner/src/server.ts b/services/runner/src/server.ts index 44208229f5..b7e7ed7e26 100644 --- a/services/runner/src/server.ts +++ b/services/runner/src/server.ts @@ -49,6 +49,7 @@ import { computeCredentialEpoch, configFingerprint, credentialEpochMismatch, + carriesMinimalHistory, mountCredentialsExpired, expectedNextHistoryFingerprint, historyFingerprint, @@ -595,9 +596,15 @@ export async function runWithKeepalive( existing.credentialEpoch, incomingEpoch, ); + // A last-message-only client sends no prior conversation, so there is nothing to compare: + // `priorConversation` is empty and the fingerprint can never match what the last turn stored. + // Comparing anyway evicts the warm session on every turn of every conversation. The session + // id already binds the request to this conversation; the client simply no longer asserts it. + const clientAssertsHistory = !carriesMinimalHistory(request); let mismatch: string | undefined; if (cfgFp !== existing.configFingerprint) mismatch = "config"; - else if (priorFp !== existing.historyFingerprint) mismatch = "history"; + else if (clientAssertsHistory && priorFp !== existing.historyFingerprint) + mismatch = "history"; else if (credMismatch) mismatch = credMismatch; else if (!tailIsFreshUserMessage(request)) mismatch = "tail"; diff --git a/services/runner/tests/unit/session-keepalive-dispatch.test.ts b/services/runner/tests/unit/session-keepalive-dispatch.test.ts index 04129c0bc4..c77f1ae55a 100644 --- a/services/runner/tests/unit/session-keepalive-dispatch.test.ts +++ b/services/runner/tests/unit/session-keepalive-dispatch.test.ts @@ -476,6 +476,16 @@ describe("runWithKeepalive: validation mismatches degrade to cold", () => { assert.equal(calls.acquire, 2); }); + it("a last-message-only turn 2 keeps the warm session (no history to compare)", async () => { + // The flag-on frontend sends ONLY the trailing user message, so `priorConversation` is + // empty and its fingerprint can never match what turn 1 stored. Comparing anyway evicted + // every conversation to cold on every turn. + const minimal = turn2("s1", { messages: [{ role: "user", content: "more" }] }); + const { calls, env1 } = await parkThen(minimal); + assert.equal(env1.destroyed, 0, "the warm env must survive a minimal-history turn"); + assert.equal(calls.acquire, 1, "no cold re-acquire"); + }); + it("credential-epoch expiry evicts to cold", async () => { // The parked mount expiry is in the past, so the next turn's epoch check fails. const { calls, env1 } = await parkThen(turn2(), {