diff --git a/apps/server/internal/adapters/postgres/movement_test.go b/apps/server/internal/adapters/postgres/movement_test.go index 79db5df..09e7980 100644 --- a/apps/server/internal/adapters/postgres/movement_test.go +++ b/apps/server/internal/adapters/postgres/movement_test.go @@ -7,6 +7,7 @@ import ( "image" "image/color" "image/png" + "slices" "strings" "testing" @@ -403,3 +404,40 @@ func TestAcceptingAFixApprovesItsBytes(t *testing.T) { t.Errorf("status = %q after accepting the fix, want accepted", state) } } + +// The frugal contract: one refusal names every missing address (#223). When +// a capture and a recording are both absent, the first answer names both — +// before the fix it named the captures alone, the recording surfaced only on +// the second push, and the one-refusal promise broke for any client sending +// videos. +func TestAMissingRecordingIsNamedInTheFirstRefusal(t *testing.T) { + ctx, repo, blobs, project, kase := freshnessFixture(t) + absentCapture := "sha256:" + strings.Repeat("ab", 32) + absentRecording := "sha256:" + strings.Repeat("cd", 32) + + svc := intake.New(repo, blobs) + _, err := svc.Take(ctx, project.Slug, contract.Manifest{ + Cases: []contract.ManifestCase{{ + ID: kase.ID, + Steps: []contract.ManifestStep{{ + Name: "opens", + Captures: []contract.ManifestCapture{{ + Variant: map[string]string{"theme": "light"}, Hash: absentCapture, + }}, + }}, + Recordings: []contract.ManifestRecording{{ + Variant: map[string]string{"theme": "light"}, Hash: absentRecording, + }}, + }}, + }) + + var missing *intake.MissingContent + if !errors.As(err, &missing) { + t.Fatalf("err = %v, want MissingContent naming both addresses", err) + } + got := slices.Sorted(slices.Values(missing.Hashes)) + want := slices.Sorted(slices.Values([]string{absentCapture, absentRecording})) + if !slices.Equal(got, want) { + t.Errorf("hashes = %v, want the capture and the recording together", missing.Hashes) + } +} diff --git a/apps/server/internal/app/intake/intake_test.go b/apps/server/internal/app/intake/intake_test.go index 1b8cb2c..1fb43cc 100644 --- a/apps/server/internal/app/intake/intake_test.go +++ b/apps/server/internal/app/intake/intake_test.go @@ -48,6 +48,11 @@ func (b refusingBlobs) Get(context.Context, string) (io.ReadCloser, error) { return nil, nil } +func (b refusingBlobs) Exists(context.Context, string) (bool, error) { + b.t.Error("intake checked a blob, want the manifest refused first") + return false, nil +} + func validHash(b byte) string { h := "sha256:" for i := 0; i < 64; i++ { diff --git a/apps/server/internal/app/intake/png.go b/apps/server/internal/app/intake/png.go index 204b359..2a6e278 100644 --- a/apps/server/internal/app/intake/png.go +++ b/apps/server/internal/app/intake/png.go @@ -47,8 +47,9 @@ func (n *NotPNG) Unwrap() error { return ErrNotAPNG } // answer cannot change under it. This is the early, useful answer; that one is // the guard. // -// Recordings are not checked: they are never compared, so their format is -// nobody's business (ADR 0013). +// Recordings are format-checked never — they are never compared, so their +// format is nobody's business (ADR 0013) — but their absence is reported +// here with everything else: one refusal names every missing address (#223). func (s *Service) checkCaptures(ctx context.Context, m contract.Manifest) error { seen := map[string]struct{}{} var absent, notPNG []string @@ -72,6 +73,19 @@ func (s *Service) checkCaptures(ctx context.Context, m contract.Manifest) error } } } + for _, r := range c.Recordings { + if _, done := seen[r.Hash]; done { + continue + } + seen[r.Hash] = struct{}{} + held, err := s.blobs.Exists(ctx, r.Hash) + if err != nil { + return err + } + if !held { + absent = append(absent, r.Hash) + } + } } if len(absent) > 0 { return &MissingContent{Hashes: absent} diff --git a/apps/server/internal/app/intake/ports.go b/apps/server/internal/app/intake/ports.go index d171b19..5fec185 100644 --- a/apps/server/internal/app/intake/ports.go +++ b/apps/server/internal/app/intake/ports.go @@ -41,9 +41,11 @@ type Repository interface { } // Blobs reads capture bytes back. Intake needs them for two things: proving a -// capture is a PNG, and comparing it against what was approved. +// capture is a PNG, and comparing it against what was approved. Existence +// alone answers for a recording (#223): its bytes are never read here. type Blobs interface { Get(ctx context.Context, hash string) (io.ReadCloser, error) + Exists(ctx context.Context, hash string) (bool, error) } // Verdict is what the comparison found for one step and variant, ready to be stored.