From d4dcba8c894450e6bd2ce4d11818fc8bf7b7d6ee Mon Sep 17 00:00:00 2001 From: ReidenXerx Date: Fri, 7 Aug 2026 20:47:28 +0300 Subject: [PATCH] fix(routes): track boolean polarity in dispatch guards, so a negated condition cannot invent a route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproduced exactly as reported. `dispatch-guard.ts` refuses to inherit a verb from an `if` whose `else` branch holds the comparison — the module's own doc comment explains why: that branch runs precisely when the condition did NOT hold, so attributing it is backwards. `!` is the same fact written as an operator, and it was not handled. A stated invariant with half an implementation, which is worse than an absent one, because the comment reads as though it were covered. Measured against the real extractor before fixing: if (!(pathname === '/api/admin')) -> '' /api/admin INVENTED if (!(req.method === 'GET') && pathname === '/x') -> GET /x INVERTED if (!(req.method === 'POST' && pathname === '/w')) -> POST /w BOTH And the review is right that this is not additive-only. Driven through the real pipeline with a policy module that serves nothing plus a one-line route table, the invented `GET /api/report` collected into `verbedUrls` and `reconcileDispatchGuardRoutes` then EVICTED the true verb-less route for that path. A false route deleted a real one. After the fix that repo yields exactly one route, verb-less, path intact. Parity, not presence: `!!x` is `x`, so counting negations and testing the parity is the only rule that keeps a doubly-negated guard working. A negated VERB drops to verb-less rather than dropping the route — `!(method === 'GET')` means every method except GET, which no single value expresses, while the path evidence is untouched. Applies to the regex arm too; `!/^\/api\/x$/.test(pathname)` had the identical hole. Deliberately NOT keeping the `statement_block` break from the suggested patch. It is unreachable — the `!` in `if (!cond) { … }` lives in the condition, a SIBLING of the block, never an ancestor of anything inside it, and the only shape that puts a `!` above a block is an IIFE, which the function-boundary stop catches first. Unreachable in the UNSAFE direction, too: breaking early under-counts negations, and an under-count reads a negated guard as positive and invents the route. Verified by mutation — with the break present, deleting it fails nothing; the other three guards each fail a test when removed. Six new cases, all previously absent (`grep -c '!(' ` over both test files was 0, and the only negation covered was `!==`, the form that already worked). Co-Authored-By: Claude Opus 5 (1M context) --- .../route-extractors/dispatch-guard.ts | 53 +++++++++++++++ .../test/unit/dispatch-guard-routes.test.ts | 64 +++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/gitnexus/src/core/ingestion/route-extractors/dispatch-guard.ts b/gitnexus/src/core/ingestion/route-extractors/dispatch-guard.ts index 0454108f2..a322a608a 100644 --- a/gitnexus/src/core/ingestion/route-extractors/dispatch-guard.ts +++ b/gitnexus/src/core/ingestion/route-extractors/dispatch-guard.ts @@ -252,6 +252,51 @@ function isPathExpression(node: SyntaxNode): boolean { return false; } +/** A logical `!`. `-` and `~` are unary too and are not negation. */ +function isNegation(node: SyntaxNode): boolean { + return node.type === 'unary_expression' && node.childForFieldName('operator')?.text === '!'; +} + +/** + * Is this comparison reached only when it is FALSE? + * + * The module already refuses to inherit a verb from an `if` whose `else` branch + * holds the comparison, for the reason stated in `governingVerb`: the branch runs + * precisely when the condition did NOT hold, so attributing it is backwards. + * `!` is the same fact written as an operator, and it was not handled — a stated + * invariant with half an implementation, which is worse than an absent one + * because the doc comment reads as though it were covered. + * + * Measured before fixing. `if (!(pathname === '/api/admin'))` INVENTED + * `/api/admin`; `if (!(req.method === 'GET') && pathname === '/api/x')` emitted + * `GET /api/x`, the one verb the branch guarantees the request does not have. + * + * PARITY, not presence: `!!x` is `x`, and a rule keyed on "is there a `!` above + * me" would refuse a positive condition. + * + * The walk stops at the FUNCTION boundary and nowhere else. An earlier draft + * also broke at `statement_block`, reasoning that `if (!cond) { … }` must not + * negate a comparison written in its body — true, but already guaranteed by the + * tree shape: the `!` lives in the if's CONDITION, which is a sibling of the + * block, never an ancestor of anything inside it. So that break could only ever + * fire where a `!` genuinely IS an ancestor across a block, i.e. an IIFE — which + * the function-boundary stop catches first. Unreachable, and unreachable in the + * UNSAFE direction: stopping early under-counts negations, and an under-count + * reads a negated guard as positive and invents the route. Removed rather than + * kept for symmetry. + */ +function isNegatedContext(node: SyntaxNode): boolean { + let negations = 0; + let current: SyntaxNode = node; + let parent = current.parent; + while (parent !== null && !FUNCTION_NODE_TYPES.has(parent.type)) { + if (isNegation(parent)) negations += 1; + current = parent; + parent = current.parent; + } + return negations % 2 === 1; +} + /** Strip redundant parentheses, which the grammar keeps as real nodes. */ function unparenthesize(node: SyntaxNode | null): SyntaxNode | null { let current = node; @@ -346,6 +391,10 @@ function governingVerb(comparison: SyntaxNode): string | null { /** First verb comparison anywhere in this subtree. */ function findVerbInSubtree(node: SyntaxNode): string | null { + // A verb under a `!` is the verb the branch EXCLUDES. Returning null keeps the + // route (the path evidence is unaffected) and leaves it verb-less, which is + // the honest answer: this branch does not tell us which method it serves. + if (isNegation(node)) return null; const direct = verbFromComparison(node); if (direct !== null) return direct; for (const child of node.namedChildren) { @@ -487,6 +536,9 @@ export function extractDispatchGuardRoutes( } function collectFromComparison(node: SyntaxNode, out: GuardRoute[], constants: ConstantMap): void { + // Reached only when the comparison is FALSE — claiming the path would be + // exactly backwards. See `isNegatedContext`. + if (isNegatedContext(node)) return; const operator = node.childForFieldName('operator')?.text ?? ''; if (!EQUALITY_OPERATORS.has(operator)) return; const left = node.childForFieldName('left'); @@ -553,6 +605,7 @@ function collectFromSwitch(node: SyntaxNode, out: GuardRoute[], constants: Const } function collectFromRegexTest(node: SyntaxNode, out: GuardRoute[]): void { + if (isNegatedContext(node)) return; const callee = node.childForFieldName('function'); if (callee === null || callee.type !== 'member_expression') return; if (callee.childForFieldName('property')?.text !== 'test') return; diff --git a/gitnexus/test/unit/dispatch-guard-routes.test.ts b/gitnexus/test/unit/dispatch-guard-routes.test.ts index a29c37d57..2195fd148 100644 --- a/gitnexus/test/unit/dispatch-guard-routes.test.ts +++ b/gitnexus/test/unit/dispatch-guard-routes.test.ts @@ -176,6 +176,70 @@ describe('dispatch-guard route extraction', () => { }); }); + // BOOLEAN POLARITY. The module refuses to inherit a verb from an `if` whose + // `else` branch holds the comparison, because that branch runs precisely when + // the condition did NOT hold. `!` is the same fact written as an operator, and + // it was not handled — a stated invariant with half an implementation, which + // is worse than an absent one because the doc comment reads as covered. + // + // Every case below was reproduced against the unguarded extractor before the + // fix: `!(path)` INVENTED a route, and `!(verb) && path` emitted the one verb + // the branch guarantees the request does not have. + describe('negated conditions', () => { + it('claims nothing when the path comparison is negated', () => { + expect(paths(`function h(req) { if (!(pathname === '/api/admin')) { return 1 } }`)).toEqual( + [], + ); + }); + + it('claims nothing when the whole guard is negated', () => { + expect( + paths( + `function h(req) { if (!(req.method === 'POST' && pathname === '/api/w')) { return 1 } }`, + ), + ).toEqual([]); + }); + + it('keeps the path but drops a NEGATED verb rather than inverting it', () => { + // The path is still evidence — this branch is reached for `/api/x`. The + // verb is not: `!(method === 'GET')` says every method EXCEPT GET, which + // no single value can express, so the honest answer is verb-less. + expect( + extract( + `function h(req) { if (!(req.method === 'GET') && pathname === '/api/x') { return 1 } }`, + ), + ).toMatchObject([{ routePath: '/api/x', httpMethod: '' }]); + }); + + it('treats double negation as positive', () => { + // PARITY, not presence. A rule keyed on "is there a `!` above me" would + // refuse this, which is a real route. + expect(paths(`function h(req) { if (!!(pathname === '/api/z')) { return 1 } }`)).toEqual([ + '/api/z', + ]); + }); + + it('does not let an outer negation leak into the branch BODY', () => { + // Polarity is a property of the expression, not of the statements a branch + // contains: the inner comparison is positive where it is written. + expect( + paths(` + function h(req) { + if (!(req.method === 'GET')) { + if (pathname === '/api/inner') { return 1 } + } + } + `), + ).toEqual(['/api/inner']); + }); + + it('negates a regex path test too', () => { + expect( + paths(`function h(req) { if (!/^\\/api\\/runs\\/[^/]+$/.test(pathname)) { return 1 } }`), + ).toEqual([]); + }); + }); + // Not in any report — the same dispatch written with different syntax. A // graph that waits for a bug report per shape stays permanently one idiom // behind the code it indexes.