# v0.1 task 01: review fixes — a writer without `Flush`, an unreadable config file **Branch:** `v0.1` (create it from `master`: `git switch master && git switch -c v0.1`; `git status --short` must be empty first, otherwise stop) **Commit subject:** `Review fixes: recorder Flush without panic; unreadable config file is an error` ## What the reviewer observed 1. `internal/proxy/proxy.go`, `statusRecorder.Flush`: ```go func (r *statusRecorder) Flush() { r.ResponseWriter.(http.Flusher).Flush() } ``` The assertion is unchecked. Every `http.ResponseWriter` the standard server hands out is a Flusher, but wrappers written by middleware or tests often are not, and then a streamed response **panics inside the reverse proxy** instead of falling back to buffering. The rule "library code never panics on input" applies to every type assertion, including this one. The task text said "forwarding to the underlying `http.Flusher`" and did not say "if it implements it" — that half is the task's fault; the panic is still a defect. 2. The given `config_test.go` never covered a file that exists but cannot be read. `Load` already handles it (an `open` error wrapped as `config: …`); the suite just did not say so. ## Files - Copy (never edit afterwards): `internal/proxy/recorder_test.go`, `internal/config/unreadable_test.go` - Modify: `internal/proxy/proxy.go`, `docs/implementer-log.md` ## Rules 1. `statusRecorder.Flush` becomes: assert with the two-value form, and call `Flush` only when it is there. Nothing else in the recorder changes. ```go if f, ok := r.ResponseWriter.(http.Flusher); ok { f.Flush() } ``` 2. Then **list every other type assertion in `internal/`** (`grep -n '\.(' internal/*/*.go`) and check each is either the two-value form or on a value you constructed yourself. Put the list, with one word per line saying why it is safe, in your log row's Notes. If you find another unchecked assertion on a value that came from outside the package, fix it the same way and say so in Deviations. 3. No change to `internal/config`: the two new tests must pass against the code as it is. If one does not, stop and report — that is a finding about `Load`, not something to patch around. ## Steps - [ ] **1. Branch and copy.** ```sh git switch master && git switch -c v0.1 cp docs/plans/v0.1/_files/internal/proxy/recorder_test.go internal/proxy/ cp docs/plans/v0.1/_files/internal/config/unreadable_test.go internal/config/ ``` - [ ] **2. See the recorder test fail.** `go test -run WithoutFlusher ./internal/proxy/`. Expected: `FAIL`, with `ServeHTTP panicked on a writer without Flush` (or a panic trace naming `statusRecorder.Flush`). If it passes already, stop and report. - [ ] **3. See the config tests pass as they are.** `go test -run 'Unreadable|Directory' ./internal/config/`. Expected: `ok`. - [ ] **4. Fix `Flush`** as in rule 1, then do the assertion inventory of rule 2. `gofmt -w internal/proxy/`. - [ ] **5. See everything pass.** `go test -race -count=1 ./...`. Expected: all `ok`. - [ ] **6. Run the gate.** `make gate`. Expected last line: `gate: ok`. - [ ] **7. Log and commit.** Row `v0.1/01-review-fixes`. Anything you changed that these rules did not name goes in **Deviations**, not only in Notes. ```sh git add internal/proxy internal/config/unreadable_test.go docs/implementer-log.md git commit ``` ## Done when - Step 2 failed before the fix and `go test -race -count=1 ./...` passes after it; `make gate` prints `gate: ok`. - `cmp` of both copied tests against `docs/plans/v0.1/_files/…` prints nothing. ## Stop and report if - Step 2 passes before any change, or step 3 fails: the plan's premise is wrong, and the owner needs to know before code moves.