From 93f2dd2f66a33cd2b8659cadd06c29a2d21d9433 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 23:09:23 +0000 Subject: [PATCH] fix(lint): dashboard-action-route-unresolved resolves the apps/NAME head and every later segment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit URL_COLLECTION_TO_STACK_KEY had no `apps` entry, so a dashboard header action's url target like `/apps/no_such_app_nope/crm_lead` was never checked at all — a button pointing at an app that does not exist passed lint clean. The loop also returned at the first recognized collection segment, resolved or not, so a bad app name combined with a bad later segment (e.g. a bad dashboard name) reported only the later one. Resolves the apps/NAME head against stack.apps (keyed by name, the same identity the runtime's /apps/:appName route and REST's GET /meta/apps/:name read by), and scans every segment of the path instead of stopping at the first recognized one — one finding per unresolved / pair. Behavior change on paths that used to pass clean: a path where an earlier segment resolves and a later one does not (e.g. /dashboards/exec/views/ bad_view) now reports the later segment instead of staying silent. Called out as its own changeset bullet. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8 --- .changeset/dashboard-route-app-segment.md | 9 ++ .../validate-dashboard-action-refs.test.ts | 86 +++++++++++++++- .../src/validate-dashboard-action-refs.ts | 99 ++++++++++++------- 3 files changed, 155 insertions(+), 39 deletions(-) create mode 100644 .changeset/dashboard-route-app-segment.md diff --git a/.changeset/dashboard-route-app-segment.md b/.changeset/dashboard-route-app-segment.md new file mode 100644 index 0000000000..ccaf5d1b02 --- /dev/null +++ b/.changeset/dashboard-route-app-segment.md @@ -0,0 +1,9 @@ +--- +"@objectstack/lint": patch +--- + +`dashboard-action-route-unresolved` now resolves the `apps/NAME` head of a dashboard header action's `url` target against `stack.apps`, and reports every unresolved `/` segment in the path rather than stopping at the first one it recognizes. + +Before this, `URL_COLLECTION_TO_STACK_KEY` had no `apps` entry, so an `actionUrl` like `/apps/no_such_app_nope/crm_lead` was never checked at all — a dashboard button pointing at an app that does not exist passed lint clean. Worse, once a bad app name was combined with a second bad segment later in the same path (e.g. `/apps/no_such_app_nope/dashboard/no_such_dashboard_nope`), the old loop returned at the FIRST recognized segment and reported only that one — so a bad app name plus a bad dashboard name reported only the dashboard, never the app. + +**Behavior change on paths that used to pass clean:** the loop no longer stops scanning a path the moment it recognizes one collection segment, resolved or not. A path like `/dashboards/exec/views/bad_view` — where `exec` is a real dashboard but `bad_view` names no view — used to report nothing (the loop returned as soon as `dashboards/exec` resolved, never reaching `views/bad_view`); it now reports one warning on the `views/bad_view` segment. Any stack with a dashboard `url` action whose path recognizes a valid collection segment followed later by an unresolved one will see a NEW warning here that did not fire before. This is intentional — it is the same false-affordance category the rule already exists to catch — but it is a real, visible change to what a clean `lint` run reports on such stacks, not a pure addition. diff --git a/packages/lint/src/validate-dashboard-action-refs.test.ts b/packages/lint/src/validate-dashboard-action-refs.test.ts index 5bb19b936b..0877427d71 100644 --- a/packages/lint/src/validate-dashboard-action-refs.test.ts +++ b/packages/lint/src/validate-dashboard-action-refs.test.ts @@ -152,15 +152,99 @@ describe('validateDashboardActionRefs (ADR-0049 references / #3367)', () => { }); it('resolves an object route embedded mid-path (app-scoped route)', () => { + // Both segments must resolve now that `apps/NAME` is itself checked + // (#16169) — the app segment is no longer a free pass just because a + // later segment in the same path resolves. const findings = validateDashboardActionRefs( dashWithHeaderActions( [{ label: 'Deals', actionType: 'url', actionUrl: '/apps/crm/objects/deal' }], - { objects: [{ name: 'deal' }] }, + { apps: [{ name: 'crm' }], objects: [{ name: 'deal' }] }, ), ); expect(findings).toEqual([]); }); + // #16169 — `resolveUrlRoute` used to `continue` past every unrecognized + // segment and return at the FIRST recognized one, so an `apps/NAME` head + // was never checked (`apps` was absent from `URL_COLLECTION_TO_STACK_KEY`) + // and a bad app name plus a bad later segment reported only the later one. + it('WARNS on a url action pointing at a non-existent app (#16169 — the reported bug)', () => { + const findings = validateDashboardActionRefs( + dashWithHeaderActions( + [{ label: 'Open', actionType: 'url', actionUrl: '/apps/no_such_app_nope/crm_lead' }], + { apps: [{ name: 'crm_enterprise' }] }, + ), + ); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'warning', + rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED, + path: 'dashboards[0].header.actions[0].actionUrl', + }); + expect(findings[0].message).toContain('apps/no_such_app_nope'); + expect(findings[0].message).toContain('app named "no_such_app_nope"'); + }); + + it('WARNS on EACH unresolved segment of a url with more than one bad segment (#16169)', () => { + const findings = validateDashboardActionRefs( + dashWithHeaderActions([ + { + label: 'Open', + actionType: 'url', + actionUrl: '/apps/no_such_app_nope/dashboard/no_such_dashboard_nope', + }, + ]), + ); + expect(findings).toHaveLength(2); + expect(findings.every((f) => f.severity === 'warning')).toBe(true); + expect(findings.every((f) => f.rule === DASHBOARD_ACTION_ROUTE_UNRESOLVED)).toBe(true); + expect(findings.every((f) => f.path === 'dashboards[0].header.actions[0].actionUrl')).toBe(true); + // Path order: the app segment's finding first, the dashboard segment's second. + expect(findings[0].message).toContain('apps/no_such_app_nope'); + expect(findings[1].message).toContain('dashboard/no_such_dashboard_nope'); + }); + + it( + 'WARNS on a later unresolved segment even when an earlier segment resolves ' + + '(#16169 — the boundary the triage flagged: this is a NEW finding, was clean before)', + () => { + // `exec` is a real dashboard; `bad_view` names no view. Before #16169 the + // loop returned at the first recognized segment (`dashboards/exec`, + // resolved) and never reached `views/bad_view` — this path was silent. + const findings = validateDashboardActionRefs( + dashWithHeaderActions([ + { label: 'Open', actionType: 'url', actionUrl: '/dashboards/exec/views/bad_view' }, + ]), + ); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'warning', + rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED, + path: 'dashboards[0].header.actions[0].actionUrl', + }); + expect(findings[0].message).toContain('views/bad_view'); + expect(findings[0].message).toContain('view named "bad_view"'); + }, + ); + + it('passes a fully resolvable app-scoped dashboard route (#16169 — both segments real)', () => { + const findings = validateDashboardActionRefs({ + apps: [{ name: 'real_app' }], + dashboards: [ + { + name: 'exec', + header: { + actions: [ + { label: 'Open', actionType: 'url', actionUrl: '/apps/real_app/dashboard/real_dashboard' }, + ], + }, + }, + { name: 'real_dashboard' }, + ], + }); + expect(findings).toEqual([]); + }); + it('skips external URLs, interpolated targets, and opaque routes (no false positives)', () => { const findings = validateDashboardActionRefs( dashWithHeaderActions([ diff --git a/packages/lint/src/validate-dashboard-action-refs.ts b/packages/lint/src/validate-dashboard-action-refs.ts index 96227dd632..b1754bb75e 100644 --- a/packages/lint/src/validate-dashboard-action-refs.ts +++ b/packages/lint/src/validate-dashboard-action-refs.ts @@ -60,12 +60,16 @@ * names the page `create_opportunity`, or it names nothing. Opening an * object's form is `actionType: 'form'`. Otherwise → ERROR. * - * actionType 'url' → a relative in-app path. WARN when a recognizable - * `/` segment (objects/reports/dashboards/pages/views) - * names an entity that does not exist in this stack. External URLs - * (`http(s)://`, `//`), interpolated targets (`${…}`), and opaque routes - * (no recognized collection segment) are skipped — they cannot be resolved - * statically and may be host/app/plugin routes. → WARNING. + * actionType 'url' → a relative in-app path. WARN once per recognizable + * `/` segment (apps/objects/reports/dashboards/pages/ + * views) whose name does not exist in this stack — EVERY such segment in + * the path, not only the first (#16169: a bad `apps/NAME` head used to + * hide a bad `dashboard/NAME` tail, and a bad app name alone was never + * checked at all, since `apps` was absent from the collection table). + * External URLs (`http(s)://`, `//`), interpolated targets (`${…}`), and + * opaque routes (no recognized collection segment anywhere in the path) + * are skipped — they cannot be resolved statically and may be host/plugin + * routes. → WARNING (one finding per unresolved segment). * * actionType 'flow' | 'api' — not checked: flow targets resolve against the * automation engine / other packages, and api targets are opaque endpoints. @@ -111,9 +115,15 @@ function strName(v: unknown): string | undefined { /** URL path segments that name a metadata collection, mapped to the stack key * whose members can appear after them in an in-app route - * (`/…/objects/crm_lead`, `/reports/forecast`, `/dashboards/exec`, …). Both the - * singular and plural spellings are accepted. */ -const URL_COLLECTION_TO_STACK_KEY: Record = { + * (`/apps/crm_enterprise/objects/crm_lead`, `/reports/forecast`, + * `/dashboards/exec`, …). Both the singular and plural spellings are + * accepted. */ +const URL_COLLECTION_TO_STACK_KEY: Record< + string, + 'apps' | 'objects' | 'reports' | 'dashboards' | 'pages' | 'views' +> = { + app: 'apps', + apps: 'apps', object: 'objects', objects: 'objects', report: 'reports', @@ -141,6 +151,10 @@ function viewContainerName(item: AnyRec): string | undefined { interface KnownTargets { /** Every action name defined in the stack (global + object-embedded). */ actions: Set; + /** App machine names (valid in `apps/` routes) — the same `name` + * identity the runtime resolves `/apps/:appName/…` against and REST's + * `GET /meta/apps/:name` reads by. */ + apps: Set; /** Object names (valid in `objects/` routes). */ objects: Set; reports: Set; @@ -153,6 +167,7 @@ interface KnownTargets { /** Build the author-time "known target" sets from a stack. */ function collectKnownTargets(stack: AnyRec): KnownTargets { const actions = new Set(); + const apps = new Set(); const objects = new Set(); const reports = new Set(); const dashboards = new Set(); @@ -168,6 +183,7 @@ function collectKnownTargets(stack: AnyRec): KnownTargets { }; collectNames(stack.actions, actions, (a) => strName(a.name)); + collectNames(stack.apps, apps, (a) => strName(a.name)); for (const obj of recordsOf(stack.objects)) { if (!obj || typeof obj !== 'object') continue; const n = strName(obj.name); @@ -181,7 +197,7 @@ function collectKnownTargets(stack: AnyRec): KnownTargets { // An object's default view is routable by the object's own name too. for (const o of objects) views.add(o); - return { actions, objects, reports, dashboards, pages, views }; + return { actions, apps, objects, reports, dashboards, pages, views }; } /** Does a `script`/`modal` `actionUrl` resolve? */ @@ -202,36 +218,39 @@ function resolveActionTarget( } /** - * Resolve a relative `url` in-app route. Returns: - * - `null` when the target is not statically resolvable (external, interpolated, - * or carries no recognized `/` segment) — SKIP, no finding. - * - `{ collection, name }` for a recognized `/` pair that does - * NOT exist in the stack — WARN. - * - `undefined` when a recognized pair DID resolve — OK, no finding. + * Resolve a relative `url` in-app route. Scans EVERY segment of the path for a + * recognized `/` pair (#16169 — a bad `apps/NAME` head used + * to hide a bad `dashboard/NAME` tail because the loop returned at the first + * recognized segment, resolved or not). Returns one entry per recognized pair + * whose name does NOT exist in the stack, in path order; an empty array means + * either the route is not statically resolvable (external, interpolated, no + * recognized segment anywhere) or every recognized pair resolved — either way, + * no finding. */ function resolveUrlRoute( target: string, known: KnownTargets, -): { collection: string; name: string } | null | undefined { +): { collection: string; name: string }[] { // External / protocol-relative — leaves the app; not an in-app route. - if (/^[a-z][a-z0-9+.-]*:\/\//i.test(target) || target.startsWith('//')) return null; + if (/^[a-z][a-z0-9+.-]*:\/\//i.test(target) || target.startsWith('//')) return []; // Interpolated — resolved by the renderer at click time, not statically known. - if (target.includes('${')) return null; + if (target.includes('${')) return []; // Only relative in-app paths are considered. - if (!target.startsWith('/')) return null; + if (!target.startsWith('/')) return []; // Strip query + hash, then split into non-empty segments. const pathPart = target.split(/[?#]/, 1)[0]; const segments = pathPart.split('/').filter(Boolean); + const unresolved: { collection: string; name: string }[] = []; for (let i = 0; i < segments.length - 1; i++) { const stackKey = URL_COLLECTION_TO_STACK_KEY[segments[i]]; if (!stackKey) continue; const name = segments[i + 1]; - if (known[stackKey].has(name)) return undefined; // resolved - return { collection: segments[i], name }; // recognized shape, unknown name + if (known[stackKey].has(name)) continue; // resolved — keep scanning later segments + unresolved.push({ collection: segments[i], name }); // recognized shape, unknown name } - return null; // no recognized collection segment — opaque route, skip + return unresolved; // empty — opaque route, or every recognized pair resolved } interface HeaderAction { @@ -295,21 +314,25 @@ export function validateDashboardActionRefs(stack: AnyRec): DashboardActionRefFi } if (actionType === 'url') { - const route = resolveUrlRoute(target, known); - if (!route) return; // skip (external/interpolated/opaque) or resolved - findings.push({ - severity: 'warning', - rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED, - where, - path, - message: - `url action target "${target}" points at ${route.collection}/${route.name}, ` + - `but no ${route.collection.replace(/s$/, '')} named "${route.name}" is registered ` + - `in this stack — the button likely navigates to a dead route.`, - hint: - `Check the path for a typo, define the referenced ${route.collection.replace(/s$/, '')}, ` + - `or ignore this if the route is served by another installed package or a host/console route.`, - }); + const unresolved = resolveUrlRoute(target, known); + // One finding per unresolved segment (#16169): a bad `apps/NAME` head and + // a bad `dashboard/NAME` tail on the SAME url are two distinct dead + // references, and each is reported on its own so neither hides the other. + for (const route of unresolved) { + findings.push({ + severity: 'warning', + rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED, + where, + path, + message: + `url action target "${target}" points at ${route.collection}/${route.name}, ` + + `but no ${route.collection.replace(/s$/, '')} named "${route.name}" is registered ` + + `in this stack — the button likely navigates to a dead route.`, + hint: + `Check the path for a typo, define the referenced ${route.collection.replace(/s$/, '')}, ` + + `or ignore this if the route is served by another installed package or a host/console route.`, + }); + } return; } // 'flow' | 'api' | custom types are out of scope (see module header).