From 89ab0b8d347acb76083887d4e31a60eba1b6b188 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Fri, 25 Sep 2026 02:42:52 -0700 Subject: [PATCH] v0 review: checklist, outside probes, six findings; fill the Model column Co-Authored-By: Claude Fable 5.1 --- docs/implementer-log.md | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 08afd7b..38b9e46 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -5,10 +5,38 @@ owner fills in the Model column. The reviewer adds findings under "Reviews" once | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | |---|---|---|---|---|---|---|---| -| v0/01-module-gate-config | 2026-09-25 | done | 1 | pass | none | `go mod download` fetched the module (network available); gate passed on the first run. | ? | -| v0/02-health | 2026-09-25 | done | 1 | pass | none | First gate run passed. `MarkDown` initially forgot to write the entry back; caught by `TestMarkDown`. | ? | -| v0/03-proxy | 2026-09-25 | done | 1 | pass | none | `SplitRoute` must reject an empty first segment (`/`, `//x`) as `ok=false`; the model peek restores the body and leaves non-JSON/empty as `""`. | ? | -| v0/04-admin-main | 2026-09-25 | done | 1 | pass | none | `timeout --signal=TERM 3` exits 124 on a timed-out child on this GNU system, so the task's `exit=0` is not observable through it; sent SIGTERM directly and confirmed crossbar's own exit code is 0 with both log lines. | ? | -| v0/05-smoke-readme-deploy | 2026-09-25 | done | 1 | pass | none | `README.md` `## Run` uses `install -m` instead of `cp` and adds `systemctl daemon-reload` before `enable --now`, which is required for systemd to see the new unit; the task said only "copy … then enable --now". | ? | +| v0/01-module-gate-config | 2026-09-25 | done | 1 | pass | none | `go mod download` fetched the module (network available); gate passed on the first run. | llama.cpp/ornith-1.5-35b-a3b | +| v0/02-health | 2026-09-25 | done | 1 | pass | none | First gate run passed. `MarkDown` initially forgot to write the entry back; caught by `TestMarkDown`. | llama.cpp/ornith-1.5-35b-a3b | +| v0/03-proxy | 2026-09-25 | done | 1 | pass | none | `SplitRoute` must reject an empty first segment (`/`, `//x`) as `ok=false`; the model peek restores the body and leaves non-JSON/empty as `""`. | llama.cpp/ornith-1.5-35b-a3b | +| v0/04-admin-main | 2026-09-25 | done | 1 | pass | none | `timeout --signal=TERM 3` exits 124 on a timed-out child on this GNU system, so the task's `exit=0` is not observable through it; sent SIGTERM directly and confirmed crossbar's own exit code is 0 with both log lines. | llama.cpp/ornith-1.5-35b-a3b | +| v0/05-smoke-readme-deploy | 2026-09-25 | done | 1 | pass | none | `README.md` `## Run` uses `install -m` instead of `cp` and adds `systemctl daemon-reload` before `enable --now`, which is required for systemd to see the new unit; the task said only "copy … then enable --now". | llama.cpp/ornith-1.5-35b-a3b | ## Reviews + +### v0 review — 2026-09-25 (reviewer: claude, as owner for the night) + +Checked: five commits `73b2435..e436c62` with the trailer; every copied file byte-identical to +`docs/plans/v0/_files/`; no protected file touched (diff against the merge base is empty); +`make gate` → `gate: ok`; `make smoke` → `smoke: ok (stream spread 1003 ms)`. Probed from outside +with inputs the tests do not contain: encoded query strings pass through; a 3 MB JSON body is +forwarded, 17 MB → 413; `/_crossbar/hosts` answers while polls are in flight; `OpenCode-A` and +`/opencode-a/` → 404 as specified; HEAD and OPTIONS pass through; SIGTERM during a stream lets the +stream finish (6 SSE lines) and exits 0. + +Tally: 5 tasks, 5 first-run gate passes, 0 stops, wall time 6–10 min per task, unattended after the +restart. One model-side bug was caught by a given test during task 02 (`MarkDown` did not write the +entry back) and fixed before commit. + +| # | Finding | Severity | Fault | +|---|---|---|---| +| 1 | `statusRecorder.Flush` does `r.ResponseWriter.(http.Flusher).Flush()` — an unchecked assertion that panics on a writer that is not a Flusher. Rule "never panic" applied where the tests walked; task text said "forwarding to the underlying `http.Flusher`" without "if it implements it". | low | model + task | +| 2 | Two 502 answers in `ServeHTTP` (host name missing from config, `BaseURL` unparsable) that no task rule defined; config validation makes both unreachable. Harmless; the task should have said what to do. | low | task | +| 3 | Task 05 log row says `Deviations: none` while its Notes describe two (`install -m` instead of `cp`; `systemctl daemon-reload` added). Both changes are right; the row is not. | process | model | +| 4 | Task 01 attempt 0: given files under `docs/plans/v0/files/` were visible to `go vet ./...`. Fixed (`_files/`). | — | task | +| 5 | Task 04: `timeout --signal=TERM 3 …; echo $?` can never show `exit=0` (GNU `timeout` reports 124). Ornith verified another way and logged it. Fixed (`--preserve-status`). | — | task | +| 6 | My given `config_test.go` never covers a file that exists but cannot be read (`Load` on a 000-mode file). Gap in the acceptance suite, not in the code. | test | test | + +Follow-ups for a `v0.1` task: fix 1 (`if f, ok := …; ok { f.Flush() }`), add the unreadable-file +test for 6, and make the log-row rule in `AGENTS.md` say that anything the Notes describe as a +change belongs in Deviations (finding 3). +