Skip to content

Commit b9186f6

Browse files
claude[bot]os-project-managerclaude
authored
fix(devx): the read-seam wrapper hop gets the shape check the vocabulary hit already has (#13444)
* fix(devx): the read-seam wrapper hop gets the shape check the vocabulary hit already has #11921 gave `isReadCall`'s vocabulary match a shape test (`contradictsDriverReadShape`) and applied it at exactly one place. The wrapper hop resolved a callee name against the flat, file-scoped `functionBodies` index with no equivalent test and no receiver check, so any call whose name collided with a same-file declaration was followed into that body. `contradictsWrapperResolution` asks the same question of the resolved declaration: a receiver that is itself a property access, index or call result names another object's member, and a call supplying fewer arguments than the declaration requires is not a call to it. Measured zero-delta on today's tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC * test(devx): pin the bare-identifier admission in the wrapper-hop shape check The receiver clause reads receiver DEPTH, not `this`-rootedness. Nothing asserted that admission, so tightening it to `this`/`super` only passed the whole suite. `const self = this;` is how the live file reaches its own members from a closure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC --------- Co-authored-by: Claude <pm@objectstack.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 0b6e479 commit b9186f6

1 file changed

Lines changed: 295 additions & 0 deletions

File tree

scripts/check-durability-degradation-log-level.mjs

Lines changed: 295 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,6 +1196,78 @@ const FAILURE_PROPAGATION_SITES = new Map([
11961196
// The census is therefore UNMOVED at 64, and #8901's restart conjunct (b) is
11971197
// not triggered by this reading.
11981198

1199+
// ── THE WRAPPER HOP'S OWN SHAPE CHECK — MEASURED AND CHANGED (#12358) ────────
1200+
//
1201+
// The block above records the fake seam and stops there, because telling
1202+
// `this.delete(ref, opts)` from `someSet.delete(x)` was #11921's provenance
1203+
// problem and out of that card's scope. This block is that fix, and the census
1204+
// #12358 required before anyone narrowed anything. See
1205+
// `contradictsWrapperResolution` for the predicate and its direction of error.
1206+
//
1207+
// RE-DERIVED, not inherited. The filing's table was measured on `origin/main`
1208+
// @ 3ddad51b5c and is five days older than this tree; the denominator has since
1209+
// moved 64 -> 66, so every figure below was re-run here on `origin/main`
1210+
// @ 5f0a9c4a. The DELTA reproduced exactly: +8 under the `walkAll` probe, the
1211+
// same 8 seams by identity, and the ablation still removes exactly two.
1212+
//
1213+
// | recognizer | read seams |
1214+
// |--------------------------------------------------|-----------:|
1215+
// | today — `walkSameTickInclusive`, depth 2 | 66 |
1216+
// | probe — `walkAll`, depth 2 | 74 |
1217+
// | probe, with the `delete` wrapper hop refused | 72 |
1218+
//
1219+
// ⚠️ THE ABLATION IS THE CONTROL, NOT THE FIX. Refusing the `delete` hop
1220+
// outright drops TWO seams — sys-metadata-repository.ts:1353 `close`->
1221+
// `terminate` (the FAKE) and :883 `promoteDraft`->`dropPromotedDraftRow` (a
1222+
// REAL `await this.delete(ref, …)`). In a summary that is indistinguishable
1223+
// from the correct outcome, which is why the pair is pinned in the self-test.
1224+
//
1225+
// THE CENSUS OVER THE WHOLE SCAN ROOT. The filing named two candidate shapes
1226+
// and refused to choose between them without one. Both were measured, on
1227+
// today's recognizer AND under the `walkAll` probe that arms the defect:
1228+
//
1229+
// | wrapper-hop guard | today | walkAll | removes |
1230+
// |--------------------------------------------|------:|--------:|----------|
1231+
// | none (as shipped before this change) | 66 | 74 | — |
1232+
// | A. receiver, bare identifiers admitted | 66 | 73 | the fake |
1233+
// | A'. receiver, strict `this`/`self` only | 66 | 73 | the fake |
1234+
// | B1. `contradictsDriverReadShape` on the hop| 66 | 74 | NOTHING |
1235+
// | B2. required-parameter count | 66 | 73 | the fake |
1236+
// | A + B2 (taken) | 66 | 73 | the fake |
1237+
//
1238+
// Every row adds zero seams — measured, not argued: the added-seam set is empty
1239+
// in all six runs.
1240+
//
1241+
// ⚠️ B1 IS A VACUOUS FIX AND THE CENSUS IS WHAT SAYS SO. Read literally, "give
1242+
// the hop a `contradictsDriverReadShape`-style argument test" means calling
1243+
// that predicate on the hop — refuse when the first argument is a function
1244+
// literal. `self.watchers.delete(subscription)` passes an identifier, so it
1245+
// refuses nothing at all, on this tree or in principle. B2 is that candidate
1246+
// read at the level the vocabulary-side predicate actually works at: not the
1247+
// literal predicate, but its METHOD — refute the call against the contract
1248+
// already in hand. Reported so the next reader does not re-derive it.
1249+
//
1250+
// WHY A + B2 RATHER THAN EITHER. The measurement cannot separate them: on this
1251+
// tree all three cost nothing and remove exactly the fake seam. Their FAILURE
1252+
// modes are independent, and each is one edit away in live code — A alone
1253+
// returns the fake seam if `delete`'s second parameter becomes optional, B2
1254+
// alone returns it if the registry is reached through a bare identifier
1255+
// (`watchers.delete(x)` after a destructure). The conjunction is measured at
1256+
// the same zero cost as either half, so both are asked.
1257+
//
1258+
// ⭐ WHAT THIS CHANGE DOES AND DOES NOT DO. The defect is LATENT: today's
1259+
// recognizer never reaches the fake read, because the wrapper recursion's
1260+
// `walkSameTickInclusive` stops at the `withTxn` callback. So the deliverable
1261+
// is a MEASURED ZERO-DELTA — the full checker output is byte-identical before
1262+
// and after on this tree, verdicts, counts and `--list` lines alike — plus a
1263+
// refusal that holds under the widening that would arm it. ⛔ No baseline entry
1264+
// was added and none was needed: an entry says a human read a seam, not that a
1265+
// rule stopped inventing one.
1266+
//
1267+
// ⛔ #12360 (the `MAX_READ_WRAPPER_DEPTH = 2` bound, which the table above shows
1268+
// costs the census 6 further seams) is a SEPARATE card on this same file and is
1269+
// deliberately not touched here.
1270+
11991271
/**
12001272
* Where the read-seam rule looks. Narrowed on purpose — see above.
12011273
*
@@ -1295,6 +1367,97 @@ function contradictsDriverReadShape(node) {
12951367
return ts.isArrowFunction(first) || ts.isFunctionExpression(first);
12961368
}
12971369

1370+
/**
1371+
* The same question one level up: can this call be a call to THAT declaration?
1372+
*
1373+
* `contradictsDriverReadShape` asks it of `IDataDriver`. The wrapper hop in
1374+
* `isReadCall` asked nothing at all. `functionBodies` is a flat, file-scoped
1375+
* index keyed by BARE NAME, so any call whose `calleeName` happens to equal a
1376+
* function declared in the same file was followed into that function's body —
1377+
* whatever it was called ON, and whatever it was passed.
1378+
*
1379+
* That is #11921's defect on the WRAPPER name instead of the vocabulary name,
1380+
* and it runs the other way. A vocabulary false positive over-counts a seam
1381+
* that exists; a wrapper false positive INVENTS one, and prints it in
1382+
* `--list` naming a real caller and a real callee, indistinguishable from a
1383+
* genuine seam. The read-seam denominator is what #5186, #6451, #9165, #8845
1384+
* and #8901 are all quoted against, so a fake member is the unsafe direction.
1385+
*
1386+
* THE LIVE INSTANCE (#12358, re-measured here — see the census below).
1387+
* `sys-metadata-repository.ts` `close()` calls `w.terminate()`; `terminate` is
1388+
* a local const arrow whose only call is `self.watchers.delete(subscription)`
1389+
* on a `private readonly watchers = new Set<...>()`. `calleeName` reads that
1390+
* as `delete`, and the hop resolved `delete` to THIS FILE's
1391+
* `async delete(ref, opts)`, whose `findOne` makes the chain read as a
1392+
* `sys_metadata` read. A `Set.prototype.delete` on an in-memory watcher
1393+
* registry, one hop from the census.
1394+
*
1395+
* Two INDEPENDENT facts refute that resolution and this predicate asks both.
1396+
* Neither is a new vocabulary and neither carries a staleness obligation: both
1397+
* are read off the declaration the index ALREADY holds.
1398+
*
1399+
* RECEIVER — a same-file body can only run for a call that names it
1400+
* unqualified (`terminate()`), or on `this`/`super`, or on some binding that
1401+
* may hold it (`w.terminate()`, which is how the live `terminate` is really
1402+
* reached). A receiver that is itself a property access, an index or a call
1403+
* result — `self.watchers.delete`, `a.b.c()` — names a member of ANOTHER
1404+
* object, and resolving it to this file's body is a name collision by
1405+
* construction.
1406+
*
1407+
* ARITY — a call supplying fewer arguments than the resolved declaration
1408+
* REQUIRES is not a call to that declaration; in a type-checked tree it would
1409+
* not compile. `self.watchers.delete(subscription)` passes one;
1410+
* `delete(ref, opts)` requires two.
1411+
*
1412+
* Each clause alone closes the live instance, and each has a failure mode the
1413+
* other does not: the receiver clause survives `opts` becoming optional, the
1414+
* arity clause survives the registry being reached through a bare identifier
1415+
* (`watchers.delete(x)` after a destructure). Measured, the conjunction costs
1416+
* exactly what either half costs — nothing — so both are asked.
1417+
*
1418+
* DIRECTION OF THE ERROR, stated up front as the vocabulary-side predicate
1419+
* states its own: this can only refuse to FOLLOW a hop. It cannot add a seam,
1420+
* so it cannot manufacture a violation; what it can do is drop a real read
1421+
* whose wrapper is reached in a shape it does not model — the under-counting
1422+
* direction every other narrowness in this file takes, and the one AGENTS.md
1423+
* declares for the family ("under-matches on purpose rather than risk a false
1424+
* positive"). Spread calls (`f(...args)`) are exempt from the arity clause for
1425+
* that same reason: the argument count is not knowable, so it is not asserted.
1426+
*
1427+
* WHY THIS IS NOT THE RECEIVER ALLOWLIST `contradictsDriverReadShape` REJECTS.
1428+
* That objection is measured and it stands — for the VOCABULARY hit, where the
1429+
* receiver IS the driver binding (`this.engine`, `driver`, `ledger`, `port`)
1430+
* and requiring a `this`-rooted receiver drops 14 of 66 seams, 12 of them real.
1431+
* This clause is not a list of receiver NAMES, and it does not require `this`:
1432+
* it reads receiver DEPTH, admits every bare identifier, and is measured at
1433+
* zero seams on the whole scan root.
1434+
*/
1435+
function contradictsWrapperResolution(node, body) {
1436+
const expr = node.expression;
1437+
if (ts.isPropertyAccessExpression(expr)) {
1438+
const recv = expr.expression;
1439+
const mayHoldIt =
1440+
recv.kind === ts.SyntaxKind.ThisKeyword ||
1441+
recv.kind === ts.SyntaxKind.SuperKeyword ||
1442+
ts.isIdentifier(recv);
1443+
if (!mayHoldIt) return true;
1444+
}
1445+
// `setParentNodes` is fixed true in `ts-parse.mjs`, so the body indexed by
1446+
// `indexFunctionBodies` reaches its own declaration — no second index, and
1447+
// no change to the `functionBodies` value shape its other consumers read.
1448+
const params = body?.parent?.parameters;
1449+
if (!params) return false;
1450+
if (node.arguments.some((a) => ts.isSpreadElement(a))) return false;
1451+
let required = 0;
1452+
for (const p of params) {
1453+
// A `this` parameter is a type annotation, never an argument position.
1454+
if (ts.isIdentifier(p.name) && p.name.text === 'this') continue;
1455+
if (p.questionToken || p.initializer || p.dotDotDotToken) break;
1456+
required += 1;
1457+
}
1458+
return node.arguments.length < required;
1459+
}
1460+
12981461
const MAX_READ_WRAPPER_DEPTH = 2;
12991462

13001463
/**
@@ -2275,6 +2438,10 @@ function isReadCall(node, functionBodies, seen = new Set(), depth = 0) {
22752438
if (depth >= MAX_READ_WRAPPER_DEPTH || seen.has(name)) return false;
22762439
const body = functionBodies.get(name);
22772440
if (!body) return false;
2441+
// #12358: the hop's own shape check. Without it a callee NAME that merely
2442+
// collides with a same-file declaration is followed into that body, and the
2443+
// seam it manufactures is indistinguishable from a real one.
2444+
if (contradictsWrapperResolution(node, body)) return false;
22782445
seen.add(name);
22792446
let found = false;
22802447
walkSameTickInclusive(body, (child) => {
@@ -5313,6 +5480,134 @@ function selfTestReadSeams() {
53135480
expectViolation: true,
53145481
expectSeams: 1,
53155482
},
5483+
5484+
// ── #12358: the WRAPPER hop's shape check, both clauses, both ways ──
5485+
//
5486+
// The live instance is LATENT on today's recognizer — the wrapper
5487+
// recursion's `walkSameTickInclusive` stops before the fake read is
5488+
// reached — so these fixtures put the read on the wrapper's own tick.
5489+
// That is deliberate: it pins the HOP, which is the defect, instead of
5490+
// pinning the callback refusal that is currently masking it. Measured:
5491+
// with `contradictsWrapperResolution` ablated, the two `expectSeams: 0`
5492+
// cases below report 1 seam each, so neither assertion is vacuous.
5493+
{
5494+
// RECEIVER clause, isolated: the arity matches, so only the
5495+
// receiver can refuse this. `self.watchers.delete(subscription)`
5496+
// reduced — a `Set.prototype.delete` whose name collides with the
5497+
// file's own `delete` method.
5498+
name: 'passes: #12358 — a compound receiver does not resolve to the same-file method it collides with',
5499+
code: `
5500+
class R {
5501+
private readonly watchers = new Set<any>();
5502+
private async delete(ref: string) {
5503+
return await this.driver.findOne('sys_metadata', { ref });
5504+
}
5505+
close(sub: any): void {
5506+
try { this.watchers.delete(sub); }
5507+
catch { /* one wedged consumer must not strand the rest */ }
5508+
}
5509+
}`,
5510+
expectViolation: false,
5511+
expectSeams: 0,
5512+
},
5513+
{
5514+
// ARITY clause, isolated: the receiver is a bare identifier, which
5515+
// this predicate deliberately admits, so only the argument count
5516+
// can refuse it. One argument against a two-parameter declaration.
5517+
name: 'passes: #12358 — a call that cannot satisfy the resolved declaration is not that declaration',
5518+
code: `
5519+
class R {
5520+
private async delete(ref: string, opts: unknown) {
5521+
return await this.driver.findOne('sys_metadata', { ref, opts });
5522+
}
5523+
close(registry: Set<any>, sub: any): void {
5524+
try { registry.delete(sub); }
5525+
catch { /* the registry is in-memory; nothing to report */ }
5526+
}
5527+
}`,
5528+
expectViolation: false,
5529+
expectSeams: 0,
5530+
},
5531+
{
5532+
// THE OTHER HALF OF THE DISCRIMINATING PAIR, and the reason the
5533+
// two above are not enough on their own. Refusing the `delete` hop
5534+
// outright — the cheap fix — drops this seam too, and a summary
5535+
// cannot tell that apart from the correct outcome. Measured on the
5536+
// live tree under the `walkAll` probe: the blunt refusal takes 74
5537+
// to 72 (both), this predicate takes 74 to 73 (the fake only).
5538+
name: 'flags: #12358 — a `this`-rooted chain into the real method is still a read seam',
5539+
code: `
5540+
class R {
5541+
async delete(ref: string, opts: unknown) {
5542+
return await this.driver.findOne('sys_metadata', { ref, opts });
5543+
}
5544+
private async dropPromotedDraftRow(ref: string, opts: unknown) {
5545+
return await this.delete(ref, opts);
5546+
}
5547+
async promoteDraft(ref: string, opts: unknown) {
5548+
try { return await this.dropPromotedDraftRow(ref, opts); }
5549+
catch { return null; }
5550+
}
5551+
}`,
5552+
expectViolation: true,
5553+
expectSeams: 1,
5554+
},
5555+
{
5556+
// The documented limitation, pinned rather than described: a spread
5557+
// call's argument count is not knowable, so the arity clause does
5558+
// not assert one. Over-counting, which is the safe direction.
5559+
name: 'flags (documented limitation): #12358 — a spread call is exempt from the arity clause',
5560+
code: `
5561+
class R {
5562+
private async delete(ref: string, opts: unknown) {
5563+
return await this.driver.findOne('sys_metadata', { ref, opts });
5564+
}
5565+
async drop(args: [string, unknown]) {
5566+
try { return await this.delete(...args); }
5567+
catch { return null; }
5568+
}
5569+
}`,
5570+
expectViolation: true,
5571+
expectSeams: 1,
5572+
},
5573+
{
5574+
// The bare-identifier ADMISSION, pinned. The receiver clause reads
5575+
// depth, not `this`-rootedness — `const self = this;` is how the
5576+
// live file reaches its own members from a closure, and requiring
5577+
// `this` would drop this seam. Without this case, tightening the
5578+
// clause to `this`/`super` only passes the whole suite.
5579+
name: 'flags: #12358 — a bare-identifier receiver is admitted, not refused',
5580+
code: `
5581+
class R {
5582+
private async loadRow(ref: string) {
5583+
return await this.driver.findOne('sys_metadata', { ref });
5584+
}
5585+
async read(ref: string) {
5586+
const self = this;
5587+
try { return await self.loadRow(ref); }
5588+
catch { return null; }
5589+
}
5590+
}`,
5591+
expectViolation: true,
5592+
expectSeams: 1,
5593+
},
5594+
{
5595+
// The clause must count REQUIRED parameters, not declared ones, or
5596+
// it drops real reads through every wrapper with an optional tail.
5597+
name: 'flags: #12358 — an optional parameter is not a required one',
5598+
code: `
5599+
class R {
5600+
private async delete(ref: string, opts?: unknown) {
5601+
return await this.driver.findOne('sys_metadata', { ref, opts });
5602+
}
5603+
async drop(ref: string) {
5604+
try { return await this.delete(ref); }
5605+
catch { return null; }
5606+
}
5607+
}`,
5608+
expectViolation: true,
5609+
expectSeams: 1,
5610+
},
53165611
];
53175612

53185613
let failures = 0;

0 commit comments

Comments
 (0)