Uh oh!
There was an error while loading. Please reload this page.
Guard every metadata binding: views, datasets and app nav resolve against dulyObjects - #57
Merged
Merged
Conversation
Stopgap pending objectstack#14105 / #14107 / #14108: no field path in the UI or analytics layer is resolved at author time, and both `validate` and `build` exit 0 on a dangling one. Walks dulyViews, dulyDatasets and dulyApps, resolving every reference against dulyObjects: view columns / filter / sort / grouping / binding blocks / bulk-action patch keys, dataset base object / include / dimension / measure fields / filter KEYS, joined paths through a real lookup, and every nav objectName + viewName. Platform objects resolve from the spec's own registry; system columns from SystemFieldName. Includes a synthetic-fixture self-test pinning both directions on all four surfaces, plus tripwires for unknown field slots, unknown nav item types, form views and platform-object boundaries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p
Picks up #54 (source defaults to self) and #56 (hierarchy-security). Both touch objects this guard resolves against; no field was renamed or removed, only `default:` flags on select options. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p
os-warren
marked this pull request as ready for review
September 1, 2026 06:13
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#51
Adds
test/metadata-bindings.test.ts— one guard walkingdulyViews,dulyDatasetsanddulyAppsand resolving every reference againstdulyObjects. Labelled a stopgap pending objectstack#14105 / #14107 / #14108, written to be deleted rather than maintained, per thetest/flow-predicates.test.tsconvention.Scope taken is the widened scope from the PM comment on the card (views + nav + datasets), not the datasets-only scope in the body.
Two things the reviewer should read first
1. No dangling reference exists in shipped metadata. The guard resolves 204 references — 145 view, 34 dataset, 25 nav — and all 204 resolve. So no metadata file is touched by this PR; the only change is the test. That is a real result, not a skipped check: the per-surface counters and the joined-path assertion below are what stop "green" from meaning "the walk did nothing".
2. The card's premise that "nothing has ever checked the five view files or the app nav" is wrong.
test/views.test.ts(landed in #49) already resolves the simple view surface and the navobjectName/viewNamepair. The card is still real — but the value is in the superset, not in first coverage. The remaining holes it closes:datasets.test.tspins caliber / date-macros / absences, never that afieldpath names anything realviews.test.tsopens its checker withif (!name || name.includes('.')) return— soduty.frequency, the one joined path this app ships, was unchecked everywhereviews.test.tscarriesbusiness_unit_id(not a platform column) and omitsowning_business_unit_id,tenant_id,user_id,deleted_at(which are). This file imports the spec's ownSystemFieldNamedulyObjects; platform names now resolve fromisPlatformProvidedObjectNameThe overlap is declared in the file header rather than resolved by deleting another card's guard — that is a follow-up, filed as #58. Until then it is benign: this file is the strict superset, so any disagreement reds here first.
What is checked
columns[](both the bare-string and{ field }spellings),columns[].summary.field,columns[].prefix.field,filter[].field,sort[].field(plus the legacy"field desc"string),grouping.fields[].field,rowColor.field, thesearchableFields/filterableFields/hiddenFields/fieldOrderlists, and every binding block —kanban,calendar,gantt,timeline,gallery,tree,map.object, everyincludepath, every dimension and measurefield, and every filter key (the column is the key; the walk recurses$and/$or/$notand never mistakes an operator for a column). Also that a joined path's relationship is actually ininclude— every segment can name something real and still have no join to travel (ADR-0071).reference-carrying field to a real field on the target. Hopping through a scalar is reported as its own, more legible failure.objectName, everyviewNameagainst the views declared on that object (a view of the right name on the wrong object is a finding),filterskeys, and the no-viewNamecase when the object has no defaultlist.Beyond the card's enumerated list, and flagged because it is an addition:
bulkActionDefswithoperation: 'update'write fields — the staticpatchkeys and the collectedparams[].name, whichBulkActionDefSchemadocuments as merged over the patch. Same defect class, one layer down. Onlyupdate; on acustomdef those are action params, not fields.Narrowings, stated rather than hidden
@objectstack/specexports the platform object name registry but no field lists, soowner.some_typocannot be judged. The hop is verified, the path is recorded, and a test fails the day one is authored — the hole is loud rather than silent.FLOW_NODE_EXPRESSION_PATHSfor flow predicates), and the list polices itself: a tripwire scans the metadata for field-ish slots and fails on any the walk cannot read.Proving it can fail — four ablations, one per surface
Each leg: anchor asserted to occur exactly once before the edit, both the injected and the removed literal grepped afterwards, a non-empty
git diff --statrequired, and atrap … EXIT INT TERMrestoring fromHEAD(the guard was committed first, so this is a real restore point and not the index).pnpm validatecategory→categoree_typo✓ Validation passed (338ms)last_update_at→last_update_attt✓ Validation passed (334ms)viewName: 'stalled'→'not_moving'✓ Validation passed (430ms)duty.frequency→duty.frequencee✓ Validation passed (432ms)Leg 3 is the one worth reading — the guard prints exactly what makes #14108 nasty:
The label stays "Not moving". The screen looks right and shows the wrong rows.
Observed direction, not the template one: leg 4 reddened two tests, not one — the finding, plus the non-vacuity assertion that the joined path was reached at all. Diagnostics went up, which is the correct reading here: mutating the app's only multi-hop path both produces a finding and removes the evidence that the multi-hop walk runs.
All four restored; tree verified byte-identical to
HEADafterwards (git status --porcelainempty,git diff HEADempty, all four anchors back at count 1).Gates
mainmoved while this was in flight (#54 source defaults, #56 hierarchy-security — both touch objects this guard resolves against). It is merged in at27d31a0; no field was renamed or removed, onlydefault:flags on select options.Gates re-run on the final commit
27d31a0, after the merge:Exit codes captured before any pipe. 407 tests = 375 on merged
main+ 32 added. (The pre-merge reading atcf62af9was 402 = 370 + 32; #54 and #56 added five.)No changeset: this repo carries no changeset tooling, and the change is test-only.
Generated by Claude Code