|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// One collection coercion, in one place (#15636). |
| 4 | +// |
| 5 | +// A stack collection is authored either as a list or as a name-keyed map, so |
| 6 | +// every rule that reads one has to coerce `unknown` into `AnyRec[]` first. That |
| 7 | +// coercion is ONE decision — what to do with a member that is not a record — |
| 8 | +// and `recordsOf` in `object-graph.ts` is where it is made: a non-record member |
| 9 | +// of the ARRAY shape is dropped whole (it carries no key, so it is nothing at |
| 10 | +// all), while the map shape keeps the author's key and drops only its |
| 11 | +// unreadable body. Its docblock argues both branches; this file only pins that |
| 12 | +// the decision has one home. |
| 13 | +// |
| 14 | +// ## Why a source-text gate and not a code review |
| 15 | +// |
| 16 | +// The decision had 40 homes. #15494 guarded the seam every field-path rule |
| 17 | +// opens with, and re-measuring the whole `AUTHORING_RULES` table over |
| 18 | +// `{ objects: [null, validObject] }` still counted 13 of 42 rules throwing, |
| 19 | +// through eleven more reader sites — every one of them a hand-copied `asArray` |
| 20 | +// whose array branch was spelled `return v as AnyRec[]`, unchecked. #15552 |
| 21 | +// re-pointed the `stack.objects` readers; #15636 re-pointed 22 more, one per |
| 22 | +// collection family (`flows`, `pages`, `dashboards`, `datasets`, `apps`, |
| 23 | +// `permissions`, `capabilities`, `data`, `hooks`, `views`, `actions`, |
| 24 | +// `translations`, and the per-object sub-collections). |
| 25 | +// |
| 26 | +// The 39 copies were not identical, which is the part worth pinning. Twelve had |
| 27 | +// already grown the array-branch filter LOCALLY, in two different spellings; |
| 28 | +// four more read only the list shape and lean on an `if (!page) continue` three |
| 29 | +// lines down; and two are load-bearing for a finding PATH rather than for a |
| 30 | +// crash. A fix applied to some copies and not their siblings is the whole |
| 31 | +// failure mode restated as evidence: a predicate with N copies is N chances to |
| 32 | +// fix one and leave N-1, and no reviewer counts to 39. So the count is asserted |
| 33 | +// here instead. |
| 34 | +// |
| 35 | +// ## The three clauses, and what each one refuses |
| 36 | +// |
| 37 | +// 1. `object-graph.ts` declares exactly one such coercion, named `recordsOf`. |
| 38 | +// Without this the other two clauses could pass over a package that had |
| 39 | +// lost the canonical one entirely. |
| 40 | +// 2. Every OTHER module declaring one is in `COPY_LEDGER`. This is the ratchet |
| 41 | +// and it is exact in BOTH directions: a new copy fails because it is not |
| 42 | +// listed, and a copy that has been re-pointed fails because its ledger row |
| 43 | +// is now a lie. The ledger may only shrink, and shrinking it is one line. |
| 44 | +// 3. No coercion carries the UNCHECKED array branch — the spelling that |
| 45 | +// actually crashes — outside `UNGUARDED_ALLOWANCE`. Clause 2 alone would let |
| 46 | +// a re-introduced copy through as long as someone added a ledger row; clause |
| 47 | +// 3 is what refuses the defect itself regardless of bookkeeping. |
| 48 | +// |
| 49 | +// Both allowances are DATED and name the change that deletes them, and both are |
| 50 | +// exact in both directions: the day an allowed file is re-pointed, this test |
| 51 | +// fails until its row goes, so an allowance cannot outlive its reason by being |
| 52 | +// forgotten. That is load-bearing, not decoration — `validate-chart-bindings.ts` |
| 53 | +// was carried in both lists for one change, #15741 re-pointed it, and both of |
| 54 | +// its rows came out because this test went red, not because anyone remembered. |
| 55 | +// |
| 56 | +// Scope: `src/*.ts` excluding tests. A coercion inside a test file is a fixture, |
| 57 | +// not a reader, and no tenant stack reaches it. |
| 58 | +import { readFileSync, readdirSync } from 'node:fs'; |
| 59 | +import { basename, dirname, join } from 'node:path'; |
| 60 | +import { fileURLToPath } from 'node:url'; |
| 61 | +import { describe, expect, it } from 'vitest'; |
| 62 | + |
| 63 | +const SRC_DIR = dirname(fileURLToPath(import.meta.url)); |
| 64 | +const SELF = basename(fileURLToPath(import.meta.url)); |
| 65 | + |
| 66 | +/** |
| 67 | + * A local collection coercion, matched by SHAPE rather than by name: a |
| 68 | + * module-level `(v: unknown) => AnyRec[]`, in either the `function` or the |
| 69 | + * arrow spelling. Matching the shape and not the identifier `asArray` is what |
| 70 | + * makes clause 2 hold against a copy that renames itself. |
| 71 | + */ |
| 72 | +const COERCION = |
| 73 | + /(?:function\s+(\w+)\s*\(\s*v:\s*unknown\s*\)\s*:\s*AnyRec\[\]|const\s+(\w+)\s*=\s*\(\s*v:\s*unknown\s*\)\s*:\s*AnyRec\[\])/g; |
| 74 | + |
| 75 | +/** |
| 76 | + * The array branch that crashes: `Array.isArray` proves it is a LIST and the |
| 77 | + * cast then asserts every MEMBER is a record, which a list out of YAML does not |
| 78 | + * promise. The back-reference keeps this to a cast of the same binding that was |
| 79 | + * just tested, and the test below is applied to a coercion's own body — never |
| 80 | + * to a whole file, or every inline `x as AnyRec[]` a rule writes for a field it |
| 81 | + * has already narrowed would answer for this predicate. |
| 82 | + */ |
| 83 | +const UNCHECKED_ARRAY_BRANCH = /Array\.isArray\((\w+)\)\s*\)?\s*(?:return|\?)\s*\(?\s*\1\s+as\s+AnyRec\[\]/; |
| 84 | + |
| 85 | +/** Where the one coercion lives. */ |
| 86 | +const CANONICAL_MODULE = 'object-graph.ts'; |
| 87 | +const CANONICAL_NAME = 'recordsOf'; |
| 88 | + |
| 89 | +/** |
| 90 | + * Modules still holding a private copy, each with the issue that removes it. |
| 91 | + * Rows may be DELETED as copies are re-pointed and must never be added: a new |
| 92 | + * entry here is a new copy of the predicate, which is the defect this file |
| 93 | + * exists to refuse. Every row is asserted to still be true below. |
| 94 | + */ |
| 95 | +const COPY_LEDGER: Readonly<Record<string, string>> = { |
| 96 | + // 2026-09-05 — the two reference-integrity members #15494 deliberately left |
| 97 | + // walking the RAW array. Their own loop guards each member with `isRec`, so |
| 98 | + // neither ever threw; what the copy buys them is the INDEX, because |
| 99 | + // `reference-integrity-suite.test.ts` pins their finding paths |
| 100 | + // (`objects[1].highlightFields[1]`) against the author's file and `recordsOf` |
| 101 | + // renumbers past a dropped member. Re-pointing them is blocked on an |
| 102 | + // index-preserving reader, not on anyone's attention (#15740). |
| 103 | + 'validate-object-field-refs.ts': '#15740', |
| 104 | + 'validate-list-view-field-refs.ts': '#15740', |
| 105 | + // 2026-09-05 — the sixteen copies that do not crash today: twelve grew a |
| 106 | + // local array-branch filter and four read only the list shape behind a |
| 107 | + // call-site `if (!page) continue`. They are not #15636's defect; they are its |
| 108 | + // cause, and re-pointing them is bookkeeping this ledger now forces. |
| 109 | + 'validate-action-body-writes.ts': '#15728', |
| 110 | + 'validate-ai-agent-authoring.ts': '#15728', |
| 111 | + 'validate-ai-surface-affinity.ts': '#15728', |
| 112 | + 'validate-ai-tool-references.ts': '#15728', |
| 113 | + 'validate-flow-node-writes.ts': '#15728', |
| 114 | + 'validate-hook-body-writes.ts': '#15728', |
| 115 | + 'validate-jsx-pages.ts': '#15728', |
| 116 | + 'validate-nav-object-servability.ts': '#15728', |
| 117 | + 'validate-nav-target-refs.ts': '#15728', |
| 118 | + 'validate-page-source-styling.ts': '#15728', |
| 119 | + 'validate-page-visualization-bindings.ts': '#15728', |
| 120 | + 'validate-react-page-props.ts': '#15728', |
| 121 | + 'validate-react-pages.ts': '#15728', |
| 122 | + 'validate-readonly-action-writes.ts': '#15728', |
| 123 | + 'validate-rule-compilability.ts': '#15728', |
| 124 | + 'validate-view-page-refs.ts': '#15728', |
| 125 | +}; |
| 126 | + |
| 127 | +/** |
| 128 | + * The coercions still spelling the array branch unchecked, dated and named by |
| 129 | + * the change that removes each. What remains is unchecked at the COERCION and |
| 130 | + * guarded at the CALL SITE — the two reference-integrity members re-test every |
| 131 | + * member with `isRec` inside their loop, and the four page walks skip on |
| 132 | + * `if (!page) continue` three lines down — so a junk member costs none of them |
| 133 | + * anything today. That is a guard standing somewhere the reader does not |
| 134 | + * promise it, which is why they are allowed rather than accepted: each is still |
| 135 | + * a copy of a predicate that has a home, and none may grow a sibling. |
| 136 | + */ |
| 137 | +const UNGUARDED_ALLOWANCE: Readonly<Record<string, string>> = { |
| 138 | + // 2026-09-05 — removed by #15740, which needs an index-preserving reader |
| 139 | + // first; both guard every member with `isRec` at the call site. |
| 140 | + 'validate-object-field-refs.ts': '#15740', |
| 141 | + 'validate-list-view-field-refs.ts': '#15740', |
| 142 | + // 2026-09-05 — removed by #15728. |
| 143 | + 'validate-jsx-pages.ts': '#15728', |
| 144 | + 'validate-page-source-styling.ts': '#15728', |
| 145 | + 'validate-react-page-props.ts': '#15728', |
| 146 | + 'validate-react-pages.ts': '#15728', |
| 147 | +}; |
| 148 | + |
| 149 | +/** Every rule/reader module — tests excluded, this file excluded. */ |
| 150 | +const modules = (): string[] => |
| 151 | + readdirSync(SRC_DIR) |
| 152 | + .filter((f) => f.endsWith('.ts') && !f.endsWith('.test.ts') && !f.endsWith('.d.ts') && f !== SELF) |
| 153 | + .sort(); |
| 154 | + |
| 155 | +const read = (file: string): string => readFileSync(join(SRC_DIR, file), 'utf8'); |
| 156 | + |
| 157 | +/** |
| 158 | + * One coercion's own body: from its declaration to the first line-initial `}` |
| 159 | + * (the `function` form), capped at twelve lines (the arrow form is one |
| 160 | + * statement, and no spelling of this helper in the package runs longer). |
| 161 | + */ |
| 162 | +const bodyAt = (src: string, index: number): string => { |
| 163 | + const window = src.slice(index).split('\n').slice(0, 12); |
| 164 | + const close = window.findIndex((line, i) => i > 0 && line === '}'); |
| 165 | + return (close >= 0 ? window.slice(0, close + 1) : window).join('\n'); |
| 166 | +}; |
| 167 | + |
| 168 | +/** The coercions a module declares, by name. */ |
| 169 | +const coercionsIn = (file: string): string[] => |
| 170 | + [...read(file).matchAll(COERCION)].map((m) => m[1] ?? m[2]); |
| 171 | + |
| 172 | +/** Whether any coercion this module declares casts its array branch unchecked. */ |
| 173 | +const castsUnchecked = (file: string): boolean => { |
| 174 | + const src = read(file); |
| 175 | + return [...src.matchAll(COERCION)].some((m) => UNCHECKED_ARRAY_BRANCH.test(bodyAt(src, m.index ?? 0))); |
| 176 | +}; |
| 177 | + |
| 178 | +describe('one collection coercion, in one place (#15636)', () => { |
| 179 | + /** |
| 180 | + * The floor first: a scan that found nothing would satisfy every assertion |
| 181 | + * below vacuously. A reading under 50 means the discovery changed, not the |
| 182 | + * package. |
| 183 | + */ |
| 184 | + it('reads the rule modules it claims to scan', () => { |
| 185 | + expect(modules().length).toBeGreaterThanOrEqual(50); |
| 186 | + expect(modules()).toContain(CANONICAL_MODULE); |
| 187 | + }); |
| 188 | + |
| 189 | + it(`declares the coercion once, as \`${CANONICAL_NAME}\` in \`${CANONICAL_MODULE}\``, () => { |
| 190 | + expect(coercionsIn(CANONICAL_MODULE)).toEqual([CANONICAL_NAME]); |
| 191 | + }); |
| 192 | + |
| 193 | + it('holds no copy that the ledger does not name', () => { |
| 194 | + const unlisted = modules() |
| 195 | + .filter((f) => f !== CANONICAL_MODULE && coercionsIn(f).length > 0) |
| 196 | + .filter((f) => !(f in COPY_LEDGER)); |
| 197 | + expect( |
| 198 | + unlisted, |
| 199 | + `${unlisted.join(', ')} declares its own \`(v: unknown) => AnyRec[]\`. Read the collection ` + |
| 200 | + `through \`recordsOf\` from './object-graph.js' instead — a second copy of this predicate ` + |
| 201 | + `is a second place to forget the non-record filter (#15636).`, |
| 202 | + ).toEqual([]); |
| 203 | + }); |
| 204 | + |
| 205 | + it('names no copy the ledger has outlived', () => { |
| 206 | + const stale = Object.keys(COPY_LEDGER) |
| 207 | + .sort() |
| 208 | + .filter((f) => coercionsIn(f).length === 0); |
| 209 | + expect( |
| 210 | + stale, |
| 211 | + `${stale.join(', ')} no longer declares a private coercion. Delete its COPY_LEDGER row — ` + |
| 212 | + `a ledger that outlives its subject stops describing the package and starts excusing it.`, |
| 213 | + ).toEqual([]); |
| 214 | + }); |
| 215 | + |
| 216 | + it('carries no unchecked array branch outside the dated allowance', () => { |
| 217 | + const offenders = modules() |
| 218 | + .filter((f) => castsUnchecked(f)) |
| 219 | + .filter((f) => !(f in UNGUARDED_ALLOWANCE)); |
| 220 | + expect( |
| 221 | + offenders, |
| 222 | + `${offenders.join(', ')} casts an array to \`AnyRec[]\` without filtering its members. ` + |
| 223 | + `A YAML list item left empty deserialises to \`null\`, and the next property read throws ` + |
| 224 | + `out of a rule that is contractually \`(stack) => Finding[]\` (#15636). Use \`recordsOf\`.`, |
| 225 | + ).toEqual([]); |
| 226 | + }); |
| 227 | + |
| 228 | + it('allows no unchecked branch the allowance has outlived', () => { |
| 229 | + const stale = Object.keys(UNGUARDED_ALLOWANCE) |
| 230 | + .sort() |
| 231 | + .filter((f) => !castsUnchecked(f)); |
| 232 | + expect( |
| 233 | + stale, |
| 234 | + `${stale.join(', ')} no longer casts unchecked. Delete its UNGUARDED_ALLOWANCE row — the ` + |
| 235 | + `allowance was dated to the change that removes it, not granted to the file.`, |
| 236 | + ).toEqual([]); |
| 237 | + }); |
| 238 | +}); |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?v=4&size=40){kind=avatar}
0 commit comments