Skip to content

Commit 93f2dd2

Browse files
committed
fix(lint): dashboard-action-route-unresolved resolves the apps/NAME head and every later segment
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 <collection>/<name> 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
1 parent 0ea5f9d commit 93f2dd2

3 files changed

Lines changed: 155 additions & 39 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
`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.
6+
7+
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.
8+
9+
**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.

packages/lint/src/validate-dashboard-action-refs.test.ts

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,15 +152,99 @@ describe('validateDashboardActionRefs (ADR-0049 references / #3367)', () => {
152152
});
153153

154154
it('resolves an object route embedded mid-path (app-scoped route)', () => {
155+
// Both segments must resolve now that `apps/NAME` is itself checked
156+
// (#16169) — the app segment is no longer a free pass just because a
157+
// later segment in the same path resolves.
155158
const findings = validateDashboardActionRefs(
156159
dashWithHeaderActions(
157160
[{ label: 'Deals', actionType: 'url', actionUrl: '/apps/crm/objects/deal' }],
158-
{ objects: [{ name: 'deal' }] },
161+
{ apps: [{ name: 'crm' }], objects: [{ name: 'deal' }] },
159162
),
160163
);
161164
expect(findings).toEqual([]);
162165
});
163166

167+
// #16169 — `resolveUrlRoute` used to `continue` past every unrecognized
168+
// segment and return at the FIRST recognized one, so an `apps/NAME` head
169+
// was never checked (`apps` was absent from `URL_COLLECTION_TO_STACK_KEY`)
170+
// and a bad app name plus a bad later segment reported only the later one.
171+
it('WARNS on a url action pointing at a non-existent app (#16169 — the reported bug)', () => {
172+
const findings = validateDashboardActionRefs(
173+
dashWithHeaderActions(
174+
[{ label: 'Open', actionType: 'url', actionUrl: '/apps/no_such_app_nope/crm_lead' }],
175+
{ apps: [{ name: 'crm_enterprise' }] },
176+
),
177+
);
178+
expect(findings).toHaveLength(1);
179+
expect(findings[0]).toMatchObject({
180+
severity: 'warning',
181+
rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED,
182+
path: 'dashboards[0].header.actions[0].actionUrl',
183+
});
184+
expect(findings[0].message).toContain('apps/no_such_app_nope');
185+
expect(findings[0].message).toContain('app named "no_such_app_nope"');
186+
});
187+
188+
it('WARNS on EACH unresolved segment of a url with more than one bad segment (#16169)', () => {
189+
const findings = validateDashboardActionRefs(
190+
dashWithHeaderActions([
191+
{
192+
label: 'Open',
193+
actionType: 'url',
194+
actionUrl: '/apps/no_such_app_nope/dashboard/no_such_dashboard_nope',
195+
},
196+
]),
197+
);
198+
expect(findings).toHaveLength(2);
199+
expect(findings.every((f) => f.severity === 'warning')).toBe(true);
200+
expect(findings.every((f) => f.rule === DASHBOARD_ACTION_ROUTE_UNRESOLVED)).toBe(true);
201+
expect(findings.every((f) => f.path === 'dashboards[0].header.actions[0].actionUrl')).toBe(true);
202+
// Path order: the app segment's finding first, the dashboard segment's second.
203+
expect(findings[0].message).toContain('apps/no_such_app_nope');
204+
expect(findings[1].message).toContain('dashboard/no_such_dashboard_nope');
205+
});
206+
207+
it(
208+
'WARNS on a later unresolved segment even when an earlier segment resolves ' +
209+
'(#16169 — the boundary the triage flagged: this is a NEW finding, was clean before)',
210+
() => {
211+
// `exec` is a real dashboard; `bad_view` names no view. Before #16169 the
212+
// loop returned at the first recognized segment (`dashboards/exec`,
213+
// resolved) and never reached `views/bad_view` — this path was silent.
214+
const findings = validateDashboardActionRefs(
215+
dashWithHeaderActions([
216+
{ label: 'Open', actionType: 'url', actionUrl: '/dashboards/exec/views/bad_view' },
217+
]),
218+
);
219+
expect(findings).toHaveLength(1);
220+
expect(findings[0]).toMatchObject({
221+
severity: 'warning',
222+
rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED,
223+
path: 'dashboards[0].header.actions[0].actionUrl',
224+
});
225+
expect(findings[0].message).toContain('views/bad_view');
226+
expect(findings[0].message).toContain('view named "bad_view"');
227+
},
228+
);
229+
230+
it('passes a fully resolvable app-scoped dashboard route (#16169 — both segments real)', () => {
231+
const findings = validateDashboardActionRefs({
232+
apps: [{ name: 'real_app' }],
233+
dashboards: [
234+
{
235+
name: 'exec',
236+
header: {
237+
actions: [
238+
{ label: 'Open', actionType: 'url', actionUrl: '/apps/real_app/dashboard/real_dashboard' },
239+
],
240+
},
241+
},
242+
{ name: 'real_dashboard' },
243+
],
244+
});
245+
expect(findings).toEqual([]);
246+
});
247+
164248
it('skips external URLs, interpolated targets, and opaque routes (no false positives)', () => {
165249
const findings = validateDashboardActionRefs(
166250
dashWithHeaderActions([

packages/lint/src/validate-dashboard-action-refs.ts

Lines changed: 61 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,16 @@
6060
* names the page `create_opportunity`, or it names nothing. Opening an
6161
* object's form is `actionType: 'form'`. Otherwise → ERROR.
6262
*
63-
* actionType 'url' → a relative in-app path. WARN when a recognizable
64-
* `<collection>/<name>` segment (objects/reports/dashboards/pages/views)
65-
* names an entity that does not exist in this stack. External URLs
66-
* (`http(s)://`, `//`), interpolated targets (`${…}`), and opaque routes
67-
* (no recognized collection segment) are skipped — they cannot be resolved
68-
* statically and may be host/app/plugin routes. → WARNING.
63+
* actionType 'url' → a relative in-app path. WARN once per recognizable
64+
* `<collection>/<name>` segment (apps/objects/reports/dashboards/pages/
65+
* views) whose name does not exist in this stack — EVERY such segment in
66+
* the path, not only the first (#16169: a bad `apps/NAME` head used to
67+
* hide a bad `dashboard/NAME` tail, and a bad app name alone was never
68+
* checked at all, since `apps` was absent from the collection table).
69+
* External URLs (`http(s)://`, `//`), interpolated targets (`${…}`), and
70+
* opaque routes (no recognized collection segment anywhere in the path)
71+
* are skipped — they cannot be resolved statically and may be host/plugin
72+
* routes. → WARNING (one finding per unresolved segment).
6973
*
7074
* actionType 'flow' | 'api' — not checked: flow targets resolve against the
7175
* automation engine / other packages, and api targets are opaque endpoints.
@@ -111,9 +115,15 @@ function strName(v: unknown): string | undefined {
111115

112116
/** URL path segments that name a metadata collection, mapped to the stack key
113117
* whose members can appear after them in an in-app route
114-
* (`/…/objects/crm_lead`, `/reports/forecast`, `/dashboards/exec`, …). Both the
115-
* singular and plural spellings are accepted. */
116-
const URL_COLLECTION_TO_STACK_KEY: Record<string, 'objects' | 'reports' | 'dashboards' | 'pages' | 'views'> = {
118+
* (`/apps/crm_enterprise/objects/crm_lead`, `/reports/forecast`,
119+
* `/dashboards/exec`, …). Both the singular and plural spellings are
120+
* accepted. */
121+
const URL_COLLECTION_TO_STACK_KEY: Record<
122+
string,
123+
'apps' | 'objects' | 'reports' | 'dashboards' | 'pages' | 'views'
124+
> = {
125+
app: 'apps',
126+
apps: 'apps',
117127
object: 'objects',
118128
objects: 'objects',
119129
report: 'reports',
@@ -141,6 +151,10 @@ function viewContainerName(item: AnyRec): string | undefined {
141151
interface KnownTargets {
142152
/** Every action name defined in the stack (global + object-embedded). */
143153
actions: Set<string>;
154+
/** App machine names (valid in `apps/<name>` routes) — the same `name`
155+
* identity the runtime resolves `/apps/:appName/…` against and REST's
156+
* `GET /meta/apps/:name` reads by. */
157+
apps: Set<string>;
144158
/** Object names (valid in `objects/<name>` routes). */
145159
objects: Set<string>;
146160
reports: Set<string>;
@@ -153,6 +167,7 @@ interface KnownTargets {
153167
/** Build the author-time "known target" sets from a stack. */
154168
function collectKnownTargets(stack: AnyRec): KnownTargets {
155169
const actions = new Set<string>();
170+
const apps = new Set<string>();
156171
const objects = new Set<string>();
157172
const reports = new Set<string>();
158173
const dashboards = new Set<string>();
@@ -168,6 +183,7 @@ function collectKnownTargets(stack: AnyRec): KnownTargets {
168183
};
169184

170185
collectNames(stack.actions, actions, (a) => strName(a.name));
186+
collectNames(stack.apps, apps, (a) => strName(a.name));
171187
for (const obj of recordsOf(stack.objects)) {
172188
if (!obj || typeof obj !== 'object') continue;
173189
const n = strName(obj.name);
@@ -181,7 +197,7 @@ function collectKnownTargets(stack: AnyRec): KnownTargets {
181197
// An object's default view is routable by the object's own name too.
182198
for (const o of objects) views.add(o);
183199

184-
return { actions, objects, reports, dashboards, pages, views };
200+
return { actions, apps, objects, reports, dashboards, pages, views };
185201
}
186202

187203
/** Does a `script`/`modal` `actionUrl` resolve? */
@@ -202,36 +218,39 @@ function resolveActionTarget(
202218
}
203219

204220
/**
205-
* Resolve a relative `url` in-app route. Returns:
206-
* - `null` when the target is not statically resolvable (external, interpolated,
207-
* or carries no recognized `<collection>/<name>` segment) — SKIP, no finding.
208-
* - `{ collection, name }` for a recognized `<collection>/<name>` pair that does
209-
* NOT exist in the stack — WARN.
210-
* - `undefined` when a recognized pair DID resolve — OK, no finding.
221+
* Resolve a relative `url` in-app route. Scans EVERY segment of the path for a
222+
* recognized `<collection>/<name>` pair (#16169 — a bad `apps/NAME` head used
223+
* to hide a bad `dashboard/NAME` tail because the loop returned at the first
224+
* recognized segment, resolved or not). Returns one entry per recognized pair
225+
* whose name does NOT exist in the stack, in path order; an empty array means
226+
* either the route is not statically resolvable (external, interpolated, no
227+
* recognized segment anywhere) or every recognized pair resolved — either way,
228+
* no finding.
211229
*/
212230
function resolveUrlRoute(
213231
target: string,
214232
known: KnownTargets,
215-
): { collection: string; name: string } | null | undefined {
233+
): { collection: string; name: string }[] {
216234
// External / protocol-relative — leaves the app; not an in-app route.
217-
if (/^[a-z][a-z0-9+.-]*:\/\//i.test(target) || target.startsWith('//')) return null;
235+
if (/^[a-z][a-z0-9+.-]*:\/\//i.test(target) || target.startsWith('//')) return [];
218236
// Interpolated — resolved by the renderer at click time, not statically known.
219-
if (target.includes('${')) return null;
237+
if (target.includes('${')) return [];
220238
// Only relative in-app paths are considered.
221-
if (!target.startsWith('/')) return null;
239+
if (!target.startsWith('/')) return [];
222240

223241
// Strip query + hash, then split into non-empty segments.
224242
const pathPart = target.split(/[?#]/, 1)[0];
225243
const segments = pathPart.split('/').filter(Boolean);
226244

245+
const unresolved: { collection: string; name: string }[] = [];
227246
for (let i = 0; i < segments.length - 1; i++) {
228247
const stackKey = URL_COLLECTION_TO_STACK_KEY[segments[i]];
229248
if (!stackKey) continue;
230249
const name = segments[i + 1];
231-
if (known[stackKey].has(name)) return undefined; // resolved
232-
return { collection: segments[i], name }; // recognized shape, unknown name
250+
if (known[stackKey].has(name)) continue; // resolved — keep scanning later segments
251+
unresolved.push({ collection: segments[i], name }); // recognized shape, unknown name
233252
}
234-
return null; // no recognized collection segment — opaque route, skip
253+
return unresolved; // empty — opaque route, or every recognized pair resolved
235254
}
236255

237256
interface HeaderAction {
@@ -295,21 +314,25 @@ export function validateDashboardActionRefs(stack: AnyRec): DashboardActionRefFi
295314
}
296315

297316
if (actionType === 'url') {
298-
const route = resolveUrlRoute(target, known);
299-
if (!route) return; // skip (external/interpolated/opaque) or resolved
300-
findings.push({
301-
severity: 'warning',
302-
rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED,
303-
where,
304-
path,
305-
message:
306-
`url action target "${target}" points at ${route.collection}/${route.name}, ` +
307-
`but no ${route.collection.replace(/s$/, '')} named "${route.name}" is registered ` +
308-
`in this stack — the button likely navigates to a dead route.`,
309-
hint:
310-
`Check the path for a typo, define the referenced ${route.collection.replace(/s$/, '')}, ` +
311-
`or ignore this if the route is served by another installed package or a host/console route.`,
312-
});
317+
const unresolved = resolveUrlRoute(target, known);
318+
// One finding per unresolved segment (#16169): a bad `apps/NAME` head and
319+
// a bad `dashboard/NAME` tail on the SAME url are two distinct dead
320+
// references, and each is reported on its own so neither hides the other.
321+
for (const route of unresolved) {
322+
findings.push({
323+
severity: 'warning',
324+
rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED,
325+
where,
326+
path,
327+
message:
328+
`url action target "${target}" points at ${route.collection}/${route.name}, ` +
329+
`but no ${route.collection.replace(/s$/, '')} named "${route.name}" is registered ` +
330+
`in this stack — the button likely navigates to a dead route.`,
331+
hint:
332+
`Check the path for a typo, define the referenced ${route.collection.replace(/s$/, '')}, ` +
333+
`or ignore this if the route is served by another installed package or a host/console route.`,
334+
});
335+
}
313336
return;
314337
}
315338
// 'flow' | 'api' | custom types are out of scope (see module header).

0 commit comments

Comments
 (0)