Repository navigation
Show Starr download paths as a list and read PATHS_0 - #835
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused frontend, backend, schema, and test changes are consistent, with no unresolved issues identified in static review; tests were not executed.
No open issues 🎉
What changed in this PR
Aligns Starr download-path settings across the WebUI and backend, addressing the indexed environment-variable mismatch reported in #831.
Changes:
- Displays download paths as a multiline list and recognizes indexed
UN_*_PATHS_0overrides. - Uses a standard string slice and removes the legacy singular
pathfield. - Updates API definitions, localized help, and regression tests.
| File | Description |
|---|---|
| pkg/unpackerr/openapi.json | Removes singular path from the API schema. |
| pkg/unpackerr/handlers.go | Changes Starr paths to a standard string slice. |
| pkg/unpackerr/configput_test.go | Updates PUT tests to use path lists. |
| pkg/unpackerr/configclone.go | Preserves deep copying of path slices. |
| pkg/unpackerr/configapi_test.go | Updates GET test fixtures to use path lists. |
| pkg/unpackerr/cnfgfile.go | Removes legacy singular-path handling. |
| pkg/unpackerr/cnfgfile_test.go | Tests indexed environment paths and updates schema checks. |
| pkg/configdef/help.go | Removes singular-path help aliases. |
| pkg/configdef/definitions_test.go | Checks the plural paths help key. |
| frontend/src/pages/settings/StarrForm.svelte | Adds multiline path editing and list serialization. |
| frontend/src/lib/types.ts | Removes the singular path property. |
| frontend/src/lib/slug.ts | Removes the singular path environment mapping. |
| frontend/src/lib/i18n/locales/nl.json | Updates Dutch path-list help. |
| frontend/src/lib/i18n/locales/es.json | Updates Spanish path-list help. |
| frontend/src/lib/i18n/locales/en.json | Updates English path-list help. |
| frontend/src/lib/i18n/locales/el.json | Updates Greek path-list help. |
| frontend/src/lib/env.spec.ts | Tests indexed Starr path matching and display. |
Review effort: Balanced
![qwen-pr-bot[bot]](https://avatars.githubusercontent.com/in/4697433?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approving. Reviewed 9cde866c against base 9f02ad8, and since this one changes both runtime behavior and what the form renders, I built and ran both commits before signing off.
Three changes actually ship: StarrConfig.Paths drops its custom StringSlice type for a plain []string; the deprecated singular path field, its help alias, and the validateApp merge shim are gone; and the Starr form renders download paths as a two-line textarea (one per line) bound to the PATHS env name. i18n, the OpenAPI schema, and the test suite follow along consistently.
The crux is why the type swap matters: cnfg only routes indexed env names (PATHS_0, PATHS_1, …) to plain slices. A type implementing UnmarshalENV short-circuits to the single-value path, so at the base commit the UN_SONARR_0_PATHS_0 the compose example teaches was silently dropped — my harness confirmed it never reaches the field, never lands in the env-used list the lock badge reads, and doesn't create a phantom instance either. The bare comma-split UN_*_PATHS was the one form that worked. Your change flips exactly that, which is what #831(2) is asking for.
Same Sonarr settings form, config with paths = ["/downloads"]: at base the old singular Path input renders empty, the file's value is invisible to it.
At head with UN_SONARR_0_PATHS_0=/downloads set, the Paths textarea shows the env value, locked, and the question mark reads UN_SONARR_0_PATHS_#.
Two non-blocking notes posted inline: the bare UN_*_PATHS comma form is now ignored as well (it worked before, though nothing documented it), and the "config files that still contain path ignore that line" claim is true but unguarded by a test.
Executed validation
go build ./...— clean.go test -count=1 ./pkg/unpackerr/ ./pkg/configdef/(the test plan here) — pass;go test -count=1 ./...— all packages pass.- Frontend at head:
npm ci && npx vitest run— 46/46 across 13 files (45 at base, all pass);npm run check— 0 errors. - Disposable harness around
unmarshalConfigat both commits: at headUN_SONARR_0_PATHS_0/_1fill the live list in index order with the file snapshot untouched and the suffix recorded in the env-used map; at base the same names are ignored while the bare comma form works. A conf file containing the deprecatedpath =line loads without error at head; the line is inert. - Served both builds from this checkout on loopback (test config, proxy auth) and captured the Sonarr settings form via Chromium over CDP — the screenshots above are the actual rendered difference.
Co-authored-by: Cursor <cursoragent@cursor.com>
| conf.DeleteDelay.Duration = u.DeleteDelay.Duration | ||
| } | ||
|
|
||
| if conf.Path != "" && !slices.Contains(conf.Paths, conf.Path) { |
There was a problem hiding this comment.
The shim was half the legacy surface. UN_*_PATH (which the old UI badge literally told people to set) and bare UN_*_PATHS=/a,/b (the old StringSlice comma-split) are ignored by this commit too, and since they're never consumed they never reach /api/config/env, so the UI shows no sign they exist. Verified both at base: one worked and locked the field, the other never showed up anywhere in the UI even though the file-side path line was merged here. The repo's own compose examples and definitions.yml documented the indexed form all along, so the blast radius is mostly people who followed the old badge, but upgrade notes should mention the env side as well. If it's wanted, a one-line startup warning naming those two forms is a cheap ~15 lines. Not a blocker.
|
|
||
| let rows = $state<InstanceRow<StarrConfig>[]>([]) | ||
| let orig = $state<Record<string, StarrConfig>>({}) | ||
| let pathsText = $state<Record<string, string>>({}) |
There was a problem hiding this comment.
remove(id) never deletes the orphaned pathsText[id] entry. Nothing reads it since every consumer keys off row.id, so it's harmless, but delete it alongside the row so the two stay in sync while this map is new.
|
|
||
| for idx, path := range conf.Paths { | ||
| conf.Paths[idx] = expandHomedir(path) | ||
| } |
There was a problem hiding this comment.
UN_*_PATHS_0= (present but empty) now lands as Paths=[""], which skips this default fill, whereas the old StringSlice decoder treated an empty value as a no-op. Nobody sets an empty path in practice, but filtering blank entries before this check keeps the fallback honest when it does happen.
Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
pathslist, one directory per line. The help text isUN_*_PATHS_0, soUN_SONARR_0_PATHS_0from the compose file shows up and locks the field.Pathsis a normal string slice. It was a custom slice whose env decoder only accepted one comma-separated value, so the_0name never applied.pathfield is gone. Config files that still contain it ignore that line.Test plan
UN_SONARR_0_PATHS_0=/downloadsshows/downloadson the Sonarr form and the question mark saysUN_SONARR_0_PATHS_#.pathsand does not writepath.go test -count=1 ./pkg/unpackerr/ ./pkg/configdef/