feat(hindsight): durable context outbox and background delivery worker #59
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/issue-17-outbox"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Motivation
Life records semantic context in Hindsight, and Hindsight is optional: it can be
disabled, unreachable, rate limiting, or mid-deployment. Two invariants follow
from
docs/technical/timeline-consistency.mdanddocs/technical/hindsight.md,and neither held in code before this change:
semantic backend may never roll back, delay or relabel a confirmed change;
acceptance (HTTP 202 + operation id), completed ingestion, and refreshed
derived observations are three different things, and "recorded locally" is a
fourth.
This issue builds the durable outbox, the background delivery worker, the
enqueue path and the delivery-state wiring. Scenarios I03 (three milestones stay
distinct), I04 (Hindsight outage does not touch a calendar write) and R12 (a lost
acceptance response keeps uncertainty explicit and resumes under the same
identity), plus W02 for the capture family.
Changes
Durable outbox —
src/history/migrations.rs,src/hindsight/outbox.rsoutboxtable is now added by its own versioned migration(
CURRENT_SCHEMA_VERSION1 → 2,002_outbox), transactional and additive(
CREATE TABLE IF NOT EXISTS+ the normative queue indexidx_outbox_queue,then
DROP INDEX IF EXISTS idx_outbox_state). The migration converges bothdatabase shapes onto one schema: a database that never held the table gains it
plus the queue index, and one created before the migration keeps its rows,
gains
idx_outbox_queueand loses the supersededidx_outbox_state, so freshand upgraded databases hold the same indexes and no existing row is touched.
The table and index definitions were removed from the v1 baseline so the
outbox is owned by exactly one migration.
outbox.rsrecords and reads deliveries through the daemon's single blockingDatabaseWorker— never a second connection, never a second worker. Rowstates are
pending/accepted/completed/retry_wait/needs_reconciliation/failed;DeliveryStatefrom #16 is reused ratherthan duplicated (its
not_requestedis a caller-facing state, never a rowstate).
NewContextDelivery::hindsightcomputes
life-event:<event_id>from the source event, and a row whose storeddocument_iddisagrees with its own payload is refused as unusable ratherthan dispatched under either identity. A retry therefore can only address the
document the first attempt used, under
update_mode: replace.enqueue_in_transactionrecords the delivery inside a transaction the calleralready owns. A delivery for a document that was never dispatched is
replaced by a newer payload; one upstream already accepted, completed or
failed is left exactly as it is — rewriting an in-flight operation's payload
would silently change what the accepted operation means.
POST /v1/context,POST /v1/context/search) will call:outbox::enqueue_context_delivery(&db, &NewContextDelivery::hindsight(event_id, retain))for a delivery in its own transaction, or
JournalService::commit_operation_result_with_delivery(..., Some(delivery))to record the operation, the fact and the delivery in one local
transaction.
ContextDelivery::from_outbox(state, document_id, operation_id, detail)reports what a row actually holds.Background worker —
src/hindsight/worker.rsserve()), not a second database worker.Per pass it (1) dispatches rows that were never accepted upstream, (2)
persists a returned operation id before the first poll of it, (3) polls
known operations instead of resubmitting while they may be active, (4)
schedules a bounded retry honouring the server's
Retry-After, and (5) halts— returning from the loop, sending nothing further — on authentication or
schema failures, leaving the operator-visible code and message on the row.
upstream_operation_idis never dispatched again; aneeds_reconciliationrow without one (a lost acceptance) is re-submittedunder the same document, so uncertainty stays explicit without inventing a
new identity.
MAX_OUTBOX_PER_PASS,OUTBOX_DRAIN_BUDGET), so abacklog is worked off over several passes; shutdown is between operations and
leaves every row as its last committed transition wrote it.
serve()now verifies the deployment (verify_compatibility), drains what isalready due within the bounded budget, and keeps draining in the background;
the graceful-shutdown path cancels the worker and awaits it.
Capture route and delivery-state wiring —
src/api/captures.rs,src/api/mod.rs,src/cli.rsPOST /v1/captures(life capture --input -) is the one place a delivery isrequested.
kind: "context"records pure context: zero calendar items, onecontext_capturedfact and one outbox row in a single transaction, answeredpartial(exit 6) withcontext_delivery: "pending"until ingestion isconfirmed.
kind: "task"/"event"with an optionalcontextis anactionable capture: the calendar part goes through the creation path exactly
as it does without a capture (same validation, conditional write, journal
transaction, idempotency key), and the rationale is recorded beside it.
context_deliveryfield and the
context_pending/context_unavailablewarnings, so a callercan tell queued from delivered. A replay reports the state the durable row
actually holds — the CLI smoke run below shows a replay answering
retry_waitwith the operator detail, not a stalepending.POST /v1/captures(both implemented kinds). Routesleft:
POST /v1/tasks,POST /v1/eventsand the update/lifecycle routes areuntouched — they carry no semantic-context member, so they neither enqueue nor
report a delivery, and no delivery is faked anywhere.
POST /v1/contextandPOST /v1/context/searchremain #18's.MutationDatadeliberately gained noalways-
not_requestedfield (per coordinator decision): the field appearswhere a delivery can actually be requested.
docs/technical/cli-api.md'sillustrative
task addexample that showscontext_deliveryon a plaincreation therefore remains aspirational until a creation payload can carry
context.
kind: "outcome"andkind: "correction"are refused with HTTP 501not_implemented(they belong to #28/#29) — never half-performed, never asilent ignore.
context_delivery: "failed"withcontext_unavailable, while the calendarpart stays confirmed. A rationale that cannot be recorded at all (broken
outbox) is reported the same way and never blocks or rewrites the calendar
result.
Journal —
src/history/journal.rscommit_operation_resultnow delegates tocommit_operation_result_with_delivery, so a capture can record itsoperation, fact and delivery atomically; the existing call sites and their
behaviour are unchanged. The
life_eventsinsert is shared through oneappend_eventhelper with an explicit conflict policy: the normal commit pathstill rejects a duplicate fact, while a capture replay ignores it.
Deliberate deviations (all reported here rather than hidden):
tokio-utilCancellationToken(the issue sketch suggested one): the briefforbids new dependencies, so the stop signal is a
tokio::sync::watchchannel.del_<uuid>rather thandel_<nanoid>(nanoidis not adependency).
HindsightClient::confirm_retaintakes a synchronous record closure, whichcannot be driven from the async-only
DatabaseWorker. The worker thereforepersists the operation id with an awaited write and only then calls
poll_operation— the same ordering guarantee, observed from the mock's sideby
a_returned_operation_id_is_persisted_before_the_first_poll.failedingestion settles the rowfailed(withupstream_ingestion_failed): upstream offers no retryability signal, anddocs/technical/hindsight.md's taxonomy treatsfailedas conclusive. Aretry is one operator decision away (
needs_reconciliation), not guessed.of duplicating ~150 lines of validation/fingerprint/conditional-write logic;
the rationale's fact and outbox row commit together in a second transaction
that the calendar result is not part of.
Verification
nix develop --command cargo test— 448 passed, 0 failed across all 20suites (374 before this branch; 18 new in
tests/outbox_tests.rs).nix develop --command cargo clippy --all-targets -- -D warnings— clean.nix develop --command cargo fmt --check— clean.tests/outbox_tests.rscovers: the migration being versioned, idempotent andadditive (version 2 recorded once,
idx_outbox_queuepresent, no supersededindex on a fresh database) and convergent (a pre-
002database gains thequeue index, loses the superseded one, keeps its rows, and is safe to upgrade
again); a mixed
capture with Hindsight down committing the item (mock Radicale
PUT, exit 6,context_delivery: pending, durablependingrow, rationale recorded as auser_reportfact); a context-only capture writing zero calendar items andreturning pending; a replay queueing exactly one delivery and a changed
payload under the same key conflicting; unrequested context reported
not_requestedand the two unowned kinds refused; a broken outbox neverblocking or rewriting a confirmed calendar result; the three milestones
staying distinct across a poll sequence with no second retain ever sent;
the operation id being durable at poll time (read from SQLite by the mock);
a lost acceptance re-submitting the same document under
replace; boundedbackoff honouring
Retry-Afterand not firing early; an authenticationfailure halting delivery with an operator error, zero further POSTs and no
poll; the worker loop returning on a halt; a schema failure halting too;
graceful shutdown preserving rows; and the bounded startup drain reporting its
leftovers while leaving them pending.
life serve+life capture --input -, Hindsight and Radicale both unreachable):{"status":"partial","data":{"context_delivery":"pending","operation_id":"op_smoke_context","operation_state":"applied","parts":[{"kind":"context","state":"pending"}]},"warnings":[{"code":"context_pending",...}]}with exit 6;the daemon log showed
Applied schema migration 002_outbox, the transientverification warning, the bounded startup drain, then retries at 2s/4s/8s/16s
with
dependency_unavailableand no payload text in any line; a replay of thesame operation key answered
retry_wait(the durable row's state) while theSQLite row count stayed 1;
kind: outcomewas refusednot_implemented(exit 1); and shutdown logged
context outbox worker stopped; queued deliveries remain durablewith the row still present.Related Issues
Closes #17
Refs #16 (client, identity and delivery-state model this builds on), #18 (owns
POST /v1/contextandPOST /v1/context/search; the enqueue API above is whatit calls), #28 (
kind: outcome), #29 (kind: correction).Coordinator review — CHANGES REQUESTED (one small item)
I read the diff and ran it on a tree based on current
main. Independent verification on459f6ce:cargo test447 passed / 0 failed (17 new outbox tests),cargo clippy --all-targets -- -D warnings0 errors,cargo fmt --checkclean.What is genuinely good, and I checked each rather than taking the summary:
serve()starts the worker, and afterwith_graceful_shutdownreturns it callsoutbox.shutdown().await— so a shutdown cannot drop a delivery mid-flight, and the log line about queued deliveries staying durable is accurate. A missing journal starts no worker at all ("context deliveries cannot be recorded, so the outbox worker is not started") rather than running one that cannot record.run(stop: watch::Receiver<bool>)treats a dropped sender as shutdown, halts with a reason instead of spinning, and the bounded per-pass budget is checked between operations — the same discipline as #6's recovery pass.POST /v1/captures(kind: context, andtask|event+context);/v1/tasks,/v1/events, update and lifecycle enqueue nothing and keep their behaviour;outcomeandcorrectionare refusednot_implementedrather than half-built;MutationDataunchanged.Change needed: make migration 002 converge, and fix the comment that claims it already does.
BASELINE_SCHEMA_SQL(v1) previously createdoutboxandidx_outbox_state; this change removes both from v1 and has002_outboxcreate the table plusidx_outbox_queue.CREATE TABLE IF NOT EXISTScovers the case you describe, but the index does not: a database created by the previous v1 keepsidx_outbox_stateand gainsidx_outbox_queue, so it ends up with a superseded duplicate index. The doc comment states the opposite — "where the table already exists — records the version and changes nothing" — which is false for that path and would mislead the next person reasoning about migrations.Please make the two paths converge and make the comment true:
002_outbox, after creatingidx_outbox_queue, addDROP INDEX IF EXISTS idx_outbox_state;(a no-op on a fresh database, and it removes the stale duplicate on a pre-existing one).Nothing else changes; keep test/clippy/fmt green. Do not merge.
459f6ce75dtob3cebb43abCoordinator re-review — APPROVED
Verified
b3cebb4myself:002_outboxnow ends withDROP INDEX IF EXISTS idx_outbox_state;after creatingidx_outbox_queue— a no-op on a fresh database and a de-duplication on a pre-existing one, so both shapes converge on one schema.pending, and a second upgrade is a no-op. Confirming it fails without theDROPline is exactly the evidence I want for a migration fix.Independent verification on
b3cebb4:cargo test448 passed / 0 failed (18 outbox tests);cargo clippy --all-targets -- -D warnings0 errors;cargo fmt --checkclean.Merging.
feat(hindsight): durable context outbox and background delivery worker (#17)to feat(hindsight): durable context outbox and background delivery worker