Repository navigation
Two binding guards now overlap: collapse test/views.test.ts's stopgap half into test/metadata-bindings.test.ts #58
Description
Activity
Claiming this.
- Session:
session_01SqkTcrxUFci7nqXdbBSe2p - Branch:
claude/issue-58-collapse-binding-guards - Worktree:
../duly-issue-58
This is a deletion card, so the plan is mutation-first: for every assertion I propose to remove from
test/views.test.ts, I will mutate a real fixture and showtest/metadata-bindings.test.tsfails on the same mutation (method per PR #77 / issue #37). Anything the surviving guard does not catch stays where it is.
Generated by Claude Code
- Session:
{ "issue": 58, "status": "done", "branch": "claude/issue-58-collapse-binding-guards", "pr": "https://github.com/objectstack-ai/duly/pull/97", "premise_still_valid": true, "summary": "Deleted the two subsumed assertions from test/views.test.ts ('names only fields that exist on the object it is bound to' and 'every nav entry resolves to a view that exists on the object it names') and handed reference resolution to test/metadata-bindings.test.ts. The card's premise held, but one part of it did not: the two guards did NOT fully overlap. metadata-bindings' walkNav recursed into children on `group` items only, while the spec ties the recursive `children` knot on the object branch too (NavigationItem = (ObjectNavItem & { children?: NavigationItem[] }) | ... | GroupNavItem), so a nav entry nested under an `object` entry was resolved by the deleted assertion and by nothing else. Closed that in walkNav and pinned it with a new self-test BEFORE deleting, then re-measured. Kept binding-block presence (#14106), the product pins, and the reachability/placement nav tests, per the card. views.test.ts 324 -> 263 lines; suite 662 -> 661 tests (two assertions deleted, one self-test added).", "tests": "Gates at d2876c5 (the commit the PR points at, clean tree), each verdict read from the tool's own line, not a piped $?: `pnpm validate` -> '✓ Validation passed (376ms)' VALIDATE_EXIT=0; `pnpm typecheck` -> no output, TYPECHECK_EXIT=0; `pnpm test` -> 'Test Files 26 passed (26) · Tests 661 passed (661)' TEST_EXIT=0; `pnpm build` -> '✓ Build complete (581ms)' BUILD_EXIT=0. The one security-enterprise warning is the documented expected state (AGENTS.md rule 7). All heavy runs went through scripts/pm/os-verify-lock.sh (VERDICT lines: command-exit 0, held 25-29s, waited 0s).\n\nABLATION (18 cases + 2 follow-ups, pre- and post-change). Each case: cp byte-backup (never an index restore); mutation applied with a hit count asserted == 1; arrival proved on disk with `grep -F -c` in BOTH directions (-F so a [...] anchor is a literal, not a character class); vitest run; restore from the byte copy verified by sha256sum; mutate+measure inside ONE invocation with a per-case EXIT/INT/TERM restore trap. No rebuild leg applies — this repo runs tests from source, no dist.\n\nRESULT: 15/15 real defects still red after the collapse, through metadata-bindings. A1-A8/A10/A13 (columns, filter, sort, grouping, rowColor, kanban.columns, gantt.tooltipFields, calendar/gantt block keys, data.object) and B1/B2 (nav viewName, nav objectName): red on both guards before, red on the survivor after. B3 (ghost viewName nested under an `object` nav entry): views-only red BEFORE (the hole), survivor red AFTER. A9b (gantt.typeField): red on the survivor only — it reads three reachable slots the deleted assertion never did.\n\nTWO reds deliberately gone, both false positives, each with a named registry as the authority: A12 (a column of `deleted_at`) — `deleted_at` IS a platform column; Object.values(SystemFieldName) = [created_at, created_by, deleted_at, id, organization_id, owner_id, owning_business_unit_id, tenant_id, updated_at, updated_by, user_id], and the deleted hand-copied list carried `business_unit_id` (not in it) while omitting four that are. The other direction is measured too: A11 puts `business_unit_id` on a view and it PASSED the deleted assertion while failing the survivor. B4 (a nav entry to `sys_user`) — the deleted assertion required nav objects to be in dulyObjects, so it rejected real platform objects. Typo net proved intact: C1 (`sys_bogus`) is still a finding, C2 (a viewName reaching into sys_user) still fails via the boundary assertion.\n\nINSTRUMENT FAILURE, DECLARED: the first pass reported case A9 as '(none — GREEN)'. It was VOID, not green — numTotalTests 0, numFailedTests 0, success false. `defineView` parses with .strict(), so the mutation threw at import and the suite never ran; a failures-only summary reads an import-time throw as an absence of failures. Driver fixed to print the collected-test count and fail loudly on zero; every other case in both passes was then audited to have collected the full suite (63 pre, 62 post). That refusal also replaced A9's intended claim with a stronger one: the 19-keys-x-6-blocks breadth of the deleted assertion is mostly UNREACHABLE, since an unknown block key cannot be authored. Reading the Zod shapes off @objectstack/spec/ui rather than the error behaviour, the reachable slots are kanban 3/3, calendar 4/4, gantt 12 of 15 (survivor reads all 15), timeline 5/5, tree 3/3, map 4 of 5 (survivor 5), gallery 0 of 3 (survivor 3) — no reachable slot was read by the deleted assertion and not by the survivor.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
os-dev-report
(Supersedes the previous comment: its leading
<!-- os-dev-report -->marker did not survive GitHub's body sanitizer, so that copy is invisible to a marker scan. Same JSON, marker as literal text.){ "issue": 58, "status": "done", "branch": "claude/issue-58-collapse-binding-guards", "pr": "https://github.com/objectstack-ai/duly/pull/97", "premise_still_valid": true, "summary": "Deleted the two subsumed assertions from test/views.test.ts ('names only fields that exist on the object it is bound to' and 'every nav entry resolves to a view that exists on the object it names') and handed reference resolution to test/metadata-bindings.test.ts. The card's premise held, but one part of it did not: the two guards did NOT fully overlap. metadata-bindings' walkNav recursed into children on `group` items only, while the spec ties the recursive `children` knot on the object branch too (NavigationItem = (ObjectNavItem & { children?: NavigationItem[] }) | ... | GroupNavItem), so a nav entry nested under an `object` entry was resolved by the deleted assertion and by nothing else. Closed that in walkNav and pinned it with a new self-test BEFORE deleting, then re-measured. Kept binding-block presence (#14106), the product pins, and the reachability/placement nav tests, per the card. views.test.ts 324 -> 263 lines; suite 662 -> 661 tests (two assertions deleted, one self-test added).", "tests": "Gates at d2876c5 (the commit the PR points at, clean tree), each verdict read from the tool's own line, not a piped $?: `pnpm validate` -> 'Validation passed (376ms)' VALIDATE_EXIT=0; `pnpm typecheck` -> no output, TYPECHECK_EXIT=0; `pnpm test` -> 'Test Files 26 passed (26) · Tests 661 passed (661)' TEST_EXIT=0; `pnpm build` -> 'Build complete (581ms)' BUILD_EXIT=0. The one security-enterprise warning is the documented expected state (AGENTS.md rule 7). All heavy runs went through scripts/pm/os-verify-lock.sh (VERDICT lines: command-exit 0, held 25-29s, waited 0s).\n\nABLATION (18 cases + 2 follow-ups, pre- and post-change). Each case: cp byte-backup (never an index restore); mutation applied with a hit count asserted == 1; arrival proved on disk with `grep -F -c` in BOTH directions (-F so a [...] anchor is a literal, not a character class); vitest run; restore from the byte copy verified by sha256sum; mutate+measure inside ONE invocation with a per-case EXIT/INT/TERM restore trap. No rebuild leg applies - this repo runs tests from source, no dist.\n\nRESULT: 15/15 real defects still red after the collapse, through metadata-bindings. A1-A8/A10/A13 (columns, filter, sort, grouping, rowColor, kanban.columns, gantt.tooltipFields, calendar/gantt block keys, data.object) and B1/B2 (nav viewName, nav objectName): red on both guards before, red on the survivor after. B3 (ghost viewName nested under an `object` nav entry): views-only red BEFORE (the hole), survivor red AFTER. A9b (gantt.typeField): red on the survivor only - it reads three reachable slots the deleted assertion never did.\n\nTWO reds deliberately gone, both false positives, each with a named registry as the authority: A12 (a column of `deleted_at`) - `deleted_at` IS a platform column; Object.values(SystemFieldName) = [created_at, created_by, deleted_at, id, organization_id, owner_id, owning_business_unit_id, tenant_id, updated_at, updated_by, user_id], and the deleted hand-copied list carried `business_unit_id` (not in it) while omitting four that are. The other direction is measured too: A11 puts `business_unit_id` on a view and it PASSED the deleted assertion while failing the survivor. B4 (a nav entry to `sys_user`) - the deleted assertion required nav objects to be in dulyObjects, so it rejected real platform objects. Typo net proved intact: C1 (`sys_bogus`) is still a finding, C2 (a viewName reaching into sys_user) still fails via the boundary assertion.\n\nINSTRUMENT FAILURE, DECLARED: the first pass reported case A9 as '(none - GREEN)'. It was VOID, not green - numTotalTests 0, numFailedTests 0, success false. `defineView` parses with .strict(), so the mutation threw at import and the suite never ran; a failures-only summary reads an import-time throw as an absence of failures. Driver fixed to print the collected-test count and fail loudly on zero; every other case in both passes was then audited to have collected the full suite (63 pre, 62 post). That refusal also replaced A9's intended claim with a stronger one: the 19-keys-x-6-blocks breadth of the deleted assertion is mostly UNREACHABLE, since an unknown block key cannot be authored. Reading the Zod shapes off @objectstack/spec/ui rather than the error behaviour, the reachable slots are kanban 3/3, calendar 4/4, gantt 12 of 15 (survivor reads all 15), timeline 5/5, tree 3/3, map 4 of 5 (survivor 5), gallery 0 of 3 (survivor 3) - no reachable slot was read by the deleted assertion and not by the survivor.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
Observation from #51 (PR #57). Not urgent, and deliberately not done there — deleting another card's guard is not a rider on the card that supersedes it.
What happened
#51's dispatch said "nothing has ever checked the five view files or the app nav". That was not accurate as of
main—test/views.test.ts(landed in #49) already carries a stopgap half that resolves the simple view surface and the nav pair, and its docblock names the same three upstream issues (#14106 / #14107 / #14108).#51 shipped
test/metadata-bindings.test.tsanyway, because the value was in the superset — datasets, dotted paths, platform objects, and a self-test, none of which existed. But that leaves two guards covering overlapping ground, which is the "keep two guards in step" cost both files' headers warn against.The overlap, precisely
test/views.test.ts›describe('every non-grid lens is bound to real fields')— the two assertions:names only fields that exist on the object it is bound toevery nav entry resolves to a view that exists on the object it names(indescribe('navigation'))Both are strict subsets of
metadata-bindings.test.ts. The rest ofviews.test.tsis product pins (the gantt bar spanningvisible_from→due_date, the timeline readinglast_update_at, one colour source, nothing ranked by a count, the deliberate absence ofgantt.colorField) — those outlive the platform gaps and must stay wherever they end up.declares the binding block its type needsis also NOT covered by the new file: that is binding-block presence (#14106), a different property from reference resolution (#14107). Keep it.Two concrete defects in the older half, which is the argument for collapsing rather than leaving both
Its system-column list is hand-copied and has already drifted.
SYSTEM_FIELDSthere listsbusiness_unit_id, which is not a platform column, and omitsowning_business_unit_id,tenant_id,user_idanddeleted_at, which are. The platform ships the registry —SystemFieldNamefrom@objectstack/spec/system— andmetadata-bindings.test.tsimports it. Net effect today: a view namingbusiness_unit_idonduly_task(which declaresbusiness_unit) passes there as a "system field".It skips every dotted path by construction. Its checker opens with
if (!name || name.includes('.')) return, so a joined path on a view is unchecked. Harmless today only because no view authors one.Suggested shape
Delete the two subsumed assertions from
test/views.test.ts, leave a one-line pointer totest/metadata-bindings.test.ts, and keep the product pins and the binding-block-presence check where they are. Roughly a 40-line deletion; the 402-test suite should stay green minus those two.Worth folding into whoever removes the stopgaps when objectstack#14105 / #14107 / #14108 land — at that point
metadata-bindings.test.tsgoes away entirely and only the product pins should remain.