diff --git a/.changeset/liveness-citation-line-bound.md b/.changeset/liveness-citation-line-bound.md new file mode 100644 index 0000000000..37b25fddd0 --- /dev/null +++ b/.changeset/liveness-citation-line-bound.md @@ -0,0 +1,26 @@ +--- +'@objectstack/spec': patch +--- + +liveness gate: bound an evidence citation's LINE, not just its file + +`check:liveness` resolved a `path/to/file.ts:NNN` pointer with `existsSync` on the +path alone — the parser did not even retain the `:NNN` — so a consumer that moved +out of a file which still exists kept a passing pointer, was counted under the word +"resolved", and left its ledger entry reading as freshly verified. A citation that is +dead but precise-looking is worse than a missing one: it survives review, and the next +agent re-verifying the entry follows it, finds nothing, and rebuilds the call graph +from scratch. + +Citations are now bounded by the cited file's length, for `evidence` and `producer` +alike (they already share one resolver). A range `:12-34` is bounded by its END. Every +citation in a `+`-joined multi-consumer entry is bounded, not just the first. +Cross-repo attributions (`objectui: …`, `cloud: …`) are still counted and never +resolved. The run prints how many citations it checked beside how many are in range, +so a parser that degraded to extracting nothing cannot read as a pass. + +Two shipped instances, both repaired here and both real: +`permission.tabPermissions` cited `hono-plugin.ts:1200` in a 717-line file that no +longer mentions the property (all three of its pointers were dead — one past EOF, two +within bounds), and `mapping.fieldMapping` cited a range ending three lines past the +end of `import-mapping.ts`. diff --git a/packages/spec/liveness/mapping.json b/packages/spec/liveness/mapping.json index 094d245760..b04242281e 100644 --- a/packages/spec/liveness/mapping.json +++ b/packages/spec/liveness/mapping.json @@ -28,9 +28,9 @@ }, "fieldMapping": { "status": "live", - "verifiedAt": "2026-08-01", - "evidence": "packages/rest/src/import-mapping.ts:98-105, packages/rest/src/import-mapping.ts:115-167", - "note": "the pipeline itself: source/target/transform/params all consumed. none/constant/map/split/join applied in applyMappingToRows (`params.separator` :124, `.value` :132, `.valueMap` :137); `lookup` copies through for the pipeline's metaMap reference resolution; `javascript` is REJECTED with a 400 (no server sandbox — implement-or-reject-loudly). SUB-WALK BOUNDARY, resolved: `params`' lookup-specific keys (`object`/`fromField`/`toField`/`autoCreate`) were read by nothing — reference resolution comes from the target object's own field definitions, not from these — and were REMOVED in the 17.x line (#10329, ADR-0049 enforce-or-remove; strict deletion, so nothing dead remains one level below the drill). Every spelling — the four canonical keys and their eleven ex-aliases — now lands on a guidance prescription at parse; the mapping-lookup-params-removed conversion (protocol 18) strips them from stored sources. The surviving params keys (`value`/`valueMap`/`separator`) are all consumed by applyMappingToRows." + "verifiedAt": "2026-08-23", + "evidence": "packages/rest/src/import-mapping.ts:95-102, packages/rest/src/import-mapping.ts:112-164", + "note": "Lines re-measured 2026-08-23 (#11210): the second range ended at :167 in a 164-line file — the file shrank under the citation and no gate could see it, since the FILE still resolved. The consumer never moved: the javascript rejection is the loop at :95-102 (was :98-105) and the pipeline is applyMappingToRows at :112-164 (was :115-167); the three inline line refs below were off by the same three lines and are corrected with them. the pipeline itself: source/target/transform/params all consumed. none/constant/map/split/join applied in applyMappingToRows (`params.separator` :121, `.value` :129, `.valueMap` :134); `lookup` copies through for the pipeline's metaMap reference resolution; `javascript` is REJECTED with a 400 (no server sandbox — implement-or-reject-loudly). SUB-WALK BOUNDARY, resolved: `params`' lookup-specific keys (`object`/`fromField`/`toField`/`autoCreate`) were read by nothing — reference resolution comes from the target object's own field definitions, not from these — and were REMOVED in the 17.x line (#10329, ADR-0049 enforce-or-remove; strict deletion, so nothing dead remains one level below the drill). Every spelling — the four canonical keys and their eleven ex-aliases — now lands on a guidance prescription at parse; the mapping-lookup-params-removed conversion (protocol 18) strips them from stored sources. The surviving params keys (`value`/`valueMap`/`separator`) are all consumed by applyMappingToRows." }, "mode": { "status": "live", diff --git a/packages/spec/liveness/permission.json b/packages/spec/liveness/permission.json index edf8782b53..2e626bf962 100644 --- a/packages/spec/liveness/permission.json +++ b/packages/spec/liveness/permission.json @@ -151,10 +151,10 @@ }, "tabPermissions": { "status": "live", - "verifiedAt": "2026-07-30", + "verifiedAt": "2026-08-23", "proof": "packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts#me-apps-and-everyone-baseline", - "evidence": "packages/plugins/plugin-hono-server/src/hono-plugin.ts:1200 (tabRank merges all four values — hidden/default_off/default_on/visible — across resolved sets, most-visible wins; app hidden ⇒ dropped from /me/apps) + packages/runtime/src/security/resolve-execution-context.ts:205 + packages/rest/src/rest-server.ts:1551 (projection)", - "note": "Re-verified 2026-07-30 — the previous note ('only hidden is read') UNDERSTATED it: the rank merge reads every value. Proof BOUND the same day after the dogfood test was extended to actually AUTHOR the property on a permission set (it had only mentioned tabPermissions in its header): hidden drops the app from /me/apps, a more-visible sibling grant wins it back." + "evidence": "packages/plugins/plugin-hono-server/src/current-user-endpoints.ts:947 (tabRank ranks all four values — hidden 0 < default_off 1 < default_on 2 < visible 3) + packages/plugins/plugin-hono-server/src/current-user-endpoints.ts:952-958 (the merge across the caller's resolved permission sets, most-visible wins; an unrecognised value is skipped rather than defaulted) + packages/plugins/plugin-hono-server/src/current-user-endpoints.ts:963 (the /me/apps drop — tabs[app.name] === 'hidden' ⇒ the app is not returned, and this filter runs BEFORE the fail-open branch, so it survives a missing SecurityPlugin) + packages/plugins/plugin-hono-server/src/current-user-endpoints.ts:772 (the same rank table for the /auth/me/permissions half) + packages/plugins/plugin-hono-server/src/current-user-endpoints.ts:862 (projected onto that response) + packages/plugins/plugin-security/src/permission-set-projection.ts:287 (the sys_permission_set.tab_permissions column parsed onto the resolved set) + packages/core/src/security/assemble-execution-context.ts:328 (onto ExecutionContext.tabPermissions)", + "note": "Re-verified 2026-08-23 (#11210) and every pointer REPLACED — all three of the previous ones were dead, in two different ways, and the gate could see neither. (a) `plugin-hono-server/src/hono-plugin.ts:1200` was past EOF: that file is 717 lines and contains no `tabPermissions` reference at all — the same code movement that killed this entry's sibling `systemPermissions` pointer (#10959/#11209) moved /me/apps and /auth/me/permissions into `current-user-endpoints.ts`. (b) `runtime/src/security/resolve-execution-context.ts:205` and `rest/src/rest-server.ts:1551` were dead WITHIN bounds — both files are long enough for the cited line, and neither mentions `tabPermissions` anywhere (0 occurrences, measured); the handler's own comment at current-user-endpoints.ts:922-926 records why, namely that resolveCtx() carries neither systemPermissions nor tabPermissions, so /me/apps re-resolves the sets itself. THE VERDICT IS UNCHANGED and was never in question — `live` with a bound dogfood proof, and the behaviour still ships; what was dead is the citation, which is exactly the rot #11210's line bound now makes impossible to leave behind silently. The 2026-07-30 reading survives the re-measurement verbatim: the rank merge reads all four values, not only 'hidden'. Two consumers, deliberately both cited: /me/apps ENFORCES (a hidden app is dropped), /auth/me/permissions PROJECTS the merged map for the renderer." }, "rowLevelSecurity": { "children": { diff --git a/packages/spec/scripts/liveness/check-liveness.mts b/packages/spec/scripts/liveness/check-liveness.mts index e8df793eb0..4e34215ca2 100644 --- a/packages/spec/scripts/liveness/check-liveness.mts +++ b/packages/spec/scripts/liveness/check-liveness.mts @@ -52,6 +52,19 @@ // flagged 48 of 227 entries with a ~100% false-positive rate, so failing on it // would have failed every build. The parse fix is what turned a hit into signal. // +// CITATION LINES (#11210): a citation written `path/to/file.ts:NNN` also has its +// LINE bounded by the file's length. The existence check alone validated the +// file and dropped the line on the floor — the parser did not even retain it — +// so a consumer that moved out of a file which still exists kept a passing +// pointer, was counted under the word "resolved", and left the entry reading as +// freshly verified. Two shipped instances, both real, neither catchable by any +// gate: `permission.tabPermissions` cited `hono-plugin.ts:1200` in a 717-line +// file that no longer mentions the property, and `mapping.fieldMapping` cited a +// range ending 3 lines past the end of `import-mapping.ts`. A range is bounded +// by its END. The complementary case — a consumer that moved WITHIN its file — +// is out of reach here by construction and is tracked separately; the ✗ text +// below says why its obvious detector is not obviously right. +// // PRODUCER-SIDE EVIDENCE (`producer`, #4837): `live` means AUTHORING the // property changes runtime behaviour. A consumer that reads the property is // necessary and not sufficient — when the effect also depends on a second input @@ -122,7 +135,7 @@ import { type VerificationEntry, type VerificationReport, } from './verification.mts'; -import { checkEvidence } from './evidence.mts'; +import { checkCitationLines, checkEvidence, countLines, type EvidenceScan } from './evidence.mts'; import { buildProducerReport, type ProducerEntry, type ProducerReport } from './producer.mts'; import { ORPHAN_GUIDANCE, findOrphanEntries, type Orphan } from './orphans.mts'; import { @@ -423,6 +436,12 @@ const report: any = { evidenceLocal: 0, // repo-rooted evidence paths attributed to THIS repo — DECLARED, not yet proven evidenceMissing: 0, // ...of which this many do not exist here (=== staleEvidence.length) — FAILS the gate evidenceForeign: 0, // evidence paths attributed to objectui / cloud — not resolvable here, never failed + // The LINE half of a citation. `evidenceMissing` above answers "does the cited + // FILE exist"; nothing answered "does the cited LINE". A consumer that moves out + // of a file which still exists therefore rotted its pointer in complete silence, + // and the summary line reported it under "resolved". + citationsChecked: 0, // local citations written `path:NNN` — the subset a line bound can falsify + citationsOutOfRange: [] as string[], // ...of which this many name a line past EOF — FAILS the gate }; // Every classified entry, for the `verifiedAt` fold below. Collected during the @@ -434,6 +453,32 @@ const producerEntries: ProducerEntry[] = []; const proofFs = { existsSync, readFileSync }; +/** + * How many lines the cited file has, or `null` when it cannot be read. `null` + * is not a pass: it means the EXISTENCE check owns this path's verdict already + * (a rotted `evidence` pointer would otherwise be reported twice, once per + * check, which teaches a reader to discount both lists). + */ +function lineCountOf(p: string): number | null { + const f = join(repoRoot, p); + if (!existsSync(f)) return null; + return countLines(readFileSync(f, 'utf8')); +} + +/** + * Bound a scanned pointer's `path:NNN` citations by the cited files' lengths. + * `producer` and `evidence` both come through here, deliberately: the producer + * pointer resolves through the SAME resolver as evidence (see below), and a + * standard that applied to one and not the other would leave the weaker one as + * the place a rotted line goes to hide. + */ +function collectOutOfRange(scan: EvidenceScan, label: string): void { + report.citationsChecked += scan.localCitations.length; + for (const c of checkCitationLines(scan, lineCountOf)) { + report.citationsOutOfRange.push(`${label} → ${c.path}:${c.line} (the file has ${c.lines} lines)`); + } +} + function classify(type: string, path: string, status: string, led: any, cat: any) { cat.classified++; cat.byStatus[status] = (cat.byStatus[status] || 0) + 1; @@ -455,6 +500,7 @@ function classify(type: string, path: string, status: string, led: any, cat: any if (typeof led?.producer === 'string') { const pv = checkEvidence(led.producer, (p) => existsSync(join(repoRoot, p))); for (const miss of pv.missing) report.producerMissing.push(`${type}/${path} → ${miss}`); + collectOutOfRange(pv, `${type}/${path} [producer]`); } if (status === 'live' && led?.evidence) { // Extract every repo-rooted path the evidence claims and resolve the ones @@ -466,6 +512,7 @@ function classify(type: string, path: string, status: string, led: any, cat: any report.evidenceLocal += ev.local.length; report.evidenceMissing += ev.missing.length; for (const miss of ev.missing) report.staleEvidence.push(`${type}/${path} → ${miss}`); + collectOutOfRange(ev, `${type}/${path}`); } // ── ADR-0054 prove-it-runs ── const boundClass = BOUND_PROOF_PATHS.get(`${type}/${path}`); @@ -702,6 +749,11 @@ const failed = // that now resolves 330 paths and reports zero. Cross-repo attribution never // reaches this list: checkEvidence only resolves the LOCAL bucket. report.staleEvidence.length > 0 || + // ...and the same verdict for a citation whose FILE exists but whose LINE does + // not. Red rather than ⚠ from the start: unlike the #5623 case there is no + // false-positive era to calibrate against — a line past EOF is arithmetic, and + // the two shipped instances it found were both real (#11210). + report.citationsOutOfRange.length > 0 || report.orphanEntries.length > 0 || report.verification.errors.length > 0 || report.producers.errors.length > 0 || @@ -738,6 +790,40 @@ if (asJson) { (report.evidenceMissing ? `, ${report.evidenceMissing} MISSING` : '') + `; ${report.evidenceForeign} attributed to another repo (objectui / cloud — not resolvable here).`, ); + // Same two-number discipline one level down, and for the same reason: printing + // only "in range" would read as a pass on a run where the parser had degraded + // to extracting no citations at all. + console.log( + `line citations: ${report.citationsChecked} pointer(s) written \`path:NNN\`, ` + + `${report.citationsChecked - report.citationsOutOfRange.length} inside the cited file` + + (report.citationsOutOfRange.length ? `, ${report.citationsOutOfRange.length} PAST EOF` : '') + '.', + ); + if (report.citationsOutOfRange.length) { + console.log(`\n✗ ${report.citationsOutOfRange.length} citation(s) name a line the cited file does not have:`); + report.citationsOutOfRange.forEach((s: string) => console.log(` ${s}`)); + console.log( + '\n The file resolves, so the existence check above is happy — and the line is gone,\n' + + ' which is the half a moved consumer rots first. This is worse than a missing file:\n' + + ' it survives review because it LOOKS precise, and the next agent re-verifying the\n' + + ' entry follows it, finds nothing, and has to rebuild the call graph from scratch.\n\n' + + ' Repairs, same three as a missing file and picked the same way:\n' + + ' • the consumer MOVED (inside this file or out of it) → repoint at the real line,\n' + + ' MEASURED — open the file and read it, do not shift the number by the diff — and\n' + + ' stamp `verifiedAt` while you have the call graph open;\n' + + ' • the consumer moved to ANOTHER repo → attribute it with a realm marker; those\n' + + ' are counted, never resolved, and never bounded here;\n' + + ' • the consumer is GONE → the verdict is not `live` any more. Re-classify under\n' + + ' ADR-0049 enforce-or-remove rather than repointing at a plausible survivor.\n\n' + + ' A RANGE (`:12-34`) is bounded by its END: a range whose tail is past EOF overruns\n' + + ' the file even when its head is inside.\n\n' + + ' What this check does NOT see: a consumer that moved WITHIN the file it is cited to.\n' + + ' The line still exists, so nothing here fires. That case needs a different signal\n' + + ' (does the cited file mention the property at all) whose false positives are a\n' + + ' design problem of their own — camelCase authoring keys are read as snake_case data\n' + + ' values throughout this platform, so a naive match on the key misses the consumer\n' + + ' for every persisted field. Measured, not assumed: see #11210.', + ); + } if (report.staleEvidence.length) { console.log(`\n✗ ${report.staleEvidence.length} 'live' entr(ies) cite a file that is missing from THIS repo:`); report.staleEvidence.forEach((s: string) => console.log(` ${s}`)); @@ -992,7 +1078,8 @@ if (asJson) { '\n✓ every governed-type property at the walk\'s one-level granularity is classified, every ' + 'registered type is governed or explicitly pending, no ledger row outlives its property, ' + 'every container inheritance is declared, every `live` entry\'s repo-local evidence path ' + - 'resolves, all bound high-risk proofs resolve, and the README state table carries a row ' + + 'resolves and every `path:NNN` citation names a line that file actually has, all bound ' + + 'high-risk proofs resolve, and the README state table carries a row ' + `for each of the ${report.readmeRowCount} governed type(s) it claims to index.`, ); console.log( diff --git a/packages/spec/scripts/liveness/check-liveness.test.ts b/packages/spec/scripts/liveness/check-liveness.test.ts index 8151c9c08d..50b8f7481e 100644 --- a/packages/spec/scripts/liveness/check-liveness.test.ts +++ b/packages/spec/scripts/liveness/check-liveness.test.ts @@ -122,6 +122,73 @@ describe('check:liveness — evidence pointers (#5623)', () => { expect(output).not.toContain('cite a file that is missing'); }); + // #11210. Everything above validates the FILE half of a citation. These pin + // the LINE half, and they run against the real gate for the same reason the + // cases above do: the grading (✗ vs ⚠, exit 1 vs exit 0) lives in + // check-liveness.mts, and #5623's defect was a check that named its findings + // correctly and exited 0 anyway. + it('FAILS when a citation names a line past the end of a file that EXISTS', () => { + const root = path.join(tmp, 'past-eof'); + cpSync(LEDGERS, root, { recursive: true }); + // A real file, so the existence check is satisfied and this run has exactly + // one cause for its verdict — the defect itself: file resolves, line is gone. + setEvidence(root, 'query', 'limit', 'packages/spec/scripts/liveness/evidence.mts:99999'); + + const { status, output } = runGate(root); + expect(status, output).toBe(1); + expect(output).toContain('citation(s) name a line the cited file does not have'); + expect(output).toContain('query/limit → packages/spec/scripts/liveness/evidence.mts:99999'); + // ✗, not ⚠ — same grading argument as the missing-file case above. + expect(output).toMatch(/✗ 1 citation\(s\) name a line/); + expect(output).not.toMatch(/⚠ \d+ citation\(s\) name a line/); + // And it must NOT be reported as a missing FILE: two checks, two verdicts. + expect(output).not.toContain('query/limit → packages/spec/scripts/liveness/evidence.mts\n'); + }); + + it('bounds EVERY citation in a concatenated entry, not just the first', () => { + // The dispatch-critical case: entries `+`-join several citations, so a + // parser that stopped at the head would leave the tail unfalsifiable — the + // exact shape of the shipped `permission.tabPermissions` string. + const root = path.join(tmp, 'past-eof-tail'); + cpSync(LEDGERS, root, { recursive: true }); + setEvidence( + root, + 'query', + 'limit', + 'packages/spec/scripts/liveness/evidence.mts:1 (fine) + packages/spec/scripts/liveness/orphans.mts:88888 (rotted tail)', + ); + + const { status, output } = runGate(root); + expect(status, output).toBe(1); + expect(output).toContain('query/limit → packages/spec/scripts/liveness/orphans.mts:88888'); + }); + + it('never bounds a citation attributed to ANOTHER repo', () => { + // Same boundary as the existence check: those files are absent here, so + // every line in them would read as past EOF and the gate would become + // unsatisfiable for every renderer-side property. + const root = path.join(tmp, 'foreign-line'); + cpSync(LEDGERS, root, { recursive: true }); + setEvidence(root, 'query', 'limit', 'objectui: packages/app-shell/src/RecordDetailView.tsx:99999'); + + const { status, output } = runGate(root); + expect(status, output).toBe(0); + expect(output).not.toContain('name a line the cited file does not have'); + }); + + it('prints the citation count and how many are in range, equal on a green run', () => { + // The #5623 lesson applied to the new counter: printing only "in range" + // would read as a pass on a run where the parser extracted no citations. + const { status, output } = runGate(path.join(tmp, 'liveness')); + expect(status, output).toBe(0); + const line = output.split('\n').find((l) => l.startsWith('line citations:')) ?? ''; + const m = /line citations: (\d+) pointer\(s\) written .*?, (\d+) inside the cited file/.exec(line); + expect(m, line).not.toBeNull(); + expect(Number(m![1])).toBeGreaterThan(100); + expect(m![2]).toBe(m![1]); + expect(line).not.toContain('PAST EOF'); + }); + it('still fails a local path that shares a string with a foreign clause', () => { // A realm marker's scope ends at the clause boundary. If it did not, one // `objectui:` anywhere in an entry would silence the whole entry — a diff --git a/packages/spec/scripts/liveness/evidence.mts b/packages/spec/scripts/liveness/evidence.mts index 03ee5d4910..7026b6a6f4 100644 --- a/packages/spec/scripts/liveness/evidence.mts +++ b/packages/spec/scripts/liveness/evidence.mts @@ -46,24 +46,57 @@ export const FOREIGN_PATH_PREFIXES = ['packages/services/service-ai/']; const PATH_RE = new RegExp(`^(?:${REPO_ROOTS.join('|')})/[\\w.@-]+(?:/[\\w.@-]+)*\\.[a-zA-Z]{1,5}$`); +/** A local path citation that names a line: `packages/…/file.ts:150`. */ +export interface EvidenceCitation { + /** The path, exactly as it appears in `local`. */ + path: string; + /** + * The line the citation names. For a RANGE (`:12-34`) this is the END: a range + * whose start is inside the file but whose end is past EOF still overruns it, + * and a start past EOF implies an end past EOF — so the end is both the + * stricter bound and the one that subsumes the other. + */ + line: number; +} + export interface EvidenceScan { /** Repo-rooted paths attributed to THIS repo — these must resolve. */ local: string[]; /** Paths attributed to another repo (realm marker or foreign prefix) — not resolved here. */ foreign: string[]; + /** + * The subset of `local` whose citation names a line, paired with that line — + * the only citations a line-bound check can falsify. Deduped on `path:line`, + * not on path: one evidence string routinely cites several lines of one file, + * and collapsing them would drop every citation but the first. + */ + localCitations: EvidenceCitation[]; +} + +interface TokenParts { + /** The token with surrounding punctuation and any line suffix removed. */ + path: string; + /** The line suffix's value, or `null` when the token carries none. */ + line: number | null; } /** - * Strip surrounding punctuation and any `:123` / `:12-34` line suffix from a - * token. The trailing class includes `:` so a realm marker written `objectui:` - * reduces to `objectui`; a line suffix (`file.ts:150`) ends in a digit, so it - * survives that pass and is removed by the line-number rule after it. + * Strip surrounding punctuation and split off any `:123` / `:12-34` line suffix. + * The trailing class includes `:` so a realm marker written `objectui:` reduces + * to `objectui`; a line suffix (`file.ts:150`) ends in a digit, so it survives + * that pass and is split off by the line-number rule after it. + * + * The line is RETURNED rather than discarded (it used to be dropped on the + * floor here) — a citation's line is the half of it a moved consumer rots + * first, and a parser that cannot see the line cannot let any gate bound it. */ -function bareToken(raw: string): string { - return raw +function bareToken(raw: string): TokenParts { + const trimmed = raw .replace(/^[([{<"'`,;]+/, '') - .replace(/[)\]}>"'`,;.:]+$/, '') - .replace(/:\d+(?:-\d+)?$/, ''); + .replace(/[)\]}>"'`,;.:]+$/, ''); + const m = /:(\d+)(?:-(\d+))?$/.exec(trimmed); + if (!m) return { path: trimmed, line: null }; + return { path: trimmed.slice(0, m.index), line: Number(m[2] ?? m[1]) }; } /** @@ -79,10 +112,11 @@ function bareToken(raw: string): string { export function scanEvidence(evidence: string): EvidenceScan { const local: string[] = []; const foreign: string[] = []; + const citations: EvidenceCitation[] = []; let realm = LOCAL_REALM; for (const raw of String(evidence).split(/\s+/)) { - const token = bareToken(raw); + const { path: token, line } = bareToken(raw); const asRealm = token.toLowerCase(); if (FOREIGN_REALMS.includes(asRealm)) { realm = asRealm; continue; } @@ -91,20 +125,38 @@ export function scanEvidence(evidence: string): EvidenceScan { if (PATH_RE.test(token)) { const isForeignPath = FOREIGN_PATH_PREFIXES.some((p) => token.startsWith(p)); if (realm !== LOCAL_REALM || isForeignPath) foreign.push(token); - else local.push(token); + else { + local.push(token); + // Every token is scanned, so a multi-citation entry ("…file.ts:267 (prose) + // + …other.ts:136 + …third.ts:897") contributes ALL of its lines, not the + // first — the concatenated form is the house style for a property with + // several consumers, and seeing only its head would leave the rest of the + // chain exactly as unfalsifiable as before. + if (line !== null) citations.push({ path: token, line }); + } } // A clause boundary ends a realm's scope; the path above is classified first. if (/[;)]/.test(raw)) realm = LOCAL_REALM; } - return { local: dedupe(local), foreign: dedupe(foreign) }; + return { local: dedupe(local), foreign: dedupe(foreign), localCitations: dedupeCitations(citations) }; } function dedupe(xs: string[]): string[] { return [...new Set(xs)]; } +function dedupeCitations(cs: EvidenceCitation[]): EvidenceCitation[] { + const seen = new Set(); + return cs.filter((c) => { + const k = `${c.path}:${c.line}`; + if (seen.has(k)) return false; + seen.add(k); + return true; + }); +} + export interface EvidenceCheck extends EvidenceScan { /** Local paths that do not exist — genuinely rotted pointers. */ missing: string[]; @@ -112,7 +164,55 @@ export interface EvidenceCheck extends EvidenceScan { /** Scan an evidence string and resolve its local paths against the filesystem. */ export function checkEvidence(evidence: unknown, exists: (path: string) => boolean): EvidenceCheck { - if (typeof evidence !== 'string') return { local: [], foreign: [], missing: [] }; + if (typeof evidence !== 'string') return { local: [], foreign: [], localCitations: [], missing: [] }; const scan = scanEvidence(evidence); return { ...scan, missing: scan.local.filter((p) => !exists(p)) }; } + +/** + * `wc -l` semantics: the number of lines a citation can address. A trailing + * newline terminates the last line rather than opening an empty one, so a + * 717-line file does not have a line 718 to cite — the off-by-one that makes + * the difference between reading a past-EOF citation as rot and reading it as + * a boundary case. + */ +export function countLines(content: string): number { + if (content === '') return 0; + const n = content.split('\n').length; + return content.endsWith('\n') ? n - 1 : n; +} + +/** A citation whose line is not inside the file it cites. */ +export interface OutOfRangeCitation extends EvidenceCitation { + /** The cited file's actual line count. */ + lines: number; +} + +/** + * Bound each local `path:NNN` citation by the cited file's length. + * + * WHY THIS IS SEPARATE FROM `checkEvidence`. Existence and line-bounding are two + * standards, and only the ledger gate has measured its citations against the + * second one. Folding the line check into `checkEvidence` would impose it on + * every current and future caller of that function silently — including + * `empty-state.mts`, whose registry has never been measured this way — and an + * optional `lineCount` parameter would do the mirror-image harm: a caller that + * omits it gets no line check and nothing says so. A separately named function + * has to be *called*, so which surfaces bound their citations stays readable. + * + * `lineCount` returns `null` for a file it cannot read. Those are skipped on + * purpose: a citation into a missing file is already reported by the existence + * check, and reporting one rot twice teaches a reader to discount the list. + */ +export function checkCitationLines( + scan: EvidenceScan, + lineCount: (path: string) => number | null, +): OutOfRangeCitation[] { + const out: OutOfRangeCitation[] = []; + for (const c of scan.localCitations) { + const lines = lineCount(c.path); + if (lines === null) continue; + if (c.line > lines) out.push({ ...c, lines }); + } + return out; +} diff --git a/packages/spec/scripts/liveness/evidence.test.ts b/packages/spec/scripts/liveness/evidence.test.ts index f6c3fcdfb4..5b8898fcc2 100644 --- a/packages/spec/scripts/liveness/evidence.test.ts +++ b/packages/spec/scripts/liveness/evidence.test.ts @@ -6,7 +6,7 @@ import { describe, it, expect } from 'vitest'; import { readFileSync, existsSync, readdirSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname, join, resolve } from 'node:path'; -import { checkEvidence, scanEvidence } from './evidence.mts'; +import { checkCitationLines, checkEvidence, countLines, scanEvidence } from './evidence.mts'; const here = dirname(fileURLToPath(import.meta.url)); const specRoot = resolve(here, '../..'); @@ -96,11 +96,131 @@ describe('checkEvidence', () => { }); it('tolerates a non-string evidence value', () => { - expect(checkEvidence(undefined, none)).toEqual({ local: [], foreign: [], missing: [] }); + expect(checkEvidence(undefined, none)).toEqual({ local: [], foreign: [], localCitations: [], missing: [] }); expect(checkEvidence(42, none).missing).toEqual([]); }); }); +// #11210: the parser used to DISCARD the `:NNN` half of a citation, so no gate +// could bound it and a consumer that moved out of a file which still exists kept +// a passing pointer. These pin the retention; the bound itself is below. +describe('scanEvidence — line citations', () => { + it('retains the line of a bare `path:line` pointer', () => { + expect(scanEvidence('packages/runtime/src/http-dispatcher.ts:120').localCitations) + .toEqual([{ path: 'packages/runtime/src/http-dispatcher.ts', line: 120 }]); + }); + + it('takes the END of a `path:start-end` range', () => { + // The end is the stricter bound AND the one that subsumes the other: a start + // past EOF implies an end past EOF, never the reverse. Fixture path for the + // reason given above `describe('checkCitationLines')`: this reads no file, + // and a bare out-of-package literal reds check:cross-package-test-inputs. + expect(scanEvidence('packages/a/src/y.ts:115-167').localCitations) + .toEqual([{ path: 'packages/a/src/y.ts', line: 167 }]); + }); + + it('sees EVERY citation in a concatenated multi-consumer entry, not just the first', () => { + // The house style for a property with several consumers, `+`-joined with + // prose between. Seeing only the head would leave the tail of every such + // chain exactly as unfalsifiable as the whole string was before. + const r = scanEvidence( + 'packages/plugins/plugin-security/src/permission-evaluator.ts:267 (getSystemPermissions) + ' + + 'packages/plugins/plugin-sharing/src/sharing-rule-service.ts:136 (assertCanManageRules) + ' + + 'packages/plugins/plugin-hono-server/src/current-user-endpoints.ts:897', + ); + expect(r.localCitations.map((c) => c.line)).toEqual([267, 136, 897]); + }); + + it('records several lines of ONE file separately', () => { + // Deduping on path rather than on `path:line` would drop all but the first — + // and a repaired entry routinely cites one file at three call sites. + const r = scanEvidence('packages/a/src/x.ts:10 (rank) + packages/a/src/x.ts:20 (merge) + packages/a/src/x.ts:10 (again)'); + expect(r.local).toEqual(['packages/a/src/x.ts']); + expect(r.localCitations).toEqual([ + { path: 'packages/a/src/x.ts', line: 10 }, + { path: 'packages/a/src/x.ts', line: 20 }, + ]); + }); + + it('leaves a path with no line out of the citation list', () => { + const r = scanEvidence('packages/a/src/x.ts (prose only)'); + expect(r.local).toEqual(['packages/a/src/x.ts']); + expect(r.localCitations).toEqual([]); + }); + + it('never bounds a FOREIGN citation — those files are legitimately absent here', () => { + const r = scanEvidence('objectui: packages/app-shell/src/views/RecordDetailView.tsx:573'); + expect(r.localCitations).toEqual([]); + }); + + it('still reduces a realm marker written `objectui:` rather than reading it as a line', () => { + // The trailing-punctuation pass strips `:`; only a digit suffix is a line. + const r = scanEvidence('objectui: packages/app-shell/src/x.tsx:12'); + expect(r.foreign).toEqual(['packages/app-shell/src/x.tsx']); + expect(r.localCitations).toEqual([]); + }); + + it('reads a citation through the punctuation an entry wraps it in', () => { + expect(scanEvidence('(packages/a/src/x.ts:44)').localCitations).toEqual([{ path: 'packages/a/src/x.ts', line: 44 }]); + expect(scanEvidence('packages/a/src/x.ts:44,').localCitations).toEqual([{ path: 'packages/a/src/x.ts', line: 44 }]); + expect(scanEvidence('packages/a/src/x.ts:44.').localCitations).toEqual([{ path: 'packages/a/src/x.ts', line: 44 }]); + }); +}); + +describe('countLines — what a citation can address', () => { + it('counts a trailing newline as terminating the last line, not opening a new one', () => { + // `wc -l` semantics. The off-by-one that decides whether a 717-line file + // "has" a line 718 to cite. + expect(countLines('a\nb\nc\n')).toBe(3); + expect(countLines('a\nb\nc')).toBe(3); + }); + + it('reads an empty file as zero addressable lines', () => { + expect(countLines('')).toBe(0); + }); +}); + +// These assert the ARITHMETIC of the bound, with the line count injected. They +// read no file at all, so their sample paths are deliberately the fixture +// spelling (`packages/a/…`) and not the two real files whose rot they are shaped +// on — `hono-plugin.ts` for the `permission.tabPermissions` instance and +// `import-mapping.ts` for `mapping.fieldMapping`. +// +// ⛔ Do not "improve" these by restoring the real paths. `pnpm +// check:cross-package-test-inputs` reads a test's path literals as declared +// inputs, and @objectstack/spec's globs cover neither package — so naming them +// reds that gate, and the only way to satisfy it would be to declare an input +// this file does not have. The counts below come from a stub, not from those +// files: a citation whose real line count matters is asserted against the real +// ledgers in the contract test at the bottom of this file. +describe('checkCitationLines', () => { + const lines = (n: number) => () => n; + + it('flags a citation past the end of the file and reports the real length', () => { + // The `permission.tabPermissions` shape: line 1200 cited, 717 lines exist. + const scan = scanEvidence('packages/a/src/x.ts:1200'); + expect(checkCitationLines(scan, lines(717))).toEqual([ + { path: 'packages/a/src/x.ts', line: 1200, lines: 717 }, + ]); + }); + + it('accepts the last line of the file — the boundary is `>`, not `>=`', () => { + expect(checkCitationLines(scanEvidence('packages/a/src/x.ts:717'), lines(717))).toEqual([]); + expect(checkCitationLines(scanEvidence('packages/a/src/x.ts:718'), lines(717))).toHaveLength(1); + }); + + it('flags a RANGE whose head is inside the file but whose tail overruns it', () => { + // The shipped `mapping.fieldMapping` shape: 115 exists, 167 does not. + const r = checkCitationLines(scanEvidence('packages/a/src/y.ts:115-167'), lines(164)); + expect(r).toEqual([{ path: 'packages/a/src/y.ts', line: 167, lines: 164 }]); + }); + + it('says nothing about a file it cannot read — that verdict belongs to the existence check', () => { + // Reporting one rot twice teaches a reader to discount both lists. + expect(checkCitationLines(scanEvidence('packages/a/src/gone.ts:9'), () => null)).toEqual([]); + }); +}); + // Contract test against the REAL ledgers: the gate reports these, so a rotted // pointer committed to a ledger fails here too. describe('shipped ledgers', () => { @@ -125,4 +245,33 @@ describe('shipped ledgers', () => { // would make the assertion above vacuously true. expect(local).toBeGreaterThan(100); }); + + it('every local `path:NNN` citation names a line that file has', () => { + const outOfRange: string[] = []; + let citations = 0; + const lineCount = (p: string): number | null => { + const f = join(repoRoot, p); + if (!existsSync(f)) return null; + return countLines(readFileSync(f, 'utf8')); + }; + for (const f of readdirSync(ledgerRoot).filter((x) => x.endsWith('.json'))) { + const ledger = JSON.parse(readFileSync(join(ledgerRoot, f), 'utf8')); + const visit = (key: string, entry: any) => { + if (entry?.status !== 'live' || typeof entry?.evidence !== 'string') return; + const scan = checkEvidence(entry.evidence, () => true); + citations += scan.localCitations.length; + for (const c of checkCitationLines(scan, lineCount)) { + outOfRange.push(`${ledger.type}/${key} → ${c.path}:${c.line} (file has ${c.lines})`); + } + }; + for (const [key, entry] of Object.entries(ledger.props || {})) { + visit(key, entry); + for (const [ck, centry] of Object.entries(entry?.children || {})) visit(`${key}.${ck}`, centry); + } + } + expect(outOfRange).toEqual([]); + // Same non-vacuity guard as above, one level down: a parser that stopped + // retaining lines would satisfy the assertion above by extracting nothing. + expect(citations).toBeGreaterThan(100); + }); });