Repository navigation
feat(native-chat): add a chat-owned command receipt store - #26653
Conversation
Adds the agent_session_command_receipts table and an insert-if-absent receipt write that runs inside a command's own journal transaction. A receipt records the command's identity, canonical hash, status and a pointer to the row holding its result; it dies with its chat record. Nothing uses it yet: the existing operation ledger stays authoritative.
Rows are keyed by caller and operation id; a send looks across callers and other methods look up only their caller's row, matching the existing ledger. Bad rows and newer table shapes read as unreadable instead of throwing.
…tion id Global lookups now take rows in insertion order, so a read and an insert agree on which receipt answers an id regardless of the host clock. The primary key leads with the operation id, so its own index serves both lookups.
Review summaryProblem. Every native-chat write (send, stop, settings and the rest) first commits a separate reservation in a shared retry table, and only then saves the user's action. That reservation has its own rules: how old a request may be, clock skew, cleanup of unrelated rows, and its own disk write. Any of them can refuse the user's write even when the write itself would have succeeded. That is how a user saw "Orca has received too many requests in the last day. Your message was not sent." An earlier PR removed the count caps; the reservation step itself is still there. User-facing change. None in this PR. It adds the storage the later PRs in this stack will use: a receipt saved in the same database transaction as the user's action, so a retry gets the original answer with no separate gate in front of the write. No chat action reads or writes it yet, and the old retry table is still the one that decides. What review found and fixed Review covered correctness, design and how it fits the rest of the stack, regressions and compatibility, and an independent architecture challenge. A readiness checklist ran at the end. The challenger designed its own solution from the issue before reading the code and arrived at the same shape: one receipt table, written in the action's own transaction, storing a pointer to the result rather than a copy.
Deferred to later PRs in this stack (they are about switching callers over, not this storage):
Minor: a test harness that reseeds chat records would wipe receipts. Only tests use it. Verified
Not verified
|
ELI5
When you press Send in a native chat, Orca first saves a separate "I've seen this request" note in a shared retry table. Only after that does it save your message. That first step has its own rules (how old a request may be, cleanup of other callers' notes, its own disk write). It can say no even when your message itself could have been saved. That is how a user got "Orca has received too many requests in the last day. Your message was not sent."
This PR is the first step of fixing that at the root. It adds a chat-owned receipt that is saved in the same database transaction as the message (or other action) itself. A retry of the same request then finds that receipt and gets the original answer back. No separate gate sits in front of the write. Nothing uses the receipt yet; later PRs in this stack switch each kind of chat action over, then remove the old gate in one step.
What Changed
User-facing: nothing yet. This PR only adds storage and tests; no chat action reads or writes the new table.
Mechanism:
New table
agent_session_command_receiptsin the chat database on the host that runs the chat. Each row holds:acceptedorrejectedIt is not a copy of the result. Only a genuine no-op answer (a cancel that stopped nothing, a queue resume that changed nothing) is stored inline, because no row exists to point at.
Ids are looked up the way the old ledger looks them up: rows are keyed by caller and operation id; a send's id is checked against every caller's rows, and any other action checks only its own caller's rows. So, for chat commands, the same id reused for a different action conflicts where it conflicts today. (Today's send check also sees ids used by non-chat actions such as terminal agent launch; the receipt table holds only chat commands. The cutover PR labels that difference.)
insertCommandReceiptIfAbsentruns only inside an open transaction (the caller's effect transaction). A second request with the same id and hash gets the existing receipt back; with a different hash it gets a conflict. A row it can't read is reported as "unreadable", never as "absent", so a damaged row can never let a retry run twice.buildCommandReceiptTransactionplugs that write into the existing per-write receipt hook (JournalOperationReceipt), which already commits inside the journal's transaction. When the id already exists, it throws, so the whole write rolls back.How a receipt dies: a receipt lives exactly as long as its chat record.
session_idreferences the chat record withON DELETE CASCADE. Chat records are updated in place (ON CONFLICT DO UPDATE), which keeps receipts, and deleting a chat record deletes them. Today nothing deletes chat records, so in practice receipts are kept for good, like the chat's journal rows; the cascade is what removes them if chat deletion is added. Receipts do not die when a rewind replaces the chat's epoch, or when a queued draft is pruned. That is why no "move the proof somewhere else before deleting" step is needed. There is no age limit, count limit or background pruning.The table is created at every writable open, as
queued_messagesis, with no schema version bump. Older builds simply ignore it. The bump question is revisited at the cutover PR, when the old ledger stops being written.Why
The old design commits a separate reservation, with its own admission policy, before the user's write. The bug class is that any bookkeeping transaction placed in front of the user's durable write can refuse it. Removing the count cap (#26543) fixed one symptom. Age expiry, clock-skew refusal, pruning of unrelated rows and the reservation's own storage failures remain.
The common pattern is that a command's receipt is inserted-if-absent inside the same transaction as the command's effect, and a duplicate reads the existing receipt. There is no separate admission step and no count or age budget.
Alternatives considered:
agent_session_operations. Rejected: terminalagent.launchstill relies on its pending-claim and expiry semantics. Native chat moves off it entirely; the launch side is a separate follow-up.Differences from the common pattern:
Linked Issue
Follow-up to #26543 (removed the ledger's count caps); this stack removes the separate admission step itself.
Visual Proof
No user-visible change: this PR adds storage with no caller, so there is no before/after screen. Proof is the receipt test run at the PR head (e31475e). It shows the duplicate and conflict answers, rollback with the effect, the chat-record cascade, survival across epoch replacement, and damaged rows reading as unreadable:
Testing
New tests in
src/main/native-chat/agent-session-journal/command-receipt-{schema,table,transaction}.test.tscover:user_versionunchangedjournal_read_onlyRun with
vitest runon those three files plusjournal-database.test.ts,journal-store-schema.test.tsandjournal-epoch-replacement.test.ts: 6 files, 65 tests passed at e31475e. At that commit the node typecheck passed through the queue, andorca-ci-checks --no-typecheckpassed 22/22. Platform: macOS. The change is SQLite inside the host process, with no path or shell behaviour, so Windows, Linux, SSH and WSL hosts run the same code on their own database.AI Disclosure
Review
Agent skill upstream boundary
Notes
Author: @BrennanKB5. Stack: this is PR 1 of the native-chat retry-proof stack. Later PRs switch each chat action (send and queued actions, Stop, settings and goals, create, rewind and
/clear) to write its receipt in its own commit. The last one removes native chat's use of the separate ledger admission and settlement. SSH and WSL: the receipt lives in the database on the host that runs the chat; clients never keep their own copy.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)