fix(gateway): make a history-dropping prompt.submit prove it meant to (#80763) - #80802
Conversation
prompt.submit honored truncate_before_user_ordinal on every request. A client that carried a leftover ordinal into an ordinary send therefore issued something the gateway could not tell apart from a real rewind — same method, same shape, an in-range target — and the cut was applied with replace_messages(), which DELETEs the durable rows. One report lost 244 messages (296 -> 52) with no prompt and nothing to restore from. The existing guard only covered ordinal 0, where the cut empties the transcript; a mid-session ordinal sailed straight through. Only the client knows whether a submit is a rewind, an edit, or a regenerate, so require it to say so: an ordinal without confirm_truncate is refused on 4029 and neither memory nor the DB is touched. Desktop sends the flag from the one place that builds these params, so every rewind path is covered and a stale build fails closed with an actionable error instead of quietly deleting a conversation.
External hosts speak this protocol directly, so the parameter that rewrites a session's stored transcript should not be folklore. Document what each truncation field means, that an ordinal without confirm_truncate is refused, and that a client must never hold the ordinal in state across ordinary submits.
alt-glitch
added
type/bug
|
I found one remaining archive-loss path in this PR. On an in-place-compacted session, the gateway history contains only the active compacted view, while the retained pre-compaction rows remain inactive under the same session id. A confirmed rewind currently calls replace_messages without active_only, whose default is to delete every row for that session. A legitimate edit/regenerate can therefore erase the entire retained compaction archive even though this PR correctly gates stale or accidental truncation. I prepared a minimal supplement directly on top of the current PR head 00b5fc4:
Commit: poisdahl@fa19ae7 To take it as-is: Validation:
An independent read-only Opus 5 xhigh review reverified the exact commit, parent, tree, and diff and returned PASS with no P0/P1 findings. One non-blocking behavior note is worth making explicit: active_only=True also stops this path from collaterally purging inactive /undo audit rows. All normal resume, live-history, recent-message, and search paths filter those rows, so they are not resurfaced; retaining them is also consistent with rewind_to_message documenting them as audit/forensic history. |
4 participants
Fixes #80763.
Summary
prompt.submithonoredtruncate_before_user_ordinalon every request. A client that carried a leftover ordinal into an ordinary send issued something the gateway could not tell apart from a real rewind — same method, same field shape, an in-range target — and the cut was applied withreplace_messages(), which DELETEs the durable rows and drops them from FTS. The reporter lost 244 messages (296 → 52) across two days of work, with no confirmation and nothing to restore from.The existing guard only covered the ordinal-0 edge, where the cut empties the transcript. A mid-session ordinal produced a non-empty result and sailed straight past it — which is exactly the shape both logged incidents had (
ordinal=2, 296 → 52 and 53 → 52).Nothing server-side can distinguish a stale ordinal from a deliberate rewind, because the two requests are identical. Only the client knows whether the user asked for a rewind, an edit, or a regenerate — so it now has to say so:
tui_gateway/methods_prompt.py): an ordinal withoutconfirm_truncateis refused with code4029. The refusal is logged with the session and the size of the history it protected, and neithersession["history"]nor the DB is touched. Theconfirm_empty_truncategate stays on top of it for the whole-transcript edge.use-prompt-actions/rewind.ts):truncateSubmitParams— the one place every rewind, edit, regenerate, and restore-checkpoint path builds these params — always sets the flag, so all four paths are covered by construction.The guard fails closed on purpose. A desktop build older than this change loses rewind against a newer gateway and gets an actionable error, instead of silently deleting a conversation; for a destructive, unrecoverable write that is the right side to fail on.
Making the truncation itself reversible (the issue's suggestion 2 — soft-archive the dropped rows the way
archive_and_compactdoes instead of DELETEing them) is worth doing and is deliberately not in this PR: it is a storage-layer change with its own retention and restore-UX questions, and it does not need to block closing the vector that caused the loss.Test plan
scripts/run_tests.sh tests/test_tui_gateway_server.py -q— 519 passed.test_prompt_submit_refuses_unconfirmed_nonempty_truncationreproduces the report: a 6-message history,ordinal=2on an ordinary submit, no confirm flag. Asserts4029, that history /history_version/runningare untouched, thatreplace_messageswas never called, that falsey confirm values do not satisfy the opt-in, and thatconfirm_empty_truncatecannot stand in for rewind intent on a non-empty cut.confirm_truncateand still pass, so edit / regenerate / display_kind-skipping / persist-failure behavior is unchanged for a confirmed rewind.npx vitest run src/app/sessioninapps/desktop— 443 passed.rewind.test.tsasserts no ordinal is ever built without the flag; the threeindex.test.tsxrestore-checkpoint assertions cover the full wire params.npm run typecheckinapps/desktop— clean.