feat(background-agent): enforce durable admission#395
Conversation
34af9b4 to
5dbd9d1
Compare
Rebase + Blacksmith testbox gateRebased Blacksmith build gate — run 30117708774:
Review note (follow-up, not blocking this PR)
Consequence: a run admitted (status Suggested follow-up (own PR, keeps this diff scoped): terminalize unclaimed runs whose persisted |
e89c980 to
405b5b8
Compare
Three review defects in the durable-admission change. 1. Stop persisting recoverable identity plaintext (security regression). Admission wrote `idempotency_key` and `auth_profile_ref` through a reversible `codewith-opaque-v1:` hex envelope, chosen so the state redactor and the local-state secrets doctor would skip those cells. A credential-shaped value therefore persisted, survived the doctor, and printed in `agent list --json` — weaker than the merge base, which bound both through `redact_state_string`. Idempotency keys are now stored only as a one-way SHA-256 digest; dedupe, replay, and immutable-identity comparisons run off the digest, preserving byte-exact matching without keeping plaintext. Auth-profile aliases must survive so the worker can load the same profile, so they go back through state redaction. Nothing decodes either column on read. `agent start` also now refuses an `--auth-profile` alias that config resolution did not select, so an unvalidated alias cannot reach admission and fail later mid-run. 2. Reclaim admission capacity after a version bump. A claim requires the persisted admission schema and execution-snapshot `packageFingerprint` to equal the running binary, but `orphan_stale_background_agent_runs` only touches rows with a supervisor, while queued/orphaned rows still count towards `DEFAULT_MAX_ACTIVE_BACKGROUND_AGENT_RUNS`. After an upgrade those rows were unclaimable forever yet kept burning slots until every admission failed with capacity exceeded and nothing self-healed. `terminalize_incompatible_background_agent_runs` now fails such rows closed with an explicit reason plus a lifecycle receipt, and runs before every admission and on each supervisor reconcile. 3. Recover from an upgrade with the old daemon still alive. `ensure_daemon_record_compatible` bailed on any mismatch and gated both `start()` and `status()`, so replacing the binary while the previous daemon ran broke `agent start` outright. `start()` now stops and replaces the mismatched daemon (still never reusing it), and read-only `status()` reports `incompatibleRunning` with the exact remediation command instead of erroring. Adds regression tests for all three and corrects ARCHITECTURE.md, which claimed state redaction ran before persistence while the code bypassed it.
prompt_snapshot_ref was bound raw by the durable-admission insert while the merge base wrote it through redact_state_string, so an admission whose prompt ref carried credential-shaped material stored it recoverably. Named-column fixes have now missed a sibling column twice, so route every caller-supplied string through one projection instead: - RedactedBackgroundAgentRunColumns produces every string column that admission writes to background_agent_runs, and the idempotent-replay identity comparison reads the same projection, so write and compare cannot drift. prompt_snapshot_ref is redacted (not digested) because the CLI, TUI, and app-server surface it on read and replay matches it verbatim. - RedactedBackgroundAgentExecutionSnapshotColumns does the same for the execution-snapshot columns, covering the raw recovery_policy and config_fingerprint bindings in both the in-tx and standalone inserts. - The thread-binding update redacts thread_id, thread_store_kind, thread_store_id, and rollout_path for the same reason. The new regression test asserts the invariant generically rather than per column: it admits a run whose every caller-supplied string carries the same secret-shaped token, then scans every cell of every background_agent* table for that token. Against the previous head it fails with 12 leaking columns (including prompt_snapshot_ref); it passes here. ARCHITECTURE.md now states the invariant and classifies every persisted column as digested, redacted, or provably non-secret.
405b5b8 to
ae44923
Compare
Round 3 — plaintext leak closed structurally + full column sweepHead: The named-column fix, and why it is no longer named-column
Since a named-column fix has now missed a sibling twice, the invariant is enforced in one place instead:
Column sweep — every write in the background-agent state path
† = raw before this commit; 12 of them were leaking. Generic regression test
Defects 2 and 3 (unchanged from round 2, re-verified green on this head)
|
Pure rustfmt output for the redacted column projection helpers and the all-cells plaintext test; no behaviour change. Fixes the failing `cargo fmt --all -- --check` gate on this branch.
1 participant
Why
Background-agent starts could reuse incompatible daemons, silently change requested auth profiles, race idempotent creation against quota checks, and duplicate lifecycle evidence after lost acknowledgements or restart recovery. Those gaps could start the wrong run, exceed capacity, or let stale supervisors mutate a reclaimed generation.
What changed
BEGIN IMMEDIATEcreate-or-adopt admission transaction with identity validation and live/recoverable capacity accounting.codex-rs/background-agent/ARCHITECTURE.md.Verification
git diff --checkTask: E-00101
Acceptance scope:
codewith-durable-agent-admission-v1