Compare commits

...
54 Commits
Author SHA1 Message Date
mathiasandClaude Sonnet 5.5 41ab1f5d50 fix(web): take the reconciled endpoint apart exactly, so a connector word used as a model name is still judged (#52)
CI / Lint / Test / Vet (pull_request) Successful in 35s
CI / Build & Import (pull_request) Skipped
CI / Deploy via GitOps (pull_request) Skipped
Skipping 'by', 'reconciled' and '+' as words everywhere let an alias with one of those
names pass as local. The two-engine shape is now split on its own separators and each alias
judged whole; a reconciler that is not local is pinned too. Hardening suggested by the third
PR #58 review.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-04 00:28:09 +02:00
mathiasandClaude Sonnet 5.5 67bd0350a3 fix(web): judge every word of a reconciled endpoint that is not a connector, so an unprefixed alias is not called local (#52)
CI / Lint / Test / Vet (pull_request) Successful in 33s
CI / Build & Import (pull_request) Skipped
CI / Deploy via GitOps (pull_request) Skipped
The previous version kept only words containing '/' and dropped the rest, so a bare
TAPIR_TRANSCRIBE_MODEL alias in the composite produced a false 'all local' result. Found
by the PR #58 re-review; red tests for a bare alias, an alias with a space and a split
'berget / x' first.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-04 00:22:41 +02:00
mathiasandClaude Sonnet 5.5 c24573bd9f docs: state what the save timeout and a racing delete actually guarantee (#51, #37)
CI / Lint / Test / Vet (pull_request) Successful in 29s
CI / Build & Import (pull_request) Skipped
CI / Deploy via GitOps (pull_request) Skipped
The tracker comment said 'never a stalled job'; each transition can wait up to the timeout.
The delete refusal is judged at the moment of the attempt. Found by the PR #58 review.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:32:14 +02:00
mathiasandClaude Sonnet 5.5 fc1e8e7ba8 test(web): a foreign post to the recording delete route leaves the recording, on an app that mounts uploads (#37, #48)
The earlier entry posted to an app without uploads, where the route does not exist, and
stayed green with the route removed. This one seeds a finished row and goes red without the
gate. Found by the PR #58 review.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:32:14 +02:00
mathiasandClaude Sonnet 5.5 4a0226d8bd test(llm): the response cap is pinned at its boundary and as the production default (#53)
Only the 8-byte override was exercised; >= vs >, a huge default and a non-positive option
all stayed green. Also says why the error-body read ignores its error. Found by the PR #58
review.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:31:30 +02:00
mathiasandClaude Sonnet 5.5 2e54d80a78 test(uploads): jobSink forwards the tracker's timestamp to the store (#51)
The worker fake ignored the heartbeat time, so the store-boundary half of the app-clock
fix had no assertion. Found by the PR #58 review.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:31:14 +02:00
mathiasandClaude Sonnet 5.5 01c1606caa test(progress): the default save and heartbeat deadline is pinned, not only the override (#51)
Every earlier test passed WithSaveTimeout; the worker passes nothing, so the 5 s default
was the only production value and a flip to 0, 1ms or 1h stayed green. Found by the PR #58
review; each flip is now red.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:30:58 +02:00
mathiasandClaude Sonnet 5.5 9aa01468ce fix(web): judge each alias of a reconciled endpoint, so an all-local transcript is not called cloud-served (#52)
The two-engine path writes '<base> + <alt>, reconciled by <model>'. IsLocalModel is a
single-alias match, so the composite was 'not local' and the note claimed the audio left
the homelab, naming a local host as the provider. Found by the PR #58 review.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:30:44 +02:00
mathiasandClaude Sonnet 5.5 25628a9994 test(web): the per-recording delete route is covered by the cross-origin gate (#37, #48)
CI / Lint / Test / Vet (pull_request) Successful in 30s
CI / Build & Import (pull_request) Skipped
CI / Deploy via GitOps (pull_request) Skipped
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:24 +02:00
mathiasandClaude Sonnet 5.5 a6691ac46b docs: per-recording delete and the recordings link are built; feature scenarios and coverage map (#37)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 c909b4e775 feat(web): confirm-then-delete control on a finished or failed recording (#37)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 f309a39027 feat(web): POST /uploads/{id}/delete, owner-scoped and refused while the recording is active (#37)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 89611164e3 test(web): per-recording delete is owner-scoped, refused while active, offered only when finished (#37), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 93860210ef feat(store): DeleteUpload removes a finished upload of the owner (#37)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 8509004eb7 test(store): an owner can delete a finished upload; an active or foreign one is refused (#37), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 453464a22b feat(web): the home list links to recordings when uploads are mounted (#37)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 4ef3e85d16 test(web): the home list links to recordings only when uploads are mounted (#37), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 b8af7f32c7 fix(progress): stamp the heartbeat with the application clock the page compares it with (#51)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 813192e5ba test: heartbeats are stamped with the application clock, not the database's NOW() (#51), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 2ec1557a8c fix(progress): bound every save and heartbeat write so a hung database cannot stall the job (#51)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 68815755e8 test(progress): a hung save or beat must not stall the reporter or the heartbeat (#51), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 ba18668796 docs(feature): cloud-served transcripts say the audio left the homelab; empty transcript also triggers the fallback (#52)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 a65f2f2492 feat(web): say in plain words when a recording's audio left the homelab (#52)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 a43ee19505 feat(web): helper naming the cloud provider of a transcript, one definition of local (#52)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 4301e691f5 test(web): a cloud-served transcript says the audio left the homelab (#52), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 61d1ff2255 fix(llm): read a bounded response body, 8 MiB on success and the error bound on failure (#53)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathiasandClaude Sonnet 5.5 9cc954df40 test(llm): the client reads a bounded response body, success and error (#53), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:22:17 +02:00
mathias e037c6b683 Merge pull request 'v0.31.1: cross-origin POST protection, ffprobe/ffmpeg limits, receive cap, SIGTERM, lint gate (#41 #47 #48 #49 #50)' (#55) from sprint/audio-followups into main
CI / Deploy via GitOps (push) Successful in 6s
CI / Lint / Test / Vet (push) Successful in 30s
CI / Build & Import (push) Successful in 10s
2026-10-03 19:19:55 +00:00
mathiasandClaude Sonnet 5.5 5baffd0189 fix(test): strip colour codes before looking for the lint skip notice (CI run 8937)
CI / Lint / Test / Vet (pull_request) Successful in 30s
CI / Build & Import (pull_request) Skipped
CI / Deploy via GitOps (pull_request) Skipped
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:18:05 +02:00
mathiasandClaude Sonnet 5.5 228bcc77da test(lint): the skip notice is found behind CI's colour codes, red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:18:01 +02:00
mathiasandClaude Sonnet 5.5 4ef446548d docs(adr): spool arithmetic and the unbounded slow-drip limitation for the receive cap (#47)
CI / Lint / Test / Vet (pull_request) Failing after 30s
CI / Build & Import (pull_request) Skipped
CI / Deploy via GitOps (pull_request) Skipped
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:12:48 +02:00
mathiasandClaude Sonnet 5.5 145949875e fix(audio): keep a bounded head of ffmpeg's output instead of buffering all of it (#49)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:12:48 +02:00
mathiasandClaude Sonnet 5.5 82aeac7c19 fix(store): FailUpload only fails a queued or transcribing upload (review of #55)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:12:48 +02:00
mathiasandClaude Sonnet 5.5 1b4747ed03 test: FailUpload must not overwrite a finished upload; ffmpeg stderr is bounded, red (review of #55)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:12:34 +02:00
mathiasandClaude Sonnet 5.5 9939afb90c test: pin the production receive limits, split the vacuous drain test, harden the lint-gate test (review of #55)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:12:34 +02:00
mathiasandClaude Sonnet 5.5 3e69c00a64 docs(adr): ADR-033 amended for SIGTERM handling and the recorded deploy limitation (#50, #47)
CI / Lint / Test / Vet (pull_request) Failing after 31s
CI / Build & Import (pull_request) Skipped
CI / Deploy via GitOps (pull_request) Skipped
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:00:55 +02:00
mathiasandClaude Sonnet 5.5 253aa567aa fix(serve): handle SIGTERM and wait for in-flight requests and the worker before exiting (#50)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:00:55 +02:00
mathiasandClaude Sonnet 5.5 5644dc3fc8 fix(uploads): record a job interrupted by shutdown at once, on its own short context (#50)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 21:00:55 +02:00
mathiasandClaude Sonnet 5.5 7df1de6628 test(serve): shutdown waits for in-flight requests and the worker, bounded (#50), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:59:57 +02:00
mathiasandClaude Sonnet 5.5 7e920102f6 test: SIGTERM cancels the process context; a shutdown mid-job records the row interrupted at once (#50), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:59:40 +02:00
mathiasandClaude Sonnet 5.5 480c7227b5 fix(uploads): bound concurrent request-body receives, 4 total and 2 per user (#47)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:59:09 +02:00
mathiasandClaude Sonnet 5.5 0eb3be9631 test(uploads): concurrent receives are bounded in total and per user (#47), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:57:31 +02:00
mathiasandClaude Sonnet 5.5 e8c5ac50bf fix(audio): give ffprobe its own 45s deadline, a timeout is the file's fault (#49)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:56:58 +02:00
mathiasandClaude Sonnet 5.5 6101dd4e2f test(audio): a hung ffprobe times out on its own deadline as unreadable (#49), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:56:47 +02:00
mathiasandClaude Sonnet 5.5 089444c19f fix(audio): stop reading ffprobe output past a cap and call the file unreadable (#49)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:56:47 +02:00
mathiasandClaude Sonnet 5.5 d44db88787 test(audio): ffprobe output past a cap is unreadable, not buffered (#49), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:56:37 +02:00
mathiasandClaude Sonnet 5.5 1f8bd849fe fix(audio): cap ffprobe/ffmpeg streams at 32 and narrow the probe output (#49)
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>
2026-10-03 20:56:28 +02:00
mathiasandClaude Sonnet 5.5 7b19801bfa test(audio): ffprobe/ffmpeg must cap streams, ffprobe must narrow its output (#49), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:55:54 +02:00
mathiasandClaude Sonnet 5.5 1260500d32 fix(web): refuse cross-origin non-safe requests at the router (#48)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:55:19 +02:00
mathiasandClaude Sonnet 5.5 10b4c43bd2 test(web): foreign-origin POSTs must be refused by the router (#48), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:55:00 +02:00
mathiasandClaude Sonnet 5.5 f1f8b6d02e fix(task): lint step fails on findings instead of printing 'not installed' (#41)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:54:20 +02:00
mathiasandClaude Sonnet 5.5 0d4c5c9d1d test(lint): task lint must exit non-zero on a finding (#41), red
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:53:50 +02:00
mathiasandClaude Sonnet 5.5 24d65b2085 docs(sprint): audio upload follow-ups sweep sprint plan, oath and decision list
CI / Lint / Test / Vet (push) Successful in 27s
CI / Build & Import (push) Successful in 22s
CI / Deploy via GitOps (push) Successful in 5s
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 20:50:34 +02:00
mathiasandClaude Sonnet 5.5 df7bd459ed docs(adr): the ingress body-size item is resolved, and uploads are on in production (infra#520)
CI / Build & Import (push) Successful in 13s
CI / Deploy via GitOps (push) Successful in 5s
CI / Lint / Test / Vet (push) Successful in 36s
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-03 16:29:32 +02:00
41 changed files with 2776 additions and 762 deletions
+28 -3
View File
@@ -1447,6 +1447,25 @@ an hour of Swedish *was* an unknown here; it is now measured, see decision 8.)
re-uploads. The single-replica assumption gains a second dependent (the semaphore is re-uploads. The single-replica assumption gains a second dependent (the semaphore is
process-local) and is recorded here. process-local) and is recorded here.
*Amended 2026-10-03 (#50, #47).* The process now handles SIGTERM (what Kubernetes sends on
every deploy; only SIGINT was handled, so shutdown code never ran). On shutdown the running job
is recorded `interrupted` at once and the process waits, bounded to 10 s each, for in-flight
requests and for that write; the start-up sweep stays the backstop for a kill. **Known
limitation, deliberately not built: a deploy still loses a job in flight** (finishing it needs
a drain of up to the job time, a ~330 s termination grace and `strategy: Recreate`, i.e.
downtime on every deploy; a 90-minute file takes ~4.5 min and a deploy lands on a running job
perhaps one time in five at current volume). Jobs still queued at shutdown stay `queued` until
the next start's sweep. The user re-uploads, with an honest message.
*Receive limits (#47).* Request bodies are received at most 4 at a time, 2 per user, answered
as busy beyond that. Together with the 20-job queue and the running job that bounds the spool
at about (4 + 20 + 1) x 200 MiB = 5 GiB of the 6 Gi `emptyDir` (infra): raising any of those or
`TAPIR_UPLOAD_MAX_BYTES` must revisit that sum (pinned by `TestNewIntake_Defaults...`, not
enforced against infra). **A slow-drip body holds its slot without a time bound**: the server
sets `ReadHeaderTimeout` only, because a `ReadTimeout` would cut legitimate 200 MiB uploads on
slow links; two slow bodies take a user's two slots and two such users make everyone's upload
busy. It needs a valid session; recorded as a limitation with a follow-up issue.
8. **Chat reuses `chat.Service`; it is not forked.** The uploaded-transcript chat handlers 8. **Chat reuses `chat.Service`; it is not forked.** The uploaded-transcript chat handlers
look the row up through the RLS-scoped store, so a non-owner gets 404 by the same mechanism look the row up through the RLS-scoped store, so a non-owner gets 404 by the same mechanism
as ADR-027's summary gate, **before any model is called**. ADR-027's decisions (ephemeral, as ADR-027's summary gate, **before any model is called**. ADR-027's decisions (ephemeral,
@@ -1531,9 +1550,15 @@ deferred. The scope cut — chat first, no rich analysis, no video — is what k
#33 specifies, with the serving endpoint shown **(recommended, matches the issue)**; (B) #33 specifies, with the serving endpoint shown **(recommended, matches the issue)**; (B)
disabled by default, a failed iguana job fails honestly and the user retries. *Blocks:* disabled by default, a failed iguana job fails honestly and the user retries. *Blocks:*
the production default only; the adapter ships either way. the production default only; the adapter ships either way.
- Verify Tapir's ingress `proxy-body-size` against the 200 MB cap (`infra`). - ~~Verify Tapir's ingress `proxy-body-size` against the 200 MB cap (`infra`).~~ **Resolved 2026-10-03:** real unauthenticated bodies of 2, 30, 120 and 210 MiB all pass NPM (TLS, piguard) and Traefik to tapir (a 302 to login); there is no body limit in that range. Uploads were enabled in production the same day, local engines only (infra#520).
- Per-transcript deletion is not built; account deletion cascades. A private-content store - ~~Per-transcript deletion is not built; account deletion cascades.~~ **Built 2026-10-03
without a delete button is a gap worth a ticket, not a silent omission. (#37):** `POST /uploads/{id}/delete`, owner-scoped (a non-owner gets 404 by the same RLS
mechanism), confirm-then-delete on the recording's page. A recording that is **queued or
transcribing is refused (409) until it ends**, because the worker would otherwise write to a
row that is gone; a job whose service was lost stays undeletable until the next start's sweep
marks it failed. The refusal reflects the row's state when the delete was attempted: a
recording that finishes in the same instant still gets a 409 once, and a retry deletes it.
The home list now links to `/uploads` when uploads are mounted.
**Reversibility.** Additive: one table (down-migration drops it), one adapter, one upload **Reversibility.** Additive: one table (down-migration drops it), one adapter, one upload
surface, one image change. Removing the upload route removes the feature; `transcripts`, `videos` surface, one image change. Removing the upload route removes the feature; `transcripts`, `videos`
+1 -1
View File
@@ -34,7 +34,7 @@ tasks:
lint: lint:
desc: golangci-lint (installed by CI; skipped locally if absent). desc: golangci-lint (installed by CI; skipped locally if absent).
cmds: cmds:
- 'command -v golangci-lint >/dev/null 2>&1 && golangci-lint run ./... || echo "golangci-lint not installed — skipping (CI installs it)"' - 'if command -v golangci-lint >/dev/null 2>&1; then golangci-lint run ./...; else echo "golangci-lint not installed — skipping (CI installs it)"; fi'
test: test:
desc: Run all tests. -p 1 serializes packages so the embedded-postgres tests desc: Run all tests. -p 1 serializes packages so the embedded-postgres tests
+17 -16
View File
@@ -17,9 +17,9 @@ import (
"errors" "errors"
"fmt" "fmt"
"log/slog" "log/slog"
"net"
"net/http" "net/http"
"os" "os"
"os/signal"
"sync" "sync"
"time" "time"
@@ -41,7 +41,7 @@ func main() {
} }
log := slog.New(slog.NewTextHandler(os.Stderr, nil)) log := slog.New(slog.NewTextHandler(os.Stderr, nil))
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) ctx, stop := shutdownContext(context.Background())
defer stop() defer stop()
var err error var err error
@@ -215,9 +215,11 @@ func cmdServe(ctx context.Context, log *slog.Logger) error {
if err != nil { if err != nil {
return err return err
} }
var workerDone chan struct{} // nil without uploads: nothing to wait for at shutdown
wireUploads(app, cfg, intake) wireUploads(app, cfg, intake)
if intake != nil { if intake != nil {
go worker.Run(ctx) workerDone = make(chan struct{})
go func() { worker.Run(ctx); close(workerDone) }()
log.Info("audio upload enabled", "model", cfg.TranscribeModel, "fallback", cfg.TranscribeFallbackModel, log.Info("audio upload enabled", "model", cfg.TranscribeModel, "fallback", cfg.TranscribeFallbackModel,
"max_mb", cfg.UploadMaxBytes>>20, "max_minutes", int(cfg.UploadMaxDuration.Minutes())) "max_mb", cfg.UploadMaxBytes>>20, "max_minutes", int(cfg.UploadMaxDuration.Minutes()))
log.Warn("audio upload assumes a SINGLE replica: the transcription queue is process-local (ADR-033, ADR-018)") log.Warn("audio upload assumes a SINGLE replica: the transcription queue is process-local (ADR-033, ADR-018)")
@@ -350,20 +352,19 @@ func cmdServe(ctx context.Context, log *slog.Logger) error {
}() }()
} }
// Graceful shutdown on signal: stop accepting, drain in-flight requests. // Graceful shutdown on signal: stop accepting, drain in-flight requests, and let the
go func() { // worker record the job it was running as interrupted (#50).
<-ctx.Done() var extra []*http.Server
shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) if metricsSrv != nil {
defer cancel() extra = append(extra, metricsSrv)
_ = srv.Shutdown(shutdownCtx) }
if metricsSrv != nil { ln, err := net.Listen("tcp", srv.Addr)
_ = metricsSrv.Shutdown(shutdownCtx) if err != nil {
} return fmt.Errorf("serve: listen: %w", err)
}() }
log.Info("serving web ui", "addr", cfg.HTTPAddr, "user", cfg.UserID) log.Info("serving web ui", "addr", cfg.HTTPAddr, "user", cfg.UserID)
if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { if err := serveHTTP(ctx, srv, ln, extra, workerDone, shutdownGrace); err != nil {
return fmt.Errorf("serve: %w", err) return err
} }
return nil return nil
} }
+55
View File
@@ -0,0 +1,55 @@
package main
import (
"context"
"errors"
"fmt"
"net"
"net/http"
"os"
"os/signal"
"syscall"
"time"
)
// shutdownGrace bounds each wait at shutdown (in-flight requests, then the worker). Two of
// them stay inside Kubernetes' default 30 s termination grace.
const shutdownGrace = 10 * time.Second
// shutdownContext is cancelled by SIGINT (a terminal) or SIGTERM (what Kubernetes sends
// on every deploy). Handling only SIGINT meant a deploy killed the process before any
// shutdown code ran (#50).
func shutdownContext(parent context.Context) (context.Context, context.CancelFunc) {
return signal.NotifyContext(parent, os.Interrupt, syscall.SIGTERM)
}
// serveHTTP serves srv on ln until it fails or ctx ends, then shuts every server down and
// waits for in-flight requests and for workerDone (nil: no worker) to finish, each for at
// most grace. It must not return earlier: Serve returns http.ErrServerClosed as soon as
// Shutdown STARTS, and returning from main kills the drain and the worker's last write.
func serveHTTP(ctx context.Context, srv *http.Server, ln net.Listener, extra []*http.Server, workerDone <-chan struct{}, grace time.Duration) error {
errc := make(chan error, 1)
go func() { errc <- srv.Serve(ln) }()
select {
case err := <-errc:
if !errors.Is(err, http.ErrServerClosed) {
return fmt.Errorf("serve: %w", err)
}
return nil
case <-ctx.Done():
}
shutdownCtx, cancel := context.WithTimeout(context.Background(), grace)
defer cancel()
for _, s := range append([]*http.Server{srv}, extra...) {
_ = s.Shutdown(shutdownCtx)
}
if workerDone != nil {
select {
case <-workerDone:
case <-time.After(grace):
}
}
return nil
}
+124
View File
@@ -0,0 +1,124 @@
package main
import (
"context"
"net/http"
"net/http/httptest"
"os"
"sync"
"syscall"
"testing"
"time"
)
// Kubernetes stops a pod with SIGTERM; only SIGINT was handled, so a deploy killed the
// process without running any shutdown code (#50).
func TestShutdownContext_IsCancelledBySIGTERMAndSIGINT(t *testing.T) {
for _, sig := range []syscall.Signal{syscall.SIGTERM, syscall.SIGINT} {
t.Run(sig.String(), func(t *testing.T) {
ctx, stop := shutdownContext(context.Background())
defer stop()
if err := syscall.Kill(os.Getpid(), sig); err != nil {
t.Fatalf("send %v: %v", sig, err)
}
select {
case <-ctx.Done():
case <-time.After(2 * time.Second):
t.Fatalf("context not cancelled by %v", sig)
}
})
}
}
// serveHTTP must not return while a request is in flight or the worker is still recording
// its job: Serve returns http.ErrServerClosed as soon as Shutdown STARTS, and returning
// from it let the process exit before either finished (#50). Each half is tested with the
// other already satisfied, so neither can hold the call open on the other's behalf.
func pendingFor(t *testing.T, returned <-chan error, why string) {
t.Helper()
select {
case err := <-returned:
t.Fatalf("serveHTTP returned (%v) %s", err, why)
case <-time.After(150 * time.Millisecond):
}
}
func TestServeHTTP_DrainsAnInFlightRequestBeforeReturning(t *testing.T) {
release := make(chan struct{})
entered := make(chan struct{})
srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
close(entered)
<-release
_, _ = w.Write([]byte("ok"))
}))
srv.Start()
defer srv.Close()
var once sync.Once
releaseHandler := func() { once.Do(func() { close(release) }) }
defer releaseHandler() // runs before srv.Close: a failing test must not hang on the blocked handler
ctx, cancel := context.WithCancel(context.Background())
workerDone := make(chan struct{})
close(workerDone) // the worker is already done: only the request can hold serveHTTP open
returned := make(chan error, 1)
go func() { returned <- serveHTTP(ctx, srv.Config, srv.Listener, nil, workerDone, 5*time.Second) }()
go func() { _, _ = http.Get(srv.URL) }()
<-entered
cancel()
pendingFor(t, returned, "while a request was still in flight")
releaseHandler()
select {
case err := <-returned:
if err != nil {
t.Fatalf("serveHTTP: %v", err)
}
case <-time.After(2 * time.Second):
t.Fatal("serveHTTP did not return once the request finished")
}
}
func TestServeHTTP_WaitsForTheWorkerBeforeReturning(t *testing.T) {
srv := httptest.NewUnstartedServer(http.NotFoundHandler())
srv.Start()
defer srv.Close()
ctx, cancel := context.WithCancel(context.Background())
workerDone := make(chan struct{})
returned := make(chan error, 1)
go func() { returned <- serveHTTP(ctx, srv.Config, srv.Listener, nil, workerDone, 5*time.Second) }() // no request in flight
cancel()
pendingFor(t, returned, "before the worker finished")
close(workerDone)
select {
case err := <-returned:
if err != nil {
t.Fatalf("serveHTTP: %v", err)
}
case <-time.After(2 * time.Second):
t.Fatal("serveHTTP did not return once the worker finished")
}
}
func TestServeHTTP_GivesUpOnAWorkerThatNeverFinishesAfterTheGrace(t *testing.T) {
srv := httptest.NewUnstartedServer(http.NotFoundHandler())
srv.Start()
defer srv.Close()
ctx, cancel := context.WithCancel(context.Background())
returned := make(chan error, 1)
go func() {
returned <- serveHTTP(ctx, srv.Config, srv.Listener, nil, make(chan struct{}), 200*time.Millisecond)
}()
cancel()
select {
case <-returned:
case <-time.After(3 * time.Second):
t.Fatal("a stuck worker must not hold the process past the grace period")
}
}
+1 -1
View File
@@ -44,7 +44,7 @@ func (f *fakeUploadsStore) SaveProgress(context.Context, string, string, []domai
return nil return nil
} }
func (f *fakeUploadsStore) Heartbeat(context.Context, string, string) error { return nil } func (f *fakeUploadsStore) Heartbeat(context.Context, string, string, time.Time) error { return nil }
func (f *fakeUploadsStore) FailInterruptedUploads(context.Context) (int, error) { func (f *fakeUploadsStore) FailInterruptedUploads(context.Context) (int, error) {
f.sweeps++ f.sweeps++
+2 -1
View File
@@ -14,7 +14,8 @@ it** — endpoints and aliases drift, and this file is a snapshot (2026-06-06),
not LiteLLM. Use `30401` for off-cluster LiteLLM. `confirm` resolved. not LiteLLM. Use `30401` for off-cluster LiteLLM. `confirm` resolved.
- **Speech-to-text (ADR-033):** the same gateway serves `POST /v1/audio/transcriptions` for - **Speech-to-text (ADR-033):** the same gateway serves `POST /v1/audio/transcriptions` for
`iguana/kb-whisper` (primary, local) and `berget/kb-whisper` (fallback, EU cloud — private audio `iguana/kb-whisper` (primary, local) and `berget/kb-whisper` (fallback, EU cloud — private audio
leaves the homelab if it is used). **Call it via the NodePort / in-cluster address above, NOT leaves the homelab if it is used: when the primary fails **or returns an empty transcript**,
e.g. a silent or music-only file; the transcript page then says so, #52). **Call it via the NodePort / in-cluster address above, NOT
`https://llm-api.d-ma.be`:** that public ingress (openresty) returns `413` for bodies over `https://llm-api.d-ma.be`:** that public ingress (openresty) returns `413` for bodies over
roughly 8–10 MB (~5 minutes of 16 kHz WAV), while the NodePort took 174 MB (90 minutes) in one roughly 8–10 MB (~5 minutes of 16 kHz WAV), while the NodePort took 174 MB (90 minutes) in one
request (measured 2026-10-01, `docs/research/audio-length-ceiling-2026-10-01.md`). Send multipart request (measured 2026-10-01, `docs/research/audio-length-ceiling-2026-10-01.md`). Send multipart
@@ -0,0 +1,88 @@
# Sprint: audio upload follow-ups (Sweep Sprint, 2026-10)
Method: Sweep Sprint (phase 1 parallel read-only research per issue: **done**; phase 2 sequential
execution, one issue per commit: **not started, waits for the owner's go**). Closing oath below.
Milestones: [v0.31.1 - security and robustness](https://git.d-ma.be/mathias/tapir/milestone/10),
[v0.32.0 - audio upload follow-ups](https://git.d-ma.be/mathias/tapir/milestone/11).
Context: v0.31.0 shipped audio upload (ADR-033/034/035) and is live in production behind auth at
`/uploads`, with the Berget cloud fallback on for audio (infra#522). A real 63.7 min upload
transcribed in 2m53s with no errors. The phase-1 investigation found two pre-existing or new
weaknesses an authenticated user can reach, so the first release is a hotfix.
## Oath (closing gates, every clause must hold; blocked = descoped and tracked, never "done")
| # | Clause | Evidence |
|---|--------|----------|
| SG1 | Every issue in the sprint is closed with a linked commit, or moved out with a stated reason | issue list of both milestones |
| SG2 | Each fix has a test that was seen red on main first, named after the mechanism it observes | red/green output in the issue |
| SG3 | `task check` AND `golangci-lint run ./...` exit 0 (checked by `$?`, not by grep); `task check` does not gate lint until #41 | gate output |
| SG4 | Control code (#48, #49, the upload cap in #47) has a recorded adversarial review verified by execution, verdict in the issue | gatekeeper verdict comment |
| SG5 | Docs current in the same commit: ADR-033/035 text, feature file, use-case coverage test | diff |
| SG6 | Each release tagged (v0.31.1, v0.32.0), pushed, deployed, and observed live (pod running the new image, no restart loop, a real upload still works) | `kubectl`/log + one upload |
| SG7 | Learnings written to the brain; deferred work filed as issues; decisions that are the owner's recorded as DECISION NEEDED | brain ids + issue numbers |
## Work order
### v0.31.1 (security and robustness; hotfix, do first)
1. **#41 lint gate.** `task check` swallows lint findings (`|| echo`). Make the gate fail on findings,
fix what it surfaces. First, so every later gate means something. (Decision D1.)
2. **#48 cross-origin POST protection.** `http.NewCrossOriginProtection()` around `Router()`.
Red: foreign-origin POST to `/account/delete`, `/uploads`, `/uploads/{id}/chat` is refused.
3. **#49 ffprobe/ffmpeg limits.** `-max_streams 32` + narrower probe output; probe stdout cap;
probe deadline. Three commits.
4. **#47 concurrent-upload cap (remaining item).** Bound the spool: reservation per accepted upload
against the 6Gi emptyDir so concurrent uploads cannot evict the pod.
5. **#50 SIGTERM interim.** Handle SIGTERM; record the interrupted job at once; fix the false
comment. Full drain recorded as a limitation. (Decision D2.)
Release v0.31.1: one adversarial review round over 2-5, then tag, push, observe.
### v0.32.0 (follow-ups)
6. **#52** label a cloud-served transcript in plain words; document the empty-transcript trigger.
7. **#53** bound the LLM client's response body read.
8. **#51** tracker: save timeout, then app-clock heartbeat.
9. **#37** nav link to uploads and per-recording delete (queued/transcribing: refuse until it
ends). (Decision D3.)
10. **#36** chat default model after measurement. (Decision D4.)
11. **#38** measurements (Berget length limit, large models on production iguana). (D5, D6.)
12. **#46** remainder of the hardening list after the items above are split out.
13. **#45** evaluate Berget as the reconciler: a question, not work. (D8.)
## DECISION NEEDED (owner's call; recommendation first)
| # | Question | Recommendation | Blocks |
|---|----------|----------------|--------|
| D1 | #41: should any linter be allowed to opt out of the gate? | No opt-out; fix findings | #41 |
| D2 | #50: full SIGTERM drain, or record as a limitation and ship the interim fix? | Limitation + interim; a drain costs 4-6 h and downtime on every deploy at current volume | #50 |
| D3 | #37: delete of a queued/transcribing recording | Refuse until it ends | #37 |
| D4 | #36: make a 31B model the chat default? | Only after measurement, conditional | #36 |
| D5 | #38: Berget length measurement needs a throwaway key | Yes, throwaway key | #38 |
| D6 | #38: load large models on production iguana | Yes, in a quiet window with the owner present | #38 |
| D7 | #46: where the ffmpeg tarball lives | Do nothing now; file an issue | none |
| D8 | #45: evaluate Berget as the reconciler | No; stay local-only | #45 |
| D9 | Who judges ADR-034's second blind check | The owner; do not reuse the one-off Opus authorisation | ADR-034 |
| D10 | When to enable reconcile in production | Needs `iguana/pianissimo` in the key pin and env vars; owner present | ADR-034 |
## Dropped or reduced, with reasons
- **#46 item 1 (`alis`/`dref` external reference):** checked, not a problem. Under the application's
flags ffmpeg skips the external track and writes a 78-byte WAV.
- **#46 item 2 / #47 item 2:** merged into #49.
- **#47 item 3 (full drain):** reduced to #50 plus a recorded limitation.
- **#46 `Beat` taking the tracker mutex:** incorrect in #46; only `logStill` does. Corrected in #51.
## Human-present items (do not run unattended)
- Production iguana model loads (#38, D6).
- Enabling reconcile in production (D10).
- Any key change or `apply` in infra (infra#523, infra#524 are open and not part of this sprint).
## Release plan
SemVer: v0.31.1 patch (fixes, no new behaviour); v0.32.0 minor (nav link, delete, cloud labelling).
Every push to tapir main redeploys the pod and fails any job in flight: push when no upload is
running (check the pod log first).
+18
View File
@@ -240,6 +240,24 @@ Feature: Upload an audio recording, get a private Swedish transcript, discuss it
Then it is transcribed anyway and its audio step says how long the audio really is Then it is transcribed anyway and its audio step says how long the audio really is
And a complete recording whose header over-estimated its length is not refused And a complete recording whose header over-estimated its length is not refused
Scenario: A transcript made by the cloud fallback says the audio left the homelab
When the transcript was produced by an endpoint that is not local
Then its page says in plain words that the audio left the homelab and names the provider
And a locally transcribed recording carries no such note
Scenario: I can delete a finished recording
When I confirm deleting a done or failed recording
Then the recording and its transcript are gone and I am back on my list
And another user's recording is a 404 and is untouched
Scenario: A recording still being transcribed cannot be deleted
When I try to delete a recording that is queued or transcribing
Then I am told it is still being transcribed and nothing is removed
Scenario: The home list links to my recordings when uploads are on
Given audio upload is enabled
Then the home list links to the recordings page
Scenario: A full queue is a retry, not an error Scenario: A full queue is a retry, not an error
When too many recordings are already waiting When too many recordings are already waiting
Then the upload is answered as busy, its row is failed honestly and its audio is deleted Then the upload is answered as busy, its row is failed honestly and its audio is deleted
+6 -2
View File
@@ -47,11 +47,15 @@ func (f *FFmpeg) ToWAV(ctx context.Context, src, dst string) error {
args = append(args, "-f", "wav", dst) args = append(args, "-f", "wav", dst)
cmd := exec.CommandContext(ctx, bin, args...) cmd := exec.CommandContext(ctx, bin, args...)
cmd.WaitDelay = killGrace cmd.WaitDelay = killGrace
if out, err := cmd.CombinedOutput(); err != nil { // Only a bounded head of ffmpeg's output is kept: a hostile file made it print ~0.4
// bytes per input byte, all of which CombinedOutput would buffer and copy (#49).
out := &headWriter{max: maxStderr}
cmd.Stdout, cmd.Stderr = out, out
if err := cmd.Run(); err != nil {
if ctxErr := ctx.Err(); ctxErr != nil { if ctxErr := ctx.Err(); ctxErr != nil {
return fmt.Errorf("audio: ffmpeg: %w", ctxErr) return fmt.Errorf("audio: ffmpeg: %w", ctxErr)
} }
return fmt.Errorf("audio: ffmpeg: %w: %s", err, strings.TrimSpace(string(out))) return fmt.Errorf("audio: ffmpeg: %w: %s", err, strings.TrimSpace(out.buf.String()))
} }
// A zero exit that wrote nothing is still a failure: never hand whisper a // A zero exit that wrote nothing is still a failure: never hand whisper a
// missing file and let the error surface three layers later. // missing file and let the error surface three layers later.
+29
View File
@@ -109,3 +109,32 @@ printf data > "$last"`)
require.NoError(t, err) require.NoError(t, err)
assert.NotContains(t, " "+strings.ReplaceAll(string(raw), "\n", " ")+" ", " -t ") assert.NotContains(t, " "+strings.ReplaceAll(string(raw), "\n", " ")+" ", " -t ")
} }
func TestFFmpegToWAV_CapsTheStreamCountBeforeTheInput(t *testing.T) {
// ffmpeg also allocated ~570 MB opening the 900-stream file (#49).
rec := filepath.Join(t.TempDir(), "argv")
bin := stub(t, `printf '%s\n' "$@" > `+rec+`
for a; do last="$a"; done
printf data > "$last"`)
err := (&audio.FFmpeg{Bin: bin}).ToWAV(context.Background(), "/spool/in", filepath.Join(t.TempDir(), "o.wav"))
require.NoError(t, err)
args := argvOf(t, rec)
ms, in := indexOf(args, "-max_streams"), indexOf(args, "-i")
require.GreaterOrEqual(t, ms, 0, "argv: %v", args)
assert.Equal(t, "32", args[ms+1])
assert.Less(t, ms, in, "an input option placed after -i is applied to the output")
}
func TestFFmpegToWAV_ABoundedPartOfAFloodingStderrIsKept(t *testing.T) {
// A hostile mp3 made ffmpeg print ~0.4 bytes of stderr per input byte (83 MB for a
// 209 MB file), all of it buffered, then copied twice into the error (#49 review).
bin := stub(t, `head -c 5000000 /dev/zero | tr '\0' 'e' >&2; exit 1`)
err := (&audio.FFmpeg{Bin: bin}).ToWAV(context.Background(), "/spool/in", filepath.Join(t.TempDir(), "o.wav"))
require.Error(t, err)
assert.Less(t, len(err.Error()), 16*1024, "the error carries a bounded head of stderr, not all of it")
assert.Contains(t, err.Error(), "eeee", "and still carries some of it")
}
+85 -9
View File
@@ -4,6 +4,7 @@
package audio package audio
import ( import (
"bytes"
"context" "context"
"encoding/json" "encoding/json"
"errors" "errors"
@@ -19,6 +20,51 @@ import (
type FFprobe struct { type FFprobe struct {
// Bin is the ffprobe executable ("ffprobe" resolves via PATH). // Bin is the ffprobe executable ("ffprobe" resolves via PATH).
Bin string Bin string
// MaxOutput bounds how much of ffprobe's stdout is read; zero means DefaultMaxOutput.
MaxOutput int
// Timeout bounds one probe; zero means DefaultProbeTimeout. A probe is milliseconds of
// work on a real file, so one that runs this long is the FILE's doing (#49).
Timeout time.Duration
}
// DefaultProbeTimeout is the probe deadline when FFprobe.Timeout is unset.
const DefaultProbeTimeout = 45 * time.Second
// DefaultMaxOutput is far above the narrowed probe output of a legitimate file (a few
// hundred bytes) and far below what a hostile one can make ffprobe print.
const DefaultMaxOutput = 1 << 20
// maxStderr is how much of ffprobe's stderr is kept for the error message.
const maxStderr = 4096
// errOutputCap is the write error that stops ffprobe once it prints past the cap.
var errOutputCap = errors.New("audio: ffprobe output exceeds the cap")
// capWriter collects up to max bytes and then refuses further writes, which closes the
// pipe and ends the process instead of buffering whatever it prints.
type capWriter struct {
buf bytes.Buffer
max int
}
func (w *capWriter) Write(p []byte) (int, error) {
if w.buf.Len()+len(p) > w.max {
return 0, errOutputCap
}
return w.buf.Write(p)
}
// headWriter keeps the first max bytes and silently drops the rest.
type headWriter struct {
buf bytes.Buffer
max int
}
func (w *headWriter) Write(p []byte) (int, error) {
if room := w.max - w.buf.Len(); room > 0 {
w.buf.Write(p[:min(room, len(p))])
}
return len(p), nil
} }
// AllowedDemuxers are the input formats an upload may be. The input is // AllowedDemuxers are the input formats an upload may be. The input is
@@ -30,9 +76,22 @@ type FFprobe struct {
// formats. "mov" covers .mp4/.m4a/.3gp; "matroska" covers .webm. // formats. "mov" covers .mp4/.m4a/.3gp; "matroska" covers .webm.
const AllowedDemuxers = "wav,mov,mp4,m4a,3gp,mp3,ogg,matroska,webm,flac,aac,opus,aiff,amr,asf" const AllowedDemuxers = "wav,mov,mp4,m4a,3gp,mp3,ogg,matroska,webm,flac,aac,opus,aiff,amr,asf"
// MaxStreams bounds how many streams a file may declare. A 1.3 MB file declaring 900 of
// them cost ~570 MB of RSS in each of ffprobe and ffmpeg (#49), against a 512 Mi pod;
// real recordings have a handful.
const MaxStreams = "32"
// inputRestrictions are ffmpeg/ffprobe INPUT options and must precede the input. // inputRestrictions are ffmpeg/ffprobe INPUT options and must precede the input.
// Local files and pipes only: no network, no other protocols. // Local files and pipes only: no network, no other protocols, and a bounded stream count.
var inputRestrictions = []string{"-protocol_whitelist", "file,pipe", "-format_whitelist", AllowedDemuxers} var inputRestrictions = []string{
"-protocol_whitelist", "file,pipe",
"-format_whitelist", AllowedDemuxers,
"-max_streams", MaxStreams,
}
// probeEntries is exactly what ParseProbe reads. -show_format/-show_streams print every
// tag too, and a 120 MB WAV made 491 MB of JSON that Output() buffers.
const probeEntries = "format=duration:stream=codec_type:stream_disposition=attached_pic"
// killGrace bounds how long Probe waits for a cancelled ffprobe's pipes to close. // killGrace bounds how long Probe waits for a cancelled ffprobe's pipes to close.
const killGrace = 2 * time.Second const killGrace = 2 * time.Second
@@ -48,21 +107,38 @@ func (f *FFprobe) Probe(ctx context.Context, path string) (uploads.Probe, error)
bin = "ffprobe" bin = "ffprobe"
} }
args := append([]string{"-v", "error"}, inputRestrictions...) args := append([]string{"-v", "error"}, inputRestrictions...)
args = append(args, "-print_format", "json", "-show_format", "-show_streams", path) args = append(args, "-print_format", "json", "-show_entries", probeEntries, path)
cmd := exec.CommandContext(ctx, bin, args...) timeout := f.Timeout
if timeout <= 0 {
timeout = DefaultProbeTimeout
}
probeCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
cmd := exec.CommandContext(probeCtx, bin, args...)
cmd.WaitDelay = killGrace cmd.WaitDelay = killGrace
out, err := cmd.Output() maxOut := f.MaxOutput
if err != nil { if maxOut <= 0 {
if ctxErr := ctx.Err(); ctxErr != nil { maxOut = DefaultMaxOutput
}
stdout, stderr := &capWriter{max: maxOut}, &headWriter{max: maxStderr}
cmd.Stdout, cmd.Stderr = stdout, stderr
if err := cmd.Run(); err != nil {
if ctxErr := ctx.Err(); ctxErr != nil { // the caller gave up: not a verdict on the file
return uploads.Probe{}, fmt.Errorf("audio: ffprobe: %w", ctxErr) return uploads.Probe{}, fmt.Errorf("audio: ffprobe: %w", ctxErr)
} }
if probeCtx.Err() != nil { // our own deadline passed while the caller was still waiting
return uploads.Probe{}, fmt.Errorf("%w: ffprobe did not finish within %s", uploads.ErrUnreadable, timeout)
}
if errors.Is(err, errOutputCap) {
return uploads.Probe{}, fmt.Errorf("%w: ffprobe printed more than %d bytes", uploads.ErrUnreadable, maxOut)
}
var exit *exec.ExitError var exit *exec.ExitError
if errors.As(err, &exit) { if errors.As(err, &exit) {
return uploads.Probe{}, fmt.Errorf("%w: ffprobe: %s", uploads.ErrUnreadable, exit.Stderr) return uploads.Probe{}, fmt.Errorf("%w: ffprobe: %s", uploads.ErrUnreadable, stderr.buf.String())
} }
return uploads.Probe{}, fmt.Errorf("audio: run ffprobe: %w", err) return uploads.Probe{}, fmt.Errorf("audio: run ffprobe: %w", err)
} }
p, err := ParseProbe(out) p, err := ParseProbe(stdout.buf.Bytes())
if err != nil { if err != nil {
return uploads.Probe{}, fmt.Errorf("audio: ffprobe output: %w", err) return uploads.Probe{}, fmt.Errorf("audio: ffprobe output: %w", err)
} }
+75
View File
@@ -184,3 +184,78 @@ func TestAllowedDemuxers_ExcludeThePlaylistFormats(t *testing.T) {
assert.Contains(t, strings.Split(audio.AllowedDemuxers, ","), good) assert.Contains(t, strings.Split(audio.AllowedDemuxers, ","), good)
} }
} }
func TestFFprobeProbe_CapsTheStreamCountBeforeTheInput(t *testing.T) {
// A 1.3 MB file declaring 900 streams cost ~570 MB of RSS in ffprobe (#49) against
// a 512 Mi pod. -max_streams is an input option: it must precede the path.
bin, argvFile := argvStub(t)
_, err := (&audio.FFprobe{Bin: bin}).Probe(context.Background(), "/spool/upload-1")
require.NoError(t, err)
args := argvOf(t, argvFile)
ms := indexOf(args, "-max_streams")
require.GreaterOrEqual(t, ms, 0, "argv: %v", args)
assert.Equal(t, "32", args[ms+1])
assert.Less(t, ms, len(args)-1, "must come before the path, which stays last")
}
func TestFFprobeProbe_AsksOnlyForTheFieldsParseProbeReads(t *testing.T) {
// -show_format/-show_streams print every tag: a 120 MB WAV produced 491 MB of JSON
// that cmd.Output() buffers in Go (#49).
bin, argvFile := argvStub(t)
_, err := (&audio.FFprobe{Bin: bin}).Probe(context.Background(), "/spool/upload-1")
require.NoError(t, err)
args := argvOf(t, argvFile)
assert.Equal(t, -1, indexOf(args, "-show_format"), "argv: %v", args)
assert.Equal(t, -1, indexOf(args, "-show_streams"), "argv: %v", args)
se := indexOf(args, "-show_entries")
require.GreaterOrEqual(t, se, 0, "argv: %v", args)
assert.Equal(t, "format=duration:stream=codec_type:stream_disposition=attached_pic", args[se+1])
}
func TestFFprobeProbe_OutputPastTheCapIsUnreadableNotBuffered(t *testing.T) {
// ffprobe's stdout is attacker-influenced (tags, 491 MB observed). Past the cap the
// probe is stopped and the file is reported as unreadable (#49).
bin := stub(t, `head -c 200000 /dev/zero | tr '\0' 'a'`)
_, err := (&audio.FFprobe{Bin: bin, MaxOutput: 4096}).Probe(context.Background(), "/x")
assert.ErrorIs(t, err, uploads.ErrUnreadable)
}
func TestFFprobeProbe_OutputWithinTheCapIsParsed(t *testing.T) {
bin := stub(t, `echo '{"streams":[{"codec_type":"audio"}],"format":{"duration":"90"}}'`)
got, err := (&audio.FFprobe{Bin: bin, MaxOutput: 4096}).Probe(context.Background(), "/x")
require.NoError(t, err)
assert.True(t, got.HasAudio)
}
func TestFFprobeProbe_AHungProbeTimesOutOnItsOwnDeadlineAsUnreadableNotAsOurFault(t *testing.T) {
// Accept runs the probe on the request context, which has no deadline of its own;
// a probe that never returns would pin a request (#49). `exec` so the sleep is the
// process itself and not a child holding the pipe open.
bin := stub(t, `exec sleep 30`)
start := time.Now()
_, err := (&audio.FFprobe{Bin: bin, Timeout: 200 * time.Millisecond}).Probe(context.Background(), "/x")
assert.ErrorIs(t, err, uploads.ErrUnreadable)
assert.Less(t, time.Since(start), 3*time.Second)
}
func TestFFprobeProbe_ParentCancellationIsStillNotUnreadableWhenATimeoutIsSet(t *testing.T) {
bin := stub(t, `exec sleep 30`)
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
_, err := (&audio.FFprobe{Bin: bin, Timeout: time.Minute}).Probe(ctx, "/x")
require.Error(t, err)
assert.False(t, errors.Is(err, uploads.ErrUnreadable), "the caller giving up is not a verdict on the file")
assert.ErrorIs(t, err, context.DeadlineExceeded)
}
+156
View File
@@ -0,0 +1,156 @@
package llm
// Observes how much of a gateway response the client will read. The body is read before
// the status is looked at, so a misbehaving gateway or proxy that streams forever was
// bounded only by the client's overall timeout (#53).
import (
"context"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
"time"
)
// endlessGateway answers with the given status and streams filler until the client hangs
// up; wrote reports how many bytes it managed to send.
func endlessGateway(t *testing.T, status int, wrote *atomic.Int64) *httptest.Server {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(status)
chunk := []byte(strings.Repeat("a", 4096))
for {
n, err := w.Write(chunk)
wrote.Add(int64(n))
if err != nil {
return
}
if f, ok := w.(http.Flusher); ok {
f.Flush()
}
}
}))
t.Cleanup(srv.Close)
return srv
}
func TestClient_ASuccessBodyLargerThanTheCapIsAnErrorAndTheClientStopsReading(t *testing.T) {
var wrote atomic.Int64
srv := endlessGateway(t, http.StatusOK, &wrote)
c := New(srv.URL, "", "m", 30*time.Second, WithMaxResponseBytes(8192))
start := time.Now()
_, err := c.Complete(context.Background(), "s", "u")
if err == nil || !strings.Contains(err.Error(), "exceeds") {
t.Fatalf("want a response-too-large error, got %v", err)
}
if time.Since(start) > 5*time.Second {
t.Fatalf("the client kept reading: took %s", time.Since(start))
}
// Hanging up lets the server's writes fail; allow what the transport had already buffered.
time.Sleep(100 * time.Millisecond)
if got := wrote.Load(); got > 8192+1<<20 {
t.Fatalf("server managed to send %d bytes after a cap of 8192", got)
}
}
func TestClient_AFailingBodyIsReadOnlyUpToTheErrorBound(t *testing.T) {
var wrote atomic.Int64
srv := endlessGateway(t, http.StatusBadGateway, &wrote)
c := New(srv.URL, "", "m", 30*time.Second)
start := time.Now()
_, err := c.Complete(context.Background(), "s", "u")
he, ok := err.(*HTTPError)
if !ok {
t.Fatalf("want *HTTPError, got %T: %v", err, err)
}
if he.Code != http.StatusBadGateway {
t.Fatalf("code %d", he.Code)
}
if len(he.Body) > maxErrorBody+1 {
t.Fatalf("kept %d bytes of an error body, bound is %d", len(he.Body), maxErrorBody)
}
if time.Since(start) > 5*time.Second {
t.Fatalf("the client kept reading an endless error body: took %s", time.Since(start))
}
}
// sizedGateway answers 200 with a valid completion padded with trailing whitespace to
// exactly size bytes, so the boundary of the cap can be probed to the byte.
func sizedGateway(t *testing.T, size int) *httptest.Server {
t.Helper()
const base = `{"choices":[{"message":{"content":"ok"}}]}`
payload := base + strings.Repeat(" ", size-len(base))
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(payload))
}))
t.Cleanup(srv.Close)
return srv
}
func TestClient_ABodyOfExactlyTheCapIsReadAndOneByteMoreIsRefused(t *testing.T) {
const capBytes = 4096
for _, tt := range []struct {
name string
size int
wantErr bool
}{
{"one under", capBytes - 1, false},
{"exactly the cap", capBytes, false},
{"one over", capBytes + 1, true},
} {
t.Run(tt.name, func(t *testing.T) {
c := New(sizedGateway(t, tt.size).URL, "", "m", 5*time.Second, WithMaxResponseBytes(capBytes))
got, err := c.Complete(context.Background(), "s", "u")
if tt.wantErr {
if err == nil || !strings.Contains(err.Error(), "exceeds") {
t.Fatalf("want a response-too-large error, got %q, %v", got, err)
}
return
}
if err != nil || got != "ok" {
t.Fatalf("want ok, got %q, %v", got, err)
}
})
}
}
// New is what production calls: no option. The default is therefore the live bound.
func TestClient_TheDefaultCapIsEightMiBWhenNoOptionIsGiven(t *testing.T) {
for _, tt := range []struct {
name string
size int
wantErr bool
}{
{"exactly 8 MiB", 8 << 20, false},
{"8 MiB plus one", 8<<20 + 1, true},
} {
t.Run(tt.name, func(t *testing.T) {
c := New(sizedGateway(t, tt.size).URL, "", "m", 10*time.Second)
_, err := c.Complete(context.Background(), "s", "u")
if (err != nil) != tt.wantErr {
t.Fatalf("size %d: err = %v, wantErr %v", tt.size, err, tt.wantErr)
}
})
}
}
func TestClient_ANonPositiveCapOptionKeepsTheDefault(t *testing.T) {
for _, n := range []int64{0, -1} {
c := New(sizedGateway(t, 1024).URL, "", "m", 5*time.Second, WithMaxResponseBytes(n))
if got, err := c.Complete(context.Background(), "s", "u"); err != nil || got != "ok" {
t.Fatalf("WithMaxResponseBytes(%d): want ok from the default cap, got %q, %v", n, got, err)
}
}
}
+25 -3
View File
@@ -37,6 +37,21 @@ type Client struct {
temp float64 temp float64
httpClient *http.Client httpClient *http.Client
usageHook func(model string, prompt, completion int) usageHook func(model string, prompt, completion int)
maxBody int64
}
// defaultMaxResponseBytes bounds a success body. A completion is a few KB of text; this
// leaves room for a long one and none for a gateway that streams forever (#53).
const defaultMaxResponseBytes = 8 << 20
// WithMaxResponseBytes overrides the bound on a success response body. A non-positive n is
// ignored (keeps the default).
func WithMaxResponseBytes(n int64) Option {
return func(c *Client) {
if n > 0 {
c.maxBody = n
}
}
} }
// Option configures a Client at construction. Variadic so the existing 4-arg // Option configures a Client at construction. Variadic so the existing 4-arg
@@ -101,6 +116,7 @@ func New(baseURL, apiKey, model string, timeout time.Duration, opts ...Option) *
maxTokens: defaultMaxTokens, maxTokens: defaultMaxTokens,
temp: defaultTemperature, temp: defaultTemperature,
httpClient: &http.Client{Timeout: timeout}, httpClient: &http.Client{Timeout: timeout},
maxBody: defaultMaxResponseBytes,
} }
for _, opt := range opts { for _, opt := range opts {
opt(c) opt(c)
@@ -184,12 +200,18 @@ func (c *Client) Complete(ctx context.Context, system, user string) (string, err
} }
defer resp.Body.Close() //nolint:errcheck defer resp.Body.Close() //nolint:errcheck
out, err := io.ReadAll(resp.Body) if resp.StatusCode != http.StatusOK {
// Only the bounded head is ever used (HTTPError.Error truncates to it), so read no more.
// A read error here only shortens a diagnostic; the status code below is what callers act on.
head, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBody+1))
return "", &HTTPError{Code: resp.StatusCode, Body: string(head)}
}
out, err := io.ReadAll(io.LimitReader(resp.Body, c.maxBody+1))
if err != nil { if err != nil {
return "", fmt.Errorf("read response: %w", err) return "", fmt.Errorf("read response: %w", err)
} }
if resp.StatusCode != http.StatusOK { if int64(len(out)) > c.maxBody {
return "", &HTTPError{Code: resp.StatusCode, Body: string(out)} return "", fmt.Errorf("LLM response exceeds %d bytes", c.maxBody)
} }
var cr chatResponse var cr chatResponse
@@ -17,6 +17,10 @@ import (
// nothing about whether someone else's upload exists (ADR-033). // nothing about whether someone else's upload exists (ADR-033).
var ErrUploadNotFound = errors.New("store: uploaded transcript not found") var ErrUploadNotFound = errors.New("store: uploaded transcript not found")
// ErrUploadInProgress is returned by DeleteUpload for an upload that is still queued or
// transcribing: the worker would write its result to a row that no longer exists.
var ErrUploadInProgress = errors.New("store: uploaded transcript is still being processed")
// Uploaded transcripts are PRIVATE per-user content, stored in their own // Uploaded transcripts are PRIVATE per-user content, stored in their own
// RLS-scoped table (migration 017) — never in the shared `transcripts` (ADR-021). // RLS-scoped table (migration 017) — never in the shared `transcripts` (ADR-021).
// Every method goes through withUser, so row-level security confines it to the // Every method goes through withUser, so row-level security confines it to the
@@ -80,23 +84,56 @@ func (s *Store) SaveProgress(ctx context.Context, userID, id string, steps []dom
WHERE id = $1 AND user_id = $2`, b, at) WHERE id = $1 AND user_id = $2`, b, at)
} }
// Heartbeat stamps the upload as alive now. It does not touch progress_at: alive says // Heartbeat stamps the upload as alive at the given time. It does not touch progress_at:
// the service is still reporting, not that work is moving (ADR-035). // alive says the service is still reporting, not that work is moving (ADR-035). The time
func (s *Store) Heartbeat(ctx context.Context, userID, id string) error { // comes from the application clock, the one progress_at and the page's health check use,
// so a database clock that disagrees cannot make a healthy job look lost (#51).
func (s *Store) Heartbeat(ctx context.Context, userID, id string, at time.Time) error {
return s.updateUpload(ctx, userID, id, "heartbeat", return s.updateUpload(ctx, userID, id, "heartbeat",
`UPDATE uploaded_transcripts SET alive_at = NOW() `UPDATE uploaded_transcripts SET alive_at = $3
WHERE id = $1 AND user_id = $2`) WHERE id = $1 AND user_id = $2`, at)
} }
// FailUpload moves an upload to failed with a typed reason. // FailUpload moves a queued or transcribing upload to failed with a typed reason. A
// finished upload is left as it was: a shutdown can record a job as failed after its
// transcript was already committed (the commit lands, the caller sees a cancelled
// context), and that must not hide the text. Such a call touches no row and returns
// ErrUploadNotFound.
func (s *Store) FailUpload(ctx context.Context, userID, id string, kind domain.UploadErrorKind) error { func (s *Store) FailUpload(ctx context.Context, userID, id string, kind domain.UploadErrorKind) error {
return s.updateUpload(ctx, userID, id, "fail upload", return s.updateUpload(ctx, userID, id, "fail upload",
`UPDATE uploaded_transcripts `UPDATE uploaded_transcripts
SET status = 'failed', error_kind = $3, finished_at = NOW() SET status = 'failed', error_kind = $3, finished_at = NOW()
WHERE id = $1 AND user_id = $2`, WHERE id = $1 AND user_id = $2 AND status IN ('queued', 'transcribing')`,
string(kind)) string(kind))
} }
// DeleteUpload removes one of the caller's finished (done or failed) uploads. An upload
// still queued or transcribing is refused with ErrUploadInProgress and left untouched; one
// that is not the caller's is ErrUploadNotFound, as for every other write (#37).
func (s *Store) DeleteUpload(ctx context.Context, userID, id string) error {
return s.withUser(ctx, userID, func(tx pgx.Tx) error {
ct, err := tx.Exec(ctx,
`DELETE FROM uploaded_transcripts
WHERE id = $1 AND user_id = $2 AND status IN ('done', 'failed')`, id, userID)
if err != nil {
return fmt.Errorf("store: delete upload: %w", err)
}
if ct.RowsAffected() > 0 {
return nil
}
var exists bool
if err := tx.QueryRow(ctx,
`SELECT EXISTS (SELECT 1 FROM uploaded_transcripts WHERE id = $1 AND user_id = $2)`,
id, userID).Scan(&exists); err != nil {
return fmt.Errorf("store: delete upload: %w", err)
}
if exists {
return ErrUploadInProgress
}
return ErrUploadNotFound
})
}
// updateUpload runs one UPDATE keyed on ($1 = id, $2 = userID) plus extra args, // updateUpload runs one UPDATE keyed on ($1 = id, $2 = userID) plus extra args,
// and maps "no row touched" to ErrUploadNotFound — which is what a write to another // and maps "no row touched" to ErrUploadNotFound — which is what a write to another
// user's upload looks like under RLS (the row is invisible, not forbidden). // user's upload looks like under RLS (the row is invisible, not forbidden).
@@ -0,0 +1,68 @@
package store_test
// Deleting one recording (#37). A finished or failed upload can be removed by its owner;
// one that is queued or transcribing is refused until it ends, because the worker would
// otherwise write its result to a row that is gone (D3 in the sprint plan).
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"git.d-ma.be/mathias/tapir/internal/adapters/store"
"git.d-ma.be/mathias/tapir/internal/domain"
)
func TestDeleteUpload_RemovesAFinishedOrFailedUploadOfTheOwner(t *testing.T) {
for name, finish := range map[string]func(*store.Store, string) error{
"done": func(s *store.Store, id string) error {
return s.CompleteUpload(context.Background(), userA, id, domain.Transcription{Text: "synthetic", Endpoint: "iguana/kb-whisper"})
},
"failed": func(s *store.Store, id string) error {
return s.FailUpload(context.Background(), userA, id, domain.UploadErrTranscriptionFailed)
},
} {
t.Run(name, func(t *testing.T) {
s := newStore(t)
id := startedUpload(t, s)
require.NoError(t, s.StartTranscribing(context.Background(), userA, id))
require.NoError(t, finish(s, id))
require.NoError(t, s.DeleteUpload(context.Background(), userA, id))
_, err := s.GetUpload(context.Background(), userA, id)
assert.ErrorIs(t, err, store.ErrUploadNotFound)
})
}
}
func TestDeleteUpload_RefusesAnUploadThatIsStillQueuedOrTranscribingAndKeepsTheRow(t *testing.T) {
s := newStore(t)
ctx := context.Background()
queued := startedUpload(t, s)
running, err := s.CreateUpload(ctx, userA, "running", false)
require.NoError(t, err)
require.NoError(t, s.StartTranscribing(ctx, userA, running))
for _, id := range []string{queued, running} {
assert.ErrorIs(t, s.DeleteUpload(ctx, userA, id), store.ErrUploadInProgress)
_, err := s.GetUpload(ctx, userA, id)
assert.NoError(t, err, "a refused delete must leave the row")
}
}
func TestDeleteUpload_AnotherUsersUploadIsNotFoundAndSurvives(t *testing.T) {
s := newStore(t)
ctx := context.Background()
id := startedUpload(t, s)
require.NoError(t, s.StartTranscribing(ctx, userA, id))
require.NoError(t, s.FailUpload(ctx, userA, id, domain.UploadErrTranscriptionFailed))
assert.ErrorIs(t, s.DeleteUpload(ctx, userB, id), store.ErrUploadNotFound,
"existence is not disclosed to a non-owner")
_, err := s.GetUpload(ctx, userA, id)
assert.NoError(t, err, "B's delete must not have removed A's row")
}
@@ -0,0 +1,47 @@
package store_test
// A shutdown can record a job as failed after its transcript was already committed (the
// commit lands, the caller sees a cancelled context). FailUpload must not be able to turn a
// finished row into a failed one and hide the text (#50 review).
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"git.d-ma.be/mathias/tapir/internal/domain"
)
func TestFailUpload_LeavesAFinishedUploadAsItWas(t *testing.T) {
s := newStore(t)
ctx := context.Background()
id := startedUpload(t, s)
require.NoError(t, s.StartTranscribing(ctx, userA, id))
require.NoError(t, s.CompleteUpload(ctx, userA, id, domain.Transcription{Text: "synthetic text", Endpoint: "iguana/kb-whisper"}))
_ = s.FailUpload(ctx, userA, id, domain.UploadErrInterrupted)
got, err := s.GetUpload(ctx, userA, id)
require.NoError(t, err)
assert.Equal(t, domain.UploadDone, got.Status)
assert.Equal(t, "synthetic text", got.Content)
assert.Empty(t, got.ErrorKind)
}
func TestFailUpload_StillFailsAQueuedOrTranscribingUpload(t *testing.T) {
s := newStore(t)
ctx := context.Background()
queued := startedUpload(t, s)
running, err := s.CreateUpload(ctx, userA, "r", false)
require.NoError(t, err)
require.NoError(t, s.StartTranscribing(ctx, userA, running))
for _, id := range []string{queued, running} {
require.NoError(t, s.FailUpload(ctx, userA, id, domain.UploadErrBusy))
got, err := s.GetUpload(ctx, userA, id)
require.NoError(t, err)
assert.Equal(t, domain.UploadFailed, got.Status)
}
}
@@ -68,13 +68,13 @@ func TestUploadProgress_AHeartbeatMovesAliveAtAndNotProgressAt(t *testing.T) {
ctx := context.Background() ctx := context.Background()
at := time.Date(2026, 10, 2, 8, 30, 0, 0, time.UTC) at := time.Date(2026, 10, 2, 8, 30, 0, 0, time.UTC)
require.NoError(t, s.SaveProgress(ctx, userA, id, []domain.ProgressStep{{Key: "audio", Label: "a", State: domain.StepRunning}}, at)) require.NoError(t, s.SaveProgress(ctx, userA, id, []domain.ProgressStep{{Key: "audio", Label: "a", State: domain.StepRunning}}, at))
before := time.Now().Add(-time.Second) beatAt := at.Add(90 * time.Second) // deliberately not the database's NOW()
require.NoError(t, s.Heartbeat(ctx, userA, id)) require.NoError(t, s.Heartbeat(ctx, userA, id, beatAt))
got, err := s.GetUpload(ctx, userA, id) got, err := s.GetUpload(ctx, userA, id)
require.NoError(t, err) require.NoError(t, err)
assert.True(t, got.AliveAt.After(before), "the heartbeat must stamp the row with now") assert.True(t, got.AliveAt.Equal(beatAt), "the heartbeat is stamped with the application's clock, the same one progress_at uses, not the database's NOW() (#51)")
assert.True(t, got.ProgressAt.Equal(at), "alive is not progress: a heartbeat must not move progress_at") assert.True(t, got.ProgressAt.Equal(at), "alive is not progress: a heartbeat must not move progress_at")
} }
@@ -99,7 +99,7 @@ func TestUploadProgress_ByAnotherUserChangesNothing(t *testing.T) {
ctx := context.Background() ctx := context.Background()
errSave := s.SaveProgress(ctx, userB, id, []domain.ProgressStep{{Key: "x", Label: "X", State: domain.StepDone}}, time.Now()) errSave := s.SaveProgress(ctx, userB, id, []domain.ProgressStep{{Key: "x", Label: "X", State: domain.StepDone}}, time.Now())
errBeat := s.Heartbeat(ctx, userB, id) errBeat := s.Heartbeat(ctx, userB, id, time.Now())
assert.ErrorIs(t, errSave, store.ErrUploadNotFound) assert.ErrorIs(t, errSave, store.ErrUploadNotFound)
assert.ErrorIs(t, errBeat, store.ErrUploadNotFound) assert.ErrorIs(t, errBeat, store.ErrUploadNotFound)
+22 -6
View File
@@ -37,12 +37,13 @@ type Reporter interface {
// is for people, so a failed write is logged and never stops the job. // is for people, so a failed write is logged and never stops the job.
type Sink interface { type Sink interface {
SaveProgress(ctx context.Context, steps []domain.ProgressStep, at time.Time) error SaveProgress(ctx context.Context, steps []domain.ProgressStep, at time.Time) error
Beat(ctx context.Context) error Beat(ctx context.Context, at time.Time) error
} }
const ( const (
defaultMinSave = 2 * time.Second defaultMinSave = 2 * time.Second
logAdvanceEach = time.Minute defaultSaveTimeout = 5 * time.Second
logAdvanceEach = time.Minute
) )
// Option tunes a Tracker. // Option tunes a Tracker.
@@ -55,6 +56,14 @@ func WithClock(now func() time.Time) Option { return func(t *Tracker) { t.now =
// Done, Fail and Finish always save. // Done, Fail and Finish always save.
func WithMinSave(d time.Duration) Option { return func(t *Tracker) { t.minSave = d } } func WithMinSave(d time.Duration) Option { return func(t *Tracker) { t.minSave = d } }
// WithSaveTimeout bounds one save or heartbeat write. Progress is cosmetic, so a database
// that stops answering costs a dropped write rather than an unbounded stall. It is still a
// delay: saves run under the tracker's lock, which Begin and Advance (called by the engines)
// need, so each transition can wait up to this long, and a hung database slows a job by
// that much per call, out of the job's own time budget (#51). A dropped final save is not
// retried; the row keeps its last saved step.
func WithSaveTimeout(d time.Duration) Option { return func(t *Tracker) { t.saveTimeout = d } }
// Tracker implements Reporter for one job. // Tracker implements Reporter for one job.
type Tracker struct { type Tracker struct {
ctx context.Context ctx context.Context
@@ -63,6 +72,8 @@ type Tracker struct {
now func() time.Time now func() time.Time
minSave time.Duration minSave time.Duration
saveTimeout time.Duration
mu sync.Mutex mu sync.Mutex
steps []domain.ProgressStep steps []domain.ProgressStep
lastAdvanceSave time.Time lastAdvanceSave time.Time
@@ -72,7 +83,7 @@ type Tracker struct {
// NewTracker plans a job's phases. ctx is used for the saves, so it should outlive the // NewTracker plans a job's phases. ctx is used for the saves, so it should outlive the
// job's own timeout: a job that timed out still gets its failure recorded. // job's own timeout: a job that timed out still gets its failure recorded.
func NewTracker(ctx context.Context, sink Sink, log *slog.Logger, plan []Planned, opts ...Option) *Tracker { func NewTracker(ctx context.Context, sink Sink, log *slog.Logger, plan []Planned, opts ...Option) *Tracker {
t := &Tracker{ctx: ctx, sink: sink, log: log, now: time.Now, minSave: defaultMinSave} t := &Tracker{ctx: ctx, sink: sink, log: log, now: time.Now, minSave: defaultMinSave, saveTimeout: defaultSaveTimeout}
for _, o := range opts { for _, o := range opts {
o(t) o(t)
} }
@@ -212,7 +223,10 @@ func (t *Tracker) Heartbeat(ctx context.Context, beatEvery, stillEvery time.Dura
case <-ctx.Done(): case <-ctx.Done():
return return
case <-beat.C: case <-beat.C:
if err := t.sink.Beat(ctx); err != nil && ctx.Err() == nil { beatCtx, cancel := context.WithTimeout(ctx, t.saveTimeout)
err := t.sink.Beat(beatCtx, t.now())
cancel()
if err != nil && ctx.Err() == nil {
t.log.Warn("progress: heartbeat not saved", "err", err) t.log.Warn("progress: heartbeat not saved", "err", err)
} }
case <-still.C: case <-still.C:
@@ -254,7 +268,9 @@ func (t *Tracker) find(key string) *domain.ProgressStep {
// dropped: progress is cosmetic and must never fail the job it describes. // dropped: progress is cosmetic and must never fail the job it describes.
func (t *Tracker) save() { func (t *Tracker) save() {
snapshot := append([]domain.ProgressStep(nil), t.steps...) snapshot := append([]domain.ProgressStep(nil), t.steps...)
if err := t.sink.SaveProgress(t.ctx, snapshot, t.now()); err != nil && t.ctx.Err() == nil { ctx, cancel := context.WithTimeout(t.ctx, t.saveTimeout)
defer cancel()
if err := t.sink.SaveProgress(ctx, snapshot, t.now()); err != nil && t.ctx.Err() == nil {
t.log.Warn("progress: not saved", "err", err) t.log.Warn("progress: not saved", "err", err)
} }
} }
+154
View File
@@ -0,0 +1,154 @@
package progress_test
// Observes what a database that stops answering does to the job it reports on (#51):
// progress is cosmetic, so a save that never returns must not stall the engine calls that
// report through the tracker, nor the heartbeat that says the process is alive.
import (
"bytes"
"context"
"log/slog"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/require"
"git.d-ma.be/mathias/tapir/internal/domain"
"git.d-ma.be/mathias/tapir/internal/progress"
)
// stallSink never answers until its context ends, like a database connection that hung.
type stallSink struct{ beats atomic.Int32 }
func (s *stallSink) SaveProgress(ctx context.Context, _ []domain.ProgressStep, _ time.Time) error {
<-ctx.Done()
return ctx.Err()
}
func (s *stallSink) Beat(ctx context.Context, _ time.Time) error {
s.beats.Add(1)
<-ctx.Done()
return ctx.Err()
}
type lockedBuf struct {
mu sync.Mutex
b bytes.Buffer
}
func (l *lockedBuf) Write(p []byte) (int, error) {
l.mu.Lock()
defer l.mu.Unlock()
return l.b.Write(p)
}
func (l *lockedBuf) String() string {
l.mu.Lock()
defer l.mu.Unlock()
return l.b.String()
}
func stallTracker(sink progress.Sink, buf *lockedBuf) *progress.Tracker {
log := slog.New(slog.NewTextHandler(buf, nil))
return progress.NewTracker(context.Background(), sink, log,
[]progress.Planned{{Key: "a", Label: "A"}}, progress.WithSaveTimeout(50*time.Millisecond))
}
func TestTracker_AHungSaveDoesNotBlockTheReporterPastItsTimeout(t *testing.T) {
buf := &lockedBuf{}
tr := stallTracker(&stallSink{}, buf)
done := make(chan struct{})
go func() { tr.Begin("a", 0); tr.Advance("a", 1); tr.Done("a"); close(done) }()
select {
case <-done:
case <-time.After(3 * time.Second):
t.Fatal("the reporter was blocked by a save that never returned")
}
require.Contains(t, buf.String(), "progress: not saved", "a dropped save is logged")
}
func TestTracker_AHungBeatDoesNotStopTheHeartbeat(t *testing.T) {
sink := &stallSink{}
tr := stallTracker(sink, &lockedBuf{})
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go tr.Heartbeat(ctx, 5*time.Millisecond, time.Hour)
require.Eventually(t, func() bool { return sink.beats.Load() >= 3 }, 3*time.Second, 5*time.Millisecond,
"each beat is abandoned at its timeout and the next one is tried")
}
func TestTracker_TheStillRunningLineIsNotHeldUpBehindAHungSave(t *testing.T) {
buf := &lockedBuf{}
tr := stallTracker(&stallSink{}, buf)
go tr.Begin("a", 0) // holds the tracker's lock inside a save that hangs until its timeout
time.Sleep(10 * time.Millisecond)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go tr.Heartbeat(ctx, time.Hour, 5*time.Millisecond)
require.Eventually(t, func() bool { return strings.Contains(buf.String(), "still running") }, 3*time.Second, 5*time.Millisecond)
}
// deadlineSink records how long each write was given, and answers at once.
type deadlineSink struct {
mu sync.Mutex
saves []time.Duration
beats []time.Duration
noDLOK bool
}
func (d *deadlineSink) record(ctx context.Context, into *[]time.Duration) {
d.mu.Lock()
defer d.mu.Unlock()
dl, ok := ctx.Deadline()
if !ok {
d.noDLOK = true
return
}
*into = append(*into, time.Until(dl))
}
func (d *deadlineSink) SaveProgress(ctx context.Context, _ []domain.ProgressStep, _ time.Time) error {
d.record(ctx, &d.saves)
return nil
}
func (d *deadlineSink) Beat(ctx context.Context, _ time.Time) error {
d.record(ctx, &d.beats)
return nil
}
// The production value is the one the worker gets: it builds a Tracker without options.
// A zero or tiny default would drop every write, a huge one would bound nothing.
func TestTracker_TheDefaultSaveAndBeatDeadlineIsAFewSecondsAway(t *testing.T) {
sink := &deadlineSink{}
tr := progress.NewTracker(context.Background(), sink, slog.New(slog.NewTextHandler(&lockedBuf{}, nil)),
[]progress.Planned{{Key: "a", Label: "A"}})
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go tr.Heartbeat(ctx, 5*time.Millisecond, time.Hour)
tr.Begin("a", 0)
require.Eventually(t, func() bool {
sink.mu.Lock()
defer sink.mu.Unlock()
return len(sink.beats) > 0
}, 3*time.Second, 5*time.Millisecond)
sink.mu.Lock()
defer sink.mu.Unlock()
require.False(t, sink.noDLOK, "a write was given no deadline at all")
require.NotEmpty(t, sink.saves)
for _, d := range append(append([]time.Duration{}, sink.saves...), sink.beats...) {
require.Greater(t, d, time.Second, "the deadline is already (nearly) spent when the write starts")
require.LessOrEqual(t, d, 5*time.Second, "the default bound is 5 s")
}
}
+26 -7
View File
@@ -17,11 +17,12 @@ import (
// fakeSink records every saved snapshot and every heartbeat. // fakeSink records every saved snapshot and every heartbeat.
type fakeSink struct { type fakeSink struct {
mu sync.Mutex mu sync.Mutex
saves [][]domain.ProgressStep saves [][]domain.ProgressStep
at []time.Time at []time.Time
beats int beats int
err error beatAt []time.Time
err error
} }
func (f *fakeSink) SaveProgress(_ context.Context, steps []domain.ProgressStep, at time.Time) error { func (f *fakeSink) SaveProgress(_ context.Context, steps []domain.ProgressStep, at time.Time) error {
@@ -32,10 +33,11 @@ func (f *fakeSink) SaveProgress(_ context.Context, steps []domain.ProgressStep,
return f.err return f.err
} }
func (f *fakeSink) Beat(context.Context) error { func (f *fakeSink) Beat(_ context.Context, at time.Time) error {
f.mu.Lock() f.mu.Lock()
defer f.mu.Unlock() defer f.mu.Unlock()
f.beats++ f.beats++
f.beatAt = append(f.beatAt, at)
return f.err return f.err
} }
@@ -297,7 +299,7 @@ func (r *retainingSink) SaveProgress(_ context.Context, steps []domain.ProgressS
r.got = append(r.got, steps) r.got = append(r.got, steps)
return nil return nil
} }
func (r *retainingSink) Beat(context.Context) error { return nil } func (r *retainingSink) Beat(context.Context, time.Time) error { return nil }
func TestTracker_ASnapshotHandedToTheSinkIsNotChangedByLaterTransitions(t *testing.T) { func TestTracker_ASnapshotHandedToTheSinkIsNotChangedByLaterTransitions(t *testing.T) {
sink := &retainingSink{} sink := &retainingSink{}
@@ -400,3 +402,20 @@ func TestTracker_AnnotateAttachesANoteToAStepAndSavesItAtOnce(t *testing.T) {
require.Equal(t, "20 min of the 79 the file claimed", sink.last(t)[0].Note) require.Equal(t, "20 min of the 79 the file claimed", sink.last(t)[0].Note)
require.NotPanics(t, func() { tr.Annotate("no-such-step", "x") }) require.NotPanics(t, func() { tr.Annotate("no-such-step", "x") })
} }
func TestTracker_BeatIsStampedWithTheTrackersClock(t *testing.T) {
// alive_at used the database's NOW() while the page compared it with the application
// clock; a skewed database clock showed a healthy job as lost (#51).
tr, sink, clk, _ := newTracker(t)
clk.tick(42 * time.Second)
want := clk.now()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go tr.Heartbeat(ctx, 5*time.Millisecond, time.Hour)
require.Eventually(t, func() bool { return sink.beatCount() >= 1 }, 2*time.Second, 5*time.Millisecond)
sink.mu.Lock()
defer sink.mu.Unlock()
require.True(t, sink.beatAt[0].Equal(want), "beat stamped %v, the tracker's clock says %v", sink.beatAt[0], want)
}
+53
View File
@@ -6,6 +6,7 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"sync"
"time" "time"
"git.d-ma.be/mathias/tapir/internal/domain" "git.d-ma.be/mathias/tapir/internal/domain"
@@ -51,6 +52,53 @@ type Intake struct {
sub Submitter sub Submitter
limits Limits limits Limits
tmpDir string tmpDir string
// MaxReceives and MaxReceivesPerUser bound how many request bodies Accept reads at
// once; zero means the defaults. Without them N parallel POSTs each spool up to
// MaxBytes while the queue still looks empty, and the spool volume fills (#47).
MaxReceives int
MaxReceivesPerUser int
mu sync.Mutex
receives int
byUser map[string]int
}
const (
defaultMaxReceives = 4
defaultMaxReceivesPerUser = 2
)
// acquireReceive reserves a receive slot for userID, or reports there is none. Every
// successful call must be paired with releaseReceive.
func (in *Intake) acquireReceive(userID string) bool {
maxAll, maxUser := in.MaxReceives, in.MaxReceivesPerUser
if maxAll <= 0 {
maxAll = defaultMaxReceives
}
if maxUser <= 0 {
maxUser = defaultMaxReceivesPerUser
}
in.mu.Lock()
defer in.mu.Unlock()
if in.receives >= maxAll || in.byUser[userID] >= maxUser {
return false
}
if in.byUser == nil {
in.byUser = map[string]int{}
}
in.receives++
in.byUser[userID]++
return true
}
func (in *Intake) releaseReceive(userID string) {
in.mu.Lock()
defer in.mu.Unlock()
in.receives--
if in.byUser[userID]--; in.byUser[userID] <= 0 {
delete(in.byUser, userID)
}
} }
// NewIntake wires an Intake. tmpDir is where uploads are spooled; it must exist. // NewIntake wires an Intake. tmpDir is where uploads are spooled; it must exist.
@@ -94,6 +142,11 @@ func (in *Intake) Accept(ctx context.Context, userID, title string, src io.Reade
if rc, ok := in.sub.(interface{ HasRoom(userID string) bool }); ok && !rc.HasRoom(userID) { if rc, ok := in.sub.(interface{ HasRoom(userID string) bool }); ok && !rc.HasRoom(userID) {
return "", &RejectError{Reason: ReasonBusy, Message: busyMessage} return "", &RejectError{Reason: ReasonBusy, Message: busyMessage}
} }
// HasRoom cannot see bodies still being received; bound those separately.
if !in.acquireReceive(userID) {
return "", &RejectError{Reason: ReasonBusy, Message: busyMessage}
}
defer in.releaseReceive(userID)
f, err := os.CreateTemp(in.tmpDir, "upload-*") f, err := os.CreateTemp(in.tmpDir, "upload-*")
if err != nil { if err != nil {
return "", fmt.Errorf("uploads: spool: %w", err) return "", fmt.Errorf("uploads: spool: %w", err)
+131
View File
@@ -0,0 +1,131 @@
package uploads_test
// Observes how many request bodies Accept will read at once (#47). HasRoom and the
// per-user queue cap act before and after the receive; N parallel POSTs all pass
// HasRoom while the queue is still empty, so the receive itself is bounded.
import (
"context"
"errors"
"io"
"strings"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"git.d-ma.be/mathias/tapir/internal/uploads"
)
// slowReceive starts an Accept whose body stays open until the returned writer is closed,
// and waits until it holds a spool file, which proves it is inside the receive.
type slowReceive struct {
w *io.PipeWriter
done chan error
finished bool
}
func startSlowReceive(t *testing.T, r *rig, in *uploads.Intake, user string, wantFiles int) *slowReceive {
t.Helper()
pr, pw := io.Pipe()
s := &slowReceive{w: pw, done: make(chan error, 1)}
go func() {
_, err := in.Accept(context.Background(), user, "t", pr)
s.done <- err
}()
require.Eventually(t, func() bool { return len(r.tmpFiles(t)) == wantFiles }, 2*time.Second, 5*time.Millisecond,
"the receive for %s should have started", user)
// An abandoned receive ends in a read error, so it never reaches the rig's fakes, which
// are not safe for concurrent use. A receive that finish() already ended is a no-op.
t.Cleanup(func() {
if s.finished {
return
}
_ = pw.CloseWithError(errors.New("test over"))
select {
case <-s.done:
case <-time.After(2 * time.Second):
}
})
return s
}
func (s *slowReceive) finish(t *testing.T) error {
t.Helper()
_, _ = s.w.Write([]byte("data"))
require.NoError(t, s.w.Close())
s.finished = true
select {
case err := <-s.done:
return err
case <-time.After(2 * time.Second):
t.Fatal("Accept did not return after its body ended")
return nil
}
}
func requireBusy(t *testing.T, err error) {
t.Helper()
var rej *uploads.RejectError
require.True(t, errors.As(err, &rej), "want a busy rejection, got %v", err)
assert.Equal(t, uploads.ReasonBusy, rej.Reason)
}
func TestAccept_APerUserLimitOnConcurrentReceivesRefusesTheNextBodyBeforeSpoolingIt(t *testing.T) {
r := newRig(t)
in := uploads.NewIntake(r.st, r.pr, r.sub, uploads.Limits{MaxBytes: 1024, MaxDuration: time.Hour}, r.tmp)
in.MaxReceivesPerUser = 2
in.MaxReceives = 10
startSlowReceive(t, r, in, "user-1", 1)
startSlowReceive(t, r, in, "user-1", 2)
_, err := in.Accept(context.Background(), "user-1", "t", strings.NewReader("data"))
requireBusy(t, err)
assert.Len(t, r.tmpFiles(t), 2, "the refused body was never spooled")
// Another user is not blocked by user-1's two receives.
other := startSlowReceive(t, r, in, "user-2", 3)
require.NoError(t, other.finish(t))
}
func TestAccept_AnOverallLimitOnConcurrentReceivesRefusesTheNextUser(t *testing.T) {
r := newRig(t)
in := uploads.NewIntake(r.st, r.pr, r.sub, uploads.Limits{MaxBytes: 1024, MaxDuration: time.Hour}, r.tmp)
in.MaxReceivesPerUser = 5
in.MaxReceives = 3
for i, u := range []string{"a", "b", "c"} {
startSlowReceive(t, r, in, u, i+1)
}
_, err := in.Accept(context.Background(), "d", "t", strings.NewReader("data"))
requireBusy(t, err)
assert.Len(t, r.tmpFiles(t), 3)
}
func TestAccept_AFinishedReceiveFreesItsSlot(t *testing.T) {
r := newRig(t)
in := uploads.NewIntake(r.st, r.pr, r.sub, uploads.Limits{MaxBytes: 1024, MaxDuration: time.Hour}, r.tmp)
in.MaxReceivesPerUser = 1
in.MaxReceives = 5
first := startSlowReceive(t, r, in, "user-1", 1)
require.NoError(t, first.finish(t))
_, err := in.Accept(context.Background(), "user-1", "t", strings.NewReader("data"))
require.NoError(t, err, "the slot is released when Accept returns")
}
func TestAccept_AFailedReceiveFreesItsSlot(t *testing.T) {
r := newRig(t)
in := uploads.NewIntake(r.st, r.pr, r.sub, uploads.Limits{MaxBytes: 1024, MaxDuration: time.Hour}, r.tmp)
in.MaxReceivesPerUser = 1
in.MaxReceives = 5
_, err := in.Accept(context.Background(), "user-1", "t", &brokenBody{n: 1})
require.ErrorIs(t, err, uploads.ErrClientRead)
_, err = in.Accept(context.Background(), "user-1", "t", strings.NewReader("data"))
require.NoError(t, err, "a dropped client must not leak a slot")
}
+31
View File
@@ -0,0 +1,31 @@
package uploads_test
// The limits production runs on are the DEFAULTS: nothing outside tests sets
// MaxReceives or MaxReceivesPerUser. Pinned here with the literal numbers, so changing a
// default is a visible decision (#47 review). With 4 receives, 20 queued and 1 running at
// the 200 MiB cap, the spool needs ~5 GiB of the 6 Gi emptyDir (infra); raising any of
// them or TAPIR_UPLOAD_MAX_BYTES must revisit that arithmetic.
import (
"context"
"strings"
"testing"
"time"
"git.d-ma.be/mathias/tapir/internal/uploads"
)
func TestNewIntake_DefaultsAllowTwoReceivesPerUserAndFourOverall(t *testing.T) {
r := newRig(t)
in := uploads.NewIntake(r.st, r.pr, r.sub, uploads.Limits{MaxBytes: 1024, MaxDuration: time.Hour}, r.tmp)
startSlowReceive(t, r, in, "a", 1)
startSlowReceive(t, r, in, "a", 2)
_, err := in.Accept(context.Background(), "a", "t", strings.NewReader("x"))
requireBusy(t, err) // a third body from one user
startSlowReceive(t, r, in, "b", 3)
startSlowReceive(t, r, in, "b", 4)
_, err = in.Accept(context.Background(), "c", "t", strings.NewReader("x"))
requireBusy(t, err) // a fifth body overall
}
+48
View File
@@ -0,0 +1,48 @@
package uploads
// Observes what jobSink hands the store: the heartbeat's timestamp is the one the tracker
// stamped (its application clock), not a second reading taken here (#51).
import (
"context"
"testing"
"time"
"git.d-ma.be/mathias/tapir/internal/domain"
)
type atStore struct{ beatAt, saveAt time.Time }
func (*atStore) StartTranscribing(context.Context, string, string) error { return nil }
func (*atStore) CompleteUpload(context.Context, string, string, domain.Transcription) error {
return nil
}
func (*atStore) FailUpload(context.Context, string, string, domain.UploadErrorKind) error { return nil }
func (s *atStore) SaveProgress(_ context.Context, _, _ string, _ []domain.ProgressStep, at time.Time) error {
s.saveAt = at
return nil
}
func (s *atStore) Heartbeat(_ context.Context, _, _ string, at time.Time) error {
s.beatAt = at
return nil
}
func TestJobSink_ForwardsTheTrackersTimestampToTheStoreUnchanged(t *testing.T) {
st := &atStore{}
sink := jobSink{st: st, userID: "u", id: "i"}
stamp := time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC)
if err := sink.Beat(context.Background(), stamp); err != nil {
t.Fatal(err)
}
if err := sink.SaveProgress(context.Background(), nil, stamp); err != nil {
t.Fatal(err)
}
if !st.beatAt.Equal(stamp) {
t.Errorf("heartbeat reached the store stamped %v, want the tracker's %v", st.beatAt, stamp)
}
if !st.saveAt.Equal(stamp) {
t.Errorf("progress save reached the store stamped %v, want the tracker's %v", st.saveAt, stamp)
}
}
+21 -10
View File
@@ -47,7 +47,7 @@ type WorkerStore interface {
CompleteUpload(ctx context.Context, userID, id string, t domain.Transcription) error CompleteUpload(ctx context.Context, userID, id string, t domain.Transcription) error
FailUpload(ctx context.Context, userID, id string, kind domain.UploadErrorKind) error FailUpload(ctx context.Context, userID, id string, kind domain.UploadErrorKind) error
SaveProgress(ctx context.Context, userID, id string, steps []domain.ProgressStep, at time.Time) error SaveProgress(ctx context.Context, userID, id string, steps []domain.ProgressStep, at time.Time) error
Heartbeat(ctx context.Context, userID, id string) error Heartbeat(ctx context.Context, userID, id string, at time.Time) error
} }
// WorkerOption tunes a Worker. // WorkerOption tunes a Worker.
@@ -91,7 +91,9 @@ func (s jobSink) SaveProgress(ctx context.Context, steps []domain.ProgressStep,
return s.st.SaveProgress(ctx, s.userID, s.id, steps, at) return s.st.SaveProgress(ctx, s.userID, s.id, steps, at)
} }
func (s jobSink) Beat(ctx context.Context) error { return s.st.Heartbeat(ctx, s.userID, s.id) } func (s jobSink) Beat(ctx context.Context, at time.Time) error {
return s.st.Heartbeat(ctx, s.userID, s.id, at)
}
// Worker transcribes accepted uploads in-process, ONE AT A TIME (ADR-033). It is // Worker transcribes accepted uploads in-process, ONE AT A TIME (ADR-033). It is
// not a second deployable: it shares the web process's lifetime, which is why it // not a second deployable: it shares the web process's lifetime, which is why it
@@ -176,9 +178,10 @@ func (w *Worker) Submit(j Job) error {
} }
// Run processes queued jobs until ctx ends, then deletes the audio of anything // Run processes queued jobs until ctx ends, then deletes the audio of anything
// still queued and refuses new jobs. The rows of those jobs stay queued or // still queued and refuses new jobs. The job running at shutdown is recorded interrupted
// transcribing and are failed as interrupted by the start-up sweep // by Process; the rows of jobs still queued stay queued and are failed as interrupted by
// (store.FailInterruptedUploads): the audio is not retained, so they cannot resume. // the start-up sweep (store.FailInterruptedUploads): the audio is not retained, so they
// cannot resume.
func (w *Worker) Run(ctx context.Context) { func (w *Worker) Run(ctx context.Context) {
defer w.stop() defer w.stop()
for { for {
@@ -227,7 +230,7 @@ func (w *Worker) stop() {
// Process runs one job: row -> transcribing, convert, transcribe, store. The // Process runs one job: row -> transcribing, convert, transcribe, store. The
// uploaded file and the converted WAV are deleted on every path — the audio is // uploaded file and the converted WAV are deleted on every path — the audio is
// never retained (ADR-033). A failure is recorded with a TYPED kind; a shutdown // never retained (ADR-033). A failure is recorded with a TYPED kind; a shutdown
// mid-job is not a failure and leaves the row for the start-up sweep. // mid-job is recorded as interrupted at once (see fail).
// //
// Every phase is reported through one progress.Tracker, which saves it to the row for // Every phase is reported through one progress.Tracker, which saves it to the row for
// the page and logs it, and a heartbeat stamps the row on its own while the job runs // the page and logs it, and a heartbeat stamps the row on its own while the job runs
@@ -269,7 +272,7 @@ func (w *Worker) Process(ctx context.Context, job Job) {
defer func() { stopBeat(); <-hbDone }() // no heartbeat outlives its job defer func() { stopBeat(); <-hbDone }() // no heartbeat outlives its job
// abort records where the job stopped. A shutdown (the parent context ended) is not // abort records where the job stopped. A shutdown (the parent context ended) is not
// a failure: the row is left for the start-up sweep, so the step stays as it was. // a failure of the step: it stays as it was, and fail records the row as interrupted.
abort := func(kind domain.UploadErrorKind) { abort := func(kind domain.UploadErrorKind) {
if ctx.Err() == nil { if ctx.Err() == nil {
tr.FailRunning() tr.FailRunning()
@@ -332,11 +335,19 @@ func (w *Worker) Process(ctx context.Context, job Job) {
"segments", len(t.Segments), "elapsed", time.Since(start).Round(time.Second)) "segments", len(t.Segments), "elapsed", time.Since(start).Round(time.Second))
} }
// fail records a transcription failure — unless the PARENT context ended, which // interruptedWriteTimeout bounds the one write a dying process makes to record its job.
// is a shutdown, not a failure: the row is left for the start-up sweep. const interruptedWriteTimeout = 3 * time.Second
// fail records a transcription failure. When the PARENT context ended the process is
// shutting down: the job is recorded interrupted at once, on a short context of its own,
// because the process may exit before any other pod's start-up sweep would do it (#50).
// The sweep remains the backstop for a process that is killed outright.
func (w *Worker) fail(ctx context.Context, job Job, log *slog.Logger, kind domain.UploadErrorKind) { func (w *Worker) fail(ctx context.Context, job Job, log *slog.Logger, kind domain.UploadErrorKind) {
if ctx.Err() != nil { if ctx.Err() != nil {
return kind = domain.UploadErrInterrupted
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(context.WithoutCancel(ctx), interruptedWriteTimeout)
defer cancel()
} }
if err := w.store.FailUpload(ctx, job.UserID, job.UploadID, kind); err != nil { if err := w.store.FailUpload(ctx, job.UserID, job.UploadID, kind); err != nil {
log.Error("upload: record failure", "kind", errkind.Of(err)) log.Error("upload: record failure", "kind", errkind.Of(err))
+10 -5
View File
@@ -53,7 +53,10 @@ func (s *wStore) CompleteUpload(_ context.Context, _, id string, t domain.Transc
return nil return nil
} }
func (s *wStore) FailUpload(_ context.Context, _, id string, k domain.UploadErrorKind) error { func (s *wStore) FailUpload(ctx context.Context, _, id string, k domain.UploadErrorKind) error {
if err := ctx.Err(); err != nil { // like the real store: a cancelled context cannot write
return err
}
s.mu.Lock() s.mu.Lock()
defer s.mu.Unlock() defer s.mu.Unlock()
s.failed[id] = k s.failed[id] = k
@@ -67,7 +70,7 @@ func (s *wStore) SaveProgress(_ context.Context, _, _ string, steps []domain.Pro
return nil return nil
} }
func (s *wStore) Heartbeat(context.Context, string, string) error { func (s *wStore) Heartbeat(context.Context, string, string, time.Time) error {
time.Sleep(s.beatDelay) time.Sleep(s.beatDelay)
s.mu.Lock() s.mu.Lock()
defer s.mu.Unlock() defer s.mu.Unlock()
@@ -242,7 +245,9 @@ func TestWorkerProcess_FailuresAreTypedAndStillDeleteTheAudio(t *testing.T) {
}) })
} }
func TestWorkerProcess_ShutdownMidJobLeavesTheRowForTheStartupSweep(t *testing.T) { func TestWorkerProcess_ShutdownMidJobRecordsTheRowInterruptedAtOnce(t *testing.T) {
// SIGTERM on a deploy used to leave the row "transcribing" until the NEXT pod's
// start-up sweep; the process may exit before any sweep runs (#50).
r := newWRig(t, time.Minute) r := newWRig(t, time.Minute)
r.tr.block = make(chan struct{}) r.tr.block = make(chan struct{})
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
@@ -255,8 +260,8 @@ func TestWorkerProcess_ShutdownMidJobLeavesTheRowForTheStartupSweep(t *testing.T
<-done <-done
_, f := r.st.snapshot() _, f := r.st.snapshot()
assert.Empty(t, f, "a shutdown is not a transcription failure: the row stays for FailInterruptedUploads to mark interrupted") assert.Equal(t, domain.UploadErrInterrupted, f["a"], "the row is recorded interrupted by the dying process, not left for a sweep")
assert.Empty(t, r.leftovers(t), "but the audio is still deleted") assert.Empty(t, r.leftovers(t), "and the audio is still deleted")
} }
func TestWorkerProcess_AJobThatOutlivesItsTimeoutIsFailedNotStuck(t *testing.T) { func TestWorkerProcess_AJobThatOutlivesItsTimeoutIsFailedNotStuck(t *testing.T) {
+151
View File
@@ -0,0 +1,151 @@
package web_test
// Observes the cross-origin gate around Router (#48): SameSite=Lax does not stop a POST
// from a sibling *.d-ma.be origin, so the router itself must refuse a non-safe request
// whose Origin or Sec-Fetch-Site says it did not come from this site's own pages.
import (
"context"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// foreign is a header set a browser sends for a form post made by another page.
var foreign = map[string]map[string]string{
"sibling subdomain, Sec-Fetch-Site": {"Origin": "https://evil.d-ma.be", "Sec-Fetch-Site": "same-site"},
"other site, Sec-Fetch-Site": {"Origin": "https://evil.example", "Sec-Fetch-Site": "cross-site"},
"sibling subdomain, Origin only": {"Origin": "https://evil.d-ma.be"},
"opaque origin": {"Origin": "null"},
}
// own is a header set for the app's own pages (or a non-browser client): must pass.
var own = map[string]map[string]string{
"same-origin Sec-Fetch-Site": {"Sec-Fetch-Site": "same-origin", "Origin": "http://example.com"},
"matching Origin only": {"Origin": "http://example.com"},
"no browser headers": {},
}
func withHeaders(req *http.Request, h map[string]string) *http.Request {
for k, v := range h {
req.Header.Set(k, v)
}
return req
}
func userCount(t *testing.T) int {
t.Helper()
var n int
require.NoError(t, rawPool(t).QueryRow(context.Background(), `SELECT count(*) FROM users WHERE id = $1`, userID).Scan(&n))
return n
}
func TestCrossOrigin_ARefusedDeleteAccountPostLeavesTheAccountInPlace(t *testing.T) {
for name, h := range foreign {
t.Run(name, func(t *testing.T) {
app, _ := newAccountApp(t)
resetDB(t, rawPool(t))
seedConnection(t, app, "youtube", "ada@channel", "ref")
rec := do(t, app, withHeaders(httptest.NewRequest(http.MethodPost, "/account/delete", nil), h))
assert.Equal(t, http.StatusForbidden, rec.Code)
assert.Equal(t, 1, userCount(t), "the account must survive a refused post")
})
}
}
func TestCrossOrigin_ASameOriginOrHeaderlessDeleteAccountPostIsServed(t *testing.T) {
for name, h := range own {
t.Run(name, func(t *testing.T) {
app, _ := newAccountApp(t)
resetDB(t, rawPool(t))
seedConnection(t, app, "youtube", "ada@channel", "ref")
rec := do(t, app, withHeaders(httptest.NewRequest(http.MethodPost, "/account/delete", nil), h))
assert.Equal(t, http.StatusSeeOther, rec.Code)
assert.Equal(t, "/auth/logout", rec.Header().Get("Location"))
})
}
}
func TestCrossOrigin_ARefusedUploadPostSubmitsNoJob(t *testing.T) {
for name, h := range foreign {
t.Run(name, func(t *testing.T) {
app, sub := newUploadApp(t, audioOK, 200<<20)
rec := do(t, app, withHeaders(uploadReq(t, "m", "m.wav", []byte("RIFF....WAVE")), h))
assert.Equal(t, http.StatusForbidden, rec.Code)
assert.Empty(t, sub.jobs, "a refused post must not enqueue transcription")
})
}
}
func TestCrossOrigin_ARefusedUploadChatPostCallsNoModel(t *testing.T) {
for name, h := range foreign {
t.Run(name, func(t *testing.T) {
app, fc := newUploadChatApp(t)
id := seedUpload(t, userID, "Roadmap sync", "done", meetingText)
req := chatPost("/uploads/"+id+"/chat", url.Values{"message": {"hej"}})
rec := do(t, app, withHeaders(req, h))
assert.Equal(t, http.StatusForbidden, rec.Code)
assert.Zero(t, fc.callCount)
})
}
}
func TestCrossOrigin_AForeignPostIsRefusedOnEachListedStateChangingPath(t *testing.T) {
routes := []string{
"/account/disconnect/youtube",
"/account/summarize-mode",
"/register",
"/paste",
"/v/abc/action",
"/v/abc/summarize",
"/v/abc/retry-now",
"/v/abc/chat",
}
for _, path := range routes {
t.Run(path, func(t *testing.T) {
app := newApp(t)
req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(""))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
rec := do(t, app, withHeaders(req, foreign["sibling subdomain, Sec-Fetch-Site"]))
assert.Equal(t, http.StatusForbidden, rec.Code)
})
}
}
func TestCrossOrigin_ARefusedRecordingDeletePostLeavesTheRecordingInPlace(t *testing.T) {
for name, h := range foreign {
t.Run(name, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
rec := do(t, app, withHeaders(httptest.NewRequest(http.MethodPost, "/uploads/"+id+"/delete", nil), h))
assert.Equal(t, http.StatusForbidden, rec.Code)
assert.True(t, uploadExists(t, id), "a refused post must not delete the recording")
})
}
}
// A login redirect comes back to /auth/callback as a cross-site GET navigation, and
// health probes carry no headers: safe methods must never be refused on origin grounds.
func TestCrossOrigin_ACrossSiteGETIsNotRefused(t *testing.T) {
app := newApp(t)
for _, path := range []string{"/healthz", "/welcome"} {
req := httptest.NewRequest(http.MethodGet, path, nil)
req.Header.Set("Sec-Fetch-Site", "cross-site")
assert.Equal(t, http.StatusOK, do(t, app, req).Code, path)
}
}
+11 -2
View File
@@ -59,6 +59,9 @@ type Store interface {
// non-owner's id is ErrUploadNotFound. // non-owner's id is ErrUploadNotFound.
GetUpload(ctx context.Context, userID, id string) (*domain.UploadedTranscript, error) GetUpload(ctx context.Context, userID, id string) (*domain.UploadedTranscript, error)
ListUploads(ctx context.Context, userID string) ([]domain.UploadedTranscript, error) ListUploads(ctx context.Context, userID string) ([]domain.UploadedTranscript, error)
// DeleteUpload removes a finished upload; ErrUploadInProgress while it is still being
// processed, ErrUploadNotFound for a non-owner.
DeleteUpload(ctx context.Context, userID, id string) error
// SetTranscriptStatus clears or updates a video's transcript backoff state. // SetTranscriptStatus clears or updates a video's transcript backoff state.
// Used by handleRetryNow to clear rate_limited_at before immediate processing. // Used by handleRetryNow to clear rate_limited_at before immediate processing.
@@ -200,6 +203,7 @@ func (a *App) Router() http.Handler {
app.HandleFunc("POST /uploads", a.handleUploadPost) app.HandleFunc("POST /uploads", a.handleUploadPost)
app.HandleFunc("GET /uploads/{id}", a.handleUploadDetail) app.HandleFunc("GET /uploads/{id}", a.handleUploadDetail)
app.HandleFunc("GET /uploads/{id}/status", a.handleUploadStatus) app.HandleFunc("GET /uploads/{id}/status", a.handleUploadStatus)
app.HandleFunc("POST /uploads/{id}/delete", a.handleUploadDelete)
// Chat over the uploaded transcript reuses the ADR-027 Chatter; mounted only // Chat over the uploaded transcript reuses the ADR-027 Chatter; mounted only
// when one is wired. // when one is wired.
if a.Chat != nil { if a.Chat != nil {
@@ -229,7 +233,12 @@ func (a *App) Router() http.Handler {
// resolved user_id. /register lives inside the auth guard but is exempt from // resolved user_id. /register lives inside the auth guard but is exempt from
// the registration gate (you must be able to reach it before you have a user). // the registration gate (you must be able to reach it before you have a user).
root.Handle("/", a.Auth.Middleware(a.registrationGate(app))) root.Handle("/", a.Auth.Middleware(a.registrationGate(app)))
return root
// The session cookie is SameSite=Lax, which still travels on a POST made from a
// sibling *.d-ma.be page (same site, different origin). Refuse any non-safe request
// whose Origin or Sec-Fetch-Site says it did not come from this origin (#48). It wraps
// everything, so a route added later is covered without remembering to ask.
return http.NewCrossOriginProtection().Handler(root)
} }
// handleWelcome renders the public landing page (/welcome). It is mounted outside // handleWelcome renders the public landing page (/welcome). It is mounted outside
@@ -304,7 +313,7 @@ func (a *App) handleList(w http.ResponseWriter, r *http.Request) {
a.render(w, r, summaryList(buckets, hasConnected, autoSummarize)) a.render(w, r, summaryList(buckets, hasConnected, autoSummarize))
return return
} }
a.render(w, r, ListPage(buckets, f, stats, takeFlash(w, r), hasConnected, channels, autoSummarize)) a.render(w, r, ListPage(buckets, f, stats, takeFlash(w, r), hasConnected, channels, autoSummarize, a.Uploads != nil))
} }
// handleDetail renders one summary in full (highlights, takeaways, action group). // handleDetail renders one summary in full (highlights, takeaways, action group).
+1 -1
View File
@@ -66,7 +66,7 @@ func TestParseYouTubeVideoID(t *testing.T) {
func TestListPageShowsPasteFormOnlyWhenConnected(t *testing.T) { func TestListPageShowsPasteFormOnlyWhenConnected(t *testing.T) {
render := func(connected bool) string { render := func(connected bool) string {
var buf bytes.Buffer var buf bytes.Buffer
if err := ListPage(listBuckets{}, Filter{}, PipelineStats{}, "", connected, nil, true).Render(context.Background(), &buf); err != nil { if err := ListPage(listBuckets{}, Filter{}, PipelineStats{}, "", connected, nil, true, false).Render(context.Background(), &buf); err != nil {
t.Fatalf("render: %v", err) t.Fatalf("render: %v", err)
} }
return buf.String() return buf.String()
+57
View File
@@ -18,6 +18,7 @@ import (
"git.d-ma.be/mathias/tapir/internal/adapters/chat" "git.d-ma.be/mathias/tapir/internal/adapters/chat"
"git.d-ma.be/mathias/tapir/internal/adapters/store" "git.d-ma.be/mathias/tapir/internal/adapters/store"
"git.d-ma.be/mathias/tapir/internal/config"
"git.d-ma.be/mathias/tapir/internal/domain" "git.d-ma.be/mathias/tapir/internal/domain"
"git.d-ma.be/mathias/tapir/internal/errkind" "git.d-ma.be/mathias/tapir/internal/errkind"
"git.d-ma.be/mathias/tapir/internal/uploads" "git.d-ma.be/mathias/tapir/internal/uploads"
@@ -209,6 +210,32 @@ func (a *App) handleUploadStatus(w http.ResponseWriter, r *http.Request) {
a.render(w, r, uploadStatus(*u, a.Chat != nil, a.now())) a.render(w, r, uploadStatus(*u, a.Chat != nil, a.now()))
} }
// handleUploadDelete removes one of the caller's finished recordings and returns to the
// list. Another user's id is a 404 like every other upload route; a recording still being
// transcribed is refused (409) rather than deleted under the worker (#37).
func (a *App) handleUploadDelete(w http.ResponseWriter, r *http.Request) {
userID, ok := a.currentUserID(w, r)
if !ok {
return
}
id := r.PathValue("id")
if !uuidShape.MatchString(id) {
http.NotFound(w, r)
return
}
switch err := a.Store.DeleteUpload(r.Context(), userID, id); {
case err == nil:
http.Redirect(w, r, "/uploads", http.StatusSeeOther)
case errors.Is(err, store.ErrUploadNotFound):
http.NotFound(w, r)
case errors.Is(err, store.ErrUploadInProgress):
a.renderUploadsPage(w, r, userID, http.StatusConflict,
"This recording is still being transcribed. You can delete it once it has finished.")
default:
a.serverError(w, r, "delete upload", err)
}
}
func (a *App) loadOwnedUpload(w http.ResponseWriter, r *http.Request) (*domain.UploadedTranscript, bool) { func (a *App) loadOwnedUpload(w http.ResponseWriter, r *http.Request) (*domain.UploadedTranscript, bool) {
userID, ok := a.currentUserID(w, r) userID, ok := a.currentUserID(w, r)
if !ok { if !ok {
@@ -243,6 +270,36 @@ func titleFromFilename(name string) string {
return t return t
} }
// cloudProvider names the cloud service that produced a transcript, or false when it was
// served locally (or by an unknown endpoint). "Local" is config.IsLocalModel: the same
// definition the start-up validation uses, not a second string match here (#52).
//
// The endpoint is one alias, except for the two-engine path (ADR-034), which writes
// "<base> + <alt>, reconciled by <model>" (ensemble.go). That shape is taken apart exactly
// and each alias is judged whole, so a composite of local models is local, and anything
// else (a cloud alias, one with no provider prefix, one with a space or a connector word in
// it) is not. A string that is not that shape is one alias.
func cloudProvider(endpoint string) (string, bool) {
endpoint = strings.TrimSpace(endpoint)
if endpoint == "" {
return "", false
}
aliases := []string{endpoint}
if engines, reconciler, ok := strings.Cut(endpoint, ", reconciled by "); ok {
aliases = append(strings.Split(engines, " + "), reconciler)
}
for _, a := range aliases {
if a = strings.TrimSpace(a); !config.IsLocalModel(a) {
provider, _, _ := strings.Cut(a, "/")
if provider = strings.TrimSpace(provider); provider == "" {
provider = a
}
return provider, true
}
}
return "", false
}
// hasNote reports whether any phase carries a note. A note is the only warning for a // hasNote reports whether any phase carries a note. A note is the only warning for a
// recording that is shorter than its file claimed, so a record that has one is shown open. // recording that is shorter than its file claimed, so a record that has one is shown open.
func hasNote(steps []domain.ProgressStep) bool { func hasNote(steps []domain.ProgressStep) bool {
+22
View File
@@ -66,6 +66,12 @@ templ uploadStatus(u domain.UploadedTranscript, chatEnabled bool, now time.Time)
Transcribed { uploads.FormatDuration(time.Duration(u.AudioSeconds * float64(time.Second))) } Transcribed { uploads.FormatDuration(time.Duration(u.AudioSeconds * float64(time.Second))) }
of audio by <span class="chip">{ u.Endpoint }</span>. of audio by <span class="chip">{ u.Endpoint }</span>.
</p> </p>
if provider, cloud := cloudProvider(u.Endpoint); cloud {
<p class="cloud-note">
This recording's audio left the homelab: it was transcribed by a cloud service ({ provider })
because the local engine could not.
</p>
}
@reconciliationNote(u.Reconciliation) @reconciliationNote(u.Reconciliation)
if len(u.Progress) > 0 { if len(u.Progress) > 0 {
<details class="what-was-done" if hasNote(u.Progress) { open }> <details class="what-was-done" if hasNote(u.Progress) { open }>
@@ -80,6 +86,7 @@ templ uploadStatus(u domain.UploadedTranscript, chatEnabled bool, now time.Time)
<summary>Transcript</summary> <summary>Transcript</summary>
<pre class="transcript">{ u.Content }</pre> <pre class="transcript">{ u.Content }</pre>
</details> </details>
@deleteRecording(u.ID)
</div> </div>
case domain.UploadFailed: case domain.UploadFailed:
<div class="upload-status upload-failed flash flash-error" role="alert"> <div class="upload-status upload-failed flash flash-error" role="alert">
@@ -91,6 +98,7 @@ templ uploadStatus(u domain.UploadedTranscript, chatEnabled bool, now time.Time)
@progressSteps(u.Progress, now, false) @progressSteps(u.Progress, now, false)
</details> </details>
} }
@deleteRecording(u.ID)
</div> </div>
default: default:
<div <div
@@ -116,6 +124,20 @@ templ uploadStatus(u domain.UploadedTranscript, chatEnabled bool, now time.Time)
} }
} }
// deleteRecording is the confirm-then-delete control, shown only once a recording has
// finished (done or failed): deleting one that is still being processed is refused.
templ deleteRecording(id string) {
<details class="confirm-delete">
<summary class="btn-danger">Delete this recording…</summary>
<div class="confirm-body">
<p>This permanently deletes the recording and its transcript. Are you sure?</p>
<form method="post" action={ templ.SafeURL("/uploads/" + id + "/delete") }>
<button type="submit" class="btn-danger">Yes, permanently delete it</button>
</form>
</div>
</details>
}
// UploadChatPage is the no-JS full page for chatting with an uploaded transcript: // UploadChatPage is the no-JS full page for chatting with an uploaded transcript:
// the same chat panel as a video's (ADR-027), scoped to this recording. // the same chat panel as a video's (ADR-027), scoped to this recording.
templ UploadChatPage(u domain.UploadedTranscript, v chatView) { templ UploadChatPage(u domain.UploadedTranscript, v chatView) {
+140
View File
@@ -0,0 +1,140 @@
package web_test
// A transcript produced by the cloud fallback means this recording's audio left the
// homelab, which the reader is told in plain words (#52). "Local" is config.IsLocalModel:
// one definition, not a second string match in the view.
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func setEndpoint(t *testing.T, id, endpoint string) {
t.Helper()
_, err := rawPool(t).Exec(context.Background(), `UPDATE uploaded_transcripts SET endpoint = $2 WHERE id = $1`, id, endpoint)
require.NoError(t, err)
}
func TestUploadStatus_ACloudServedTranscriptSaysTheAudioLeftTheHomelab(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, "berget/kb-whisper")
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.Contains(t, b, "left the homelab")
assert.Contains(t, b, "cloud service (berget)")
}
func TestUploadStatus_ALocallyServedTranscriptCarriesNoCloudNote(t *testing.T) {
for _, endpoint := range []string{"iguana/kb-whisper", "koala/whisper", ""} {
t.Run(endpoint, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, endpoint)
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.NotContains(t, b, "left the homelab")
})
}
}
func TestUploadStatus_AnEndpointThatOnlyLooksLocalIsStillNamedAsCloud(t *testing.T) {
// The prefix alone must not decide it: IsLocalModel refuses path tricks.
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, "iguana/../berget/kb-whisper")
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.Contains(t, b, "left the homelab")
}
// The two-engine path (ADR-034) writes one composite endpoint string. Every name in it is
// a local model, so the audio never left the homelab and the note must not say it did.
func TestUploadStatus_AReconciledAllLocalEndpointCarriesNoCloudNote(t *testing.T) {
for _, endpoint := range []string{
"iguana/kb-whisper + koala/whisper, reconciled by iguana/gemma4-26b",
"iguana/kb-whisper + koala/whisper, reconciled by iguana/gemma4-26b\n",
} {
t.Run(endpoint, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, endpoint)
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.NotContains(t, b, "left the homelab")
})
}
}
func TestUploadStatus_AReconciledEndpointNamingACloudEngineStillNamesTheCloudProvider(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, "iguana/kb-whisper + berget/kb-whisper, reconciled by iguana/gemma4-26b")
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.Contains(t, b, "cloud service (berget)")
}
// An alias without a provider prefix is not a local model, wherever it sits in the
// composite: it must never be dropped from the judgement (PR #58 re-review).
func TestUploadStatus_AReconciledEndpointWithAnUnprefixedAliasIsNotCalledLocal(t *testing.T) {
for _, endpoint := range []string{
"iguana/kb-whisper + whisper-1, reconciled by iguana/gemma4-26b",
"iguana/kb-whisper + whisper 1, reconciled by iguana/gemma4-26b",
"iguana/kb-whisper + berget / x, reconciled by iguana/gemma4-26b",
} {
t.Run(endpoint, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, endpoint)
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.Contains(t, b, "left the homelab")
assert.NotContains(t, b, "cloud service ()", "the note names a provider")
})
}
}
// "by", "reconciled" and "+" are connectors only inside the composite format. As a whole
// endpoint, or as an alias within the composite, they are model names and are judged.
func TestUploadStatus_AConnectorWordUsedAsAModelNameIsStillJudged(t *testing.T) {
for _, endpoint := range []string{
"by",
"reconciled",
"+",
"iguana/kb-whisper + by, reconciled by iguana/gemma4-26b",
"iguana/kb-whisper + reconciled, reconciled by iguana/gemma4-26b",
"iguana/kb-whisper + +, reconciled by iguana/gemma4-26b",
"/x", // no provider at all: the note must still name something
} {
t.Run(endpoint, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, endpoint)
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.Contains(t, b, "left the homelab")
assert.NotContains(t, b, "cloud service ()")
})
}
}
func TestUploadStatus_AReconcilerThatIsNotLocalNamesTheCloudProvider(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", "done", "Synthetic transcript text.")
setEndpoint(t, id, "iguana/kb-whisper + koala/whisper, reconciled by berget/mistral-large")
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
assert.Contains(t, b, "cloud service (berget)")
}
@@ -0,0 +1,85 @@
package web_test
// Deleting one recording (#37). Owner-scoped like every other upload route: another user's
// id is a 404 and the row survives; an upload still being processed is refused.
import (
"context"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func deleteReq(id string) *http.Request {
return httptest.NewRequest(http.MethodPost, "/uploads/"+id+"/delete", nil)
}
func uploadExists(t *testing.T, id string) bool {
t.Helper()
var n int
require.NoError(t, rawPool(t).QueryRow(context.Background(), `SELECT count(*) FROM uploaded_transcripts WHERE id = $1`, id).Scan(&n))
return n == 1
}
func TestUploadDelete_AnOwnerRemovesAFinishedOrFailedRecordingAndLandsOnTheList(t *testing.T) {
for _, status := range []string{"done", "failed"} {
t.Run(status, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "Roadmap sync", status, "transcription_failed")
rec := do(t, app, deleteReq(id))
assert.Equal(t, http.StatusSeeOther, rec.Code)
assert.Equal(t, "/uploads", rec.Header().Get("Location"))
assert.False(t, uploadExists(t, id))
})
}
}
func TestUploadDelete_AnotherUsersRecordingIs404AndSurvives(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
theirs := seedUpload(t, otherUserID, "Their private meeting", "done", meetingText)
rec := do(t, app, deleteReq(theirs))
assert.Equal(t, http.StatusNotFound, rec.Code)
assert.True(t, uploadExists(t, theirs), "a non-owner's delete must not remove the row")
}
func TestUploadDelete_ARecordingStillBeingTranscribedIsRefusedWith409AndKept(t *testing.T) {
for _, status := range []string{"queued", "transcribing"} {
t.Run(status, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", status, "")
rec := do(t, app, deleteReq(id))
assert.Equal(t, http.StatusConflict, rec.Code)
assert.Contains(t, body(t, rec), "still being transcribed")
assert.True(t, uploadExists(t, id))
})
}
}
func TestUploadDelete_ANonIDIsA404NotAServerError(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
assert.Equal(t, http.StatusNotFound, do(t, app, deleteReq("not-a-uuid")).Code)
}
func TestUploadPage_OffersDeleteOnlyForAFinishedOrFailedRecording(t *testing.T) {
for status, want := range map[string]bool{"done": true, "failed": true, "queued": false, "transcribing": false} {
t.Run(status, func(t *testing.T) {
app, _ := newUploadApp(t, audioOK, 200<<20)
id := seedUpload(t, userID, "m", status, "transcription_failed")
b := body(t, do(t, app, get("/uploads/"+id)))
assert.Equal(t, want, strings.Contains(b, `action="/uploads/`+id+`/delete"`))
})
}
}
+19
View File
@@ -0,0 +1,19 @@
package web_test
// /uploads was mounted but nothing linked to it (#37). The shared Layout has no access to
// the App, so the entry point is the home list, shown only when uploads are mounted.
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestHomeList_LinksToRecordingsOnlyWhenUploadsAreMounted(t *testing.T) {
with, _ := newUploadApp(t, audioOK, 200<<20)
without := newApp(t)
assert.Contains(t, body(t, do(t, with, get("/"))), `href="/uploads"`)
assert.NotContains(t, body(t, do(t, without, get("/"))), `href="/uploads"`,
"a link to a route that is not mounted would be a dead end")
}
File diff suppressed because it is too large Load Diff
+4 -1
View File
@@ -107,9 +107,12 @@ templ flashBanner(code string) {
// #summary-list region; a non-HTMX request renders the whole page. flash carries // #summary-list region; a non-HTMX request renders the whole page. flash carries
// a one-shot notification (e.g. "connected", "registered") surfaced on arrival // a one-shot notification (e.g. "connected", "registered") surfaced on arrival
// after a POST→redirect. // after a POST→redirect.
templ ListPage(b listBuckets, f Filter, stats PipelineStats, flash string, hasConnected bool, channels []string, autoSummarize bool) { templ ListPage(b listBuckets, f Filter, stats PipelineStats, flash string, hasConnected bool, channels []string, autoSummarize bool, uploadsEnabled bool) {
@Layout("Tapir — Summaries") { @Layout("Tapir — Summaries") {
@flashBanner(flash) @flashBanner(flash)
if uploadsEnabled {
<p class="muted"><a href="/uploads">Recordings</a>: upload audio and get a private transcript.</p>
}
if hasConnected { if hasConnected {
@pasteForm() @pasteForm()
} }
File diff suppressed because it is too large Load Diff
@@ -122,6 +122,12 @@ var scenarioCoverage = map[string]string{
"I can ask about my recording and get a grounded answer": "TestUploadChatPost_SendsTheOwnersTranscriptWithTheUploadBudget", "I can ask about my recording and get a grounded answer": "TestUploadChatPost_SendsTheOwnersTranscriptWithTheUploadBudget",
"A long recording is not cut to its first third": "TestAnswerMaxCharsOverridesTheServiceCap", "A long recording is not cut to its first third": "TestAnswerMaxCharsOverridesTheServiceCap",
"A transcript made by the cloud fallback says the audio left the homelab": "TestUploadStatus_ACloudServedTranscriptSaysTheAudioLeftTheHomelab",
"I can delete a finished recording": "TestUploadDelete_AnOwnerRemovesAFinishedOrFailedRecordingAndLandsOnTheList",
"A recording still being transcribed cannot be deleted": "TestUploadDelete_ARecordingStillBeingTranscribedIsRefusedWith409AndKept",
"The home list links to my recordings when uploads are on": "TestHomeList_LinksToRecordingsOnlyWhenUploadsAreMounted",
// audio_upload.feature, ADR-034 // audio_upload.feature, ADR-034
"Two engines transcribe the audio and their disagreements are reconciled": "TestEnsemble_BothEnginesRunInOrderAndTheirDisagreementIsReconciled", "Two engines transcribe the audio and their disagreements are reconciled": "TestEnsemble_BothEnginesRunInOrderAndTheirDisagreementIsReconciled",
"Trivial disagreements never reach the model": "TestReconcile_TrivialDisputesAreResolvedWithoutAskingTheModel", "Trivial disagreements never reach the model": "TestReconcile_TrivialDisputesAreResolvedWithoutAskingTheModel",
+143
View File
@@ -0,0 +1,143 @@
package lintgate
// Observes the exit status of `task lint` (the lint step of `task check`), not the lint
// rules themselves. It runs the repo's real Taskfile in a scratch module so a change to the
// lint command that stops it failing on findings (#41) turns this red.
import (
"bytes"
"errors"
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
"testing"
)
const scratchWithFinding = `package main
import "os"
func main() { os.Remove("x") }
`
func scratchModule(t *testing.T) string {
t.Helper()
taskfile, err := os.ReadFile(filepath.Join("..", "..", "Taskfile.yml"))
if err != nil {
t.Fatalf("read Taskfile.yml: %v", err)
}
dir := t.TempDir()
for name, body := range map[string]string{
"Taskfile.yml": string(taskfile),
"go.mod": "module scratch\n\ngo 1.21\n",
"main.go": scratchWithFinding,
} {
if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil {
t.Fatalf("write %s: %v", name, err)
}
}
return dir
}
// ansi matches the colour escape sequences task prints when it thinks it has a terminal.
var ansi = regexp.MustCompile("\x1b\\[[0-9;]*m")
// hasSkipNotice reports whether the skip notice was printed as output. Task echoes the command
// line first, and that line contains the same words, so a substring match would be vacuous.
func hasSkipNotice(out string) bool {
for _, line := range strings.Split(ansi.ReplaceAllString(out, ""), "\n") {
if strings.HasPrefix(line, "golangci-lint not installed") {
return true
}
}
return false
}
// runLint runs `task lint` in dir with the given PATH and returns its combined output and exit code.
func runLint(t *testing.T, dir, path string) (string, int) {
t.Helper()
return runTask(t, dir, path, "lint")
}
func runTask(t *testing.T, dir, path string, args ...string) (string, int) {
t.Helper()
taskBin, err := exec.LookPath("task")
if err != nil {
t.Skip("task not installed")
}
cmd := exec.Command(taskBin, args...)
cmd.Dir = dir
cmd.Env = append(os.Environ(), "PATH="+path)
var out bytes.Buffer
cmd.Stdout, cmd.Stderr = &out, &out
err = cmd.Run()
var exitErr *exec.ExitError
switch {
case err == nil:
return out.String(), 0
case errors.As(err, &exitErr):
return out.String(), exitErr.ExitCode()
default:
t.Fatalf("run task lint: %v", err)
return "", -1
}
}
func TestTaskLint_ExitsNonZeroWhenGolangciLintReportsAFinding(t *testing.T) {
if _, err := exec.LookPath("golangci-lint"); err != nil {
t.Skip("golangci-lint not installed")
}
out, code := runLint(t, scratchModule(t), os.Getenv("PATH"))
if code == 0 {
t.Fatalf("task lint exited 0 on a module with an unchecked error; output:\n%s", out)
}
if hasSkipNotice(out) {
t.Fatalf("task lint blamed a missing binary for a lint failure; output:\n%s", out)
}
if !strings.Contains(out, "errcheck") {
t.Fatalf("a non-zero exit without the finding means the linter crashed, not that it found something; output:\n%s", out)
}
}
// The lint step only gates anything if `task check` runs it.
func TestTaskCheck_RunsTheLintStep(t *testing.T) {
out, code := runTask(t, scratchModule(t), os.Getenv("PATH"), "--dry", "check")
if code != 0 {
t.Fatalf("task --dry check exited %d; output:\n%s", code, out)
}
if !strings.Contains(out, "task: [lint]") {
t.Fatalf("task check no longer runs the lint step; output:\n%s", out)
}
}
func TestTaskLint_SkipsWithANoticeWhenGolangciLintIsAbsent(t *testing.T) {
taskBin, err := exec.LookPath("task")
if err != nil {
t.Skip("task not installed")
}
bin := t.TempDir() // a PATH holding task and nothing else
if err := os.Symlink(taskBin, filepath.Join(bin, "task")); err != nil {
t.Fatalf("symlink task: %v", err)
}
out, code := runLint(t, scratchModule(t), bin)
if code != 0 {
t.Fatalf("task lint exited %d without golangci-lint on PATH; output:\n%s", code, out)
}
if !hasSkipNotice(out) {
t.Fatalf("expected the skip notice; output:\n%s", out)
}
}
// CI runs task with colour on, so the notice line starts with an escape sequence; a bare
// prefix match missed it there while passing on a terminal-less local run.
func TestHasSkipNotice_SeesTheNoticeBehindColourCodes(t *testing.T) {
out := "\x1b[32mtask: [lint] ...\n\x1b[0mgolangci-lint not installed — skipping (CI installs it)\n"
if !hasSkipNotice(out) {
t.Fatalf("notice not found in coloured output: %q", out)
}
if hasSkipNotice("\x1b[32mtask: [lint] if command -v golangci-lint ...; echo \"golangci-lint not installed\"\n") {
t.Fatal("the echoed command line must not count as the notice")
}
}