# Review brief: the implementation plans for `carlos db` region forward (rounds 1 and 2 answered)

You are an independent second reader of three implementation plans for the
CARLOS platform (this repository). Round 1
(`CODEX-FINDINGS-DB-REGION-FORWARD-PLANS.md`) found 24 findings (8 P0, 4 P1,
12 P2) and asked for all three plans to be revised. This round checks the
revisions. Nothing is built.

**Plans**, at the commit named in the prompt:

- `docs/superpowers/plans/2026-10-22-db-region-forward-plan-1-box.md`: the
  box side and the wire format, shipped dark behind the `db-read-1`
  capability (17 tasks);
- `docs/superpowers/plans/2026-10-22-db-region-forward-plan-2-query.md`: the
  console and the CLI for `query` and `query --at` (14 tasks);
- `docs/superpowers/plans/2026-10-22-db-region-forward-plan-3-pull.md`:
  `pull` and `pull --at` (10 tasks).

Each plan now opens with a "Plan review, round 1" section saying where each
of its findings landed.

**Spec:** `docs/superpowers/specs/2026-10-22-db-region-forward-design.md`,
rev 6. Rev 6 is rev 5 (accepted in round 5, approved by Paul on 2026-10-22,
rulings in §16) plus four clarifications the plan review forced, each marked
**(plan review N)**: §5.4 (what a result names when a chunk cannot be
confirmed), §5.5 steps 3 and 4 and §5.8 (the audit row first, `cancel` only
once a request has ended, and a box's `cancel` read before it elects), §5.7
(a retry's reservation), §9.3 (the audit row's order). No mechanism changed.
Read `AGENTS.md`, then the spec's status header and the four marked places,
then the plans.

## Fixed: do not re-argue

- **The spec's design**, rev 6 included. Report a plan that fails to build
  what the spec says, or builds something the spec rules out.
- **Paul's rulings (spec §16).** Plans 4 and 5 are listed, not written.
- **The repository's rules** each plan carries as Global Constraints.
- **Round 1's findings as disposed below**, unless a fix is wrong or
  incomplete: then say so as a new finding, naming the round 1 number.

## Round 1 dispositions

| # | Sev | Plan, task | Disposition |
|---|---|---|---|
| 1 | P0 | 1: 7, 12, 13 | Fixed. `SelfView` reads `/proc/self/net/dev` (`parseNetDev`); `net.Interfaces` needs `AF_NETLINK`, refused by `RestrictAddressFamilies=AF_UNIX`. Unobserved facts go in `HelloReply.Errors`; the probe fails on any. Tests `TestParseNetDev`, the probe's "an observation that failed". |
| 2 | P0 | 1: 15; 2: 7 | Fixed. `waitFor` (`internal/host/housekeeper_test.go:139`) became `dbreadWaitFor`; `hookStore` (`internal/console/temphook_test.go:16`) became `dbReadHookStore`. Checked with `git grep` on `origin/main`. |
| 3 | P0 | 1: 15 | Fixed. The payload prints one `DBREAD-REPORT ` line; the parent scans for it. |
| 4 | P0 | 1: 15 | Fixed. `dbreadServiceFile` is `/etc/systemd/system/carlos-dbread@.service`, errors checked, restored in cleanup through `mustSystemctl`. |
| 5 | P0 | 1: 17 (2, 3: census) | Fixed. `TestZZDBForwardFingerprints` computes each `@legacy` line from the row's seven fields; plans 2 and 3 use it. |
| 6 | P0 | 2: 4 to 6 | Fixed. `putFixtureObject` reads the etag and replaces with it; `putEdgesFile` and `putBoxStatus` use it. |
| 7 | P0 | 2: 6 | Fixed. `TestDBReadAdmissionBounds` moved to Task 7, where the stream holds requests open. |
| 8 | P0 | 3: 4 | Fixed. The installed-unit pull test is Task 5 Step 5, after the agent's pull; `submit` already takes the op (plan 1). |
| 9 | P1 | 1: 11 | Fixed. `flushLocked` advances `seq` only after the create is confirmed. Test `TestResultNamesOnlyConfirmedChunks` (a chunk store that never confirms; result `failed`/`interrupted`, `last_seq` 1). Spec §5.4 rev 6. |
| 10 | P1 | 1: 9 to 11 | Fixed. `admission.retry` keeps the first attempt's reservation under `id#attempt`; `settle` waits for the unit count before the retry dials. Tests `TestRetryWaitsForTheFirstWorkerToStop` (a lingering unit, space for one), `TestAdmissionRetryHoldsTheOldReservation`. Spec §5.7 rev 6. |
| 11 | P1 | 2: 6 (and 1: 12) | Fixed in part. The audit row is written first, while the request is open and before any box can see the read; after the queue entry, an ended request writes `cancel` and returns, with no serial bump; the bump runs on the request's context (`announceDBRead`, not the detached `bumpSerial`). Boxes read `cancel` before electing. Tests `TestDBReadEndedDuringAdmissionWritesOnlyTheCancel`, `TestPassSkipsACancelledRequest`. Spec §5.5 and §9.3 rev 6. Declined: the audit insert's own cancellation (below). |
| 12 | P1 | 2: 7 | Fixed. Phase deadline contexts; every store call through `dbfCall` (5 s, wait lines while it blocks); the bound checked before the result line; write deadlines clamped to the bound plus 10 s. Tests `TestDBReadStalledStoreKeepsTheStreamAndItsBound`, `TestDBReadSlowRelayEndsAtTheBound`. |
| 13 | P2 | 2: 6 | Fixed. Read deadline `preHeader` until admission, then the read's bound. Test `TestDBReadStalledBodyIsCutAtThePreHeaderBound`. |
| 14 | P2 | 1: 10, 12, 13 | Fixed. `AgentConfig.Draining`, from the box's own edges record, checked before admission and after the jitter. Test `TestADrainingBoxDoesNotPick`. Nothing on `main` sets `Draining` yet; the plan says so. |
| 15 | P2 | 1: 11 | Fixed. Every pre-ready transport failure asks for a probe. Tests `TestAWorkerThatNeverStartsAsksForAProbe`, T13's fail-start case through the installed unit. |
| 16 | P2 | 1: 15 | Fixed. T12 submits the second read once the first's unit is running. |
| 17 | P2 | 1: 15, 17 | Fixed. The witness reports the interfaces the unit sees and fails on any but loopback (removing `PrivateNetwork` turns it red); ATTACH targets a real SQLite file. |
| 18 | P2 | 2: 6, 7 | Fixed. `TestDBReadAdmissionBounds`: both slots freed before pickup and again after, the seventeenth read across nine people, a restart under open reads that writes nothing and starts with empty counts. |
| 19 | P2 | 2: 12 | Fixed. `dbfGateStore` holds the real box at publication until the console's request has ended, by disconnect and by the bound; late writes are asserted after the end; the nothing-replicated branch is gone. |
| 20 | P2 | 2: 14 | Fixed. An operation-order check (each `list` preceded by its round's `result`) with its own mutation row, apart from the chunk-count check. |
| 21 | P2 | 3: 1 | Fixed. Plan 1's agent logs planning-and-restore time and confirmed output; plan 2's console logs its relay; plan 3 Task 1 measures from those lines. |
| 22 | P2 | 3: 7 | Fixed. `checkPullManifest` checks sizes, levels, ranges, duplicates and an overflow-safe sum before any file is created. Tests: four new malformed cases, each DBF-13, no panic, no file. |
| 23 | P2 | 3: 7, 9 | Fixed. The spool failure and Ctrl-C run `dbPull` against a stand-in console with `TMPDIR` scratch and a spool-write seam; Task 9 sends SIGINT to the binary mid-pull. Each asserts no `--out` and no `carlos-db-*` left. |
| 24 | P2 | 3: 10 | Fixed. `TestNewAgentClampsMaxPull` calls `NewAgent`; the malformed-manifest cases assert no file; the record-changed case asserts the worker never received `run` (frames recorded at the agent's end). |

**Fixed:** 23 in full, 1 in part (11). **Declined:** the part of 11 below.

## Round 2 dispositions

Round 2 (`CODEX-FINDINGS-DB-REGION-FORWARD-PLANS-R2.md`) found 14 more (3 P0, 1 P1, 10 P2). There is no plan round 3: the build starts from the tip that answers them, with per-task reviews and a whole-branch review after. Every new top-level identifier in the three plans was checked against `origin/main` for a same-package declaration; `dbStream` was the only collision.

| # | Sev | Plan, task | Disposition |
|---|---|---|---|
| 25 | P0 | 2: 7 | Fixed. The relay type is `dbReadRelay`. |
| 26 | P0 | 3: 5 | Fixed. The replacement keeps plan 1's two timing lines (`held through` for a pull); `TestPullShipsThePlannedFiles` asserts them. |
| 27 | P0 | 1: 15 | Fixed. The `/etc` unit is swapped for a `/dev/null` symlink, `LoadState=masked` is asserted, and the symlink is removed before the file is restored. |
| 28 | P1 | 2: 6 | Fixed as far as checks can: the request's context is checked immediately before each of the four writes, `cancel` once anything is published; one test case per step. A write already past its check still lands; the next check cancels, and boxes read `cancel` before electing and every 5 s while running. Spec §5.5 step 3. |
| 29 | P2 | 1: 11, 12 | Fixed. A skipped probe stays owed (`probeRetry`); the failed connection closes before the probe is asked for. Tested through `Run` with a lingering unit. |
| 30 | P2 | 2: 6, 7 | Fixed. Accelerated windows allow for `issued_at`'s truncation. |
| 31 | P2 | 2: 7, 14 | Fixed. `TestDBReadResultWaitsForAChunkTheListingMissed`. |
| 32 | P2 | 3: 9, 10 | Fixed. Real files, a valid manifest, a wrong digest. |
| 33 | P2 | 2: 6, 14 | Fixed. "ended during the serial read", with a store that refuses a cancelled write. |
| 34 | P2 | 2: 10, 11 | Fixed. The edges run in package `edge`'s test binary as a helper process; the real CLI and console in the root package; progress observed at the CLI's stderr and socket, through the local edge too. |
| 35 | P2 | 2: 7, 14 | Fixed. `TestDBReadResultAfterTheDeadlineIsNotRelayed`; the "no witness" note is gone. |
| 36 | P2 | 2: 6, 8 | Fixed. CLI refusal with a new row, DBF-32 (pending copy review), and a console 400 for U+FFFD; console, CLI and binary tests. Spec §5.5 step 3 and §10. |
| 37 | P2 | 2: 8; 3: 7 | Fixed. `dbfChunkBound`: a count cap, a size floor, nothing after an empty chunk, checked before memory or the spool keeps a chunk. |
| 38 | P2 | 2: 10, 14 | Fixed. `TestDBReadSetsBothReadDeadlines` watches the connection's read deadlines. |

## Declined plan findings

- **11, in part: a cancellation-aware audit insert.** `RecordActivityContext`
  takes a context, but its claim plumbing ends in `claimRawRecordActivity`'s
  context-free `s.db.Exec` (`internal/console/store.go:2588`, called from
  `:2668`), shared by every audited route. Changing it is a change to the
  claim SQL layer and its census, for one local SQLite insert. The order
  makes it unnecessary: the row is written while the request is open
  (checked just before) and before any object a box can see, so a request
  that ends during the insert leaves the row of an admission and no read.
  The one bucket write that announced reads after the request ended, the
  serial bump, is now request-bound.

## What to check

Report **new P0, P1 and P2 findings only**; leave P3s out of this round.

1. **Each fix as built.** Does it do what the disposition says, against the
   code on `main` (cite `file:line`), and does its test fail without it, by
   the mutation the plan names? Where a plan says a check has no witness of
   its own (plan 2 Task 14's note on the `ownCtx` check before the result
   line), is that true and acceptable?
2. **What the fixes broke.** Each fix moved code or tests: the stream's
   rewrite (`dbfCall`, phase contexts, `stop`), the audit-first admission,
   the moved tests, the gate store in the MinIO test, the retry's
   reservation and `settle`, `SelfView`, the frame tap, the stand-in console
   in plan 3. Do names, signatures and imports still line up across tasks
   and plans, and does every task still build on the ones before it?
3. **Rev 6 against the plans.** Do the four marked clarifications match
   what the plans build, and does any of them contradict J1 to J5 or
   another part of the spec?
4. **Anything round 1 missed** that is P0 to P2 under the round 1
   definitions below.

## Output

A numbered list of findings, numbered from 25. Each one gives:

- **severity:** P0 the plan cannot be executed as written (a task that
  cannot build or whose gate cannot pass); P1 the built code would break a
  spec invariant or ruling, cross a tenant, or expose box credentials; P2
  wrong behaviour, a missing spec item, or a test that proves nothing;
- the plan and task, and the round 1 number if it reopens one;
- the evidence, with `file:line` in the code or `plan:Task N` in the plans;
- a concrete fix to the plan.

Then a verdict per plan: accept, or revise. Do not rewrite the plans. No
praise.