Skip to content

Record recovery runner fetches, races and changes in the v5 audit log - #3399

Open
erskingardner wants to merge 8 commits into
masterfrom
t3/recovery-runner-audit-rows
Open

erskingardner wants to merge 8 commits into
masterfrom
t3/recovery-runner-audit-rows

Conversation

@erskingardner

@erskingardner erskingardner commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Problem

The recovery runner leaves no audit trail. When a busy group's history keeps showing as incomplete, an exported audit log can't say whether the runner kept trying to catch it up, what it thought it owed, or when it gave up. Diagnosing that today means guessing from notices and logs.

Fix

Three v5 operational row kinds record the runner's work per group. recovery_runner_fetched writes one row per fetch it settles: what was owed, why it fetched, how the relay call ended, and what it covered, fetched and staged. recovery_runner_raced records begins that found nothing owed, at most one row per route per minute. recovery_runner_changed records unit breaks, failed probes, re-arms, suspensions, retired relays and parked remainders. Rows are built only while v5 recording is on, each is written where the change it records is stored (so a retried step never repeats one), and the runner's behavior is unchanged.

Verification

  • Local: cargo fmt --all; cargo clippy -p marmot-app -p marmot-forensics -p storage-sqlite --all-targets --all-features --locked -- -D warnings
  • Local: cargo nextest run -p marmot-app -E 'test(recovery) | test(audit)' (419 passed); -p storage-sqlite recovery units (1438 passed); -p marmot-forensics (169 passed)
  • Local: end-to-end smoke (app_runtime_journeys/process_orchestrator and wn-cli cli, 4 passed)
  • Review: Codex and Cursor recommend merge at 3989b67

Part of mdk#2660 phase 4.

Merge held: the White Noise audit receiver validates v5 rows strictly, so this waits for confirmation that it accepts the three new kinds before it is enqueued.

retire_gone_recovery_units now counts a relay already retired as gone as nothing on a retry, so a settled route change never repeats its row. events_fetched counts the events in every relay answer the runner reads; a second the relay cannot prove exposes no events, only bytes.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added detailed recovery-runner audit records for fetches, races, and state changes, including progress, outcomes, and stop or retry reasons.
    • Added visibility into re-armed recovery work, route suspensions, and parked or retired recovery items.
  • Bug Fixes
    • Corrected retired-unit counts during repeated recovery settlements, preventing already-retired units from being counted again while allowing newly owed time to be recorded.
  • Documentation
    • Expanded audit-log and recovery documentation to describe the new records and their coverage.

erskingardner and others added 8 commits November 13, 2026 18:25
The recovery runner left no audit trail. That made it hard to tell whether a group kept trying to catch up, what
it believed it owed, or when it stopped trying. Three v5 operational kinds now record this:

- recovery_runner_fetched: one row per fetch the runner settles, with the owed window, purpose, outcome, how the
  relay call ended, and what it covered, fetched and staged.
- recovery_runner_raced: begins that found nothing owed, coalesced to at most one row per route per minute.
- recovery_runner_changed: unit breaks, failed probes and re-arms, route suspends and resumes, relays retired or
  reopened, and remainders the retention floor parks or widens.

Rows are built only when v5 auditing is on. Surveys write none, and the runner's behavior is unchanged.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
After #3379, a broken unit is retried by a timed probe, and the coverage ledger raises no notices. The audit-row
docs still described a unit's "one probe", a remainder that "only a dismissal clears", and rows joining the
route's live notice; they now describe timed probes, remainders that a rejoin or the route ending clears, and joins
to notice rows older producers wrote. ledger_route_tag loses the dead-code allow #3379 left for these rows.

The break-and-park audit test now follows the unit to its timed probe: nothing is written while it waits, then one
probe fetch row and one probe_failed change under the same park.

The retention-prune audit test raced the account's live inbox subscription: when the mock relay answered first,
the window was already covered and the prune parked nothing. It now points the account at a relay that never
answers.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
A recovery_runner_fetched row whose window spans every second (since 0, until u64::MAX) overflowed the
"no more than its window" check: decoding panicked with overflow checks on and wrongly rejected the row without
them. The check now compares owed_secs - 1 with the window's span, which cannot overflow once owed_secs is
positive. A test writes and strictly decodes a whole-clock row.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…t path

A relay that broke while required and is later best effort keeps its unit row, and a timed probe can arm it.
Its fetch rows then claimed both probe and best_effort, which the v5 writer rejects, so every later fetch on that
relay went unrecorded. A best-effort fetch now never reports a probe.

A raced begin that closed its route's window awaited a worker turn only to write the coalesced row, while the
runner held the account's recovery credit and even with v5 recording off. The row now waits for the next survey
turn, which every step takes anyway.

The broken and probe_failed rows in audit-logging.md describe the timed probe instead of the retired notice.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
#3299 moved the runner. The audit rows test imports and the audit-logging.md producer link now use the new path.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
- A slice staged after its coverage mark went stale commits its events but records no coverage. Both stage helpers
  dropped that count, and the sweep and comparison stats count only slices the sink accepted, so the fetch row said
  nothing was fetched or staged. The sinks now count every event the relay returned, and the stage helpers return
  the staged count with or without coverage.
- A route settlement retried after a later write failed retired the same gone units again, and the
  retired_gone row was written twice. retire_gone_recovery_units now counts a unit only when it newly retires it as
  route gone; a retry still retires the newly owed seconds.
- Queued raced rows are drained inside the survey turn, so a turn whose preparation fails before its closure leaves
  them for the retry. The test host can fail a debt turn's preparation to show it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ed events at the relay

A survey turn wrote its suspended and resumed rows inside its closure, but the
runner stored the suspended set only once the turn answered. When the host's
fence read failed after the closure, the next survey wrote the same rows again.
The survey now stores the set inside its turn, where it writes the rows, as it
already drains the queued raced rows. The debt turn's audit hook documents the
rule: a row is written where what it records is committed or stored.

events_fetched counted only events the sweep handed to its sink, so a capped
answer the sweep asked again in halves, down to a second the relay could not
prove, read as nothing fetched. The sweep's metered relay and a counting
wrapper around the comparison relay now count the events in every answer the
runner reads.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
While the recorder keeps no v5 rows, the survey still stored the routes it
found suspended, so a route already suspended when recording started was never
recorded. The survey now forgets them while recording is off, and drops the
queued raced rows as the recorder would, so the first survey once it records
writes every suspended route.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Walkthrough

Adds v5 audit events for recovery-runner fetches, coalesced raced begins, and state changes. The runner records group-aware events and fetch counts. The v5 schema and validation cover the new event types. SQLite retirement counts now exclude units already retired as route gone.

Changes

Recovery Runner Audit Events

Layer / File(s) Summary
Define v5 recovery audit events
crates/marmot-forensics/src/audit/recovery.rs, crates/marmot-forensics/src/audit.rs, crates/marmot-forensics/schema/audit-log-event.v5.schema.json, crates/marmot-forensics/src/v5/*, crates/marmot-forensics/src/lib.rs, crates/marmot-forensics/AGENTS.md, crates/marmot-app/CHANGELOG.md, docs/marmot-architecture/*
Adds the recovery-runner event vocabulary, schema definitions, writer validation, and public exports. Tests and documentation describe the event fields and recording cadence.
Record fetch quantum outcomes
crates/marmot-app/src/client/audit.rs, crates/marmot-app/src/client/history_recovery/recovery_runner.rs, crates/marmot-app/src/client/history_recovery/recovery_runner/*, crates/marmot-app/src/client/history_recovery/recovery_runner/tests.rs, crates/marmot-app/src/client/history_recovery/recovery_runner/tests/audit_rows.rs, crates/marmot-app/src/history_notices.rs
Adds group-aware audit hooks and records fetch outcomes, timing, coverage, and event counts. Tests cover fetch rows, including timeout and stale-coverage cases.
Record route and unit transitions
crates/marmot-app/src/client/history_recovery/recovery_coverage.rs, crates/marmot-app/src/client/history_recovery/recovery_runner.rs, crates/marmot-app/src/client/history_recovery/recovery_runner/*, crates/marmot-app/src/client/history_recovery/recovery_runner/host/tests.rs, crates/marmot-app/src/client/history_recovery/recovery_runner/tests/audit_rows.rs, crates/storage-sqlite/src/app/recovery_units.rs, crates/storage-sqlite/src/app/recovery_units/tests.rs, crates/storage-sqlite/CHANGELOG.md, crates/marmot-app/AGENTS.md
Adds audit rows for unit and route changes, suspension and resumption, coalesced races, and retention remainders. SQLite counts a route-gone unit only when it is newly retired. Tests cover these transitions and repeated retirement.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: Minimal · up to 3989b

This change adds forensic audit rows for recovery-runner fetches, races and state changes, and leaves runner behavior unchanged. No concrete merge-blocking defect was established. Keep the author's hold until the audit receiver is confirmed to accept the three new row kinds.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "Record recovery runner fetches, races and changes in the v5 audit log" matches the main change. The changeset adds v5 audit rows for recovery-runner fetches (recovery_runner_fetched), races…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "Record recovery runner fetches, races and changes in the v5 audit log" matches the main change. The changeset adds v5 audit rows for recovery-runner fetches (recovery_runner_fetched), races (recovery_runner_raced), and state changes (recovery_runner_changed), along with the enum vocabularies, schema, validation, and tests. The PR objectives state the same purpose.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional comments: 1

crates/marmot-app/src/client/history_recovery/recovery_runner/audit.rs-436-436: 1

436-436: Minor · Performance & Scalability · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Bound and reclaim open race windows.

If a route races once and then retires, this entry remains until the runner stops. Neither raced nor take removes it without more work on that same route. Route churn therefore grows Races.open for the process lifetime, including while recording is disabled.

Prune entries for routes removed from the survey and enforce a size or expiry bound.

Review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 683909a7-6643-4648-a73e-bc2ae83df65b
Commits

Reviewing files that changed from the base of the PR and between a5ede93 and 3989b67.

Files selected for processing: 30

  • crates/marmot-app/AGENTS.md
  • crates/marmot-app/CHANGELOG.md
  • crates/marmot-app/src/client/audit.rs
  • crates/marmot-app/src/client/history_recovery/recovery_coverage.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/audit.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/compare.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/host.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/host/tests.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/quantum.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/recheck.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/repair.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/select.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/tails.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/tests.rs
  • crates/marmot-app/src/client/history_recovery/recovery_runner/tests/audit_rows.rs
  • crates/marmot-app/src/history_notices.rs
  • crates/marmot-forensics/AGENTS.md
  • crates/marmot-forensics/schema/audit-log-event.v5.schema.json
  • crates/marmot-forensics/src/audit.rs
  • crates/marmot-forensics/src/audit/recovery.rs
  • crates/marmot-forensics/src/audit/tests.rs
  • crates/marmot-forensics/src/lib.rs
  • crates/marmot-forensics/src/v5/operational.rs
  • crates/marmot-forensics/src/v5/validation.rs
  • crates/storage-sqlite/CHANGELOG.md
  • crates/storage-sqlite/src/app/recovery_units.rs
  • crates/storage-sqlite/src/app/recovery_units/tests.rs
  • docs/marmot-architecture/audit-logging.md
  • docs/marmot-architecture/further-context/account-recovery.md
Files with no reviewable changes: 1

  • crates/marmot-app/src/history_notices.rs

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant