From 441dd596ade21704bb856e222bd3f852f5dcd72f Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Fri, 7 Aug 2026 19:09:46 -0600 Subject: [PATCH 1/7] fix(core): honor min_successful_image_uploads when a host drops uploads min_successful_image_uploads is parsed from config and shipped in the example config, but nothing ever read it. Any single image an image host failed to accept failed the whole host batch, which records a tracker-scoped image-hosting failure and removes that tracker from the downstream set. With one selected tracker that empties the upload plan. Image hosts drop individual uploads under concurrency, and larger screenshots are dropped far more often, so a 2160p release could lose an otherwise complete upload to one refused image. Resolve the configured floor in internal/core, which is the only layer that sees reused plus newly published images for a target, and accept a partially failed host batch once the target has published at least that many images. Zero keeps the previous strict behavior, and a requested count below the floor cannot clear it. --- internal/config/config.go | 10 ++ internal/config/defaults/example.yaml | 2 + internal/core/media.go | 36 +++++++ internal/core/media_test.go | 132 ++++++++++++++++++++++++++ 4 files changed, 180 insertions(+) diff --git a/internal/config/config.go b/internal/config/config.go index 651b758f..f510b0c4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -203,6 +203,16 @@ func (c *ScreenshotHandlingConfig) ResolvedMaxMenuItems() int { return c.MaxMenuItems } +// ResolvedMinSuccessfulUploads returns how many images a host must publish for +// a tracker before a partially failed image-host batch still counts as usable. +// Zero or negative disables the allowance, so any failed image fails the batch. +func (c *ScreenshotHandlingConfig) ResolvedMinSuccessfulUploads() int { + if c.MinSuccessfulUploads <= 0 { + return 0 + } + return c.MinSuccessfulUploads +} + type DescriptionSettingsConfig struct { AddLogo bool `yaml:"add_logo"` LogoSize int `yaml:"logo_size"` diff --git a/internal/config/defaults/example.yaml b/internal/config/defaults/example.yaml index 95d6fe10..6dddc818 100644 --- a/internal/config/defaults/example.yaml +++ b/internal/config/defaults/example.yaml @@ -49,6 +49,8 @@ screenshot_handling: screens: 4 # Automatic DVD menu capture is opt-in. This caps stored distinct menu screens (1-32). max_menu_items: 6 + # When an image host drops some uploads, the batch still counts as usable once + # this many images are published. Zero requires every image to succeed. min_successful_image_uploads: 3 cutoff_screens: 1 frame_overlay: false diff --git a/internal/core/media.go b/internal/core/media.go index 5074c4f9..48c5862b 100644 --- a/internal/core/media.go +++ b/internal/core/media.go @@ -636,6 +636,10 @@ func (m *mediaModule) uploadImagesToTarget( len(images), ) uploaded, err := m.images.Upload(progressCtx, imageHostingSubject(meta), target.Host, target.UsageScope, images) + if err != nil && m.partialHostUploadIsUsable(target, len(images), len(uploaded), err) { + emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), nil) + return uploaded, nil + } emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err) return wrapCoreResult(uploaded, err) } @@ -703,6 +707,10 @@ func (m *mediaModule) uploadImagesToTarget( ) uploaded, err := m.images.Upload(progressCtx, imageHostingSubject(meta), target.Host, target.UsageScope, missing) results = append(results, uploaded...) + if err != nil && m.partialHostUploadIsUsable(target, len(images), len(results), err) { + emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), nil) + return results, nil + } emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err) if err != nil { return results, fmt.Errorf("core: %w", err) @@ -710,6 +718,34 @@ func (m *mediaModule) uploadImagesToTarget( return results, nil } +// partialHostUploadIsUsable reports whether a partially failed host batch still +// published enough images for the target's trackers to upload with. The +// allowance is the configured min_successful_image_uploads floor, which is +// disabled at zero so every failed image keeps failing the batch. Requested +// counts below the floor cannot clear it, so those batches stay strict. +func (m *mediaModule) partialHostUploadIsUsable( + target trackers.ImageUploadTarget, + requested int, + published int, + err error, +) bool { + minimum := m.cfg.ScreenshotHandling.ResolvedMinSuccessfulUploads() + if minimum <= 0 || published < minimum || requested < minimum { + return false + } + m.logger.Warnf( + "core: accepting partial image host upload host=%s tracker=%s trackers=%v requested=%d published=%d minimum=%d decision=continue err=%s", + target.Host, + m.imageHostOwnerLogValue(target.Host), + target.Trackers, + requested, + published, + minimum, + logging.SanitizeMessage(uploadFailureMessage(err)), + ) + return true +} + func emitCoreImageUploadResult( ctx context.Context, target api.ImageUploadProgressTarget, diff --git a/internal/core/media_test.go b/internal/core/media_test.go index 1f9df14b..91e377d5 100644 --- a/internal/core/media_test.go +++ b/internal/core/media_test.go @@ -362,3 +362,135 @@ func sortedCallHosts(calls []imageHostCall) []string { slices.Sort(hosts) return hosts } + +// partialImageHostingService publishes a fixed number of images and then fails +// the batch, mirroring a host that drops individual uploads under load. +type partialImageHostingService struct { + published int +} + +func (*partialImageHostingService) ListCandidates(context.Context, api.ImageHostingSubject) ([]api.ScreenshotImage, error) { + return nil, nil +} + +func (s *partialImageHostingService) Upload( + _ context.Context, + _ api.ImageHostingSubject, + host string, + usageScope string, + images []api.ScreenshotImage, +) ([]api.UploadedImageLink, error) { + published := min(s.published, len(images)) + links := make([]api.UploadedImageLink, 0, published) + for _, image := range images[:published] { + links = append(links, api.UploadedImageLink{ + ImagePath: image.Path, + Host: host, + UsageScope: usageScope, + RawURL: "https://images.example.invalid/" + host, + }) + } + if published == len(images) { + return links, nil + } + return links, fmt.Errorf("image hosting: %d of %d uploads failed", len(images)-published, len(images)) +} + +func TestUploadImagesAcceptsPartialHostBatchAtConfiguredMinimum(t *testing.T) { + t.Parallel() + + images := make([]api.ScreenshotImage, 0, 6) + for index := range 6 { + images = append(images, api.ScreenshotImage{Path: fmt.Sprintf("screen%d.png", index)}) + } + target := trackers.ImageUploadTarget{ +Host: "pixhost", + UsageScope: "global", + Trackers: []string{"ONE"}, +} + + for _, testCase := range []struct { + name string + minimum int + published int + wantLinks int + wantFailure bool + }{ + { +name: "above minimum", + minimum: 3, + published: 5, + wantLinks: 5, +}, + { +name: "at minimum", + minimum: 3, + published: 3, + wantLinks: 3, +}, + { +name: "below minimum", + minimum: 3, + published: 2, + wantFailure: true, +}, + { +name: "allowance disabled", + minimum: 0, + published: 5, + wantFailure: true, +}, + { +name: "minimum above requested", + minimum: 8, + published: 5, + wantFailure: true, +}, + } { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + module := &mediaModule{ + cfg: config.Config{ + ImageHosting: config.ImageHostingConfig{Host1: "pixhost"}, + ScreenshotHandling: config.ScreenshotHandlingConfig{MinSuccessfulUploads: testCase.minimum}, + }, + images: &partialImageHostingService{published: testCase.published}, + logger: &recordingMediaLogger{}, + registry: mediaImageHostRegistry(t), + } + result, err := module.uploadImagesToTargetsWithFallback( + context.Background(), + api.UploadSubject{SourcePath: "Example.Release.2026.mkv"}, + "pixhost", + nil, + []trackers.ImageUploadTarget{target}, + images, + ) + if err != nil { + t.Fatalf("upload images: %v", err) + } + if testCase.wantFailure { + if len(result.Failures) != 1 || !slices.Equal(result.Failures[0].Trackers, []string{"ONE"}) { + t.Fatalf("expected a tracker-scoped failure, got %#v", result.Failures) + } + if !slices.Contains(result.FailedHosts, "pixhost") { + t.Fatalf("expected pixhost to be marked failed, got %v", result.FailedHosts) + } + return + } + if len(result.Failures) != 0 { + t.Fatalf("partial batch above the minimum must not report a failure: %#v", result.Failures) + } + if len(result.FailedHosts) != 0 { + t.Fatalf("partial batch above the minimum must not mark the host failed: %v", result.FailedHosts) + } + if len(result.Links) != testCase.wantLinks { + t.Fatalf("published links = %d, want %d", len(result.Links), testCase.wantLinks) + } + if len(result.Attempts) != 1 || result.Attempts[0].Failure != nil { + t.Fatalf("attempt results = %#v", result.Attempts) + } + }) + } +} From d4ded9d457498d409922e5373d6028ab2292e307 Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Fri, 7 Aug 2026 19:09:54 -0600 Subject: [PATCH 2/7] fix(releaseworkflow): name image-host exhaustion instead of an empty tracker set When image hosting fails for every downstream tracker, resolveDownstreamTrackerSet returned an empty set with a nil error. That set travels into the upload plan and only surfaces at contract validation as "upload dry run requires target tracker IDs", which names neither the stage that emptied it nor the cause. Fail with ErrInvalidTransition naming the blocked trackers when image hosting is what removed the last one. Sets that were already empty for other reasons are untouched. --- internal/releaseworkflow/eligibility.go | 13 +++++++++++++ internal/releaseworkflow/eligibility_test.go | 17 +++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/internal/releaseworkflow/eligibility.go b/internal/releaseworkflow/eligibility.go index 50c2ec45..963a300b 100644 --- a/internal/releaseworkflow/eligibility.go +++ b/internal/releaseworkflow/eligibility.go @@ -314,11 +314,24 @@ func resolveDownstreamTrackerSet( if !ok || media.Revision != state.Workflow.Media.Revision { return downstreamTrackerSet{}, fmt.Errorf("%w: retained media is stale", ErrInvalidTransition) } + blocked := make([]string, 0, len(base)) for trackerID := range base { if _, failed := TrackerImageHostFailure(media, trackerID); failed { delete(base, trackerID) + blocked = append(blocked, string(trackerID)) } } + // An empty set here would otherwise travel silently into the upload + // plan and only surface as a contract validation error about missing + // target tracker IDs, which names neither the stage nor the cause. + if len(base) == 0 && len(blocked) > 0 { + slices.Sort(blocked) + return downstreamTrackerSet{}, fmt.Errorf( + "%w: image hosting failed for every downstream tracker (%s)", + ErrInvalidTransition, + strings.Join(blocked, ", "), + ) + } } selected := base if len(requested) > 0 { diff --git a/internal/releaseworkflow/eligibility_test.go b/internal/releaseworkflow/eligibility_test.go index b26a495c..8c9ab782 100644 --- a/internal/releaseworkflow/eligibility_test.go +++ b/internal/releaseworkflow/eligibility_test.go @@ -228,6 +228,23 @@ TrackerID: "BETA", if err != nil || !slices.Equal(webTargets.TrackerIDs(), []api.TrackerID{"ALPHA"}) { t.Fatalf("WebUI upload targets after image-host failure = %#v err=%v", webTargets, err) } + + // Losing every tracker to image hosting must name the cause here instead of + // handing an empty target set to upload-plan contract validation. + allFailed := media + allFailed.Failures = append(append([]api.WorkflowFailure(nil), media.Failures...), api.WorkflowFailure{ + Failure: api.OperationFailure{Operation: api.OperationKindImageHosting}, + TrackerID: "ALPHA", + }) + state.Media[media.ID] = allFailed + emptied, err := resolveDownstreamTrackerSet(&state, nil, downstreamStageUpload, now) + if !errors.Is(err, ErrInvalidTransition) || len(emptied.TrackerIDs()) != 0 { + t.Fatalf("upload targets with every tracker image-host blocked = %#v err=%v", emptied, err) + } + if !strings.Contains(err.Error(), "ALPHA, BETA") { + t.Fatalf("image-host exhaustion error must name the blocked trackers: %v", err) + } + state.Media[media.ID] = media state.Workflow.Media = nil state.TrackerDecisionMode = TrackerDecisionModePostDupeGate From 5c7fc1d797d7f0d43fbf897080a06782b2c49021 Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Fri, 7 Aug 2026 19:16:39 -0600 Subject: [PATCH 3/7] fix(httpclient): raise image-host upload deadline to 120s UploadTimeout is one constant shared by every image host in the uploader registry, and the 60s whole-request deadline was too tight for all of them. The same timeout failure was seen on pixhost and imgbox before reelflix hosting was configured, so this is not one host being slow. The deadline is not a transfer budget. The failure looks like: Post "https:///api/1/upload": context deadline exceeded (Client.Timeout exceeded while awaiting headers) "while awaiting headers" means the multipart body was already sent and the deadline expired waiting on the host to answer, so what the ceiling has to cover is host-side processing of a 5-8MB image. Every host is slower at that than at a 1080p screenshot, which is why this shows up as a 2160p problem across hosts rather than a per-host one. imgbox spends the same budget three times per batch: its CSRF fetch and token generation each clone the client at UploadTimeout before the upload itself runs, so a slow host burns the ceiling on requests that publish nothing. The batch with recorded timings reported mean_attempt_duration=42.02s across 6 attempts, and since attemptDurations records failed attempts too, the 5 successes averaged around 38s. That is barely 1.5x headroom for an operation whose duration is set by how fast a remote host answers. 120s restores roughly 3x. UploadTimeout is consumed only by internal/imagehosting; every tracker upload path uses DefaultTimeout, so no other deadline widens. --- internal/httpclient/httpclient.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/httpclient/httpclient.go b/internal/httpclient/httpclient.go index db70cea2..85dc58d6 100644 --- a/internal/httpclient/httpclient.go +++ b/internal/httpclient/httpclient.go @@ -11,7 +11,12 @@ import ( const ( // DefaultTimeout and UploadTimeout are whole-request deadlines assigned to [http.Client.Timeout]. DefaultTimeout = 45 * time.Second - UploadTimeout = 60 * time.Second + // UploadTimeout covers image-host uploads, where the deadline spans the host + // processing a multi-megabyte image and not just the transfer. 2160p + // screenshots run 5-8MB and hosts have been observed answering well past a + // minute under load, so this keeps roughly three times the headroom over + // their typical response time. + UploadTimeout = 120 * time.Second ) // New returns a client whose timeout defaults to [DefaultTimeout] when timeout is non-positive. From 92df420f0ac68a53d67bb0dcc5e625f8476504af Mon Sep 17 00:00:00 2001 From: nitrobass24 Date: Fri, 7 Aug 2026 22:17:53 -0600 Subject: [PATCH 4/7] test(core): cover repository link reuse in the partial image-upload allowance The floor lives in internal/core because that is the only layer that sees reused plus newly published images for a target. Two cases pin that: a host that publishes fewer images than the floor still succeeds once reuse makes up the difference, and reuse that is still short of the floor fails. Applying the floor in internal/imagehosting, which only receives the missing subset, would fail the first case. --- internal/core/media_test.go | 50 +++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/internal/core/media_test.go b/internal/core/media_test.go index 91e377d5..355378c5 100644 --- a/internal/core/media_test.go +++ b/internal/core/media_test.go @@ -363,6 +363,33 @@ func sortedCallHosts(calls []imageHostCall) []string { return hosts } +// reusableImageRepository reports images a previous run already published, so +// the reuse branch of uploadImagesToTarget can be exercised. Only the uploaded +// image lookup carries behavior; the rest satisfies the interface. +type reusableImageRepository struct { + mediaRepository + links []api.UploadedImageLink +} + +func (r *reusableImageRepository) ListUploadedImagesByPath(context.Context, string) ([]api.UploadedImageLink, error) { + return slices.Clone(r.links), nil +} + +// reusedImageLinks builds host records that uploadedImagesByPathForTarget +// matches back to the given images. +func reusedImageLinks(images []api.ScreenshotImage, target trackers.ImageUploadTarget) []api.UploadedImageLink { + links := make([]api.UploadedImageLink, 0, len(images)) + for _, image := range images { + links = append(links, api.UploadedImageLink{ + ImagePath: image.Path, + Host: target.Host, + UsageScope: target.UsageScope, + RawURL: "https://images.example.invalid/reused", + }) + } + return links +} + // partialImageHostingService publishes a fixed number of images and then fails // the batch, mirroring a host that drops individual uploads under load. type partialImageHostingService struct { @@ -413,6 +440,7 @@ Host: "pixhost", name string minimum int published int + reused int wantLinks int wantFailure bool }{ @@ -445,17 +473,39 @@ name: "minimum above requested", minimum: 8, published: 5, wantFailure: true, +}, + // Reuse counts toward the floor: the host publishes fewer images than + // the floor on its own, so this only passes when the allowance is + // applied where reused links are visible. + { +name: "reuse completes the minimum", + minimum: 3, + published: 2, + reused: 2, + wantLinks: 4, +}, + { +name: "reuse still short of the minimum", + minimum: 5, + published: 2, + reused: 2, + wantFailure: true, }, } { t.Run(testCase.name, func(t *testing.T) { t.Parallel() + var repo mediaRepository + if testCase.reused > 0 { + repo = &reusableImageRepository{links: reusedImageLinks(images[:testCase.reused], target)} + } module := &mediaModule{ cfg: config.Config{ ImageHosting: config.ImageHostingConfig{Host1: "pixhost"}, ScreenshotHandling: config.ScreenshotHandlingConfig{MinSuccessfulUploads: testCase.minimum}, }, images: &partialImageHostingService{published: testCase.published}, + repo: repo, logger: &recordingMediaLogger{}, registry: mediaImageHostRegistry(t), } From cdaeb2cc8f1df8c23322f5fe63f4592dfff3a025 Mon Sep 17 00:00:00 2001 From: Audionut Date: Sun, 9 Aug 2026 21:21:23 +1000 Subject: [PATCH 5/7] fix(core): preserve usable partial image uploads --- internal/core/media.go | 4 +- internal/core/media_test.go | 97 +++++++++++--------- internal/trackers/description_assets.go | 3 + internal/trackers/description_assets_test.go | 50 ++++++++++ 4 files changed, 110 insertions(+), 44 deletions(-) diff --git a/internal/core/media.go b/internal/core/media.go index 48c5862b..14d7bcb5 100644 --- a/internal/core/media.go +++ b/internal/core/media.go @@ -637,7 +637,7 @@ func (m *mediaModule) uploadImagesToTarget( ) uploaded, err := m.images.Upload(progressCtx, imageHostingSubject(meta), target.Host, target.UsageScope, images) if err != nil && m.partialHostUploadIsUsable(target, len(images), len(uploaded), err) { - emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), nil) + emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err) return uploaded, nil } emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err) @@ -708,7 +708,7 @@ func (m *mediaModule) uploadImagesToTarget( uploaded, err := m.images.Upload(progressCtx, imageHostingSubject(meta), target.Host, target.UsageScope, missing) results = append(results, uploaded...) if err != nil && m.partialHostUploadIsUsable(target, len(images), len(results), err) { - emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), nil) + emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err) return results, nil } emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err) diff --git a/internal/core/media_test.go b/internal/core/media_test.go index 355378c5..4091fedc 100644 --- a/internal/core/media_test.go +++ b/internal/core/media_test.go @@ -431,10 +431,10 @@ func TestUploadImagesAcceptsPartialHostBatchAtConfiguredMinimum(t *testing.T) { images = append(images, api.ScreenshotImage{Path: fmt.Sprintf("screen%d.png", index)}) } target := trackers.ImageUploadTarget{ -Host: "pixhost", - UsageScope: "global", - Trackers: []string{"ONE"}, -} + Host: "pixhost", + UsageScope: "global", + Trackers: []string{"ONE"}, + } for _, testCase := range []struct { name string @@ -445,52 +445,52 @@ Host: "pixhost", wantFailure bool }{ { -name: "above minimum", - minimum: 3, - published: 5, - wantLinks: 5, -}, + name: "above minimum", + minimum: 3, + published: 5, + wantLinks: 5, + }, { -name: "at minimum", - minimum: 3, - published: 3, - wantLinks: 3, -}, + name: "at minimum", + minimum: 3, + published: 3, + wantLinks: 3, + }, { -name: "below minimum", - minimum: 3, - published: 2, - wantFailure: true, -}, + name: "below minimum", + minimum: 3, + published: 2, + wantFailure: true, + }, { -name: "allowance disabled", - minimum: 0, - published: 5, - wantFailure: true, -}, + name: "allowance disabled", + minimum: 0, + published: 5, + wantFailure: true, + }, { -name: "minimum above requested", - minimum: 8, - published: 5, - wantFailure: true, -}, + name: "minimum above requested", + minimum: 8, + published: 5, + wantFailure: true, + }, // Reuse counts toward the floor: the host publishes fewer images than // the floor on its own, so this only passes when the allowance is // applied where reused links are visible. { -name: "reuse completes the minimum", - minimum: 3, - published: 2, - reused: 2, - wantLinks: 4, -}, + name: "reuse completes the minimum", + minimum: 3, + published: 2, + reused: 2, + wantLinks: 4, + }, { -name: "reuse still short of the minimum", - minimum: 5, - published: 2, - reused: 2, - wantFailure: true, -}, + name: "reuse still short of the minimum", + minimum: 5, + published: 2, + reused: 2, + wantFailure: true, + }, } { t.Run(testCase.name, func(t *testing.T) { t.Parallel() @@ -509,8 +509,12 @@ name: "reuse still short of the minimum", logger: &recordingMediaLogger{}, registry: mediaImageHostRegistry(t), } + var progressUpdates []api.ImageUploadProgressUpdate + ctx := api.WithImageUploadProgressReporter(context.Background(), func(update api.ImageUploadProgressUpdate) { + progressUpdates = append(progressUpdates, update) + }) result, err := module.uploadImagesToTargetsWithFallback( - context.Background(), + ctx, api.UploadSubject{SourcePath: "Example.Release.2026.mkv"}, "pixhost", nil, @@ -541,6 +545,15 @@ name: "reuse still short of the minimum", if len(result.Attempts) != 1 || result.Attempts[0].Failure != nil { t.Fatalf("attempt results = %#v", result.Attempts) } + if len(progressUpdates) == 0 { + t.Fatal("accepted partial batch emitted no progress") + } + terminal := progressUpdates[len(progressUpdates)-1] + wantFailed := len(images) - testCase.reused - testCase.published + if terminal.Status != api.ImageUploadProgressFailed || terminal.Completed != len(images) || + terminal.Succeeded != testCase.published || terminal.Reused != testCase.reused || terminal.Failed != wantFailed { + t.Fatalf("accepted partial terminal progress = %#v", terminal) + } }) } } diff --git a/internal/trackers/description_assets.go b/internal/trackers/description_assets.go index 794e5331..d20f3edb 100644 --- a/internal/trackers/description_assets.go +++ b/internal/trackers/description_assets.go @@ -868,6 +868,9 @@ func preloadUploadAssetData( } } applyUploadedVariantsToSlots(preloaded.screenshotSlots, preloaded.uploads) + for index := range preloaded.screenshotSlots { + preloaded.screenshotSlots[index].RenderInScreenshots = len(preloaded.screenshotSlots[index].Variants) > 0 + } preloaded.screenshotSlotsLoaded = true return preloaded, nil } diff --git a/internal/trackers/description_assets_test.go b/internal/trackers/description_assets_test.go index bc699ab7..93ad39bf 100644 --- a/internal/trackers/description_assets_test.go +++ b/internal/trackers/description_assets_test.go @@ -1714,6 +1714,56 @@ func TestResolveDescriptionAssetsAttachesExactUploadedVariants(t *testing.T) { } } +func TestResolveDescriptionAssetsUsesOnlyExactUploadedVariants(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + sourcePath := filepath.Join(tempDir, "Example.Release.2026.1080p-GRP.mkv") + screenshots := make([]api.ScreenshotImage, 3) + for index := range screenshots { + screenshots[index] = api.ScreenshotImage{ + Path: filepath.Join(tempDir, fmt.Sprintf("screen-%d.png", index)), + Purpose: api.ScreenshotPurposeFinal, + } + } + upload := api.UploadedImageLink{ + SourcePath: sourcePath, + ImagePath: screenshots[0].Path, + Host: "pixhost", + UsageScope: globalImageUsageScope, + ImgURL: "https://img.example.invalid/thumb.png", + RawURL: "https://img.example.invalid/screen.png", + WebURL: "https://img.example.invalid/view", + } + meta := api.UploadSubject{ + SourcePath: sourcePath, + ExactMedia: &api.ExactMediaAssets{ + Screenshots: screenshots, + ScreenshotUploads: []api.UploadedImageLink{upload}, + }, + } + repo := &stubRepo{uploads: []api.UploadedImageLink{{ + SourcePath: sourcePath, + ImagePath: screenshots[1].Path, + Host: "imgbb", + RawURL: "https://ambient.example.invalid/screen.png", + }}} + + assets, err := ResolveDescriptionAssets(context.Background(), "HHD", meta, repo, api.NopLogger{}, descriptionAssetsTestRegistry(t)) + if err != nil { + t.Fatalf("resolve partial exact assets: %v", err) + } + if len(assets.Slots) != len(screenshots) || len(renderableSlots(assets.Slots)) != 1 { + t.Fatalf("exact slots = %#v", assets.Slots) + } + if len(assets.Screenshots) != 1 || assets.Screenshots[0].Path != upload.ImagePath || assets.Screenshots[0].RawURL != upload.RawURL { + t.Fatalf("exact screenshots = %#v", assets.Screenshots) + } + if repo.uploadsCalls != 0 { + t.Fatalf("partial exact assets queried ambient uploads %d time(s)", repo.uploadsCalls) + } +} + func TestResolveDescriptionAssetsExactEmptyUploadsDoNotReadAmbientRepositoryState(t *testing.T) { t.Parallel() From b0dc2fd4194ffb6a856e4ff8dcf18ed2e529893d Mon Sep 17 00:00:00 2001 From: Audionut Date: Sun, 9 Aug 2026 21:58:28 +1000 Subject: [PATCH 6/7] fix(releaseworkflow): enforce hosted screenshot requirements --- internal/releaseworkflow/module.go | 59 +++++++++++++++++++++++- internal/releaseworkflow/module_test.go | 61 +++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/internal/releaseworkflow/module.go b/internal/releaseworkflow/module.go index dc8ffef2..7be26e14 100644 --- a/internal/releaseworkflow/module.go +++ b/internal/releaseworkflow/module.go @@ -4654,6 +4654,10 @@ func (m *Module) publishMediaReplacement( return result, nil } +// refreshMutatedMediaStatus recomputes media readiness after a mutation while +// retaining image-host failures and pending reconciliation actions. Before +// required hosting runs, selected local assets determine readiness; afterward, +// each tracker must have enough applicable hosted screenshot sources. func refreshMutatedMediaStatus(snapshot *api.MediaArtifactSet, projections []api.TrackerReleaseProjection) { hostFailures := make([]api.WorkflowFailure, 0, len(snapshot.Failures)) for _, failure := range snapshot.Failures { @@ -4684,11 +4688,15 @@ func refreshMutatedMediaStatus(snapshot *api.MediaArtifactSet, projections []api } snapshot.Failures = hostFailures snapshot.RequiredActions = reconcileActions - if selectedScreenshots < requiredScreenshots || selectedMenus < requiredMenus { + screenshotsReady := selectedScreenshots >= requiredScreenshots + if snapshot.ImageRequirementsPrepared { + screenshotsReady = hostedScreenshotRequirementsMet(*snapshot, projections) + } + if !screenshotsReady || selectedMenus < requiredMenus { snapshot.Status = api.StageStatusBlocked snapshot.RequiredActions = append(snapshot.RequiredActions, api.RequiredAction{ Kind: api.RequiredActionProvideTrackerInput, - Prompt: "Capture or select the required release images before continuing.", + Prompt: "Capture, select, or host the required release images before continuing.", }) return } @@ -4703,6 +4711,53 @@ func refreshMutatedMediaStatus(snapshot *api.MediaArtifactSet, projections []api snapshot.Status = api.StageStatusCompleted } +// hostedScreenshotRequirementsMet reports whether every projection has enough +// unique selected final-screenshot sources in applicable host attempts. Multiple +// hosted variants of the same local screenshot count once; DVD menus do not count. +func hostedScreenshotRequirementsMet(snapshot api.MediaArtifactSet, projections []api.TrackerReleaseProjection) bool { + artifacts := make(map[api.PublicResourceID]api.MediaArtifact, len(snapshot.Artifacts)) + for _, artifact := range snapshot.Artifacts { + artifacts[artifact.ID] = artifact + } + for _, projection := range projections { + if projection.Artifacts.ScreenshotCount <= 0 { + continue + } + trackerID := normalizeDownstreamTrackerID(projection.TrackerID) + sources := make(map[api.PublicResourceID]struct{}, projection.Artifacts.ScreenshotCount) + for _, attempt := range snapshot.HostAttempts { + if !hostedAttemptAppliesToTracker(attempt, trackerID) { + continue + } + for _, result := range attempt.Results { + hosted := artifacts[result.ID] + source := artifacts[api.PublicResourceID(strings.TrimSpace(hosted.Source))] + if hosted.Selected && hosted.Kind == api.MediaArtifactHostedImage && hosted.Purpose == api.ScreenshotPurposeFinal && + source.Selected && source.Kind == api.MediaArtifactScreenshot && source.Purpose == api.ScreenshotPurposeFinal { + sources[source.ID] = struct{}{} + } + } + } + if len(sources) < projection.Artifacts.ScreenshotCount { + return false + } + } + return true +} + +// hostedAttemptAppliesToTracker reports whether an attempt explicitly targeted +// the tracker and used either global scope or that tracker's owned scope. An +// empty usage scope is treated as global. +func hostedAttemptAppliesToTracker(attempt api.HostedImageAttempt, trackerID api.TrackerID) bool { + if !slices.ContainsFunc(attempt.TrackerIDs, func(candidate api.TrackerID) bool { + return normalizeDownstreamTrackerID(candidate) == trackerID + }) { + return false + } + scope := strings.TrimSpace(attempt.UsageScope) + return scope == "" || strings.EqualFold(scope, "global") || strings.EqualFold(scope, "tracker:"+string(trackerID)) +} + func (m *Module) publishMedia( _ string, state *State, diff --git a/internal/releaseworkflow/module_test.go b/internal/releaseworkflow/module_test.go index 9963e6e2..5a6cbf49 100644 --- a/internal/releaseworkflow/module_test.go +++ b/internal/releaseworkflow/module_test.go @@ -3442,6 +3442,67 @@ func TestRefreshMutatedMediaStatusPreservesOnlyGenuineMediaActions(t *testing.T) } } +func TestRefreshMutatedMediaStatusRequiresHostedScreenshotsPerTracker(t *testing.T) { + t.Parallel() + + const trackerID api.TrackerID = "SYNTHETIC" + projections := []api.TrackerReleaseProjection{{ + TrackerID: trackerID, + Artifacts: api.TrackerArtifactRequirements{ScreenshotCount: 6}, + }} + snapshot := api.MediaArtifactSet{ImageRequirementsPrepared: true} + results := make([][]api.MediaArtifact, 2) + for index := range 6 { + sourceID := api.PublicResourceID(fmt.Sprintf("screen-%d", index)) + hosted := api.MediaArtifact{ + ID: api.PublicResourceID(fmt.Sprintf("hosted-%d", index)), + Kind: api.MediaArtifactHostedImage, + Purpose: api.ScreenshotPurposeFinal, + Selected: true, + Source: string(sourceID), + } + snapshot.Artifacts = append(snapshot.Artifacts, api.MediaArtifact{ + ID: sourceID, + Kind: api.MediaArtifactScreenshot, + Purpose: api.ScreenshotPurposeFinal, + Selected: true, + }, hosted) + results[index/3] = append(results[index/3], hosted) + } + duplicate := api.MediaArtifact{ + ID: "hosted-duplicate", + Kind: api.MediaArtifactHostedImage, + Purpose: api.ScreenshotPurposeFinal, + Selected: true, + Source: "screen-0", + } + snapshot.Artifacts = append(snapshot.Artifacts, duplicate) + results[0] = append(results[0], duplicate) + snapshot.HostAttempts = []api.HostedImageAttempt{ + { + UsageScope: "global", + TrackerIDs: []api.TrackerID{trackerID}, + Results: results[0], + }, + { + UsageScope: "tracker:OTHER", + TrackerIDs: []api.TrackerID{trackerID}, + Results: results[1], + }, + } + + refreshMutatedMediaStatus(&snapshot, projections) + if snapshot.Status != api.StageStatusBlocked || len(snapshot.RequiredActions) != 1 { + t.Fatalf("three tracker-usable hosted screenshots satisfied six required screenshots: %#v", snapshot) + } + + snapshot.HostAttempts[1].UsageScope = "global" + refreshMutatedMediaStatus(&snapshot, projections) + if snapshot.Status != api.StageStatusCompleted || len(snapshot.RequiredActions) != 0 { + t.Fatalf("six tracker-usable hosted screenshots did not satisfy the requirement: %#v", snapshot) + } +} + func waitForWorkflowOperation( t *testing.T, module *Module, From 6e5071fdd47570aca3ea15c415f2b1f6040350b6 Mon Sep 17 00:00:00 2001 From: Audionut Date: Sun, 9 Aug 2026 22:54:48 +1000 Subject: [PATCH 7/7] fix(screenshots): validate ffmpeg compression levels --- internal/config/config.go | 3 +++ internal/config/validate_penetration_test.go | 16 ++++++++++++++++ internal/services/screenshots/ffmpeg.go | 6 +----- internal/services/screenshots/ffmpeg_test.go | 17 +++++++++++++++++ webui/src/pages/screenshots/index.test.tsx | 4 ++++ webui/src/pages/screenshots/index.tsx | 2 ++ 6 files changed, 43 insertions(+), 5 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index c7bd4c60..938d89b1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -953,6 +953,9 @@ func (c Config) Validate() error { if c.ScreenshotHandling.MaxMenuItems > MaxDVDMenuItems { return fmt.Errorf("config: screenshot_handling.max_menu_items must not exceed %d", MaxDVDMenuItems) } + if c.ScreenshotHandling.FFmpegCompression < 0 || c.ScreenshotHandling.FFmpegCompression > 9 { + return errors.New("config: screenshot_handling.ffmpeg_compression must be between 0 and 9") + } if c.PostUpload.MaxConcurrentTrackers < 0 { return errors.New("config: post_upload.max_concurrent_tracker_uploads must be zero or greater") } diff --git a/internal/config/validate_penetration_test.go b/internal/config/validate_penetration_test.go index 92e8e74a..24ffdd26 100644 --- a/internal/config/validate_penetration_test.go +++ b/internal/config/validate_penetration_test.go @@ -44,6 +44,22 @@ func TestValidateScreensZeroOrNegative(t *testing.T) { } } +func TestValidateFFmpegCompressionRange(t *testing.T) { + for _, compression := range []int{0, 9} { + cfg := withBase(func(c *Config) { c.ScreenshotHandling.FFmpegCompression = compression }) + if err := cfg.Validate(); err != nil { + t.Errorf("ffmpeg_compression=%d: expected valid, got %v", compression, err) + } + } + for _, compression := range []int{-1, 10} { + cfg := withBase(func(c *Config) { c.ScreenshotHandling.FFmpegCompression = compression }) + err := cfg.Validate() + if err == nil || !strings.Contains(err.Error(), "ffmpeg_compression") { + t.Errorf("ffmpeg_compression=%d: want range error, got %v", compression, err) + } + } +} + func TestValidateMaxConcurrentTrackersNegative(t *testing.T) { t.Parallel() diff --git a/internal/services/screenshots/ffmpeg.go b/internal/services/screenshots/ffmpeg.go index a898a972..af7e4f01 100644 --- a/internal/services/screenshots/ffmpeg.go +++ b/internal/services/screenshots/ffmpeg.go @@ -551,16 +551,12 @@ func buildFFmpegPreviewArgs(req previewRequest) []string { func buildFFmpegArgs(req captureRequest, useLibplacebo bool) []string { vf := buildFilterChain(req, useLibplacebo) - compression := req.Compression - if compression <= 0 { - compression = 6 - } args := []string{"-hide_banner", "-y", "-loglevel", "error", "-ss", fmt.Sprintf("%.3f", req.Timestamp), "-i", req.InputPath, "-frames:v", "1"} if useLibplacebo { args = append(args, "-init_hw_device", "vulkan") } - args = append(args, "-vf", vf, "-compression_level", strconv.Itoa(compression), "-pred", "mixed", req.OutputPath) + args = append(args, "-vf", vf, "-compression_level", strconv.Itoa(req.Compression), "-pred", "mixed", req.OutputPath) return args } diff --git a/internal/services/screenshots/ffmpeg_test.go b/internal/services/screenshots/ffmpeg_test.go index 98974be3..1aea250a 100644 --- a/internal/services/screenshots/ffmpeg_test.go +++ b/internal/services/screenshots/ffmpeg_test.go @@ -15,6 +15,7 @@ import ( "path/filepath" "runtime" "slices" + "strconv" "strings" "testing" @@ -95,6 +96,22 @@ func TestRoundToEvenUsesNearestEvenForHalves(t *testing.T) { } } +func TestBuildFFmpegArgsPreservesPNGCompressionBounds(t *testing.T) { + for _, compression := range []int{0, 9} { + args := buildFFmpegArgs(captureRequest{ + InputPath: "example.mkv", + OutputPath: "screen.png", + Compression: compression, + }, false) + if got := ffmpegValueAfter(args, "-compression_level"); got != strconv.Itoa(compression) { + t.Fatalf("compression level = %q, want %d", got, compression) + } + if got := ffmpegValueAfter(args, "-pred"); got != "mixed" { + t.Fatalf("PNG prediction = %q, want mixed", got) + } + } +} + func TestCaptureFrameBytesRejectsEmptySuccessfulOutput(t *testing.T) { runner := &singleResultRunner{result: CommandResult{ExitCode: 0}} diff --git a/webui/src/pages/screenshots/index.test.tsx b/webui/src/pages/screenshots/index.test.tsx index 768ba157..4149d162 100644 --- a/webui/src/pages/screenshots/index.test.tsx +++ b/webui/src/pages/screenshots/index.test.tsx @@ -192,6 +192,10 @@ describe("ScreenshotsPage", () => { fireEvent.change(screen.getByLabelText("Screenshot count"), { target: { value: "6" } }); expect(updateScreenshotConfigValue).toHaveBeenCalledWith("Screens", 6); + + const compression = screen.getByLabelText("FFmpeg compression"); + expect(compression).toHaveAttribute("min", "0"); + expect(compression).toHaveAttribute("max", "9"); }); it("renders and mutates workflow-owned screenshots by opaque artifact ID", () => { diff --git a/webui/src/pages/screenshots/index.tsx b/webui/src/pages/screenshots/index.tsx index 68b54332..db5a9219 100644 --- a/webui/src/pages/screenshots/index.tsx +++ b/webui/src/pages/screenshots/index.tsx @@ -360,6 +360,8 @@ export default function ScreenshotsPage({ FFmpeg compression