51 lines
3.2 KiB
Markdown
51 lines
3.2 KiB
Markdown
# v1.1 implementation plan: review follow-ups
|
||
|
||
> **For the implementing model:** do not work from this file. The owner gives you one task file at
|
||
> a time. This file is the index for the owner and the reviewer.
|
||
|
||
**Goal:** close findings 1 and 2 of the v1 review (`docs/implementer-log.md`): a request whose
|
||
client disconnects — mid-stream or while queued — must still write its accounting row (status
|
||
499), and `/_crossbar/usage` with no rows must answer `[]`, not `null`.
|
||
|
||
**How this plan was made:** acceptance tests first, from the findings; no reference
|
||
implementation. Both given tests were run against `master` at the merge of `v1`: all three fail
|
||
there (the two cancel tests find no 499 row; the empty-usage test gets `null`).
|
||
|
||
## Tasks
|
||
|
||
| # | File | Delivers | Tests that define it |
|
||
|---|---|---|---|
|
||
| 01 | `01-review-fixes.md` | 499 rows on both cancel paths; `[]` for empty usage | `internal/proxy/cancel_test.go`, `internal/admin/usage_empty_test.go` |
|
||
|
||
Branch `v1.1`. One task, one fresh OpenCode session, one commit.
|
||
|
||
## For the reviewer
|
||
|
||
1. `git log --oneline master..v1.1`: one commit with the trailer.
|
||
2. `cmp` both copied tests; `git diff master..v1.1 --stat -- PLAN.md AGENTS.md docs/plans` empty.
|
||
3. `make gate`, `make smoke`.
|
||
4. Probe: cut a stream with `curl -m 0.4 -N …` against the smoke rig and confirm one `status="499"` line in `/_crossbar/metrics`.
|
||
|
||
## Changes during the run
|
||
|
||
- 2026-09-25, task 01, first session: ended after ~8 min with no commit and no row, right after
|
||
the sandbox refused a `/tmp` scratch program (the I9 pattern, third time tonight). The rule
|
||
against ending a turn on a refusal lived only in v1's task 05; it is now in `AGENTS.md`, so every
|
||
task carries it. Resumed from the working tree.
|
||
- 2026-09-25, task 01, second session: two of three tests passing; the mid-stream cancel wrote
|
||
no row because `httputil.ReverseProxy` does not return when the client disconnects mid-copy on a
|
||
real server — it panics with `http.ErrAbortHandler`, so code after `rp.ServeHTTP` never runs.
|
||
Ornith tried to read the Go source tree to find that out; the sandbox refused (outside the
|
||
repository) and the session ended on the refusal again. Two faults: the task text did not state
|
||
the environment's behaviour (mine — the customer describes the world the code runs in), and the
|
||
model ended a turn on a refusal (its, fourth time). Third session given the fact and told to
|
||
record from a deferred function with `recover()`.
|
||
- 2026-09-25, task 01, third session: the fix was complete and all three tests passed, but the
|
||
session measured the proxy package failing 7 of 20 runs and went looking for the cause,
|
||
ending its turn on a refused `/tmp` copy (fifth refusal-ending tonight). The owner ran the
|
||
package 12 times on an idle machine: 0 failures. The flakes were CPU contention from the
|
||
model's own inference on the same host hitting the timing-based tests (queue, spread, cancel).
|
||
Two faults: timing margins in the given tests are too tight for a loaded machine (owner's test
|
||
design — widen in v2.1 or run those tests with a retry), and the model again ended a turn on a
|
||
refusal. A fourth session was told to skip the investigation and finish steps 4–7.
|