fix(lint): validateStackExpressions reads an object's fields through the guarded reader instead of an inline cast (#15742) - #15791
Conversation
…dFieldIndex `buildFieldIndex` in `validate-expressions.ts` cast every member of an object's `fields:` list inline (`fields.map(f => (f as AnyRec).name)`). `Array.isArray` proves the LIST, not its MEMBERS: an empty YAML list item deserialises to `null`, and the dereference threw out of the whole rule before the `.filter` two calls later could drop it. The list is now read through `recordsOf` — the single home of that coercion — which drops a non-record array member whole and in silence, the same disposition the two sibling readers in this file (`buildFieldTypeIndex`, `fieldEntries`) already had. The map shape keeps `Object.keys`: there the author's key IS the field name. The sweep's `RESIDUAL_THROWS` rows for `objects[].fields` come out in the same change — it is exact in both directions, so it now asserts the throw is gone. `RESIDUAL_INVENTED` is unchanged, measured: the repaired reader raises no finding about the dropped member. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
They were appended inside the `#15137` assignment-value describe, which reads as a claim about that suite rather than about `buildFieldIndex`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
…lidate-expressions-non-record-field
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. 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 4137f2defd67c515f1b36b87b18d4642cb8c6a22 && git checkout 4137f2defd67c515f1b36b87b18d4642cb8c6a22
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d701e657d74dc9d5cd17cfbfa7c96db9a55a435d 408c471761acb127cd4b5e5ef1dfaced10eff102 && git checkout -B drift-repro d701e657d74dc9d5cd17cfbfa7c96db9a55a435d && git merge --no-ff 408c471761acb127cd4b5e5ef1dfaced10eff102
node scripts/docs-audit/affected-docs.mjs --json d701e657d74dc9d5cd17cfbfa7c96db9a55a435d |
|
ACCEPT — PR #15791 (head As ruled: the inline cast at Measured by this seat: The unfiled finding (flow Flip + arm when both required jobs report success on Generated by Claude Code |
|
CI note — Generated by Claude Code |
|
Correction: the re-run request was refused — Generated by Claude Code |
|
Flipped ready + auto-merge enabled (08:26Z, Generated by Claude Code |
…cord-field to re-run CI on a fresh base Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
|
Correction + action (09:2xZ) — The 08:26Z flip note said "the two required jobs decide the flip". That was wrong. The ruleset on What was done: no re-run means exist (403 on record above), and an empty commit is not allowed, so Seat-wide: the "two required jobs" reading in this lane's flip notes is retired from this tick on — the flip gate is all seven contexts non-failing. Generated by Claude Code |
|
Correction to the note above: Generated by Claude Code |
|
Fresh head Generated by Claude Code |
|
Merge conflict (10:31Z) — Generated by Claude Code |
…cord-field (resolve validate-expressions.test.ts with #15792) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
|
merge-conflict-resolved New head The conflict
One hunk, at end of file: both sides append a new top-level Resolution — both sides kept, verified as a strict unionNot resolved by hand-editing between the markers. The three merge stages were extracted (
Proof that nothing was dropped from either side: Both blocks are present in the merged file:
Verification (exit codes captured before any pipe; verdict lines quoted)1. Scope unchanged — still this PR's 4 files. Identical line counts to the pre-merge diff, so the merge added no content of its own to the PR's delta. 2. No conflict markers. 3. Dependency closure build (fresh worktree, 4. Vitest, the conflicted file plus this PR's sweep file — Both sides' new arms are inside that run: the file is the merged one, and 5. Typecheck — 6. Mergeable again. After a fresh Not measured, by name
Labels, assignee, draft state and auto-merge untouched. Generated by Claude Code |
|
Conflict resolved on record (10:4xZ) — head Generated by Claude Code |
|
CI red on Same test as on Why it is not this PR's, read from the tree: the PR changes Action: ONE re-run of the failed jobs on run Generated by Claude Code |
|
Third red — no more re-runs; this needs a maintainer call. The re-run of What the seat now believes, stated as a hypothesis: the PR changes Options for the maintainer (
Until one of these happens, #15791 stays ready and un-armed (auto-merge is not re-enabled on a red required context) and the seat keeps it watched. Card #15742 is Generated by Claude Code |
Fixes #15742
buildFieldIndexinpackages/lint/src/validate-expressions.tscast every member of an object'sfields:list inline, so an empty YAML list item — which deserialises tonull— threwTypeError: Cannot read properties of null (reading 'name')out of the whole rule, before the.filtertwo calls later could drop it:Array.isArrayproves the LIST, never its MEMBERS. The list is now read throughrecordsOf(./object-graph.js), the single home of that coercion — no new copy, socollection-coercion-single-copy.test.tscounts what it counted before:The map branch keeps
Object.keys: on that shape the author's KEY is the field name, which is what this "did you mean?" index needs, andrecordsOf's map branch would let an innernameoverride it. That is a different question and is deliberately left where it was.The sibling readers, and the finding this now raises
Both sibling field readers in the same file already guard, and both drop such a member in silence — no finding. Quoted from
validate-expressions.tsas they stand onmain:recordsOfmakes the third reader agree with them: an array member that is not a record carries no author-written name, so there is nothing to report about it and it is dropped whole. The crash becomes silence, not a finding — measured, not assumed: the sweep's second arm (invents no finding about the entry no author wrote) now countsvalidateStackExpressionsfor these two keys instead of skipping it, and passes withRESIDUAL_INVENTEDunchanged.The sweep rows removed
packages/lint/src/non-record-object-entry.test.tsis exact in both directions, so the rows had to go in this PR:(the declaration keeps its real
Readonlyrecord type in the file; the type is elided here only to keep the quote free of angle brackets)RESIDUAL_THROWSis now empty and its docblock says so as a measurement, listing the two rows that have come out (stack.datasetsvia #15741,objects[].fieldshere).RESIDUAL_INVENTEDis unchanged — its one row (stack.agents · an array) is about the agent readers, not this one, and the suite stays green with it in place.Two focused arms were added to
validate-expressions.test.tsfor what the rule does INSTEAD of crashing, which a crash-only sweep cannot say: the junk member is silent, and the readable siblings are still indexed (arecord.amonttypo on that object still draws its did-you-mean finding — the failure mode a baretry/catchrepair would have produced).Ablation (trap-guarded, blob-hash restore)
The tests import the source through a relative specifier (
./validate-expressions.jsfrom insidesrc/), so vitest loads the TypeScript source — there is nodistleg in this ablation and no rebuild is needed between the legs. The mutation was proven on disk before the run (anchor counts), and the restore proven by blob hash plus an emptygit diff HEAD.recordsOf(fields)→ the old inline castguarded-count=0 cast-count=1; blob6e7de66d2091df6b1b67ca86ab2924d90e40670d(HEAD bloba4cfe89a0435564cd51450666eafd7260c8c6621)command-exit 1—Tests 6 failed | 576 passed (582)git checkout HEAD -- PATH, pinned to HEAD, never a bare checkout)restored-blob=a4cfe89a0435564cd51450666eafd7260c8c6621,git diff HEADemptytestcommand-exit 0—Test Files 97 passed (97),Tests 3324 passed | 5 skippedThe mutated leg reds naming the rule, in both arms and both shapes:
Verification
All heavy runs went through
scripts/pm/os-verify-lock.sh(OS_VERIFY_LOCK_SLOT=issue-15742); exit codes captured before any pipe, verdict lines quoted from the gates themselves.pnpm --filter '@objectstack/lint^...' buildVERDICT command-exit 0 · held the lock 228svitest run src/non-record-object-entry.test.ts src/validate-expressions.test.ts src/collection-coercion-single-copy.test.tsVERDICT command-exit 0—Test Files 3 passed (3),Tests 588 passed (588)(586 before the two new arms)pnpm --filter @objectstack/lint testVERDICT command-exit 0—Test Files 97 passed (97),Tests 3324 passed | 5 skipped (3329)pnpm --filter @objectstack/lint typecheckVERDICT command-exit 0—check:test-typecheck: OK — @objectstack/lint's test layer compiles under packages/lint/tsconfig.test.json(so the new arms ARE typechecked)node scripts/pm/dispatch-gates.mjs --changed --commands --repo objectstack-ai/objectstackEXIT=0, 53 commands, derived atfd82de279after mergingorigin/main(the first derivation warned STALE TREE; the merge cleared it)EXIT=0, 3 ×EXIT=3(see NOT MEASURED)pnpm check:pm-dispatch-gatesEXIT=0—✓ dispatch-gates self-test: 1478 cases pass.pnpm check:nul-bytesEXIT=0; plus a direct control-byte scan of the four changed paths (grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]', no match)node scripts/pm/check-governed-merges.mjs --test(the 4 changed paths)EXIT=0—✅ NOT governed — ordinary queue landing applies to a PR with exactly this file list.pnpm lint(eslint . --no-inline-config, repo-wide)EXIT=0— the whole-repo run, so no narrowing to declareNOT MEASURED, by name
pnpm check:dual-build-cjs-loads— NOT MEASURED (exit 3: PREREQUISITE NOT MET — reads built output; 84 packages have nodist/, needs a whole-repopnpm build). Its own words: "⛔ This is NOT a pass: nothing was measured."pnpm check:type-check-debt— NOT MEASURED (exit 3: PREREQUISITE NOT MET — needs the fullturbo run buildclosure over./packages/*). No ledger number is read from this run in either direction.pnpm check:docs-transcript-drift— first run exit 3 (@objectstack/lintnot built; the closure build is^..., which excludes the package itself). Re-run afterpnpm --filter @objectstack/lint buildthrough the lock:EXIT=0—✓ check-docs-transcript-drift: 4 declared transcript value(s) across 405 page(s) … equal what the registry derives today. Measured.turbo ls --affected(base7dafaaedd) names 49 packages, because@objectstack/lintis a wide dependency. Only@objectstack/lintwas run locally (test + typecheck, both green). The 48 downstream packages consume@objectstack/lint's exports, and this change alters no export, signature or type —git grep validateStackExpressions -- ':!packages/lint'finds only changelogs, docs and ADR prose, no consumer test. CI runs the farm.Out of scope, and it could not be filed
The card body flagged two more inline casts in this file (
flow.nodes,graph.nodes) as unmeasured at filing time. They were measured on this branch, i.e. WITH the fix above, by calling the rule directly:This is a real, separate defect and is not repaired here: no sweep can express a flow's inner node list today (
non-record-object-entry.test.tsdrives collections), and forgraph.nodesthe producer iscollectFlowGraphsin@objectstack/spec/automation, so the contract-first question of where the repair belongs needs a ruling rather than a second consumer-side guard. Attempting to file it as a new card was refused in this box (the issue-creation call was blocked by the permission classifier, and a dedup search over open + closed issues found no existing card), so the full text is handed to the PM in theos-dev-reportcomment for filing. #15742 is the only card this PR addresses.🤖 Generated with Claude Code
https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
Generated by Claude Code