fix(autoscan): address PR review findings

Codex and CodeRabbit review of #515. Each finding verified against the code
before acting; two were declined with reasons recorded below.

Correctness:
- source_config is plaintext JSONB returned verbatim by the source API, unlike
  connection keys which take the repository's encrypted path. Drop secret and
  PASSWORD fields from scan-source config forms rather than rendering a masked
  input over a value stored in the clear. Plugins needing a credential should
  take a connection.
- Plugin-declared webhook delivery is dropped at parse time. resolveDeliveryMode
  accepts webhook only for the built-in ARR identity, so honouring the claim
  offered a setup path whose every submission ended in HTTP 400. The built-in
  supplies its descriptor directly, so it is unaffected.
- Compat lookup now requires plugin id AND capability id. Capability ids are
  author-chosen and not unique, so an OR handed CephFS's path/exclusion form to
  any plugin naming a capability "cephfs".
- Config values stay typed until submit. Stringifying every change turned a
  false switch into the truthy "false" (re-rendering it enabled) and a
  multi-select array into a join the renderer read back as no selection.
- Creation is gated on config-form validity. The host stores source_config
  without interpreting it, so required and validated fields are only enforced
  client-side; the dialog previously ignored the renderer's verdict.
- Row editing now honours the descriptor it was already given: a `required`
  source cannot be unbound, a `none` source shows no picker, and only
  compatible connection kinds are offered.
- sourceTargets skips disabled libraries. The scanner rejects them, so counting
  one as a target showed a source as wired while its deliveries were dropped.
- Inline reuse compares request_integration_id, not connection id — different
  identifier spaces, so the dedupe matched nothing and could create a second
  connection to the same server. Disabled integrations are also excluded, since
  they are rejected at poll time.
- Webhook setup instructions follow the operator's chosen provider rather than
  the descriptor, which advertises both kinds and so always resolved to "auto".
- Webhook rows render their plugin-declared config form (the built-in's
  provider field stays with the endpoint section, not duplicated).

Declined:
- Collapsing sibling paths to a common ancestor is kept. It is what takes a
  real install's 96 library paths to 2 rows. The reviewer is right that one
  rule cannot serve branches the arr exposes under different roots, so the
  editor now offers to split a collapsed row into one row per branch
  (expandedRootsFor) instead of forcing the operator to retype paths.
- Legacy source_config keys are still migrated away on first save; the comment
  claiming both keys are written was wrong and has been corrected to match.

Also: stable ids on mapping rows so removing one does not throw focus into a
neighbouring field, memoized config fields, and descriptor marked optional in
the TS type to match descriptorFor's documented fallback.
This commit is contained in:
Quick
2026-07-29 14:52:16 -04:00
parent 616d262e99
commit bcea996172
14 changed files with 505 additions and 113 deletions
+25 -1
View File
@@ -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
}
+4 -1
View File
@@ -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
+8 -1
View File
@@ -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;
+60 -3
View File
@@ -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)
+2 -1
View File
@@ -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 {
@@ -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<AutoscanConnectionTestResult | null>(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;
@@ -22,15 +22,25 @@ export function SourceConfigForm({
values,
onChange,
idPrefix = "source-config",
onValidityChange,
}: {
descriptor: AutoscanScanSourceDescriptor;
values: Record<string, string>;
onChange: (next: Record<string, string>) => 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<string, unknown>;
onChange: (next: Record<string, unknown>) => 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({
<SchemaForm
descriptor={form}
values={values}
onChange={(next) => {
const out: Record<string, string> = {};
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 && (
+80 -24
View File
@@ -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<string, string>;
/** Typed while the renderer owns them; serialized only on save. */
sourceConfig: Record<string, unknown>;
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<string, string>,
): 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<string, string> = {
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<string, string>;
globalPollInterval: number | null;
onDelete: (source: AutoscanSource) => void;
@@ -711,7 +727,7 @@ function SourceRow({
}) {
const update = useUpdateAutoscanSource();
const libraries = useAdminLibraries();
const [edit, setEdit] = useState<RowEdit>(() => sourceToRowEdit(source));
const [edit, setEdit] = useState<RowEdit>(() => 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<string, string>) {
function handleSourceConfigSave(nextConfig?: Record<string, unknown>) {
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({
</span>
);
// 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 ? (
<span className="text-muted-foreground text-xs">
Not needed — Sonarr/Radarr deliver directly
</span>
) : !needsConnectionStep(descriptor, source.delivery_mode) ? (
<span className="text-muted-foreground text-xs">Not needed — reads locally</span>
) : source.connection_id === null && !edit.connectionId ? (
<div className="flex max-w-full min-w-0 flex-col gap-2 sm:flex-row sm:items-center">
<Select value="__none__" onValueChange={handleConnectionChange}>
@@ -943,16 +969,23 @@ function SourceRow({
<SelectValue placeholder="No connection" />
</SelectTrigger>
<SelectContent>
<SelectItem value="__none__">— No connection —</SelectItem>
{connectionOptions.map((c) => (
{!rowConnectionRequired && <SelectItem value="__none__">— No connection —</SelectItem>}
{rowEligibleConnections.map((c) => (
<SelectItem key={c.id} value={c.id}>
{c.name}
</SelectItem>
))}
</SelectContent>
</Select>
<Badge variant="outline" className="text-muted-foreground w-fit shrink-0">
No connection
<Badge
variant="outline"
className={
rowConnectionRequired
? "text-destructive w-fit shrink-0"
: "text-muted-foreground w-fit shrink-0"
}
>
{rowConnectionRequired ? "Connection required" : "No connection"}
</Badge>
</div>
) : (
@@ -964,8 +997,8 @@ function SourceRow({
<SelectValue placeholder="No connection" />
</SelectTrigger>
<SelectContent>
<SelectItem value="__none__">— No connection —</SelectItem>
{connectionOptions.map((c) => (
{!rowConnectionRequired && <SelectItem value="__none__">— No connection —</SelectItem>}
{rowEligibleConnections.map((c) => (
<SelectItem key={c.id} value={c.id}>
{c.name}
</SelectItem>
@@ -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 ? (
<div className="border-border mt-3 space-y-3 rounded-md border p-3">
<SourceConfigForm
descriptor={descriptor}
descriptor={{
...descriptor,
config_form: { ...descriptor.config_form, fields: rowConfigFields },
}}
values={edit.sourceConfig}
onChange={(next) => setEdit((ed) => ({ ...ed, sourceConfig: next }))}
onValidityChange={(configValid) => setEdit((ed) => ({ ...ed, configValid }))}
idPrefix={`source-config-${source.id}`}
/>
<Button
type="button"
variant="outline"
size="sm"
disabled={update.isPending}
disabled={update.isPending || !edit.configValid}
onClick={() => handleSourceConfigSave()}
>
{update.isPending ? "Saving…" : "Save configuration"}
@@ -1021,6 +1064,9 @@ function SourceRow({
isSaving={update.isPending}
/>
{rewriteEditor}
{/* Webhook sources can carry declared config too — the built-in's provider
is handled above, but a plugin-supplied form must still render. */}
{sourceConfigEditor}
</>
) : (
<>
@@ -1160,7 +1206,10 @@ interface AddSourceForm {
deliveryMode: AutoscanDeliveryMode | "";
connectionId: string; // "" / "__none__" means no connection
intervalStr: string;
sourceConfig: Record<string, string>;
/** Typed while the renderer owns them; serialized only on submit. */
sourceConfig: Record<string, unknown>;
/** False while a required or validated config field is unsatisfied. */
configValid: boolean;
/** Webhook-only: arr path -> Silo path rows, seeded from library paths. */
mappings: MappingDraft[];
}
@@ -1171,6 +1220,7 @@ const BLANK_ADD_SOURCE: AddSourceForm = {
connectionId: "",
intervalStr: "",
sourceConfig: {},
configValid: true,
mappings: [],
};
@@ -1312,7 +1362,7 @@ function AddSourceDialog({
delivery_mode: deliveryMode,
poll_interval_seconds: pollInterval,
path_rewrites: isWebhookFlow ? usableMappings(form.mappings) : [],
source_config: normalizeSourceConfig(form.sourceConfig),
source_config: normalizeSourceConfig(serializeConfigValues(form.sourceConfig)),
},
{
onSuccess: (created) => {
@@ -1352,7 +1402,10 @@ function AddSourceDialog({
(!connectionRequired || Boolean(form.connectionId && form.connectionId !== "__none__")) &&
// A webhook source with no mapping accepts deliveries and resolves nothing,
// so require at least one before it can be created.
(!isWebhookFlow || hasUsableMapping(form.mappings));
(!isWebhookFlow || hasUsableMapping(form.mappings)) &&
// The host stores source_config without interpreting it, so a plugin's
// required/validated fields are only enforced here.
form.configValid;
return (
<Dialog open={open} onOpenChange={(o) => (o ? onOpenChange(true) : close())}>
@@ -1374,7 +1427,7 @@ function AddSourceDialog({
{createdWebhookSource.webhook_url ? (
<WebhookInstructions
url={absoluteWebhookURL(createdWebhookSource.webhook_url)}
provider={webhookProviderOf(descriptor)}
provider={webhookProviderOf(descriptor, createdWebhookSource.source_config)}
/>
) : (
<div className="border-destructive/30 bg-destructive/10 space-y-3 rounded-md border p-3">
@@ -1470,6 +1523,7 @@ function AddSourceDialog({
<WebhookMappingEditor
mappings={form.mappings}
onChange={(mappings) => setForm((f) => ({ ...f, mappings }))}
libraryPaths={(libraries.data ?? []).flatMap((library) => library.paths ?? [])}
/>
)}
@@ -1512,6 +1566,7 @@ function AddSourceDialog({
descriptor={descriptor}
values={form.sourceConfig}
onChange={(sourceConfig) => setForm((f) => ({ ...f, sourceConfig }))}
onValidityChange={(configValid) => setForm((f) => ({ ...f, configValid }))}
idPrefix="add-source-config"
/>
)}
@@ -1563,6 +1618,7 @@ export default function SourcesPanel() {
id: c.id,
name: c.name,
kind: c.kind,
requestIntegrationId: c.request_integration_id ?? null,
}));
const globalPollInterval = settings.data?.default_poll_interval_seconds ?? null;
@@ -8,7 +8,7 @@ import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
import type { MappingDraft } from "./webhookSetup";
import { settingsPathFor, triggersFor } from "./webhookSetup";
import { expandedRootsFor, newMapping, settingsPathFor, triggersFor } from "./webhookSetup";
/**
* The copy-the-URL-into-your-arr half of webhook setup.
@@ -114,14 +114,32 @@ export function WebhookInstructions({
export function WebhookMappingEditor({
mappings,
onChange,
libraryPaths = [],
}: {
mappings: MappingDraft[];
onChange: (next: MappingDraft[]) => void;
/**
* Every library path the seeded rows were derived from. Used to offer a
* per-branch breakdown when a row was collapsed to a shared root but the
* operator's arr exposes those branches under different roots.
*/
libraryPaths?: readonly string[];
}) {
function update(index: number, patch: Partial<MappingDraft>) {
onChange(mappings.map((row, i) => (i === index ? { ...row, ...patch } : row)));
}
/** Replace a collapsed row with one row per child directory. */
function expand(index: number, children: string[]) {
const row = mappings[index];
if (!row) return;
onChange([
...mappings.slice(0, index),
...children.map((to) => newMapping(to, row.from)),
...mappings.slice(index + 1),
]);
}
return (
<div className="space-y-3">
<div className="space-y-1">
@@ -138,45 +156,60 @@ export function WebhookMappingEditor({
</p>
) : (
<div className="space-y-2">
{mappings.map((row, index) => (
<div key={index} className="flex flex-col gap-2 sm:flex-row sm:items-end">
<div className="min-w-0 flex-1 space-y-1">
<Label htmlFor={`map-from-${index}`} className="text-muted-foreground text-xs">
Path your download manager reports
</Label>
<Input
id={`map-from-${index}`}
placeholder="/downloads/tv"
className="font-mono text-xs"
value={row.from}
onChange={(e) => update(index, { from: e.target.value })}
/>
{mappings.map((row, index) => {
const children = expandedRootsFor(row.to, libraryPaths);
return (
<div key={row.id} className="space-y-1">
<div className="flex flex-col gap-2 sm:flex-row sm:items-end">
<div className="min-w-0 flex-1 space-y-1">
<Label htmlFor={`map-from-${index}`} className="text-muted-foreground text-xs">
Path your download manager reports
</Label>
<Input
id={`map-from-${index}`}
placeholder="/downloads/tv"
className="font-mono text-xs"
value={row.from}
onChange={(e) => update(index, { from: e.target.value })}
/>
</div>
<span className="text-muted-foreground hidden pb-2 text-xs sm:block">→</span>
<div className="min-w-0 flex-1 space-y-1">
<Label htmlFor={`map-to-${index}`} className="text-muted-foreground text-xs">
Path Silo uses
</Label>
<Input
id={`map-to-${index}`}
placeholder="/mnt/media/tv"
className="font-mono text-xs"
value={row.to}
onChange={(e) => update(index, { to: e.target.value })}
/>
</div>
<Button
type="button"
variant="ghost"
size="icon-sm"
aria-label={`Remove mapping ${index + 1}`}
className="sm:mb-1"
onClick={() => onChange(mappings.filter((_, i) => i !== index))}
>
<Trash2 className="text-destructive" />
</Button>
</div>
{children.length > 0 && (
<button
type="button"
className="text-muted-foreground hover:text-foreground text-xs underline-offset-4 hover:underline"
onClick={() => expand(index, children)}
>
Does your download manager use a different folder per type? Split into{" "}
{children.length} rows
</button>
)}
</div>
<span className="text-muted-foreground hidden pb-2 text-xs sm:block">→</span>
<div className="min-w-0 flex-1 space-y-1">
<Label htmlFor={`map-to-${index}`} className="text-muted-foreground text-xs">
Path Silo uses
</Label>
<Input
id={`map-to-${index}`}
placeholder="/mnt/media/tv"
className="font-mono text-xs"
value={row.to}
onChange={(e) => update(index, { to: e.target.value })}
/>
</div>
<Button
type="button"
variant="ghost"
size="icon-sm"
aria-label={`Remove mapping ${index + 1}`}
className="sm:mb-1"
onClick={() => onChange(mappings.filter((_, i) => i !== index))}
>
<Trash2 className="text-destructive" />
</Button>
</div>
))}
);
})}
</div>
)}
@@ -184,7 +217,7 @@ export function WebhookMappingEditor({
type="button"
variant="outline"
size="sm"
onClick={() => onChange([...mappings, { from: "", to: "" }])}
onClick={() => onChange([...mappings, newMapping()])}
>
<Plus />
Add a mapping
@@ -14,6 +14,8 @@ import {
initialConfigValues,
needsConnectionStep,
needsDeliveryChoice,
parseConfigValues,
serializeConfigValues,
} from "./sourceDescriptor";
function available(descriptor?: Partial<AutoscanScanSourceDescriptor>): 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"] },
@@ -96,11 +96,55 @@ export function configFields(descriptor: AutoscanScanSourceDescriptor): PluginAd
*/
export function initialConfigValues(
descriptor: AutoscanScanSourceDescriptor,
): Record<string, string> {
const out: Record<string, string> = {};
): Record<string, unknown> {
const out: Record<string, unknown> = {};
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<string, unknown>): Record<string, string> {
const out: Record<string, string> = {};
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<string, string>,
): Record<string, unknown> {
const byKey = new Map(configFields(descriptor).map((field) => [field.key, field]));
const out: Record<string, unknown> = {};
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;
}
+10 -5
View File
@@ -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
@@ -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);
});
+68 -12
View File
@@ -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<string, string[]>();
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<string>();
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);