Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions services/runner/src/engines/sandbox_agent/reconstruct-history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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,
Expand All @@ -33,18 +35,26 @@ export async function reconstructHistoryIfNeeded(
): Promise<AgentRunRequest | null> {
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] };
}
14 changes: 14 additions & 0 deletions services/runner/src/engines/sandbox_agent/session-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion services/runner/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
computeCredentialEpoch,
configFingerprint,
credentialEpochMismatch,
carriesMinimalHistory,
mountCredentialsExpired,
expectedNextHistoryFingerprint,
historyFingerprint,
Expand Down Expand Up @@ -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";

Expand Down
10 changes: 10 additions & 0 deletions services/runner/tests/unit/session-keepalive-dispatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(), {
Expand Down
45 changes: 45 additions & 0 deletions services/runner/tests/unit/session-reconstruct-history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Loading