Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/dashboard-route-app-segment.md
Original file line number Diff line number Diff line change
@@ -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 `<collection>/<name>` 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.
86 changes: 85 additions & 1 deletion packages/lint/src/validate-dashboard-action-refs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
99 changes: 61 additions & 38 deletions packages/lint/src/validate-dashboard-action-refs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
* `<collection>/<name>` 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
* `<collection>/<name>` 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.
Expand Down Expand Up @@ -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<string, 'objects' | 'reports' | 'dashboards' | 'pages' | 'views'> = {
* (`/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',
Expand Down Expand Up @@ -141,6 +151,10 @@ function viewContainerName(item: AnyRec): string | undefined {
interface KnownTargets {
/** Every action name defined in the stack (global + object-embedded). */
actions: Set<string>;
/** App machine names (valid in `apps/<name>` routes) — the same `name`
* identity the runtime resolves `/apps/:appName/…` against and REST's
* `GET /meta/apps/:name` reads by. */
apps: Set<string>;
/** Object names (valid in `objects/<name>` routes). */
objects: Set<string>;
reports: Set<string>;
Expand All @@ -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<string>();
const apps = new Set<string>();
const objects = new Set<string>();
const reports = new Set<string>();
const dashboards = new Set<string>();
Expand All @@ -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);
Expand All @@ -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? */
Expand All @@ -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 `<collection>/<name>` segment) — SKIP, no finding.
* - `{ collection, name }` for a recognized `<collection>/<name>` 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 `<collection>/<name>` 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 {
Expand Down Expand Up @@ -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).
Expand Down
Loading