ledger: count issue-tagged commits and repo seat messages (#1506)
This commit is contained in:
@@ -0,0 +1,43 @@
|
||||
# Ledger review request, #1506
|
||||
|
||||
Author Darkwing requests independent read-only review by Filbert of piece 3.
|
||||
Baseline 889d8750 on refactor. Approved brief is Piece 3 on
|
||||
`docs/plans/2026-09-12_control-board-mvp.md`. No author self-approval.
|
||||
|
||||
Review these exact candidates and run `node --test packages/ledger/tests/`.
|
||||
Use only fixtures, no real Gitea/session reads needed. Do not edit source or
|
||||
unrelated work. Write the verdict, tested hashes and findings to
|
||||
`docs/plans/reviews/2026-09-12_ledger-verdict.md`, then notify darkwing.
|
||||
|
||||
SHA-256:
|
||||
|
||||
```text
|
||||
601db1de6d4838785f334dd93dd191d444276f306e016b3a4137659eaae76561 packages/ledger/README.md
|
||||
e33fc5be5cdeb69f6f443a44c5b02a7bc346ac74dd69f984538e424a8d1b5de9 packages/ledger/src/cli.mjs
|
||||
fbd95e45c5753cb44e39dfead0c6e0d2a5f07859dd9411f1dd659dde98af2ffc packages/ledger/src/ledger.mjs
|
||||
c836ecd64068cccf7f515ab699c6a218e5ea1618c39e7e88be21bfa74d96414e packages/ledger/tests/gitea-helper.test.mjs
|
||||
8e838a5333cbe6aafda5d72b810eb2840c4439973841d9c2bf5b55bc660455a3 packages/ledger/tests/ledger.test.mjs
|
||||
033e6d5fdd4626fb0ca1c981b6d85eba961a4618a303faf2bb7b30ae024ed9a6 scripts/gitea-api.sh
|
||||
```
|
||||
|
||||
Author fixtures 20/20. Node syntax, shell syntax and diff whitespace pass.
|
||||
No root Gitea/GitHub workflow directory exists, so no CI run is claimed.
|
||||
Direct refactor publication follows the approved brief; no PR/trunk merge or
|
||||
live deployment. Gate D stays with Jason.
|
||||
|
||||
Important choices to assess: UTC inclusive dates, in-range follow-ups, unique
|
||||
commit total versus per-issue links, zero/unknown denominator handling, and
|
||||
one Gitea call. Gitea capped an unfiltered request at 50; updated-since returned
|
||||
19 issues for this week. Full 50-row pages fail, missing row metadata is unknown.
|
||||
Session counts are literal entries, without copied-log dedup or authorship claims.
|
||||
|
||||
Blocking helper defect discovered by real use: GET returned HTTP 200 with valid
|
||||
JSON but status 1 because the EXIT cleanup's final conditional failed when there
|
||||
was no body file. One-line `if` correction preserves successful GET status; fake
|
||||
Node/curl tests cover GET 200, POST 201 and GET 403 without reading credentials.
|
||||
|
||||
Live pre-publication measurement: 8 issues closed, 2.7 median hours open,
|
||||
40 tagged commits, 1.9 follow-ups per issue, 18.9 human messages per closed issue.
|
||||
These are changing source counts, not the final post-publication measurement.
|
||||
Archived numeric references can point to unrelated same-number current issues;
|
||||
the README explicitly disclaims remapping, matching the literal-subject brief.
|
||||
@@ -0,0 +1,114 @@
|
||||
# Ledger review verdict, LEDGER-1506-R1
|
||||
|
||||
Reviewer: Filbert (independent; author is Darkwing, no self-approval).
|
||||
Request: `docs/plans/reviews/2026-09-12_ledger-review-request.md`.
|
||||
Brief: Piece 3, `docs/plans/2026-09-12_control-board-mvp.md` (approved by
|
||||
Jason 2026-09-12). Review date: 2026-09-12.
|
||||
|
||||
## Verdict: APPROVED
|
||||
|
||||
Non-blocking observations are listed below; none contradicts the brief, the
|
||||
boundaries, or the fail-closed rules. Gate D remains Jason's. This verdict
|
||||
authorizes the transition to publication per the approved brief; it grants no
|
||||
push, merge, or deployment authority.
|
||||
|
||||
## Candidate identity
|
||||
|
||||
Baseline `889d8750` verified on `refactor`: it is an ancestor of current HEAD
|
||||
`db0d784b`; the single intervening commit (`db0d784b`) touches only
|
||||
`docs/plans/DEFERRED.md` and produces an empty diff on all six candidate
|
||||
paths. The reviewed worktree files are therefore byte-identical to the
|
||||
baseline candidate.
|
||||
|
||||
Recomputed SHA-256 (all six match the pinned values):
|
||||
|
||||
```text
|
||||
601db1de6d4838785f334dd93dd191d444276f306e016b3a4137659eaae76561 packages/ledger/README.md
|
||||
e33fc5be5cdeb69f6f443a44c5b02a7bc346ac74dd69f984538e424a8d1b5de9 packages/ledger/src/cli.mjs
|
||||
fbd95e45c5753cb44e39dfead0c6e0d2a5f07859dd9411f1dd659dde98af2ffc packages/ledger/src/ledger.mjs
|
||||
c836ecd64068cccf7f515ab699c6a218e5ea1618c39e7e88be21bfa74d96414e packages/ledger/tests/gitea-helper.test.mjs
|
||||
8e838a5333cbe6aafda5d72b810eb2840c4439973841d9c2bf5b55bc660455a3 packages/ledger/tests/ledger.test.mjs
|
||||
033e6d5fdd4626fb0ca1c981b6d85eba961a4618a303faf2bb7b30ae024ed9a6 scripts/gitea-api.sh
|
||||
```
|
||||
|
||||
## Independent test evidence
|
||||
|
||||
- `node --test packages/ledger/tests/` on node v26.8.1: **20 tests, 20 pass,
|
||||
0 fail** (duration ~1.4 s). Offline: fixtures are temp git repos, temp JSONL
|
||||
logs, and stubbed `gitea-api.sh` on PATH; the helper regression test points
|
||||
`MOSAIC_GITEA_CREDENTIAL_FILE` at a nonexistent file and stubs `node` before
|
||||
any credential read. No real API call, no private-session read, no
|
||||
credential access occurred during review.
|
||||
- `node --check` on all four `.mjs` files: OK. `bash -n scripts/gitea-api.sh`:
|
||||
OK. No trailing whitespace in any candidate file.
|
||||
- Helper defect reproduced mechanically: a `cleanup() { [ -n "$X" ] && rm …; }`
|
||||
EXIT trap under `set -euo pipefail` turns a successful run into exit 1;
|
||||
the candidate's `if` form exits 0. Verified with an isolated script outside
|
||||
the repository. The suite's GET 200 → exit 0, POST 201 → exit 0, and
|
||||
GET 403 → exit 1 cases directly regression-cover the fix.
|
||||
|
||||
## Brief conformance
|
||||
|
||||
- CLI shape matches: `--since` required; `--until` defaults to today UTC;
|
||||
`--json`; `--no-issues`; unknown/duplicate arguments refused (exit 1).
|
||||
Sources are consulted in the brief's order: local `git log refactor`
|
||||
(subjects only, range self-filtered — correct, since `git --since` prunes at
|
||||
out-of-order dates), then the single Gitea call, then repo-seat session
|
||||
logs.
|
||||
- Table 1 columns, Table 2 board/agent/human classification by first-line
|
||||
preamble, and the totals line all match the brief. Issue seats come from
|
||||
`#N` in in-range user text.
|
||||
- The "unknown, never a guess" rule holds everywhere it applies: missing issue
|
||||
metadata, `--no-issues` aggregates, zero-denominator ratios with nonzero
|
||||
human counts, negative durations, and malformed/partial session evidence
|
||||
(refuses with file and line, never echoing content).
|
||||
- Read-only boundaries hold: no filesystem writes; PATH mutation restored in a
|
||||
`finally`; no credential reads in ledger code; API error bodies and stderr
|
||||
are never echoed (tested); control characters are stripped from printed
|
||||
titles; symlinked source directories are refused and symlinked `.jsonl`
|
||||
files are skipped; no fleet paths; no network beyond the one Gitea call.
|
||||
|
||||
The five flagged choices are each implemented as documented in the README and
|
||||
internally consistent, each with a covering test:
|
||||
|
||||
1. UTC dates inclusive at both endpoints (tested at 00:00:00.000 and
|
||||
23:59:59.999, next-day exclusive).
|
||||
2. Follow-ups counted in range only (`max(commits − 1, 0)`), explicitly not a
|
||||
lifetime count.
|
||||
3. A commit naming several issues counts once in total commits and once per
|
||||
linked row (tested with `both #1 #2 #2`).
|
||||
4. Zero/unknown denominators: empty report → 0 totals; human messages with
|
||||
zero closed issues → `unknown`; `--no-issues` → `unknown` closed-count,
|
||||
median, and ratio.
|
||||
5. One Gitea call: updated-since, first page, limit 50; a full 50-row page
|
||||
refuses (exit 2) rather than silently undercounting; a commit-linked issue
|
||||
the query did not return still gets a row with `unknown` metadata.
|
||||
|
||||
## Non-blocking observations
|
||||
|
||||
1. `scripts/gitea-api.sh` writes the API response body to
|
||||
`/tmp/gitea-api-response.$$` — predictable name, not `mktemp`, no 0600.
|
||||
On a multi-user host this could expose or race a private repo's issue
|
||||
content. Single-user POC host; the POST body file already uses
|
||||
`mktemp` + `chmod 600`. Suggest `mktemp` here in a later pass.
|
||||
2. `REPO_PATH` in `gitea-api.sh` is computed but never used; the header usage
|
||||
example also references `repos/mosaicstack/stack-v2` while the canonical
|
||||
repo is `mosaicstack/stack`. Cosmetic.
|
||||
3. The credential file is validated once for `BASE` and read a second time in
|
||||
`gen_curl_cfg` (TOCTOU window). Acceptable on this host.
|
||||
4. A literal `|` in an issue title shifts text-table columns (JSON output is
|
||||
unaffected; no structured-parse contract exists for the text table).
|
||||
5. `readSessions` refuses a partially written final JSONL line. This is
|
||||
documented in the README as intended fail-closed behavior; note it can
|
||||
refuse while a seat is mid-write.
|
||||
6. The `directories()` helper's specific message "Session source must be a
|
||||
real directory" is always wrapped by the catch into "Cannot read ledger
|
||||
directory: <path>"; the specific string is unreachable. The path is still
|
||||
reported, so diagnosis is unimpaired. Cosmetic.
|
||||
|
||||
## Scope statement
|
||||
|
||||
Read-only review: hashes, `git rev-parse`/`git log`/`git diff` provenance
|
||||
checks (no mutations), the test suite, and the six files. No source edits, no
|
||||
git mutations, no real API or private-session reads. This verdict and the
|
||||
SESSIONS.md registration are the only files written.
|
||||
Reference in New Issue
Block a user