From 690470865e81b09ae3f9329e2d2d77b0975c72bd Mon Sep 17 00:00:00 2001 From: ReidenXerx Date: Fri, 7 Aug 2026 19:14:30 +0300 Subject: [PATCH] fix(scope-resolution): require the return-shape producer to RESOLVE, not merely to name-match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding 2, reached independently by three Claude lanes and two Codex legs, and reproduced here. `emitReturnShapeMemberAccesses` took the receiver's type binding, then filtered a WHOLE-GRAPH property index with `idNamesMember` — a textual match on the node id. Any node whose id happened to read `.` qualified, in any file and any language, and it emitted at the 0.9 PRECISE tier where a `minConfidence` floor cannot filter it out. The sibling unique-name pass was given a per-language restriction for exactly this hazard; this pass consumed the same shared index with none. Three guards, catching different shapes: - the producer must RESOLVE to a definition (`findCallableBindingInScope` — a CALLABLE lookup: the producer is the function whose return shape owns the member, and it resolves through finalized import bindings so a producer in another file still yields its own file); - the member must live in that definition's file; - that file must belong to the language being resolved. The third is not redundant with the second, which is the part worth recording. A receiver typed by CONSTRUCTION (`const bound = new Loyalty()`) resolves through the shared class registry, which is polyglot — so the producer resolves into `Loyalty.java`, its members legitimately live in that same file, and file equality waves the cross-language edge straight through. Also fixes the sibling P2: a site where the receiver IS typed to a producer that owns no such member now claims the site. That branch is the strongest negative evidence the pipeline can produce, and letting it fall through meant the 0.5 name fallback answered a question the precise pass had just DISPROVED — measured, linking a read to an unrelated same-named key in another file. `polyglot-property-isolation` gains the bound-receiver arm the review asked for, and it is the right arm: the pre-existing case has an untyped receiver and so only ever exercised the unique-name pass, while one extra token routes an identical read through this one. Mutation-verified — restoring the pre-fix matching makes exactly the new leak assertion fail. The first version of that arm was silently vacuous (it introduced a JS key of the same name, which destroyed the fixture's Java-only premise), which is why it now asserts on the TARGET FILE rather than on the absence of a name. KNOWN LIMIT, stated rather than papered over: a member-call producer (`const r = svc.make()`) binds `svc.make`, which resolves to no callable, so this pass now declines it. Codex B3 raised that converse case and it is real. Fixing it means typing `svc` and then finding `make` on that type — a larger piece of work, queued for the follow-up PR. Declining is the correct interim behaviour: the alternative is matching `make.` by name across the graph, which is the fabrication this commit removes. Co-Authored-By: Claude Opus 5 (1M context) --- .../passes/return-shape-members.ts | 92 ++++++++++++++++++- .../polyglot-property-isolation/settings.js | 17 ++++ .../polyglot-property-isolation.test.ts | 27 ++++++ 3 files changed, 131 insertions(+), 5 deletions(-) diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts index 567387fff..1ffb2da65 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts @@ -41,7 +41,7 @@ import type { KnowledgeGraph } from '../../../graph/types.js'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import { resolveCallerGraphId } from '../graph-bridge/ids.js'; -import { findReceiverTypeBinding } from '../scope/walkers.js'; +import { findCallableBindingInScope, findReceiverTypeBinding } from '../scope/walkers.js'; import { callableFlowSiteKey } from './callable-value-flow.js'; import type { PropertyNameIndex } from './unique-name-properties.js'; @@ -97,6 +97,19 @@ export function emitReturnShapeMemberAccesses( let memberNotOnShape = 0; const seen = new Set(); + // The files of the language being resolved. `parsedFiles` is already scoped to + // it, so this needs no new plumbing — it is the same restriction the sibling + // unique-name pass gets from `candidatesForLanguage`. + // + // The file guard below is not sufficient on its own, and the reason is worth + // keeping: a receiver typed by CONSTRUCTION (`const cfg = new Loyalty()`) + // resolves `Loyalty` through the shared class registry, which is polyglot. The + // producer then legitimately resolves to `Loyalty.java`, its members + // legitimately live in that same file, and a file-equality check waves the + // cross-language edge straight through. Restricting to the current language's + // own files is what actually closes it. + const ownFilePaths = new Set(parsedFiles.map((p) => p.filePath)); + for (const parsed of parsedFiles) { for (const site of parsed.referenceSites) { if (site.kind !== 'read' && site.kind !== 'write') continue; @@ -109,17 +122,86 @@ export function emitReturnShapeMemberAccesses( // whole point: `formatSpikeAlert` is a function, and before R3-4 there // was nothing named after it to look a member up on. const typeRef = findReceiverTypeBinding(site.inScope, receiver, indexes); - const producer = typeRef?.rawName; - if (producer === undefined || producer.length === 0) continue; + const producerRef = typeRef?.rawName; + if (producerRef === undefined || producerRef.length === 0) continue; + + // R3-4 qualifies a returned key by the producing function's own name, so + // the owner segment to match is the LAST one. For a plain producer this is + // a no-op. + // + // A MEMBER-CALL producer (`const r = svc.make()`) binds `svc.make`, and + // that spelling resolves to no value binding below, so this pass DECLINES + // rather than resolving it. That is a known coverage limit, not a fix: + // answering it means typing `svc` first and then finding `make` on that + // type, which is a different (and larger) piece of work. Declining is the + // correct behaviour in the meantime — the alternative, matching + // `make.` by name across the graph, is precisely the fabrication + // the file guard below exists to stop. + const producer = producerRef.slice(producerRef.lastIndexOf('.') + 1); + if (producer.length === 0) continue; + + // Resolve the producer to a real definition and keep only members that + // live in ITS file. + // + // Without this the join is textual over a whole-graph index: any node + // whose id happens to read `.` matches, in any file and + // any LANGUAGE. Measured, that fabricated a 0.9-confidence edge from a JS + // component to a Java field — and 0.9 is the precise tier, so a + // `minConfidence` floor cannot filter it out. The sibling unique-name pass + // was given a per-language restriction for exactly this; this pass + // consumes the same shared index and had none. + // + // The file identity is the evidence, not a heuristic: R3-4 anchors a + // returned literal's keys to the function that returns them, so the + // member's node necessarily sits in the same file as that function. A + // candidate elsewhere is a different symbol wearing the same name. + // A CALLABLE lookup, not a value one: the producer is the function whose + // return shape owns the member. It also resolves through finalized import + // bindings, so a producer imported from another file still yields its own + // file — the guard restricts to the RIGHT file, it does not force same-file. + // Three guards, and they catch different shapes — none is redundant: + // + // producerDef — the producer must RESOLVE. This is the one that stops + // the measured cross-language leak: `new Loyalty()` in JS + // yields `producerRef = 'Loyalty'`, and a Java class does + // not resolve as a callable from a JS scope, so the pass + // declines instead of name-matching into `Loyalty.java`. + // Mutation-verified by `polyglot-property-isolation`. + // filePath — among same-named producers, keep the members of the one + // actually resolved. Defence in depth for the case where + // the producer DOES resolve and a same-named function + // exists in another file. + // ownFilePaths — a receiver typed by construction resolves through the + // shared, POLYGLOT class registry, so a producer can + // resolve into another language with its members + // legitimately in that same file. File equality passes + // there; only the language restriction closes it. + const producerDef = findCallableBindingInScope(site.inScope, producerRef, indexes); + const producerFile = producerDef?.filePath; + if (producerFile === undefined) continue; + if (!ownFilePaths.has(producerFile)) continue; const candidates = propertyNameIndex.get(site.name); if (candidates === undefined) continue; - const owned = candidates.filter((c) => idNamesMember(c.id, producer, site.name)); + const owned = candidates.filter( + (c) => c.filePath === producerFile && idNamesMember(c.id, producer, site.name), + ); // Exactly one, or nothing. Two nodes claiming `.` would // mean the id qualifier failed to separate them, and picking between them // would be the guess this pass exists to avoid. if (owned.length !== 1) { - if (owned.length === 0) memberNotOnShape++; + if (owned.length === 0) { + memberNotOnShape++; + // CLAIM THE SITE ANYWAY. This branch is the strongest NEGATIVE + // evidence the pipeline can produce: the receiver is typed to a + // producer, that producer's shape is known, and it owns no member of + // this name. Falling through let the 0.5 name fallback answer a + // question the precise pass had just DISPROVED — measured, it linked + // a read to an unrelated same-named key in another file. Disproving a + // member and then inventing it one pass later is worse than either + // answer alone. + handledSink.add(siteKey); + } continue; } const target = owned[0]!; diff --git a/gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/settings.js b/gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/settings.js index f6e16684d..063584ee8 100644 --- a/gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/settings.js +++ b/gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/settings.js @@ -14,3 +14,20 @@ export const jsConfig = { export function readsJsOnly(bag) { return bag.jsOnlyThreshold; } + +// BOUND-RECEIVER ARM (review finding 2). The reads above have UNTYPED receivers, +// so they route through unique-name inference — the pass this fixture was +// written to police. One extra token gives the receiver a type and routes an +// identical read through `return-shape-members.ts` instead: a sibling pass that +// consumed the same whole-graph index with no language restriction, and emitted +// at the 0.9 PRECISE tier where a `minConfidence` floor cannot filter it out. +// +// `Loyalty` is declared ONLY in Java. Construction types the receiver through +// the shared (polyglot) class registry, so the producer resolves into +// `Loyalty.java` and its member genuinely lives in that same file — which is +// why a same-FILE check alone waves this through and only a same-LANGUAGE check +// stops it. Nothing here may resolve. +export function readsBoundLoyalty() { + const bound = new Loyalty(); + return bound.loyaltyPointsBalance; +} diff --git a/gitnexus/test/integration/resolvers/polyglot-property-isolation.test.ts b/gitnexus/test/integration/resolvers/polyglot-property-isolation.test.ts index 2755d1486..004431abc 100644 --- a/gitnexus/test/integration/resolvers/polyglot-property-isolation.test.ts +++ b/gitnexus/test/integration/resolvers/polyglot-property-isolation.test.ts @@ -36,6 +36,33 @@ describe('cross-language property inference (RV-5)', () => { expect(readersOf('loyaltyPointsBalance')).not.toContain('renderLoyalty'); }); + // The SAME boundary, reached through the other pass. `renderLoyalty` above has + // an untyped receiver, so it routes through unique-name inference — the pass + // this fixture was written to police. Typing the receiver by construction + // routes an identical read through `return-shape-members.ts` instead, which + // consumed the same whole-graph index with no language restriction and emitted + // at 0.9 rather than 0.5 — the PRECISE tier, where a `minConfidence` floor + // cannot filter the result out. + // + // Note WHY a same-file check was not enough here. `new Loyalty()` types the + // receiver through the shared class registry, which is polyglot, so the + // producer resolves into `Loyalty.java` and its member genuinely lives in that + // same file. File equality is satisfied; only the language restriction stops + // the edge. + describe('the bound-receiver path (review finding 2)', () => { + const targetsOf = (source: string): string[] => + getRelationships(result, 'ACCESSES') + .filter((e) => e.source === source) + .map((e) => e.targetFilePath ?? ''); + + it('never reaches the Java field through the bound path', () => { + // Asserted on TARGET FILE, not on absence of the name: the leak this + // catches is an edge that exists and points into another language, which + // a name-only assertion would not distinguish from a correct local edge. + expect(targetsOf('readsBoundLoyalty').some((f) => f.includes('Loyalty.java'))).toBe(false); + }); + }); + // The other half: restricting by language must not disable the pass. it('still resolves a same-language unique name', () => { expect(readersOf('jsOnlyThreshold')).toContain('readsJsOnly');