feat(hindsight): durable context outbox and background delivery worker #59

Merged
yuris merged 1 commit from feat/issue-17-outbox into main 2026-09-22 23:44:20 +00:00
Owner

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.md and docs/technical/hindsight.md,
and neither held in code before this change:

  • a calendar write must commit independently of Hindsight — an unavailable
    semantic backend may never roll back, delay or relabel a confirmed change;
  • a delivery must never claim a milestone Life did not observe. Upstream
    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.rs

  • The outbox table is now added by its own versioned migration
    (CURRENT_SCHEMA_VERSION 1 → 2, 002_outbox), transactional and additive
    (CREATE TABLE IF NOT EXISTS + the normative queue index idx_outbox_queue,
    then DROP INDEX IF EXISTS idx_outbox_state). The migration converges both
    database 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_queue and loses the superseded idx_outbox_state, so fresh
    and 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.rs records and reads deliveries through the daemon's single blocking
    DatabaseWorker — never a second connection, never a second worker. Row
    states are pending / accepted / completed / retry_wait /
    needs_reconciliation / failed; DeliveryState from #16 is reused rather
    than duplicated (its not_requested is a caller-facing state, never a row
    state).
  • Document identity is derived, not supplied: NewContextDelivery::hindsight
    computes life-event:<event_id> from the source event, and a row whose stored
    document_id disagrees with its own payload is refused as unusable rather
    than dispatched under either identity. A retry therefore can only address the
    document the first attempt used, under update_mode: replace.
  • enqueue_in_transaction records the delivery inside a transaction the caller
    already 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.
  • The internal API #18 (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.rs

  • One Tokio task owned by the daemon (serve()), 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.
  • A row with a recorded upstream_operation_id is never dispatched again; a
    needs_reconciliation row without one (a lost acceptance) is re-submitted
    under the same document, so uncertainty stays explicit without inventing a
    new identity.
  • Passes are bounded (MAX_OUTBOX_PER_PASS, OUTBOX_DRAIN_BUDGET), so a
    backlog 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 is
    already 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.rs

  • POST /v1/captures (life capture --input -) is the one place a delivery is
    requested. kind: "context" records pure context: zero calendar items, one
    context_captured fact and one outbox row in a single transaction, answered
    partial (exit 6) with context_delivery: "pending" until ingestion is
    confirmed. kind: "task" / "event" with an optional context is an
    actionable 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.
  • The result carries per-part states plus the documented context_delivery
    field and the context_pending / context_unavailable warnings, so a caller
    can tell queued from delivered. A replay reports the state the durable row
    actually holds — the CLI smoke run below shows a replay answering
    retry_wait with the operator detail, not a stale pending.
  • Routes wired: POST /v1/captures (both implemented kinds). Routes
    left:
    POST /v1/tasks, POST /v1/events and the update/lifecycle routes are
    untouched — they carry no semantic-context member, so they neither enqueue nor
    report a delivery, and no delivery is faked anywhere. POST /v1/context and
    POST /v1/context/search remain #18's. MutationData deliberately gained no
    always-not_requested field (per coordinator decision): the field appears
    where a delivery can actually be requested. docs/technical/cli-api.md's
    illustrative task add example that shows context_delivery on a plain
    creation therefore remains aspirational until a creation payload can carry
    context.
  • kind: "outcome" and kind: "correction" are refused with HTTP 501
    not_implemented (they belong to #28/#29) — never half-performed, never a
    silent ignore.
  • A disabled or unconfigured Hindsight queues nothing and reports
    context_delivery: "failed" with context_unavailable, while the calendar
    part 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.rs

  • commit_operation_result now delegates to
    commit_operation_result_with_delivery, so a capture can record its
    operation, fact and delivery atomically; the existing call sites and their
    behaviour are unchanged. The life_events insert is shared through one
    append_event helper with an explicit conflict policy: the normal commit path
    still rejects a duplicate fact, while a capture replay ignores it.

Deliberate deviations (all reported here rather than hidden):

  • No tokio-util CancellationToken (the issue sketch suggested one): the brief
    forbids new dependencies, so the stop signal is a tokio::sync::watch channel.
  • Delivery ids are del_<uuid> rather than del_<nanoid> (nanoid is not a
    dependency).
  • HindsightClient::confirm_retain takes a synchronous record closure, which
    cannot be driven from the async-only DatabaseWorker. The worker therefore
    persists the operation id with an awaited write and only then calls
    poll_operation — the same ordering guarantee, observed from the mock's side
    by a_returned_operation_id_is_persisted_before_the_first_poll.
  • An upstream failed ingestion settles the row failed (with
    upstream_ingestion_failed): upstream offers no retryability signal, and
    docs/technical/hindsight.md's taxonomy treats failed as conclusive. A
    retry is one operator decision away (needs_reconciliation), not guessed.
  • The capture's actionable part composes the existing creation handler instead
    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 20
    suites (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.rs covers: the migration being versioned, idempotent and
    additive (version 2 recorded once, idx_outbox_queue present, no superseded
    index on a fresh database) and convergent (a pre-002 database gains the
    queue 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, durable pending row, rationale recorded as a
    user_report fact); a context-only capture writing zero calendar items and
    returning pending; a replay queueing exactly one delivery and a changed
    payload under the same key conflicting; unrequested context reported
    not_requested and the two unowned kinds refused; a broken outbox never
    blocking 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; bounded
    backoff honouring Retry-After and not firing early; an authentication
    failure 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.
  • Manual end-to-end smoke run against the real binary (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 transient
    verification warning, the bounded startup drain, then retries at 2s/4s/8s/16s
    with dependency_unavailable and no payload text in any line; a replay of the
    same operation key answered retry_wait (the durable row's state) while the
    SQLite row count stayed 1; kind: outcome was refused not_implemented
    (exit 1); and shutdown logged context outbox worker stopped; queued deliveries remain durable with the row still present.

Closes #17
Refs #16 (client, identity and delivery-state model this builds on), #18 (owns
POST /v1/context and POST /v1/context/search; the enqueue API above is what
it calls), #28 (kind: outcome), #29 (kind: correction).

## 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.md` and `docs/technical/hindsight.md`, and neither held in code before this change: * a calendar write must commit independently of Hindsight — an unavailable semantic backend may never roll back, delay or relabel a confirmed change; * a delivery must never claim a milestone Life did not observe. Upstream 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.rs` * The `outbox` table is now added by its own versioned migration (`CURRENT_SCHEMA_VERSION` 1 → 2, `002_outbox`), transactional and additive (`CREATE TABLE IF NOT EXISTS` + the normative queue index `idx_outbox_queue`, then `DROP INDEX IF EXISTS idx_outbox_state`). The migration converges both database 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_queue` and loses the superseded `idx_outbox_state`, so fresh and 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.rs` records and reads deliveries through the daemon's single blocking `DatabaseWorker` — never a second connection, never a second worker. Row states are `pending` / `accepted` / `completed` / `retry_wait` / `needs_reconciliation` / `failed`; `DeliveryState` from #16 is reused rather than duplicated (its `not_requested` is a caller-facing state, never a row state). * Document identity is derived, not supplied: `NewContextDelivery::hindsight` computes `life-event:<event_id>` from the source event, and a row whose stored `document_id` disagrees with its own payload is refused as unusable rather than dispatched under either identity. A retry therefore can only address the document the first attempt used, under `update_mode: replace`. * `enqueue_in_transaction` records the delivery inside a transaction the caller already 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. * The internal API #18 (`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.rs` * One Tokio task owned by the daemon (`serve()`), 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. * A row with a recorded `upstream_operation_id` is never dispatched again; a `needs_reconciliation` row *without* one (a lost acceptance) is re-submitted under the same document, so uncertainty stays explicit without inventing a new identity. * Passes are bounded (`MAX_OUTBOX_PER_PASS`, `OUTBOX_DRAIN_BUDGET`), so a backlog 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 is already 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.rs` * `POST /v1/captures` (`life capture --input -`) is the one place a delivery is requested. `kind: "context"` records pure context: zero calendar items, one `context_captured` fact and one outbox row in a single transaction, answered `partial` (exit 6) with `context_delivery: "pending"` until ingestion is confirmed. `kind: "task"` / `"event"` with an optional `context` is an actionable 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. * The result carries per-part states plus the documented `context_delivery` field and the `context_pending` / `context_unavailable` warnings, so a caller can tell queued from delivered. A replay reports the state the durable row actually holds — the CLI smoke run below shows a replay answering `retry_wait` with the operator detail, not a stale `pending`. * **Routes wired:** `POST /v1/captures` (both implemented kinds). **Routes left:** `POST /v1/tasks`, `POST /v1/events` and the update/lifecycle routes are untouched — they carry no semantic-context member, so they neither enqueue nor report a delivery, and no delivery is faked anywhere. `POST /v1/context` and `POST /v1/context/search` remain #18's. `MutationData` deliberately gained no always-`not_requested` field (per coordinator decision): the field appears where a delivery can actually be requested. `docs/technical/cli-api.md`'s illustrative `task add` example that shows `context_delivery` on a plain creation therefore remains aspirational until a creation payload can carry context. * `kind: "outcome"` and `kind: "correction"` are refused with HTTP 501 `not_implemented` (they belong to #28/#29) — never half-performed, never a silent ignore. * A disabled or unconfigured Hindsight queues nothing and reports `context_delivery: "failed"` with `context_unavailable`, while the calendar part 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.rs` * `commit_operation_result` now delegates to `commit_operation_result_with_delivery`, so a capture can record its operation, fact and delivery atomically; the existing call sites and their behaviour are unchanged. The `life_events` insert is shared through one `append_event` helper with an explicit conflict policy: the normal commit path still rejects a duplicate fact, while a capture replay ignores it. **Deliberate deviations** (all reported here rather than hidden): * No `tokio-util` `CancellationToken` (the issue sketch suggested one): the brief forbids new dependencies, so the stop signal is a `tokio::sync::watch` channel. * Delivery ids are `del_<uuid>` rather than `del_<nanoid>` (`nanoid` is not a dependency). * `HindsightClient::confirm_retain` takes a *synchronous* record closure, which cannot be driven from the async-only `DatabaseWorker`. The worker therefore persists the operation id with an awaited write and only then calls `poll_operation` — the same ordering guarantee, observed from the mock's side by `a_returned_operation_id_is_persisted_before_the_first_poll`. * An upstream `failed` ingestion settles the row `failed` (with `upstream_ingestion_failed`): upstream offers no retryability signal, and `docs/technical/hindsight.md`'s taxonomy treats `failed` as conclusive. A retry is one operator decision away (`needs_reconciliation`), not guessed. * The capture's actionable part composes the existing creation handler instead 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 20 suites (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.rs` covers: the migration being versioned, idempotent and additive (version 2 recorded once, `idx_outbox_queue` present, no superseded index on a fresh database) and convergent (a pre-`002` database gains the queue 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`, durable `pending` row, rationale recorded as a `user_report` fact); a context-only capture writing zero calendar items and returning pending; a replay queueing exactly one delivery and a changed payload under the same key conflicting; unrequested context reported `not_requested` and the two unowned kinds refused; a broken outbox never blocking 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`; bounded backoff honouring `Retry-After` and not firing early; an authentication failure 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. * Manual end-to-end smoke run against the real binary (`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 transient verification warning, the bounded startup drain, then retries at 2s/4s/8s/16s with `dependency_unavailable` and no payload text in any line; a replay of the same operation key answered `retry_wait` (the durable row's state) while the SQLite row count stayed 1; `kind: outcome` was refused `not_implemented` (exit 1); and shutdown logged `context outbox worker stopped; queued deliveries remain durable` with the row still present. ## Related Issues Closes #17 Refs #16 (client, identity and delivery-state model this builds on), #18 (owns `POST /v1/context` and `POST /v1/context/search`; the enqueue API above is what it calls), #28 (`kind: outcome`), #29 (`kind: correction`).
Durable context delivery, decoupled from every calendar result.

The `outbox` table is added by its own versioned, idempotent migration
(`002_outbox`, additive and `IF NOT EXISTS`, so a database that already has
the table records the version and changes nothing). A row keeps the three
milestones apart — `pending`, `accepted` (upstream queued the retain and its
operation is durable) and `completed` (ingestion settled upstream) — beside
`retry_wait`, `failed` and `needs_reconciliation`, and never collapses
accepted into completed.

`src/hindsight/outbox.rs` records deliveries through the daemon's single
blocking database worker: the document identity is derived from the source
event (`life-event:<event_id>`), so a retry can only ever address the same
document under `update_mode: replace`, and a row whose stored identity
contradicts its own payload is refused rather than dispatched.

`src/hindsight/worker.rs` is one Tokio task that dispatches rows that were
never accepted, persists a returned operation id *before* the first poll of
it, polls the known operation instead of resubmitting while it is active,
honours the server's `Retry-After` within a bounded exponential step, and
halts — without spinning — on authentication or schema failures, leaving an
operator-visible error on the row. Shutdown stops between operations and
leaves every durable row as its last committed transition wrote it.

`POST /v1/captures` is the one place a delivery is requested: `kind:
context` records pure context with no calendar item and answers partial
(exit 6) until delivery is confirmed, and `kind: task|event` with an
optional `context` commits the calendar part through the creation path
exactly as before and reports the rationale's delivery state beside it.
`kind: outcome` and `kind: correction` are refused explicitly, and an
unavailable Hindsight never touches a calendar result.

`serve()` now verifies the deployment, drains what is already due within a
bounded budget, and keeps draining in the background.

Closes #17
Author
Owner

Coordinator review — CHANGES REQUESTED (one small item)

I read the diff and ran it on a tree based on current main. Independent verification on 459f6ce: cargo test 447 passed / 0 failed (17 new outbox tests), cargo clippy --all-targets -- -D warnings 0 errors, cargo fmt --check clean.

What is genuinely good, and I checked each rather than taking the summary:

  • Lifecycle is awaited, not abandoned. serve() starts the worker, and after with_graceful_shutdown returns it calls outbox.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.
  • A row whose stored identity contradicts its own payload is refused rather than dispatched under either identity — that is the concrete form of "never a fresh document id".
  • Scope held exactly as directed: enqueue happens only on POST /v1/captures (kind: context, and task|event + context); /v1/tasks, /v1/events, update and lifecycle enqueue nothing and keep their behaviour; outcome and correction are refused not_implemented rather than half-built; MutationData unchanged.

Change needed: make migration 002 converge, and fix the comment that claims it already does.

BASELINE_SCHEMA_SQL (v1) previously created outbox and idx_outbox_state; this change removes both from v1 and has 002_outbox create the table plus idx_outbox_queue. CREATE TABLE IF NOT EXISTS covers the case you describe, but the index does not: a database created by the previous v1 keeps idx_outbox_state and gains idx_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:

  1. In 002_outbox, after creating idx_outbox_queue, add DROP INDEX IF EXISTS idx_outbox_state; (a no-op on a fresh database, and it removes the stale duplicate on a pre-existing one).
  2. Reword the comment to say what actually happens: a database that already holds the table gains the queue index and loses the superseded one, so both fresh and upgraded databases end in the same shape; existing rows are untouched.
  3. Extend the migration test to assert the index set converges (or at minimum that 002 is re-runnable after the drop), so the claim is enforced rather than described.

Nothing else changes; keep test/clippy/fmt green. Do not merge.

## Coordinator review — CHANGES REQUESTED (one small item) I read the diff and ran it on a tree based on current `main`. Independent verification on `459f6ce`: `cargo test` **447 passed / 0 failed** (17 new outbox tests), `cargo clippy --all-targets -- -D warnings` 0 errors, `cargo fmt --check` clean. What is genuinely good, and I checked each rather than taking the summary: - **Lifecycle is awaited, not abandoned.** `serve()` starts the worker, and after `with_graceful_shutdown` returns it calls `outbox.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. - **A row whose stored identity contradicts its own payload is refused** rather than dispatched under either identity — that is the concrete form of "never a fresh document id". - **Scope held exactly as directed:** enqueue happens only on `POST /v1/captures` (`kind: context`, and `task|event` + `context`); `/v1/tasks`, `/v1/events`, update and lifecycle enqueue nothing and keep their behaviour; `outcome` and `correction` are refused `not_implemented` rather than half-built; `MutationData` unchanged. **Change needed: make migration 002 converge, and fix the comment that claims it already does.** `BASELINE_SCHEMA_SQL` (v1) previously created `outbox` and `idx_outbox_state`; this change removes both from v1 and has `002_outbox` create the table plus `idx_outbox_queue`. `CREATE TABLE IF NOT EXISTS` covers the case you describe, but the index does not: a database created by the previous v1 keeps `idx_outbox_state` **and** gains `idx_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: 1. In `002_outbox`, after creating `idx_outbox_queue`, add `DROP INDEX IF EXISTS idx_outbox_state;` (a no-op on a fresh database, and it removes the stale duplicate on a pre-existing one). 2. Reword the comment to say what actually happens: a database that already holds the table gains the queue index and loses the superseded one, so both fresh and upgraded databases end in the same shape; existing rows are untouched. 3. Extend the migration test to assert the index set converges (or at minimum that 002 is re-runnable after the drop), so the claim is enforced rather than described. Nothing else changes; keep test/clippy/fmt green. Do not merge.
yuris force-pushed feat/issue-17-outbox from 459f6ce75d to b3cebb43ab 2026-09-22 23:43:15 +00:00 Compare
Author
Owner

Coordinator re-review — APPROVED

Verified b3cebb4 myself:

  • 002_outbox now ends with DROP INDEX IF EXISTS idx_outbox_state; after creating idx_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.
  • The comment now describes what actually happens on each path instead of claiming the migration "changes nothing", and it records that both migrations stay recorded and existing rows are untouched.
  • The new test is the right kind of regression test, and you verified it defends the fix rather than merely passing: it seeds a delivery, reconstructs the pre-002 shape, upgrades, then asserts the queue index is present, the superseded one is gone, the index set equals a fresh database's, the row is still pending, and a second upgrade is a no-op. Confirming it fails without the DROP line is exactly the evidence I want for a migration fix.

Independent verification on b3cebb4: cargo test 448 passed / 0 failed (18 outbox tests); cargo clippy --all-targets -- -D warnings 0 errors; cargo fmt --check clean.

Merging.

## Coordinator re-review — APPROVED Verified `b3cebb4` myself: - `002_outbox` now ends with `DROP INDEX IF EXISTS idx_outbox_state;` after creating `idx_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. - The comment now describes what actually happens on each path instead of claiming the migration "changes nothing", and it records that both migrations stay recorded and existing rows are untouched. - The new test is the right kind of regression test, and you verified it defends the fix rather than merely passing: it seeds a delivery, reconstructs the pre-002 shape, upgrades, then asserts the queue index is present, the superseded one is gone, the index set equals a fresh database's, the row is still `pending`, and a second upgrade is a no-op. Confirming it fails without the `DROP` line is exactly the evidence I want for a migration fix. Independent verification on `b3cebb4`: `cargo test` **448 passed / 0 failed** (18 outbox tests); `cargo clippy --all-targets -- -D warnings` 0 errors; `cargo fmt --check` clean. Merging.
yuris merged commit 3554c12eae into main 2026-09-22 23:44:20 +00:00
yuris changed title from feat(hindsight): durable context outbox and background delivery worker (#17) to feat(hindsight): durable context outbox and background delivery worker 2026-09-23 05:12:04 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
yuris/life-service!59
No description provided.