store: the A2A task store and the MCP call log are durable, and a restart is what proves it - #7
Merged
Merged
Conversation
The MCP plane's call evidence rode the admin audit ring: an in-memory, size-bounded working set shared with admin mutations. Two things follow, and both are bad. Data-plane tool calls arrive at request rate, so a busy afternoon evicts every admin row from the ring -- the question of who changed a registration becomes unanswerable exactly when an incident makes somebody ask. And nothing survived a restart at all. So: `mcp_calls`, its own table, a different population from `audit_log` on purpose. The chain is scoped to the PRINCIPAL -- (principal, seq) is the primary key -- because a global chain would serialise every caller's calls behind one append and would make one caller's evidence unverifiable without possessing every other caller's rows. Opaque `body` plus only the columns a query needs. `principal` and `ts` are the index columns (scoped read; the retention sweep's age key). The chain columns -- seq, prev_hash, hash -- are REAL columns rather than buried in the body, because durability here is established by READING THE CHAIN BACK and verifying it, and a digest reachable only by decoding a blob can be neither constrained nor indexed. `version` and `expires_at` are carried now and written by nothing: adding a column to a populated table later is a rewrite, carrying it from the first migration is free. The store never computes or recomputes a digest. It persists what it was handed, verbatim, and returns it verbatim -- a digest a store could recompute is one a compromised store could forge consistently. A record landing on an occupied (principal, seq) is settled the way the contract settles it: byte-identical is the at-least-once retry and succeeds, DIFFERENT is a forked or tampered log and is an error. Overwriting would destroy precisely the case worth reporting. A trigger enforces that a persisted record is never rewritten, so the property survives an operator with the sqlite3 CLI; there is deliberately no matching delete guard, because retention has to remain possible and bounded-retention is not an integrity claim. v6 -> v7 is purely additive: the table is new, so the unconditional CREATE TABLE IF NOT EXISTS is the whole migration and no backfill arm exists. The test that carries this is not a unit test against a live handle -- that cannot tell a backend that wrote to disk from one holding a HashMap behind the same trait. It writes a chain, DROPS the store (closing every connection and its WAL), reopens the same file, and asserts the chain still links from the bytes that came back off disk. Run before the methods existed, against the trait's accept-and-keep-nothing defaults, it fails with "got 0 records back after reopening the file"; that is the behaviour this backend replaces. `.busbar-ref` moves off the 1.5.3 pin, which predates the contract, onto the `dev` commit that carries it. CI already builds against the same-named core branch, so CI and release now name the same line without a hand-held pin.
…t proves it A2A is asynchronous by design. A task spans turns, can sit interrupted waiting on a human, and can outlive the process that started it. The engine has had a seam for that since 1.5.6 -- `put_task`/`get_task`/`list_tasks`/ `purge_tasks_before`/`append_task_event`/`list_task_events` -- and this backend implemented none of them, so the trait's defaults applied: accept the write, return `Ok(())`, keep nothing. Every in-flight task was lost on every deploy, and nothing anywhere reported it, because the return value of a write is not evidence that anything was stored. So: `tasks` and `task_events`, their own tables. Every field is a real column rather than the opaque `body` that `mcp_calls` uses, and the difference is not inconsistency. A call record carries open-ended narration no query filters on, so it pays for a blob and saves the columns. A task row is a small fixed state record where every field is part of what a resume reads back, `state` and `updated_at` are the retention sweep's own predicates, and there is no field left over to make opaque. `put_task` UPSERTS by `task_id`: the engine writes through on every state transition, so a second write for one task must replace the row rather than leave a second one behind. `append_task_event` also upserts, and this is the one place the task contract genuinely DIFFERS from `append_mcp_call`'s. That method treats a different record at an occupied sequence as a forked log and refuses it. A task event is specified to upsert on `(task_id, seq)` so the engine's write-through is idempotent on replay -- "rejecting or duplicating a replayed seq breaks the chain the engine will verify on read". Copying the call log's fork check here would have been wrong in a way that looks right, so `task_events` deliberately has no no-update trigger to match `mcp_calls`'s. Retention drops TERMINAL rows only, and the terminal set is named as a closed list rather than derived by negation: a state token this build has never heard of reads as not-terminal, so a store compiled before a state existed cannot delete a task it does not understand. An interrupt waiting on a human is exactly the row that legitimately sits still for a long time; compacting it is losing the work, not reclaiming space. A purged task takes its provenance chain with it, via a trigger rather than a foreign key. The cascade is load-bearing -- `purge_tasks_before` is the only retention method the contract gives this data, so leaving the events behind would leave `task_events` with no bound anywhere in the trait. A trigger rather than an FK because an FK would also impose an ORDER on the writes, and the engine is under no such obligation: a `task.submitted` event and the first `put_task` are two independent write-throughs. A `u64` past `i64::MAX` is refused rather than mangled, the same guard `append_audit` already applies to its own seq, factored out now that five fields across two methods need it. Here the value that would silently change is the ARTIFACT CURSOR -- how much of a stream has been relayed -- so a wrapped one either replays delivered artifacts or skips undelivered ones, with no error ever reported. v7 -> v8 is purely additive: two new tables, no backfill, nothing dropped. The test that carries this is not a unit test against a live handle -- that cannot tell a backend that wrote to disk from one holding a HashMap behind the same trait, nor either of those from the defaults. It writes a task, DROPS the store (closing every connection and its WAL), reopens the same file, and reads the row back off disk. Run before the methods existed, against the accept-and-keep-nothing defaults, all seven fail, the headline being "got None back after reopening the file"; that is the behaviour this backend replaces.
Owner's call, 2026-08-11: the release carrying MCP and A2A is 1.6.0, not 1.5.5. Field 2 was stale (1.5.3 / 1.5.5) and is now the real number. Field 1 (the engine SHA) is deliberately UNCHANGED — a separate change owns the pin.
…is what proves it Every existing test of the ten task / call-log methods calls SqliteStore directly, in-process, and none of them can see the failure that matters in production. `busbar_api::Store` defaults all ten to accept-and-keep-nothing, so a plugin seam that does not RELAY them silently substitutes those defaults: every write returns Ok, every read answers empty, and a deployment running this backend as a plugin -- the only way it ever runs -- loses every in-flight A2A task and every tool-call record while reporting success. So this goes through `busbar_plugin_loader::load_store`: a real dlopen of the packed cdylib, the real C ABI, the real DynStore. It writes at ARITY > 1 (three tasks across three states, three events on one task and one on another, three call records for one principal and one for a second), drops the handle -- which unloads the library -- then dlopens again over the same file and reads everything back, retention counts included. A single-row round trip cannot tell a relayed method from a lucky default; a multi-row one across a reload cannot be faked by either. RED against busbar dev @ afde0814 (`list_tasks` answers 0 of 3), GREEN against fix/abi-a2a-task-methods @ 3f1bc096, both watched. The red is the truthful report that durable tasks do not yet work through the only path that ships, and it clears the moment the engine-side ABI relay lands on the ref this repo builds against.
The over-the-ABI durability test looked for the plugin cdylib in target/<profile>/, which only `cargo build` ever refreshes. Under `cargo test` cargo emits the artifact into target/<profile>/deps/ and never uplifts it, so the test read whatever an earlier build happened to leave behind — or, on a fresh clone, nothing at all, in which case it printed "skip:" and reported GREEN with zero coverage of the ten task/call-log methods. Both outcomes are lies about durability. A cdylib older than an ABI change answers every write Ok(()) and every read empty, which is byte-for-byte the unrelayed-seam defect this test exists to catch; and an artifact newer than a regression reports green while the shipped ABI is broken. Proven, not argued: with a regressed plugin in the tree and a good cdylib in target/debug/, the old lookup PASSED and this one FAILS. plugin_path() now reads only deps/<name> — the output of the same build graph that produced this test binary — panics instead of skipping when it is absent, and asserts the artifact is no older than any workspace src/**/*.rs, failing with a message that says STALE ARTIFACT and explicitly not a durability verdict. Note the dev-dependency edge used elsewhere for this does NOT make the artifact exist: cargo satisfies a dev-dep with the rlib. Verified by deleting the cdylib and re-running.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the A2A goal item D.19 for this backend, plus the same-defect-class gap in the MCP tool-call log.
The defect
busbar_api::Storehas ten methods whose defaults are accept and keep nothing —put_task/append_task_event/append_mcp_callreturnOk(()),get_taskreturnsNone, thelist_*methods return empty. This backend overrode none of them. Every in-flight A2A task, and every MCP tool-call record, was lost on every restart, and nothing anywhere reported it: the return value of a write is not evidence that anything was stored.Two commits, one per plane:
mcp_calls— the tool-call log, chained per principal,(principal, seq)primary key, append-only by trigger. A record landing on an occupied(principal, seq)is byte-compared: identical is the at-least-once retry and succeeds, different is a forked log and errors.tasks/task_events— the durable task store.put_taskupserts bytask_id.append_task_eventupserts on(task_id, seq), which is where the task contract genuinely differs fromappend_mcp_call's fork check — copying the call log's behaviour here would have been wrong in a way that looks right. Retention drops terminal rows only, from a closed named set so an unrecognised state token from a newer engine is never deleted, and takes the task's provenance chain with it (there is nopurge_task_events_beforein the trait, so without the cascade that table has no bound).Schema v6 → v8, additive at each step: new tables only, no backfill, nothing dropped.
.busbar-refBumped from
c8780349(1.5.3) to5a4f0195, thedevcommit that carries both contracts — the old pin predates them and would not compile these methods. The engine-side diff ofcrates/apibetween the two is purely additive:TaskRow,TaskEventRow, and six defaulted trait methods. Nothing else in theStoretrait moved, so the bump drags in no other work.ci.ymlis unchanged and still builds against the same-named core branch, so CI and release now name the same line without a hand-held pin.How this was verified
Red before green. Every new test was run first against the unimplemented state and seen to fail. All 7 task tests failed on the accept-and-keep-nothing defaults, headline
got None back after reopening the file; the MCP tests likewise withgot 0 records back.The property is "it survives a restart," and a restart is what proves it. The durability tests do not round-trip through one live handle — that cannot tell a backend that wrote to disk from one holding a
HashMapbehind the same trait. They drop the store, closing every SQLite connection and its WAL, reopen the same file, and read the row back off disk.Gates run locally
public-hygiene-lint --selftest/--rootcargo fmt --all -- --checkcargo build --all-targetscargo clippy --all-targets -- -D warningscargo test(lib)cargo test(plugin lib)executable-config-lint --selftest/--rootsigning-gate.shKnown local-only failure: the two
tests/e2e.rscases that boot a real busbar fail on macOS withmust create the configured db_path within 15s. Diagnosed rather than assumed: the boot completes and the db file is created, but the firstdlopenof a freshly-extracted unsigned debug dylib costs ~15–18s under macOS code-signing scanning, landing right on the test's deadline. Timed at 14.9s on the parent commit (no task-store code) and 14.2–18.4s on this one — identical, so not a regression. A boot with the plugin present but the store never opened is instant, which locates the cost indlopen, not in this code. Linux CI is the arbiter.