Uh oh!
There was an error while loading. Please reload this page.
fix(lint): resolve app navigation viewName against the target object's list views - #14286
Conversation
…views An app navigation entry's `viewName` was resolved by nothing at author time. The schema documents it as "Default list view to open", so an unresolvable name fell back rather than failing: the entry kept its authored label and icon and opened a different view, with `os validate --json` reporting valid: true and `os build` green. Extends #2554's `lintViewRefs` to the navigation door of the same `listViews` namespace. Resolution mirrors objectui's `resolveViewId` in all three directions so a name that works at runtime is never reported. Part of #14108 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📓 Docs Drift Check5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e3ca88f05ecf0f4581811fca4fc1a019a0fe945c && git checkout e3ca88f05ecf0f4581811fca4fc1a019a0fe945c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin aca23aba40883b2c181053229fc6748d6f7ad0d5 43f68b243c5a0c29073c193fb6705d1bf774669a && git checkout -B drift-repro aca23aba40883b2c181053229fc6748d6f7ad0d5 && git merge --no-ff 43f68b243c5a0c29073c193fb6705d1bf774669a
node scripts/docs-audit/affected-docs.mjs --json aca23aba40883b2c181053229fc6748d6f7ad0d5 |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14108
App navigation's
viewNamewas resolved by nothing at author time.ObjectNavItemSchemadocuments it as "Default list view to open. Defaults to 'all'", so an unresolvable name never
failed — it fell back. The nav entry kept its authored label and icon and opened a different
view, which is why the failure is invisible in review: a "Schedule" entry that opens the plain
grid still reads correctly in the diff.
This extends #2554's existing
lintViewRefsto the navigation door of the samelistViewsnamespace. It is not a new rule class — the file already guarded the
type:'form'action-targetdoor into that namespace, and navigation is the other, more travelled one.
Premise checks (run first, on
origin/main@aca23aba4)1. The gap is real — confirmed. A probe stack whose nav carries
viewName: 'A4_no_such_view', run throughrunAuthoringRuleson both commands:Zero findings about the bogus
viewNameon either command. The positive control showslintViewRefsitself is wired and live — a bogustype:'form'target on the same stack isreported. The three rules the family landed since the card was written (#14105 / #14148 /
#14107) cover datasets, widgets and list-view field positions; none of them reaches nav
viewName.2.
objectNameresolution — the card's hedge, answered: it IS already covered, so this PR isthe
viewNamehalf only. Two live checks own it, and between them they leave no hole:packages/spec/src/stack.zod.ts'svalidateCrossReferenceserrors onApp 'APP' navigation references object 'OBJECT_NAME' which is not defined in objects.Unlike thesibling
dashboard/page/reportarms it carries nosize > 0gate, so it does notswitch itself off on a stack that declares no objects.
packages/lint/src/validate-object-references.tscovers the case that check exempts — a navitem carrying
requiresObject.runActionis covered too (the samestack.zod.tsblock, plusvalidate-action-name-refs, which walks app navigation explicitly). See "variants" below.3. The default view's real key is
default, not'all'. The dispatch asked me not to trusteither the card's
'all'or a paraphrase, and the paraphrase turns out to be the wrong reading:expandViewContainerkeys a bare defaultlistasOBJECT.default, and objectui'sresolveViewIdhas no special case for'all'either. The schema's "Defaults to 'all'"describes the convention of declaring a
listViews.all—examples/app-crmdeclares exactlythat on
activity/lead/opportunity, and it resolves normally as a declared key. So therule needs no
'all'special case, and a test pins both halves of that.Resolution mirrors the runtime matcher, deliberately
The authority is objectui's
resolveViewId(@object-ui/core, objectstack#2217), whichObjectViewcalls on every nav landing. It accepts three directions — exact id, short nameretried as
OBJECT.NAME, and a qualified name retried with theOBJECT.prefix stripped —and on a miss
console.warns and falls back todefaultViewId || views[0]. That browser-consolewarning is the only existing signal, which is why the check belongs at author time.
The lint reimplements those three directions and no others. A stricter match would red names that
work at runtime; a looser one would bless names that do not.
Severity: error, and what makes that safe
The sibling
view-ref-form-target-missingis a warning because a miss might be a view the lintfailed to collect. That vector is closed by construction here: the rule fires only when it has
already collected a non-empty list-view namespace for that object out of this stack. Every
other case is skipped rather than guessed at:
viewNameis interpolatedrecordIdis setviewNameas ignored in that pairingrequiresObjectWhat stays outside its knowledge is a runtime-saved view (
savedViewsinObjectView), whichno author-time pass over declared metadata can see — the same boundary every rule in this suite
has. It is stated in the file header rather than left implicit.
Variants — covered, or why not
runAction— already covered; adding it here would double-report.stack.zod.tserrors onan unresolved
runAction(size-gated), andvalidate-action-name-refsspeaks when that gate isoff.
recordId— deliberately not a lint target. It is record data, carrying templatevariables (
{current_user_id},{current_org_id}) resolved by the shell at render time; thereis no author-time set to resolve an id against.
filters— needs nothing: the schema already makes it mutually exclusive withrecordId/viewName, so the ambiguous combination is unrepresentable rather than linted.Acceptance
Pinned end-to-end on
validateANDbuild, following the #14148 / #14107 precedent ratherthan inferring it from the registry entry — the card measured
os validate --jsonreturningvalid: trueandos buildgreen, so a validate-only fix was not acceptable, and nothing else inthe file would notice if the suite entry's
commandswere narrowed later.Reverse verification: with the finding's severity ablated
errortowarning, exactly 3 of the34 tests go red — the unit severity assertion and both acceptance limbs — and the other 31 stay
green, so the acceptance block is what carries the gating claim. The mutation was confirmed on
disk (
git diff --statnon-empty plus a match count on the injected text) before the run, and thetree was restored from
HEADafterwards, proven bygit hash-objectequalling the HEAD blob.The subject is a same-package relative import, so vitest resolves it from
srcand the ablationneeded no rebuild.
Downstream stopgap — a follow-up in that repo, not this PR
objectstack-ai/duly'stest/views.test.tsasserts every navviewNameresolves against thedeclared
listViews, and the reporter wrote it to be deleted when this ships. Deleting it is afollow-up in
objectstack-ai/duly, out of scope for this repo's PR — recorded here ratherthan left silent, so a stale stopgap asserting a now-gated invariant is not forgotten. It should
be removed only after this lands and
dulypicks up the release.Verification
Run on the final commit
43f68b243.pnpm --filter @objectstack/lint test— 91 files, 2676 passed (5 skipped)pnpm --filter @objectstack/lint typecheck— clean.no test file —
packages/lint/tsconfig.jsonexcludes**/*.test.ts, and--listFilesreturns 0 hits for the edited test. The test edit was type-checked through a program that does
include it: my files clean, 0 errors.
pnpm lint(full repoeslint . --no-inline-config) — clean, 92s. No narrowing claimed.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(path-derived + convention-triggered, harvested runnablyso neither section nor spelling could be dropped): 32 green, 1 NOT MEASURED —
check-test-completenessexits 3 (PREREQUISITE NOT MET) because it grades a savedturbo run testlog that only CI produces; its own text says this is not a red.check:ratchet-remedy-authority(183 scripts swept) andcheck:declared-population-live(156 of 200 families reach this tree). Both green.check:dual-build-cjs-loadsandcheck:type-check-debtneeded the workspace closure built(70/70 tasks) and are green on it;
check:type-check-debt --re-measurere-measured 27 ledgerentries with none above its recorded number.
Generated by Claude Code