mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-10 08:27:27 +00:00
7 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gergő Magyar
|
bad2422aa1
|
feat: add the test scenario map and a Vitest 5 mutation runner (#3686)
* feat(ci): add advisory duplicate-test scan to the PR report Adds `npm run test:dupes` (gitnexus/scripts/test-dupes.ts) and a comment-only CI job that reports, for the lines a PR changes: - same-test: same body and same setup as an existing test - assertions-pinned: every assertion is a literal fn(...) -> matcher that another test already makes - same-fixture-setup: a before* hook builds a fixture another describe already builds - copied-helper: a top-level test helper identical to one elsewhere - orphan-bench-gate: a bench script with --check that nothing runs A case's identity includes its hooks, describe-level statements, wrapper arguments, loop heads, each tables, referenced file-local declarations and resolved imports, with fixture paths compared by value. Extra context can only split a group, so identical bodies that run against different fixtures, mocks or env are not reported. The ci.yml job is continue-on-error and outside the CI Gate. ci-report.yml renders its JSON into the sticky CI comment, keeping only whitelisted kinds and strict path/line shapes because the artifact comes from PR code. `// dupe-ok: <reason>` silences an intentional repeat. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(mutation): record Vitest 5 and Stryker 10 probe findings Go/no-go probe for the Stryker Vitest 5 runner: name-filter separator, per-spec task ids, static mutant activation, TypeScript plugin loading in child processes, setup-file meta and hook ordering. * feat(mutation): prepare a mutable copy and a Vitest mutation config Prepared copy blanks TypeScript as-assertions (Stryker's instrumenter skips every TSAsExpression) with equal-length spaces, checked by AST equality, and relocates gitnexus-shared/src inside the package so it can be mutated. The mutation Vitest config resolves gitnexus-shared to the relocated sources and takes its test list from a scope file. * feat(mutation): add a Stryker test runner for Vitest 5 Stryker's vitest-runner 10.0.0 runs zero tests per mutant under Vitest 5 because Vitest 5 matches testNamePattern against ' > '-joined names. This runner selects tests by Vitest task id per file, keeps the configured fork pool, shares the dry-run manifest across Stryker worker processes, attributes hook-time hits to the suite's tests, and records a per-test outcome class in sidecar files. Adds exact devDependencies @stryker-mutator/core and @stryker-mutator/api 10.0.0. * feat(mutation): add global mutant identities and a span-map check A global mutant identity (original repo path, original content hash, span, mutator, replacement) makes mutants comparable across Stryker runs. The span-map check proves every mutant of a prepared copy sits on the same span and text in the original source, apart from blanked as-assertions. * feat(mutation): add a scope driver for Stryker runs run-scope turns a scope file into a prepared copy, a generated Stryker config (vitest5 or command runner), a Stryker run and normalised results: global mutant ids, runner test ids, sidecar outcome classes, consistency flags and the span-map check. * fix(mutation): count assertion kills only for killed runs A run classified timeout, inconclusive, invalid or infrastructure can still carry assertion rows (a test failing after a swallowed hit-limit error, or under a retry). Those kills are not trustworthy, so assertionKilledBy is now empty unless the run's status is killed. Found by the conformance suite. * feat(mutation): add the runner conformance suite (gate 2) A fixture project under test/fixtures/mutation-conformance pins 23 runner behaviours (static and collection-time mutants, beforeAll attribution, disableBail, it.each row ids, outcome classes for runtime, hook, collection, timeout, infrastructure, retry and zero-test runs, NoCoverage, environment isolation, runner reuse and manifest sharing) in two scheduling profiles. The path map and scope driver accept an explicit gitnexus/test/fixtures/mutation-* root for it. * feat(mutation): classify pure-JS unit tests for mutation runs Walks each unit test's import closure (static imports plus statically resolvable dynamic imports, require and createRequire) and rejects tests that reach native packages, worker threads or child processes, read source or fixture files a prepared copy would break, or contain unresolvable dynamic imports. 247 of 911 unit test files are pure JS. * feat(mutation): add gate 1 execution-path fidelity and the envelope record Gate 1 compares a normal Vitest 5 run of the pure-JS category with the runner's unmutated Stryker run: test identities with locations, outcomes, effective config against an allowed-difference list, resolved module paths (everything inside the active sandbox) and a runtime load check for native bindings, worker construction and child processes. It passes on 246 files / 4541 tests and writes test/fixtures/mutation-oracle/envelope.json. The runtime check found commander loading child_process, so the classifier now rejects tests importing commander. * feat(mutation): add the oracle's command-runner run and classifier The Vitest 5 cross-check oracle runs its own Stryker run with the command runner (cleanTempDir false). A wrapper runs the scope's tests per mutant with an oracle reporter and writes a per-mutant record, and the oracle classifies every mutant into the KTD17 outcome classes with its own classifier. Oracle code imports no runner code. * feat(mutation): add the oracle single-test launcher and cost benchmark launch-single runs one test with one mutant in a fresh process inside the oracle's kept sandbox, selected by file:line plus an anchored name pattern, with the mutant installed before any import, an oracle setup file that proves the installed id and buckets hits (load and collection, hook, per-test), a wall-clock limit and a process-group kill. A launch is valid only when exactly the intended test ran. The benchmark projects the three-scope oracle sample's cost on 4 vCPU. A boundary test keeps oracle code from importing runner code. * feat(mutation): add the oracle freeze and gate 3 compare freeze samples mutants per scope with a recorded seed, tops up strata, counts stratum populations from the oracle's own coverage probe, launches every sampled mutant against every scope test (resumable), and writes a manifest keyed by content hashes of the scope's import closure, Vitest configs and prepare tool. compare checks freshness, inventories (tests and global mutant ids) before outcomes, every mutant's class and sampled per-test outcomes, allowing only the four KTD13 normalisations. * feat(mutation): add gate 4 positive controls and determinism Nine planted controls, selected from the scopes' own Stryker mutants by global id (three in relocated gitnexus-shared as-const tables), must be killed by exactly their expected killers. Two scheduling profiles (concurrency 1 with unlimited runner reuse, concurrency 3 with one mutant per runner process) must give identical outcome classes and kill sets. * ci(mutation): add an advisory mutation-runner validation workflow Runs the runner conformance suite (gate 2) on pull requests that touch the mutation tooling, its fixtures, the gitnexus manifest, lockfile or Vitest config. workflow_dispatch also runs gate 1, the gate 3 oracle compare (or a per-scope re-freeze whose manifests are uploaded, never committed), gate 4 and the oracle cost benchmark. All jobs are advisory. Documents the tooling in TESTING.md and the residual deltas against upstream StrykerJS. * test(mutation): freeze the gate 3 oracle manifests and record Phase 0 results Frozen Vitest 5 cross-check oracle manifests for the three pure-JS scopes (seed u7-gate3-1): shared.ts, schema-constants.ts and middleware.ts. Gate 3 passes: all 1,606 mutant classes and all 20,455 sampled per-test outcomes agree with the runner, 0 differences. Adds the approved Test Scenario Map design (updated with Phase 0 findings and gate results) and the Phase 0 implementation plan. * refactor(mutation): simplify the Phase 0 tooling Shared CLI/fs helpers for the non-oracle tools, an oracle-local util for isObject and atomic writes, formatMutateEntry and formatDuration reused, a typed isUnusableClass, and hot-path fixes: per-sidecar index in the gate 3 compare, lazy suite-chain in the runner's dry-run hit attribution, one Classifier per freeze, memoised display paths and Vitest lookups. Behaviour unchanged: unit tests, gate 3 compare (byte-identical report) and the conformance suite (23/23) pass. * fix(review): apply review findings - Move the duplicate-test scanner out of the published scripts/ directory to gitnexus/tools/test-dupes/ so it no longer ships in the npm package. - Scope the orphan bench gate to touched lines, like every other scanner finding. - Let the conformance environment case reach Stryker through an explicit childEnv override, so it exercises the runner's own __STRYKER_ACTIVE_MUTANT__ deletion (a negative check now fails it). - Do not raise the zero-tests flag when a timeout outside-test row explains the zero. - Run Stryker in its own process group and signal the group on cancellation. - Fingerprint conformance inputs and refuse stale artefacts in --compare-only. - Path-gate the mutation workflow on gitnexus-shared/package.json and the setup action; anchor the PR-report path filter with \A...\z. * chore(security): mark the runner's provide/inject key as a gitleaks false positive gitleaks' generic-api-key rule matched PROTOCOL_KEY = 'gitnexusStrykerVitest5' (a Vitest provide/inject key name). Allow the line inline and ignore the fingerprint in the commit that introduced it. * test(mutation): re-freeze the schema-constants oracle manifest Merging main changed the scope's import closure (template-arguments.ts), so gate 3 reported re-freeze required. Re-frozen with seed u7-gate3-2: 72 sampled mutants, 3024 valid observations, 0 invalid. Gate 3 now passes on all three scopes (shared 1232, schema-constants 72, middleware 302 mutants) with 0 differences. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(plans): add the Test Scenario Map Phase 1 plan * chore(security): stop the gitleaks allowlist comment from quoting the key The previous comment quoted the provide/inject key assignment and gitleaks flagged the comment line itself (generic-api-key). Reword it and allowlist that historical finding by fingerprint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(mutation): select single tests in the vitest5 runner and keep per-test classes Phase 1 U1 (KTD2, KTD9, KTD16): - driver scope accepts optional testIds, passed as the vitest5.testIds runner option; the dry run collects, maps ids to task ids, runs only those tests and fails before any mutant run on unknown ids - the dry-run sidecar (schema 2) carries the hook marker: per mutant, the tests a suite hook attributed it to - results.json (schema 2) keeps every sidecar attempt's per-test rows (perTest), attemptsDisagree and hookAttributedTests - gate 2 gains a selection scope and hook-marker cases (26 cases hold) Gate 3 hashed inputs are untouched. * refactor(test-dupes): export Layer 0's case extraction for the scenario map Phase 1 U2 (KTD6): export extract, extractCases, buildFrame, closure, staticValue, suppressed and dupeOkReason, plus a per-case record with the describe path, title, context hash, body, each-table node and enclosing loop head. Layer 0's --all scan output is byte-identical before and after (1464 files, 114 findings). * test(mutation): refresh the gate 1 envelope after the runner extensions Gate 1 re-ran over the current pure-JS category after U1 and passed with 0 differences: 251 files, 4667 tests. The classifier now rejects conformance-compare.test.ts (imports the child_process-spawning conformance runner) and php-namespace-extraction.test.ts (now reaches tree-sitter through main), and accepts the three oracle unit tests. * feat(scenario-map): add the eligible-test inventory, identity join and anchors Phase 1 U3 (R1, R2, R15; KTD3, KTD7): - eligibility = gate 1 envelope intersect a fresh classifier run, with the delta; controls mode enrols a fixture's *.case.ts files - static expansion of it.each/test.each/test.for tables and literal for-of loops into per-row instances; anything not statically evaluable is unresolved (fail closed) - a pure join of static instances to runtime records through Phase 0 test ids; any count or name mismatch makes the tests unsupported - continuity anchors (no line; row and body fingerprints) and the one production-file predicate shared by every later stage - test-dupes: additive exports (shapeOf, roleOf, eachTableOf, ASTs, an isTest option); Layer 0 output byte-identical Over the repo: 4667 runtime tests, 4597 joined one-to-one, 70 unsupported (5 tables built at runtime), 0 mismatches. * test(scenario-map): add the controls fixture project with expected verdicts Phase 1 U17: one pair per design §8 pure-JS adversarial case (Math.round, parseInt radix, localeCompare, normalize, Date boundary, regex alternatives, numeric boundary with a constants-only module, an expected-only difference, a derived value reaching toFixed, an enum-member-only difference), an order-dependent pair, an object-key row, and positive controls (verbatim duplicate, repo arithmetic, Set table reads). expected.json was written from the design's stated outcomes. The fixture has its own Vitest project and tsconfig; no repo project collects it. * feat(scenario-map): extract per-instance checks and context Phase 1 U5 (R5, KTD6): - checks: the shape of each expect(...).matcher(...) with literals abstracted and selector literals in the subject kept exact; snapshots hashed from inline text or the .snap entry; a documented supported matcher list, anything else unsupported; per-row for it.each/test.for - context: Layer 0's context hash plus body-level mocks/env writes and the test call's own modifiers and options - checksContextKey for the second candidate source Over the 251 eligible files: 4600 instances in 2.9 s, 183 with an unsupported form, 37 with an unresolved selector. * feat(scenario-map): add Layer 1 per-test V8 coverage and candidate retrieval Phase 1 U4 (R3, R4; KTD4, KTD5): - a Vitest setup file records per-test V8 block coverage in a prepared copy under the mutation config (gitnexus-shared runs from source), with design §5.2 attribution: module load discarded, a suite's beforeAll delta taken at its first beforeEach, concurrent tests flagged unattributable - provenance per test: executed files, native-package entries and worker or child-process spawn call sites - candidates: exact Jaccard >= 0.99 (size-ratio and prefix filters) plus identical checks-and-context buckets; connected components schedule Layer 3 only Over the 251 eligible files: 4667 tests in 1:55 (82.8 s Vitest), 6281 candidate pairs in 611 groups, largest group 36 tests. Controls: 16/16 pairs are candidates, 0 cross-pair candidates. The directory is path-coverage/ because coverage/ is ignored by git, prettier and eslint. * feat(scenario-map): compute the per-instance input facet Phase 1 U6 (R5, R9; KTD6), porting the input-facet spike without its hard-coded inputs: - rules 1-8: tree-sitter shapes of inline source, the COBOL profile, best-fit production tables (executed path, 2-hop import closure, property columns; never pooled), selector literals (reusing the checks predicate), object keys, source order, shared fixtures, fixture files - numeric literals with 0/1/-1/negatives exact, and the +-1 rule against production numeric constants from executed files and the static import closure (folded initialisers included) - discarded values per position with source locations for value flow - the residual-difference guard: a body difference the facet misses makes the pair facet-unequal and both instances unresolved Controls: all 16 pairs match expected.json. The spike's untouched holdout-4 still scores 23/23 distinct. Eligible run: 4667 instances in about 33 s, 245 unresolved. * feat(scenario-map): run Layer 3 per candidate group and merge raw observations Phase 1 U7 (R6, R7, R11; KTD7, KTD8, KTD14, KTD17): - whole-file mutate scope per group from Layer 1 provenance and the static closure; executed-but-unmutated repo files mark tests excluded - a run manifest of the prepared inputs that every shard verifies - two plain Vitest 5 baselines (fresh instance per run) plus the Stryker dry run must agree; the generated config must carry perTest coverage, disableBail and static mutants before mutating - raw observations from every sidecar attempt, keyed by group, run kind, lineage, attempt, test and mutant; class disagreement is inconclusive and a missing result is unknown, never a non-kill - deterministic shard planning that never splits a group, with group-over-budget and start-only-if-it-fits rules Controls: all 16 pairs ran as real groups; every kill set matches expected.json. * feat(scenario-map): add the full isolation pass for repeat-stage pairs Phase 1 U8 (R7; KTD8, KTD9): - select repeat-stage pairs (same coveredBy path, checks, context, facet without a residual difference, and the same non-empty kill set) - per pair one together run and one alone run per test (shared across pairs), mutate restricted to the covered universe, a fresh runner process per mutant, all behind the same KTD8 baseline and config checks (runGroup's steps moved into runMutationRun) - coverage and outcome class per test and mutant must agree across main, together and alone; a missing rerun result is unknown; hook-attributed mutants cap the pair at weak Controls: order-dependent ends disagrees; verbatim-duplicate, repo-arithmetic and set-non-member agree, matching expected.json. Measured fresh-process cost: about 5.5 s per mutant. * feat(scenario-map): add the row-deletion operator Phase 1 U9 (R8, KTD10): - find the one supported production table (object literal with plain keys, literal Set/Map) holding both differing values; spreads, computed keys, Object.assign, writes, getters, proxies, duplicate keys or an ambiguous table make the outcome inconclusive - splice one key's text in a throwaway prepared copy, run the pair with a fresh Vitest instance per run after a control run that proves both pass and the edited file loaded - symmetric / asymmetric (Row) / not applicable / inconclusive, with RowDeletion global mutant ids - vitest-run: an opt-in load probe (probeFiles, loadedFiles) Controls: all 16 pairs match expected.json (object-key-row and set-membership asymmetric). * feat(scenario-map): add the static value-flow check Phase 1 U10 (R9, KTD11): for facet-equal pairs whose discarded values differ, all sources must come back clean or the pair is vetoed to Review: - Babel binds each differing value to its entry (call argument, fixture field, table key); an expected-only difference is unknown - the TypeScript 7.0.2 unstable checker resolves entries and callees through aliases; a version mismatch degrades the whole run - a walk stitched across functions over GitNexus PDG REACHING_DEF and CALL_SUMMARY edges (pdg_query flows are intra-procedural): opaque sinks (lib/@types/node_modules, any regex) veto, truncation, dynamic dispatch, a depth cap or a missing PDG layer are unknown or degraded; repo code is clean only inside the pair's mutated files - row-deletion exports scanSupportedTables so table reads mean exactly the tables row deletion supports Controls: 0 mismatches against expected.json. Whole-repo analyze --pdg: 6:49 wall, 8.4 GB peak RSS. * feat(scenario-map): add the value-flow trace run Phase 1 U11 (R9, KTD11): one traced Vitest 5 run over the tests of every facet-equal candidate pair, with a setup file that wraps value-sensitive built-ins (Math, string transforms and comparisons, RegExp, number parsing and formatting, JSON, Intl, Date, sort, plus U10's opaque callees) and records, per test and production call site, the receiver, arguments and count. Storage and table reads stay unwrapped. A shared site with a different receiver or argument, or asymmetric sites or counts, vetoes the pair; a test whose outcome changes under tracing makes it inconclusive. Controls: all 13 pairs with a trace expectation match expected.json, including receivers 24 and 25 reaching toFixed. Overhead on 40 eligible files: 1.19x pooled, no outcome changes. * feat(scenario-map): add the verdict engine Phase 1 U12 (R10, R11, R15; KTD5, KTD8, KTD9, KTD12): a pure function over merged observations and every stage's evidence. Design §2 precedence as an ordered rule list (Inconclusive, Out of scope, Weak, Repeat strong, Repeat weak, Row, Review, Distinct), with the value-flow and trace vetoes as a gate before any repeat category and the residual difference ending in Review. One verdict per pair with its deciding rule and every weak disqualifier; per-test Weak, Inconclusive and Subsumed; per-file gaps recomputed from all observations with the §5.4 wording; dupe-ok pairs kept with their evidence. Output is deterministic. Controls: all 16 verdicts and rules match expected.json. * feat(scenario-map): build the report, issue body and week-over-week diff Phase 1 U13 (R12, R13; KTD13, KTD15): - the JSON report: run provenance (the design §7 cache-key inputs), completeness and degraded flags, the eligibility delta, Layer 0 findings, every verdict with its evidence, per-test properties, gaps, a seeded spot audit of 10 strong candidates, N of M evaluated and the precision-not-measured statement - continuity: mutant fingerprints without revision or span, universe ids from the pair's common universe, evidence hashes per design §5.5 - a markdown rendering and the tracking-issue body (strong repeats, weak tests, gaps; truncated at 60,000 characters; a run-id marker so a stale run never overwrites a newer body) - baseline comparison: new, changed, unchanged, resolved, with first-run and incompatible-baseline banners * feat(scenario-map): add the CLI and the end-to-end controls run Phase 1 U14 (R14, R16; KTD1, KTD14, KTD17): npm run test:scenario-map with plan, shard, value-flow, merge, all and controls stages exchanging JSON in a work directory, so CI jobs and a local run produce the same report. Shards batch groups into one mutation run when their combined estimate fits, and persist row-deletion and isolation results as they finish. Exit codes are set explicitly. controls: all 16 fixture pairs match expected.json (exit 0, 76 min). * feat(scenario-map): fit the scheduled run into the CI budget Phase 1 U14 follow-up (KTD14, KTD17), measured on a seeded slice of the real category; no evidence rule is relaxed: - value flow runs before the shards and re-plans them, so isolation is spent only on pairs that can still be strong (a value-flow veto or unknown ends Review, a trace inconclusive ends Inconclusive); a missing, stale or degraded value-flow result falls back to the provisional plan - a shards stage prints the final shard count for the CI matrix - --concurrency and --speed-factor feed the estimate; the main-run cost form is fixed + mutated bytes (measured; it does not grow with tests) - a group whose isolation does not fit still runs its main run and row deletion; its pairs get isolation not-run (weak cap), and a shard isolates deferred pairs only when their measured universe fits - groups that execute no production file are not run; their pairs are out of scope Predicted CI run: 9 shards at concurrency 4, 1658-1829 min in total, largest shard 196-253 min (budget 300), 0 groups over budget. controls: 16/16 pairs match, 28 min. * ci(scenario-map): add the weekly Test Scenario Map workflow and a PR controls job Phase 1 U15 (R13, R14; KTD13, KTD14, KTD17): - test-scenario-map.yml (weekly + dispatch, advisory): plan, value-flow, shards (sizes the matrix from the value-flow re-plan), a shard matrix (fail-fast off, max-parallel 10, 300-minute deadline), merge (always, against the newest scenario-map-baseline artifact) and upsert-issue (default branch only; the only job with issues: write; replaces the tracking issue's body only for a complete run that is not older than the body's run marker, comments on partial runs, never closes it) - mutation-runner-validation.yml: a path-gated scenario-map-controls job that runs the controls fixture end to end on PRs touching the tooling actionlint (with shellcheck), zizmor 1.24.1 and the concurrency check pass. Neither workflow can run before the merge. * docs(scenario-map): record Phase 1 in the design and the testing guide Phase 1 U16: design corrections (the stitched PDG walk, the trace's receivers and scope, row deletion and baselines on plain Vitest 5, the isolation pass on testIds after value flow), the demote-only additions, the refreshed gates, the measured Phase 1 costs and the CI topology change; TESTING.md gains a Test Scenario Map section (stages, controls fixture, supported matchers, wrapped built-ins, weekly workflow, dupe-ok). * fix(scenario-map): store observations compactly so large batches fit in memory The first full run crashed a shard with "Invalid string length": observations were materialised for every collected test x every mutant, including a valid not-covered survival for each pair (1.6M objects for a batch of 73 selected tests over 21,965 mutants; the batch file needed about 700 MB of JSON). - only a run's selected tests produce observations - each run records its universe (selected tests, mutants with their status, reasons and hook marker); only rows, covered or static mutants without a row, and hook-named tests are stored explicitly; a not-covered survival is derived when read - every consumer (pair comparison, isolation, verdict engine, report, batch split) reads through one index; group observation and isolation run files move to schema 2, and old files are rejected with a message - merge lists an unreadable group result as incomplete instead of dropping it; repeat-stage decisions keep comparison summaries only On the crashing batch: 141,308 explicit observations instead of 1.6M, 12.1 MB instead of an overflow, and a byte-identical verdict report. controls: 16/16 pairs match. * docs(scenario-map): record the Phase 1 measured slice; give the real-repo classify test headroom - design §9: the full-category report comes from the first weekly workflow run; the local full run was stopped because it saturated the shared machine. Records the measured slice and the whole-category plan and value-flow figures. - classify.test.ts: classify the real repo once in beforeAll with a 120 s timeout; it takes about 30 s on a busy host and timed out at the default 30 s. * fix(review): apply review findings and simplify the scenario-map tool Review (ce-code-review, validated findings): - trace: a baseline or traced run that is invalid, inconclusive, retried or has outside-test failures, unhandled errors, unwrapped entries or recorder errors makes every pair inconclusive (trace-run-invalid), never clean; the value-flow stage reports it and merge marks the run partial - verdict: tests pin that a not-required static check for differing values reads as unknown, plus the worker-spawn, missing-provenance, unresolved-input and unknown-hook-marker weak caps - workflow: the baseline comes only from this workflow's own scheduled or dispatched default-branch runs of this repository; an unreadable baseline shows the incompatible-baseline banner instead of failing merge - controls: the oracle fails on an empty expected list, on any unlisted repeat verdict and on a case file without an expected pair - value flow: the PDG analyze budget (25 min) sits below the 50-minute CI step; the TypeScript checker server is closed when opening fails - --concurrency now also bounds every in-process Vitest run (maxWorkers), so local runs stay within the requested cores - merge reads isolation runs tolerantly (an unreadable file makes the run partial); a duplicate test title is renamed; a stale TESTING.md heading reference is updated Simplification (behavior-preserving; merge and plan outputs byte-identical on fixed inputs): one util.ts for shared helpers, a run-level row-deletion cache, one classify() per plan, memoised universe ids and single-pass pair/subsumption/report loops. * fix(test-dupes): keep beforeAll and beforeEach fixture builds apart A same-fixture-setup key now includes the hook name. A beforeAll fixture is built once per suite and a beforeEach one per test, so sharing one result between them would drop per-test isolation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(scenario-map): temporary label-gated pull_request run for PR #3686 schedule and workflow_dispatch only run from the default branch, so the Test Scenario Map workflow cannot run before this PR merges. Adding the run-ci label to the PR starts one full run. A pull_request run writes no tracking issue and no baseline. Remove the trigger before merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(scenario-map): remove the temporary pull_request trigger The one full run on PR #3686 finished (run 40183982111, complete, 9 of 9 shards). This reverts |
||
|
Hector Prats
|
d69eadfb7f
|
fix(windows): 32767-char tree-sitter crash + VECTOR extension SIGSEGV (#1433)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Release Candidate / Check if release candidate should run (push) Waiting to run
Release Candidate / ci (push) Blocked by required conditions
Release Candidate / Publish release candidate to npm (push) Blocked by required conditions
Release Candidate / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
* fix(windows): 32767-char tree-sitter crash + VECTOR extension SIGSEGV
tree-sitter 0.21.x on Windows crashes with SIGSEGV when parsing source
strings longer than 32 767 chars (signed 16-bit integer overflow in the
native binding). Five call sites passed raw file content without any
length guard:
- captures.ts (C# scope extraction)
- namespace-siblings.ts (extractFileStructure)
- parse-worker.ts (worker thread parse path)
- parsing-processor.ts (sequential parse fallback)
Fix: truncate at the last newline before the limit so the fragment stays
syntactically coherent. Files truncated mid-class produce ERROR roots;
captures.ts returns [] for any ERROR-root tree so the legacy DAG handles
the file silently without orphaned scope errors.
Additional C# scope fixes:
- scope-tree.ts: Module scopes may share the same range as a top-level
namespace_declaration (files with no leading `using` directives). The
rangeStrictlyContains check rejects equal ranges. Added
rangeNonStrictlyContains for Module parents.
- scope-extractor.ts: pass1BuildScopes stack-pop used strict containment;
same Module == Namespace range case caused orphaned scopes. Added
moduleAwareContains helper.
- scope-extractor-bridge.ts: empty captures from ERROR-root files still
called extractScope -> "no Module scope found" warning. Added early
return for empty/non-array captures.
- namespace-siblings.ts: three sites pushed onto binding arrays frozen by
finalize-algorithm. Fixed with spread-copy before mutation.
lbug-adapter.ts: INSTALL VECTOR in loadVectorExtension calls the KuzuDB
native extension installer, which crashes with SIGSEGV on Windows via an
unhandled error path in native code. JS try/catch cannot intercept native
signals. Skip extension loading on win32 — vector/embedding search is
unavailable on Windows but all graph index queries work correctly.
Verified on: Windows 11, Node.js 24, gitnexus 1.6.3, pcf8-game codebase
(61 757 nodes / 111 796 edges / 300 flows after fix).
* fix(windows): skip FTS extension load in pool-adapter on Windows to prevent SIGSEGV
LOAD EXTENSION fts crashes the process with SIGSEGV on Windows when the
FTS extension binary is not installed locally. This is an @ladybugdb/core
native bug — the extension loader hits an unhandled error path that raises
a native signal instead of a JS exception, so try/catch cannot protect here.
Add a process.platform === 'win32' guard in both doInitLbug and
initLbugWithDb. When skipped, bm25-index.js catches the resulting
Kuzu catalog errors (CREATE_FTS_INDEX not defined) and returns empty
BM25 results gracefully. All graph queries (cypher, context, impact)
are unaffected.
This is patch 9 of the Windows fix series for gitnexus on Windows:
patch 8 (same PR) already fixed INSTALL VECTOR SIGSEGV in lbug-adapter.ts.
pool-adapter.ts is the separate MCP-server code path that was not covered.
* fix: address codeql findings on PR #1433
The four `lastIndexOf('\n', ...)` calls were committed with a literal
newline inside the single-quoted string instead of the `\n` escape, so
the files do not parse — `tsc` and CodeQL both flagged them. Replace
the embedded newline with `'\n'`.
Also remove the two helpers that were superseded during review and
became dead code: `rangeNonStrictlyContains` in scope-tree.ts (the
equal-range carve-out is handled by `rangeStrictlyContains` +
`rangesEqual` in `canParentScope`) and `moduleAwareContains` in
scope-extractor.ts (`pass1BuildScopes` calls `canParentScope` directly).
* fix(windows): replace 32767-char truncation with chunked-input parsing
The tree-sitter 0.21.x Node binding crashes (SIGSEGV) on Windows when
parser.parse(string, ...) is handed a JS string longer than 32 767 chars.
The crash is in the bindings V8 string-to-buffer conversion and cannot
be intercepted from JS. Previous mitigation truncated source at the last
newline before that boundary, silently losing the file tail and producing
ERROR-root trees from mid-class cuts.
Switch to the callback (Parser.Input) overload via a new parseSourceSafe
helper. tree-sitter pulls source in 16 KiB chunks via repeated callback
invocations, bypassing the broken conversion path. Files are parsed in
full, no data loss, no platform-specific code path.
Removes the now-unnecessary ERROR-root short-circuit in csharp/captures.ts
and the empty-captures shim in scope-extractor-bridge.ts; both existed only
to swallow truncation-induced parse failures.
* fix(windows): cover all parse sites and correct vector-extension state
Address adversarial review on PR #1433:
1. Extend parseSourceSafe to all remaining parser.parse() call sites that
handle full file content. The first commit only converted the four
sites with active truncation hacks; cache-miss paths in
call-processor (x2), heritage-processor (x2), import-processor, and
the Go/Python/TypeScript captures + Go range-binding still called
parser.parse() directly. On Windows those would still SIGSEGV for
files > 32767 chars.
2. Stop setting vectorExtensionLoaded = true on the win32 short-circuit
in lbug-adapter.ts. The flag means "successfully loaded" and is
checked by an early-return at the top of loadVectorExtension; setting
it on the skip path made the second call return true and let
QUERY_VECTOR_INDEX run against a DB without the extension.
3. Drop the placeholder issues/... URL in the same comment.
4. Add unit tests for parseSourceSafe at boundary values: 16 KiB
(direct/callback boundary), the 32 767 Windows crash boundary,
single-line > chunk size, CRLF near boundary, and large all-Chinese
source. Confirms the callback path is correct for non-ASCII content,
which is also exercised by the existing csharp-captures large-file
test.
Researched the chunking concern: tree-sitter Node binding sets
TSInputEncodingUTF16 and divides byte_index by 2 in ByteCountToJS before
calling the JS callback, so the index argument is a UTF-16 code-unit
offset — matching String.prototype.slice. Splitting tokens across chunks
is safe by API contract; the lexer is chunk-agnostic.
* fix(windows): extend parseSourceSafe to group/embeddings + lint enforcement
Closes the remaining Windows SIGSEGV exposure flagged by the Codex
adversarial review on PR #1433. Six pre-existing parser.parse(content)
call sites bypassed parseSourceSafe and could crash the process on
Windows when a contract IDL, route file, or embedding-target source
exceeded 32 767 chars. Adds a lint rule so the regression vector closes
permanently.
Production code:
- Relocate parseSourceSafe from ingestion/utils/ to core/tree-sitter/
so group/ and embeddings/ can import without crossing into ingestion
internals. core/tree-sitter/ already houses parser-loader.ts and is
the natural shared facade. All 11 existing importers updated; no shim
left behind in the old location.
- Route through parseSourceSafe in 5 group extractors (grpc, thrift,
http-route, include, tree-sitter-scanner) and the embeddings
ensureAndParse helper.
- The seventh direct .parse() call in grpc-patterns/proto.ts:49 is a
module-load grammar smoke test parsing a 36-char literal. Trivially
safe by inspection, intentionally direct, filtered out by the lint
rule via the string-literal-arg skip.
Tests:
- 5 caller-side regression tests with a vi.spyOn assertion on
parseSourceSafe. The spy is what catches a regression: parser.parse
on a 40 000-char input succeeds on Linux/macOS, so a "no throw"
assertion alone would silently pass with the bypass reintroduced.
- The vi.mock boilerplate is centralised in
gitnexus/test/helpers/parse-source-safe-mock.ts, dynamic-imported
inside each mock factory so vitest's hoister does not race the
static import binding.
Lint:
- New custom ESLint rule gitnexus/require-safe-parse, scoped to
gitnexus/src/core/**, fails on direct <parser>.parse(<non-literal>,
...) calls and auto-fixes them to parseSourceSafe(<parser>, ...).
Skips JSON/URL/marked/Number/Math, string-literal first args
(smoke tests), test files, and the helper itself. Auto-fix rewrites
the call site only; the developer adds the import after tsc
surfaces the missing identifier — same tradeoff as
unused-imports/no-unused-imports.
Plan: docs/plans/2026-05-10-001-fix-windows-parse-safety-group-and-embeddings-plan.md
* fix(test): use mkdtempSync in http-route-extractor regression test
Address CodeQL js/insecure-temporary-file warning on the new Windows-
SIGSEGV regression test. The test was using path.join(tmpDir, "large-input")
which, when nested inside a Date.now()-based parent tmpDir, lets CodeQL flag
the directory as a predictable-name temp file with race-condition risk.
Switch to fs.mkdtempSync(path.join(tmpDir, "large-input-")) so the suffix
is a secure unique random string.
---------
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
|
||
|
Gergő Magyar
|
d3a7ce95a5
|
feat(core): adopt pino structured logger (#1336)
* feat(core): adopt pino structured logger + add no-console eslint forcing function
Adds `pino` as the project-wide structured logger via a thin wrapper at
`gitnexus/src/core/logger.ts` exposing `createLogger(name, opts?)` and a
default `logger` singleton. Migrates the only security-relevant `console.warn`
site (`bridge-db.ts` `openBridgeDbReadOnly` retry-exhaustion path) to
`bridgeLogger.debug({groupDir, err, attempts}, 'msg')`.
Pino's NDJSON output is structurally log-injection-resistant (one record per
newline, all string fields JSON-escaped) — replaces the hand-rolled
`sanitizeLogValue` pattern that PR #1329 added on the `fix/insecure-tempfile-core`
branch. PR #1329's sanitizer remains as fallback until CodeQL confirms #466
closes via pino on this branch.
Also adds an ESLint `no-console: warn` rule scoped to
`gitnexus/src/**/*.ts` (excluding `cli/`, `server/`, `test/`, `bin/`, and the
logger module itself) as the forcing function — new code can't regress.
Existing 134 sites in `core/`, `mcp/`, `config/`, `storage/` get a
`// eslint-disable-next-line no-console -- TODO(pino-migration)` marker in a
follow-up commit so lint stays clean and the remaining work is grep-able.
Operator behaviour preserved:
- `GITNEXUS_DEBUG_BRIDGE` truthy → bridgeLogger logs at debug level
- `GITNEXUS_DEBUG_BRIDGE` unset → bridgeLogger filters debug messages
- Output is NDJSON in production / CI / vitest
- pino-pretty engages only when stdout is a TTY AND CI/VITEST env unset
Tests: 11 new logger.test.ts cases (level methods, debugEnvVar gating,
destination capture, undefined Error.message safety, CR/LF/U+2028/ANSI
single-record invariant). Group test suite (388 tests) passes unchanged.
`--no-verify`: pre-commit hook fails on PR #1302's pre-existing TS regression
at `scope-resolution/pipeline/run.ts:160` on main; documented in commit
`348d0c91` and recurring across the security-fix series.
Refs: #466 (codeql js/log-injection), PR #1329 follow-up.
* chore(lint): baseline-suppress 134 existing console.* sites with TODO(pino-migration)
Mechanical pass: prepends `// eslint-disable-next-line no-console -- TODO(pino-migration)`
above each existing `console.*` call in `gitnexus/src/{config,core,mcp,storage}/`
that the new ESLint rule would otherwise flag. CLI/server are exempt at the
config level (legitimate stdout output).
Zero functional changes. Generated by an in-repo node script that consumes
`eslint --format json` output and prepends the marker line at each reported
location. Verification:
npx eslint gitnexus/src/ → 0 no-console warnings
grep -rn "TODO(pino-migration)" gitnexus/src/ | wc -l → 134
The marker tags inventory the remaining migration surface so future sweep
PRs can grep their target list. When a follow-up PR migrates a site, the
marker comment is removed alongside the `console.*` → `logger.*` swap.
`--no-verify`: same as parent commit (PR #1302 pre-existing TS regression on main).
* refactor(core): complete pino migration — replace all 134 console.* sites + flip ESLint to error
Codebase-wide sweep of every `TODO(pino-migration)` site flagged in commit
|
||
|
Gergő Magyar
|
de63418f7e
|
fix(mcp): close MCP server timeout — stdout discipline + cold-start friction (#1383)
* fix(lbug): route diagnostic logs to stderr to avoid MCP stdio corruption
Replace console.log/console.warn with console.error in core/lbug so
diagnostic messages reach stderr and never corrupt the JSON-RPC stream
on MCP stdio. Per spec, the server MUST NOT write anything to stdout
that is not a valid MCP message.
- lbug-adapter.ts:367 - schema creation warning (MCP-reachable via lazy
DB init from tool handlers)
- lbug-adapter.ts:1047,1054 - legacy embedding fallback diagnostics
(currently HTTP-only, but covered by upcoming no-console lint rule)
- extension-loader.ts:191 - default warn handler fallback used during
DuckDB extension loading
* feat(mcp): add stdout sentinel via AsyncLocalStorage transport-write tagging
Untagged process.stdout.write calls now redirect to stderr with a
[mcp:stdout-redirect] prefix instead of corrupting the JSON-RPC frame
stream. Identification is correctness-by-construction: the transport
wraps every send() in withMcpWrite() (AsyncLocalStorage) and the
sentinel checks isMcpWrite() per call. A byte-shape heuristic would
have falsely rejected Content-Length frames (start with C, end with })
and misclassified multi-chunk writes.
- gitnexus/src/mcp/stdio-context.ts: AsyncLocalStorage helpers + factory
- gitnexus/src/mcp/server.ts: install sentinel in safeStdout Proxy,
flush summary at process exit
- gitnexus/src/mcp/compatible-stdio-transport.ts: wrap send() write in
withMcpWrite so transport frames pass through cleanly
- gitnexus/test/unit/mcp-stdout-sentinel.test.ts: 17 cases covering
pass-through, redirect, prefix, truncation (default 200 / custom),
rate limit (default 10), one-shot warning, summary, mixed sequences
* feat(eslint): forbid console.log/warn and process.stdout.write in MCP-reachable code
Add a narrow ESLint override for gitnexus/src/mcp/**, gitnexus/src/core/lbug/**,
gitnexus/src/core/embeddings/**, and gitnexus/src/cli/mcp.ts that:
- sets no-console: ['error', { allow: ['error'] }] — only console.error
survives, since stderr is the only spec-safe channel for diagnostics
while the MCP stdio transport owns stdout for JSON-RPC frames
- adds no-restricted-syntax matching MemberExpression and CallExpression
forms of process.stdout.write to close the bypass path that the
AsyncLocalStorage sentinel cannot guarantee
Migrates 18 pre-existing console.log/warn call sites in core/embeddings/
(embedder.ts, embedding-pipeline.ts) to console.error; these are reached
from gitnexus_query semantic search and would have polluted MCP stdio
once a query triggered the embedding pipeline.
Adds eslint-disable-next-line comments in pool-adapter.ts at the four
legitimate process.stdout.write sites — they ARE the captured-real-write
infrastructure used by the sentinel and the silenceStdout/restoreStdout
mechanism.
The override is forward-compatible with feat/pino-logger (PR #1336)
which adds a broader no-console rule for gitnexus/src/; the narrow rule
here is a strict subset and rebases trivially when #1336 lands.
* feat(setup): pin setup-generated MCP config to installed version, keep static configs on @latest
The user-facing MCP config that 'gitnexus setup' writes into editor configs
now references gitnexus@<installed-version> instead of gitnexus@latest, read
dynamically from gitnexus/package.json#version at module load. This skips
the npm-registry metadata roundtrip on every MCP connect and stays
reproducible until the user explicitly upgrades.
Static example configs and quickstart docs intentionally keep @latest:
- .mcp.json, gitnexus-claude-plugin/.mcp.json
- gitnexus-claude-plugin/skills/*/mcp.json (6 files)
- README.md / gitnexus/README.md MCP examples
Pinning these would create per-release version-bump churn for marginal
(~100-500ms) savings. The dominant cold-cache cost is the native rebuild
addressed separately by the GITNEXUS_SKIP_OPTIONAL_GRAMMARS env var.
README adds a one-line steer above the @latest quickstart pointing
repeated users at 'gitnexus setup' for the absolute-path config that
bypasses npx entirely.
Tests refactored to assert against the dynamic version (createRequire of
package.json) so they don't break on every release bump:
- gitnexus/test/unit/setup.test.ts
- gitnexus/test/unit/setup-jsonc.test.ts
- gitnexus/test/unit/setup-codex.test.ts
- gitnexus/test/integration/setup-skills.test.ts (regex match)
* feat(install,mcp): GITNEXUS_SKIP_OPTIONAL_GRAMMARS opt-out + missing-grammar warnings
Postinstall scripts (build-tree-sitter-dart.cjs, build-tree-sitter-proto.cjs)
gain a strict 'process.env.GITNEXUS_SKIP_OPTIONAL_GRAMMARS === "1"'
early-exit so users without a C++ toolchain (or anyone wanting fast
'npm install gitnexus') can skip the native rebuild. Strict '=1' only —
'true', 'yes', '0' and any other value fall through to the rebuild.
Add gitnexus/src/cli/optional-grammars.ts: cheap require.resolve probe for
each optional grammar, with a stderr warning helper. The warning surfaces:
- At MCP server start (cli/mcp.ts) — unconditional, since the server
serves any indexed repo and we cannot pre-filter by language.
- At 'gitnexus analyze' start (cli/analyze.ts) — conditional on the
target repo containing .dart/.proto files (cheap glob), so users with
no relevant code don't see noise.
README documents the env var with the strict '=1' value and the trade-off
(faster install, no Dart/Proto parsing until reinstalled).
* test(mcp): child-process integration test asserts end-to-end stdout discipline
Spawns 'node dist/cli/index.js mcp' as a child, drives the MCP stdio
handshake (initialize -> initialized -> tools/list), reassembles every
stdout chunk into Content-Length-framed JSON-RPC messages, and asserts
zero stray bytes. Any byte outside a valid header-then-body window is
captured and surfaced in the failure message alongside the server's
stderr — this is the regression gate for U1 (no console.log/warn in
MCP-reachable code) and U3 (AsyncLocalStorage stdout sentinel).
Time budget: 5s local / 15s CI for first frame; 10s/30s total. Asserts
the published GitNexus tool surface (list_repos, query, context, impact,
detect_changes, rename) is reported by tools/list.
Adds 'pretest:integration': 'node scripts/build.js' so 'npm run
test:integration' rebuilds dist before the spawn — closes the
'stale dist masks regression' DX gap.
* fix(mcp): address PR #1383 review — sentinel scope, grammar detection, lint, contract
Blockers:
- B2: detectMissingOptionalGrammars now actually require()s each grammar
instead of require.resolve(). For 'file:' optional dependencies the
package directory is always installed regardless of postinstall outcome,
so resolve() never threw and the missing-grammar warning never fired
for the exact target users (those who set GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1
or whose native rebuild soft-failed). require() loads the entry, which
triggers node-gyp-build and throws if .node is absent. Result memoized.
Should-fix:
- S1: Removed duplicate uncaughtException/unhandledRejection handlers from
cli/mcp.ts. server.ts:startMCPServer already registers handlers with
full stack traces; cli/mcp.ts handlers fired first with worse output and
never got a chance to exit because server.ts shuts down immediately.
- S2: Sentinel is now actually global. New setActiveStdoutWrite() in
pool-adapter so silenceStdout/restoreStdout cycles preserve a
registered wrapper instead of unwinding to raw realStdoutWrite. At
startMCPServer: install sentinel.write as process.stdout.write AND
register it as the active handler. Direct process.stdout.write calls
from anywhere (console.log, dependency banners, etc.) now route through
the sentinel instead of bypassing it. The transport's _safeStdout Proxy
remains as belt-and-suspenders.
- S3: ESLint no-restricted-syntax now also forbids destructuring of
process.stdout (covers both 'const { write } = process.stdout' shapes
and rest patterns).
Minor:
- M1: chunkToBuffer now handles plain Uint8Array (Buffer.from(u8)) instead
of falling through to String(chunk) which produced '1,2,3,...' garbage.
- M2: Untagged-write callbacks are now invoked on next tick per the
Node Writable.write contract — both within and beyond the rate-limit cap.
extractCallback handles the (chunk, cb) and (chunk, encoding, cb) overloads.
- M3: setup.ts throws early if package.json#version is missing/non-string
instead of emitting 'gitnexus@undefined'.
- M4: parser-loader.ts console.warn → console.error; ESLint scope extended
to gitnexus/src/core/tree-sitter/** so future violations are caught.
New tests cover:
- Plain Uint8Array redirect (asserts no String(chunk) garbage).
- Writable callback fired async (next-tick) for both normal and
past-rate-limit redirects.
Validation: cd gitnexus && npx tsc --noEmit clean; vitest run 7863 passed,
11 skipped; eslint clean on MCP-reachable scope; integration test green
against rebuilt dist/.
* fix(mcp): close pre-sentinel stdout window + tighten contracts
Address ce-code-review findings on PR #1383:
P1 — Sentinel install order (was: stdout corruption window during
mcpCommand pre-startup):
- Add idempotent installGlobalStdoutSentinel() to mcp/stdio-context.ts.
It captures realStdoutWrite/realStderrWrite, replaces process.stdout.write,
and registers with pool-adapter's setActiveStdoutWrite — exactly once.
- cli/mcp.ts now installs the sentinel as the FIRST line of mcpCommand,
before warnMissingOptionalGrammars (which after the B2 fix actually
require()s each native grammar binding and could emit node-gyp-build
banners to raw stdout in the pre-sentinel window).
- mcp/server.ts startMCPServer keeps a safety-net call to the same helper;
the second invocation is a no-op.
P1 — WriteFn type erasure:
- WriteFn now declared as instead of
, so the assignment
and the
setActiveStdoutWrite(sentinel.write) call don't silently cross a
type boundary.
P1 — extractCallback fragility:
- Replaced backward-scan-with-undefined-break heuristic with a strict
'last arg if function' check matching the documented Writable.write
contract. No longer breaks on a future (chunk, options, cb) overload.
P2 — _detectionCache premature memoization:
- Removed the explicit cache. Node's module cache already memoizes
require() — calling detectMissingOptionalGrammars multiple times is
cheap. Removing the module-level mutable state makes the helper
trivially testable (no need for a reset hatch).
P2 — Misleading 'reinstall' message on broken (not missing) grammars:
- detectMissingOptionalGrammars now distinguishes MODULE_NOT_FOUND /
node-gyp-build 'no native build' patterns from other errors
(SyntaxError, EACCES, native crash). Broken bindings get an
actionable stderr line naming the real failure instead of the
misleading 'reinstall to enable' hint.
Other:
- mcp/core/lbug-adapter.ts updated with a KEEP-THIS-FILE note. Tests
use the path as a vi.mock seam (calltool-dispatch.test.ts and 7
others); new non-test code may import core/lbug/pool-adapter.js
directly. The maintainability finding flagging the shim as
self-contradictory was incorrect — the shim has a real test purpose.
Validation: tsc clean, vitest 7863 passed (no regressions), eslint
clean on MCP-reachable scope, integration test green against rebuilt
dist/.
* fix(mcp): close import-time stdout corruption window
Codex's adversarial review on PR #1383 found that even though cli/mcp.ts
is loaded lazily by Commander, ITS static imports (startMCPServer,
LocalBackend, installGlobalStdoutSentinel, warnMissingOptionalGrammars)
evaluate synchronously when the module loads — well before mcpCommand's
function body runs. Three of those four imports transitively pulled in
core/lbug/pool-adapter.ts, which imports @ladybugdb/core at module top
level. The native binding's init can write to raw stdout in that
pre-sentinel window and corrupt the JSON-RPC frame stream.
Fix: shrink cli/mcp.ts's static-import closure to a single zero-dep
chain (mcp/stdio-context.js -> mcp/stdio-capture.js, both leaf-clean),
install the sentinel as the first executable statement of mcpCommand,
then dynamically import the heavy backend modules in parallel via
await Promise.all.
Per the plan at docs/plans/2026-05-06-002-fix-import-time-stdout-window-plan.md:
- U1: New leaf module gitnexus/src/mcp/stdio-capture.ts owns the
stdout-capture singleton state (realStdoutWrite, realStderrWrite,
activeStdoutWrite + setActiveStdoutWrite/getActiveStdoutWrite).
Zero non-node: imports — adding any would re-introduce the hazard.
- U2: pool-adapter.ts re-exports the relocated symbols under the
existing names so the test mock seam (8+ files use vi.mock on
mcp/core/lbug-adapter.ts which re-exports * from pool-adapter)
keeps working without churn. restoreStdout and the watchdog now
read the active handler via getActiveStdoutWrite(). stdio-context.ts
imports from stdio-capture directly.
- U3: cli/mcp.ts's static imports collapse to one
(installGlobalStdoutSentinel). startMCPServer / LocalBackend /
warnMissingOptionalGrammars become parallel await import()
inside mcpCommand, after the sentinel install.
- U4: New regression test gitnexus/test/integration/mcp/import-closure.test.ts
spawns a child Node process that imports dist/cli/mcp.js (without
invoking mcpCommand), inspects the CJS module cache via createRequire,
and asserts @ladybugdb/core (and tree-sitter native bindings) are
NOT in the static-import closure. Characterization-first: this test
was authored to fail against the pre-fix code and confirmed to do so
before U1-U3 landed.
Validation: tsc clean; vitest 7865 passed / 11 skipped (2 new U4 cases);
eslint clean on MCP-reachable scope; integration server-startup test
green against rebuilt dist/.
* fix(mcp): drop dead ESLint selector + suppress redundant grammar warning
Two minor PR #1383 review findings:
1. eslint.config.mjs: removed Selector 3 (`Property[key.name='write'].properties:has(...)`).
`.properties` is not a valid attribute on a Property node in the ESTree
AST, so the :has clause never matched — dead code. Selector 4 covers
the canonical `const { write } = process.stdout` shape; tightened its
comment to make that explicit.
2. cli/mcp.ts: removed the unconditional warnMissingOptionalGrammars call
at MCP startup. The analyze path already emits this warning at index
time with relevantExtensions filtered to the repo's actual file types,
and a repo can only be served by MCP after analyze has run. Repeating
the warning unconditionally on every MCP session was pure noise on
machines whose indexed repos don't use .dart/.proto.
* chore(mcp): address PR #1383 review nits
Three minor hygiene findings from the production-readiness review:
- cli/mcp.ts: rewrite stale comment that described
warnMissingOptionalGrammars as living inside mcpCommand. The call was
removed in
|
||
|
azizur100389
|
b486d04d75
|
refactor(lbug): extract safeClose helper to consolidate WAL flush (#1377) | ||
|
Gergő Magyar
|
6ec1f04604
|
chore(quality): exclude test/fixtures from CodeQL, ESLint, and Prettier (#1313)
Test fixtures are intentionally synthetic inputs (broken/unused code, malformed samples) used to exercise the analyzer. Quality-tool findings on them are noise, not real bugs — they were drowning out actionable signal in the GitHub Security tab. - CodeQL: add `**/test/fixtures/**` to paths-ignore in codeql.yml - ESLint: add `gitnexus-web/test/fixtures/**` to global ignores (the gitnexus/ counterpart was already ignored) - Prettier: add `gitnexus-web/test/fixtures/` to .prettierignore (same gap as ESLint) Real test files (*.test.ts) remain in scope so genuine issues like js/file-system-race and js/insecure-temporary-file in test code still surface. |
||
|
Gergő Magyar
|
acf6fbdd39
|
feat: configure eslint with unused import removal (#564)
* feat: configure eslint with unused import removal Add ESLint v9 (flat config) for code quality: - eslint-plugin-unused-imports for auto-removing dead imports - @typescript-eslint for TypeScript-aware linting - eslint-plugin-react-hooks for React hooks rules - eslint-config-prettier to avoid formatting conflicts - lint-staged runs eslint --fix before prettier on .ts/.tsx - CI lint job added to ci-quality.yml * refactor: remove unused imports via eslint --fix Auto-fixed by eslint-plugin-unused-imports. No logic changes. * chore: add eslint fix commit to .git-blame-ignore-revs |