stack/agents/filbert/work/ledger-t3-source-review-2026-09-26.md

# Gate F brief: Filbert's review of the T3 ledger source

Reviewer: Filbert, 2026-09-26. Requested by Sage.

Candidate: `docs/plans/2026-09-26_ledger-t3-source.md`, sha256
`08959a05574264e4f8243a90af94746e73a2fde3706f22e38f4ff1105b7a45a8`. I
verified the hash. The file is uncommitted and has no code.

I reviewed it against Sage's rulings, not as open choices:
1. on by default, where a missing or unreadable database exits 1 and the
   message names `--no-t3`;
2. the `t3:unmapped` row stays;
3. the 14 free-text Discord Bot headers stay as recorded;
4. case-insensitive classes (the 6a fix) are folded in.

**Verdict: revise.** The design is sound, and the counts check out against
the live database. Three findings would make the build's tests fail or
depend on the host, and each needs a text change:
- one acceptance check expects the wrong result (W1);
- the "never writes" claim is inaccurate (W2);
- existing tests would read the real T3 database (F1).

The rest are smaller additions. None needs a new ruling from Sage or Jason.

## What I checked

- **Scratch WAL tests** with `node:sqlite` on Node 26.8.1 (SQLite 3.53.4),
  in `/tmp`.
- **The live database**, opened read-only with `mode=ro`. I read the schema,
  project roots, thread titles, and message counts per thread. I read no
  message text except the first line of each user message, which the
  classifier runs over. I printed text only for my own thread's one human
  message.
- **`packages/ledger/src/ledger.mjs` and `tests/ledger.test.mjs`** at HEAD.

The brief's numbers hold:

| Check | Live result |
|---|---|
| Discord Bot user messages | 68 |
| Typed-in-app (human) messages, outside imported and deleted threads | 113 = the brief's 99 + 14 |
| Agent messages with the class fix | 101 = the brief's 80 + 16, plus 5 sent since 20:54Z |
| Journal and locking mode | `wal` and `normal` |
| The three required tables | the columns the brief names, all present |

The `orchestration_events` diagnostic query costs about 30 ms on the 394 MB
database.

## 1. WAL and the read-only open (the part Sage asked about first)

The core claim holds. In my test, a `mode=ro` reader saw all three rows
while they were still only in the `-wal` file, with the writer attached.
`immutable=1` did worse than "lose the newest messages": with the table
created inside the WAL, it failed with `no such table`. So the brief's ban
on `immutable=1` and on copies is right.

Two of the brief's statements are wrong, though, and one acceptance check
would fail as written.

**W1. `-wal` without `-shm` does not fail. (Blocking.)** I made that case by
copying a SIGKILLed writer's `-wal` without its `-shm`.
- A `mode=ro` open succeeded, read all three rows, and created the missing
  `-shm` itself.
- It failed, with SQLite error 14 ("unable to open database file"), only
  when the directory was not writable.
- Adding `readonly_shm=1` didn't help. It made both this case and the
  clean-stop case fail, and it still created a `-wal`.

So the acceptance line "Each fail-closed case above has its own test,
including … `-wal` without `-shm`" expects a refusal that doesn't happen.
Change it to two tests:
- **`-wal` without `-shm`, directory writable:** the read counts the WAL
  rows.
- **`-wal` without `-shm`, directory not writable:** exit 1, naming
  `--no-t3`.

Also fix line 142, which lists this case as a refusal.

**W2. The reader can create files in `~/.t3/userdata`. (Blocking as a
claim.)** Line 157 says "The source never writes to the database", and line
160 says the one outside effect is read locks in `-shm`. Line 44 says "no
locks", which contradicts line 160. What actually happens:
- **T3 stopped cleanly:** `-wal` and `-shm` are gone. A `mode=ro` open
  creates an empty `-wal` and a 32 KiB `-shm`, and leaves both after it
  closes. The main file is untouched, and the read works even with the
  directory read-only.
- **`-wal` without `-shm`:** the reader rebuilds the WAL index into a new
  `-shm`.

This is standard SQLite behaviour, and T3 opens normally afterwards.
Replace the claim with the accurate one:
- the reader never writes the main database file;
- it may create or update `-wal` and `-shm` beside it, as any SQLite
  connection does.

The T3-stopped test should then assert:
- the counts are correct;
- the main file's bytes are unchanged.

**W3. One read transaction, and a busy timeout.** The brief runs several
queries (schema, project, threads, messages, the diagnostic's events) while
T3 writes.
- In autocommit mode, each statement gets its own snapshot. Wrap the whole
  read in `BEGIN` … `COMMIT` (a deferred read transaction), so every query
  sees one committed state.
- Set `DatabaseSync`'s `timeout` (a few seconds). Then a transient
  `SQLITE_BUSY`, for example while T3 runs a checkpoint or WAL recovery,
  waits instead of failing Gate F. A `BUSY` after the timeout is exit 1,
  like any other open failure.

**W4. Build the URI with `pathToFileURL`, not string concatenation.** In my
test, a string `file:<path>?mode=ro` worked on Node 26.8.1. But a `?`, `#`
or `%` in the home path would break it. `new URL` plus `searchParams` avoids
that.

**The two untested cases, as acceptance checks.** With W1 and W2 applied,
both are proper checks: a fixture can create each state deterministically.
- **Stopped:** close a WAL database cleanly.
- **`-wal` without `-shm`:** SIGKILL a child writer that set
  `wal_autocheckpoint=0`, then delete `-shm`. Copying the `-wal` before any
  checkpoint also works.

Add one more: the newest message is in the WAL while the writer holds the
database open. The brief has this ("One test opens a database whose newest
message is still in the WAL"). Say that the writer is still attached,
because that is the live-T3 case.

## 2. Thread-to-seat title rule

Checked live:
- The five 2026-09-26 seat threads have `title_state_json.source` set to
  `manual`.
- "Darkwing in Claude" and "Dewey in Claude" predate that field (null).
- T3 also generates titles itself: the table has `title_regeneration_*`
  columns and `needsRefinement`.

**How the rule can misassign:**
1. **An auto-generated title.** If Jason opens a thread without naming it,
   T3 titles it from his first prompt. A title like "Rocko review of the
   plan" maps to rocko.
2. **A rename.** Titles are current state. Renaming a thread moves its
   whole history to another row.
3. **A seat's thread titled for a topic.** For example, "review: queue"
   drops that seat's messages into `t3:unmapped`.

Misassignment never changes the Human total or the human-per-closed ratio.
It only moves counts between rows. It does matter for Gate F, which reads
one seat's row.

**T1. Add a header cross-check. (Required.)** The headers already carry the
answer. Every agent header addressed to a seat thread names the recipient
and that thread's own id. Live, every one agrees with the title rule:

| Thread | Headers addressed to it |
|---|---|
| Sage | 39, all `to: sage` |
| Darkwing | 14, all `to: darkwing` |
| Filbert | 18, all `to: filbert` |
| Dewey | 15, all `to: dewey` |
| Rocko | 15, all `to: rocko` |

Rule: a user message whose header matches 6a, and whose `to:` id is the
message's own thread, must name the seat the title rule gave that thread.
The same goes for an unmapped thread receiving a seat-addressed header. On
a mismatch, the report refuses with exit 1 and names the thread id and both
roles. This uses only message text, not T3 metadata, so it fits the brief's
principle that `origin` decides nothing.

It catches:
- a seat thread renamed to another seat;
- a seat thread renamed to a topic, once any agent writes to it;
- a Jason-started thread auto-titled with a seat name that agents then
  address by a different seat.

It doesn't catch a thread that no agent ever writes to. That case can only
add human counts to a seat row, never hide them, so for Gate F it errs
toward a visible failure. Say so in the README.

**T2. List the mapping in the JSON.** Give each seat's T3 thread ids and
titles, and the unmapped thread ids. A reader of a Gate F result can then
see which threads made up the row.

**T3. Test the rule** with these fixtures:
- a same-title thread in another project (the brief has this);
- a seat thread renamed to another seat, which must refuse under T1;
- a title that starts with a seat name but not followed by a space ("Sagebrush"),
  which stays unmapped;
- "Researcher", a real directory with no thread today.

## 3. The fail-closed departure

The reasoning is right, and it follows Sage's ruling 1. A missing `.pi`
directory means no Pi seat ran here. A missing or changed T3 database on a
host that uses T3 means the evidence moved, and a zero there would be the
false pass that Gate F exists to catch. Refusing on a missing table or
column, a missing project or more than one project, a bad row, or a symlink
is consistent with how the Pi reader refuses malformed JSONL.

**F1. The existing tests would read the real T3 database. (Blocking.)**
- `tests/ledger.test.mjs` spawns `cli.mjs` with `env: { ...process.env, … }`,
  so `HOME` is the developer's.
- With the source on by default, every existing test would open the real
  `~/.t3/userdata/state.sqlite`, find no project for the temp fixture root,
  and exit 1.
- On a host without T3, they would exit 1 too.

The brief needs one explicit way to point the reader at a fixture:
- either a fixture `HOME` in the spawned environment, or a documented
  override such as `--t3-db PATH`;
- an acceptance line saying no test opens the real `~/.t3`. For example,
  the tests set `HOME` to a temp dir for every run.

Existing tests that aren't about T3 should pass `--no-t3` or get an empty
fixture database. Say which.

**F2. Symlink check.** The brief checks `state.sqlite` and `~/.t3/userdata`.
Check `~/.t3` as well, because the Pi reader checks every ancestor from the
root down ("Check every source ancestor"). Today all three are real
directories or files.

**F3. Match the repository root exactly.** Compare `workspace_root` byte for
byte with the ledger's root. The CLI gets that root from the realpath of its
own URL. `~/src/mosaic-stack-dev-test` is a compatibility symlink to this
checkout. A T3 project opened through it would not match, and then "no
project row" is the correct refusal. The README should say this.

**F4. Name the blind spot.** Live, there is a T3 project at `/home/jwoltje`
and a deleted one at `/mnt/storage/src`. A thread in either could work on
this repository and would not be counted. The workspace-root rule is still
right, but the README's exclusions should name this case.

**F5. When the diagnostic's table is missing.** The diagnostic (ruling 3)
reads `orchestration_events`. Say what happens if that table or its columns
are missing:
- The report prints the diagnostic as `unknown`, and the counts are
  unaffected, because the diagnostic decides nothing.
- Or add the table to the required schema and refuse.

I'd take the first, because a T3 change to internal metadata shouldn't stop
the counts. Either way, the brief should say which.

## 4. The class fix and the counts

The fix is correct and scoped as Sage ruled. HEAD's
`packages/ledger/src/ledger.mjs:81` (tmux) and `:83` (T3) both have
`class=[a-z-]+`. Make both case-insensitive, not only the T3 one. The
acceptance line "an uppercase class counts as agent after the fix and as
human before it" should cover both preambles. One thing the count doesn't
show: the only human-classified message in my thread is Jason's opening
assignment (19:31:07Z). With the fix, all 18 agent headers to Filbert
classify as agent.

## To reach approve

- **W1:** correct the `-wal`-without-`-shm` expectation and the line-142
  refusal; split it into writable and non-writable tests.
- **W2:** replace the "never writes" and "no locks" statements with the
  accurate effect; the stopped test asserts the main file's bytes are
  unchanged.
- **W3:** one read transaction and a busy timeout. **W4:** build the URI
  from a URL.
- **T1:** the header cross-check, refusing on a conflict. **T2:** the
  mapping in the JSON. **T3:** the extra fixtures.
- **F1:** a fixture path and no test opening the real `~/.t3`. **F2–F5:**
  the ancestor symlink check, an exact root match, the blind spot in the
  README, and the diagnostic when its table is missing.
- **§4:** the class fix covers both preambles.

Send the revision with its hash and I'll review the delta.

## Correction (Filbert, 2026-09-26, after R2)

W2 said the clean-stop read "works even with the directory read-only". That
was wrong, and Darkwing's measurement is right. My test for that case reused
a database that the previous test had already opened, so its leftover `-wal`
(0 bytes) and `-shm` were still present. It was not a clean stop. I re-ran it
with a truly clean stop (no `-wal`, no `-shm`) in a non-writable directory
on Node 26.8.1 / SQLite 3.53.4. The open fails with errcode 1544, "attempt
to write a readonly database". So a stopped T3 in a non-writable directory
is exit 1, as R2 expects. The rest of W2 stands: the reader never writes the
main file, and it may create `-wal` and `-shm`.

## R2 delta review

Candidate: `docs/plans/2026-09-26_ledger-t3-source.md`, sha256
`e8300cb6abea70819aba7cf10040d19b4d6019b5663c37203209537a5f10ee62`. I
verified Darkwing's delta files against `manifest.sha256`:
- `r1.md` is `08959a05…45a8`, the R1 I reviewed;
- `r1-to-r2.diff` is `aa4740ae…71fc`.

I read R2 in full.

**Verdict: approve** `e8300cb6`. Every finding is answered:
- W1 and W2: the section 1 table, the accurate claim about side files, and
  split tests.
- W3 and W4: one transaction, a 5 s timeout, and `pathToFileURL`.
- T1: as asked, plus Darkwing's addition that an unmapped thread can't
  receive an own-id header naming a seat.
- T2 and T3: the mapping in the JSON, and the fixtures.
- F1: `--t3-db`, `HOME` set at both spawn sites, and a no-database test.
- F2–F4: done. F5 follows Sage's ruling.
- §4: both preambles.

Three nits, none blocking. Carry them into the build, or fix them in an R3
and I'll confirm the hash:
1. **The measurement is settled.** Lines 71–74 and 279–282 treat the
   non-writable clean stop as disputed. I've corrected my review above: it
   fails with 1544. State it as agreed. Make the test a plain exit-1
   assertion, and drop the "if the build reads there instead" branch.
2. **Real path in the fixture project row.** The fixture's project row
   should store `fs.realpathSync(root)`. The CLI takes its root from the
   realpath of `cli.mjs`, and a temp directory under a symlinked `/tmp`
   would otherwise fail with "no project row".
3. **Say which database was read.** `--t3-db` can point a real Gate F run at
   any file. The JSON should record the database path it read and whether it
   was the default. The text report should add one line when it wasn't. A
   Gate F result then can't come from a fixture without saying so.