From 2fbb9ec1cd9b9b03d21dac3193676ad288c2463f Mon Sep 17 00:00:00 2001 From: Nadia Rodionova <273119990+sosidudku1@users.noreply.github.com> Date: Thu, 12 Nov 2026 03:44:29 +0300 Subject: [PATCH] fix(http): a failed question on screen never takes back a stored answer After a cleared transcript the client counts the question's position from what it shows, so a failed "continue" the agent never stored could line up with an earlier "continue" and remove its real answer. expectUnanswered: true says the client saw no answer, and a stored reply then refuses with turn_mismatch (ATO-1420). --- src/http/README.md | 2 +- src/http/route-rollback-last-turn.test.ts | 17 ++++++++++++ src/http/route-rollback-last-turn.ts | 21 ++++++++++---- src/session/session-truncate.test.ts | 34 +++++++++++++++++++++++ src/session/session-truncate.ts | 25 ++++++++++++++++- 5 files changed, 91 insertions(+), 8 deletions(-) diff --git a/src/http/README.md b/src/http/README.md index 017e9f4d5..cc590fcab 100644 --- a/src/http/README.md +++ b/src/http/README.md @@ -55,6 +55,6 @@ Auth on the network (ATO-1126): the bearer token is compared in constant time ([ Per-chat folders (ATO-731, [session-working-dir.ts](session-working-dir.ts)): every tool step reads its session's `workingDir`, so `POST /v1/chat/completions` takes `working_dir` (absolute, existing) for a NEW session, `POST /api/sessions/{id}/working-dir` `{ workingDir }` moves one chat (`409 turn_in_progress` only while that chat runs a turn), and `GET /api/sessions?scope=all` lists chats from every folder. None of it restarts `serve`. -Taking a turn back (ATO-1298, [route-rollback-last-turn.ts](route-rollback-last-turn.ts)): `POST /api/sessions/{id}/rollback-last-turn` removes the chat's last question and its answer and returns `200 { id, text }`, the question's text. The body may carry `{ expectText, expectPriorQuestions }`, the question the client means by its text and by how many questions come before it (steers not counted); if the stored last question differs in either (a failed turn the agent never recorded), it answers `409 turn_mismatch` and removes nothing. It only removes: the client sends `text` again through `POST /v1/chat/completions`, so a retry runs the same path as a first try and the old answer is no longer in the chat the model reads. It does not reach outside the transcript: what the turn's reflection already wrote to long-term memory stays, and a chat cut back to nothing keeps its title and leaves `GET /api/sessions` (no question in it) until the question is sent again. Only a turn that changed nothing outside the transcript qualifies: a question answered by one reply, or by nothing (a provider error). A stopped turn does not: a stop can cut a tool batch short, and a local turn stores no trace of that step. Otherwise `409` with `code` `turn_has_tools`, `turn_steered`, `turn_compacted`, `turn_stopped`, `turn_in_progress`, `nothing_to_undo`, `turn_unrecognized` or `turn_mismatch`, and the chat is left as it was. `404 session_not_found` is an unknown chat; an agent without the route answers 404 without that code. The cut itself is [truncateSessionAt](../session/session-truncate.ts). +Taking a turn back (ATO-1298, [route-rollback-last-turn.ts](route-rollback-last-turn.ts)): `POST /api/sessions/{id}/rollback-last-turn` removes the chat's last question and its answer and returns `200 { id, text }`, the question's text. The body may carry `{ expectText, expectPriorQuestions, expectUnanswered }`, the question the client means by its text, by how many questions come before it (steers not counted, questions a cleared transcript hides counted) and, with `expectUnanswered: true`, by the client having seen no answer to it; if the stored last question differs in any (a failed turn the agent never recorded), it answers `409 turn_mismatch` and removes nothing. `expectUnanswered` (ATO-1420) keeps a wrong count from removing a real answer: a failed question on screen never matches a stored one that has a reply. It only removes: the client sends `text` again through `POST /v1/chat/completions`, so a retry runs the same path as a first try and the old answer is no longer in the chat the model reads. It does not reach outside the transcript: what the turn's reflection already wrote to long-term memory stays, and a chat cut back to nothing keeps its title and leaves `GET /api/sessions` (no question in it) until the question is sent again. Only a turn that changed nothing outside the transcript qualifies: a question answered by one reply, or by nothing (a provider error). A stopped turn does not: a stop can cut a tool batch short, and a local turn stores no trace of that step. Otherwise `409` with `code` `turn_has_tools`, `turn_steered`, `turn_compacted`, `turn_stopped`, `turn_in_progress`, `nothing_to_undo`, `turn_unrecognized` or `turn_mismatch`, and the chat is left as it was. `404 session_not_found` is an unknown chat; an agent without the route answers 404 without that code. The cut itself is [truncateSessionAt](../session/session-truncate.ts). [Model tiers route](route-model-tiers.ts): `GET /api/model-tiers` returns the Cheap / Balanced / Best picks per provider key (`openrouter`, `aimlapi`, `gemini`, `claude-cli`) from the one data file, [model-tiers.ts](../llm/provider/presets/model-tiers.ts) (ATO-32). The TUI pickers list those models first under the same labels. diff --git a/src/http/route-rollback-last-turn.test.ts b/src/http/route-rollback-last-turn.test.ts index 37132591e..0dac6c9b5 100644 --- a/src/http/route-rollback-last-turn.test.ts +++ b/src/http/route-rollback-last-turn.test.ts @@ -98,6 +98,23 @@ describe("POST /api/sessions/{id}/rollback-last-turn (ATO-1298)", () => { expect((await post("rb-6", { expectText: "continue", expectPriorQuestions: 0 })).status).toBe(200); }); + it("keeps a real answer when the failed question on screen lines up with it after a clear (ATO-1420)", async () => { + // "first" (cleared from the window), "continue" answered, a second + // "continue" failed and never stored: the window counts 1 question before it. + seed("rb-8", [ + { kind: "user", text: "first", at: 1 }, + { kind: "assistant_reply", text: "one", at: 2 }, + { kind: "user", text: "continue", at: 3 }, + { kind: "assistant_reply", text: "real answer", at: 4 }, + ], [2, 4]); + const res = await post("rb-8", { expectText: "continue", expectPriorQuestions: 1, expectUnanswered: true }); + expect(res.status).toBe(409); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe("turn_mismatch"); + expect(harness.runtime.sessionStore.load("rb-8")?.turns).toHaveLength(4); + expect((await post("rb-8", { expectUnanswered: "yes" })).status).toBe(400); + expect(harness.runtime.sessionStore.load("rb-8")?.turns).toHaveLength(4); + }); + it("refuses a stopped turn", async () => { seed("rb-7", [ { kind: "user", text: "delete the build folder", at: 1 }, diff --git a/src/http/route-rollback-last-turn.ts b/src/http/route-rollback-last-turn.ts index f67cdd192..ad6d5d685 100644 --- a/src/http/route-rollback-last-turn.ts +++ b/src/http/route-rollback-last-turn.ts @@ -11,17 +11,20 @@ const REFUSAL_MESSAGES: Record = { turn_compacted: "the last turn is already part of a compacted summary, so it cannot be taken back", turn_stopped: "the last turn was stopped, and a stop can cut a tool call short without a trace, so it cannot be taken back; send the question again instead", turn_unrecognized: "the last turn is not a single question and answer, so it cannot be taken back", - turn_mismatch: "the chat's last question is not the one given (expectText, expectPriorQuestions), so nothing was taken back", + turn_mismatch: "the chat's last question is not the one given (expectText, expectPriorQuestions, expectUnanswered), so nothing was taken back", }; /** * `POST /api/sessions/{id}/rollback-last-turn` (ATO-1298): remove the * chat's last question and its answer so a client can ask it again * without the model reading the old answer. Body `{}` or - * `{ expectText?, expectPriorQuestions? }`: the question the client means, - * by its text and by how many questions come before it (steers not - * counted). When the stored last question differs in either, `409 - * turn_mismatch` and nothing is removed. + * `{ expectText?, expectPriorQuestions?, expectUnanswered? }`: the + * question the client means, by its text, by how many questions come + * before it (steers not counted) and, with `expectUnanswered: true`, by + * the client having seen no answer to it (ATO-1420: a stored reply then + * means a different question, however the count lines up). When the + * stored last question differs in any, `409 turn_mismatch` and nothing + * is removed. * * Only removes. The answer is `200 { id, text }` with the question's * text; the client sends it again through `/v1/chat/completions` like @@ -41,7 +44,7 @@ export function createRollbackLastTurnHandler(): HttpHandler { const options: LastTurnRollbackOptions = {}; try { const body = (await readJsonBody(req)) as - | { expectText?: unknown; expectPriorQuestions?: unknown } + | { expectText?: unknown; expectPriorQuestions?: unknown; expectUnanswered?: unknown } | null; const text = body?.expectText; if (text !== undefined && typeof text !== "string") { @@ -55,6 +58,12 @@ export function createRollbackLastTurnHandler(): HttpHandler { return; } if (prior !== undefined) options.expectPriorQuestions = prior; + const unanswered = body?.expectUnanswered; + if (unanswered !== undefined && typeof unanswered !== "boolean") { + sendError(res, 400, openaiError("expectUnanswered must be a boolean", "invalid_request_error", "expectUnanswered")); + return; + } + if (unanswered !== undefined) options.expectUnanswered = unanswered; } catch (err) { sendError(res, 400, openaiError(`Invalid JSON: ${err instanceof Error ? err.message : String(err)}`)); return; diff --git a/src/session/session-truncate.test.ts b/src/session/session-truncate.test.ts index 165640a18..63c22f9db 100644 --- a/src/session/session-truncate.test.ts +++ b/src/session/session-truncate.test.ts @@ -306,6 +306,40 @@ describe("planLastTurnRollback", () => { expect(planLastTurnRollback(state, { expectPriorQuestions: 1 }).ok).toBe(true); }); + it("never removes a real answer for a failed question on screen, after a cleared transcript (ATO-1420)", () => { + // "first" answered, then the window was cleared (the agent keeps it); + // "continue" answered; a second "continue" failed and was never stored. + let state = turn(fresh(), user("first", 1), reply("one", 2)); + state = turn(state, user("continue", 3), reply("real answer", 4)); + // The window counts one question before the failed one (the cleared + // "first" is not on screen), which is the stored "continue"'s position. + const counted = { expectText: "continue", expectPriorQuestions: 1 }; + expect(planLastTurnRollback(state, counted).ok).toBe(true); + // The window saw no answer to the failed one: the stored "continue" is not it. + expect(planLastTurnRollback(state, { ...counted, expectUnanswered: true })) + .toEqual({ ok: false, code: "turn_mismatch" }); + expect(state.turns.map((t) => t.kind === "assistant_reply" && t.text)).toContain("real answer"); + }); + + it("still takes back a failed question that has no answer when expectUnanswered is set", () => { + let state = turn(fresh(), user("first", 1), reply("one", 2)); + state = { ...turn(state, user("continue", 3)), status: "failed" as const, lastError: "402" }; + const plan = planLastTurnRollback(state, { + expectText: "continue", + expectPriorQuestions: 1, + expectUnanswered: true, + }); + expect(plan.ok).toBe(true); + if (!plan.ok) return; + expect(plan.state.turns).toHaveLength(2); + }); + + it("keeps the stop refusal for a stopped turn when expectUnanswered is set", () => { + const state = turn(fresh(), user("delete the build folder", 1), stoppedTurnMarker(2)); + expect(planLastTurnRollback(state, { expectUnanswered: true })) + .toEqual({ ok: false, code: "turn_stopped" }); + }); + it("refuses two answers after one prompt", () => { const state = turn(fresh(), user("a", 1), reply("b", 2), reply("c", 3)); expect(planLastTurnRollback(state)).toEqual({ ok: false, code: "turn_unrecognized" }); diff --git a/src/session/session-truncate.ts b/src/session/session-truncate.ts index 46035e285..02f55f394 100644 --- a/src/session/session-truncate.ts +++ b/src/session/session-truncate.ts @@ -121,7 +121,7 @@ export type LastTurnRollbackRefusal = | "turn_unrecognized" /** * The last question is not the one the caller meant (`expectText`, - * `expectPriorQuestions`). + * `expectPriorQuestions`, `expectUnanswered`). */ | "turn_mismatch"; @@ -154,6 +154,18 @@ export interface LastTurnRollbackOptions { * `turn_mismatch`. */ expectPriorQuestions?: number; + /** + * The caller saw no answer to the question it means (its turn failed + * on screen). A stored last question that has a reply is then not that + * question, whatever its text and position say, and it refuses with + * `turn_mismatch` (ATO-1420). The position is counted by the client + * from what it shows, and a client whose transcript was cleared, or + * that counts wrong for any other reason, could otherwise line a failed + * "continue" up with an earlier "continue" and remove its real answer. + * Taking back a question that has no answer loses no answer, so the + * opposite case (a reply on screen, none stored) is not checked. + */ + expectUnanswered?: boolean; } /** @@ -201,6 +213,17 @@ export function planLastTurnRollback( } const after = turns.slice(start + 1); + if ( + options.expectUnanswered === true && + after.some( + (turn) => + turn.kind === "assistant_reply" && + turn.progressNote !== true && + turn.stopped !== true, + ) + ) { + return { ok: false, code: "turn_mismatch" }; + } if ( after.some( (turn) =>