Compare commits
61
Commits
+55
-11
@@ -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
|
||||
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
|
||||
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,
|
||||
@@ -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)
|
||||
disabled by default, a failed iguana job fails honestly and the user retries. *Blocks:*
|
||||
the production default only; the adapter ships either way.
|
||||
- Verify Tapir's ingress `proxy-body-size` against the 200 MB cap (`infra`).
|
||||
- Per-transcript deletion is not built; account deletion cascades. A private-content store
|
||||
without a delete button is a gap worth a ticket, not a silent omission.
|
||||
- ~~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.~~ **Built 2026-10-03
|
||||
(#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
|
||||
surface, one image change. Removing the upload route removes the feature; `transcripts`, `videos`
|
||||
@@ -1735,17 +1760,36 @@ wording while only `content` is merged.
|
||||
`network`, `error`), never its text, which can hold a provider's response body, the spool path or
|
||||
a word from the transcript (`internal/errkind`). The stored reconcile reason and an engine's
|
||||
failure reason are kinds; the page no longer shows provider text.
|
||||
- **Every local engine call has its own deadline** (`TAPIR_TRANSCRIBE_ENGINE_TIMEOUT`, default 25
|
||||
minutes), so a wedged engine fails while the job lives and the fallback can run; the job's own
|
||||
- **Every local engine call has its own deadline** (`TAPIR_TRANSCRIBE_ENGINE_TIMEOUT`, default 20
|
||||
minutes: two wedged engines in sequence then leave 20 of the job's 60 minutes for a fallback),
|
||||
so a wedged engine fails while the job lives and the fallback can run; the job's own
|
||||
deadline or a cancel still never reaches the cloud.
|
||||
- **The duration limit is checked against what is converted**, not the file's header: ffmpeg is
|
||||
capped at the limit plus a minute, a longer result fails the job (`audio_too_long`), and a result
|
||||
under 80% of the claimed length and more than two minutes short fails it
|
||||
(`audio_incomplete`) instead of returning a partial transcript with no warning.
|
||||
- **The queue is bounded**, 20 waiting in total and 3 per user; a full queue is a 429 with a typed
|
||||
reason, never a stuck row or a server error.
|
||||
- **Bad input is a 4xx**: a non-UUID id is a 404, a malformed multipart header is a 400 whose text is
|
||||
not logged, and a title or file name with control bytes or invalid UTF-8 is cleaned, not a 500.
|
||||
under 80% of the claimed length and more than two minutes short is **transcribed and noted** on
|
||||
the audio step ("the audio is 20 min long; the file claimed 1 h 19 min"), not refused. The second
|
||||
adversarial review showed a complete variable-bitrate MP3 whose header over-estimated its length
|
||||
by 4x was wrongly refused by the first version of this check; a note keeps a truncated file
|
||||
visible without turning away real recordings.
|
||||
- **The queue is bounded**, 20 waiting in total and 3 per user, and its room is checked before the
|
||||
upload is spooled (not after 200 MB and an ffprobe run); a full queue is a 429 with a typed reason,
|
||||
never a stuck row or a server error.
|
||||
- **The configured upload chat model is callable.** It is added to the shared chat service's model
|
||||
list when uploads are on and it is local, because the service re-maps a model it does not offer
|
||||
(a valid `iguana/gemma4-31b` was being silently swapped for `koala/phi4-mini`). The video chat's own switcher
|
||||
lists that local model too while uploads are on.
|
||||
- **A client that hangs up mid-upload is not an error**: the handler logs INFO and writes nothing
|
||||
(rendering the error page ran a database read on a cancelled context and logged a false ERROR). The
|
||||
400 "cut off" answer is keyed on a marker for errors from *reading the client's body*
|
||||
(`uploads.ErrClientRead`), not on `io.ErrUnexpectedEOF`, which is also what a database drop looks like.
|
||||
- **A note is not hidden**: a finished upload's record opens by default when any step carries a note.
|
||||
- **Bad input is a 4xx**: a non-UUID id is a 404, a malformed or cut-off multipart body is a 400
|
||||
whose text is not logged, and a title with control bytes, bidirectional controls or invalid UTF-8 is
|
||||
cleaned (emoji joiners, Swedish letters and variation selectors are kept), not a 500.
|
||||
- **The image was built and inspected** (buildah on koala, 2026-10-02): the first build FAILED
|
||||
(`tar: xz: Cannot exec`, the golang image has no xz); fixed, rebuilt at 180 MB, ffmpeg and ffprobe
|
||||
run as `nonroot` in distroless. CI does not build the image on a pull request
|
||||
(`if: github.event_name != 'pull_request'`), so the first CI build of it is the merge to main.
|
||||
Not done, filed as follow-ups: a vendored ffmpeg tarball, CSRF cookie review, ffprobe CPU and
|
||||
stream-count limits, mov external references, `alive_at` clock skew, a hung database blocking the
|
||||
tracker, an unbounded gateway response body, a user-facing notice for a recording that is partial but
|
||||
|
||||
@@ -36,7 +36,11 @@ FROM golang:1.26 AS ffmpeg
|
||||
ARG FFMPEG_VERSION=7.0.2
|
||||
ARG FFMPEG_SHA256=abda8d77ce8309141f83ab8edf0596834087c52467f6badf376a6a2a4c87cf67
|
||||
|
||||
# xz is not in the golang image (found by building it: `tar: xz: Cannot exec`).
|
||||
RUN set -eu; \
|
||||
apt-get update; \
|
||||
apt-get install -y --no-install-recommends xz-utils; \
|
||||
rm -rf /var/lib/apt/lists/*; \
|
||||
mkdir -p /out; \
|
||||
for dir in releases old-releases; do \
|
||||
curl -fsSL -o /out/ffmpeg.tar.xz "https://johnvansickle.com/ffmpeg/${dir}/ffmpeg-${FFMPEG_VERSION}-amd64-static.tar.xz" && break; \
|
||||
|
||||
+1
-1
@@ -34,7 +34,7 @@ tasks:
|
||||
lint:
|
||||
desc: golangci-lint (installed by CI; skipped locally if absent).
|
||||
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:
|
||||
desc: Run all tests. -p 1 serializes packages so the embedded-postgres tests
|
||||
|
||||
+15
-14
@@ -17,9 +17,9 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"os/signal"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
@@ -41,7 +41,7 @@ func main() {
|
||||
}
|
||||
|
||||
log := slog.New(slog.NewTextHandler(os.Stderr, nil))
|
||||
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt)
|
||||
ctx, stop := shutdownContext(context.Background())
|
||||
defer stop()
|
||||
|
||||
var err error
|
||||
@@ -215,9 +215,11 @@ func cmdServe(ctx context.Context, log *slog.Logger) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
var workerDone chan struct{} // nil without uploads: nothing to wait for at shutdown
|
||||
wireUploads(app, cfg, intake)
|
||||
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,
|
||||
"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)")
|
||||
@@ -350,20 +352,19 @@ func cmdServe(ctx context.Context, log *slog.Logger) error {
|
||||
}()
|
||||
}
|
||||
|
||||
// Graceful shutdown on signal: stop accepting, drain in-flight requests.
|
||||
go func() {
|
||||
<-ctx.Done()
|
||||
shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
_ = srv.Shutdown(shutdownCtx)
|
||||
// Graceful shutdown on signal: stop accepting, drain in-flight requests, and let the
|
||||
// worker record the job it was running as interrupted (#50).
|
||||
var extra []*http.Server
|
||||
if metricsSrv != nil {
|
||||
_ = metricsSrv.Shutdown(shutdownCtx)
|
||||
extra = append(extra, metricsSrv)
|
||||
}
|
||||
ln, err := net.Listen("tcp", srv.Addr)
|
||||
if err != nil {
|
||||
return fmt.Errorf("serve: listen: %w", err)
|
||||
}
|
||||
}()
|
||||
|
||||
log.Info("serving web ui", "addr", cfg.HTTPAddr, "user", cfg.UserID)
|
||||
if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||
return fmt.Errorf("serve: %w", err)
|
||||
if err := serveHTTP(ctx, srv, ln, extra, workerDone, shutdownGrace); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -134,6 +134,13 @@ func chatModels(cfg config.Config) []string {
|
||||
add(cfg.SummarizerModel)
|
||||
add(cfg.FallbackModel)
|
||||
add(cfg.CloudFallbackModel)
|
||||
// The model chat over an uploaded recording opens on must be one the shared service can
|
||||
// call: it re-maps a model it does not offer, which silently swapped a configured 31B for
|
||||
// phi4-mini, whose context cannot hold an hour of speech. Local only, and only when
|
||||
// uploads are on, so the video chat is unchanged otherwise (second adversarial review).
|
||||
if cfg.UploadsEnabled && config.IsLocalModel(cfg.UploadChatModel) {
|
||||
add(cfg.UploadChatModel)
|
||||
}
|
||||
return models
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -44,3 +44,42 @@ func TestUploadChatModels_NoLocalModelMeansNoneOfferedNotACloudDefault(t *testin
|
||||
t.Errorf("uploadChatModels = %v, want none", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A valid local TAPIR_UPLOAD_CHAT_MODEL that is not in the video chat's chain used to be
|
||||
// accepted by config and then silently ignored: the shared chat service re-maps a model it
|
||||
// does not offer, so the user got koala/phi4-mini, whose 8,192-token context cannot hold an
|
||||
// hour of speech (second adversarial review).
|
||||
|
||||
func TestChatModels_AnUploadChatModelOutsideTheChainIsOfferedWhenUploadsAreOn(t *testing.T) {
|
||||
var cfg config.Config
|
||||
cfg.SummarizerModel = "koala/phi4-mini"
|
||||
cfg.FallbackModel = "iguana/gemma4-26b"
|
||||
cfg.UploadsEnabled = true
|
||||
cfg.UploadChatModel = "iguana/gemma4-31b"
|
||||
|
||||
service := chatModels(cfg)
|
||||
upload := uploadChatModels(cfg)
|
||||
|
||||
if !reflect.DeepEqual(service, []string{"koala/phi4-mini", "iguana/gemma4-26b", "iguana/gemma4-31b"}) {
|
||||
t.Errorf("chatModels = %v: the shared service must be able to call the configured upload model", service)
|
||||
}
|
||||
if !reflect.DeepEqual(upload, service) {
|
||||
t.Errorf("uploadChatModels = %v, want %v", upload, service)
|
||||
}
|
||||
}
|
||||
|
||||
func TestChatModels_NothingIsAddedWhenUploadsAreOffOrTheModelIsNotLocal(t *testing.T) {
|
||||
var cfg config.Config
|
||||
cfg.SummarizerModel = "koala/phi4-mini"
|
||||
cfg.UploadChatModel = "iguana/gemma4-31b"
|
||||
|
||||
if got := chatModels(cfg); !reflect.DeepEqual(got, []string{"koala/phi4-mini"}) {
|
||||
t.Errorf("uploads off: chatModels = %v, want the video chain unchanged", got)
|
||||
}
|
||||
|
||||
cfg.UploadsEnabled = true
|
||||
cfg.UploadChatModel = "berget/mistral-small"
|
||||
if got := chatModels(cfg); !reflect.DeepEqual(got, []string{"koala/phi4-mini"}) {
|
||||
t.Errorf("a cloud upload chat model was added to the service: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -44,7 +44,7 @@ func (f *fakeUploadsStore) SaveProgress(context.Context, string, string, []domai
|
||||
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) {
|
||||
f.sweeps++
|
||||
|
||||
@@ -43,3 +43,36 @@ func TestBuildTranscriber_AWedgedPrimaryReachesTheFallbackBeforeTheJobDeadline(t
|
||||
require.NoError(t, err, "the wedged primary used the whole job and the fallback never ran")
|
||||
assert.Equal(t, "berget/kb-whisper", got.Endpoint)
|
||||
}
|
||||
|
||||
func TestBuildTranscriber_AWedgedBaseEngineIsASoftFailureInTheEnsemble(t *testing.T) {
|
||||
// The same deadline must guard the ensemble's BASE engine, not only the chain's primary
|
||||
// (second adversarial review: the wiring was tested for the chain alone).
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_ = r.ParseMultipartForm(1 << 20)
|
||||
if r.FormValue("model") == "iguana/pianissimo" {
|
||||
<-r.Context().Done()
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"text":"hej","duration":1,"segments":[{"start":0,"end":1,"text":"hej"}]}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
cfg := uploadCfg(t, true)
|
||||
cfg.GatewayURL = srv.URL
|
||||
cfg.TranscribeModel = "iguana/kb-whisper"
|
||||
cfg.ReconcileBaseModel = "iguana/pianissimo"
|
||||
cfg.ReconcileModel = "iguana/gemma4-31b"
|
||||
cfg.TranscribeEngineTimeout = 150 * time.Millisecond
|
||||
wav := filepath.Join(t.TempDir(), "a.wav")
|
||||
require.NoError(t, os.WriteFile(wav, []byte("RIFF....WAVEfmt "), 0o600))
|
||||
job, cancel := context.WithTimeout(context.Background(), 20*time.Second)
|
||||
defer cancel()
|
||||
|
||||
got, err := buildTranscriber(cfg).Transcribe(job, wav)
|
||||
|
||||
require.NoError(t, err, "a wedged base engine must not use up the job")
|
||||
assert.Equal(t, "hej", got.Text)
|
||||
require.NotNil(t, got.Reconciliation)
|
||||
assert.Equal(t, "single_engine", string(got.Reconciliation.Status))
|
||||
assert.Contains(t, got.Reconciliation.Reason, "timeout")
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
- **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
|
||||
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
|
||||
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
|
||||
@@ -23,7 +24,7 @@ it** — endpoints and aliases drift, and this file is a snapshot (2026-06-06),
|
||||
`{"error":"failed to process audio"}` string — it has had at least five causes. Tapir's knobs:
|
||||
`TAPIR_UPLOADS_ENABLED` (off by default), `TAPIR_UPLOAD_MAX_MB` (200), `TAPIR_UPLOAD_MAX_MINUTES`
|
||||
(90), `TAPIR_TRANSCRIBE_MODEL`, `TAPIR_TRANSCRIBE_FALLBACK_MODEL` (empty disables the cloud
|
||||
fallback), `TAPIR_TRANSCRIBE_TIMEOUT` (60m, the whole job), `TAPIR_TRANSCRIBE_ENGINE_TIMEOUT` (25m, one engine call: a wedged engine then fails while the job lives and the fallback can run), `TAPIR_UPLOAD_TMP_DIR`, `TAPIR_FFMPEG_BIN`,
|
||||
fallback), `TAPIR_TRANSCRIBE_TIMEOUT` (60m, the whole job), `TAPIR_TRANSCRIBE_ENGINE_TIMEOUT` (20m, one engine call: a wedged engine then fails while the job lives and the fallback can run), `TAPIR_UPLOAD_TMP_DIR`, `TAPIR_FFMPEG_BIN`,
|
||||
`TAPIR_FFPROBE_BIN`, `TAPIR_MAX_UPLOADED_TRANSCRIPT_CHARS` (90000 — the measured ceiling of
|
||||
`iguana/gemma4-26b`, which silently drops the head above it), `TAPIR_UPLOAD_CHAT_MODEL`
|
||||
(`iguana/gemma4-26b`; must be a local `koala/` or `iguana/` alias, since the whole private transcript is sent to it; `koala/phi4-mini`'s 8,192-token context 400s on an hour of speech). `ffmpeg` + `ffprobe` are in the runtime image (static, pinned by sha256; #34 decided A on 2026-10-02); Tapir's own ingress body limit for a 200 MB upload is unverified (infra).
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
# ADR-034 second blind check: protocol (ready to run; tapir#33, D9, D10)
|
||||
|
||||
This repo is public and the recordings are private, so this file holds the method only. The
|
||||
sheet, the key and the filled-in verdicts stay outside the repo, and the trial data is deleted
|
||||
after the check. Results go back here as aggregate numbers, as in
|
||||
`reconcile-blind-check-2026-10-02.md`.
|
||||
|
||||
## What the check decides
|
||||
|
||||
ADR-034 (two engines, a local model reconciles their disagreements) is `Proposed` and off in
|
||||
production. Two conditions were set for keeping it. The first run answered the first one on one
|
||||
reading only and did not answer the second:
|
||||
|
||||
1. **The reconciled reading is right at least 85% of the time.** Open flaw in the bar: it did not
|
||||
say how ties count. The first run gave 95% (decided items only) or 70% (all 30), so it was met on
|
||||
one reading and missed on the other.
|
||||
2. **It is no worse than the better single engine.** Not answered: before ADR-035 the spots the
|
||||
model *kept* as the base were not stored, so kb-whisper alone could not be compared.
|
||||
|
||||
ADR-035 now stores every judged spot (`reconciliation.contested`: both readings, which one was
|
||||
used, unsure or not, a few words of left context, a time). That makes condition 2 measurable.
|
||||
|
||||
## Rules to fix BEFORE looking at any result (the owner confirms or changes them)
|
||||
|
||||
| Rule | Proposed |
|
||||
|---|---|
|
||||
| Who judges | The speaker, hearing the audio where the text cannot settle it (D9). Not Opus: it cannot hear, and its agreement with the speaker was only shown on items picked as decidable. |
|
||||
| Sample | 30 spots drawn at random with a fixed seed, from ALL judged spots (changed and kept-base alike), not only the changed ones. Insertions and deletions that cannot be placed are listed separately and not sampled. |
|
||||
| Verdict per spot | `x`, `y` (the two readings, shown in random order, key kept apart), `neither`, or `cannot tell`. |
|
||||
| Condition 1 | Reconciled is right on at least 85% of the spots where the judge could tell (`cannot tell` excluded, reported, and `neither` counts as not right). Also report all-30. Pass needs the decided-only figure at 85% or more AND the all-30 figure above 70%. |
|
||||
| Condition 2 | Over the same spots, the reconciled text is right at least as often as the better single engine (base engine alone is right where the base reading is right; the other engine alone where the alt reading is right). Ties favour the single engine. |
|
||||
| If it fails | Switch reconcile off by configuration; the single-engine path is unchanged (ADR-034 reversibility). |
|
||||
|
||||
Why a fixed all-30 floor: the literal 70% last time sat well under the bar, and "decided only" can
|
||||
hide a model that mostly lands on spelling differences.
|
||||
|
||||
## What a run needs (D10: the owner is present)
|
||||
|
||||
- The same real recording as before (about 60 minutes). Only the owner has it.
|
||||
- A **throwaway** tapir instance with its own Postgres, not the production pod, so production's
|
||||
key pin and deployment are untouched. Settings: `TAPIR_RECONCILE_BASE_MODEL=iguana/pianissimo`,
|
||||
`TAPIR_TRANSCRIBE_MODEL=iguana/kb-whisper`, `TAPIR_RECONCILE_MODEL=iguana/gemma4-31b`, cloud
|
||||
fallbacks empty. Job time was 21 min 49 s per 63.6 min last run.
|
||||
- iguana with Low Power Mode off and gemma4-31b able to load (the production-iguana model load
|
||||
that needs a quiet window; D6).
|
||||
|
||||
## Steps
|
||||
|
||||
1. Start the throwaway instance, upload the recording, wait for `done` (about 22 minutes).
|
||||
2. Read `reconciliation.contested` from the throwaway database. From it build, **outside the repo**:
|
||||
`sheet` (spot number, time, left context, reading x, reading y, random order) and `key` (which
|
||||
of x/y is the base, which is the other engine, which one the reconciler used).
|
||||
3. The speaker fills in the sheet (about 20 to 30 minutes for 30 spots, audio open at the times).
|
||||
The agent does not see the key until the sheet is returned.
|
||||
4. Score with the rules above. Write the aggregate numbers into a dated note next to this one,
|
||||
and amend ADR-034's validation section (kept, or switched off, and the tie rule used).
|
||||
5. Delete the throwaway instance, its database and the sheet, as last time.
|
||||
6. Only if the check passes and the owner wants it in production: enabling reconcile there is
|
||||
the separate D10 step (it needs `iguana/pianissimo` and the reconciler added to the tapir key
|
||||
pin in the infra repo plus the env vars on the deployment, `risk/medium`, owner present).
|
||||
|
||||
## Open and owner-only
|
||||
|
||||
- Confirm or change the rules table above before the run.
|
||||
- The **tool that builds the sheet and key** is the one piece not in the repo (last time it was a
|
||||
one-off script). It can be built test-first on request; until then the agent builds it ad hoc
|
||||
outside the repo.
|
||||
- A Berget adjudicator (D8, tapir#45) is a different question with its own measurement and is
|
||||
not part of this check: this check judges the local setup that ADR-034 describes.
|
||||
@@ -0,0 +1,115 @@
|
||||
# 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: **v0.31.1 and v0.32.0 shipped 2026-10-03/04, the owner-held items
|
||||
below remain open**). 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 | **Decided 2026-10-04 by the owner: yes, Berget may be used, and each user must be able to keep a recording on local models only** (reverses the recommendation). Opt-out first: #60; #45 stays as the model measurement | #60, #45 |
|
||||
| D9 | Who judges ADR-034's second blind check | The owner; do not reuse the one-off Opus authorisation. Protocol: `docs/research/reconcile-second-check-protocol.md` | 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).
|
||||
|
||||
## Status (2026-10-04)
|
||||
|
||||
**Shipped and deployed:** v0.31.1 (PR #55, merge e037c6b) and v0.32.0 (PR #58, merge 85f4cc3; pod
|
||||
tapir-597dbfccc-ng78q 1/1, 0 restarts, image 85f4cc3). Each went through an adversarial review
|
||||
round by agent with the verdicts recorded on the PR (v0.31.1: BLOCK then APPROVE; v0.32.0: BLOCK,
|
||||
BLOCK, APPROVE, APPROVE; every block was a real defect, fixed test-first).
|
||||
|
||||
**Live evidence:** a foreign-origin `POST` to `/account/delete` and to `/uploads/{id}/delete` is
|
||||
refused (403) on production, a same-origin one reaches the login redirect (302).
|
||||
|
||||
**Not observed live (needs an authenticated session, so it is the owner's look; closed on word or screenshot):**
|
||||
|
||||
- #52, cloud side: a transcript served by the Berget fallback names `berget`; a reconciled all-local
|
||||
one shows no note. The local side was observed on 2026-10-04 (a new clip served by iguana: chip
|
||||
`iguana/…`, no cloud note). The cloud side is carried as an acceptance item in #60.
|
||||
|
||||
**Observed live since:** #37 (owner, 2026-10-04: link, confirm-then-delete, no control while transcribing; closed). #50: at the 2026-10-04 rollout the old pod went `Terminating` then `Completed`
|
||||
(not `Error`), 0 restarts; #50 closed. No job was in flight, so a drained job is still unobserved
|
||||
(a recorded limitation, D2).
|
||||
|
||||
**Still open (owner's call, milestone 11):** #36, #38, #45 (now the Berget adjudicator measurement), #60 (per-recording local-only choice, 4 sub-decisions), #46 remainder, #56 (slow-drip bodies hold
|
||||
a receive slot), #57 (shutdown logging, second signal). Decisions D4-D6, D9, D10 above are unchanged; D8 is decided (above).
|
||||
|
||||
**Deferred and filed from the reviews:** #56, #57, #59 (start-up exits when the identity provider's OIDC
|
||||
discovery returns 503 (seen once at 19:40 on 2026-10-03; the pod restarted itself and recovered)).
|
||||
@@ -235,9 +235,28 @@ Feature: Upload an audio recording, get a private Swedish transcript, discuss it
|
||||
When a file claims to be short but converts to more than the limit
|
||||
Then the job fails with a plain explanation and no engine receives the audio
|
||||
|
||||
Scenario: A truncated recording is refused, not half transcribed
|
||||
When a file converts to far less audio than it claimed
|
||||
Then the job fails and asks for the complete recording
|
||||
Scenario: A recording much shorter than its file claimed is transcribed and noted
|
||||
When a file converts to under 80 percent of the length it claimed and more than two minutes short
|
||||
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
|
||||
|
||||
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
|
||||
When too many recordings are already waiting
|
||||
|
||||
@@ -33,9 +33,9 @@ require (
|
||||
github.com/rogpeppe/go-internal v1.15.0 // indirect
|
||||
github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect
|
||||
go.yaml.in/yaml/v2 v2.4.2 // indirect
|
||||
golang.org/x/sync v0.18.0 // indirect
|
||||
golang.org/x/sync v0.21.0 // indirect
|
||||
golang.org/x/sys v0.41.0 // indirect
|
||||
golang.org/x/text v0.31.0 // indirect
|
||||
golang.org/x/text v0.39.0 // indirect
|
||||
google.golang.org/protobuf v1.36.8 // indirect
|
||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||
)
|
||||
|
||||
@@ -113,12 +113,12 @@ go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
|
||||
go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
|
||||
golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs=
|
||||
golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q=
|
||||
golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I=
|
||||
golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
|
||||
golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM=
|
||||
golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
|
||||
golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k=
|
||||
golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
|
||||
golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM=
|
||||
golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
|
||||
golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus=
|
||||
golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM=
|
||||
golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U=
|
||||
golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno=
|
||||
google.golang.org/protobuf v1.36.8 h1:xHScyCOEuuwZEc6UtSOvPbAT4zRh0xcNRYekJwfqyMc=
|
||||
|
||||
@@ -47,11 +47,15 @@ func (f *FFmpeg) ToWAV(ctx context.Context, src, dst string) error {
|
||||
args = append(args, "-f", "wav", dst)
|
||||
cmd := exec.CommandContext(ctx, bin, args...)
|
||||
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 {
|
||||
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
|
||||
// missing file and let the error surface three layers later.
|
||||
|
||||
@@ -78,7 +78,7 @@ printf data > "$last"`)
|
||||
assert.Less(t, fw, in)
|
||||
}
|
||||
|
||||
func TestFFmpegToWAV_CapsTheOutputLengthWhenAMaximumIsSet(t *testing.T) {
|
||||
func TestFFmpegToWAV_PassesAnOutputLengthCapAfterTheInputWhenAMaximumIsSet(t *testing.T) {
|
||||
// The duration limit is a CLAIM in the file's header; a file that lies about it can
|
||||
// convert to hours of audio (adversarial review F2: a 2 h file claiming 10 s became a
|
||||
// 120-minute WAV). The cap is an OUTPUT option, after -i, with a minute of slack.
|
||||
@@ -97,7 +97,7 @@ printf data > "$last"`)
|
||||
assert.Less(t, strings.Index(joined, " -i /spool/in "), strings.Index(joined, " -t 5460 "), "-t after -i is an output cap; before it, an input one")
|
||||
}
|
||||
|
||||
func TestFFmpegToWAV_WithNoMaximumThereIsNoLengthCap(t *testing.T) {
|
||||
func TestFFmpegToWAV_PassesNoLengthCapWhenNoMaximumIsSet(t *testing.T) {
|
||||
rec := filepath.Join(t.TempDir(), "argv")
|
||||
bin := stub(t, `printf '%s\n' "$@" > `+rec+`
|
||||
for a; do last="$a"; done
|
||||
@@ -109,3 +109,32 @@ printf data > "$last"`)
|
||||
require.NoError(t, err)
|
||||
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")
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
package audio
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
@@ -19,6 +20,51 @@ import (
|
||||
type FFprobe struct {
|
||||
// Bin is the ffprobe executable ("ffprobe" resolves via PATH).
|
||||
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
|
||||
@@ -30,9 +76,22 @@ type FFprobe struct {
|
||||
// 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"
|
||||
|
||||
// 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.
|
||||
// Local files and pipes only: no network, no other protocols.
|
||||
var inputRestrictions = []string{"-protocol_whitelist", "file,pipe", "-format_whitelist", AllowedDemuxers}
|
||||
// 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,
|
||||
"-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.
|
||||
const killGrace = 2 * time.Second
|
||||
@@ -48,21 +107,38 @@ func (f *FFprobe) Probe(ctx context.Context, path string) (uploads.Probe, error)
|
||||
bin = "ffprobe"
|
||||
}
|
||||
args := append([]string{"-v", "error"}, inputRestrictions...)
|
||||
args = append(args, "-print_format", "json", "-show_format", "-show_streams", path)
|
||||
cmd := exec.CommandContext(ctx, bin, args...)
|
||||
args = append(args, "-print_format", "json", "-show_entries", probeEntries, path)
|
||||
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
|
||||
out, err := cmd.Output()
|
||||
if err != nil {
|
||||
if ctxErr := ctx.Err(); ctxErr != nil {
|
||||
maxOut := f.MaxOutput
|
||||
if maxOut <= 0 {
|
||||
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)
|
||||
}
|
||||
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
|
||||
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)
|
||||
}
|
||||
p, err := ParseProbe(out)
|
||||
p, err := ParseProbe(stdout.buf.Bytes())
|
||||
if err != nil {
|
||||
return uploads.Probe{}, fmt.Errorf("audio: ffprobe output: %w", err)
|
||||
}
|
||||
|
||||
@@ -184,3 +184,78 @@ func TestAllowedDemuxers_ExcludeThePlaylistFormats(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -37,6 +37,21 @@ type Client struct {
|
||||
temp float64
|
||||
httpClient *http.Client
|
||||
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
|
||||
@@ -101,6 +116,7 @@ func New(baseURL, apiKey, model string, timeout time.Duration, opts ...Option) *
|
||||
maxTokens: defaultMaxTokens,
|
||||
temp: defaultTemperature,
|
||||
httpClient: &http.Client{Timeout: timeout},
|
||||
maxBody: defaultMaxResponseBytes,
|
||||
}
|
||||
for _, opt := range opts {
|
||||
opt(c)
|
||||
@@ -184,12 +200,18 @@ func (c *Client) Complete(ctx context.Context, system, user string) (string, err
|
||||
}
|
||||
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 {
|
||||
return "", fmt.Errorf("read response: %w", err)
|
||||
}
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return "", &HTTPError{Code: resp.StatusCode, Body: string(out)}
|
||||
if int64(len(out)) > c.maxBody {
|
||||
return "", fmt.Errorf("LLM response exceeds %d bytes", c.maxBody)
|
||||
}
|
||||
|
||||
var cr chatResponse
|
||||
|
||||
@@ -17,6 +17,10 @@ import (
|
||||
// nothing about whether someone else's upload exists (ADR-033).
|
||||
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
|
||||
// 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
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
// Heartbeat stamps the upload as alive now. It does not touch progress_at: alive says
|
||||
// the service is still reporting, not that work is moving (ADR-035).
|
||||
func (s *Store) Heartbeat(ctx context.Context, userID, id string) error {
|
||||
// Heartbeat stamps the upload as alive at the given time. It does not touch progress_at:
|
||||
// alive says the service is still reporting, not that work is moving (ADR-035). The time
|
||||
// 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",
|
||||
`UPDATE uploaded_transcripts SET alive_at = NOW()
|
||||
WHERE id = $1 AND user_id = $2`)
|
||||
`UPDATE uploaded_transcripts SET alive_at = $3
|
||||
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 {
|
||||
return s.updateUpload(ctx, userID, id, "fail upload",
|
||||
`UPDATE uploaded_transcripts
|
||||
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))
|
||||
}
|
||||
|
||||
// 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,
|
||||
// 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).
|
||||
|
||||
@@ -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()
|
||||
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))
|
||||
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)
|
||||
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")
|
||||
}
|
||||
|
||||
@@ -99,7 +99,7 @@ func TestUploadProgress_ByAnotherUserChangesNothing(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
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, errBeat, store.ErrUploadNotFound)
|
||||
|
||||
@@ -66,7 +66,7 @@ const (
|
||||
defaultTranscribeModel = "iguana/kb-whisper"
|
||||
defaultTranscribeFallbackModel = "berget/kb-whisper"
|
||||
defaultTranscribeTimeout = 60 * time.Minute
|
||||
defaultTranscribeEngineTimeout = 25 * time.Minute
|
||||
defaultTranscribeEngineTimeout = 20 * time.Minute
|
||||
defaultMaxUploadedChars = 90000
|
||||
defaultUploadChatModel = "iguana/gemma4-26b"
|
||||
)
|
||||
|
||||
@@ -76,14 +76,14 @@ func TestLoad_ALocalUploadChatModelIsAccepted(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoad_TheEngineTimeoutDefaultsToTwentyFiveMinutesAndMustBePositive(t *testing.T) {
|
||||
func TestLoad_TheEngineTimeoutDefaultsToTwentyMinutesAndMustBePositive(t *testing.T) {
|
||||
clearUploadEnv(t)
|
||||
cfg, err := Load()
|
||||
if err != nil {
|
||||
t.Fatalf("Load: %v", err)
|
||||
}
|
||||
if cfg.TranscribeEngineTimeout != 25*time.Minute {
|
||||
t.Errorf("TranscribeEngineTimeout = %v, want 25m: a 90-minute recording takes about 8 minutes on kb-whisper", cfg.TranscribeEngineTimeout)
|
||||
if cfg.TranscribeEngineTimeout != 20*time.Minute {
|
||||
t.Errorf("TranscribeEngineTimeout = %v, want 20m: a 90-minute recording takes about 8 minutes on kb-whisper, and two wedged engines in sequence must leave the job time to reach a fallback", cfg.TranscribeEngineTimeout)
|
||||
}
|
||||
|
||||
setEnv(t, map[string]string{"TAPIR_TRANSCRIBE_ENGINE_TIMEOUT": "7m"})
|
||||
|
||||
@@ -203,6 +203,10 @@ type ProgressStep struct {
|
||||
FinishedAt time.Time `json:"finished_at,omitzero"`
|
||||
Done int `json:"done,omitempty"`
|
||||
Total int `json:"total,omitempty"`
|
||||
// Note is a short plain remark about the phase that the reader should see even though
|
||||
// it did not fail, for example that the audio is much shorter than the file claimed.
|
||||
// Durations and counts only: never transcript text, a name or a path.
|
||||
Note string `json:"note,omitempty"`
|
||||
}
|
||||
|
||||
// UploadStatus is the state of an uploaded-audio transcription job.
|
||||
@@ -233,10 +237,6 @@ const (
|
||||
// UploadErrBusy: too many recordings were already waiting, so this one was not
|
||||
// queued. The user's to retry later (ADR-035 review).
|
||||
UploadErrBusy UploadErrorKind = "queue_full"
|
||||
// UploadErrIncomplete: the audio, once converted, ends far earlier than the file
|
||||
// claimed: a truncated recording that would otherwise give a partial transcript with
|
||||
// no warning.
|
||||
UploadErrIncomplete UploadErrorKind = "audio_incomplete"
|
||||
)
|
||||
|
||||
// UploadedTranscript is a user's PRIVATE transcript of audio they uploaded. It is
|
||||
|
||||
@@ -17,13 +17,20 @@ import (
|
||||
// HTTPStatuser is implemented by errors that are a non-2xx answer from a service.
|
||||
type HTTPStatuser interface{ HTTPStatus() int }
|
||||
|
||||
// Of is the kind of err: "timeout", "canceled", "http_<code>", "network", or "error".
|
||||
// SQLStater is implemented by database errors that carry a SQLSTATE (pgx's PgError does).
|
||||
// The five-character code is a fixed vocabulary; the error's message is not: Postgres quotes
|
||||
// the offending value, which for a transcript row is transcript text.
|
||||
type SQLStater interface{ SQLState() string }
|
||||
|
||||
// Of is the kind of err: "timeout", "canceled", "http_<code>", "sql_<state>", "network",
|
||||
// or "error".
|
||||
// It is empty for nil.
|
||||
func Of(err error) string {
|
||||
if err == nil {
|
||||
return ""
|
||||
}
|
||||
var hs HTTPStatuser
|
||||
var ss SQLStater
|
||||
var ne net.Error
|
||||
switch {
|
||||
case errors.Is(err, context.DeadlineExceeded):
|
||||
@@ -32,6 +39,8 @@ func Of(err error) string {
|
||||
return "canceled"
|
||||
case errors.As(err, &hs):
|
||||
return fmt.Sprintf("http_%d", hs.HTTPStatus())
|
||||
case errors.As(err, &ss) && ss.SQLState() != "":
|
||||
return "sql_" + ss.SQLState()
|
||||
case errors.As(err, &ne):
|
||||
if ne.Timeout() {
|
||||
return "timeout"
|
||||
|
||||
@@ -54,6 +54,24 @@ func TestOf_ReducesAnErrorToAKindAndNeverItsText(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
type sqlErr struct{}
|
||||
|
||||
func (sqlErr) Error() string {
|
||||
return `ERROR: invalid input syntax for type json: "Anders ansvarar för virket"`
|
||||
}
|
||||
func (sqlErr) SQLState() string { return "22P02" }
|
||||
|
||||
func TestOf_ADatabaseErrorIsReducedToItsSQLStateNeverItsMessage(t *testing.T) {
|
||||
// A Postgres error can quote the offending value, which for a transcript row is
|
||||
// transcript text. The five-character SQLSTATE is a fixed vocabulary (fourth review).
|
||||
err := fmt.Errorf("store: complete upload: %w", sqlErr{})
|
||||
|
||||
got := errkind.Of(err)
|
||||
|
||||
assert.Equal(t, "sql_22P02", got)
|
||||
assert.NotContains(t, got, "Anders")
|
||||
}
|
||||
|
||||
func TestOf_AKindCarriesNoWordFromTheError(t *testing.T) {
|
||||
err := fmt.Errorf("convert /spool/upload-555: %w", errors.New("Anders ansvarar för virket"))
|
||||
|
||||
|
||||
@@ -37,11 +37,12 @@ type Reporter interface {
|
||||
// is for people, so a failed write is logged and never stops the job.
|
||||
type Sink interface {
|
||||
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 (
|
||||
defaultMinSave = 2 * time.Second
|
||||
defaultSaveTimeout = 5 * time.Second
|
||||
logAdvanceEach = time.Minute
|
||||
)
|
||||
|
||||
@@ -55,6 +56,14 @@ func WithClock(now func() time.Time) Option { return func(t *Tracker) { t.now =
|
||||
// Done, Fail and Finish always save.
|
||||
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.
|
||||
type Tracker struct {
|
||||
ctx context.Context
|
||||
@@ -63,6 +72,8 @@ type Tracker struct {
|
||||
now func() time.Time
|
||||
minSave time.Duration
|
||||
|
||||
saveTimeout time.Duration
|
||||
|
||||
mu sync.Mutex
|
||||
steps []domain.ProgressStep
|
||||
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
|
||||
// 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 {
|
||||
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 {
|
||||
o(t)
|
||||
}
|
||||
@@ -186,6 +197,20 @@ func (t *Tracker) Finish() {
|
||||
t.save()
|
||||
}
|
||||
|
||||
// Annotate attaches a short plain note to a phase and saves it at once. It is for a
|
||||
// remark the reader should see although nothing failed. Durations and counts only: a note
|
||||
// is shown on a page about a private recording.
|
||||
func (t *Tracker) Annotate(key, note string) {
|
||||
t.mu.Lock()
|
||||
defer t.mu.Unlock()
|
||||
s := t.find(key)
|
||||
if s == nil {
|
||||
return
|
||||
}
|
||||
s.Note = note
|
||||
t.save()
|
||||
}
|
||||
|
||||
// Heartbeat stamps the job's row every beatEvery and logs which phase is still running
|
||||
// every stillEvery, until ctx ends. It runs on its own, independent of any engine call:
|
||||
// a heartbeat that stopped means the process is gone, not that a request is slow.
|
||||
@@ -198,7 +223,10 @@ func (t *Tracker) Heartbeat(ctx context.Context, beatEvery, stillEvery time.Dura
|
||||
case <-ctx.Done():
|
||||
return
|
||||
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)
|
||||
}
|
||||
case <-still.C:
|
||||
@@ -240,7 +268,9 @@ func (t *Tracker) find(key string) *domain.ProgressStep {
|
||||
// dropped: progress is cosmetic and must never fail the job it describes.
|
||||
func (t *Tracker) save() {
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -21,6 +21,7 @@ type fakeSink struct {
|
||||
saves [][]domain.ProgressStep
|
||||
at []time.Time
|
||||
beats int
|
||||
beatAt []time.Time
|
||||
err error
|
||||
}
|
||||
|
||||
@@ -32,10 +33,11 @@ func (f *fakeSink) SaveProgress(_ context.Context, steps []domain.ProgressStep,
|
||||
return f.err
|
||||
}
|
||||
|
||||
func (f *fakeSink) Beat(context.Context) error {
|
||||
func (f *fakeSink) Beat(_ context.Context, at time.Time) error {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
f.beats++
|
||||
f.beatAt = append(f.beatAt, at)
|
||||
return f.err
|
||||
}
|
||||
|
||||
@@ -297,7 +299,7 @@ func (r *retainingSink) SaveProgress(_ context.Context, steps []domain.ProgressS
|
||||
r.got = append(r.got, steps)
|
||||
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) {
|
||||
sink := &retainingSink{}
|
||||
@@ -387,3 +389,33 @@ func TestWithAndFrom_ACarriedReporterIsTheOneReturned(t *testing.T) {
|
||||
|
||||
require.Equal(t, domain.StepRunning, stateOf(sink.last(t), "audio"))
|
||||
}
|
||||
|
||||
func TestTracker_AnnotateAttachesANoteToAStepAndSavesItAtOnce(t *testing.T) {
|
||||
tr, sink, _, _ := newTracker(t)
|
||||
tr.Start()
|
||||
tr.Begin("audio", 0)
|
||||
before := sink.saveCount()
|
||||
|
||||
tr.Annotate("audio", "20 min of the 79 the file claimed")
|
||||
|
||||
require.Equal(t, before+1, sink.saveCount())
|
||||
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") })
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -37,10 +37,10 @@ func TestReconcileSpots_AutoResolvedDisputesAreNotStored(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestReconcileSpots_ASpotCarriesTheWordsJustBeforeIt(t *testing.T) {
|
||||
out := merge(t, tr("vi skickar in ett vanligt meddelande till kassasystemet"), tr("vi skickar in ett KORT meddelande till kassasystemet"), always(reconcile.VerdictAlt))
|
||||
out := merge(t, tr("grannen målar ett vanligt staket mot gatan"), tr("grannen målar ett KORT staket mot gatan"), always(reconcile.VerdictAlt))
|
||||
|
||||
require.Len(t, out.Reconciliation.Contested, 1)
|
||||
assert.Equal(t, "vi skickar in ett", out.Reconciliation.Contested[0].Before)
|
||||
assert.Equal(t, "grannen målar ett", out.Reconciliation.Contested[0].Before)
|
||||
}
|
||||
|
||||
func TestReconcileSpots_TheContextIsAFewWordsNotTheWholeMeeting(t *testing.T) {
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
@@ -51,6 +52,53 @@ type Intake struct {
|
||||
sub Submitter
|
||||
limits Limits
|
||||
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.
|
||||
@@ -58,12 +106,47 @@ func NewIntake(s Store, p Prober, sub Submitter, l Limits, tmpDir string) *Intak
|
||||
return &Intake{store: s, prober: p, sub: sub, limits: l, tmpDir: tmpDir}
|
||||
}
|
||||
|
||||
// ErrClientRead marks an error that came from READING the client's body: a connection that
|
||||
// dropped, a body cut short. It is the sender's to retry, unlike a spool, probe or database
|
||||
// fault, which is ours. It is a marker on the error, not a sentinel that others may wrap:
|
||||
// io.ErrUnexpectedEOF alone is also what a database drop looks like.
|
||||
var ErrClientRead = errors.New("uploads: client body read failed")
|
||||
|
||||
// clientReader remembers the last non-EOF error its source returned, so Accept can tell
|
||||
// a failure reading the client from a failure writing the spool file.
|
||||
type clientReader struct {
|
||||
r io.Reader
|
||||
err error
|
||||
}
|
||||
|
||||
func (c *clientReader) Read(p []byte) (int, error) {
|
||||
n, err := c.r.Read(p)
|
||||
if err != nil && !errors.Is(err, io.EOF) {
|
||||
c.err = err
|
||||
}
|
||||
return n, err
|
||||
}
|
||||
|
||||
// busyMessage is what a user is told when the queue is full.
|
||||
const busyMessage = "Too many recordings are waiting to be transcribed. Please try again in a few minutes."
|
||||
|
||||
// Accept spools src to disk (bounded by MaxBytes), probes it, validates it, records
|
||||
// a queued upload and submits the job. It returns the upload id. A file that fails
|
||||
// validation yields a *RejectError and creates NO row — rejection is synchronous
|
||||
// and leaves nothing behind. On success the temp file is handed to the job and the
|
||||
// worker deletes it; on every failure path Accept deletes it itself.
|
||||
func (in *Intake) Accept(ctx context.Context, userID, title string, src io.Reader) (string, error) {
|
||||
// Refuse before spooling when the queue has no room, if the submitter can say so: a full
|
||||
// queue otherwise cost up to the upload cap on disk, an ffprobe run and a failed row for
|
||||
// every refused attempt.
|
||||
if rc, ok := in.sub.(interface{ HasRoom(userID string) bool }); ok && !rc.HasRoom(userID) {
|
||||
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-*")
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("uploads: spool: %w", err)
|
||||
@@ -77,11 +160,15 @@ func (in *Intake) Accept(ctx context.Context, userID, title string, src io.Reade
|
||||
}()
|
||||
|
||||
// Read one byte past the cap so "exactly at the limit" and "over" are told apart.
|
||||
n, err := io.Copy(f, io.LimitReader(src, in.limits.MaxBytes+1))
|
||||
cr := &clientReader{r: io.LimitReader(src, in.limits.MaxBytes+1)}
|
||||
n, err := io.Copy(f, cr)
|
||||
if cerr := f.Close(); err == nil {
|
||||
err = cerr
|
||||
}
|
||||
if err != nil {
|
||||
if cr.err != nil && errors.Is(err, cr.err) {
|
||||
return "", fmt.Errorf("%w: %w", ErrClientRead, err)
|
||||
}
|
||||
return "", fmt.Errorf("uploads: receive: %w", err)
|
||||
}
|
||||
if n > in.limits.MaxBytes {
|
||||
@@ -114,7 +201,7 @@ func (in *Intake) Accept(ctx context.Context, userID, title string, src io.Reade
|
||||
if ferr := in.store.FailUpload(ctx, userID, id, domain.UploadErrBusy); ferr != nil {
|
||||
return "", fmt.Errorf("uploads: submit: %w (and failing the row: %v)", err, ferr)
|
||||
}
|
||||
return "", &RejectError{Reason: ReasonBusy, Message: "Too many recordings are waiting to be transcribed. Please try again in a few minutes."}
|
||||
return "", &RejectError{Reason: ReasonBusy, Message: busyMessage}
|
||||
}
|
||||
// A row that will never run must not sit queued: that is the stuck spinner.
|
||||
if ferr := in.store.FailUpload(ctx, userID, id, domain.UploadErrInterrupted); ferr != nil {
|
||||
|
||||
@@ -3,8 +3,11 @@ package uploads_test
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
@@ -29,3 +32,65 @@ func TestAccept_AFullQueueIsABusyRejectionNotAServerErrorAndLeavesNothingBehind(
|
||||
assert.Equal(t, domain.UploadErrBusy, r.st.failed["up-1"], "the row must not sit queued: that is the stuck spinner")
|
||||
assert.Empty(t, r.tmpFiles(t), "the audio is deleted")
|
||||
}
|
||||
|
||||
// roomSubmitter is a Submitter that can say, before anything is spooled, that there is no room.
|
||||
type roomSubmitter struct {
|
||||
fakeSubmitter
|
||||
room bool
|
||||
}
|
||||
|
||||
func (r *roomSubmitter) HasRoom(string) bool { return r.room }
|
||||
|
||||
func TestAccept_ANoRoomQueueIsRefusedBeforeAnythingIsSpooledProbedOrStored(t *testing.T) {
|
||||
// Checking only at Submit meant 200 MB was spooled and ffprobe run for every refused
|
||||
// attempt, and a failed row was left behind each time (second adversarial review).
|
||||
r := newRig(t)
|
||||
full := &roomSubmitter{room: false}
|
||||
in := uploads.NewIntake(r.st, r.pr, full, uploads.Limits{MaxBytes: 1024, MaxDuration: 90 * time.Minute}, r.tmp)
|
||||
|
||||
_, err := in.Accept(context.Background(), "user-1", "t", strings.NewReader("data"))
|
||||
|
||||
var rej *uploads.RejectError
|
||||
require.True(t, errors.As(err, &rej))
|
||||
assert.Equal(t, uploads.ReasonBusy, rej.Reason)
|
||||
assert.Empty(t, r.st.created, "no row for an upload that was never accepted")
|
||||
assert.Empty(t, r.pr.paths, "ffprobe was never run on it")
|
||||
assert.Empty(t, r.tmpFiles(t), "nothing was spooled")
|
||||
assert.Empty(t, full.jobs)
|
||||
}
|
||||
|
||||
func TestAccept_ASubmitterThatCannotSayStillWorksAsBefore(t *testing.T) {
|
||||
r := newRig(t)
|
||||
|
||||
_, err := r.in.Accept(context.Background(), "user-1", "t", strings.NewReader("data"))
|
||||
|
||||
require.NoError(t, err, "a Submitter without HasRoom is accepted: the check at Submit remains")
|
||||
}
|
||||
|
||||
// brokenBody is a client body that fails mid-read, like a connection that dropped.
|
||||
type brokenBody struct{ n int }
|
||||
|
||||
func (b *brokenBody) Read(p []byte) (int, error) {
|
||||
if b.n > 0 {
|
||||
b.n = 0
|
||||
return copy(p, "partial audio"), nil
|
||||
}
|
||||
return 0, io.ErrUnexpectedEOF
|
||||
}
|
||||
|
||||
func TestAccept_AnErrorReadingTheClientsBodyIsMarkedAsTheClientsAndAWrappedSubmitErrorIsNot(t *testing.T) {
|
||||
r := newRig(t)
|
||||
|
||||
_, err := r.in.Accept(context.Background(), "user-1", "t", &brokenBody{n: 1})
|
||||
|
||||
require.Error(t, err)
|
||||
assert.ErrorIs(t, err, uploads.ErrClientRead, "a dropped client body is the sender's to retry")
|
||||
assert.Empty(t, r.tmpFiles(t))
|
||||
|
||||
// A fault that is ours must NOT carry the marker, even if it wraps the same sentinel.
|
||||
r2 := newRig(t)
|
||||
r2.sub.err = fmt.Errorf("db: %w", io.ErrUnexpectedEOF)
|
||||
_, err = r2.in.Accept(context.Background(), "user-1", "t", strings.NewReader("data"))
|
||||
require.Error(t, err)
|
||||
assert.NotErrorIs(t, err, uploads.ErrClientRead)
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
+72
-28
@@ -47,7 +47,7 @@ type WorkerStore interface {
|
||||
CompleteUpload(ctx context.Context, userID, id string, t domain.Transcription) 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
|
||||
Heartbeat(ctx context.Context, userID, id string) error
|
||||
Heartbeat(ctx context.Context, userID, id string, at time.Time) error
|
||||
}
|
||||
|
||||
// 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)
|
||||
}
|
||||
|
||||
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
|
||||
// not a second deployable: it shares the web process's lifetime, which is why it
|
||||
@@ -130,6 +132,24 @@ func NewWorker(s WorkerStore, c Converter, t Transcriber, tmpDir string, timeout
|
||||
return w
|
||||
}
|
||||
|
||||
// HasRoom reports whether a job from this user would be accepted right now. It lets the
|
||||
// intake refuse BEFORE spooling up to the upload cap and running ffprobe; Submit still
|
||||
// enforces the same limits atomically, so a race costs at worst one refused submit.
|
||||
func (w *Worker) HasRoom(userID string) bool {
|
||||
w.mu.Lock()
|
||||
defer w.mu.Unlock()
|
||||
if w.stopped || len(w.queue) >= w.maxQueued {
|
||||
return false
|
||||
}
|
||||
mine := 0
|
||||
for _, q := range w.queue {
|
||||
if q.UserID == userID {
|
||||
mine++
|
||||
}
|
||||
}
|
||||
return mine < w.maxPerUser
|
||||
}
|
||||
|
||||
// Submit queues a job. It never blocks. It satisfies Submitter.
|
||||
func (w *Worker) Submit(j Job) error {
|
||||
w.mu.Lock()
|
||||
@@ -158,9 +178,10 @@ func (w *Worker) Submit(j Job) error {
|
||||
}
|
||||
|
||||
// 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
|
||||
// transcribing and are failed as interrupted by the start-up sweep
|
||||
// (store.FailInterruptedUploads): the audio is not retained, so they cannot resume.
|
||||
// still queued and refuses new jobs. The job running at shutdown is recorded interrupted
|
||||
// by Process; the rows of jobs still queued stay queued and are failed as interrupted by
|
||||
// the start-up sweep (store.FailInterruptedUploads): the audio is not retained, so they
|
||||
// cannot resume.
|
||||
func (w *Worker) Run(ctx context.Context) {
|
||||
defer w.stop()
|
||||
for {
|
||||
@@ -209,7 +230,7 @@ func (w *Worker) stop() {
|
||||
// Process runs one job: row -> transcribing, convert, transcribe, store. The
|
||||
// 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
|
||||
// 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
|
||||
// the page and logs it, and a heartbeat stamps the row on its own while the job runs
|
||||
@@ -224,7 +245,7 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
start := time.Now()
|
||||
|
||||
if err := w.store.StartTranscribing(ctx, job.UserID, job.UploadID); err != nil {
|
||||
log.Error("upload: start transcribing", "err", err)
|
||||
log.Error("upload: start transcribing", "kind", errkind.Of(err))
|
||||
w.fail(ctx, job, log, domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
@@ -251,7 +272,7 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
defer func() { stopBeat(); <-hbDone }() // no heartbeat outlives its job
|
||||
|
||||
// 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) {
|
||||
if ctx.Err() == nil {
|
||||
tr.FailRunning()
|
||||
@@ -261,7 +282,7 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
|
||||
wav, err := w.tempWAV()
|
||||
if err != nil {
|
||||
log.Error("upload: temp wav", "err", err)
|
||||
log.Error("upload: temp wav", "kind", errkind.Of(err))
|
||||
abort(domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
@@ -275,11 +296,18 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
abort(domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
if kind, bad := w.audioLengthProblem(wav, job.Duration); bad {
|
||||
abort(kind)
|
||||
log.Error("upload: converted audio length rejected", "kind", string(kind))
|
||||
got, tooLong := w.audioLengthProblem(wav)
|
||||
if tooLong {
|
||||
abort(domain.UploadErrTooLong)
|
||||
log.Error("upload: converted audio is over the limit", "audio_s", int(got.Seconds()), "limit_s", int(w.maxDuration.Seconds()))
|
||||
return
|
||||
}
|
||||
if w.maxDuration > 0 && job.Duration > 0 {
|
||||
if note := shortfallNote(got, job.Duration); note != "" {
|
||||
log.Warn("upload: audio is much shorter than the file claimed", "audio_s", int(got.Seconds()), "claimed_s", int(job.Duration.Seconds()))
|
||||
tr.Annotate("audio", note)
|
||||
}
|
||||
}
|
||||
tr.Done("audio")
|
||||
|
||||
if !planned {
|
||||
@@ -297,7 +325,7 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
|
||||
tr.Begin("save", 0)
|
||||
if err := w.store.CompleteUpload(ctx, job.UserID, job.UploadID, t); err != nil {
|
||||
log.Error("upload: store transcript", "err", err)
|
||||
log.Error("upload: store transcript", "kind", errkind.Of(err))
|
||||
abort(domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
@@ -307,14 +335,22 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
"segments", len(t.Segments), "elapsed", time.Since(start).Round(time.Second))
|
||||
}
|
||||
|
||||
// fail records a transcription failure — unless the PARENT context ended, which
|
||||
// is a shutdown, not a failure: the row is left for the start-up sweep.
|
||||
// interruptedWriteTimeout bounds the one write a dying process makes to record its job.
|
||||
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) {
|
||||
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 {
|
||||
log.Error("upload: record failure", "err", err)
|
||||
log.Error("upload: record failure", "kind", errkind.Of(err))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -343,21 +379,29 @@ const (
|
||||
)
|
||||
|
||||
// audioLengthProblem measures the converted WAV (16 kHz mono 16-bit PCM is 32,000 bytes a
|
||||
// second after a 44-byte header) against the limit and against what the file claimed.
|
||||
func (w *Worker) audioLengthProblem(wav string, claimed time.Duration) (domain.UploadErrorKind, bool) {
|
||||
// second after a 44-byte header) against the limit. It returns the real length and whether
|
||||
// that is over the limit by more than the tolerance.
|
||||
func (w *Worker) audioLengthProblem(wav string) (got time.Duration, tooLong bool) {
|
||||
if w.maxDuration <= 0 {
|
||||
return "", false
|
||||
return 0, false
|
||||
}
|
||||
info, err := os.Stat(wav)
|
||||
if err != nil {
|
||||
return "", false
|
||||
return 0, false
|
||||
}
|
||||
got := time.Duration(float64(max(info.Size()-44, 0)) / 32000 * float64(time.Second))
|
||||
switch {
|
||||
case got > w.maxDuration+tooLongSlack:
|
||||
return domain.UploadErrTooLong, true
|
||||
case float64(got) < incompleteBelow*float64(claimed) && claimed-got > incompleteShortfall:
|
||||
return domain.UploadErrIncomplete, true
|
||||
got = time.Duration(float64(max(info.Size()-44, 0)) / 32000 * float64(time.Second))
|
||||
return got, got > w.maxDuration+tooLongSlack
|
||||
}
|
||||
|
||||
// shortfallNote is a remark for the audio step when the converted audio is much shorter
|
||||
// than the file claimed, or "" when it is not. It is a NOTE and never a failure: a
|
||||
// container header can over-estimate a complete file (a variable-bitrate MP3 without a Xing
|
||||
// header claimed 79 minutes for 20 real ones), so refusing would turn away real
|
||||
// recordings, while saying nothing about a truncated one would hide a partial transcript.
|
||||
func shortfallNote(got, claimed time.Duration) string {
|
||||
if float64(got) >= incompleteBelow*float64(claimed) || claimed-got <= incompleteShortfall {
|
||||
return ""
|
||||
}
|
||||
return "", false
|
||||
return fmt.Sprintf("The audio is %s long; the file claimed %s. If the recording was cut off, upload the complete one.",
|
||||
FormatDuration(got), FormatDuration(claimed))
|
||||
}
|
||||
|
||||
@@ -69,15 +69,86 @@ func TestWorkerDuration_JustOverTheLimitWithinTheSlackStillTranscribes(t *testin
|
||||
assert.Contains(t, completed, "a", "ffmpeg rounds: 20 s over a 90-minute limit is not a lie")
|
||||
}
|
||||
|
||||
func TestWorkerDuration_AFileThatEndsFarEarlierThanItClaimedIsRefusedNotHalfTranscribed(t *testing.T) {
|
||||
func audioStepNote(r *wrig) string {
|
||||
for _, st := range r.st.lastSteps() {
|
||||
if st.Key == "audio" {
|
||||
return st.Note
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func TestWorkerDuration_AFileThatEndsFarEarlierThanItClaimedIsStillTranscribedAndSaysSo(t *testing.T) {
|
||||
// A container header can over-estimate a complete file (VBR MP3 without a Xing header
|
||||
// claimed 79 minutes for 20 real ones). Refusing it was wrong; staying silent about a
|
||||
// truncated one is wrong too. So it is transcribed, and the audio step carries a note.
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, 20*60, 90*time.Minute)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, 60*time.Minute))
|
||||
w.Process(context.Background(), jobWith(r, t, 79*time.Minute))
|
||||
|
||||
completed, failed := r.st.snapshot()
|
||||
assert.Empty(t, failed, "a complete recording with a bad header estimate must not be refused")
|
||||
assert.Contains(t, completed, "a")
|
||||
note := audioStepNote(r)
|
||||
assert.Contains(t, note, "20 min", "what was really transcribed")
|
||||
assert.Contains(t, note, "1 h 19 min", "what the file claimed")
|
||||
assert.Contains(t, note, "cut off", "and what to do if that is not a surprise")
|
||||
}
|
||||
|
||||
func TestWorkerDuration_TheShortfallNoteIsPinnedAtEightyPercentAndTwoMinutes(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
claimed time.Duration
|
||||
wav float64
|
||||
noted bool
|
||||
}{
|
||||
{"79 percent of an hour", 60 * time.Minute, 0.79 * 3600, true},
|
||||
{"exactly 80 percent of an hour", 60 * time.Minute, 2880, false},
|
||||
{"one second under 80 percent of an hour", 60 * time.Minute, 2879, true},
|
||||
{"81 percent of an hour", 60 * time.Minute, 0.81 * 3600, false},
|
||||
{"exactly two minutes short and under 80 percent", 6 * time.Minute, 4 * 60, false},
|
||||
{"two minutes and a second short and under 80 percent", 6 * time.Minute, 4*60 - 1, true},
|
||||
{"unknown claimed length", 0, 20 * 60, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, tt.wav, 90*time.Minute)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, tt.claimed))
|
||||
|
||||
assert.Equal(t, tt.noted, audioStepNote(r) != "")
|
||||
completed, _ := r.st.snapshot()
|
||||
assert.Contains(t, completed, "a", "a note never stops the job")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkerDuration_TheTooLongToleranceIsExactlyThirtySeconds(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
wav float64
|
||||
fail bool
|
||||
}{
|
||||
{"at the limit plus 30 s", 90*60 + 30, false},
|
||||
{"at the limit plus 31 s", 90*60 + 31, true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, tt.wav, 90*time.Minute)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, 90*time.Minute))
|
||||
|
||||
_, failed := r.st.snapshot()
|
||||
assert.Equal(t, domain.UploadErrIncomplete, failed["a"], "a recording truncated at a third must not return a third of a transcript with no warning")
|
||||
assert.Empty(t, r.tr.paths)
|
||||
if tt.fail {
|
||||
assert.Equal(t, domain.UploadErrTooLong, failed["a"])
|
||||
} else {
|
||||
assert.Empty(t, failed)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkerDuration_AnEstimateThatIsMerelyALittleOffIsNotTruncation(t *testing.T) {
|
||||
|
||||
@@ -197,3 +197,19 @@ func TestWorkerProgress_AFailureLogsAKindNeverTheErrorTextOrThePath(t *testing.T
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkerProgress_AStoreFailureLogsItsSQLStateNeverTheValueItQuotes(t *testing.T) {
|
||||
// A Postgres error can quote the offending value; for a transcript row that is the
|
||||
// transcript (fourth adversarial review). The log gets the SQLSTATE.
|
||||
r := newWRig(t, time.Minute)
|
||||
r.st.completeErr = dbErr{}
|
||||
buf := &syncLog{}
|
||||
w := newProgressWorker(t, r, r.tr, uploads.WithLogger(slog.New(slog.NewTextHandler(buf, nil))))
|
||||
|
||||
w.Process(context.Background(), r.spool(t, "a"))
|
||||
|
||||
out := buf.String()
|
||||
assert.NotContains(t, out, "zebrakaka")
|
||||
assert.Contains(t, out, "upload: store transcript")
|
||||
assert.Contains(t, out, "kind=sql_22P02")
|
||||
}
|
||||
|
||||
@@ -78,3 +78,31 @@ func TestWorkerSubmit_AJobThatHasLeftTheQueueFreesItsPlace(t *testing.T) {
|
||||
|
||||
assert.NoError(t, w.Submit(r.spool(t, "second")), "the cap counts jobs waiting, not jobs ever submitted")
|
||||
}
|
||||
|
||||
func TestWorkerHasRoom_SaysWhetherASubmitWouldBeAccepted(t *testing.T) {
|
||||
w := queueWorker(t, 3, 2)
|
||||
assert.True(t, w.HasRoom("alice"))
|
||||
require.NoError(t, submit(w, "alice", 1))
|
||||
require.NoError(t, submit(w, "alice", 2))
|
||||
|
||||
assert.False(t, w.HasRoom("alice"), "alice is at her share")
|
||||
assert.True(t, w.HasRoom("bob"), "bob is not")
|
||||
require.NoError(t, submit(w, "bob", 1))
|
||||
assert.False(t, w.HasRoom("carol"), "the queue as a whole is at its cap")
|
||||
}
|
||||
|
||||
func TestWorkerHasRoom_AgreesWithSubmitAtEveryStepAndIsFalseOnceStopped(t *testing.T) {
|
||||
w := queueWorker(t, 2, 1)
|
||||
for i, user := range []string{"a", "a", "b", "c"} {
|
||||
room := w.HasRoom(user)
|
||||
err := submit(w, user, i)
|
||||
assert.Equal(t, room, err == nil, "step %d (%s): HasRoom=%v but Submit returned %v", i, user, room, err)
|
||||
}
|
||||
|
||||
r := newWRig(t, time.Minute)
|
||||
stopped := uploads.NewWorker(r.st, r.conv, r.tr, r.tmp, time.Minute)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
stopped.Run(ctx)
|
||||
assert.False(t, stopped.HasRoom("a"), "a worker that has stopped has no room")
|
||||
}
|
||||
|
||||
@@ -53,7 +53,10 @@ func (s *wStore) CompleteUpload(_ context.Context, _, id string, t domain.Transc
|
||||
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()
|
||||
defer s.mu.Unlock()
|
||||
s.failed[id] = k
|
||||
@@ -67,7 +70,7 @@ func (s *wStore) SaveProgress(_ context.Context, _, _ string, steps []domain.Pro
|
||||
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)
|
||||
s.mu.Lock()
|
||||
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.tr.block = make(chan struct{})
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
@@ -255,8 +260,8 @@ func TestWorkerProcess_ShutdownMidJobLeavesTheRowForTheStartupSweep(t *testing.T
|
||||
<-done
|
||||
|
||||
_, f := r.st.snapshot()
|
||||
assert.Empty(t, f, "a shutdown is not a transcription failure: the row stays for FailInterruptedUploads to mark interrupted")
|
||||
assert.Empty(t, r.leftovers(t), "but the audio is still deleted")
|
||||
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), "and the audio is still deleted")
|
||||
}
|
||||
|
||||
func TestWorkerProcess_AJobThatOutlivesItsTimeoutIsFailedNotStuck(t *testing.T) {
|
||||
@@ -324,3 +329,11 @@ func TestWorkerRun_StoppingDeletesQueuedAudioAndRefusesNewJobs(t *testing.T) {
|
||||
assert.Empty(t, r.leftovers(t), "audio of queued jobs is deleted on stop: the rows are swept to interrupted at next start")
|
||||
assert.ErrorIs(t, r.w.Submit(r.spool(t, "c")), uploads.ErrStopped)
|
||||
}
|
||||
|
||||
// dbErr is an error shaped like pgx's: a message that quotes a value, and an SQLSTATE.
|
||||
type dbErr struct{}
|
||||
|
||||
func (dbErr) Error() string {
|
||||
return `ERROR: invalid input syntax for type json: "zebrakaka secret words"`
|
||||
}
|
||||
func (dbErr) SQLState() string { return "22P02" }
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -59,6 +59,9 @@ type Store interface {
|
||||
// non-owner's id is ErrUploadNotFound.
|
||||
GetUpload(ctx context.Context, userID, id 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.
|
||||
// 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("GET /uploads/{id}", a.handleUploadDetail)
|
||||
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
|
||||
// when one is wired.
|
||||
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
|
||||
// the registration gate (you must be able to reach it before you have a user).
|
||||
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
|
||||
@@ -304,7 +313,7 @@ func (a *App) handleList(w http.ResponseWriter, r *http.Request) {
|
||||
a.render(w, r, summaryList(buckets, hasConnected, autoSummarize))
|
||||
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).
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
package oidc_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/web/oidc"
|
||||
)
|
||||
|
||||
func testConfig(issuer string) oidc.Config {
|
||||
return oidc.Config{
|
||||
Issuer: issuer,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: "test-client-secret",
|
||||
RedirectURL: "http://tapir.test/auth/callback",
|
||||
SessionSecret: "test-session-secret-please-change",
|
||||
}
|
||||
}
|
||||
|
||||
// #59: a discovery 503 at start-up is retried instead of failing New.
|
||||
func TestNewRetriesDiscoveryAfter503UntilIssuerAnswers(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 2
|
||||
|
||||
auth, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(5, time.Millisecond))
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, auth)
|
||||
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||
}
|
||||
|
||||
func TestNewReturnsDiscoveryErrorAfterAttemptsExhausted(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 1000
|
||||
|
||||
_, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(3, time.Millisecond))
|
||||
|
||||
require.ErrorContains(t, err, "503")
|
||||
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||
}
|
||||
|
||||
func TestNewStopsDiscoveryBackoffWhenContextCancelled(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 1000
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
|
||||
defer cancel()
|
||||
|
||||
start := time.Now()
|
||||
_, err := oidc.New(ctx, testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(5, time.Hour))
|
||||
|
||||
require.True(t, errors.Is(err, context.DeadlineExceeded), "err = %v", err)
|
||||
require.Less(t, time.Since(start), 5*time.Second)
|
||||
require.Equal(t, int32(1), f.discoveryHits.Load())
|
||||
}
|
||||
+66
-16
@@ -57,6 +57,10 @@ const (
|
||||
pendingTTL = 10 * time.Minute
|
||||
sessionCookie = "tapir_session"
|
||||
loginPath = "/auth/login"
|
||||
|
||||
// Discovery retry (#59): 5 tries, 2+4+8+16 s back-off ≈ 30 s, then fail.
|
||||
defaultDiscoveryAttempts = 5
|
||||
defaultDiscoveryDelay = 2 * time.Second
|
||||
)
|
||||
|
||||
// DexAuth implements web.Auth against Dex.
|
||||
@@ -70,6 +74,9 @@ type DexAuth struct {
|
||||
sessionTTL time.Duration
|
||||
now func() time.Time
|
||||
insecure bool // tests only: drop the Secure attribute so cookies flow over http
|
||||
|
||||
discoveryAttempts int
|
||||
discoveryDelay time.Duration
|
||||
}
|
||||
|
||||
var _ web.Auth = (*DexAuth)(nil)
|
||||
@@ -96,14 +103,55 @@ func WithSessionTTL(ttl time.Duration) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// WithDiscoveryRetry overrides the discovery attempt count and the initial
|
||||
// back-off delay (doubled after each failed attempt).
|
||||
func WithDiscoveryRetry(attempts int, delay time.Duration) Option {
|
||||
return func(d *DexAuth) {
|
||||
if attempts > 0 {
|
||||
d.discoveryAttempts = attempts
|
||||
}
|
||||
if delay > 0 {
|
||||
d.discoveryDelay = delay
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// WithInsecureCookies drops the Secure cookie attribute. TEST ONLY — it lets
|
||||
// the session cookie travel over plain http; never use it in production.
|
||||
func WithInsecureCookies() Option {
|
||||
return func(d *DexAuth) { d.insecure = true }
|
||||
}
|
||||
|
||||
// New discovers the issuer and returns a configured DexAuth. ctx bounds the
|
||||
// discovery request only.
|
||||
// discover runs OIDC discovery up to attempts times, doubling delay between
|
||||
// tries (#59: an issuer briefly unavailable behind its proxy answered 503 and
|
||||
// serve exited). A cancelled ctx ends the back-off immediately with ctx's error.
|
||||
func discover(ctx context.Context, issuer string, attempts int, delay time.Duration) (*oidc.Provider, error) {
|
||||
var lastErr error
|
||||
for i := range attempts {
|
||||
if i > 0 {
|
||||
t := time.NewTimer(delay)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
t.Stop()
|
||||
return nil, fmt.Errorf("after %d attempts (last: %v): %w", i, lastErr, ctx.Err())
|
||||
case <-t.C:
|
||||
}
|
||||
delay *= 2
|
||||
}
|
||||
p, err := oidc.NewProvider(ctx, issuer)
|
||||
if err == nil {
|
||||
return p, nil
|
||||
}
|
||||
lastErr = err
|
||||
if ctx.Err() != nil {
|
||||
return nil, fmt.Errorf("after %d attempts: %w", i+1, ctx.Err())
|
||||
}
|
||||
}
|
||||
return nil, fmt.Errorf("after %d attempts: %w", attempts, lastErr)
|
||||
}
|
||||
|
||||
// New discovers the issuer and returns a configured DexAuth. ctx bounds discovery,
|
||||
// including its retry back-off.
|
||||
func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||
for name, val := range map[string]string{
|
||||
"issuer": cfg.Issuer,
|
||||
@@ -117,28 +165,30 @@ func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||
}
|
||||
}
|
||||
|
||||
provider, err := oidc.NewProvider(ctx, cfg.Issuer)
|
||||
d := &DexAuth{
|
||||
cfg: cfg,
|
||||
pending: newPendingStore(),
|
||||
secret: []byte(cfg.SessionSecret),
|
||||
sessionTTL: defaultSessionTTL,
|
||||
now: time.Now,
|
||||
discoveryAttempts: defaultDiscoveryAttempts,
|
||||
discoveryDelay: defaultDiscoveryDelay,
|
||||
}
|
||||
for _, o := range opts {
|
||||
o(d)
|
||||
}
|
||||
|
||||
provider, err := discover(ctx, cfg.Issuer, d.discoveryAttempts, d.discoveryDelay)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("oidc: discover issuer: %w", err)
|
||||
}
|
||||
|
||||
d := &DexAuth{
|
||||
cfg: cfg,
|
||||
verifier: provider.Verifier(&oidc.Config{ClientID: cfg.ClientID}),
|
||||
oauth: &oauth2.Config{
|
||||
d.verifier = provider.Verifier(&oidc.Config{ClientID: cfg.ClientID})
|
||||
d.oauth = &oauth2.Config{
|
||||
ClientID: cfg.ClientID,
|
||||
ClientSecret: cfg.ClientSecret,
|
||||
Endpoint: provider.Endpoint(),
|
||||
RedirectURL: cfg.RedirectURL,
|
||||
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
|
||||
},
|
||||
pending: newPendingStore(),
|
||||
secret: []byte(cfg.SessionSecret),
|
||||
sessionTTL: defaultSessionTTL,
|
||||
now: time.Now,
|
||||
}
|
||||
for _, o := range opts {
|
||||
o(d)
|
||||
}
|
||||
return d, nil
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -34,6 +35,11 @@ type fakeIssuer struct {
|
||||
sub string
|
||||
email string
|
||||
nonce string
|
||||
|
||||
// discoveryHits counts requests to the discovery document; the first
|
||||
// unavailableFor of them answer 503 (issuer briefly down behind its proxy).
|
||||
discoveryHits atomic.Int32
|
||||
unavailableFor int32
|
||||
}
|
||||
|
||||
const testKID = "test-key"
|
||||
@@ -47,6 +53,10 @@ func newFakeIssuer(t *testing.T) *fakeIssuer {
|
||||
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) {
|
||||
if f.discoveryHits.Add(1) <= f.unavailableFor {
|
||||
http.Error(w, "no available server", http.StatusServiceUnavailable)
|
||||
return
|
||||
}
|
||||
writeJSON(t, w, map[string]any{
|
||||
"issuer": f.server.URL,
|
||||
"authorization_endpoint": f.server.URL + "/authorize",
|
||||
|
||||
@@ -66,7 +66,7 @@ func TestParseYouTubeVideoID(t *testing.T) {
|
||||
func TestListPageShowsPasteFormOnlyWhenConnected(t *testing.T) {
|
||||
render := func(connected bool) string {
|
||||
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)
|
||||
}
|
||||
return buf.String()
|
||||
|
||||
+106
-3
@@ -18,6 +18,7 @@ import (
|
||||
|
||||
"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/config"
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||
@@ -95,6 +96,9 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
if err != nil {
|
||||
var tooBig *http.MaxBytesError
|
||||
if a.abandoned(w, r) {
|
||||
return
|
||||
}
|
||||
if !errors.As(err, &tooBig) {
|
||||
// A malformed multipart body (a header with a control byte, a truncated
|
||||
// part) is the sender's to fix: a 400, not a server error, and the error
|
||||
@@ -110,6 +114,9 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) {
|
||||
case "title":
|
||||
b, err := io.ReadAll(io.LimitReader(part, maxTitleRunes*utf8.UTFMax))
|
||||
if err != nil {
|
||||
if a.abandoned(w, r) {
|
||||
return
|
||||
}
|
||||
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "That upload could not be read. Please try again.")
|
||||
return
|
||||
}
|
||||
@@ -130,10 +137,27 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) {
|
||||
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "Choose an audio file to upload.")
|
||||
}
|
||||
|
||||
// abandoned reports, logs at INFO and answers 499, because the client has already gone: its request
|
||||
// context is cancelled, so there is nobody to answer and rendering a page would run its
|
||||
// database read on a dead context and log a false ERROR. A hang-up mid-upload is routine.
|
||||
func (a *App) abandoned(w http.ResponseWriter, r *http.Request) bool {
|
||||
if r.Context().Err() == nil {
|
||||
return false
|
||||
}
|
||||
a.logger().Info("upload abandoned", "reason", "client_gone")
|
||||
// 499 (the "client closed request" convention): a client that half-closed after sending
|
||||
// can still read, and an unwritten status would reach it as a false, empty 200.
|
||||
w.WriteHeader(499)
|
||||
return true
|
||||
}
|
||||
|
||||
// uploadFailure maps an Accept (or multipart) error to a response: a validation
|
||||
// rejection is the user's to fix (422, or 413 for size) and carries its own
|
||||
// message; anything else is ours.
|
||||
func (a *App) uploadFailure(w http.ResponseWriter, r *http.Request, userID string, err error) {
|
||||
if a.abandoned(w, r) {
|
||||
return
|
||||
}
|
||||
var rej *uploads.RejectError
|
||||
var tooBig *http.MaxBytesError
|
||||
switch {
|
||||
@@ -152,6 +176,11 @@ func (a *App) uploadFailure(w http.ResponseWriter, r *http.Request, userID strin
|
||||
a.logger().Info("upload rejected", "reason", string(uploads.ReasonTooLarge))
|
||||
a.renderUploadsPage(w, r, userID, http.StatusRequestEntityTooLarge,
|
||||
"This file is larger than the upload limit.")
|
||||
case errors.Is(err, uploads.ErrClientRead):
|
||||
// An error reading the CLIENT's body: theirs to retry, not a server fault. Only that
|
||||
// marker counts: io.ErrUnexpectedEOF alone is also what a database drop looks like.
|
||||
a.logger().Info("upload rejected", "reason", "truncated")
|
||||
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "The upload was cut off before it finished. Please try again.")
|
||||
default:
|
||||
// Not err: it can carry the file's name, a spool path or a database value.
|
||||
a.logger().Error("upload failed", "kind", errkind.Of(err))
|
||||
@@ -181,6 +210,32 @@ func (a *App) handleUploadStatus(w http.ResponseWriter, r *http.Request) {
|
||||
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) {
|
||||
userID, ok := a.currentUserID(w, r)
|
||||
if !ok {
|
||||
@@ -215,9 +270,56 @@ func titleFromFilename(name string) string {
|
||||
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
|
||||
// recording that is shorter than its file claimed, so a record that has one is shown open.
|
||||
func hasNote(steps []domain.ProgressStep) bool {
|
||||
for _, s := range steps {
|
||||
if s.Note != "" {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// uuidShape is the form of an upload id. Anything else cannot be one, so it is a 404.
|
||||
var uuidShape = regexp.MustCompile(`^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$`)
|
||||
|
||||
// isBidiControl reports the invisible characters that change text direction. Emoji joiners
|
||||
// (ZWJ, ZWNJ) and variation selectors are not among them and are kept.
|
||||
func isBidiControl(r rune) bool {
|
||||
return r >= 0x202A && r <= 0x202E || r >= 0x2066 && r <= 0x2069 || r == 0x200E || r == 0x200F || r == 0x061C
|
||||
}
|
||||
|
||||
// cleanTitle makes a title safe to store and show: invalid UTF-8 dropped, control
|
||||
// characters (NUL, newlines, escapes) turned into spaces, runs of space collapsed, trimmed
|
||||
// and clipped. A title is cosmetic; bad bytes in it must never fail an upload, and Postgres
|
||||
@@ -225,7 +327,10 @@ var uuidShape = regexp.MustCompile(`^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4
|
||||
func cleanTitle(s string) string {
|
||||
s = strings.ToValidUTF8(s, "")
|
||||
s = strings.Map(func(r rune) rune {
|
||||
if unicode.IsControl(r) {
|
||||
switch {
|
||||
case isBidiControl(r):
|
||||
return -1 // dropped: U+202E would reverse the rest of the title on its owner's page
|
||||
case unicode.IsControl(r):
|
||||
return ' '
|
||||
}
|
||||
return r
|
||||
@@ -250,8 +355,6 @@ func uploadFailureMessage(k domain.UploadErrorKind) string {
|
||||
return "Too many recordings were waiting to be transcribed when this one was uploaded. Please upload it again in a few minutes."
|
||||
case domain.UploadErrTooLong:
|
||||
return "This recording is longer than the limit once its audio is read (the file's header said otherwise). Please upload a shorter recording."
|
||||
case domain.UploadErrIncomplete:
|
||||
return "This recording ends much earlier than the file says it should; it may be cut off or damaged. Please upload the complete recording."
|
||||
case domain.UploadErrTranscriptionFailed:
|
||||
return "Transcription failed on every available service. Please upload it again later."
|
||||
default:
|
||||
|
||||
@@ -66,9 +66,15 @@ templ uploadStatus(u domain.UploadedTranscript, chatEnabled bool, now time.Time)
|
||||
Transcribed { uploads.FormatDuration(time.Duration(u.AudioSeconds * float64(time.Second))) }
|
||||
of audio by <span class="chip">{ u.Endpoint }</span>.
|
||||
</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)
|
||||
if len(u.Progress) > 0 {
|
||||
<details class="what-was-done">
|
||||
<details class="what-was-done" if hasNote(u.Progress) { open }>
|
||||
<summary>What was done</summary>
|
||||
@progressSteps(u.Progress, now, false)
|
||||
</details>
|
||||
@@ -80,6 +86,7 @@ templ uploadStatus(u domain.UploadedTranscript, chatEnabled bool, now time.Time)
|
||||
<summary>Transcript</summary>
|
||||
<pre class="transcript">{ u.Content }</pre>
|
||||
</details>
|
||||
@deleteRecording(u.ID)
|
||||
</div>
|
||||
case domain.UploadFailed:
|
||||
<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)
|
||||
</details>
|
||||
}
|
||||
@deleteRecording(u.ID)
|
||||
</div>
|
||||
default:
|
||||
<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:
|
||||
// the same chat panel as a video's (ADR-027), scoped to this recording.
|
||||
templ UploadChatPage(u domain.UploadedTranscript, v chatView) {
|
||||
@@ -218,6 +240,9 @@ templ progressSteps(steps []domain.ProgressStep, now time.Time, live bool) {
|
||||
if t := stepTime(s, now); t != "" {
|
||||
<span class="step-time">{ t }</span>
|
||||
}
|
||||
if s.Note != "" {
|
||||
<span class="step-note">{ s.Note }</span>
|
||||
}
|
||||
if s.State == domain.StepRunning && s.Total > 0 {
|
||||
<progress class="step-bar" value={ strconv.Itoa(s.Done) } max={ strconv.Itoa(s.Total) }></progress>
|
||||
<span class="step-count">{ strconv.Itoa(s.Done) } of { strconv.Itoa(s.Total) }</span>
|
||||
|
||||
@@ -2,6 +2,7 @@ package web_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"testing"
|
||||
@@ -60,3 +61,33 @@ func TestUploadPost_AMalformedMultipartHeaderIsA400AndItsTextIsNotLogged(t *test
|
||||
assert.Empty(t, sub.jobs)
|
||||
assert.NotContains(t, buf.String(), "zebrakaka", "a file name is private content")
|
||||
}
|
||||
|
||||
func TestUploadPost_EveryBidirectionalControlCharacterIsStrippedFromATitle(t *testing.T) {
|
||||
// U+202E reverses the text that follows it, so a title could render reversed on its
|
||||
// owner's own page (second adversarial review). Every one of the twelve is pinned.
|
||||
controls := []rune{0x202A, 0x202B, 0x202C, 0x202D, 0x202E, 0x2066, 0x2067, 0x2068, 0x2069, 0x200E, 0x200F, 0x061C}
|
||||
for _, c := range controls {
|
||||
t.Run(fmt.Sprintf("U+%04X", c), func(t *testing.T) {
|
||||
app, sub := newUploadApp(t, audioOK, 200<<20)
|
||||
|
||||
rec := do(t, app, uploadReq(t, "Sprint"+string(c)+"review", "a.m4a", []byte("x")))
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, rec.Code)
|
||||
var title string
|
||||
require.NoError(t, rawPool(t).QueryRow(context.Background(), `SELECT title FROM uploaded_transcripts WHERE id = $1`, sub.jobs[0].UploadID).Scan(&title))
|
||||
assert.Equal(t, "Sprintreview", title, "dropped, not replaced by a space: a space would leave a visible gap")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadPost_SwedishLettersAndEmojiSurviveTheTitleCleaning(t *testing.T) {
|
||||
app, sub := newUploadApp(t, audioOK, 200<<20)
|
||||
|
||||
// Includes a ZWJ sequence (woman + ZWJ + laptop) and a ZWNJ: joiners are not direction controls.
|
||||
rec := do(t, app, uploadReq(t, "Möte åäö 🎙️ 👩\u200d💻 a\u200cb", "a.m4a", []byte("x")))
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, rec.Code)
|
||||
var title string
|
||||
require.NoError(t, rawPool(t).QueryRow(context.Background(), `SELECT title FROM uploaded_transcripts WHERE id = $1`, sub.jobs[0].UploadID).Scan(&title))
|
||||
assert.Equal(t, "Möte åäö 🎙️ 👩\u200d💻 a\u200cb", title, "joiners and variation selectors belong to the text")
|
||||
}
|
||||
|
||||
@@ -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"`))
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,124 @@
|
||||
package web_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// A client that hangs up in the middle of an upload is routine, not a server fault. The
|
||||
// in-process recorder cannot see it (its context never cancels), so this talks to a real
|
||||
// listener (third adversarial review: over a socket every such case logged an ERROR and
|
||||
// half-close returned a 500, because the error page was rendered on a cancelled context).
|
||||
|
||||
// body positions at which the client can hang up: inside the title field, inside the file
|
||||
// part's own headers, and mid-file. Every one must be handled alike (fourth adversarial
|
||||
// review: the title branch had no guard and still logged an ERROR).
|
||||
const (
|
||||
inTitle = "in the title"
|
||||
inHeaders = "in the file part's headers"
|
||||
midFile = "mid-file"
|
||||
)
|
||||
|
||||
func partialUpload(t *testing.T, addr, where string, hangUp func(*net.TCPConn)) {
|
||||
t.Helper()
|
||||
c, err := net.Dial("tcp", addr)
|
||||
require.NoError(t, err)
|
||||
tc := c.(*net.TCPConn)
|
||||
const boundary = "xBOUNDARYx"
|
||||
var body string
|
||||
switch where {
|
||||
case inTitle:
|
||||
body = "--" + boundary + "\r\nContent-Disposition: form-data; name=\"title\"\r\n\r\nhalf a tit"
|
||||
case inHeaders:
|
||||
body = "--" + boundary + "\r\nContent-Disposition: form-data; name=\"file\"; filena"
|
||||
default:
|
||||
body = "--" + boundary + "\r\nContent-Disposition: form-data; name=\"file\"; filename=\"a.m4a\"\r\nContent-Type: audio/mp4\r\n\r\n" + strings.Repeat("a", 4000)
|
||||
}
|
||||
_, err = fmt.Fprintf(tc, "POST /uploads HTTP/1.1\r\nHost: x\r\nContent-Type: multipart/form-data; boundary=%s\r\nContent-Length: 200000\r\n\r\n%s", boundary, body)
|
||||
require.NoError(t, err)
|
||||
time.Sleep(50 * time.Millisecond) // let the server start reading
|
||||
hangUp(tc)
|
||||
}
|
||||
|
||||
func TestUploadPost_AClientThatHangsUpAnywhereInTheBodyIsLoggedAtInfoAndNeverAsAnError(t *testing.T) {
|
||||
hangUps := map[string]func(*net.TCPConn){
|
||||
"a half close": func(c *net.TCPConn) { _ = c.CloseWrite(); time.Sleep(100 * time.Millisecond); _ = c.Close() },
|
||||
"a full close": func(c *net.TCPConn) { _ = c.Close() },
|
||||
"a reset (RST)": func(c *net.TCPConn) { _ = c.SetLinger(0); _ = c.Close() },
|
||||
}
|
||||
for _, where := range []string{inTitle, inHeaders, midFile} {
|
||||
for name, hangUp := range hangUps {
|
||||
t.Run(where+", "+name, func(t *testing.T) {
|
||||
app, sub := newUploadApp(t, audioOK, 200<<20)
|
||||
buf := &lockedBuf{}
|
||||
app.Log = slog.New(slog.NewTextHandler(buf, nil))
|
||||
srv := httptest.NewServer(app.Router())
|
||||
defer srv.Close()
|
||||
|
||||
partialUpload(t, strings.TrimPrefix(srv.URL, "http://"), where, hangUp)
|
||||
|
||||
require.Eventually(t, func() bool {
|
||||
out := buf.String()
|
||||
return strings.Contains(out, "upload abandoned") || strings.Contains(out, "upload rejected") || strings.Contains(out, "level=ERROR")
|
||||
}, 3*time.Second, 20*time.Millisecond, "the server never finished handling the aborted upload")
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
out := buf.String()
|
||||
assert.NotContains(t, out, "level=ERROR", "a client hanging up is not a server fault:\n%s", out)
|
||||
assert.Empty(t, sub.jobs)
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// goneMidAccept is an Uploader during whose Accept the client disappears: the request's
|
||||
// context is cancelled, then Accept fails the way a read from a dead connection does.
|
||||
type goneMidAccept struct{ cancel context.CancelFunc }
|
||||
|
||||
func (g goneMidAccept) Accept(context.Context, string, string, io.Reader) (string, error) {
|
||||
g.cancel()
|
||||
return "", fmt.Errorf("receive: %w", io.ErrClosedPipe)
|
||||
}
|
||||
|
||||
func TestUploadPost_AnAbandonedRequestGetsA499NotAnImplicitEmpty200(t *testing.T) {
|
||||
// If the context is cancelled but the connection can still be written to (a client that
|
||||
// half-closed after sending), a missing status becomes 200 with an empty body: a false
|
||||
// success (fourth adversarial review).
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
buf := &lockedBuf{}
|
||||
app.Log = slog.New(slog.NewTextHandler(buf, nil))
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
app.Uploads = goneMidAccept{cancel: cancel}
|
||||
|
||||
rec := do(t, app, uploadReq(t, "t", "a.m4a", []byte("x")).WithContext(ctx))
|
||||
|
||||
assert.Equal(t, 499, rec.Code, "never an implicit 200 for an upload that was not accepted")
|
||||
assert.Contains(t, buf.String(), "upload abandoned")
|
||||
assert.NotContains(t, buf.String(), "level=ERROR")
|
||||
}
|
||||
|
||||
func TestUploadPost_ADatabaseDropThatLooksLikeAnEOFIsStillAServerError(t *testing.T) {
|
||||
// pgx turns a connection that drops mid-message into io.ErrUnexpectedEOF. Mapping that
|
||||
// sentinel to "your upload was cut off" (a 400) would hide a real fault: only an error
|
||||
// from reading the CLIENT's body is theirs (third adversarial review).
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
buf := &lockedBuf{}
|
||||
app.Log = slog.New(slog.NewTextHandler(buf, nil))
|
||||
app.Uploads = failingUploader{err: fmt.Errorf("failed to receive message: %w", io.ErrUnexpectedEOF)}
|
||||
|
||||
rec := do(t, app, uploadReq(t, "t", "a.m4a", []byte("x")))
|
||||
|
||||
assert.Equal(t, http.StatusInternalServerError, rec.Code)
|
||||
assert.Contains(t, buf.String(), "level=ERROR")
|
||||
}
|
||||
@@ -465,7 +465,6 @@ func TestUploadStatus_ALengthFailureIsExplainedInWordsAndNotAsAGenericError(t *t
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
tests := []struct{ kind, want string }{
|
||||
{"audio_too_long", "longer than the limit"},
|
||||
{"audio_incomplete", "ends much earlier than"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.kind, func(t *testing.T) {
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
@@ -251,3 +251,37 @@ func TestUploadProgress_TheFullPageCarriesTheVerySameProgressListAsTheFragment(t
|
||||
assert.Equal(t, stepsBlock(t, fragment), stepsBlock(t, full),
|
||||
"the page a user opens and the fragment HTMX swaps in must show the same list, or the display jumps on the first poll")
|
||||
}
|
||||
|
||||
func TestUploadProgress_AStepNoteIsShownDirectlyUnderItsStepAndEscaped(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
fixedClock(app)
|
||||
id := seedUpload(t, userID, "m", "transcribing", "")
|
||||
steps := twoEngineRun()
|
||||
steps[1].Note = "The audio is 20 min long; the file claimed 1 h 19 min. <b>cut off</b>?"
|
||||
seedProgress(t, id, steps, progressNow.Add(-10*time.Second), progressNow.Add(-4*time.Second))
|
||||
|
||||
b := statusBody(t, app, id)
|
||||
|
||||
audio := strings.Index(b, "Preparing the audio")
|
||||
note := strings.Index(b, "The audio is 20 min long")
|
||||
next := strings.Index(b, "Transcribing with iguana/pianissimo")
|
||||
require.True(t, audio >= 0 && note > audio && next > note, "the note must sit between its own step and the next one:\n%s", b)
|
||||
assert.Contains(t, b, "step-note")
|
||||
assert.NotContains(t, b, "<b>cut off</b>", "a note is words, never markup")
|
||||
}
|
||||
|
||||
func TestUploadProgress_ADonePageShowsItsRecordOpenWhenAStepHasANote(t *testing.T) {
|
||||
// A truncated recording must not look like a clean one: the note that replaced the
|
||||
// refusal is the only warning, so it cannot sit inside a collapsed list (third review).
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
fixedClock(app)
|
||||
noted := seedUpload(t, userID, "noted", "done", "Synthetic transcript text.")
|
||||
steps := []domain.ProgressStep{step("audio", "Preparing the audio", domain.StepDone, 20*time.Minute, 40*time.Second, 0, 0)}
|
||||
steps[0].Note = "The audio is 20 min long; the file claimed 1 h 19 min."
|
||||
seedProgress(t, noted, steps, progressNow.Add(-time.Minute), progressNow.Add(-time.Minute))
|
||||
clean := seedUpload(t, userID, "clean", "done", "Synthetic transcript text.")
|
||||
seedProgress(t, clean, []domain.ProgressStep{step("audio", "Preparing the audio", domain.StepDone, 20*time.Minute, 40*time.Second, 0, 0)}, progressNow.Add(-time.Minute), progressNow.Add(-time.Minute))
|
||||
|
||||
assert.Contains(t, statusBody(t, app, noted), `<details class="what-was-done" open>`)
|
||||
assert.NotContains(t, statusBody(t, app, clean), `<details class="what-was-done" open>`, "a clean record stays collapsed")
|
||||
}
|
||||
|
||||
@@ -47,6 +47,9 @@ func TestRenderUploadProgressStatesForReview(t *testing.T) {
|
||||
step("save", "Saving the transcript", domain.StepDone, time.Minute, time.Second, 0, 0),
|
||||
}
|
||||
|
||||
noted := twoEngineRun()
|
||||
noted[1].Note = "The audio is 20 min long; the file claimed 1 h 19 min. If the recording was cut off, upload the complete one."
|
||||
|
||||
cases := []struct {
|
||||
name, status, content string
|
||||
steps []domain.ProgressStep
|
||||
@@ -58,6 +61,7 @@ func TestRenderUploadProgressStatesForReview(t *testing.T) {
|
||||
{"lost", "transcribing", "", twoEngineRun(), 3 * time.Minute, 3 * time.Minute},
|
||||
{"failed", "failed", "transcription_failed", failed, 2 * time.Minute, 2 * time.Minute},
|
||||
{"done", "done", "Synthetic transcript text for the review.", done, time.Minute, time.Minute},
|
||||
{"noted", "transcribing", "", noted, 10 * time.Second, 4 * time.Second},
|
||||
{"queued", "queued", "", nil, 0, 0},
|
||||
}
|
||||
for _, c := range cases {
|
||||
|
||||
@@ -62,11 +62,11 @@ func TestUploadStatus_ASpotShowsTheWordsJustBeforeIt(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
id := seedUpload(t, userID, "m", "done", "Synthetic transcript.")
|
||||
setReconciliation(t, id, `{"status":"reconciled","base":"a","alt":"b","reconciler":"c","disputes":1,"changed":1,
|
||||
"contested":[{"seconds":2640,"base":"vanligt","alt":"KORT","used":"alt","unsure":false,"before":"vi skickar in ett"}]}`)
|
||||
"contested":[{"seconds":2640,"base":"vanligt","alt":"KORT","used":"alt","unsure":false,"before":"grannen målar ett"}]}`)
|
||||
|
||||
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
|
||||
|
||||
assert.Contains(t, b, "vi skickar in ett", "the words before the spot are how a reader finds it in the audio")
|
||||
assert.Contains(t, b, "grannen målar ett", "the words before the spot are how a reader finds it in the audio")
|
||||
assert.Contains(t, b, "vanligt")
|
||||
assert.Contains(t, b, "KORT")
|
||||
}
|
||||
|
||||
+431
-327
File diff suppressed because it is too large
Load Diff
@@ -780,6 +780,7 @@ a.btn, a.btn:visited { color: var(--accent-fg); }
|
||||
.step-skipped { opacity: .7; }
|
||||
.step-pending { opacity: .75; }
|
||||
.step-bar { flex: none; width: calc(100% - 1.6rem); margin: 0 0 0 1.6rem; height: .5rem; accent-color: var(--accent); }
|
||||
.step-note { flex: none; width: calc(100% - 1.6rem); margin-left: 1.6rem; font-weight: 400; color: var(--muted); font-family: system-ui, -apple-system, sans-serif; font-size: .82rem; }
|
||||
.step-count { flex: none; width: calc(100% - 1.6rem); margin-left: 1.6rem; font-weight: 400; color: var(--muted); }
|
||||
.step-running .step-mark-warn { color: var(--err-fg); }
|
||||
.spinner { display: inline-block; vertical-align: -.1em; width: .8rem; height: .8rem; box-sizing: border-box; border: 2px solid var(--line); border-top-color: var(--accent); border-radius: 50%; animation: tapir-spin 1s linear infinite; }
|
||||
|
||||
@@ -107,9 +107,12 @@ templ flashBanner(code string) {
|
||||
// #summary-list region; a non-HTMX request renders the whole page. flash carries
|
||||
// a one-shot notification (e.g. "connected", "registered") surfaced on arrival
|
||||
// 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") {
|
||||
@flashBanner(flash)
|
||||
if uploadsEnabled {
|
||||
<p class="muted"><a href="/uploads">Recordings</a>: upload audio and get a private transcript.</p>
|
||||
}
|
||||
if hasConnected {
|
||||
@pasteForm()
|
||||
}
|
||||
|
||||
+418
-408
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",
|
||||
"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
|
||||
"Two engines transcribe the audio and their disagreements are reconciled": "TestEnsemble_BothEnginesRunInOrderAndTheirDisagreementIsReconciled",
|
||||
"Trivial disagreements never reach the model": "TestReconcile_TrivialDisputesAreResolvedWithoutAskingTheModel",
|
||||
@@ -149,7 +155,7 @@ var scenarioCoverage = map[string]string{
|
||||
"A forged cloud model is never sent the transcript": "TestUploadChatPost_AForgedCloudModelIsReplacedByALocalOneBeforeTheTranscriptIsSent",
|
||||
"A wedged engine does not use up the whole job": "TestChain_AHungPrimaryReachesTheFallbackWhenItHasItsOwnDeadline",
|
||||
"A file that lies about its length is refused": "TestWorkerDuration_AConvertedLengthOverTheLimitFailsTheJobBeforeAnyEngineSeesIt",
|
||||
"A truncated recording is refused, not half transcribed": "TestWorkerDuration_AFileThatEndsFarEarlierThanItClaimedIsRefusedNotHalfTranscribed",
|
||||
"A recording much shorter than its file claimed is transcribed and noted": "TestWorkerDuration_AFileThatEndsFarEarlierThanItClaimedIsStillTranscribedAndSaysSo",
|
||||
"A full queue is a retry, not an error": "TestAccept_AFullQueueIsABusyRejectionNotAServerErrorAndLeavesNothingBehind",
|
||||
"One user cannot fill the queue for everyone": "TestWorkerSubmit_OneUserCannotFillTheQueueForEveryoneElse",
|
||||
"Errors are shown as kinds, never their text": "TestReconcile_TheBreakerReasonCarriesAKindNotTheModelsText",
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user