Phase 2 of the audio-upload follow-up sprint (epic #54, plan docs/sprints/audio-upload-followups-2026-10.md), release v0.31.1. Red test commit then fix commit for every item; task check and golangci-lint run ./... exit 0 on the head.
#41task lint fails on findings (was A && B || echo). Test runs the real Taskfile in a scratch module.
#48 (risk/high)http.NewCrossOriginProtection() wraps Router(): foreign-origin POSTs get 403 on delete-account, uploads, upload chat and the listed other POST routes; same-origin and header-less pass; GET is never refused.
#49-max_streams 32 on ffprobe and ffmpeg (900-stream file: 564 MB RSS -> 18 MB, measured with the static 7.0.2 binaries), probe output narrowed with -show_entries and capped at 1 MiB, probe deadline 45 s (a timeout is the file's fault; a cancelled caller is not).
#47 item 1 Accept bounds concurrent body receives: 4 total, 2 per user, answered with the existing busy rejection before anything is spooled.
#50 SIGTERM handled; a shutdown records the running job interrupted at once on a 3 s context of its own; serveHTTP waits (10 s each) for in-flight requests and the worker instead of returning when Serve returns ErrServerClosed. ADR-033 amended: the full drain is a recorded limitation (D2).
Review lane: control code (#48, #49, the receive cap). Adversarial review verified by execution is required before merge; verdict goes in #48/#54.
Post-deploy checks: old pod Completed not Error after the rollout; curl -X POST -H 'Origin: https://evil.d-ma.be' https://tapir.d-ma.be/account/delete is 403 and the same with Origin: https://tapir.d-ma.be is not (proves the proxy preserves Host); a real upload still works.
Closes#41, #48, #49, #50. Part of #47 (item 1; items 2/3 moved to #49/#50).
Phase 2 of the audio-upload follow-up sprint (epic #54, plan `docs/sprints/audio-upload-followups-2026-10.md`), release v0.31.1. Red test commit then fix commit for every item; `task check` and `golangci-lint run ./...` exit 0 on the head.
- **#41** `task lint` fails on findings (was `A && B || echo`). Test runs the real Taskfile in a scratch module.
- **#48 (risk/high)** `http.NewCrossOriginProtection()` wraps `Router()`: foreign-origin POSTs get 403 on delete-account, uploads, upload chat and the listed other POST routes; same-origin and header-less pass; GET is never refused.
- **#49** `-max_streams 32` on ffprobe and ffmpeg (900-stream file: 564 MB RSS -> 18 MB, measured with the static 7.0.2 binaries), probe output narrowed with `-show_entries` and capped at 1 MiB, probe deadline 45 s (a timeout is the file's fault; a cancelled caller is not).
- **#47 item 1** Accept bounds concurrent body receives: 4 total, 2 per user, answered with the existing busy rejection before anything is spooled.
- **#50** SIGTERM handled; a shutdown records the running job `interrupted` at once on a 3 s context of its own; `serveHTTP` waits (10 s each) for in-flight requests and the worker instead of returning when `Serve` returns `ErrServerClosed`. ADR-033 amended: the full drain is a recorded limitation (D2).
**Review lane:** control code (#48, #49, the receive cap). Adversarial review verified by execution is required before merge; verdict goes in #48/#54.
**Post-deploy checks:** old pod `Completed` not `Error` after the rollout; `curl -X POST -H 'Origin: https://evil.d-ma.be' https://tapir.d-ma.be/account/delete` is 403 and the same with `Origin: https://tapir.d-ma.be` is not (proves the proxy preserves Host); a real upload still works.
Closes #41, #48, #49, #50. Part of #47 (item 1; items 2/3 moved to #49/#50).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
A 900-stream file cost ~570 MB RSS in each tool; with the cap the same file fails
in ~18 MB and is reported as unreadable.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Adversarial review round 1 (pr-gatekeeper, review-only, verified by execution): BLOCK on head 3e69c00. Recorded here and on #48/#54. The reviewer is an agent, so this is an independent pass, not four-eyes.
Blocking findings, all fixed test-first in 9939afb..4ef4465:
#50 FailUpload could overwrite a done row with failed/interrupted when a shutdown races the transcript commit (reproduced by the reviewer with a lost-ack wrapper on real Postgres: the row ended failed, text hidden). Fix: FailUpload only touches queued/transcribing. Tests TestFailUpload_LeavesAFinishedUploadAsItWas (red before the fix) and ..._StillFailsAQueuedOrTranscribingUpload.
#47 the production limits (4 total / 2 per user) had no test: setting both defaults to 1000 left the suite green. Fix: TestNewIntake_DefaultsAllowTwoReceivesPerUserAndFourOverall, which I confirmed fails when both defaults are 1000. The reviewer's spool arithmetic ((4+20+1) x 200 MiB = 5 GiB of 6 Gi) is now recorded in ADR-033.
#50TestServeHTTP_Returns... was vacuous for the in-flight half (Shutdown -> Close stayed green). Split into ..._DrainsAnInFlightRequestBeforeReturning (worker pre-satisfied) and ..._WaitsForTheWorkerBeforeReturning; the first goes red under Close.
Non-blocking, also done: ffmpeg's combined output is now a bounded head (a hostile mp3 produced ~83 MB of stderr, ~250-300 MB transient); the lint-gate test now asserts the finding text and that task check runs the lint step (removing - task: lint goes red).
Recorded, not built: slow-drip body holds a slot without a time bound -> #56 (ADR-033 says "unbounded"); shutdown logging/second signal -> #57.
What held under attack (reviewer's 33 requests against the real Router): foreign Origin/Sec-Fetch-Site/null/suffix and userinfo tricks, other methods and method-override all refused; /auth/* login/callback/logout still work; header-less and same-origin pass; -max_streams 32 works with the real binaries; capWriter boundary exact.
Not verified by the reviewer: CI conclusion, whether NPM rewrites Host (post-deploy curl check below), a real SIGTERM against the real binary.
Next: narrow re-review of 3e69c00..4ef4465, then merge on APPROVE.
**Adversarial review round 1 (pr-gatekeeper, review-only, verified by execution): BLOCK on head 3e69c00.** Recorded here and on #48/#54. The reviewer is an agent, so this is an independent pass, not four-eyes.
Blocking findings, all fixed test-first in 9939afb..4ef4465:
1. **#50 FailUpload could overwrite a `done` row** with `failed/interrupted` when a shutdown races the transcript commit (reproduced by the reviewer with a lost-ack wrapper on real Postgres: the row ended `failed`, text hidden). Fix: `FailUpload` only touches `queued`/`transcribing`. Tests `TestFailUpload_LeavesAFinishedUploadAsItWas` (red before the fix) and `..._StillFailsAQueuedOrTranscribingUpload`.
2. **#47 the production limits (4 total / 2 per user) had no test**: setting both defaults to 1000 left the suite green. Fix: `TestNewIntake_DefaultsAllowTwoReceivesPerUserAndFourOverall`, which I confirmed fails when both defaults are 1000. The reviewer's spool arithmetic ((4+20+1) x 200 MiB = 5 GiB of 6 Gi) is now recorded in ADR-033.
3. **#50 `TestServeHTTP_Returns...` was vacuous for the in-flight half** (`Shutdown` -> `Close` stayed green). Split into `..._DrainsAnInFlightRequestBeforeReturning` (worker pre-satisfied) and `..._WaitsForTheWorkerBeforeReturning`; the first goes red under `Close`.
Non-blocking, also done: ffmpeg's combined output is now a bounded head (a hostile mp3 produced ~83 MB of stderr, ~250-300 MB transient); the lint-gate test now asserts the finding text and that `task check` runs the lint step (removing `- task: lint` goes red).
Recorded, not built: slow-drip body holds a slot without a time bound -> #56 (ADR-033 says "unbounded"); shutdown logging/second signal -> #57.
What held under attack (reviewer's 33 requests against the real Router): foreign Origin/Sec-Fetch-Site/`null`/suffix and userinfo tricks, other methods and method-override all refused; `/auth/*` login/callback/logout still work; header-less and same-origin pass; `-max_streams 32` works with the real binaries; capWriter boundary exact.
Not verified by the reviewer: CI conclusion, whether NPM rewrites Host (post-deploy `curl` check below), a real SIGTERM against the real binary.
Next: narrow re-review of 3e69c00..4ef4465, then merge on APPROVE.
---
_Created via git-mcp on behalf of @mathias_
Adversarial review round 2 (pr-gatekeeper, review-only, verified by execution): APPROVE for delta 3e69c00..4ef4465. Agent review, not four-eyes.
FailUpload guard: lost-ack scenario on real Postgres + real Worker.Process ends status=done, text kept (without the guard: failed/interrupted). The committed test goes red when the guard is removed. Every FailUpload caller checked: the only rows now refused are done/failed, neither a spinner state; FailInterruptedUploads untouched.
Defaults test red on 9 flips (total/per-user, both directions, fallback ignored).
serveHTTP split tests: each half goes red independently (Shutdown->Close, worker wait removed, zero grace, unbounded wait); every failing run under 3.5 s, no hang.
ffmpeg bounded output: exit classification and ctx handling unchanged; shared headWriter race-free (-race -count=3, 3 MB on each stream, error 4126 bytes).
Lint-gate test red on: lint removed from check, vet substituted for lint, old && || echo restored, || true appended.
Reviewer's gates by $?: task check 0, golangci-lint 0, go test -race -count=2 0.
Non-blocking notes: Worker.fail now logs an error-level "record failure" after a lost ack (log noise only); shutdown.go still discards Shutdown's error (#57); the lint-gate finding test skips where golangci-lint is absent (CI installs it).
CI: run 8937 on 4ef4465 was red: my own TestTaskLint_SkipsWithANoticeWhenGolangciLintIsAbsent looked for the notice with a bare prefix and CI runs task with colour on (the line starts with ESC[0m), so it failed in CI while passing locally; the same blind spot would also have made the "did not blame a missing binary" check vacuous there. Fixed in 5baffd0 (red test first, FORCE_COLOR=1 locally now green). CI run 8938 on 5baffd0: success. The delta from the reviewed head is test-only. Merging.
**Adversarial review round 2 (pr-gatekeeper, review-only, verified by execution): APPROVE for delta 3e69c00..4ef4465.** Agent review, not four-eyes.
- FailUpload guard: lost-ack scenario on real Postgres + real Worker.Process ends `status=done`, text kept (without the guard: `failed/interrupted`). The committed test goes red when the guard is removed. Every FailUpload caller checked: the only rows now refused are `done`/`failed`, neither a spinner state; `FailInterruptedUploads` untouched.
- Defaults test red on 9 flips (total/per-user, both directions, fallback ignored).
- serveHTTP split tests: each half goes red independently (Shutdown->Close, worker wait removed, zero grace, unbounded wait); every failing run under 3.5 s, no hang.
- ffmpeg bounded output: exit classification and ctx handling unchanged; shared headWriter race-free (`-race -count=3`, 3 MB on each stream, error 4126 bytes).
- Lint-gate test red on: lint removed from `check`, vet substituted for lint, old `&& || echo` restored, `|| true` appended.
- Reviewer's gates by `$?`: `task check` 0, `golangci-lint` 0, `go test -race -count=2` 0.
Non-blocking notes: `Worker.fail` now logs an error-level "record failure" after a lost ack (log noise only); `shutdown.go` still discards `Shutdown`'s error (#57); the lint-gate finding test skips where golangci-lint is absent (CI installs it).
**CI:** run 8937 on 4ef4465 was red: my own `TestTaskLint_SkipsWithANoticeWhenGolangciLintIsAbsent` looked for the notice with a bare prefix and CI runs task with colour on (the line starts with `ESC[0m`), so it failed in CI while passing locally; the same blind spot would also have made the "did not blame a missing binary" check vacuous there. Fixed in 5baffd0 (red test first, `FORCE_COLOR=1` locally now green). **CI run 8938 on 5baffd0: success.** The delta from the reviewed head is test-only. Merging.
---
_Created via git-mcp on behalf of @mathias_
mathias
merged commit e037c6b683 into main2026-10-03 19:19:55 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Phase 2 of the audio-upload follow-up sprint (epic #54, plan
docs/sprints/audio-upload-followups-2026-10.md), release v0.31.1. Red test commit then fix commit for every item;task checkandgolangci-lint run ./...exit 0 on the head.task lintfails on findings (wasA && B || echo). Test runs the real Taskfile in a scratch module.http.NewCrossOriginProtection()wrapsRouter(): foreign-origin POSTs get 403 on delete-account, uploads, upload chat and the listed other POST routes; same-origin and header-less pass; GET is never refused.-max_streams 32on ffprobe and ffmpeg (900-stream file: 564 MB RSS -> 18 MB, measured with the static 7.0.2 binaries), probe output narrowed with-show_entriesand capped at 1 MiB, probe deadline 45 s (a timeout is the file's fault; a cancelled caller is not).interruptedat once on a 3 s context of its own;serveHTTPwaits (10 s each) for in-flight requests and the worker instead of returning whenServereturnsErrServerClosed. ADR-033 amended: the full drain is a recorded limitation (D2).Review lane: control code (#48, #49, the receive cap). Adversarial review verified by execution is required before merge; verdict goes in #48/#54.
Post-deploy checks: old pod
CompletednotErrorafter the rollout;curl -X POST -H 'Origin: https://evil.d-ma.be' https://tapir.d-ma.be/account/deleteis 403 and the same withOrigin: https://tapir.d-ma.beis not (proves the proxy preserves Host); a real upload still works.Closes #41, #48, #49, #50. Part of #47 (item 1; items 2/3 moved to #49/#50).
🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
Adversarial review round 1 (pr-gatekeeper, review-only, verified by execution): BLOCK on head
3e69c00. Recorded here and on #48/#54. The reviewer is an agent, so this is an independent pass, not four-eyes.Blocking findings, all fixed test-first in 9939afb..4ef4465:
donerow withfailed/interruptedwhen a shutdown races the transcript commit (reproduced by the reviewer with a lost-ack wrapper on real Postgres: the row endedfailed, text hidden). Fix:FailUploadonly touchesqueued/transcribing. TestsTestFailUpload_LeavesAFinishedUploadAsItWas(red before the fix) and..._StillFailsAQueuedOrTranscribingUpload.TestNewIntake_DefaultsAllowTwoReceivesPerUserAndFourOverall, which I confirmed fails when both defaults are 1000. The reviewer's spool arithmetic ((4+20+1) x 200 MiB = 5 GiB of 6 Gi) is now recorded in ADR-033.TestServeHTTP_Returns...was vacuous for the in-flight half (Shutdown->Closestayed green). Split into..._DrainsAnInFlightRequestBeforeReturning(worker pre-satisfied) and..._WaitsForTheWorkerBeforeReturning; the first goes red underClose.Non-blocking, also done: ffmpeg's combined output is now a bounded head (a hostile mp3 produced ~83 MB of stderr, ~250-300 MB transient); the lint-gate test now asserts the finding text and that
task checkruns the lint step (removing- task: lintgoes red).Recorded, not built: slow-drip body holds a slot without a time bound -> #56 (ADR-033 says "unbounded"); shutdown logging/second signal -> #57.
What held under attack (reviewer's 33 requests against the real Router): foreign Origin/Sec-Fetch-Site/
null/suffix and userinfo tricks, other methods and method-override all refused;/auth/*login/callback/logout still work; header-less and same-origin pass;-max_streams 32works with the real binaries; capWriter boundary exact.Not verified by the reviewer: CI conclusion, whether NPM rewrites Host (post-deploy
curlcheck below), a real SIGTERM against the real binary.Next: narrow re-review of 3e69c00..4ef4465, then merge on APPROVE.
Created via git-mcp on behalf of @mathias
Adversarial review round 2 (pr-gatekeeper, review-only, verified by execution): APPROVE for delta 3e69c00..4ef4465. Agent review, not four-eyes.
status=done, text kept (without the guard:failed/interrupted). The committed test goes red when the guard is removed. Every FailUpload caller checked: the only rows now refused aredone/failed, neither a spinner state;FailInterruptedUploadsuntouched.-race -count=3, 3 MB on each stream, error 4126 bytes).check, vet substituted for lint, old&& || echorestored,|| trueappended.$?:task check0,golangci-lint0,go test -race -count=20.Non-blocking notes:
Worker.failnow logs an error-level "record failure" after a lost ack (log noise only);shutdown.gostill discardsShutdown's error (#57); the lint-gate finding test skips where golangci-lint is absent (CI installs it).CI: run 8937 on
4ef4465was red: my ownTestTaskLint_SkipsWithANoticeWhenGolangciLintIsAbsentlooked for the notice with a bare prefix and CI runs task with colour on (the line starts withESC[0m), so it failed in CI while passing locally; the same blind spot would also have made the "did not blame a missing binary" check vacuous there. Fixed in5baffd0(red test first,FORCE_COLOR=1locally now green). CI run 8938 on5baffd0: success. The delta from the reviewed head is test-only. Merging.Created via git-mcp on behalf of @mathias