diff --git a/internal/autoscan/adminform.go b/internal/autoscan/adminform.go index c9ab15f7..bb2c4149 100644 --- a/internal/autoscan/adminform.go +++ b/internal/autoscan/adminform.go @@ -1,6 +1,9 @@ package autoscan -import "encoding/json" +import ( + "encoding/json" + "strings" +) // AdminForm describes per-source configuration fields for a scan source, in the // same shape the admin UI's generic schema renderer already consumes for plugin @@ -50,6 +53,7 @@ const ( ControlText = "TEXT" ControlTextarea = "TEXTAREA" ControlSelect = "SELECT" + ControlPassword = "PASSWORD" ) // Fill sources the admin UI understands for AdminFormField.FillFrom. @@ -111,8 +115,28 @@ func adminFormFromMetadata(value any) *AdminForm { if err := json.Unmarshal(raw, &form); err != nil { return nil } + form.Fields = withoutSecretFields(form.Fields) if len(form.Fields) == 0 { return nil } return &form } + +// withoutSecretFields drops fields a plugin marked as holding a credential. +// +// A source's values land in autoscan_sources.source_config, which is plain +// JSONB and is returned verbatim by the source API — unlike connection API +// keys, which go through the repository's encrypted path. Rendering a masked +// input over a value stored in the clear would misrepresent how it is held, so +// the host declines to collect it at all. Plugins needing a credential should +// take a connection instead. +func withoutSecretFields(fields []AdminFormField) []AdminFormField { + kept := make([]AdminFormField, 0, len(fields)) + for _, field := range fields { + if field.Secret || strings.EqualFold(field.Control, ControlPassword) { + continue + } + kept = append(kept, field) + } + return kept +} diff --git a/internal/autoscan/compat.go b/internal/autoscan/compat.go index 5e1f5bfb..91e2e5a8 100644 --- a/internal/autoscan/compat.go +++ b/internal/autoscan/compat.go @@ -134,7 +134,10 @@ func ApplyCompatibilityDescriptor(pluginID, capabilityID string, declared ScanSo // compatibilityDescriptor returns the stopgap descriptor for a known // first-party plugin, if one exists. func compatibilityDescriptor(pluginID, capabilityID string) (ScanSourceDescriptor, bool) { - if pluginID == cephFSPluginID || capabilityID == cephFSCapabilityID { + // Both must match. Capability ids are chosen by plugin authors and are not + // unique across plugins, so an OR here would hand CephFS's path/exclusion + // form to any unrelated plugin that happened to name a capability "cephfs". + if pluginID == cephFSPluginID && capabilityID == cephFSCapabilityID { return cephFSCompatibilityDescriptor(), true } return ScanSourceDescriptor{}, false diff --git a/internal/autoscan/descriptor.go b/internal/autoscan/descriptor.go index ad774b63..373dbcb2 100644 --- a/internal/autoscan/descriptor.go +++ b/internal/autoscan/descriptor.go @@ -129,8 +129,15 @@ func DescriptorFromMetadata(metadata map[string]any) ScanSourceDescriptor { valid := make([]string, 0, len(modes)) for _, m := range modes { switch m := strings.ToLower(strings.TrimSpace(m)); m { - case DeliveryModePoll, DeliveryModeWebhook: + case DeliveryModePoll: valid = append(valid, m) + case DeliveryModeWebhook: + // Deliberately dropped for plugin capabilities. Webhook delivery + // is only accepted for the host's built-in ARR identity (see + // resolveDeliveryMode in the autoscan handler), which supplies + // its descriptor directly rather than through this parser. + // Honouring a plugin's claim here would surface a setup option + // whose every submission ends in HTTP 400. } } // Only adopt the declared list when at least one mode is understood; diff --git a/internal/autoscan/descriptor_test.go b/internal/autoscan/descriptor_test.go index 593f21ca..18e4b3ec 100644 --- a/internal/autoscan/descriptor_test.go +++ b/internal/autoscan/descriptor_test.go @@ -32,7 +32,7 @@ func TestDescriptorFromMetadataDefaultsPreserveLegacyBehavior(t *testing.T) { func TestDescriptorFromMetadataReadsDeclaredContract(t *testing.T) { got := DescriptorFromMetadata(map[string]any{ "scan_source": map[string]any{ - "delivery_modes": []any{"webhook"}, + "delivery_modes": []any{"poll"}, "connection": "none", "connection_kinds": []any{"sonarr", "radarr"}, "emits_native_paths": true, @@ -44,8 +44,8 @@ func TestDescriptorFromMetadataReadsDeclaredContract(t *testing.T) { if got.Connection != ConnectionNone { t.Errorf("connection = %q, want none", got.Connection) } - if got.SupportsDeliveryMode(DeliveryModePoll) { - t.Errorf("delivery modes = %v, want webhook only", got.DeliveryModes) + if !got.SupportsDeliveryMode(DeliveryModePoll) { + t.Errorf("delivery modes = %v, want poll", got.DeliveryModes) } if !got.EmitsNativePaths { t.Error("emits_native_paths not read") @@ -58,6 +58,53 @@ func TestDescriptorFromMetadataReadsDeclaredContract(t *testing.T) { } } +// Webhook delivery is accepted only for the host's built-in ARR identity (see +// resolveDeliveryMode). Honouring a plugin's claim would offer a setup option +// whose every submission is rejected with HTTP 400. +func TestDescriptorFromMetadataIgnoresPluginDeclaredWebhook(t *testing.T) { + got := DescriptorFromMetadata(map[string]any{ + "scan_source": map[string]any{"delivery_modes": []any{"webhook"}}, + }) + if got.SupportsDeliveryMode(DeliveryModeWebhook) { + t.Errorf("delivery modes = %v, plugin-declared webhook must be dropped", got.DeliveryModes) + } + if !got.SupportsDeliveryMode(DeliveryModePoll) { + t.Errorf("delivery modes = %v, want fallback to poll", got.DeliveryModes) + } +} + +// The built-in identity supplies its descriptor directly rather than through +// the metadata parser, so its webhook mode survives. +func TestBuiltinWebhookDescriptorKeepsWebhookMode(t *testing.T) { + if !BuiltinArrWebhookSource().Descriptor.SupportsDeliveryMode(DeliveryModeWebhook) { + t.Fatal("built-in ARR identity must keep webhook delivery") + } +} + +// Secret-marked fields are dropped: source_config is plaintext JSONB, so a +// masked input would misrepresent how the value is stored. +func TestConfigFormDropsSecretFields(t *testing.T) { + got := DescriptorFromMetadata(map[string]any{ + "config_schema": []any{ + map[string]any{"key": "scan_source", "admin_form": map[string]any{ + "fields": []any{ + map[string]any{"key": "root", "label": "Root", "control": "TEXT"}, + map[string]any{"key": "token", "label": "Token", "control": "PASSWORD"}, + map[string]any{"key": "other", "label": "Other", "control": "TEXT", "secret": true}, + }, + }}, + }, + }) + if got.ConfigForm == nil { + t.Fatal("expected the non-secret field to survive") + } + for _, field := range got.ConfigForm.Fields { + if field.Key != "root" { + t.Errorf("secret-bearing field %q must not be collected", field.Key) + } + } +} + // A malformed or unrecognized descriptor must degrade to defaults rather than // make an otherwise working plugin undiscoverable. func TestDescriptorFromMetadataToleratesBadValues(t *testing.T) { @@ -179,6 +226,16 @@ func TestApplyCompatibilityDescriptorDoesNotOverrideManifest(t *testing.T) { } } +// Capability ids are author-chosen and not unique across plugins, so both parts +// must match before CephFS's form is applied. +func TestApplyCompatibilityDescriptorRequiresBothIdentifiers(t *testing.T) { + declared := DescriptorFromMetadata(nil) + got := ApplyCompatibilityDescriptor("com.example.other", cephFSCapabilityID, declared) + if got.ConfigForm != nil { + t.Error("a matching capability id alone must not apply the CephFS form") + } +} + func TestApplyCompatibilityDescriptorLeavesUnknownPluginsAlone(t *testing.T) { declared := DescriptorFromMetadata(nil) got := ApplyCompatibilityDescriptor("com.example.future", "watcher", declared) diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 5a5a82c0..f44a84d8 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -2135,7 +2135,8 @@ export interface AutoscanAvailableSource { capability_id: string; display_name: string; description?: string; - descriptor: AutoscanScanSourceDescriptor; + /** Optional: an older server omits it, and descriptorFor falls back. */ + descriptor?: AutoscanScanSourceDescriptor; } export interface AutoscanAvailableSourcesResponse { diff --git a/web/src/pages/admin/autoscan/InlineConnectionPicker.tsx b/web/src/pages/admin/autoscan/InlineConnectionPicker.tsx index dc2bb062..858ddade 100644 --- a/web/src/pages/admin/autoscan/InlineConnectionPicker.tsx +++ b/web/src/pages/admin/autoscan/InlineConnectionPicker.tsx @@ -23,6 +23,8 @@ export interface ConnectionOption { id: string; name: string; kind: string; + /** Set when this connection is a live link to a Requests integration. */ + requestIntegrationId?: string | null; } /** Sentinel values for the picker's non-connection choices. */ @@ -74,10 +76,17 @@ export function InlineConnectionPicker({ const [reuseId, setReuseId] = useState(""); const [testResult, setTestResult] = useState(null); - // Requests integrations this source could bind, minus any already saved as a - // connection — re-offering those would create a duplicate of the same server. - const linkedIntegrationIds = new Set(options.map((option) => option.id)); + // Requests integrations this source could bind, minus any already linked by a + // saved connection — re-offering those would create a second connection to + // the same server. Compare integration ids, not connection ids: they are + // different identifier spaces, and mixing them matches nothing. + const linkedIntegrationIds = new Set( + options.map((option) => option.requestIntegrationId).filter(Boolean), + ); const reusable = (requestIntegrations.data ?? []).filter((integration) => { + // A disabled integration is rejected at poll time, so binding one here + // would produce a connection that is known-unusable the moment it is made. + if (!integration.enabled) return false; const kind = integrationKind(integration); if (!kind) return false; if (connectionKinds.length > 0 && !connectionKinds.includes(kind)) return false; diff --git a/web/src/pages/admin/autoscan/SourceConfigForm.tsx b/web/src/pages/admin/autoscan/SourceConfigForm.tsx index 2f2205ab..7aa64876 100644 --- a/web/src/pages/admin/autoscan/SourceConfigForm.tsx +++ b/web/src/pages/admin/autoscan/SourceConfigForm.tsx @@ -22,15 +22,25 @@ export function SourceConfigForm({ values, onChange, idPrefix = "source-config", + onValidityChange, }: { descriptor: AutoscanScanSourceDescriptor; - values: Record; - onChange: (next: Record) => void; + /** + * Live form values, typed as the renderer produced them. Booleans and arrays + * stay boolean and array here: stringifying on every keystroke turned `false` + * into the truthy `"false"` (so a switch re-rendered enabled) and an array + * into a comma-joined string the renderer read back as no selection. + * Serialization to source_config happens once, at submit. + */ + values: Record; + onChange: (next: Record) => void; idPrefix?: string; + /** Reports whether every required/validated field is satisfied. */ + onValidityChange?: (valid: boolean) => void; }) { const libraries = useAdminLibraries(); const form = descriptor.config_form; - const fields = configFields(descriptor); + const fields = useMemo(() => configFields(descriptor), [descriptor]); // Fields that can be populated from a host-known value, paired with what that // value currently is. Computed together so the button can be disabled when @@ -59,15 +69,9 @@ export function SourceConfigForm({ { - const out: Record = {}; - for (const [key, value] of Object.entries(next)) { - if (value === undefined || value === null) continue; - out[key] = String(value); - } - onChange(out); - }} + onChange={onChange} idPrefix={idPrefix} + onValidityChange={onValidityChange} /> {fillable.length > 0 && ( diff --git a/web/src/pages/admin/autoscan/SourcesPanel.tsx b/web/src/pages/admin/autoscan/SourcesPanel.tsx index fe73eb7d..1694a82a 100644 --- a/web/src/pages/admin/autoscan/SourcesPanel.tsx +++ b/web/src/pages/admin/autoscan/SourcesPanel.tsx @@ -81,12 +81,14 @@ import { import { useAdminLibraries } from "@/hooks/queries/admin/libraries"; import SourceConfigForm from "./SourceConfigForm"; import { ChoiceCard, StepTrail } from "./ChoiceCard"; -import InlineConnectionPicker from "./InlineConnectionPicker"; +import InlineConnectionPicker, { type ConnectionOption } from "./InlineConnectionPicker"; import { sourceTargets } from "./sourceTargets"; import { WebhookInstructions, WebhookMappingEditor } from "./WebhookSetupStep"; import { hasUsableMapping, seedMappings, usableMappings, type MappingDraft } from "./webhookSetup"; import { connectionIsMandatory, + parseConfigValues, + serializeConfigValues, connectionMatchesKinds, defaultDeliveryMode, DEFAULT_DESCRIPTOR, @@ -143,17 +145,24 @@ interface RowEdit { connectionId: string; // "" means no connection intervalStr: string; // "" means use default rewrites: AutoscanPathRewrite[]; - sourceConfig: Record; + /** Typed while the renderer owns them; serialized only on save. */ + sourceConfig: Record; label: string; + /** False while a required or validated config field is unsatisfied. */ + configValid: boolean; } -function sourceToRowEdit(source: AutoscanSource): RowEdit { +function sourceToRowEdit( + source: AutoscanSource, + descriptor: AutoscanScanSourceDescriptor, +): RowEdit { return { connectionId: source.connection_id ?? "", intervalStr: source.poll_interval_seconds != null ? String(source.poll_interval_seconds) : "", rewrites: source.path_rewrites.map((r) => ({ ...r })), - sourceConfig: sourceConfigForEdit(source), + sourceConfig: parseConfigValues(descriptor, sourceConfigForEdit(source)), label: source.label ?? "", + configValid: true, }; } @@ -170,7 +179,14 @@ function isWebhookSource(source: AutoscanSource): boolean { */ function webhookProviderOf( descriptor: AutoscanScanSourceDescriptor, + sourceConfig?: Record, ): AutoscanWebhookProvider | "auto" { + // An explicit choice in the source's own config wins: the built-in descriptor + // advertises both arr kinds, so relying on it alone would always yield "auto" + // and show combined instructions even after the operator picked one. + const chosen = sourceConfig?.[WEBHOOK_PROVIDER_KEY]; + if (chosen === "sonarr" || chosen === "radarr") return chosen; + const kinds = descriptor.connection_kinds ?? []; if (kinds.length === 1 && (kinds[0] === "sonarr" || kinds[0] === "radarr")) { return kinds[0]; @@ -186,8 +202,8 @@ function absoluteWebhookURL(url: string): string { /** * Legacy source_config keys that were superseded by a newer key. Rows written * before the rename still carry the old key, so its lines are merged into the - * new one when the editor loads. Both keys are sent back on save, which is what - * lets an older plugin build keep reading its original key. + * new one when the editor loads and the old key is dropped — the first save + * from this editor migrates the row forward rather than carrying both. */ const LEGACY_CONFIG_KEY_ALIASES: Record = { movie_nested_paths: "movie_flat_paths", @@ -703,7 +719,7 @@ function SourceRow({ source: AutoscanSource; /** Setup contract for this source's capability; drives its config fields. */ descriptor: AutoscanScanSourceDescriptor; - connectionOptions: Array<{ id: string; name: string }>; + connectionOptions: ConnectionOption[]; pluginDisplayNames: Map; globalPollInterval: number | null; onDelete: (source: AutoscanSource) => void; @@ -711,7 +727,7 @@ function SourceRow({ }) { const update = useUpdateAutoscanSource(); const libraries = useAdminLibraries(); - const [edit, setEdit] = useState(() => sourceToRowEdit(source)); + const [edit, setEdit] = useState(() => sourceToRowEdit(source, descriptor)); const [intervalError, setIntervalError] = useState(false); const isDirty = @@ -731,7 +747,7 @@ function SourceRow({ enabled: source.enabled, poll_interval_seconds: intervalVal, path_rewrites, - source_config: normalizeSourceConfig(edit.sourceConfig), + source_config: normalizeSourceConfig(serializeConfigValues(edit.sourceConfig)), label: edit.label.trim(), ...overrides, }; @@ -789,11 +805,11 @@ function SourceRow({ }); } - function handleSourceConfigSave(nextConfig?: Record) { + function handleSourceConfigSave(nextConfig?: Record) { const sourceConfig = nextConfig ?? edit.sourceConfig; update.mutate({ id: source.id, - body: fullBody({ source_config: normalizeSourceConfig(sourceConfig) }), + body: fullBody({ source_config: normalizeSourceConfig(serializeConfigValues(sourceConfig)) }), }); } @@ -929,10 +945,20 @@ function SourceRow({ ); + // Editing must respect the same contract the add flow enforced, or an + // operator can unbind a `required` source, bind an incompatible kind, or + // attach credentials to a `none` source right after creating it correctly. + const rowConnectionRequired = connectionIsMandatory(descriptor, source.delivery_mode); + const rowEligibleConnections = connectionOptions.filter((c) => + connectionMatchesKinds(descriptor, c.kind), + ); + const connectionControl = isWebhook ? ( Not needed — Sonarr/Radarr deliver directly + ) : !needsConnectionStep(descriptor, source.delivery_mode) ? ( + Not needed — reads locally ) : source.connection_id === null && !edit.connectionId ? (
- - No connection + + {rowConnectionRequired ? "Connection required" : "No connection"}
) : ( @@ -964,8 +997,8 @@ function SourceRow({ - — No connection — - {connectionOptions.map((c) => ( + {!rowConnectionRequired && — No connection —} + {rowEligibleConnections.map((c) => ( {c.name} @@ -990,19 +1023,29 @@ function SourceRow({ // Per-source config, rendered from the capability's declared fields. Absent // for sources that declare none, which is why this can be null. - const sourceConfigEditor = descriptor.config_form?.fields?.length ? ( + // The built-in webhook's provider field is rendered by WebhookEndpointSection + // above, so exclude it here rather than showing the same control twice. + const rowConfigFields = (descriptor.config_form?.fields ?? []).filter( + (field) => !(isWebhook && field.key === WEBHOOK_PROVIDER_KEY), + ); + + const sourceConfigEditor = rowConfigFields.length ? (
setEdit((ed) => ({ ...ed, sourceConfig: next }))} + onValidityChange={(configValid) => setEdit((ed) => ({ ...ed, configValid }))} idPrefix={`source-config-${source.id}`} /> +
+ {children.length > 0 && ( + + )} - → -
- - update(index, { to: e.target.value })} - /> -
- - - ))} + ); + })} )} @@ -184,7 +217,7 @@ export function WebhookMappingEditor({ type="button" variant="outline" size="sm" - onClick={() => onChange([...mappings, { from: "", to: "" }])} + onClick={() => onChange([...mappings, newMapping()])} > Add a mapping diff --git a/web/src/pages/admin/autoscan/sourceDescriptor.test.ts b/web/src/pages/admin/autoscan/sourceDescriptor.test.ts index 39eabd7e..0934d44f 100644 --- a/web/src/pages/admin/autoscan/sourceDescriptor.test.ts +++ b/web/src/pages/admin/autoscan/sourceDescriptor.test.ts @@ -14,6 +14,8 @@ import { initialConfigValues, needsConnectionStep, needsDeliveryChoice, + parseConfigValues, + serializeConfigValues, } from "./sourceDescriptor"; function available(descriptor?: Partial): AutoscanAvailableSource { @@ -153,7 +155,8 @@ describe("initialConfigValues", () => { ], }, }); - expect(got).toEqual({ a: "x", b: "2" }); + // Values stay typed here; stringifying happens once, at submit. + expect(got).toEqual({ a: "x", b: 2 }); }); it("returns nothing for a source with no config form", () => { @@ -161,6 +164,68 @@ describe("initialConfigValues", () => { }); }); +describe("serializeConfigValues / parseConfigValues", () => { + const descriptor: AutoscanScanSourceDescriptor = { + delivery_modes: ["poll"], + connection: "none", + config_form: { + fields: [ + { + key: "on", + label: "On", + control: "SWITCH", + required: false, + secret: false, + multiline: false, + }, + { + key: "tags", + label: "Tags", + control: "MULTI_SELECT", + required: false, + secret: false, + multiline: false, + }, + { + key: "count", + label: "Count", + control: "NUMBER", + required: false, + secret: false, + multiline: false, + }, + ], + }, + }; + + // Stringifying on every change turned `false` into the truthy "false" (the + // switch re-rendered enabled) and an array into a join the renderer read back + // as no selection. + it("round-trips a false switch without flipping it", () => { + const stored = serializeConfigValues({ on: false }); + expect(stored).toEqual({ on: "false" }); + expect(parseConfigValues(descriptor, stored).on).toBe(false); + }); + + it("round-trips a multi-select as an array", () => { + const stored = serializeConfigValues({ tags: ["a", "b"] }); + expect(stored).toEqual({ tags: "a,b" }); + expect(parseConfigValues(descriptor, stored).tags).toEqual(["a", "b"]); + }); + + it("round-trips an empty multi-select", () => { + expect(parseConfigValues(descriptor, { tags: "" }).tags).toEqual([]); + }); + + it("round-trips a number", () => { + expect(parseConfigValues(descriptor, serializeConfigValues({ count: 4 })).count).toBe(4); + }); + + it("drops null and undefined rather than writing them as text", () => { + expect(serializeConfigValues({ a: null, b: undefined, c: "keep" })).toEqual({ c: "keep" }); + }); +}); + describe("fillValueFromLibraries", () => { const libraries = [ { id: 1, name: "Movies", type: "movie", enabled: true, paths: ["/mnt/movies"] }, diff --git a/web/src/pages/admin/autoscan/sourceDescriptor.ts b/web/src/pages/admin/autoscan/sourceDescriptor.ts index f4ec3dc1..faecf748 100644 --- a/web/src/pages/admin/autoscan/sourceDescriptor.ts +++ b/web/src/pages/admin/autoscan/sourceDescriptor.ts @@ -96,11 +96,55 @@ export function configFields(descriptor: AutoscanScanSourceDescriptor): PluginAd */ export function initialConfigValues( descriptor: AutoscanScanSourceDescriptor, -): Record { - const out: Record = {}; +): Record { + const out: Record = {}; for (const field of configFields(descriptor)) { if (field.default_value === undefined || field.default_value === null) continue; - out[field.key] = String(field.default_value); + out[field.key] = field.default_value; + } + return out; +} + +/** + * Serialize a typed form draft into the string map `source_config` stores. + * + * Done once at submit rather than per keystroke: booleans and multi-selects + * must stay typed while the renderer owns them, or `false` round-trips as the + * truthy string "false" and an array collapses into an unreadable join. + */ +export function serializeConfigValues(values: Record): Record { + const out: Record = {}; + for (const [key, value] of Object.entries(values)) { + if (value === undefined || value === null) continue; + out[key] = Array.isArray(value) ? value.map(String).join(",") : String(value); + } + return out; +} + +/** Values as the renderer wants them, from the string map the API returns. */ +export function parseConfigValues( + descriptor: AutoscanScanSourceDescriptor, + stored: Record, +): Record { + const byKey = new Map(configFields(descriptor).map((field) => [field.key, field])); + const out: Record = {}; + + for (const [key, value] of Object.entries(stored)) { + switch (byKey.get(key)?.control) { + case "SWITCH": + out[key] = value === "true"; + break; + case "MULTI_SELECT": + out[key] = value ? value.split(",").filter(Boolean) : []; + break; + case "NUMBER": { + const parsed = Number(value); + out[key] = value !== "" && Number.isFinite(parsed) ? parsed : value; + break; + } + default: + out[key] = value; + } } return out; } diff --git a/web/src/pages/admin/autoscan/sourceTargets.ts b/web/src/pages/admin/autoscan/sourceTargets.ts index ffd35b44..13fbf13e 100644 --- a/web/src/pages/admin/autoscan/sourceTargets.ts +++ b/web/src/pages/admin/autoscan/sourceTargets.ts @@ -95,11 +95,16 @@ export function sourceTargets( return { libraries: [], unresolvable: true }; } - const matched = libraries.filter((library) => - (library.paths ?? []).some((root) => { - const normalizedRoot = normalizePath(root); - return paths.some((path) => isWithin(path, normalizedRoot)); - }), + // Disabled libraries are skipped by the scanner, so counting one as a target + // would present a source as correctly wired while every delivery it resolves + // there is dropped. + const matched = libraries.filter( + (library) => + library.enabled && + (library.paths ?? []).some((root) => { + const normalizedRoot = normalizePath(root); + return paths.some((path) => isWithin(path, normalizedRoot)); + }), ); // Paths exist but match no library root. Not "unresolvable" in the sense diff --git a/web/src/pages/admin/autoscan/webhookSetup.test.ts b/web/src/pages/admin/autoscan/webhookSetup.test.ts index 28fbead2..26601273 100644 --- a/web/src/pages/admin/autoscan/webhookSetup.test.ts +++ b/web/src/pages/admin/autoscan/webhookSetup.test.ts @@ -4,6 +4,8 @@ import type { Library } from "@/api/types"; import { collapseToRoots, + expandedRootsFor, + newMapping, hasUsableMapping, seedMappings, settingsPathFor, @@ -114,6 +116,32 @@ describe("collapseToRoots", () => { }); }); +describe("expandedRootsFor", () => { + // A collapsed row assumes the arr mirrors Silo's tree below the root. When it + // does not (/downloads/films vs /downloads/series), the operator needs a row + // per branch — these are the branches the editor offers. + it("offers the distinct child directories under a collapsed root", () => { + expect( + expandedRootsFor("/mnt/media", [ + "/mnt/media/movies/00s", + "/mnt/media/movies/10s", + "/mnt/media/tv/anime", + ]).sort(), + ).toEqual(["/mnt/media/movies", "/mnt/media/tv"]); + }); + + it("offers nothing when every path shares one child", () => { + // Splitting here would produce the same single rule, so it is not an option. + expect( + expandedRootsFor("/mnt/media", ["/mnt/media/movies/00s", "/mnt/media/movies/10s"]), + ).toEqual([]); + }); + + it("ignores paths outside the root", () => { + expect(expandedRootsFor("/mnt/media", ["/srv/other/movies", "/srv/other/tv"])).toEqual([]); + }); +}); + describe("seedMappings", () => { // These libraries all sit under /mnt/media, so each provider's selection // collapses to that single shared root — one row, not one per library. @@ -176,15 +204,15 @@ describe("seedMappings", () => { describe("usableMappings", () => { it("keeps only rows with both sides filled", () => { const got = usableMappings([ - { from: "/downloads/tv", to: "/mnt/media/tv" }, - { from: "", to: "/mnt/media/movies" }, - { from: "/downloads/movies", to: "" }, + newMapping("/mnt/media/tv", "/downloads/tv"), + newMapping("/mnt/media/movies", ""), + newMapping("", "/downloads/movies"), ]); expect(got).toEqual([{ from: "/downloads/tv", to: "/mnt/media/tv" }]); }); it("trims surrounding whitespace", () => { - expect(usableMappings([{ from: " /a ", to: " /b " }])).toEqual([{ from: "/a", to: "/b" }]); + expect(usableMappings([newMapping(" /b ", " /a ")])).toEqual([{ from: "/a", to: "/b" }]); }); }); @@ -192,15 +220,15 @@ describe("hasUsableMapping", () => { // A webhook source with no mapping accepts deliveries and resolves nothing — // the silent failure the guided flow exists to prevent. it("is false when every row is incomplete", () => { - expect(hasUsableMapping([{ from: "", to: "/mnt/media/tv" }])).toBe(false); + expect(hasUsableMapping([newMapping("/mnt/media/tv", "")])).toBe(false); expect(hasUsableMapping([])).toBe(false); }); it("is true once one row is complete", () => { expect( hasUsableMapping([ - { from: "", to: "/mnt/media/movies" }, - { from: "/downloads/tv", to: "/mnt/media/tv" }, + newMapping("/mnt/media/movies", ""), + newMapping("/mnt/media/tv", "/downloads/tv"), ]), ).toBe(true); }); diff --git a/web/src/pages/admin/autoscan/webhookSetup.ts b/web/src/pages/admin/autoscan/webhookSetup.ts index f8a49c04..a3c4dae3 100644 --- a/web/src/pages/admin/autoscan/webhookSetup.ts +++ b/web/src/pages/admin/autoscan/webhookSetup.ts @@ -1,4 +1,4 @@ -import type { AutoscanWebhookProvider, Library } from "@/api/types"; +import type { AutoscanPathRewrite, AutoscanWebhookProvider, Library } from "@/api/types"; /** * The arr notification triggers Silo actually consumes. @@ -93,10 +93,24 @@ export function settingsPathFor(provider: AutoscanWebhookProvider | "auto"): str * paths and the operator supplies the `from` side. */ export interface MappingDraft { + /** + * Stable client-side identity for React keys. Rows are reorderable and + * removable, so keying on array index would remap DOM nodes by position and + * throw focus (and IME composition) into the wrong field mid-edit. + */ + id: string; from: string; to: string; } +let mappingSeq = 0; + +/** Build a draft row with a fresh client-side id. */ +export function newMapping(to = "", from = ""): MappingDraft { + mappingSeq += 1; + return { id: `map-${mappingSeq}`, from, to }; +} + function libraryKind(type: string): "movie" | "tv" | "mixed" | null { switch (type.trim().toLowerCase()) { case "movie": @@ -118,14 +132,22 @@ function libraryKind(type: string): "movie" | "tv" | "mixed" | null { /** * Collapse paths to their shared ancestors. * - * The host applies rewrites by longest matching prefix at a segment boundary - * (see applyRewrites in internal/autoscan/rewrite.go), so a rule for - * `/mnt/media` already covers `/mnt/media/movies/80s` and everything else - * beneath it. Emitting a row per library path is therefore pure noise: a real - * library of 96 paths collapses to 4 roots, 85 of them under one. + * A rewrite replaces a matched prefix and keeps the remaining suffix verbatim + * (see applyRewrites in internal/autoscan/rewrite.go), so one rule for + * `/mnt/media` covers `/mnt/media/movies/80s` and everything else beneath it — + * but only when the arr-side tree mirrors the Silo-side tree below that point. * - * Only paths that are genuinely nested under another are dropped — siblings - * survive, because no rewrite rule would otherwise cover them. + * That holds when a single mount is involved, which is the common case: a real + * install here has 96 library paths across 2 mounts, so per-path rows would be + * 96 lines of near-identical noise. It does NOT hold when the operator has to + * supply different arr-side roots per branch (`/downloads/films` for movies, + * `/downloads/series` for TV) — collapsing those to one row would silently + * mis-map one of them. + * + * The seeded rows are a starting point, not a constraint: an operator whose arr + * exposes each branch under a different root can delete the collapsed row and + * add one per branch. `expandedRootsFor` supplies that breakdown so the editor + * can offer it rather than making them retype paths. */ export function collapseToRoots(paths: readonly string[]): string[] { const cleaned = paths @@ -136,8 +158,9 @@ export function collapseToRoots(paths: readonly string[]): string[] { if (unique.length === 0) return []; // Group by mount point — the first two segments, e.g. "/mnt/sharedrives". - // Paths under different mounts are genuinely different storage and must not - // be merged, but everything under one mount can share a single rewrite. + // Paths under different mounts are separate storage and must never merge, + // but one mount's tree is almost always exposed to the arr as a single + // remapped volume, so it needs one rule rather than one per leaf. const groups = new Map(); for (const path of unique) { const segments = path.split("/").filter(Boolean); @@ -149,11 +172,44 @@ export function collapseToRoots(paths: readonly string[]): string[] { const roots: string[] = []; for (const [mount, members] of groups) { + // A lone member is left exactly as supplied — widening it would claim more + // of the filesystem than the operator actually configured. roots.push(members.length === 1 ? members[0]! : commonAncestor(members) || mount); } return roots; } +/** + * The next level down from a collapsed root: the distinct child directories the + * supplied paths actually live under. + * + * A rewrite keeps the suffix after the matched prefix, so one row per mount is + * only correct while the arr sees the same tree below that point. When it does + * not — `/downloads/films` for movies but `/downloads/series` for TV — the + * operator needs a row per branch. Offering these lets the editor expand a + * collapsed row instead of asking them to retype the Silo side. + * + * Returns [] when there is nothing more specific to offer. + */ +export function expandedRootsFor(root: string, paths: readonly string[]): string[] { + const prefix = root.replace(/\/+$/, ""); + const children = new Set(); + + for (const path of paths) { + const cleaned = path.trim().replace(/\/+$/, ""); + if (!cleaned.startsWith(`${prefix}/`)) continue; + const rest = cleaned + .slice(prefix.length + 1) + .split("/") + .filter(Boolean); + const next = rest[0]; + if (next) children.add(`${prefix}/${next}`); + } + + // One child is the same rule as the root, so it is not an alternative. + return children.size > 1 ? [...children] : []; +} + /** * Longest path prefix shared by every input, truncated at a segment boundary. * Returns "" when the inputs share nothing below the root. @@ -199,7 +255,7 @@ export function seedMappings( } } - return collapseToRoots(paths).map((to) => ({ from: "", to })); + return collapseToRoots(paths).map((to) => newMapping(to)); } /** @@ -207,7 +263,7 @@ export function seedMappings( * configured" rather than an error, so an operator can leave one blank and * still save the rest. */ -export function usableMappings(drafts: readonly MappingDraft[]): MappingDraft[] { +export function usableMappings(drafts: readonly MappingDraft[]): AutoscanPathRewrite[] { return drafts .map((draft) => ({ from: draft.from.trim(), to: draft.to.trim() })) .filter((draft) => draft.from.length > 0 && draft.to.length > 0);