GitNexus/gitnexus/test/integration/parse-impl-quarantine-cache-skip.test.ts
henry201605 b565c7c990
feat(ingestion): resolve FastAPI include_router(prefix=...) cross-file routes (#1877)
* feat(ingestion): resolve FastAPI include_router(prefix=...) cross-file routes

FastAPI sub-route files declare paths via @router.<verb> while the entry
file mounts the router with app.include_router(<router>, prefix='/x').
Previously both the ingestion-layer Route graph nodes and the group-layer
ExtractedContract URLs lost the cross-file prefix, breaking provider <->
consumer matching.

Ingestion layer:
  - parse-worker emits routerIncludes / routerImports + decoratorReceiver
  - parsing-processor / parse-impl thread the new fields and aggregate
    prefixesByModule across chunks; decorator routes whose receiver is
    'router' are duplicated once per matching prefix
  - routes.ts joins prefix via normalizeExtractedRoutePath

Group layer:
  - HttpLanguagePlugin gains an optional prepareRepo() pre-pass and a
    repoContext arg to scan(); python.ts builds prefixesByModule and
    falls back to the bare path when no entry matches
  - http-route-extractor caches one repoContext per plugin

Tests:
  - 3 new http-route-extractor cases (attr / named-import / no-prefix)
  - ParseWorkerResult literals in 3 test files updated to the new shape

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(ingestion,group): address PR #1877 review — relative imports, cross-package collisions, host names, ingestion tests

Follow-ups to the FastAPI `include_router(prefix=...)` cross-file fix
based on PR #1877's automated production-readiness review. Three
correctness gaps and one test coverage gap addressed:

1. Relative-import support in the worker regex (FINDING 2)
   `FROM_IMPORT_ROUTER_RE` now accepts module paths starting with a
   `.` (e.g. `from .calls import router as calls_router`). The
   previous `[A-Za-z_][\w.]*` rejected leading dots and silently
   dropped every relative-import Shape-B include — a real pattern
   from the PR description's own motivating example. The matching
   helpers now strip leading dots before keying so absolute and
   relative imports collapse to the same module key.

2. Cross-package same-name module collisions (FINDING 3)
   Two-tier module keying replaces the previous basename-only key:
     • short key — `users`            (file basename without `.py`)
     • long  key — `api/users`        (parent dir + stem)
   `prefixesByLongKey` is consulted first and only falls back to
   `prefixesByShortKey` when no long-key match is available. Both
   the ingestion pipeline (parse-impl.ts) and the group extractor
   (http-patterns/python.ts) carry the same scheme so the graph
   nodes and HTTP contracts agree on which prefix applies.

   New protocol field `ExtractedRouterModuleAlias` (parse-worker →
   parsing-processor → parse-impl) lets Shape-A
   `<host>.include_router(<mod>.router, prefix='/x')` calls promote
   to a long key when the same file imports `<mod>` via
   `from <pkg> import <mod>`. Without this, `api/users.py` and
   `admin/users.py` collided on the basename `users` and the admin
   file's routes inherited the `/users` prefix that was only meant
   for `api/users.py`.

3. Non-`app` host variable names (FINDING 4)
   The group-layer `INCLUDE_ROUTER_*_PATTERNS` queries pinned the
   host identifier to the literal `"app"` and dropped every
   `application = FastAPI()` / `api = FastAPI()` pattern — the
   constraint was redundant given that the call shape
   (`include_router` invoked with a router argument and a
   `prefix=` keyword) is already specific enough. The pin is
   removed; the ingestion regex was already unrestricted.

4. Ingestion-layer regression tests (FINDING 1)
   The previous PR added group-layer tests
   (`http-route-extractor.test.ts`) but zero in-tree tests for the
   ingestion path. Two new suites pin the
   worker → parse-impl → routes flow:

   - `test/unit/fastapi-router-bindings.test.ts` (23 cases):
     `extractFastAPIRouterBindings()` is split into a stand-alone
     module so it can be unit-tested without booting a worker
     thread, then pinned for regex shape, two-tier key emission,
     relative-import support, and negative cases.
   - `test/integration/fastapi-prefix-pipeline.test.ts` (5 cases)
     plus `test/fixtures/fastapi-prefix-app/` — runs the full
     `runPipelineFromRepo()` against a realistic multi-package
     fixture (containing both `api/users.py` and `admin/users.py`)
     and inspects the resulting `Route` graph nodes for cross-file
     prefix joining and absence of cross-package bleed.

Verification

  - `npx tsc --noEmit`: pass
  - PR-touched test suites (6 files / 117 cases): all green
  - `npx prettier --check`: pass on touched files
  - `npx eslint`: 0 errors on touched files

Cache / compatibility

  The new `routerModuleAliases?` field on `ParseWorkerResult` and
  `routerModuleAliases` on `WorkerExtractedData` are optional /
  guarded with `?? []`, so historical parse-cache entries continue
  to load without forced re-scan.

Refs PR #1877.

* refactor(ingestion): move fastapi-router-bindings out of workers/ — pure module, not a worker

Addresses @magyargergo's `CHANGES_REQUESTED` review on PR #1877:

> Sorry I just found that we are introducing a new worker in the PR.

`gitnexus/src/core/ingestion/workers/fastapi-router-bindings.ts` was a
**pure-function module** — it never imported `worker_threads` or
`parentPort`, never spawned a worker, and was never registered as a
worker entry. It was placed in `workers/` purely because it was split
out of `workers/parse-worker.ts` to make its functions unit-testable
without booting a worker thread (parse-worker is itself the worker
entry and cannot be loaded from the main thread).

To remove the misleading directory placement:

  • The implementation moves to
    `gitnexus/src/core/ingestion/route-extractors/fastapi-router-bindings.ts`,
    alongside the other framework-specific route extractors (`expo`,
    `nextjs`, `php`, `laravel`, `middleware`, `response-shapes`).
  • `workers/parse-worker.ts` keeps a thin re-export so the worker
    entry can keep using `extractFastAPIRouterBindings` directly. The
    re-export now carries an explicit comment stating that the imported
    file is **not** a worker and that the `workers/` directory
    deliberately hosts only true worker entries (`parse-worker.ts`,
    `worker-pool.ts`, `quarantine.ts`).
  • The new file's leading docstring opens with "NOT A WORKER" and
    explains why it exists where it does.
  • The unit test (`test/unit/fastapi-router-bindings.test.ts`) is
    updated to import from the new path.

No behaviour change. The function body, signatures, and exported types
are identical.

Verification

  • `npx tsc --noEmit`: pass
  • `npx tsc` (dist rebuild): pass
  • `test/unit/fastapi-router-bindings.test.ts` (23 cases): all green
  • `test/integration/fastapi-prefix-pipeline.test.ts` (5 cases): all green
  • `test/unit/group/http-route-extractor.test.ts` (63 cases): all green
  • `npx prettier --check` on touched files: pass
  • `npx eslint` on touched files: 0 errors

Refs PR #1877.

* refactor(ingestion): drop parse-worker re-exports; consumers import router types directly from route-extractors

Addresses @magyargergo's two remaining review comments on PR #1877:

1. **`gitnexus/src/core/ingestion/workers/parse-worker.ts:247`** —
   "Can you please remove them and update the call sites?"

   The `export type { ExtractedRouterInclude, ExtractedRouterImport,
   ExtractedRouterModuleAlias } from '../route-extractors/...'` block
   in parse-worker.ts is gone. The remaining `import type {…}` is
   purely local — used only to type the corresponding fields on
   `ParseWorkerResult` below — and the leading comment now says so
   explicitly ("this file does NOT re-export them"). The
   `extractFastAPIRouterBindings` symbol is also no longer re-exported
   from parse-worker.ts; it's still imported here so the worker entry
   can call it per file, but downstream consumers must reach it via
   `route-extractors/fastapi-router-bindings` directly.

   Call sites updated:
     - `gitnexus/src/core/ingestion/parsing-processor.ts`
     - `gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts`

   Both files now `import type { ExtractedRouterInclude,
   ExtractedRouterImport, ExtractedRouterModuleAlias }` directly from
   `route-extractors/fastapi-router-bindings.js`. The worker types
   they still need (`ParseWorkerResult`, `ExtractedToolDef`, etc.)
   keep coming from `workers/parse-worker.js`.

   The unit + integration tests already imported from the new path,
   so no test changes were required.

2. **`gitnexus/src/core/ingestion/parsing-processor.ts:168`** —
   suggested simplification:

       for (const item of result.routerIncludes ?? []) allRouterIncludes.push(item);
       for (const item of result.routerImports ?? []) allRouterImports.push(item);
       for (const item of result.routerModuleAliases ?? []) allRouterModuleAliases.push(item);

   Applied verbatim. Replaces the previous `if (result.…) for …`
   guards. The cache-compat semantics are unchanged — historical
   parse-cache entries that lack these fields still load cleanly,
   the new form just spells the fallback inline.

No behavior change, no tests touched, no public API change.

Verification

  • `npx tsc --noEmit`: pass
  • `npx tsc` (dist rebuild): pass
  • PR-touched test suites (6 files / 117 cases): all green
  • `npx prettier --check` on touched files: pass
  • `npx eslint` on touched files: 0 errors

Refs PR #1877.

* refactor(ingestion): hoist fastapi-router-bindings type imports to top of parse-worker.ts

Move the `import type { ExtractedRouterInclude, ExtractedRouterImport,
ExtractedRouterModuleAlias }` block to the top of the file with the
other type imports, and drop the comment that previously sat next to
ExtractedDecoratorRoute.

---------

Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-05-28 19:04:19 +01:00

391 lines
15 KiB
TypeScript

/**
* U20 — Integration regression test for chunk-cache corruption on
* worker quarantine.
*
* Pins the fix for the Codex adversarial review finding on PR #1693
* (`docs/plans/2026-05-20-002-fix-chunk-cache-corruption-on-worker-quarantine-plan.md`):
*
* The chunk hash is computed from every file in the chunk, but the
* worker pool's Layer 3 quarantine filters quarantined files out of
* dispatch. Before the fix, the chunk-loop would cache the partial
* worker results under the full-coverage chunk hash, locking in
* silent corruption that the next analyze would replay.
*
* Runs with REAL `worker_threads` + `createWorkerPool`. Injects a
* custom worker script via `workerUrlForTest` that:
* 1. Implements the U17/U19 IPC protocol (decode Buffer or hybrid
* envelope/contents shape, decode header + JSON payload).
* 2. Emits a `{type:'ready'}` handshake so the pool's
* `waitForWorkerReady` resolves promptly.
* 3. On a sub-batch containing `poison.ts`, emits a starting-file
* and exits with code 134 — deterministic worker death the pool
* attributes to `poison.ts` via the in-flight signal, then
* adds to its session-scoped quarantine.
* 4. On a sub-batch without poison, synthesizes a minimal valid
* ParseWorkerResult with a Function node per file (no
* tree-sitter dependency in the test worker — the synthesized
* nodes give the merge step deterministic content to add to the
* graph).
*
* U20 design pivot — no sequential fallback. The U1 sequential
* reparse for quarantined chunk files was removed: relying on the
* worker pool's resilience layers (respawn budget, circuit breaker,
* quarantine, slot-attribution, cumulative timeout) as the SOLE
* contract avoids re-triggering tree-sitter native crashes on the
* main thread and gives operators a clear hard signal when workers
* exhaust. Quarantined files are missing from this run's graph;
* they're surfaced in the per-chunk warn log; U2's cache-skip keeps
* the chunk uncached so the next analyze with a fresh pool retries.
*
* Assertions exercised here:
* - Worker-path runs and produces results for surviving files
* (good_a, good_c) via the synthesized worker output.
* - The quarantined file (poison.ts) is NOT in the graph — no
* sequential reparse fired.
* - U2 (cache-write suppression): `parseCache.entries` does NOT
* contain the chunk hash after the run. `parseCache.usedKeys`
* DOES contain it (chunk was processed; the cache write was
* specifically skipped). A cross-run scenario verifies that a
* subsequent dispatch with a fresh pool re-attempts the chunk
* (cache miss) and the cache stays empty for that chunk.
*
* Why integration over unit:
* - The fix lives at the boundary between processParsing
* (`parsing-processor.ts`) and the chunk-loop
* (`pipeline-phases/parse-impl.ts`) under a real
* workerPool. Unit-mocking the worker-pool import bypasses the
* structured-clone boundary, the dispatch lifecycle, and the
* actual quarantine flow — it verifies the test setup rather
* than the contract. The real worker thread executing the test
* script through the U17/U19 IPC protocol IS the load-bearing
* surface; this test exercises it end-to-end.
* - The `writeReadyWorker` pattern from `worker-pool.test.ts` is
* reused inline here (the READY_PREAMBLE + test-worker script
* composition).
*
* Wall-clock budget: well under 5 s under normal CI conditions.
*/
import { describe, it, expect, afterEach, beforeEach } from 'vitest';
import { tmpdir } from 'node:os';
import { mkdtempSync, writeFileSync, mkdirSync, rmSync, statSync } from 'node:fs';
import path from 'node:path';
import { pathToFileURL } from 'node:url';
import { createKnowledgeGraph } from '../../src/core/graph/graph.js';
import { runChunkedParseAndResolve } from '../../src/core/ingestion/pipeline-phases/parse-impl.js';
import { computeChunkHash, fileContentHash } from '../../src/storage/parse-cache.js';
import type { ParseWorkerResult } from '../../src/core/ingestion/workers/parse-worker.js';
/**
* Inline READY preamble + IPC decode wrapper (mirrors
* `test/integration/worker-pool.test.ts`'s READY_PREAMBLE). Lets the
* test worker script below speak the production U17/U19 IPC protocol
* without importing dist/protocol.js (the script runs as a standalone
* CJS file at a temp path, so it can't resolve dist/ via relative
* paths reliably).
*/
const READY_PREAMBLE = `
const { parentPort: __pp } = require('node:worker_threads');
const __decoder = new TextDecoder('utf-8');
const __decodeFrame = (raw) => {
if (
raw && typeof raw === 'object' &&
raw.type === 'sub-batch' &&
Array.isArray(raw.files)
) {
return {
type: 'sub-batch',
files: raw.files.map((f) => ({
path: f.path,
content: typeof f.content === 'string' ? f.content : __decoder.decode(f.content),
})),
};
}
return raw;
};
const __origOn = __pp.on.bind(__pp);
__pp.on = (event, handler) => {
if (event !== 'message') return __origOn(event, handler);
return __origOn(event, (raw) => handler(__decodeFrame(raw)));
};
__pp.postMessage({ type: 'ready' });
`;
/**
* Test worker script. Synthesizes minimal ParseWorkerResult entries for
* non-poison files; deterministically crashes on poison.ts via
* `process.exit(134)`. Accumulates across sub-batches; emits the
* accumulated result on `flush`.
*/
const TEST_WORKER_SCRIPT = `
const { parentPort } = require('node:worker_threads');
const accumulated = {
nodes: [],
relationships: [],
symbols: [],
imports: [],
calls: [],
assignments: [],
heritage: [],
routes: [],
fetchCalls: [],
fetchWrapperDefs: [],
decoratorRoutes: [],
routerIncludes: [],
routerImports: [],
toolDefs: [],
ormQueries: [],
constructorBindings: [],
fileScopeBindings: [],
parsedFiles: [],
skippedLanguages: {},
fileCount: 0,
};
parentPort.on('message', (msg) => {
if (msg && msg.type === 'sub-batch') {
const poison = msg.files.find((f) => f.path.endsWith('poison.ts'));
if (poison) {
parentPort.postMessage({ type: 'starting-file', path: poison.path });
process.exit(134);
}
for (const file of msg.files) {
const baseName = file.path.split('/').pop().replace(/\\.ts$/, '');
accumulated.nodes.push({
id: 'func:' + file.path,
label: 'Function',
properties: {
name: baseName,
filePath: file.path,
startLine: 1,
endLine: 1,
language: 'typescript',
isExported: true,
},
});
accumulated.fileCount++;
}
parentPort.postMessage({ type: 'progress', filesProcessed: accumulated.fileCount });
parentPort.postMessage({ type: 'sub-batch-done' });
return;
}
if (msg && msg.type === 'flush') {
parentPort.postMessage({ type: 'result', data: accumulated });
}
});
`;
const FIXTURE_FILES = {
'src/good_a.ts': 'export function good_a() { return 1; }\n',
'src/poison.ts': 'export function poison() { return 2; }\n',
'src/good_c.ts': 'export function good_c() { return 3; }\n',
};
describe('U20: parse-impl quarantine + chunk-cache integration (PR #1693 Codex finding)', () => {
let tempDir: string;
let repoDir: string;
let workerPath: string;
beforeEach(() => {
tempDir = mkdtempSync(path.join(tmpdir(), 'parse-impl-quarantine-cache-skip-'));
repoDir = path.join(tempDir, 'repo');
mkdirSync(repoDir, { recursive: true });
// Write the fixture files to repoDir so filesystem-walker / chunk
// loop pick them up by relative path.
for (const [rel, content] of Object.entries(FIXTURE_FILES)) {
const full = path.join(repoDir, rel);
mkdirSync(path.dirname(full), { recursive: true });
writeFileSync(full, content);
}
// Write the test worker script to the same tempDir so it doesn't
// collide with anything else. The READY preamble + test script
// share one .js file the pool spawns via `new Worker(URL)`.
workerPath = path.join(tempDir, 'test-quarantine-worker.js');
writeFileSync(workerPath, READY_PREAMBLE + TEST_WORKER_SCRIPT);
});
afterEach(() => {
rmSync(tempDir, { recursive: true, force: true });
});
it('worker quarantine leaves poison.ts out of the graph AND suppresses chunk-cache write', async () => {
const filePaths = Object.keys(FIXTURE_FILES);
const scanned = filePaths.map((rel) => ({
path: rel,
size: statSync(path.join(repoDir, rel)).size,
}));
// The chunk hash is computed from EVERY file's content hash. The
// load-bearing U2 assertion below checks `parseCache.entries.has`
// against this exact value, so we compute it the same way
// parse-impl does.
const expectedChunkHash = computeChunkHash(
filePaths.map((p) => ({
filePath: p,
contentHash: fileContentHash(FIXTURE_FILES[p as keyof typeof FIXTURE_FILES]),
})),
);
const parseCache = {
version: 'test',
entries: new Map<string, ParseWorkerResult[]>(),
usedKeys: new Set<string>(),
};
const graph = createKnowledgeGraph();
await runChunkedParseAndResolve(
graph,
scanned,
filePaths,
filePaths.length,
repoDir,
Date.now(),
() => {},
{
skipWorkers: false,
// Force the worker-pool gate to open on the 3-file fixture.
workerThresholdsForTest: { minFiles: 1, minBytes: 1 },
// Inject the custom worker script — the pool will spawn it
// instead of the production parse-worker.js.
workerUrlForTest: pathToFileURL(workerPath) as URL,
// Test-only worker pool size — keep at 1 so the poison-file
// sub-batch deterministically lands on the only slot (no
// chance of poison + good landing in different slots).
workerPoolSize: 1,
parseCache,
},
);
const nodes = Array.from(graph.nodes.values());
// Quarantine contract: poison.ts is genuinely missing from the
// graph for this run. The custom worker crashed on it; no
// sequential reparse rescued it; the operator sees the per-chunk
// quarantine warn log. A future analyze with a fresh pool gets
// another chance via U2's cache-skip below.
expect(
nodes.some(
(n) => n.label === 'Function' && (n.properties as { name?: string }).name === 'poison',
),
).toBe(false);
// Surviving files' symbols come from the custom worker's
// synthesized output via the normal worker-path merge. Pinning
// them here catches a regression that would drop worker results
// entirely when quarantine fires.
expect(
nodes.some(
(n) => n.label === 'Function' && (n.properties as { name?: string }).name === 'good_a',
),
).toBe(true);
expect(
nodes.some(
(n) => n.label === 'Function' && (n.properties as { name?: string }).name === 'good_c',
),
).toBe(true);
// U2 assertion: chunk-cache write was suppressed. The chunk hash
// is in usedKeys (chunk WAS processed) but absent from entries
// (cache write skipped because of the quarantine intersection).
// This is the load-bearing cross-run protection: a future analyze
// with unchanged content will re-derive the same chunkHash, miss
// the cache, and re-dispatch — giving the file another chance
// against a fresh-quarantine pool.
expect(parseCache.entries.has(expectedChunkHash)).toBe(false);
expect(parseCache.usedKeys.has(expectedChunkHash)).toBe(true);
expect(parseCache.entries.size).toBe(0);
});
it('cross-run: unchanged fixture re-dispatches on a second pass because the cache was empty', async () => {
// First pass: same setup as the previous test. Cache stays empty
// because poison.ts triggered quarantine.
const filePaths = Object.keys(FIXTURE_FILES);
const scanned = filePaths.map((rel) => ({
path: rel,
size: statSync(path.join(repoDir, rel)).size,
}));
const expectedChunkHash = computeChunkHash(
filePaths.map((p) => ({
filePath: p,
contentHash: fileContentHash(FIXTURE_FILES[p as keyof typeof FIXTURE_FILES]),
})),
);
const parseCache = {
version: 'test',
entries: new Map<string, ParseWorkerResult[]>(),
usedKeys: new Set<string>(),
};
// FIRST PASS.
{
const graph = createKnowledgeGraph();
await runChunkedParseAndResolve(
graph,
scanned,
filePaths,
filePaths.length,
repoDir,
Date.now(),
() => {},
{
skipWorkers: false,
workerThresholdsForTest: { minFiles: 1, minBytes: 1 },
workerUrlForTest: pathToFileURL(workerPath) as URL,
workerPoolSize: 1,
parseCache,
},
);
// Confirm the precondition for the second-pass test: cache is
// empty for this chunk hash.
expect(parseCache.entries.has(expectedChunkHash)).toBe(false);
}
// SECOND PASS — same content, same parseCache, fresh worker pool
// (createWorkerPool is called per `runChunkedParseAndResolve`, so
// every invocation gets a clean quarantine slate). With the cache
// empty for this chunk, the second pass MUST dispatch the chunk
// again rather than replaying a cache entry. The custom worker
// crashes again on poison.ts → quarantine again → cache still
// skipped. Symptom: cache state unchanged, graph still complete.
{
const graph2 = createKnowledgeGraph();
await runChunkedParseAndResolve(
graph2,
scanned,
filePaths,
filePaths.length,
repoDir,
Date.now(),
() => {},
{
skipWorkers: false,
workerThresholdsForTest: { minFiles: 1, minBytes: 1 },
workerUrlForTest: pathToFileURL(workerPath) as URL,
workerPoolSize: 1,
parseCache,
},
);
// Cache stayed empty (still no entry for this chunk hash) — the
// load-bearing cross-run protection.
expect(parseCache.entries.has(expectedChunkHash)).toBe(false);
expect(parseCache.usedKeys.has(expectedChunkHash)).toBe(true);
// Worker path ran again; surviving files in the graph; poison
// still absent per the U20 contract (workers are the sole
// resilience layer, no sequential reparse).
const nodes2 = Array.from(graph2.nodes.values());
expect(
nodes2.some(
(n) => n.label === 'Function' && (n.properties as { name?: string }).name === 'good_a',
),
).toBe(true);
expect(
nodes2.some(
(n) => n.label === 'Function' && (n.properties as { name?: string }).name === 'poison',
),
).toBe(false);
}
});
});