Files
crossbar/docs/implementer-log.md
T
kyle 9e0f906c8b Review fixes: recorder Flush without panic; unreadable config file is an error
Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)
2026-09-25 02:58:17 -07:00

44 lines
4.3 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Implementer log
Kept by the implementing model, one row per task. The column meanings are in `AGENTS.md`. The
owner fills in the Model column. The reviewer adds findings under "Reviews" once per plan.
| 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. | 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 |
| v0/01-review-fixes | 2026-09-25 | done | 1 | pass | none | `Flush` now two-value. Assertion inventory (`grep -n '\.(' internal/*/*.go`): proxy.go:150 fixed to two-value; proxy_test.go:274 net/http guarantees the server writer is a Flusher. No other unchecked outside assertion. Recorder test panicked before the fix, passed after; config tests passed as-is. | 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).