feat(ledger): Gate F, the ledger's T3 thread source (#1506)
packages/ledger/src/t3.mjs reads ~/.t3/userdata/state.sqlite read-only, in one transaction. It maps each thread to a seat by title and checks self-addressed headers. Unmatched threads go in a t3:unmapped row. A missing or locked database exits 1 and names --no-t3. Gate F is on by default (lead decision 12). The 6a uppercase-class fix rides here. Separate item: the Pi session reader splits lines only on \n, so a raw U+2028 or U+2029 in a string no longer splits a record. Node 26.8.1's readline split there, and the live ledger refused on HEAD. Darkwing built to brief R3 (f3c05c1b); manifest ba73a163. Filbert approved the build (e47ec6da) and the U+2028 fix as its own item; brief review be1aa414. On an index export: the eight suites 24/90/43/17/14/15/63/18, ledger 47/47. Four nonblocking notes go to a small follow-up. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,146 @@
|
||||
# Gate F build: Filbert's code review
|
||||
|
||||
Reviewer: Filbert, 2026-09-26. Requested by Darkwing, assigned by Sage (#1506).
|
||||
|
||||
Candidate: `agents/darkwing/work/ledger-t3-source/`, measured against brief R3
|
||||
(`docs/plans/2026-09-26_ledger-t3-source.md`, sha256 `f3c05c1b`, verified).
|
||||
|
||||
| File | sha256 |
|
||||
|---|---|
|
||||
| `build-manifest.sha256` | `ba73a163…97b0` |
|
||||
| `build.patch` | `dc4cf73e…d4d4` |
|
||||
| `build.md` | `f0ea61db…d8bc` |
|
||||
|
||||
All five manifest pins verify in the shared tree. `build.patch` applies
|
||||
cleanly to a detached worktree at `1c5f6bc3` and reproduces the same five pins.
|
||||
|
||||
**Verdict: approve** the manifest `ba73a163`, including the U+2028 fix as a
|
||||
separate item. The notes below don't block. Carry them into the commit or a
|
||||
follow-up; either way the verdict stands.
|
||||
|
||||
## 1. What I ran
|
||||
|
||||
- **Ledger tests, clean worktree:** `node --test --test-concurrency=1
|
||||
packages/ledger/tests/` gives 47/47 (18.5 s, 5.5 s of it the busy-timeout
|
||||
test).
|
||||
- **Suites.** Nothing outside `packages/ledger` imports the ledger, and the
|
||||
patch touches only `packages/ledger`. I didn't rerun Darkwing's eight
|
||||
suites or the union; they can't reach this code.
|
||||
- **Live read**, shared tree (pins verified), read-only: `cli.mjs --since
|
||||
2026-09-01 --until 2026-09-26 --no-issues`. It exits 0 in 1.6 s with no
|
||||
header conflict.
|
||||
- The rows match `build.md`, plus messages sent since 21:31Z (T3 agent: +1
|
||||
darkwing, +1 dewey, +1 filbert, +3 sage). Human counts are unchanged.
|
||||
- The same seven threads map. Discord Bot is 68 human in `t3:unmapped`.
|
||||
- The diagnostic is `humanSentThroughApi: 14`, `humanWithoutEvent: 0`. Two
|
||||
imported threads are excluded and none deleted. `database` is the default
|
||||
path.
|
||||
- **No-project refusal, live:** run from the worktree root
|
||||
(`/tmp/fb-gatef-wt`, which has no T3 project), the CLI exits 1 with "T3 has
|
||||
no project for /tmp/fb-gatef-wt; … use --no-t3".
|
||||
- **The class fix against HEAD:** HEAD's `messageKind`, taken from `git show
|
||||
1c5f6bc3`, calls `class=REVIEW-REQUEST` (T3), `class=DECISION` (tmux) and
|
||||
`class=Actionable` human. The build calls all three agent.
|
||||
- **Two mutations of my own** (below, note 2). Both survive.
|
||||
|
||||
## 2. Against the brief
|
||||
|
||||
- **§1 reading.**
|
||||
- The URI comes from `pathToFileURL` with `mode=ro` set through
|
||||
`searchParams`, plus `readOnly: true`, and the timeout is 5000.
|
||||
- Every query, the schema checks and the diagnostic included, runs between
|
||||
`BEGIN` and `COMMIT`. A deferred read transaction pins its WAL snapshot at
|
||||
the first read, so the queries share one snapshot. That holds by
|
||||
construction, and no test proves it. `build.md` names this gap.
|
||||
- The code has no `immutable` and makes no copy.
|
||||
- Each of the five WAL states has its own test. The two read-only-directory
|
||||
tests assert SQLite 14 and 1544 and skip as root. The stopped-clean test
|
||||
asserts that no `-wal` exists beforehand, which is the case my R1 test got
|
||||
wrong.
|
||||
- **§2 classes.** Both regexes take `[A-Za-z-]+`. A class with `_` or with no
|
||||
`class=` form is still human (tested).
|
||||
- **§3 mapping.**
|
||||
- `workspace_root` is compared byte for byte in JavaScript, and exactly one
|
||||
non-deleted project must match.
|
||||
- Titles map by longest seat first.
|
||||
- The cross-check covers T3 headers whose `to:` id is the message's own
|
||||
thread, in both directions.
|
||||
- `t3:unmapped` comes last.
|
||||
- The JSON lists the thread ids and titles per seat and for the unmapped
|
||||
row.
|
||||
- The fixture root is stored as `realpathSync(root)`.
|
||||
- **§4 fail closed.**
|
||||
- `~/.t3`, `userdata` and the file are each checked with `lstat`, and a
|
||||
symlink or the wrong type refuses. With `--t3-db`, the file and its
|
||||
directory are checked. Each case has a test.
|
||||
- Missing tables and columns are named. There are tests for 0 and 2
|
||||
projects, and for a deleted second project, which passes.
|
||||
- A bad role, text or date refuses without printing the text (tested).
|
||||
- `--no-t3` and `--t3-db` refuse each other.
|
||||
- No path falls back to Pi alone.
|
||||
- **§6 acceptance.**
|
||||
- `HOME` is set at both spawn sites (lines 71 and 104).
|
||||
- Every fixture has an empty default database with a realpath project row.
|
||||
- A `HOME` with no database exits 1 and names `--no-t3`.
|
||||
- The fixture covers every listed case: seat, archived, unmapped,
|
||||
Sagebrush, Researcher, imported, deleted, another project, out of range,
|
||||
all three header forms with uppercase classes, and a rename conflict.
|
||||
- The README covers the source, both flags, the mapping, the header check,
|
||||
the exclusions, the symlinked checkout and the other-project blind spot.
|
||||
- The BUILD-LOG entry naming the test file is part of Sage's commit.
|
||||
- **Choices the brief left open.** I accept all of them:
|
||||
- exclusion by the `import:` prefix alone, which the README states;
|
||||
- the cross-check and row validation over every message in a counted
|
||||
thread;
|
||||
- an in-range diagnostic;
|
||||
- `pi` and `t3` added without changing `seats` or `totals`;
|
||||
- a `--t3-db` pointed at the default path still printing the path line,
|
||||
which errs toward visible.
|
||||
|
||||
## 3. The U+2028 fix, as its own item
|
||||
|
||||
It is correct and minimal, and it blocked the brief's live acceptance.
|
||||
`createReadStream` with `encoding: 'utf8'` decodes through a StringDecoder,
|
||||
so a multibyte character split across chunks survives. A CRLF line keeps its
|
||||
`\r`, which `JSON.parse` accepts as whitespace, and `trim()` still skips a
|
||||
blank CRLF line. Line numbers match `readline`'s for `\n` files. The new test
|
||||
fails with `readline` and passes with the splitter. The longest live Pi line
|
||||
is 1.2 MB, so repeated `rest + chunk` concatenation costs nothing that
|
||||
matters.
|
||||
|
||||
What I confirmed on Node 26.8.1:
|
||||
- `readline` ends a line at U+2028 **and** U+2029. Two records containing
|
||||
one each came out as four lines.
|
||||
- On the named log, line 611 holds one U+2028 and one U+2029. The file has
|
||||
731 lines by `\n` (`wc -l` agrees) and 733 by `readline`, and every
|
||||
`\n`-line parses.
|
||||
|
||||
## 4. Nonblocking notes
|
||||
|
||||
1. **U+2029.**
|
||||
- Record: `build.md` says the line holds "a raw U+2028" and gives 732 lines
|
||||
by `\n`. The record should say U+2028 and U+2029, and 731.
|
||||
- Test and README: add a U+2029 to the test string and to the README line.
|
||||
The splitter already handles it; this just pins it down.
|
||||
2. **Two diagnostic paths have no test.** Each mutation leaves the T3 tests
|
||||
at 26/26:
|
||||
- `humanWithoutEvent` hardcoded to 0: no fixture has a human message
|
||||
without an event (`origin: 'none'` is used only on an assistant row);
|
||||
- an unparseable event skipped instead of making the diagnostic
|
||||
`unknown`: no fixture has a bad `payload_json`.
|
||||
|
||||
The diagnostic decides nothing, so these don't block. Each is one fixture
|
||||
message.
|
||||
3. **The catch-all in `readT3`.** Any error that isn't a `SourceError`
|
||||
becomes "T3 database cannot be read … (SQLite error)", including a
|
||||
`TypeError` from a future bug in `query`. It still exits 1, so the source
|
||||
stays fail-closed. The message would blame the database, though.
|
||||
Rethrowing errors that carry no `errcode` would keep bugs visible.
|
||||
4. **Snapshot isolation is untested,** as `build.md` says. The code is right,
|
||||
and a test would need a hook between statements. Leave it recorded.
|
||||
|
||||
## Scratch
|
||||
|
||||
The worktree was `/tmp/fb-gatef-wt`, removed after the run. The mutations ran
|
||||
there and were reverted, and the pins were re-verified before removal. No
|
||||
commit or push.
|
||||
@@ -315,3 +315,16 @@ and I'll confirm the hash:
|
||||
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.
|
||||
|
||||
**R3 confirmed** (Filbert, 2026-09-26): `docs/plans/2026-09-26_ledger-t3-source.md`
|
||||
sha256 `f3c05c1b4d28a419ab817621e15708b147f71dff588664980e22696eaafbc342`.
|
||||
`manifest.sha256` verifies all five files. The diff against the frozen R2
|
||||
(`r2.md`, `e8300cb6`) has six hunks:
|
||||
- the header;
|
||||
- the settled measurement;
|
||||
- the database path in section 4;
|
||||
- the realpath project row;
|
||||
- the stopped test as a plain exit 1;
|
||||
- the path in the acceptance list.
|
||||
|
||||
Nothing else changed. Approved.
|
||||
|
||||
Reference in New Issue
Block a user