Skip to content

feat(lint): check nav page/report/dashboard targets (ADR-0072) - #4574

Merged
os-zhuang merged 1 commit into
mainfrom
claude/adr-0072-nav-target-refs
Aug 2, 2026
Merged

feat(lint): check nav page/report/dashboard targets (ADR-0072)#4574
os-zhuang merged 1 commit into
mainfrom
claude/adr-0072-nav-target-refs

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

The nav-target gap identified while verifying ADR-0078 Phase 3 (#4544), landed in the module that actually owns it.

The hole is inside an existing check, not a missing one

defineStack's validateCrossReferences already validates these three nav target kinds. But each arm is gated on the collection being non-empty:

if (nav.type === 'page' && typeof nav.pageName === 'string'
    && pageNames.size > 0 && !pageNames.has(nav.pageName)) {  }

So a stack that declares no pages at all has its page-nav check silently switched off, and { type: 'page', pageName: 'anything' } sails through. That is exactly the state a stack is in when the target was never written — the most likely way to reach this bug, not the least.

Note the asymmetry the guard creates. The object arm of the same block has no size gate: it errors unless the item carries requiresObject, an explicit opt-in to "another package provides this". Objects have to say so out loud; pages, reports and dashboards got an implicit exemption keyed on an unrelated property of the stack.

validateNavTargetRefs joins REFERENCE_INTEGRITY_RULES (16 → 17), so it runs on validate / lint / compile with no CLI rewiring.

Warning, not error — and why

validate-object-references can say ERROR for an unresolved object because it resolves against the curated PLATFORM_PROVIDED_OBJECT_NAMES registry and knows which cross-package names are real. No such registry exists for pages, reports or dashboards, so "unresolved" cannot honestly be distinguished from "provided by a package we cannot see from here". Advisory is the honest ceiling.

Tightening defineStack's throw was the other option and was rejected: a throw has no escape hatch for a legitimately cross-package page, and ADR-0072 D1's rule is that one dead finding costs more than a missed one. Where defineStack's check is live it still hard-fails first; this rule speaks when that check has switched itself off, and the message says so explicitly.

Three nav types deliberately NOT covered — each verified, not assumed

type why not
action already owned by validate-action-name-refs, which walks app navigation explicitly — adding it here would double-report
component verified NON-rule. An unregistered componentRef does not fail silently: ComponentNavView renders a named diagnostic ("Component not registered … Ensure the plugin that provides this surface is installed and has called registerAppComponent()"). The registry exists precisely so plugin-provided surfaces may legitimately be absent — flagging it would break valid plugin nav and prescribe a fix for something already reported better at runtime
url external by definition

Both NON-rules are pinned by tests, so "completing" the module by adding them fails there first.

Scope honesty

All 35 authored nav page/report/dashboard targets in this repo resolve, so this closes a latent hole rather than a shipped bug — worth stating plainly, unlike Phase 1 which found a real dead roll-up.

The rule was proven to go red and then green through the real validateReferenceIntegrity entry point on a known-bad stack, not only in unit tests. A green check that has never been made to fail is the recurring defect this campaign keeps finding in its own instruments, so the instrument gets the same treatment as the code.

The suite's own membership pin caught the new rule on first run — which is exactly what it's for: adding a member is a deliberate two-place edit, not something that slips in unreviewed.

Verification

  • 14 new tests; @objectstack/lint 776/776
  • Full monorepo suite 132/132
  • API surface unchanged (lint-only change); i18n ratchet unchanged

Generated by Claude Code

`defineStack`'s `validateCrossReferences` already validates these three nav
target kinds. But each arm is gated on the collection being non-empty:

    if (nav.type === 'page' && typeof nav.pageName === 'string'
        && pageNames.size > 0 && !pageNames.has(nav.pageName)) { … }

So a stack that declares NO `pages` at all has its page-nav check silently
switched off, and `{ type: 'page', pageName: 'anything' }` sails through. That
is exactly the state a stack is in when the target was never written — the most
likely way to reach this, not the least. The hole is INSIDE an existing check,
not a missing one.

Note the asymmetry the guard creates. The `object` arm of the same block has no
size gate: it errors unless the item carries `requiresObject`, an EXPLICIT
opt-in to "another package provides this". Objects have to say so out loud;
pages, reports and dashboards got an implicit exemption keyed on an unrelated
property of the stack.

`validateNavTargetRefs` joins REFERENCE_INTEGRITY_RULES (16 -> 17), so it runs
on `validate` / `lint` / `compile` with no CLI rewiring.

## Warning, not error — and why

`validate-object-references` can say ERROR for an unresolved OBJECT because it
resolves against the curated `PLATFORM_PROVIDED_OBJECT_NAMES` registry and knows
which cross-package names are real. No such registry exists for pages, reports
or dashboards, so "unresolved" cannot honestly be distinguished from "provided
by a package we cannot see from here". Advisory is the honest ceiling.

Tightening `defineStack`'s throw was the other option and was rejected: a throw
has no escape hatch for a legitimately cross-package page, and ADR-0072 D1's
rule is that one dead finding costs more than a missed one. Where defineStack's
check IS live it still hard-fails first; this rule speaks when that check has
switched itself off, and the message says so.

## Three nav types deliberately NOT covered — each verified, not assumed

  action      already owned by validate-action-name-refs, which walks app
              navigation explicitly; adding it here would double-report
  component   VERIFIED NON-RULE. An unregistered `componentRef` does not fail
              silently — ComponentNavView renders a named diagnostic
              ("Component not registered … Ensure the plugin that provides this
              surface is installed and has called registerAppComponent()"), and
              the registry exists precisely so plugin surfaces may legitimately
              be absent. Flagging it would break valid plugin nav and prescribe
              a fix for something already reported better at runtime.
  url         external by definition

Both NON-rules are pinned by tests, so "completing" the module by adding them
fails there first.

## Scope honesty

All 35 authored nav page/report/dashboard targets in this repo resolve, so this
closes a LATENT hole rather than a shipped bug. The rule was proven to go red
and then green through the real `validateReferenceIntegrity` entry point on a
known-bad stack, not only in unit tests — a green check that has never been made
to fail is the recurring defect this campaign keeps finding in its own
instruments.

The suite's membership pin caught the new rule on first run, which is what it is
for: adding a member is a deliberate two-place edit, not something that slips in
unreviewed.

Verification: 14 new tests; lint 776/776; full suite 132/132; API surface
unchanged; i18n ratchet unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WnqGjQFQMqd5k81LYV8SCY
@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 2, 2026 8:01am

Request Review

@os-zhuang
os-zhuang marked this pull request as ready for review August 2, 2026 08:01
@os-zhuang
os-zhuang enabled auto-merge August 2, 2026 08:01
@github-actions github-actions Bot added the size/m label Aug 2, 2026
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/lint.

2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/hook-bodies.mdx (via @objectstack/lint)
  • content/docs/permissions/authorization.mdx (via @objectstack/lint)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 2, 2026
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 2, 2026
Merged via the queue into main with commit 85a966f Aug 2, 2026
20 checks passed
@os-zhuang
os-zhuang deleted the claude/adr-0072-nav-target-refs branch August 2, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants