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/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(), { 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); + }); });