mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
205 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
dependabot[bot]
|
31c9d9223e
|
chore(deps): bump the codeql-action group with 3 updates (#3056)
Bumps the codeql-action group with 3 updates: [github/codeql-action/init](https://github.com/github/codeql-action), [github/codeql-action/analyze](https://github.com/github/codeql-action) and [github/codeql-action/upload-sarif](https://github.com/github/codeql-action). Updates `github/codeql-action/init` from 4.37.6 to 4.37.7 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits]( |
||
|
dependabot[bot]
|
414687ad10
|
chore(deps): bump docker/setup-buildx-action from 4.2.0 to 4.3.0 (#3057)
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 4.2.0 to 4.3.0.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](
|
||
|
dependabot[bot]
|
c056d136ad
|
chore(deps): bump actions/attest-build-provenance from 4.1.1 to 4.2.2 (#3005)
Bumps [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance) from 4.1.1 to 4.2.2.
- [Release notes](https://github.com/actions/attest-build-provenance/releases)
- [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md)
- [Commits](
|
||
|
azizur100389
|
b77d6f662b
|
fix(kotlin): resolve imports from declared packages (#2990)
Some checks failed
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / Classify release event (push) Waiting to run
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
Skill copy sync / shipped skills drift guard (push) Has been cancelled
|
||
|
azizur100389
|
7f0ab16ffe
|
feat(routes): support JS data route tables (#2972)
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
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / Classify release event (push) Waiting to run
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
|
||
|
dependabot[bot]
|
02008e0288
|
chore(deps): bump the codeql-action group with 3 updates (#2947)
Bumps the codeql-action group with 3 updates: [github/codeql-action/init](https://github.com/github/codeql-action), [github/codeql-action/analyze](https://github.com/github/codeql-action) and [github/codeql-action/upload-sarif](https://github.com/github/codeql-action). Updates `github/codeql-action/init` from 4.37.3 to 4.37.6 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits]( |
||
|
dependabot[bot]
|
8c2452a4e8
|
chore(deps): bump dorny/paths-filter from 4.0.2 to 4.0.3 (#2948)
Bumps [dorny/paths-filter](https://github.com/dorny/paths-filter) from 4.0.2 to 4.0.3.
- [Release notes](https://github.com/dorny/paths-filter/releases)
- [Changelog](https://github.com/dorny/paths-filter/blob/master/CHANGELOG.md)
- [Commits](
|
||
|
Gergő Magyar
|
18bc51dfd2
|
perf(import-resolvers): index every scanning resolver, consolidate the memo, gate every registered language (#2911)
* perf(import-resolvers): build buildSuffixIndex's dirMap lazily (#2903) `buildSuffixIndex` eagerly built three maps. `dirMap` is the array-valued one — one entry per directory suffix per file, so O(files x depth) in entries and array churn — and only four call sites ever read it, all via `getFilesInDir`: `import-resolvers/{php,csharp,jvm}.ts` and `import-resolvers/configs/python.ts`. Ruby (through workspace-file-index), the TypeScript scope resolver, Vue's import-target and the include-extractor never ask a directory question, and built it anyway. Since #2880 these indexes are retained for a whole resolution pass rather than rebuilt per import, so that waste is now resident memory. Deferring it to the first `getFilesInDir` call is behaviour-identical — same key, same descending-suffix order, same per-bucket push order, same `substring(lastIndexOf('.'))` extension clamp. The builder assigns the MAP on completion, so a repeated miss cannot rebuild it. Measured on `buildSuffixIndex` alone, 32k paths, index built and `getFilesInDir` never called: C# layout, 13 segments 79,018,680 -> 66,580,488 B -15.74% Ruby layout, 11 segments 60,752,792 -> 48,656,856 B -19.91% and on the whole retained WorkspaceFileIndex the bench measures: csharp 32k 73.62 -> 61.76 MiB ruby 32k 55.26 -> 43.69 MiB When `getFilesInDir` IS called the footprint is unchanged, so the deferral is never a loss. No new retention: all five construction sites already hold both input arrays alive beside the index. The laziness is pinned structurally rather than by timing. The test's corpus is a `string[]` whose elements are accessor properties, so an indexed read is observable and the read count IS the pass count: 14 after construction, still 14 after any number of get/getInsensitive, 28 after the first `getFilesInDir`, 28 after five more. Memoizing the decision instead of the map would read 42. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * perf(php): resolve imports from a per-run index, not a scan per import (#2901) PHP was the last language whose import resolution scanned the workspace per import. Both `resolvePhpImportTarget` and `resolvePhpImportTargetInternal` materialized two full arrays from the Set on every call, then passed `undefined` as the `index` argument — so `resolvePhpImportInternal` fell through to `suffixResolve`'s linear `findIndex`, once per extension per path part. Measured at 20,000 files: 96.40 ms per import. **Handing it the shared SuffixIndex would have moved IMPORTS edges.** All three index-fed sites answer a different question than the scan they short-circuit, each found by differential with a concrete witness: 1. `getInsensitive` — the scan leg is `allFiles.has(path)`, exact whole-path with no case-insensitive counterpart; the shared index answers a ci SUFFIX probe. 2. `getFilesInDir` — the scan is root-anchored `startsWith(nsDir + '/')`; `dirMap` is keyed on every directory SUFFIX, so a vendor copy can win. 3. `suffixResolve` — the scan's `endsWith('/' + S)` matches only a PROPER suffix; `buildSuffixIndex` indexes j=0, so a root-level `Foo.php` starts resolving `use Foo` where it returned null. 3b. the scan's `endsWith(p) || lower.endsWith(lower(p))` has a second disjunct that subsumes the first, so it is purely first-in-Set-order and case-insensitive; `get(S) || getInsensitive(S)` lets a case-exact hit anywhere beat an earlier ci hit. So this is not Ruby's #2880 shape. Both sites take `getWorkspaceFileIndex` for the memoized arrays and hand the internal resolver a PARITY `SuffixIndex` memoized on the same Set identity: `getInsensitive` disabled, `get` implementing the scan's real rule via the shared ci lookup plus one O(files) whole-path correction map, `getFilesInDir` root-anchored in Set order. no composer.json 96.40 -> 0.036 ms/import steady state with composer.json 100.19 -> 0.068 ms/import steady state Also closes PHP's last per-import traversal, in `import-resolvers/php.ts`: its namespace-directory scan ran whenever `getFilesInDir` came back EMPTY, not merely when no index was supplied — despite the comment above it claiming "only when SuffixIndex unavailable". An empty bucket is already the answer, so the scan could only confirm it, at one full pass per import whose namespace matches a PSR-4 prefix but whose directory has no direct `.php` child (measured 11 traversals for 10 imports; now 1). Moving it into the `else` is safe because the bucket is a SUPERSET of what the scan finds — a root-anchored direct child `nsDir/<x>.php` has its directory exactly equal to `nsDir`, and a directory is always one of its own suffixes, so both index shapes contain it. Nine mutations of the new code are caught, including M1 "pass the raw shared index" (the naive fix) at 23 arms. The adapter guard reads 600 instead of 1 under a defensive `new Set(allFilePaths)` — the #1918 P1 hazard the unit differential is structurally blind to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * perf(java): index import resolution instead of scanning per import (#2908) Java scanned the whole workspace twice per import: once for the three-tier direct match, and again INSIDE the progressive prefix-stripping loop — so a single unresolvable import cost one full pass per stripped segment. No WeakMap, no index, and it is registered in `SCOPE_RESOLVERS`, so it ran in production. This is byte-for-byte the C# shape #2878 fixed, so Java now reads the same machinery: `getWorkspaceFileIndex` for `normToRaw` + the segment-suffix index, and a Java-owned `PackageDirIndex` WeakMap over `buildPackageDirIndex(_, n => n.endsWith('.java'))` read through `firstFileDirectlyInPkgDir`. Structure mirrors C#'s `narrowContext` / `resolveDirectMatch` / `resolveByProgressiveStripping`. 20k files, 256 imports, 7-in-8 unresolvable: 8.05 -> 0.62 ms/import steady state once the index is built: 0.0036 ms/import Tie-breaks preserved, and Java's are NOT identical to C#'s: - tier 1 `break`s on the exact match, so an exact whole-path hit wins even when a suffix or directory-child hit came earlier in iteration order — hence `normToRaw.get` before `index.get`, which conflates them; - the stripping loop instead returns at the FIRST hit of `f === tailFile || f.endsWith('/' + tailFile)` and only yields its directory child after the scan completes, so the conflated `index.get` is the correct lookup THERE. Applying tier 1's exact-wins rule inside the loop is a real behaviour change (mutation M6); - `.*` wildcard stripping stays ahead of everything; - `firstFileDirectlyInPkgDir` reproduces Java's at-root/at-nested predicate exactly, including the first-`indexOf` rule — proved algebraically rather than assumed: the `atRoot` branch matches iff `dir === pathLike`, which is `D.indexOf(P) === 0 === D.length - P.length`, and the `atNested` branch's first occurrence in `f` is the first occurrence in `D` shifted by one. Six mutations are caught; a seventh (swapping the two index builds) is a true equivalence and is recorded as such. Hand-derivation also corrected four cases where the legacy code resolves and I had predicted null — including `java.util.List` reaching a local `util/List.java`, because Java has no in-repo-namespace gate like C#'s #1881. That is preserved here and filed separately as #2910; the parity test pins it so the fix is visible. The adapter guard reads 800 instead of 2 under a defensive `new Set(allFilePaths)`. Two traversals is correct: the workspace index and the package-dir index are separate WeakMaps and each iterates the Set once, the same accounting as C#. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * perf(cobol): index COPY resolution instead of two scans per statement (#2908) `cobolScopeResolver.resolveImportTarget` ran two full workspace scans per `COPY`, each calling `path.extname` + `path.basename` + `.toUpperCase()` on every entry: tier 1 over `.cpy`/`.copybook`, tier 2 over `.cbl`/`.cob`/ `.cobol`. No WeakMap, no index, and registered in `SCOPE_RESOLVERS`. Two uppercased-basename maps, one per tier, filled in a SINGLE pass over the Set and memoized on Set identity. Lookup is `copybooks.get(upper) ?? sources.get(upper) ?? null`. 20k files, 500 COPY operands: 3879-4082 -> 10.5-11.7 us/import (~350-369x) steady state once built: 0.253 us/import Tie-breaks preserved: - TIER ORDER. A `.cpy` match beats a `.cbl` match even when the source file appears EARLIER in Set-iteration order. This is the one a naive single-map rewrite silently breaks, so it gets its own fixture. - Within a tier, first in Set-iteration order wins (`if (!tier.has(...))`, mirroring the scans' first-match return). - The key is built with the identical call sequence, `basename(fp, extname(fp).toLowerCase()).toUpperCase()`, so `Foo.CPY` still keys under `FOO.CPY` rather than `FOO`. - `path` stays in the loop rather than hand-rolled `/`-slicing, so backslash handling is unchanged on every platform — pinned by a `dir\sub\BOOK.cpy` case. All six mutations are caught: collapsing the tiers, within-tier last-wins, dropping the target uppercase, dropping the extension lowercase, hand-rolled slicing, and the adapter's defensive copy. The first five are caught by the differential and are invisible to the adapter guard; the sixth is the reverse, which is the layering working as intended — the guard reads 600 instead of 1. `COBOL_SOURCE_EXTENSIONS` was being re-allocated on every call; hoisted to module scope beside `COPYBOOK_EXTENSIONS`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * perf(csharp): index the csproj leg's namespace-directory scan (#2902) #2878 moved C#'s no-csproj leg onto memoized indexes; the csproj leg kept a per-import full scan in `resolveCSharpImportInternal` step 3, measured at ~1.10 ms per import at 50,000 `.cs` files. **The fix the issue proposed would have moved edges.** It suggested skipping the fallback when an exhaustive index is available, on the assumption that step 2's `getFilesInDir` answers the same question. It does not: step 2's `dirMap` is keyed on segment-aligned directory suffixes, while step 3's `normalized.indexOf(dirPrefix + '/')` is an UNANCHORED substring match, so step 3 finds a strict superset — and it runs only when step 2 came back empty, so those extra hits are observable, not shadowed: dirPrefix 'ubModels' step 2 [] step 3 ['src/SubModels/Widget.cs'] dirPrefix 'rc/Models' step 2 [] step 3 src/Models/* AND vendor/mysrc/Models/* So the predicate is kept byte-for-byte and made fast instead. It depends only on the file's directory (the needle ends with `/`, so every occurrence lies wholly inside `D + '/'`), which reduces to the `package-dir-index` formula minus the anchoring leading slash. `PackageDirIndex` itself cannot be reused for the same reason — its matcher is anchored. The index is memoized on the `normalizedFileList` array identity and built lazily at the point step 3 is first reached, so BCL usings — which `continue` out at the root-namespace gate — never pay for it. Candidates come from an exact last-segment bucket when `dirPrefix` contains a slash, a last-segment key sweep when it does not, and `singleSegmentDirs` when it is empty. Positions rather than paths, merged and sorted when several directories match, so file-list order survives. App.Missing @ {App, src} 1103.0 -> 7.6 us (145x, and flat in file count: 7.3 @10k, 7.6 @50k, 8.4 @200k) App.Missing @ {App, ''} 626.7 -> 108.5 us App @ {App, ''} 1077.9 -> 2.0 us (539x) App.Ns8 @ {App, src} 0.6 -> 0.6 us (step-2 hit, untouched) `relative === ''` is preserved exactly, including the no-`projectDir` case where the needle is a bare `/` and the answer is "every `.cs` whose directory has no slash of its own" — `getFilesInDir('', '.cs')` cannot answer that over repo-relative paths, so it has its own arm. 13 of 14 mutations are caught, including M1, the naive skip-when-indexed cleanup, at 9 arms. The survivor drops the empty-prefix fast path and is a true equivalence. M9 initially survived and exposed a real corpus gap — no non-`.cs` file lived inside a directory — now covered. The remaining non-constant term is the slash-free sweep, O(distinct last segments): 456 us at 200k files on a unique-name layout, but 7.9 us on a `SrcN/Models` layout, which is how C# repos are actually laid out. Closing the unique-name case needs a character-suffix map over segments — the O(files x depth) memory shape `package-dir-index.ts` cites #2649 to avoid — so it is documented in the code as a design change rather than tuned here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * test(scope-resolution): assert index reuse for every registered language (#2909) Index reuse was asserted by nine hand-written per-language files, so the guarantee existed exactly for the languages someone remembered — and #2908 is the proof that is not good enough: Java and COBOL were registered, quadratic and unguarded until this branch. `resolveImportTarget` is a required member of `ScopeResolver` with one signature and 16 registrations, so "calling it N times against a stable `allFilePaths` must not traverse the set N times" is a property of the CONTRACT. `import-target-index-reuse.contract.test.ts` drives every entry of `SCOPE_RESOLVERS`, modelled on `construction-syntax-wiring.test.ts` — the established shape here for a property plus a justified inventory. Measured counts, all memoized: c 1 cobol 1 cpp 1 csharp 2 dart 1 go 1 java 2 javascript 2 kotlin 1 php 1 python 1 ruby 1 rust 0 swift 1 typescript 2 vue 2 **`KNOWN_UNINDEXED` is empty.** The audit that produced it also cleared C, C++, Rust, Swift, TypeScript, Vue and JavaScript by hand — Rust's memo lives in `qualified-call.ts::moduleIndexFor`, C's and Swift's loops are inside their WeakMap builders. The empty map stays as a mechanism: a 17th language cannot opt out silently, and the inventory arm fails when a registered resolver has no fixture. Two things the assertion had to get right: - it is `scans(200) === scans(2)`, not `scans === 1`. Per-language counts legitimately differ (C# and Java build two indexes), and comparing two counts needs no per-language expected value. - Rust legitimately scans ZERO times — it answers every leg with `allFilePaths.has(candidate)` probes — so the floor is a per-language `minimumScans`, 1 for fifteen languages and 0 for Rust with the reason on the interface. Paired with a `hitTarget` that must resolve non-null, so the property cannot pass vacuously on a resolver that stopped answering. Miss targets are distinct per import, which defeats the TS/JS/Vue per-target `resolveCache`. Also unifies the instrument. Kotlin and Python counted index BUILDS from production; the other seven count traversals of a `CountingSet`. The build counter is strictly weaker — a scan added BESIDE a reused index moves no build count, which is exactly the mutation `baselines.json` `_blind_spot` records as invisible to every timing arm — and it costs two production modules that ship in the bundle purely for tests, holding module-global state every test must `reset()`. Both guards migrate to `CountingSet`, and `languages/{kotlin,python}/index-stats.ts` plus both call sites are gone, for -59 lines of shipped source. (Mechanical note: the two `index-stats.ts` file deletions appear in the #2901 commit rather than this one. They were staged with `git rm` while a concurrent commit swept the index. The final tree is correct; only that attribution is off, and rewriting a sibling commit to move them was not worth the risk.) Coverage went up in the swap: Kotlin's old "rebuilds when the file set is a different object" arm (3 sets, 3 builds) would have PASSED under a defensive adapter copy. Its replacement fails, as do all six arms across the two files. Verified by mutation: `new Set(allFilePaths)` inserted into the kotlin, python and go adapters fails exactly those three and no others — `python: 200 imports cost 201 traversals, 2 cost 3`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * test(import-target): gate the four newly-indexed resolvers, retighten heap The bench covered go/csharp/dart/ruby/kotlin. The four resolvers indexed on this branch shipped unmeasured, and #2903's memory win was not locked in. **php, java and cobol join the shared corpus**, each with the two load-bearing properties the header requires: imports scale with file count, and most imports MISS so the full cascade runs (resolve rates php 36.0%, java 34.4%, cobol 36.0%). Java's miss families were measured rather than assumed, since it has no in-repo-namespace gate (#2910): `java.*` 1041 imports and `com.google.*` 1006, both resolving 0. COBOL's collide layout repeats a bookname across BOTH extension tiers, so it reaches the copybook-over-source tie-break rather than only the basename map. **`csharp_csproj` is a sixth LANGS entry**, not a new arm dimension — an entry needs five small additions and inherits all five arms and all seven gates, where a context axis would have to be threaded through `buildRepo`, `resolveAll`, `identityPass`, the report shape and every gate. `buildFiles` aliases it to `csharp`, so the two share one corpus by construction and cannot drift. Two configs (`{App, 'src'}`, `{Lib, ''}`) produce all three `dirPrefix` shapes — slashed, slash-free and empty — in five arms instead of ten: App.Ns{d} 30.6% src/Ns{d} step 2 hit App.Missing{n} 25.5% src/Missing{n} step 3, last-segment bucket Lib 14.0% (empty) step 3, singleSegmentDirs Lib.Missing{n} 12.0% Missing{n} step 3, KEY SWEEP — the one non-constant path BCL / Ghost 12.4% — root-namespace-gate control **2221 of 3200 imports reach the indexed leg**, only 12.4% `continue` out. What that arm pins is stated plainly rather than overclaimed: step 3 answers null for all 2221 here (the hits land at step 2), so it gates that leg's COST and its null answers; its positive tie-breaks stay pinned by the unit parity test. **Heap ceilings retightened.** #2903 dropped the measured figures, leaving the 1.5x ceilings at ~1.9x — a straight revert to the old size would have passed: csharp 116,000,000 -> 98,000,000 B (measured 61.76 MiB) ruby 87,000,000 -> 69,000,000 B (measured 43.69 MiB) php new 106,000,000 B (measured 67.29 MiB) java new 154,000,000 B (measured 97.32 MiB, the largest in the file — Maven layout is 18 segments) php and java are gated because both retained NOTHING across imports at BASE and now retain the O(files x depth) suffix index — the same argument that gates C#. cobol is not: two `Map<basename, path>`, O(files) with no depth term, and its retained delta does not clear measurement noise, so a ceiling would gate nothing. `csharp_csproj` is not: same corpus, same index, a duplicate number — its one distinguishing footprint, the lazily-built `dirMap` its `getFilesInDir` forces back, is measured at +20.8% and recorded as a residual instead, because gating it would licence eager-dirMap everywhere. csharp's `depth_ratio` also fell 3.318 -> 2.31 (the no-csproj leg never asks a directory question, so the deep arm stopped paying an eager dirMap build). Budget 5 -> 3.5, restoring the file's 1.5x convention — and `_arms_note` says plainly that 3.5 does NOT lock that win in, because locking it needs ~2.9, which is 1.25x over a 1.05x spread and the kind of tightening `_triage` warns buys flake rather than signal. All five pre-existing languages are byte-identical: 25 cells x 5 fields = 125 values, 0 mismatches. The new arms were proven live by a doctored baseline (cobol ceiling 0.01, php heap 1000 B, java resolved 999) producing three correctly-worded failures and exit 1. Wall-clock 10.9 -> 26.1 s, php and csharp_csproj ~11 s of it — both cascades end in `suffixResolve`'s ~50-extension probe, and both gate the two largest wins on this branch, so neither is a candidate to drop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * perf(javascript): build the suffix index JS resolution never had JavaScript's `PassCache` was TypeScript's minus one field: `index`. So JS called the shared `resolveTsTarget` with `ctx.index === undefined`, and `import-resolvers/standard.ts` fell through to `suffixResolve`'s linear `findIndex` — scanning the materialized path list once per extension (~39) per path part, per import. 2000 files 6448.9 -> 28.5 us/import (TypeScript: 25.0) 8000 files 25972.6 -> 27.4 us/import (TypeScript: 27.0) Per-import scaling over 4x the files: 4.12x -> 1.09x. **Every instrument on this branch was blind to it.** `CountingSet` counts traversals of the Set; this walked the array the adapter had already materialized — the blind spot `counting-file-set.ts` documents in its own header and `baselines.json` records under `_blind_spot`. Under mutation M1, which drops `index` and reproduces the shipped defect exactly, the sixteen- language contract test stays GREEN for javascript, because the pass cache is still reused and `files.scans` reads 2 either way. Two new arms do catch it: a `suffixResolve` linear-branch counter that runs the legacy adapter first as its control (135 entries legacy, 0 now), and a mock-free behavioural assertion that a repo-root module resolves by bare specifier. Adding an index moves output, exactly as it did for PHP in #2901, so it was characterized rather than assumed — 211,200 pairs (400 corpora x 3 importers x 176 targets) plus 184 hand cases. **Two classes move and there is no third:** A null -> repo-root file (108) `require('config')` with root `config.js`. The scan tests `endsWith('/' + suffix)`, so a path with no slash has no proper suffix and was unreachable through that leg — while `./config` from the root already resolved via the exact `Set.has` branch. JS was internally inconsistent. B file -> different file (5679) `import 'app/main'` was resolving to `node_modules/dep0/lib/main.js`; the scan skipped the whole-path candidate at the 2-segment suffix and fell through to the 1-segment `/main.js`, taking the first such file in Set order. C hit -> null ZERO, and impossible: proper-suffix keys are a subset of the index's keys. Both moved classes are JS being wrong. **JS-new agrees with TypeScript on all 211,200 pairs and every corpus case, 0 disagreements** — which is the intended design, since JS delegates to the TS resolver and differed only by this field. Also swaps the single-slot `let cached: PassCache | null` in JS, TS and Vue for a module-level `WeakMap`, matching every other language. Two alternating file sets rebuilt everything on every call: 12.0 -> 1438.2 ms at 4000 files x 400 imports (120x); after, 11.0 -> 15.7 ms. This is LATENT, not live — `pipeline/run.ts:673` builds one Set per provider pass and the three are separate providers — but it is why these were the only languages that could not carry the standard distinct-set guard. They can now: the arm fails on HEAD for all three (`expected 42 to be 2`) and passes after. Six mutations caught, including a global `resolveCache` (M5), which needed a new arm — `expectDistinctFileSetsGetOwnIndex` builds two IDENTICAL corpora, so a stale answer carried between them is also the right answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * refactor(ingestion): one per-file-set memo primitive, twenty-one call sites Every language that indexes its import resolution hand-rolled the same memo: declare a module-level `WeakMap` keyed on the file-set object, `get`, `if undefined` build and `set`, return. One concept, written twenty-one times, and this branch had just added five more. `import-resolvers/per-file-set.ts` exports it once: perFileSet<K extends object, T extends object>(build: (key: K) => T): (key: K) => T Two decisions, both recorded in the file. `T extends object` rather than `has`-then-`get`: `WeakMap.get` returning `undefined` cannot distinguish "not built" from "built as undefined", and the `has` form needs a cast or a non-null assertion, both banned here — the constraint makes the ambiguous case unrepresentable instead, and a future caller wanting `string | null` gets a compile error pointing at the decision. A throwing build stores nothing and runs again next call, so failures are not memoized and a half-filled index is never published — inert for these pure builders, and the safer direction. `K extends object` rather than `ReadonlySet<string>` is what lets C#'s `readonly string[]`-keyed cache share the helper. Twenty-one sites migrated across `import-resolvers/` and fifteen languages. Every existing doc comment was re-homed onto the new call rather than deleted — several record real invariants (the Set-identity contract, the #1918 pass-through rule, why Rust's memo lives on a different hook). TypeScript, JavaScript and Vue additionally had byte-identical `PassCache` interfaces and builders. `import-resolvers/pass-cache.ts` now holds the one builder, taking a single argument — every difference the three have lives in the CONSUMER (`tsconfigPaths`, the extension list), not the builder. The builder is shared, the memo deliberately is not: each adapter keeps its own `perFileSet`, hence its own index and its own `resolveCache`, because the three disagree about what a specifier resolves to and one shared cache would hand a language another language's answers. It buys no runtime reuse and the module says so — each provider pass builds its own `allFilePaths` Set, so the three are always different keys. C and C++'s `augmentedFilePaths` was a two-LEVEL memo, and needed no new abstraction: the outer memo's value is a function and a function is an object, so `perFileSet(perFileSet(...))` composes. The two instances stay one per file, and the reason is now in BOTH doc comments rather than only C++'s — cpp delegates to `resolveCImportTarget`, whose `suffixIndex` is keyed on the augmented set, so a shared memo would cross the two languages' indexes. Two sites are deliberately NOT migrated, each with the reason written at the declaration so the next sweep does not re-litigate them: - `configs/swift.ts` is a two-input memo keyed on one. `targets` is not derivable from the key; re-keying on `ctx` would force a banned non-null assertion or an unreachable fallback inside a memo builder. - `rust/qualified-call.ts` `MODULE_SCOPE_CACHE` is three inputs keyed on one, and sits ten lines below a `perFileSet` in the same file — the likeliest thing to be "fixed" by mistake. The other ten remaining `WeakMap`s are different concerns and stay: AST-node caches, worker-pool runtime state, graph metadata, mutable lazily-filled accumulators, and the C++ ADL / inline-namespace indexes, which are reassigned by explicit clear functions and epoch-stamped on read — validity rules beyond key identity that a closure over a private cache cannot express. Net −20 lines of code, +22 of the two "why not" notes. The primitive's own doc is where the cost sits: the Set-identity contract and the two design decisions are written once instead of being twenty-one implicit facts. Pure refactor: 1764 unit tests, 42 guard tests, all sixteen contract-test traversal counts unchanged (c 1, cobol 1, cpp 1, csharp 2, dart 1, go 1, java 2, javascript 2, kotlin 1, php 1, python 1, ruby 1, rust 0, swift 1, typescript 2, vue 2), 647 C/C++ tests, and every bench fingerprint unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B * test(import-target): gate every registered language, not nine of sixteen The bench pinned output fingerprints and scaling for 9 of the 16 languages in `SCOPE_RESOLVERS`. The other seven — c, cpp, javascript, python, rust, swift, typescript, vue — resolve imports in production with nothing pinning their output or their cost. JavaScript was the sharpest case: the 25,972 us/import defect fixed earlier on this branch was gated by unit tests alone. All 16 are now gated, plus the `csharp_csproj` variant: 17 entries. **The nine existing languages are byte-identical** — 234 committed values (9 x 5 arms x 5 fields, plus 9 top-level fingerprints), 0 changed, and no pre-existing budget touched. Measured both before and after the memo consolidation in |
||
|
Carter LaSalle
|
81100e2c74
|
fix(python): resolve calls through __init__.py re-exports (#2864)
* fix(python): resolve calls through `__init__.py` re-exports
A call to a name imported from a package never resolved when the package's
`__init__.py` re-exported it rather than defining it:
pkg/impl.py def target_fn(x): ...
pkg/__init__.py from pkg.impl import target_fn
caller.py from pkg import target_fn
def calls_it(): return target_fn(21) # no CALLS edge
`caller.py` gets no CALLS edge. Both IMPORTS hops are recorded, and all four
functions are extracted as nodes — only the call binding is missing. Because
`__init__.py` re-exports are how Python packages declare a public surface, this
misses a large fraction of real call edges, and the failure is silent: the
defining file looks like dead code with zero callers.
The re-export closure that should carry this already exists and is fully general
(`buildReexportClosures` — SCC over the re-export subgraph, bounded fixpoint for
cycles, transitive `via` chains). Python just never fed it: the subgraph admits
only `kind: 'reexport'` and `kind: 'wildcard'`, and Python emits neither for
`from m import x`.
Python has no dedicated re-export form. A module-level `from pkg.impl import X`
binds X locally AND publishes it as `pkg.X`, so it is both a named import and a
re-export. Emitting `kind: 'reexport'` would be wrong — that form drops the local
binding, which Python's does create. Instead add an optional `reexportsName` flag
to the `named`/`alias` variants, alongside the existing provider-specific
`importedSymbolKind` / `targetIncludesImportedName` flags, and admit flagged
imports into the closure subgraph. Languages with an explicit form keep emitting
`kind: 'reexport'` and leave the flag unset, so nothing changes for them — a
negative-control test asserts a plain named import still does not resolve.
Verified on a fixture covering the three shapes (direct, top-level-via-re-export,
function-local-via-re-export): 1 of 3 CALLS edges resolved before, 3 of 3 after.
On a 12.4k-file Python/Go/TypeScript repository: edges 294,416 -> 301,443
(+7,027) and execution flows 300 -> 813. A previously "100% orphaned" module
(`shared/db/event_writer.py`) now correctly reports its caller.
5 new finalize tests (single hop, 3-hop chain, alias keying, cycle termination,
and the negative control) plus 6 updated Python fixture shapes.
`npx tsc --noEmit` clean in both packages; full unit suite shows no regression
against baseline (remaining failures are pre-existing load-sensitive flakes in
analyzer-identity / evidence-provenance-helper / skip-git-cli / hooks, each
verified passing in isolation).
* fix(python): set reexportsName only for module-level imports
`interpretPythonImport` flagged every `from m import x` as republishing the
name, but only a module-level statement does. A `from m import X` inside a
`def` or `class` body binds locally and puts nothing in the module namespace,
so flagging it fabricates a re-export of a name no importer can reach:
# pkg/__init__.py
def loader():
from pkg.impl import InternalHelper
# caller.py
from pkg import InternalHelper # CPython: ImportError
resolved to `def:pkg.impl.InternalHelper`. Worse, with declaration-order
first-wins in the closure, a scope-blind entry could claim a name ahead of the
real module-level import and give a WRONG def for legal, running code.
`interpretImport` receives a `CaptureMatch`, which is `{name, range, text}`
with no syntax node, so the scope is not recoverable there — and it is not
recoverable downstream either: `pass3CollectImports` applies no scope filter
and `ImportEdgeDraft.fromScope` is hardcoded to the module scope. The decision
therefore moves up to `import-decomposer.ts`, which still holds the live
`import_from_statement` node, and rides down as an `@import.publishes` marker.
Computed once per statement, not once per imported name, with the existing
`findAncestorBeforeBoundary` helper.
Only `function_definition` and `class_definition` suppress publication.
`if` / `try` / `for` / `with` do NOT — Python has no block scope — so the
predicate is an ancestor walk for those two node types and nothing else.
Verified against CPython 3.11 in both directions; both are now pinned by
tests, including the counterpart control that a branch-nested import still
republishes.
Also corrects the docblock in `scope-extractor.ts` that sent this change the
wrong way. It claims pass 3 attaches imports "not to any `Scope` — finalize
reconstructs the owning scope via `provider.importOwningScope` during Phase
2". Finalize does no such thing: `importOwningScope` is declared on
`LanguageProvider` and implemented by a dozen providers, and
`grep -rnE "\.importOwningScope\b" gitnexus/src/` returns exactly one hit —
that doc comment. Nothing invokes it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rzsb6mdGtbu66BG1EaF6Zz
* fix(shared): stop guessing ambiguous and namespace re-exports; bound the via chain
Four changes to the re-export closure, all reachable only now that Python
feeds it.
1. AMBIGUOUS NAMES ARE DROPPED, NOT GUESSED. `populateFileClosure` documented
"declaration order first-wins for duplicates of the same exported name",
which is sound only where a duplicate export is illegal — two
`export { X } from …` is a TypeScript compile error, so the rule never
fires. Python has no such guarantee:
from .v1 import Client # legacy, left behind
from .v2 import Client # the actual public Client
CPython binds v2 (verified on 3.11); first-wins attributed every
`from pkg import Client` in the repo to the DEAD implementation, and
`impact("Client")` pointed at the wrong file. Last-wins is not the fix
either: for the equally common `try:`/`except ImportError:` and
`if sys.version_info` pairs exactly one branch runs, and which one is not
decidable here. Both directions are wrong on real code, so the entry is
dropped — the importer stays unresolved, which is exactly the pre-#2864
answer, and the file-level IMPORTS edge is untouched.
`collectAmbiguousReexports` runs as a PRE-PASS over data phase 0 froze,
so the poisoned set is constant across the fixpoint. That matters: a set
that grew mid-fixpoint would need retraction to propagate to files that
already inherited the name, would make `myClosure.size > before` an
unsound progress signal, and would invalidate the `|SCC| + 1` cap. As a
pre-pass the closure map stays monotone and every existing termination
argument survives unchanged. Only two flagged drafts resolving to two
DIFFERENT in-workspace files count; duplicates of one target are
harmless, and unresolvable targets never entered the closure.
Checked in both loops. Named re-exports take precedence over wildcards,
so suppressing only the named loop would hand the name to a later
`import *` and reinstate an arbitrary winner through the back door.
2. NAMESPACE-RECLASSIFIED DRAFTS ARE EXCLUDED. The admission guards tested
`draft.source.kind` while `tryFinalize` tests the post-reclassification
`draft.base.kind`. Python's `from . import logger` is emitted as `named`,
reclassified to `namespace` by `isNamespaceImport`, and was still
admitted — republishing whatever def shared the module's simple name. For
a `logger.py` holding a module-level `logger = logging.getLogger(...)`,
importers of `from pkg import logger` bound to that Variable instead of
the module. Reproduced end to end. Both predicates now take the draft and
test `base.kind`; this is a no-op for TS/Rust, whose only
`isNamespaceImport` implementation is Python's.
3. `transitiveVia` IS CAPPED AT 32. Each hop copies the inherited path, so
an unbounded chain is Theta(depth^2) in time AND retained memory, and
Theta(|SCC|^2) for a cycle whose chain tracks it. `MAX_REEXPORT_DEPTH =
100` covered this until
|
||
|
DuduPhudu
|
fa31a7d824
|
fix: close the nine follow-up review findings from #2856 (routes, receiver typing, truncation honesty) (#2899)
* fix(typescript): a type parameter shadows a declared type of the same name (W2-8)
First item of wave 2, promised to the reviewer on #2856.
`export function unwrap<Result>(value: Result): Result` names the PARAMETER, not
the `interface Result` beside it — tsc resolves both annotations to the
parameter. The type-reference capture that makes a contract answerable ("what
breaks if I remove this field?") had no notion of a parameter binding, so every
annotation mentioning `Result` inside `unwrap` minted a `USES` edge into the
interface, at the same confidence as a real consumer and indistinguishable from
one. Measured on the new fixture: `unwrap` produced TWO false edges while the
genuine consumer produced one.
Blast radius is every generic whose parameter name collides with a declared
type, and the colliding names are ordinary choices for both: `Result`, `Key`,
`Value`, `Item`, `Node`, `Options`, `Config`, `Props`, `State`, `Response`.
TWO HALVES, and the first is why upstream's fix could not reach this. #2833
introduced `bindsTypeParameter` for the CALL-receiver path, where a workspace
`class T` was answering for `<T>`. Reusing it here changed nothing at first, and
the reason is its own documented contract: `@declaration.type-parameters` was
captured for class/interface declarations ONLY, so a generic FUNCTION recorded
no parameter list and the predicate correctly returned false — absence is not
evidence. The data was missing, not the logic. So:
- TYPESCRIPT_SCOPE_QUERY now captures type parameters on `function_declaration`,
`generator_function_declaration` and `type_alias_declaration`;
- the graph bridge consults `bindsTypeParameter` before emitting `USES`.
Both are load-bearing — removing either one fails the fixture.
The fixture carries two controls, because the obvious wrong fix is to stop
emitting: a genuine consumer of the interface must still link, and a generic
whose parameter does NOT collide must still link its real reference. Both are
asserted, and the "genuine consumer" case is asserted FIRST so the absences
below it cannot pass vacuously.
SCHEMA_BUMP 53 -> 54: parse-time capture change. A warm cache replays defs with
no parameter list, so the guard reads nothing and the feature is inert while
looking implemented.
Capture fingerprint re-baselined with justification. NO NEW CAPTURE NAME —
diffing the capture-name sets against the wave-1 branch returns empty; the tag
existed and now fires on more declarations. capture_groups_fp 2338 -> 2371,
fixture_count 151 -> 152, scaling 1.06 < 1.5, and JavaScript's fingerprint does
not move at all, which is the check that this is the TS declaration rules rather
than something broader.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(analyze): close the four false-success paths in the graph-write-collapse guard (W2-6)
Second wave-2 item, promised on #2856. All four were reported; all four
reproduced by reading the code they name.
(a) A SAME-COMMIT RE-RUN REPORTED SUCCESS FOREVER. Every other meta-driven
trigger — schema fingerprint, PDG mode, runner identity, CJK segmentation,
embedding dims — has a block that forces a rebuild before the
`alreadyUpToDate` fast path. `graphWriteCollapsed` had none; `grep -rn` found
writes and no reads. So the one state meaning "most of your edges are gone"
was the one state that repaired itself only if the user happened to pass
`--force`. Now forces a full rebuild, and forcing is right rather than merely
re-running: the persisted graph disagrees with what the pipeline produced, so
an incremental pass over unchanged files would write nothing and re-stamp the
same broken index as fresh.
(b) AN INCREMENTAL RE-RUN ERASED THE STAMP. `saveMeta` is a full atomic
overwrite, and the field was spread in only when the CURRENT run had a
verdict. `undefined` meant two different things at that site — "full run, no
collapse" (a positive all-clear) and "incremental write, not comparable" (no
opinion) — so the second case silently dropped `graph-write-collapsed` from
meta.json while the edges were still missing. Now three-way: stamp on
detection, CLEAR on a healthy full run, CARRY FORWARD when there is no
verdict. That is the shape `branch: branchLabel ?? existingMeta?.branch` two
lines away had all along.
(c) THE SERVER PATH NEVER CONSUMED IT. `analyze-worker-ipc.ts` projects the field
"so a server-side caller sees the same degraded outcome the CLI does" — but
nothing read it, so the comment described an intention and every collapsed
run reported `complete` to the UI and to every API consumer. Now reports
`failed` with the counts and the remedy, matching the CLI, which prints
`Repository indexed INCOMPLETELY` and exits non-zero. A consumer that reads
"complete" will query the index and get confident wrong answers.
(d) --pdg ROWS MASKED TOTAL STRUCTURAL LOSS. `expected` counts the in-memory
graph plus the streamed STRUCTURAL manifest; the streamed PDG layers never
enter `graph.relationshipCount`. But `persisted` was `stats.edges`, a count
of EVERY `CodeRelation` row, and PDG writes into that same table. With 1,000
structural edges expected and 4,000 PDG rows persisted, losing every
structural edge still read `persisted = 4000`, cleared the ratio, and stayed
silent — on exactly the large repos `--pdg` is used for.
Worth recording that the OBVIOUS fix does not work. Padding `expected` with
the PDG rows makes the two universes match but leaves the ratio judging a
minority population: 4,000 of 5,000 still clears 0.5. I wrote that first, and
the test I wrote to prove it failed. Only comparing structural against
structural asks the question the check exists to ask, so `getLbugStats` gains
a `structuralEdges` count excluding `PDG_EDGE_TYPES`. `TAINT_PATH` is
deliberately NOT in that set — it is a whole-program Function→Function edge
persisted by the normal emit, so it is structural and stays counted on both
sides.
`index-freshness-graph-collapse.test.ts` had pinned the masking as correct
(`detectGraphWriteCollapse(1000, 4000)` → undefined, "PDG layers write into
the same table, so persisted > expected is normal"). True about the table,
and it licensed the hole. Replaced with the case that matters and a note on
why the fix is at the caller.
The new `structuralEdges` assertion in `lbug-core-adapter` is there because the
failure mode is silent: the query sits in a try/catch that yields `undefined`,
and `undefined` makes the collapse check decline to compare — so a typo in the
Cypher would throw nothing, fail nothing, and switch the guard off. Verified
against a real LadybugDB and mutation-checked by breaking the query.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(processes): make process selection insertion-order invariant (W2-5)
Third wave-2 item. Reproduced before fixing: two equal three-step flows with
`maxProcesses: 1` select `handleAlpha`; inserting the identical nodes and CALLS
edges in reverse select `handleBeta`. Same repository, same commit, a different
persisted graph — so a filesystem that enumerates differently, or an incremental
run that reorders assembly, silently changes what the tool reports.
Four sorts ranked by score or length alone and returned 0 on a tie.
`Array.prototype.sort` is stable, so a 0 preserves INPUT order, which traces
back to `graph.iterNodes()`. Under `maxProcesses` capping that decided which
`Process` and `STEP_IN_PROCESS` nodes were persisted at all. Each now falls
through to a totally-ordered, content-derived key — node id for entry points,
the joined path for traces.
WHAT IS ACTUALLY VERIFIED, stated precisely because "four fixes" would overclaim:
- the ENTRY-POINT sort is individually mutation-verified;
- the two DEDUP sorts are collectively mutation-verified;
- the TRACE-RANK tiebreak is NOT individually observable, and the source says
so. The dedup sorts already impose a total order on the list that reaches
it, so removing it alone fails nothing. Kept as defence in depth: it cannot
misbehave — it only makes an already-deterministic order explicit — and it
is what stops a change to dedup ordering from silently re-opening this.
Finding that out took two fixtures. The first (three chains, three entry points)
is separated by the entry-point sort before trace ranking is reached, so it never
exercises the trace comparator at all; the second gives ONE entry point two
equal-length branches to different terminals, which is the only shape where the
trace comparator decides. Both are kept — they gate different sites.
The invariance tests assert the INVARIANT rather than any single sort, so they
cover all four sites and any future one without needing to know where they are.
Three assertions: same selection under a cap, identical set uncapped, and
identical ORDER — the last because order is what the cap consumes, so a set-only
assertion would pass while the defect persisted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(impact): UNKNOWN dominates a mixed candidate set, instead of reporting the known floor (W2-4)
Fourth wave-2 item. The all-UNKNOWN branch here was reasoned about carefully and
is correct — its comment even names the two ways a set can be all-UNKNOWN. The
MIXED case fell straight through it.
`RISK_ORDER` is `['LOW','MEDIUM','HIGH','CRITICAL']` and has no `UNKNOWN` entry,
so `indexOf('UNKNOWN')` is -1 and an UNKNOWN candidate can never win the reduce.
An ambiguous name with one caller-less candidate (UNKNOWN, per the round-1 fix)
beside one single-caller candidate (LOW) reported `maxRisk: 'LOW'` — a confident
floor over a set containing an interpretation nobody measured. That is the same
false-safe the all-UNKNOWN branch exists to prevent, one case over, and it
surfaced in the UI as "Max blast radius N (LOW risk)".
`maxRisk` answers "how bad could this be?", and an unresolved candidate could be
CRITICAL — so any UNKNOWN in the set makes the aggregate UNKNOWN. Narrowing it
that way would normally cost information, so the measured part travels alongside
as `knownMaxRisk`, present only when the two differ: absent on a fully-resolved
set, where it would duplicate `maxRisk`, and absent on a fully-unknown one, where
there is no measured part. A reader gets "at least LOW among what resolved, and
one interpretation could not be walked at all", which is strictly more than
either value alone. The human-readable message says the same thing.
The seed gained a mixed pair because the existing one could not reach this: both
its twins are caller-less, so it only ever exercises the all-UNKNOWN branch —
which is precisely why the gap survived a round of review. Three assertions,
both halves mutation-verified.
`eval-server.ts` needs no change: it renders `result.maxRisk ?? 'UNKNOWN'`, so it
now shows UNKNOWN where it previously showed the floor.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(routes): track ternary polarity in dispatch guards, so a selected verb cannot be inverted (W2-9)
`if ((req.method === 'GET' ? false : true) && pathname === '/api/i')` emitted
`GET /api/i` — the one method that branch guarantees the request does NOT have.
A ternary SELECTS between its arms, so a verb inside one is not reached merely
because the whole condition is truthy, but `findVerbInSubtree` descended into
both arms and returned the first verb it saw. Same inversion `!` produced before
|
||
|
Gergő Magyar
|
78ecce1b92
|
perf(import-target): index the workspace once per run for go/csharp/dart/ruby (#2898)
* perf(import-target): index the workspace once per run for go/csharp/dart/ruby Four import-target resolvers answered their lookups with a full `allFilePaths` scan per import, making resolution O(imports x files): - go (#2877): `findRootPackageFiles` / `findAllFilesInPkgDir`, the latter once per path segment on the GOPATH fallback. Most Go imports are external, so the whole cascade ran to completion before returning null. - csharp (#2878): the no-csproj leg took the raw Set past the memoized index the csproj leg was already using - up to eight passes for a four-segment `using`. - dart (#2879): one scan per candidate path, and for an external package both candidates miss, so both always ran to completion. - ruby (#2880): a complete `buildSuffixIndex` rebuilt and discarded per `require` - every require paid to index every file in the repo. Each now reads an index memoized on the `allFilePaths` Set identity, the shape `getPythonFileIndex` (#1918) and csharp's own `getWorkspaceFileIndex` (#1881) already used. Two shared modules back them: - `workspace-file-index.ts`: normalized list + `SuffixIndex` + a normalized->raw map, for csharp and ruby. - `package-dir-index.ts`: "which files live directly inside a directory ending with <path>", for go and csharp. Candidates are bucketed by the directory's last segment rather than by indexing every directory suffix, which would cost O(files x depth) entries at kernel scale (#2649). Behaviour is unchanged, including the tie-breaks that are expressed only through Set-iteration order and `indexOf` positions: the go root leg stays sorted and its package leg stays unsorted, the first-occurrence rule that excludes a directory nested inside a same-named directory is preserved, csharp's whole-path match still beats an earlier suffix match, and dart still tries `lib/<rel>` fully before bare `<rel>` and matches raw paths. Verified two ways. `import-target-index-parity.test.ts` keeps verbatim copies of the pre-change implementations and diffs against them over a deterministic corpus plus hand-built layouts for each tie-break; six mutations of the new code were confirmed to fail it. Separately, the bench corpus produces byte-identical fingerprints against the pre-change resolvers at both 400 and 1600 files. `bench/import-target/measure.mjs` gates both arms in CI: per-language output fingerprints, a scaling budget (measured 0.98-1.12 here, 3.32-4.10 against the pre-change scans), and the corpus shape, so the corpus cannot be shrunk below the size the scaling arm needs and still print PASS. Closes #2877 Closes #2878 Closes #2879 Closes #2880 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCptYZWRgnnJ821rzebQyj * perf(import-target): cover kotlin, and add depth + absolute-cost arms #2872 landed the same index hoist for Kotlin while this branch was open. Fold it into the shared measures so all five resolvers are gated on one corpus, and adopt the two arms that PR's review proved a scaling ratio alone cannot carry. - `bench/import-target/measure.mjs` gains a kotlin arm: a Gradle-shaped corpus with per-module source roots over one package namespace, `.kt` and `.kts` stems, a nested same-name package directory, and a share of wildcard `.*` imports so the package fan-out tier — the only tier whose output is order-bearing — is inside the fingerprint. - `depth_ratio`: deep arm at a FIXED file count with ~6x the path components. `scaling_ratio` divides the file count out, so it is scale-invariant and structurally cannot see a cost that grows with path depth instead, and `buildSuffixIndex` (C#, Ruby) and Kotlin's `suffixByStem` each emit one entry per component. Measured: go 0.98, dart 0.88 (depth-free indexes), ruby 1.48, kotlin 2.20, csharp 3.45 — which is why the budget is per language. One global budget would have to sit at 5.0 and would let Dart go 0.88 -> 4.9 unnoticed. - `small_ms_ceiling`: an absolute bound at 4x the measured arm, because a constant-factor regression that grows both scale arms equally passes every ratio. - The deep arm must resolve exactly what the small arm resolves. Padding was supposed to change depth and nothing else; a deep arm that stopped resolving would be timing the null path. The five fingerprints are unchanged by this commit - verified against the previous baseline before rewriting it, so adding the kotlin arm and the deep scale did not perturb the four languages' output. Kotlin joins the Set-iteration counter in `import-target-index-parity.test.ts` too. Its own guard (`kotlin-import-index-reuse.test.ts`) counts index BUILDS, which a scan added beside a reused index does not move. That counter is also the only DETERMINISTIC guard against a reintroduced scan, and this commit documents why rather than pretending otherwise: a full workspace scan on 1-in-32 imports was measured to pass every timing arm here (dart, 1.458 scaling against a 1.8 budget, 1.736 ms against a 4 ms ceiling) while the counter reads 14 instead of 1. Tightening the ceilings toward the noise floor to chase that case would only buy flaky CI. `bench/kotlin-import-target/` stays: it fingerprints both file-set iteration orders and probes the four-tier cascade shape by shape, neither of which this corpus does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCptYZWRgnnJ821rzebQyj * perf(import-target): merge matching package dirs in one pass `filesDirectlyInPkgDir` re-spread its accumulator once per matching directory, costing O(files x dirs^2) copies per import. On a Go monorepo where many services carry the same package directory (`svcN/internal/pkg`, which Go's GOPATH cascade queries by two-segment tail) that made the index SLOWER than the scan it replaced: 13.4x at 1600 matching directories. Append into one array and sort once. Measured against a verbatim copy of the pre-change scan, output byte-identical at every k: k=200 1400 files old 0.126 ms was 0.169 ms now 0.042 ms k=800 5600 files old 0.457 ms was 3.002 ms now 0.185 ms k=1600 11200 files old 0.960 ms was 12.890 ms now 0.232 ms The index now beats the scan by 2.5-4.1x on this shape instead of losing to it by up to 13x. Also drop the min-`ord` comparison in `firstFileDirectlyInPkgDir`: the build loop appends a directory to its last-segment bucket the moment it accepts that directory's first file, so bucket order already IS ascending first-file-`ord` order and the first hit is the minimum. Differentially verified at 0 divergences. The invariant, and the build-loop edits that would silently break it, are now recorded at the early return. Type the index containers as deeply readonly so Go's deliberate `[...rootFiles].sort()` copy is compile-enforced rather than comment-enforced, and correct the header's claim that a polyglot repo "never pays" -- only the stored index is per-language. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * test(import-target): guard index reuse at the adapter boundary `workspace-file-index.ts` documented the hazard as "a defensive `new Set(allFilePaths)` in an ADAPTER" -- the bug #1918 shipped -- and named the unit parity test as the guard. It is not: that test imports the resolvers directly, while production reaches them through `<lang>ScopeResolver.resolveImportTarget`. Inserting the copy at `go/scope-resolver.ts:31`, `csharp:35`, `dart:189` and `ruby:268` left the parity test 28/28 green and `measure.mjs --check` PASS in all four cases. Kotlin and Python already had adapter-level guards; go/csharp/dart/ruby had none. Add `test/integration/<lang>-import-index-reuse.test.ts` for the four, mirroring the Kotlin/Python precedent: resolve through the scope resolver, assert the file set is traversed once (twice for C#, which builds two indexes), and pair every count with a result assertion so a count of 1 cannot be the count of an adapter that resolves nothing. Each was proven to fail under the copy it exists to catch: go expected 600 to be 1 dart expected 600 to be 1 ruby expected 400 to be 1 csharp expected 600 to be 2 `CountingSet` moves to `test/helpers/counting-file-set.ts` and now counts `forEach`, `values`, `keys` and `entries` as well as `[Symbol.iterator]`. It missed a rescan spelled `allFilePaths.forEach(...)` entirely; with the overrides that mutation reads 14 instead of 1. Four fixtures that pinned the guard next door, each now shown to kill its mutation: - the Dart "matched RAW" case used a forward-slash target, so the basename bucket missed before the raw comparison was reached and it asserted `null === null`. A positive twin carrying the backslash in the TARGET catches both half-mutations. - no C# or Ruby target addressed the corpus's `win\dir\thing` file, so deleting the backslash normalization in `workspace-file-index.ts` passed both gates. Now 4 failures. - `normToRaw`'s first-wins rule had no normalization twin in any corpus. - the Go nested-package fixture was decided by the `endsWith` half and never reached the first-occurrence branch its title names; addressing the directory as a single segment makes it reach it. The parity test's own docstring no longer claims the scan count is a complete census -- it names the three materialized arrays it cannot see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * test(import-target): assert every scale, add collide and retained-heap arms Three holes in the gate this PR ships as its own proof. 1. `--check` computed three fingerprints per language, stored all three, and compared one. `DEEP_PAD = 16 -> 0` deleted the entire depth arm and still printed PASS, because depth padding is count-neutral by design so no asserted number moved. Assert `fingerprint` per scale, and assert `deep.fingerprint !== small.fingerprint` so the padding's EFFECT is pinned, not just its output. 2. The corpus minted per-index directory names (`src/pkg${d}`, `src/Ns${d}`, `lib/feature${d}`), so max last-segment bucket and max matching dirs were both 1 -- and bucket cardinality is the only non-constant term the index has. The `dirCount > 1` merge branch had never executed in any arm. Add a `collide` arm on shared-leaf layouts with identical files/imports/resolved counts; it reaches 9,269 multi-directory merges per run, up to 34 directories at once. Go and C#/Dart legitimately score above the linear budget there and get their own; Ruby and Kotlin stay at 1.8 because their keyed maps are collision-immune and that immunity is the assertion. 3. No arm measured memory, while the C# no-csproj leg newly retains an O(files x depth) suffix index. Add a retained-heap arm on the `bench/cfg` pattern, including its loud failure when `--expose-gc` is missing rather than a silent skip. Measured at 32k files: csharp 73.62 MiB, ruby 55.26 MiB. Ceiling is 1.5x, NOT the 4x the timing arms use -- the measurement is byte-stable to 0.00085% across processes, so 4x would be throwing away the gate. `_arms_note` records why, so nobody harmonises it back. `depth_ratio`, added by this PR, flaked ~1-in-20: go peaked at 1.748 and dart at 2.043 against a 1.6 budget, both ratios of two sub-3 ms minima. Fixed at the estimator, not the threshold -- REPS 5 -> 15, matching `bench/cfg`, `schema-pairs` and `callable-value-flow` (5 was the lowest in the repo; the sibling `kotlin-import-target` uses 7, which was not enough here). 22/22 PASS, every arm now at 70-78% of its budget with a <=1.26x swing. No budget was widened; the distributions are recorded in `_arms_note` so the headroom is visibly earned. Three copies of the same overclaim corrected: the parity test NARROWS the 1-in-32 blind spot, it does not close it -- it watches the Set while the resolvers hold materialized arrays. `_floor` no longer claims its ratios "match" the issues' (different corpora, both quadratic). The step moves to the END of the benchmarks job and runs with `--expose-gc`. A failing step aborts every step after it (#2895), so the newest, least-proven gate must not sit ahead of eight established ones. All five output fingerprints are byte-identical to before this session -- the proof that every change here was behaviour-preserving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * docs(import-target): point the reuse contract at the guard that guards it `workspace-file-index.ts` told callers the unit parity test guards the adapter-copy hazard. It does not -- it never crosses the adapter. Name both layers and say which catches what: the per-language `test/integration/<lang>-import-index-reuse.test.ts` files at the adapter boundary, the parity test for a rescan reintroduced inside a resolver. The C# namespace-dir index comment named `findDirectChild`, which this PR deleted; it feeds `firstFileDirectlyInPkgDir` now. Drop `GoResolveContext`, dead since the legacy call-resolution DAG was removed in #942 -- zero importers, and `gitnexus`'s package.json declares no `main`, `exports` or `types`, so it is not a published surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * refactor(import-target): quality pass over the review-round changes Four cleanup lanes (reuse / simplification / efficiency / altitude) over the previous four commits. No behaviour change anywhere: all 25 bench cells (5 languages x 5 arms) are byte-identical on files, imports, resolved, distinct_outcomes and fingerprint, re-verified after each individual edit. **Restores a fast path the last commit lost.** Fixing the O(k^2) accumulator made the SINGLE-directory case — the overwhelmingly common one — copy the bucket where the original aliased it: measured 1.11x slower at 4 files/dir rising to 1.72x at 128. Holding the first bucket by reference and promoting to an accumulator only when a second directory appears is 0.65-0.97x of the previous code at dirCount=1 and parity at dirCount=64. 176-case differential, 0 divergences. **`sortedRootFiles` accessor.** `rootFiles` was the only index container read directly from outside the module. `readonly` is erased at runtime and `Array.isArray` widens it back, so the copy rule now lives with the code that owns the invariant instead of at the call site. No `Object.freeze`: V8's PACKED_FROZEN_ELEMENTS read cost lands on the hot `matchingDirs` path. **One shared arm for the four reuse guards.** The distinct-file-set test was copy-pasted four ways, 33-38 identical lines each, and this repo's own helpers (`mini-repo.ts`, `scope-model.ts`) document extracting at the SECOND verbatim consumer. `expectDistinctFileSetsGetOwnIndex` takes what actually varies; its `expected` type excludes `null` so the pairing rule cannot be reinstated as a hole. The per-language first and third arms stay duplicated on purpose — corpora and payload shapes genuinely differ. Re-proven: all four still fail under an adapter-inserted `new Set(allFilePaths)`. **Bench.** `dirsFor` shared by the two functions that must agree on directory fan-out (they mint and address the same files). `SCALES` derived from the arm table, so a future arm cannot be measured, printed and silently never asserted. Five timing checks with one shape collapsed to a table — the trailing sentence had already drifted into four wordings. `uniqueTarget`/`collideTarget` as flat functions, mirroring the `uniqueDir`/`collideDir` split rather than nesting a second axis four ternaries deep. One `identityPass` replaces two untimed full resolution passes per cell: -371 ms median. **CI step moved back where it belongs.** It was parked last "until #2895 lands", but that reasoning was backwards twice over: the flake that motivated it was fixed at the estimator in the previous commit, and #2895's own audit measured the last slot as executing zero times in 13 runs. It sits with the other resolver-index guards; #2899 carries the `if: !cancelled()` that fixes step masking for every step at once. Filed rather than fixed here: #2908 (java and cobol still scan the workspace per import, same shape as #2877-#2880, neither memoized), #2909 (make index reuse a contract test over SCOPE_RESOLVERS on one instrument). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
glier
|
c6b24162d9
|
perf(kotlin): index import resolution instead of scanning per import (#2872)
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
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Skill copy sync / shipped skills drift guard (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* perf(kotlin): index import resolution instead of scanning per import
`resolveKotlinImportTarget` walked the entire workspace on every import.
Its four tiers — exact/suffix, directory child, package fan-out and
progressive prefix strip — each ran `for (const raw of allFilePaths)` with a
`replace(/\\/g, '/')` and several string scans per entry, and they are tried
in cascade, so one unresolved import cost two to four full passes.
Across a repository with tens of thousands of Kotlin files that is
O(imports x files): on the order of 10^10 string operations on a single
thread. It does not look like a hot loop from the outside - analyze sits at
exactly 1.00 core with a completely flat heap and emits nothing for hours,
because every allocation is a short-lived string and nothing accumulates to
hint at progress. Small repositories hide it entirely: at a few hundred files
each pass is free.
Three maps, built once per `allFilePaths` Set and memoized on its identity,
make each tier O(1): stem -> path for the exact tier, every component-suffix
of the stem for the suffix tier, and directory -> direct children for both the
fan-out and the first-child fallback. Cost becomes O(files) once plus O(1) per
import. This mirrors the existing Python index (`getPythonFileIndex`), down to
the WeakMap keying and the build counter.
Semantics are unchanged, including the parts the scans expressed only through
iteration order:
- an exact match anywhere beats a suffix match found earlier, because the
scan returned on the first exact hit but merely remembered the first
suffix hit;
- "first match" stays first in set-iteration order, so both stem maps keep
the earliest path inserted for a key;
- a directory-name match still honours the scan's `startsWith`-then-`indexOf`
rule, which only ever considered the FIRST occurrence of `/dir/`. A path
like `data/src/main/kotlin/com/example/data/Repo.kt` is therefore still
NOT a child of `data`. That is arguably wrong, but fixing it here would
silently move edges in every Kotlin repository; it belongs in its own
change with its own fixtures.
That claim is gated, not asserted. `bench/kotlin-import-target` fingerprints
every `fromFile | targetRaw -> result` triple over an exhaustive branch matrix
plus a deterministic fuzz, each file set resolved in BOTH iteration orders
because that is the only place the tie-breaks above are expressed. The
committed baseline is the value the PRE-INDEX implementation produces: both
implementations print
5ad605c179081505705ff7698a09dbdbdc4831080af6d9fdec5499cc6bce28ee over the same
20074 cases, 11612 of them non-null, and anyone can re-run it by pointing the
harness's module specifier at the old file.
Its second arm is the scaling ratio, `(t_large/t_small)/(1600/400)` over a
synthetic Kotlin monorepo whose imports are ~40% unresolvable — only a miss
drives all four tiers, which is where the scan was worst. The index measures
0.99 (8.0 ms / 31.7 ms); the implementation it replaces measures 3.737
(2207.8 ms / 33003.5 ms) on that same corpus, so the budget of 1.6 separates
them by a wide margin. Take the absolute times as an order of magnitude only
(~276x, ~1041x): the floor arm was run once cold because best-of-seven against
a quadratic implementation costs minutes, while the index arm is the usual
best-of-seven. The ratios are the comparable pair. Both arms run in the
existing always-on `benchmarks (GITNEXUS_BENCH)` job, next to the C++ guard
from #2788 and the Python one from #1918.
Two unit-level guards sit alongside it: a parity test pinning the curated
cases, and an integration test asserting the index is built once across many
imports — the adapter must pass the Set through, since a defensive copy would
hand a fresh WeakMap key per call and restore the old behaviour (the same trap
Python hit in PR #1918).
Two other providers have the same defect and are left alone here, having no
repository at hand to verify a change against:
- `go/import-target.ts`: `findRootPackageFiles` and `findAllFilesInPkgDir`
scan unmemoized, and the GOPATH fallback calls the latter once per path
segment but the last, so a single import can trigger several full passes;
- `dart/import-target.ts`: the `package:` branch scans once per candidate
path — `lib/<rel>` and bare `<rel>` — and `resolveRelative` scans again in
its suffix fallback, also unmemoized.
`csharp/import-target.ts` is a partial case worth noting: it already builds a
memoized `getWorkspaceFileIndex`, but that is reached only when a `.csproj` is
found; the no-csproj path hands the raw Set to `resolveDirectMatch` and
`resolveByProgressiveStripping`, which scan past it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(kotlin): close the blind axes in the import-resolution gate
Review of #2872 found the weak part was the gate, not the resolver: four
plausible follow-up mutations passed `--check` with a byte-identical
fingerprint, `cases` AND `non_null`. Each is now caught, and each was
re-checked against the mutation it exists to stop.
- The hashed record carried `order | fromFile | targetRaw | result` but not
the FILE SET, so a corpus edit that swapped the workspace under a case
while leaving its result string alone was invisible. Leaving the resolver
untouched and editing only the corpus, two documented load-bearing cases
could be gutted — the "exact beats an earlier suffix" case losing its
competing file, the repeated-directory negative case losing its file
entirely — with the gate green. The file set is now part of the record, and
that same edit now moves the fingerprint.
- The corpus capped path depth at 8 components and packages at 16 files,
which are precisely the two axes the loops this change added run on. It now
carries 11- and 13-component paths, queries against suffix keys deeper than
seven segments, a 40-file package, and a fuzz that spans both. Verified:
capping suffix-key depth at 7, skipping the `dirChildren` suffix loop above
depth 8, and capping a bucket at 17 entries each now move the fingerprint,
where all three previously passed.
- `non_null` was reported but never asserted; it is asserted beside `cases`.
That closes only the "resolves nothing at all" hole — it stayed 11612 under
all three code mutations above and under the corpus edit — so it is a
companion to the two fixes above, not a substitute for either.
- A ratio cannot see a constant factor, and a file-count ratio cannot see a
depth cost. `--check` now also asserts a DEPTH ratio (file count fixed,
paths 24 components against 8) and an absolute ceiling on the small arm: a
full workspace scan reintroduced on 1-in-32 imports scores 1.490, inside
the scaling budget, while running 2.8x slower.
The baseline is re-derived, not adjusted: the pre-index implementation and the
index both print
ebf1790bf1d42dad483a51f2cbdeb2351e493b9e8236e4eedeef592dd81e2c5c over the new
20106-case corpus, 13256 of them non-null.
Both test suites were shown to be non-load-bearing and now are:
- the parity test's repeated-directory case put `data` at the LEADING
segment, so the `startsWith` guard fired and the `indexOf` rule its own
comment describes was never reached — a resolver with that check relaxed to
`>= 0` passed all 18 cases. A mid-path case now pins it, and a backslash
fan-out case pins `norm.lastIndexOf` against `raw.lastIndexOf`, which was
also bench-only. Both mutations now fail the unit suite.
- the index-reuse test discarded all 200 return values, so a build count of 1
was equally true of an adapter that had stopped resolving anything. It now
asserts results, and its docstring premise is corrected: every one of its
imports hit the tier-1 suffix lookup and none reached the fan-out it
claimed to exercise. Half now genuinely do. The `undefined as never` casts
and the `?.` are gone — both trailing parameters are optional and the
member is required.
Resolver changes, all output-identical against the differential above:
- `dirChildren` buckets are frozen once built. `findKotlinPackageFiles` hands
a bucket straight out of the index, and the `readonly string[]` return type
does not survive the caller: the finalize pass normalizes with
`Array.isArray(t) ? t : [t]`, and `isArray`'s `arg is any[]` predicate
widens the true branch, so `tsc --strict` accepts a `.sort()` there. A
downstream sort would permanently reorder the cached bucket and flip the
first-child tier for every later import in the run.
- `stripped` is computed only after tier 1 misses, with `lastIndexOf`/`slice`
instead of `split`/`slice`/`join`. Measured -20% small arm, -21% large arm.
- `KOTLIN_EXTENSIONS` now comes from the existing `import-resolvers/jvm.ts`
export instead of a fourth inlined copy.
- A note on why the shared `buildSuffixIndex` is not reused, with the four
probes that diverge, and the measured basename-bucket comparison — the one
place this was less documented than the Python precedent it follows, and
the question the Go/Dart/C# follow-ups will each face.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
|
||
|
dependabot[bot]
|
29929b7488
|
chore(deps): bump docker/login-action from 4.4.0 to 4.6.0 (#2851)
Bumps [docker/login-action](https://github.com/docker/login-action) from 4.4.0 to 4.6.0.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](
|
||
|
dependabot[bot]
|
911fdb1ae1
|
chore(deps): bump ossf/scorecard-action from 2.4.3 to 2.4.4 (#2852)
Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.4.3 to 2.4.4.
- [Release notes](https://github.com/ossf/scorecard-action/releases)
- [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md)
- [Commits](
|
||
|
dependabot[bot]
|
665e7bb44a
|
chore(deps): bump release-drafter/release-drafter from 7.6.0 to 7.7.0 (#2853)
Bumps [release-drafter/release-drafter](https://github.com/release-drafter/release-drafter) from 7.6.0 to 7.7.0.
- [Release notes](https://github.com/release-drafter/release-drafter/releases)
- [Commits](
|
||
|
Gergő Magyar
|
74409a37f6
|
perf(cpp): index qualified namespace members once per pipeline run (#2788) (#2794)
* perf(cpp): index qualified namespace members once per pipeline run (#2788)
`resolveCppQualifiedNamespaceMember` walked every parsed file — rebuilding a
per-file `scopesById` map each time — once per qualified `ns::member()` call
site, so the scope-resolution emit phase cost O(callsites x scopes). On a
1,473-file C++ repo that was 25.3 min of a 33-min analyze, with 75% of total
self-time in this one function. Its inner `findMemberInNamespaceTransitive`
compounded it: each recursion step filtered `scopesById.values()` by parent,
O(scopes^2) per file on its own.
This is the same bug #1990 fixed in the sibling ADL path (`pickCppAdlCandidates`
-> `AdlCandidateIndex`), so it gets the same fix: a `QualifiedNsMemberIndex`
(receiver simple name -> member simple name -> callable defs) built lazily once
per `parsedFiles` identity and reset by `clearCppInlineNamespaces`, which runs
from `cppScopeResolver.loadResolutionConfig` at the start of every pass. Per
call site the work drops to two Map lookups.
Ordering is preserved exactly — file-major, `parsed.scopes` declaration order,
a namespace's own `ownedDefs` before its inline-namespace children, depth-first
— because the caller takes `allHits[0]` for the single-hit case and
`narrowOverloadCandidates` is first-wins. Non-inline nested namespaces are
still not descended into, and same-name hits across inline children still
report `'ambiguous'` (#1564).
Measured with `PROF_SCOPE_RESOLUTION=1 analyze --force --index-only` on a
synthetic corpus (`namespace ns_i { inline namespace v1 { ... } }` plus 20
`ns_j::fn()` call sites per file):
| files | emit before | emit after |
|-------|-------------|------------|
| 100 | 153ms | 16ms |
| 200 | 704ms | 24ms |
| 400 | 3,293ms | 42ms |
| 800 | 16,898ms | 78ms |
Before, doubling the file count quadrupled emit; now it doubles. At 800 files
total scope resolution goes 17.2s -> 394ms.
Output is unchanged, verified rather than assumed: a full graph dump (sorted
nodes + relationships) from a baseline build at the parent commit and from this
one are byte-identical on all 134 `cpp-*` fixtures merged into a single repo
(1573 nodes / 1997 relationships) and on the 400-file synthetic corpus.
`test/integration/resolvers/cpp.test.ts` passes 334/334.
#1990 shipped its ADL fix without a scaling gate, which is how the bug class
came straight back here, so this adds one: `bench/cpp-qualified-ns` measures
`(t_large/t_small)/(1600/400)` — 0.93-1.21 indexed versus 3.45 for the old
per-call-site scan — alongside a fingerprint over every
`receiver::member -> outcome` the corpus resolves, and CI runs it with
`--check`. `test/unit/cpp-qualified-ns-index.test.ts` covers the cache
invalidation the index introduces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cpp): address tri-review findings on the qualified-namespace index (#2788)
Multi-engine review of this PR (Claude swarm + ce-code-review, Codex
gpt-5.6-sol swarm + ce + adversarial) returned two P1s and five smaller
findings. All are fixed here.
P1 — the index defeated the pipeline's post-language memory release.
`scope-resolution/pipeline/phase.ts` evicts each language's files and then
calls `forceGc()`, on a stated premise that "This language's ParsedFiles are
now unreachable", sizing C/C++ at ~17-20GB on the Linux kernel. The
module-level `let qualifiedNsIndexSource` falsified that: it pinned the whole
`parsedFiles` array, and the index held defs reaching into those files'
scopes, until the *next* C++ pass cleared it — which in a single analyze never
comes. C++ is 7th of 16 in SCOPE_RESOLVERS, so the set survived nine later
language passes plus emit. Replaced with a
`WeakMap<readonly ParsedFile[], QualifiedNsMemberIndex>`, the pattern already
used by `moduleScopeIndexByPass` in `cpp/file-local-linkage.ts`.
`clearCppInlineNamespaces` still swaps in a fresh WeakMap, because the index
has a second input (`inlineNamespaceScopeIds`) the key cannot observe.
Measured with `--expose-gc`: 61.2MB retained after the caller drops the array
before, 0.1MB after.
The ADL twin (`adl.ts`) has the same pattern, so the hazard predates this PR —
but `pickCppAdlCandidates` returns early before `ensureAdlIndex` on
`noAdlSites`/empty `argInfoBySite`, so it rarely arms, whereas a qualified
`ns::member()` index arms on almost every C++ workspace. Moving the ADL twin
to a WeakMap is left as a follow-up.
P1 — the new bench could not see the regression class it exists to gate.
`callSites()` drew every receiver from `ns_${...}`, so the receiver lookup
never missed; production is the opposite, since Case 1.5 in
`receiver-bound-calls.ts` is reached by every plain-identifier receiver call
and misses on most. A rescan reintroduced only on the receiver-bucket-absent
path scored 1.279 and PASSED the old bench. The corpus now mirrors production
(~1 in 5 receivers name a declared namespace) and adds a namespace reopened
across files, a same-name inline nest, a member declared at both namespace and
inline-child level, and call sites carrying a real `Callsite` so
`narrowOverloadCandidates`/`cppConversionRank`/
`isOverloadAmbiguousAfterNormalization` are inside the fingerprinted surface at
all. That same rescan now measures 4.538 and FAILS; defeating the dedup now
fails the fingerprint arm where it previously passed byte-identical. The
fingerprint moved once, deliberately, for the corpus expansion — recorded in
`_rebaseline_2788_review`, explicitly not precedent.
Also fixed:
- Unbounded recursion aborted analyze. `collectNamespaceMembers` recursed per
inline child with no bound and threw an uncontained `RangeError` at inline
depth 8000 (`phase.ts`'s try has a `finally`, no `catch`), and a receiver
*miss* paid full recursion where the deleted walker skipped on a name
mismatch. An explicit work-stack alone would only have converted that into
an OOM at depth 6000, because the eager table was quadratic in memory too:
for a depth-D chain it legitimately holds D(D+1)/2 entries, since `v2::foo()`
is a valid receiver at every level. Replaced with a lazily-queried node graph
(per-scope own-member buckets plus direct child links, resolved on demand and
memoized per receiver+member). Build is now linear; depth 100000 costs 133ms
where 8000 previously threw.
- "#1990 shipped without a scaling gate" was false. #1990 did ship
`test/integration/cpp-adl-benchmark.test.ts` (
|
||
|
Gergő Magyar
|
911151e230
|
fix(resolution): resolve Go pointer-receiver calls, and report the program boundary instead of hedging (#2766) (#2782)
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
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
|
||
|
ivangegovdve-sudo
|
565287528d
|
fix(ci): stop CI Report dying silently when the tests job fails (#2728)
* fix(ci): stop CI Report dying silently when the tests job fails
The "Build report" step in ci-report.yml runs under
`bash --noprofile --norc -e -o pipefail`. It located its inputs with
UNIT_SUMMARY=$(find "$DIR/test-reports" -name ... 2>/dev/null | head -1)
`coverage-merge` in ci-tests.yml is `needs: tests` with no `if: always()`,
so any failing shard skips it and the `test-reports` artifact is never
uploaded. `find` then runs against a directory that does not exist and
exits 1; `-o pipefail` carries that status through `| head -1`, the
command substitution hands it to the assignment, and `-e` kills the step.
The death is invisible: `2>/dev/null` discards find's error and the whole
report is built into `$GITHUB_OUTPUT`, so the step logs nothing and just
reports "Process completed with exit code 1". "Comment on PR" is then
skipped, so the CI Report workflow fails and posts nothing on exactly the
PRs whose tests failed — when the report is most useful. The
"Coverage data unavailable" fallback already existed for this case but
was unreachable, because the script died ~160 lines before it.
Route the four lookups through a `find_first` helper that returns empty
when the root is absent. Verified by extracting the step body and running
it against both artifact layouts: with `test-reports` present the output
is byte-identical to the previous script (1335 bytes), and with it absent
the step now exits 0 and emits the coverage-unavailable report instead of
exiting 1 with an empty $GITHUB_OUTPUT.
Observed on 32 of the last 100 failed runs; correlation with the tests
job's conclusion was 6/6 failure and 4/4 success in the sampled runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(ci): let the prebuild assertion report a missing .node
`Build prebuild` deletes `$pkgdir/prebuilds` before running prebuildify,
so a run that emits nothing without failing leaves `find` searching a path
that no longer exists. Under the step's `shell: bash` (`-e -o pipefail`)
that `find` exits 1 and kills the step before the `test -n "$out"` guard
below it — the guard written to explain exactly this case never runs, and
the job dies with a bare "Process completed with exit code 1".
Same shape as the `ci-report.yml` fix in this PR: a lookup that exits
non-zero on an absent root pre-empts the fallback beneath it. `|| true`
hands the empty result to the guard, which still fails the build, now with
`::error::prebuildify produced no .node`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
|
||
|
dependabot[bot]
|
51095c19f8
|
chore(deps): bump actions/setup-python from 6.3.0 to 7.0.0 (#2757)
Some checks are pending
Gitleaks / gitleaks (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.3.0 to 7.0.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](
|
||
|
dependabot[bot]
|
e0dc0c2d5e
|
chore(deps): bump release-drafter/release-drafter from 7.5.1 to 7.6.0 (#2756)
Bumps [release-drafter/release-drafter](https://github.com/release-drafter/release-drafter) from 7.5.1 to 7.6.0.
- [Release notes](https://github.com/release-drafter/release-drafter/releases)
- [Commits](
|
||
|
dependabot[bot]
|
909f2f85b6
|
chore(deps): bump the codeql-action group across 1 directory with 3 updates (#2755)
Bumps the codeql-action group with 3 updates in the / directory: [github/codeql-action/init](https://github.com/github/codeql-action), [github/codeql-action/analyze](https://github.com/github/codeql-action) and [github/codeql-action/upload-sarif](https://github.com/github/codeql-action). Updates `github/codeql-action/init` from 4.37.0 to 4.37.3 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits]( |
||
|
MyShining
|
de84ad6297
|
feat(spring): index @Bean factories and @Resource injection (#2740)
* feat(spring): index Bean factories and Resource injection * fix(spring): address Bean and Resource review findings * refactor(lbug): keep relation pair parsing in router * test(lbug): preserve schema exports in WAL mocks * test(cache): align schema bump pin --------- Co-authored-by: Shining <xuenning@qiyi.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> |
||
|
Gergő Magyar
|
27ab37c432
|
feat(resolution): type receiver chains from AST structure across all 14 languages (#2708) + epistemic lower-bound (#2744) (#2747) | ||
|
Gergő Magyar
|
13c77db4d9
|
fix(ci): stop the placeholder review, verify citations, repair once (#2733) | ||
|
Gergő Magyar
|
b0cacd05ee
|
fix(ci): stop the review agent rejecting its own graph-backed reviews (#2731)
* fix(ci): stop the review agent rejecting its own graph-backed reviews
The context-evidence gate only counted a `context` call when the call
itself passed `file_path` equal to a changed path. The review skill
teaches plain `context({name})`, so 17 of the 26 review-agent run
failures were complete, graph-backed reviews thrown away after full
model spend, with no log line saying which invariant failed.
Prove the evidence from the result instead: `status=found` plus a
`symbol.filePath` inside the repo-scoped changed-path set. Every other
check stays exactly as it was - strict JSON, orchestrator-only turns,
result ordering, duplicate tool-id rejection - and the `repo` argument
still selects the head or the merge-base path set.
Same failure inventory, smaller classes:
- rejection now logs why (in-scope, out-of-scope, sidechain, unresolved
and off-path counts plus up to three sanitized paths), and the
envelope error names the message count and first-message shape
- Glob/Grep leave the tool set: they were enabled through `--tools` but
never allow-listed, so every lane call was denied and burned turns
- both pinned `npm ci` installs retry three times; one registry
ECONNRESET killed a whole run
- the prompt matches the new contract and asks for the structured body
even when the analysis is incomplete
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(skills): mirror the review-skill tool-set change into the shipped copies
The npm package, Claude plugin, and Cursor integration ship byte-identical
copies of .claude/skills/gitnexus-review, and the drift guard compares them.
Dropping Glob/Grep from the lane frontmatter and the SKILL.md sentence only
landed in the canonical tree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(ci): stop one junk context result discarding a proven review
Tri-review of this PR found that the previous commit fixed one spurious
rejection and created another. Widening evidence candidacy from "the call
that named a changed path" to "every orchestrator context call" also
widened the *strict-parse* surface: `contextResultProvesChangedPath`
throws rather than returning false, so a single malformed payload
anywhere in the transcript now discarded a review that an earlier call
had already proven. The MCP makes that reachable without any misbehaving
model - `GITNEXUS_MCP_DEFAULT_MAX_TOKENS=12000` truncates any context
payload over ~48 KB mid-JSON and appends a marker - and it also destroyed
docs-only runs that the `no_indexable_changed_symbols` mode exempts.
Reproduced by running the workflow's own embedded script on both trees:
a proving evidence call followed by one truncated exploratory call gave
`failure_code: null` on the base and `invalid_execution_transcript` on
the head; it is `null` again here.
- payload-shape failures are caught and counted (`malformedResults`)
instead of thrown; transcript-structural invariants (envelope, tool
shapes, duplicate ids, empty tool_result) still fail closed
- diagnostics gained the reasons they were blind to: errored results,
results that arrived out of order or via a sidechain, unanswered
in-scope calls, and malformed payloads. A rejection can no longer
print an in-scope call with every reason at zero
- a deletion-only PR no longer registers head-scoped candidates that can
never be satisfied: an empty eligible set is out of scope, not a result
"outside the changed paths"
- the mandatory-body prompt clause now pairs with a required `complete`
boolean. An incomplete analysis publishes its partial body labelled
`incomplete_analysis` instead of passing as an accepted review
- `Agent(a,b,c)` is split into six separate `Agent(x)` rules: the pinned
base action parses allowedTools with `.flatMap((v) => v.split(","))`
(parse-sdk-options.ts at 3553f843), which shattered the grouped rule
into `Agent(ci-correctness-lens`, four bare names, and
`ci-critic-lens)` before the SDK saw it. Pre-existing and unproven at
runtime, but the split form is correct under either reading and lets
the header's dispatch canary actually prove something
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(ci): require a line range for context evidence
The tri-review's adversarial lane executed `context({name: 'AGENTS.md'})`
and had the result accepted: the gate checked only that the resolved
filePath was in the changed set, so a bare File node passed for a review
of that file's contents. The trusted prescan already defines an indexable
symbol as one with startLine and endLine, so require the same here.
Pre-existing rather than introduced by this branch, but it is the same
"what counts as proof" surface the rest of this PR tightens.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(ci): close the remaining tri-review findings
Addresses every finding the tri-review left open after
|
||
|
Gergő Magyar
|
4906daf27b
|
fix(scope-resolution): resolve calls through a closure-valued binding across languages (#2693) (#2695)
* fix(scope-resolution): resolve calls through a closure-valued binding (#2693)
`val f = { }; f()` emitted no CALLS edge in Kotlin or Swift, so `impact` on
such a symbol under-reported to zero — the same false all-clear as #2687.
The cause was not, as first suspected, that these languages fail to feed
`callable-value-flow`. They do: `synthesizeCallableFlowCaptures` is called
from 15 language capture modules, and Kotlin already resolves reassignment
through the pass (`var f = ::a; if (c) f = ::b; f(1)` reaches both targets).
Their captures are already exactly right — the seed names the binding as its
own callable, per the anonymous-callable convention in
callable-flow-captures.ts.
They died one layer later, at the `buildGraphTargetIndex` gate:
if (!isCallable(def) && providerTarget?.(def) !== true) continue;
`isCallable` is Function/Method/Constructor, but the scope-resolution layer
declares a closure binding with its VALUE label (Kotlin/Swift `Property`),
and `isCallableValueTarget` is implemented by exactly one provider — COBOL.
So the binding never entered `graphTargets`; `lexicalCallableLookup` then
returned `shadowed: true` with no targets, which also suppressed the
workspace-wide fallback, and the seed resolved to nothing.
Only the graph knows a value binding holds a callable — since #2687 it emits
a single `Function` node for one. So value bindings now resolve their graph
id first and are admitted on the label of the node they actually reach.
This is self-limiting: a genuine constant keeps its own Const/Property node,
so `resolveDefGraphId`'s qualified key hits before the label-agnostic
`simpleKey` fallback can reach a same-named callable. Only a binding whose
own value node was replaced by a callable one gets through.
No scope kind changes — Kotlin's `lambda_literal` stays `@scope.block`, so
#1757 smart-cast semantics are untouched by construction. The fix is
language-neutral: it discriminates on the graph node label, never on a
language name.
Dart is fixed separately; its root cause is independent.
* fix(dart): resolve calls through a closure-valued binding (#2693)
Dart needed more than the shared gate fix: neither of its closure-binding
forms could resolve, for two different reasons, and the plan's one-line
diagnosis turned out to be incomplete.
TOP-LEVEL `var f = (x) => x;`
A graph Function node already existed (#2687), but no `@declaration.*`
matched the binding, so scope resolution had no SymbolDefinition to attach
a flow seed to. Adding the declaration exposed a second problem: Dart's
`initialized_identifier` is FIELDLESS, so the shared field-based assignment
fallback (`left`/`name`/`value`/…) decomposed nothing and the binding still
emitted no flow captures at all. Kotlin's fieldless `assignment` node hit
exactly this and took the same remedy — a provider `extractAssignment`.
FUNCTION-LOCAL `void m() { var f = (x) => x; }`
Locals parse as `initialized_variable_definition`, which the top-level
graph-node rules are deliberately anchored under (program) to avoid, so a
local closure had no graph node at all — nothing for the widened
`buildGraphTargetIndex` gate to admit.
Both new rules are restricted to a `function_expression` value. Declaring
every Dart variable would mint defs and nodes repo-wide for no resolution
benefit; ordinary locals stay unindexed exactly as before. The top-level
declaration reuses the (program) anchor the graph-node query already relies
on, so class-body fields — which share `initialized_identifier_list` and are
already `@declaration.property` — are never matched twice.
Also drops the now-false note in tree-sitter-queries.ts claiming `f()` does
not resolve for Dart. That node is now the evidence that makes it resolve.
* docs(scope-resolution): document the callable-flow capture contract (#2693)
The module is 1200+ lines behind a nine-line docblock, and the only worked
example was C. Both root causes fixed in this series were "the contract was
discoverable only by reading the emitter":
- the anonymous-callable convention (a seed whose source is a closure takes
its DESTINATION's name) is what makes closure bindings resolvable at all,
and is the reason the widened target gate is correct;
- a fieldless binding node silently decomposes to nothing under the shared
assignment fallback, which cost Kotlin one debugging cycle in #2522 and
Dart another here;
- captures alone are never enough — the bound name also needs a
`@declaration.*` or there is no cell to key the seed on.
Records the cell/site model, both traps, and points at the fullest and
smallest worked examples.
Bumps INCREMENTAL_SCHEMA_VERSION 15 → 16 and the parse-cache SCHEMA_BUMP
22 → 23: this series emits NEW CALLS edges and new Dart Function nodes, and
the incremental write set only covers changed files, so an existing index
would keep reporting a zero blast radius for exactly the symbols the fix is
about.
* perf(scope-resolution): pre-filter value bindings in the callable target index (#2693)
Widening the `buildGraphTargetIndex` gate to consider VALUE bindings put the
hot loop on a much larger def population — value bindings outnumber callables
in real source — and the naive version paid full price per binding. Measured
on a synthetic 800-file corpus (8 value bindings per file, 1 of them a closure
binding), the widening cost 2.50-2.82x the pre-#2693 callable-only build.
Two wastes, both provable rather than guessed:
1. `definitionAnchorKey` ran for every def, including value bindings. The
anchor index is keyed by callable LABEL and the key is built from
`def.type`, so a value def can never hit it — and the key costs a regex
per def.
2. Every value binding paid the whole `resolveDefGraphId` key chain only to be
rejected. It need not: every qualified key that function tries embeds
`def.type`, so for a VALUE def those can only ever reach a value-labelled
node. Its one route to a callable is the label-agnostic
`simpleKey(filePath, simpleName)` fallback, which by construction requires
a callable node with the SAME file and simple name. So a value binding with
no such node cannot resolve to a callable, and one Set lookup decides it.
That set is derived in the graph walk the anchor index already performs, so it
costs no extra pass.
large_ms 7.79-8.37 -> 4.90-5.02 (1.61x faster)
widening_overhead 2.50-2.82 -> 1.45-1.50
The resolved target-set fingerprint is byte-identical across both, which is
the point: this is a cost change, not a behaviour change.
Adds bench/callable-value-flow/ (fingerprint + scaling + widening-overhead
gates) and wires it into ci-tests.yml beside the other build-free benches. The
overhead budget of 1.9 sits between the measured with-filter and without-filter
bands, so it cannot be met if the pre-filter is removed. Timings use the MIN of
15 warmed reps, not the median: the same build reported 1.65 idle and 2.03
under load, and a median-based gate would have to be loosened past the point of
detecting the regression it exists to catch.
`buildGraphTargetIndex` is exported for the bench; it is pure and not part of
the pass's public contract.
* test(scope-resolution): assert the declaration route does not double-emit (#2693)
Go, Python, C++ and TS/JS already resolved a closure-binding call through
their `@declaration.function` capture. The widened `buildGraphTargetIndex`
gate gives the same call a SECOND possible route, so each must still produce
exactly one edge.
`tryEmitEdge` dedups by key, but a collapsed key and a site-anchored key are
DIFFERENT keys — a real double-emit would show up as two ids for one call
site, not be silently collapsed. Asserting on edge ids rather than target ids
is what makes that visible.
* fix(scope-resolution): join value bindings to their callable node by POSITION (#2693)
Review found the first cut of this series minted FALSE CALLS edges. Admitting a
value binding whose *resolved* graph node is callable let `resolveDefGraphId`
fall through to its label-agnostic, first-write-wins
`simpleKey(filePath, simpleName)` and bind the name to ANY same-named callable
in the file.
The safety argument in the previous commit — "a genuine constant keeps its own
Const/Property node, so the qualified key hits first" — silently assumed
`def.type === node.label`. It does not hold:
- TypeScript declares `const` as `Variable` but emits a `Const` NODE, so the
qualified key misses even though the value node exists;
- Rust `let` bindings get no graph node at all, so the fallback is the only
route.
Reproduced, all previously emitting a fabricated caller:
const save = (x: number) => x * 2; // next to an unrelated Svc.save
-> Method:svc.ts:Svc.save#1 // Svc never instantiated
const handler = other; // shadowing a top-level handler
-> Function:app.ts:handler // unreachable from here
let handler = cb; // Rust
-> Function:main.rs:handler
Worse in Dart, where the same collision INVERTED the feature: the only edge went
to the class method and the closure's own node got none. The result was also
declaration-order dependent — two files differing only in declaration order got
different CALLS sets — and it propagated through argument-to-formal binding into
functions whose source never mentions the name.
A closure binding IS its callable node: same file, same line, same name. An
aliasing local is not. So the join is positional now — a file/line/name index
built in the graph walk `byAnchor` already performs — and value bindings never
run the key chain at all. That is both correct and cheaper:
large_ms 4.90-5.02 -> 4.37-4.63
widening_overhead 1.45-1.50 -> 1.43-1.58 (name-match design: 2.50-2.82)
with a byte-identical target-set fingerprint on the bench corpus.
Also from review:
- `Static` dropped from VALUE_BINDING_DEF_TYPES: `normalizeNodeLabel` has no
`static` case, so no def can carry that type — it was an entry no fixture
could ever exercise. The remaining set now documents why it deliberately
does NOT reuse `isOwnableValueLabel`, which is contracted to a different
consumer.
- Dart `final`/`const` top-level closures (static_final_declaration_list) and
every declarator after the first in a multi-name local now resolve; both
parse into shapes the earlier rules never reached.
- The bench source carried a literal NUL byte, so git recorded it as BINARY
and the only artifact pinning the target set was unreviewable in the PR
diff. It is written as an escape now. Its corpus also modelled `startLine`
as 1-based where graph nodes are 0-based, which would have stopped it
exercising the value-binding path at all.
- `call-summary-schema-version.test.ts` asserted `passesReuseGate(15)` is
true; the 15 to 16 bump made that false and the test RED. It now pins 16 as
current and 15 as rejected, matching the pattern every prior bump followed.
- The v23 parse-cache comment is at the top of the list, not mid-list.
Tests: the five collision cases above are new regression tests, each confirmed
failing against the previous commit. Also added Kotlin class-body closures (the
only case exercising the Method arm), Dart top-level `final`, Dart multi-name
locals, and a warm-parse-cache replay for Kotlin and Dart — the #2693 captures
are replayed verbatim, so a serialization change would surface only on a SECOND
analyze and every other test here runs cold. The previous negative tests were
vacuous: they paired names that did not collide (`maxSize` vs `size`), so the
pre-filter rejected them before the guard they were named after could run.
* docs(storage): fix the schema-version changelog blocks (#2693)
Two problems, one mine and one not.
MINE: the `INCREMENTAL_SCHEMA_VERSION` block is ASCENDING (v2 … v15), and I
inserted v16 above v15 rather than at the end — I had just moved the parse-cache
entry to the top of ITS block, which is descending, and applied the same habit
to a list ordered the other way. Moved to the end; both blocks are now
internally consistent.
NOT MINE: the parse-cache block carries TWO v21 entries, with v20 wedged between
them. Tracing it: #2632 (Spring DI facts) bumped 20 -> 21 and merged first;
#2653 (Java JLS local-class identities) had branched at 20, also bumped to 21,
and merged second — so it shipped with NO invalidation of its own. An index
already stamped 21 by the first change was treated as current by the second and
kept serving stale local-class identities from the warm cache.
Numbers left alone: both genuinely shipped as 21, and renumbering them now would
misstate what users' indexes actually contain. Instead the entry says so
explicitly, and points at the process fix — re-check the constant against
origin/main immediately before merging, not just when the branch is cut. The
identical collision hit INCREMENTAL_SCHEMA_VERSION in #2653/#2654, so this is a
recurring failure mode of concurrent PRs, not a one-off typo.
Comment-only; no constant changes value.
* feat(scope-resolution): resolve closure bindings in Ruby, Java, C#, PHP and JS/TS var (#2693)
Ruby, Java, C# and PHP already emitted correct callable-flow seeds and invokes.
What they lacked was the #2687 piece — a CALLABLE graph node at the binding,
which is what buildGraphTargetIndex joins to by position. PHP additionally had
no scope declaration for the bound name, so the flow pass had nothing to attach
its seed to.
ruby handler = ->(x) { x } handler.call(1) -> Function:a.rb:handler
java Function<..> handler = x->x handler.apply(1) -> Function:A.java:A.handler
csharp Func<int,int> handler = ... handler(1) -> Function:A.cs:A.handler
php $handler = fn($x) => $x $handler(1) -> Function:a.php:handler
Ruby and Java invoke through the callable-object protocol; C# and PHP call the
binding directly. Locals work in all four, and a binding whose name collides
with a same-named method resolves to the CLOSURE, not the method.
Two things the sweep caught:
JAVA TWIN. Anchoring the rule on the inner variable_declarator produced BOTH a
Function and a Property node — the exact double-indexing #2687 removed. The
parse-worker dedup keys on (definition node, name), and Java's value rule
anchors on field_declaration, so the keys never matched. Re-anchored on
field_declaration / local_variable_declaration.
JS/TS `var`. `var f = (x) => x` kept a Variable label while const/let got
Function, because `var` is a different grammar node (variable_declaration vs
lexical_declaration) that no closure rule covered. A call through the binding
still resolved via the declaration route, so the CALLS edge pointed at a
NON-callable node. Now consistent across const/let/var.
That last one flipped an existing assertion in const-function-twin.test.ts,
which expected `Variable` for a var-bound function-expression. Its comment
explained why — "var has no matching @definition.function pattern, so nothing
claims the name" — i.e. it documented the gap rather than defending it. The
property it was really protecting (an UNCLAIMED value node survives) now has
its own case with a non-function initializer, and the var-closure case asserts
the collapse to one node, which is also the twin guard for the new rule.
Known limits, both pre-existing and both failing safe:
- A PHP local closure whose name collides with a top-level function gets no
edge: both want id Function:<file>:<name>, so the closure never gets its own
node. This is the file-scoped node-identity convention — TypeScript, Python
and Dart collapse identically at base.
- TS/JS class-field arrows stay Property (Kotlin's equivalent emits Method).
They already resolve; changing the label risks the HAS_PROPERTY ownership
regression #2687 hit once.
The invalidation constants already bumped in this PR (INCREMENTAL_SCHEMA_VERSION
16, SCHEMA_BUMP 23) cover these additional languages; their notes now say so.
Tests: one case per newly-resolving language plus the PHP anonymous-function
form and the JS var form, in closure-binding-labels.test.ts. The file now spins
a worker pool per test across a dozen languages, so its timeout is raised
file-wide — a case that takes ~7s alone was exceeding the 30s default under
that contention.
* fix(ingestion): class-field closures are callable members in TS/JS (#2693)
A CALLS edge must target a callable node. `class A { handler = (x) => x }` emitted
a Property, so calling it produced `CALLS -> Property:A.ts:A.handler` — an edge
pointing at something the graph says is not callable. Same defect class as the
JS/TS `var` binding fixed in the previous commit, and the last place a closure
binding still carried a value label.
Kotlin already models its class-body closure as Method + HAS_METHOD; TS/JS now
match, so all three agree:
class-field closure -> Method + HAS_METHOD (CALLS target is callable)
plain class field -> Property + HAS_PROPERTY (unchanged, no CALLS)
Anchored on public_field_definition / field_definition — the same nodes the
property rules use — so the parse-worker dedup collapses the pair rather than
leaving a Method/Property twin, the failure the Java rule hit in the previous
commit.
ON MATCHING THE COMPILERS. This deliberately diverges from tsc and SCIP. The
TypeScript compiler classes `handler = () => {}` as a PropertyDeclaration
("a property declaration independently from what it's assigned to"), and SCIP
gives it a `.` term descriptor, the same suffix as any field — both call it a
property, and Kotlin's compiler likewise treats `val f = { }` as a property with
a function type. The divergence is intentional: GitNexus's Function/Method label
does not mean "tsc SymbolFlags", it means "this node can be the target of a
CALLS edge", which is the convention #2687 set for closure bindings in every
language. Modelling it the compiler's way would mean either dropping call
resolution for these members or emitting a separate node for the lambda and
flowing the property to it — the two-node shape #2687 removed. Recorded here so
the next reader does not "fix" it back.
Tests: TS and JS class-field arrows resolve to their Method node, plus a guard
that a NON-closure class field stays a Property — the closure rule must key on
the initializer, not on the field syntax.
* fix(php): keep the $ sigil on closure-binding nodes so locals stop colliding (#2693)
A PHP local closure whose name matched a file-level function got NO edge at all:
function save($x) { return $x; }
function run() {
$save = fn($x) => $x * 2;
return $save(1); // no CALLS edge
}
Both minted the id Function:<file>:save, so the closure's node was swallowed by
the function's and the positional join found nothing at the binding's line.
The fix is PHP's own semantics rather than a change to node identity across the
graph. PHP holds variables and functions in SEPARATE namespaces — $save and
save() cannot collide in the language — and the sigil is what separates them.
Dropping it was the bug. The node rule now captures the whole variable_name, so
the closure is Function:<file>:$save and the function stays Function:<file>:save.
languages/php/query.ts already keeps the sigil on property declarations for the
same reason, so this makes the two consistent.
The positional join normalises a leading $/@ on both sides, matching what the
scope layer and the callable-flow synthesizer already do, so the binding still
matches its own declaration while its NODE stays distinct.
local closure + same-named function -> Function:c.php:$save (the closure)
calling the real function -> Function:f.php:save (unchanged)
plain $max = 10 -> no node, no edge (unchanged)
WHAT THIS DOES NOT FIX. The general problem is wider than PHP: GitNexus node ids
are file-scoped, so a function-local symbol and a file-level one with the same
name collapse in TypeScript, Python and Dart too, and Java/C# only escape by
qualifying on the enclosing CLASS (so two same-named locals in different methods
still collide). SCIP solves it with a separate `local <id>` keyspace that is
document-scoped and never globally addressable. That is issue #2699 — it changes
persisted ids for every function-local symbol and needs its own invalidation, so
it is not bundled here. PHP is fixed on its own merits: the sigil belongs in the
identity regardless of how locals are eventually scoped.
* test(scope-resolution): pin the closure-binding caller-attribution limit (#2693)
Review of this PR found the new callable nodes are call TARGETS but never call
SOURCES: a call made INSIDE a closure binding is attributed to the enclosing
scope, so `impact(handler, direction:"downstream")` reports nothing even though
the closure calls out. Consistent across Kotlin, Dart, Ruby and PHP; TS/JS free
bindings are the exception because their arrow carries a @scope.function whose
range matches.
Not fixed here — pinned, so the boundary is visible instead of surprising, and
so a change in EITHER direction fails a test.
The cause is precise: `pickCallerCallableDef` (graph-bridge/ids.ts) finds the
caller by walking CHILD scopes whose range contains the call site, gated on
`child.kind === 'Function'`. A closure literal is a BLOCK scope in these
languages (Kotlin deliberately, #1757 smart casts), AND the binding's def is
owned by the enclosing scope rather than by the closure's scope — so neither
half of the link exists. Fixing it needs "callable boundary" decoupled from
scope `kind` plus an association between the closure scope and its binding.
That is a change to the caller anchor used by every call in the repo, which is
not something to land at the tail of this PR.
Also adds a unit suite for `buildGraphTargetIndex` itself, covering what the
integration tier cannot isolate: a binding is admitted only on POSITIONAL
evidence, a name-only match is rejected, a non-callable node at that position is
rejected, an ambiguous position claimed by two callables is rejected, and the
PHP dollar sigil normalises across the join while still not matching a
same-named function on another line. That last one closes the review's LOW —
the node/declaration name asymmetry now has an executable contract rather than
resting on a comment.
* docs(test): correct the per-language cause of the attribution limit (#2693)
The comment on the pinned attribution tests claimed "a closure literal is a
BLOCK scope in these languages". That is true for Kotlin (lambda_literal
@scope.block, #1757) and Ruby (do_block/block @scope.block) and FALSE for PHP:
anonymous_function and arrow_function are already @scope.function
(php/query.ts:61-62). Dart is a third case again — it has no scope over a
closure literal at all.
So the four languages fail at three different points, not one:
Kotlin, Ruby fail the `child.kind === 'Function'` gate
PHP passes that gate; its closure scope owns no callable def,
because the binding's def belongs to the enclosing scope
Dart has no child scope for the walk to consider
Worth correcting carefully rather than tidying: a follow-up plan re-stated this
comment instead of re-deriving it, and inherited the misdiagnosis — it proposed
"relax the kind gate" as required for all four, which is a no-op for PHP and
unreachable for Dart. A review caught it. The comment now states each language's
actual blocker and says why the distinction matters.
Comment-only; the three pinned tests are unchanged and still pass.
* fix(scope-resolution): an ordinary JS/TS `function` binds its own `this` (#2701)
`this.m()` inside a nested `function` resolved to the lexically enclosing
class, so it emitted a CALLS edge that does not exist at runtime — including
the exact `forEach(function () { this.m(); })` shape arrow functions were
introduced to avoid:
class D {
m() {}
build() { const h = function () { this.m(); }; return h; }
}
// CALLS: Function:D.ts:D.h -> Method:D.ts:D.m#0 FALSE
ECMA-262 gives an arrow `[[ThisMode]] = lexical`: it has no `this` binding in
its environment record, so the lookup passes through to the enclosing
environment. Every other function form binds `this` at call time. `tsc` draws
the same line by resolving `this` through `getThisContainer` with
`includeArrowFunctions = false`. That one rule is the whole fix.
Languages declare it; shared code never learns a language. The query files —
the one place that already names grammar nodes — tag every non-arrow function
form with `@receiver-owner.this`, which becomes `Scope.ownsReceivers`. A
receiver walk that reaches such a scope without finding the name stops there
instead of borrowing an enclosing scope's binding. Every other language leaves
the field unset and is bit-for-bit unchanged; a Kotlin lambda, which DOES
capture the enclosing `this`, still resolves (pinned as a test).
THREE GATES, ALL LOAD-BEARING. The false edge survived each one alone, which
is why the tests assert on the emitted edge rather than any single walk:
1. `Scope.ownsReceivers` stops BOTH receiver-type walks — `findReceiver
TypeBinding` here and its twin `lookupReceiverType` in gitnexus-shared's
`lookup-core`, which was resolving the receiver independently.
2. `LanguageTypeConfig.thisBoundaryNodeTypes` stops the type-env AST walk
that infers a receiver's type during capture.
3. `isReceiverOwnedButUnbound` makes `receiver-bound-calls` SUPPRESS the
site. Without it the member still resolved by NAME through `lookupCore`'s
lexical chain — the class-body scope binds `m` two scopes up — merely at
lower confidence. An owned-but-unbound receiver is a definitive negative,
not a miss, so it must not reach a receiver-blind fallback.
Also fixed: `function*(){}` as an expression was not a `@scope.function` at
all, so `this` inside one read as the enclosing method's.
WHAT THIS GIVES UP. The fix REMOVES edges, and some were correct:
`.bind(this)`, `.call(this)` and `forEach(fn, thisArg)` do make `this` the
instance at runtime. Their correctness is fixed at the CALL SITE, which no
scope-level rule can see, so the choice is between losing them and keeping
every detached-callback false positive. All three are pinned as tests
asserting the empty result, so changing the trade later is deliberate.
`this` in a static method also stops resolving to the INSTANCE member — that
edge was wrong in the other direction.
INVALIDATION. Both constants move, and the parse-cache one is not optional:
`ownsReceivers` lives on the cached `Scope`, and a warm cache replays scopes
without it — verified by probe that `--force` alone does NOT re-derive it, so
the fix silently did nothing until SCHEMA_BUMP moved. INCREMENTAL_SCHEMA_
VERSION 16 -> 17 (the incremental write set covers only changed files, so
unchanged TS/JS files would keep their fabricated `this` edges);
SCHEMA_BUMP 23 -> 24.
Verified against a built index, not by reading: all three false edges from the
issue gone, every correct edge kept, same result in JavaScript through its
separate grammar. 64 tests green across the new suite plus the closure-binding
and schema-version suites. The full suite's 36 failures are pre-existing
load-flakes — confirmed by A/B: `skip-git-cli` fails FOUR tests on a clean
HEAD versus three with this change, and `pipeline-pdg-streaming` passes in
isolation either way.
Refs #2701
* fix(ingestion): give function-local callables their own identity (#2699)
Graph node ids were file-scoped, so a local callable and a same-named
file-level one collapsed onto ONE node. That is a wrong answer, not a missing
one — the local call was attributed to the file-level symbol:
export function save(x) { return x; }
export function run() { const save = x => x * 2; return save(1); }
export function other() { const save = x => x * 3; return save(2); }
// ONE node Function:a.ts:save, and BOTH run and other pointed at it, so
// `impact` on the top-level save reported two callers that never call it.
A local's identity is now its enclosing-callable chain plus its own position —
`run.save@2:2`. The chain is for humans reading `impact`; the position is what
makes it correct. Names alone cannot express what ECMAScript actually
specifies, and the gap is the language's, not the grammar's: an environment
record is created per function AND per block, so an anonymous function has no
name to contribute and sibling blocks hold distinct bindings under the same
name. One positional rule settles both, with no conditionals and no
"disambiguate only when it looks ambiguous" heuristic — the ambiguity-flag
class of bug that bit #2514. SCIP reaches the same place with its
document-scoped `local <id>` keyspace.
Top-level functions and class methods are NOT locals and keep their ids
byte-for-byte. That is the bound on the churn: this touches only symbols that
are unreachable from outside their own document anyway.
RESOLUTION JOINS BY POSITION, NOT BY NAME. `resolveDefGraphId` matches a def
to its node on (file, label, line, simple name). A def and its node are the
same construct, so this needs no scope chain at all — which is the point:
re-deriving the chain in the resolver would be a second implementation that
could silently disagree with the first. A genuine tie (two callables on one
line) stores an AMBIGUOUS_POSITION tombstone and falls through to the existing
name keys rather than picking by source order. Without this the node ids were
already correct and calls STILL resolved to the file-level symbol — the fix is
only half a fix without it.
JS/TS GAIN BLOCK SCOPES. They emitted no `@scope.block` at all, so the
resolver could not tell two `const pick` in sibling branches apart. Giving
them distinct ids made that visible as DUPLICATE edges — each call resolving
to BOTH — which is worse than the collapse it replaced. `(statement_block)
@scope.block` supplies the missing environment record. The other half of the
ECMAScript rule was already implemented and waiting: `tsBindingScopeFor`
hoists `var` past blocks to the enclosing Function/Module while `let`/`const`
bind innermost, and its docblock already claimed "the innermost default covers
these" for block scopes that did not exist. All 82 scope-resolution test files
pass with blocks on.
Verified by probe, per case: two locals in different functions, a local inside
an ANONYMOUS function (`outer.fn@1:9.save@2:4`), sibling blocks resolving to
their own binding, `var` still hoisting out of its block, a nested named
`function` vs a file-level one, PHP composing with the `$` sigil from #2693,
and Python. Top-level/method ids unchanged, asserted directly.
Every assertion is on the EDGE, not on node existence. Ids are built twice and
independently — definition phase and caller attribution — and a one-character
disagreement makes the caller attach to a node that does not exist and the
edge vanish, with nothing thrown and no test failing. An edge assertion can
only pass if both phases agree.
INVALIDATION. INCREMENTAL_SCHEMA_VERSION 17 -> 18 and SCHEMA_BUMP 24 -> 25:
persisted node ids change for every function-local callable, and the cached
scope tree lacks block scopes. A top-up would leave unchanged files on the old
ids while changed files emit the new ones, splitting each symbol in two.
Bench fingerprint unchanged and both timing budgets pass. The one full-suite
failure (incremental-orchestration) passes in isolation — its log shows stale
init locks and WAL reclaim, i.e. LadybugDB contention under the parallel run.
Refs #2699
* perf(ingestion): emit block scopes only where they bind something (#2699)
Block scopes make `let`/`const` in sibling blocks distinct bindings, which is
what stopped a call in one branch resolving to both. Emitted naively — one
scope per `statement_block` — they also cost ~10% of analyze wall time, because
every scope-chain walk in every function then steps through levels that bind
nothing.
Two emit-side filters keep the semantics and drop the waste:
1. A block that IS a function body duplicates the enclosing Function scope.
Nothing can be declared between a function and its own body, so a binding
in either resolves identically — the inner scope is pure depth.
2. A block that declares no `let`/`const`/`class`/`function` binds nothing,
so it is transparent: a lookup finds nothing in it and walks to the
parent. `var` is deliberately excluded from that list — it hoists past the
block to the function, so a block containing only `var` still binds
nothing.
MEASURED, on a 762-file / 228k-line TypeScript corpus (gitnexus/src), min of 6
warmed reps with the cold first rep discarded:
block scopes emitted 19,389 -> 5,331 (-72%)
total scopes 35,942 -> 21,884 (-39%)
analyze wall time +9.8% -> +1.6-2.5% vs pre-#2699
peak RSS (whole tree) 2398MB -> 2434MB (+1.5%, inside run-to-run noise)
The filters themselves are free: scope emission over the same corpus measured
12.6s naive vs 12.5s filtered.
Wall-clock on a shared runner has a ±10% spread run to run, which is wider than
the effect being optimised, so the durable gate added here counts scopes
instead. `bench/scope-emission/measure.mjs --check` asserts an EXACT scope set
over a synthetic corpus that mixes the shapes the filters discriminate between
— function/method/arrow bodies, non-declaring if/else/for/while/try, blocks
that declare `const`, and a `var`-only block. Baseline is 2 block scopes per
module: only the two `if`/`else` branches that declare `const chosen`. If the
filters regress that number jumps immediately, in a way wall-clock CI could
never resolve from noise. Wired into the existing benchmarks job.
Behaviour is unchanged: 86 scope-resolution and identity test files, 1371
tests, all green — including the sibling-block case this could plausibly have
broken — and the callable-value-flow fingerprint is untouched.
Refs #2699
* test(bench): re-baseline the TS/JS scope-capture fingerprints for #2701
`bench/scope-capture` fingerprints the full capture set per language, and
#2701 added a `@receiver-owner.this` marker to every non-arrow function form
so a scope that BINDS its own `this` can terminate the receiver walk. That is
a capture-set change, so the TypeScript and JavaScript fingerprints moved and
the benchmarks job has been failing since that commit — I pushed it without
checking CI.
A fingerprint is a correctness gate, so this does not simply adopt the new
value. Verified first by diffing the capture-name HISTOGRAM over the same
fixture corpus against
|
||
|
Copilot
|
d3d4fa31bb
|
fix(scope-resolution): gate C#/Kotlin free calls by instance ownership (#2563) (#2654)
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
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* Initial plan * fix(scope-resolution): gate C# and Kotlin free calls * fix(scope-resolution): keep Kotlin ownership gate safe * Apply remaining changes * perf(scope-resolution): benchmark and cache ownership gates * test(scope-resolution): simplify benchmark scaling loop * refactor(scope-resolution): encapsulate ownership cache * test(scope-resolution): enforce subquadratic ownership scaling * fix(scope-resolution): address ownership review findings * test(csharp): regenerate capture golden for #2563 fixtures The committed expected-captures.json was missing the new NamespaceOwnerCollision.cs entry and carried a stale SameFileCases.cs digest/count (56 → 67), so csharp-captures-golden.test.ts was the sole red check on the PR. Regenerate with UPDATE_GOLDEN=1 to match the fixtures the bench fingerprint already reflects. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
f37c126f0c
|
Merge branch 'main' into dependabot/github_actions/softprops/action-gh-release-3.0.2 | ||
|
dependabot[bot]
|
e50c49949c
|
chore(deps): bump softprops/action-gh-release from 3.0.1 to 3.0.2
Bumps [softprops/action-gh-release](https://github.com/softprops/action-gh-release) from 3.0.1 to 3.0.2.
- [Release notes](https://github.com/softprops/action-gh-release/releases)
- [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md)
- [Commits](
|
||
|
dependabot[bot]
|
47f3932c8c
|
chore(deps): bump actions/setup-node from 6.4.0 to 7.0.0
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6.4.0 to 7.0.0.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](
|
||
|
Gergő Magyar
|
7f7255aef8
|
fix(analyze): load VECTOR before the incremental writeback touches embedding rows (#2623) (#2624)
* feat(lbug): add ensureEmbeddingRowDmlSafe VECTOR gate for embedding-row DML LadybugDB refuses every mutation of a table carrying an HNSW index while the VECTOR extension is not loaded on that connection: DELETE and CREATE raise a Binder exception, DROP TABLE is refused while the index references it, and SET segfaults the process. Dropping the index is not an available recovery either — CALL DROP_VECTOR_INDEX is itself a VECTOR-extension function and is undefined in exactly that state. Add a single primitive that loads VECTOR under the analyze install policy and, only when that fails, reads CALL SHOW_INDEXES (which works without the extension) to decide whether an index actually exists to trip over. No call sites yet. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the #2623 VECTOR gate for embedding-row DML Three cases: no index + VECTOR unavailable stays safe (no needless escalation); index present + VECTOR unavailable is reported blocked AND the raw deleteNodesForFiles genuinely throws 'extension is not loaded' (proving the hazard is real, not theoretical); index present + VECTOR loadable is safe, the delete works, and the HNSW index survives — the invariant run-analyze relies on when it keeps the index across a surgical incremental run. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): load VECTOR before the incremental writeback touches embedding rows Incremental analyze died on every content change once a repo had built code_embedding_idx: Analysis failed: Binder exception: Trying to delete from an index on table CodeEmbedding but its extension is not loaded. The surgical writeback's first statement is deleteNodesForFiles' CodeEmbedding join-delete, but nothing on that path loaded VECTOR until Phase 4 — so the engine refused the delete. This is an ordering defect, not an environment one: it reproduces on machines where VECTOR loads fine. The dirty-flag recovery then forced a full rebuild on the next run, which is why it read as 'just slow'. Call ensureEmbeddingRowDmlSafe() once, before the escalation gate and before any row is touched — the same 'index lifecycle before row DML' seam dropSearchFTSIndexes occupies for FTS (#2589). Unconditional, because a DB carrying the index from an earlier --embeddings run hits the same wall on a plain incremental run. When VECTOR truly cannot load the table is immutable (the index cannot be dropped without the extension either), so the run falls through to the existing wipe-and-COPY escalation with a message naming cause, consequence and remedy. Fixes #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): pin the #2623 VECTOR-before-embedding-DML ordering end-to-end Sibling of the #2589 FTS drop-before-delete suite, same shape: drive the real runFullAnalysis incremental path over a real git repo and a real LadybugDB, seed real embedding rows, build the HNSW index, then assert the index state at the exact moment deleteNodesForFiles is invoked. Both cases were confirmed to discriminate — with the run-analyze change reverted they fail with the reported 'Trying to delete from an index on table CodeEmbedding but its extension is not loaded', and pass with it: - surgical path: the run completes, the index is still present AND extension_loaded at delete time, exactly one row per nodeId survives, and the untouched file's rows are preserved - blocked path: with GITNEXUS_LBUG_EXTENSION_INSTALL=never the run escalates to a full DB write and says so, instead of crashing Also applies prettier's reindent to the run-analyze log ternary. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(lbug): cite the pinned LadybugDB version in the #2623 probe note The probe matrix behind ensureEmbeddingRowDmlSafe was first recorded on 0.18.0, but gitnexus/package-lock.json pins 0.18.2 (#2587). Re-ran every case on 0.18.2: refused DELETE, refused CREATE, SIGSEGV on SET, DROP_VECTOR_INDEX undefined, DROP TABLE refused, SHOW_INDEXES readable with extension_loaded intact. Identical on both, so the design is unchanged — only the citation was wrong. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): preserve embeddings across the VECTOR-blocked rebuild, and check the catalog before loading Three follow-ups from reviewing the fix itself. 1. Data loss on the blocked path. Escalating wipes the DB files, and Phase 3.5 restores embedding rows from cachedEmbeddings — which deriveEmbeddingMode only populates when meta.stats.embeddings > 0. A DB holding embedding rows that its meta does not account for therefore had every vector destroyed silently by a rebuild it never asked for. Probe on a 3-file repo: 3 rows before, 0 after, no warning. Read the rows before escalating (a plain MATCH, no extension needed) so the existing restore has something to restore, and say so in the log. The blocked-path test now asserts the seeded rows survive exactly once, and that assertion fails without this rescue. 2. Catalog before extension. ensureEmbeddingRowDmlSafe loaded VECTOR first and only read SHOW_INDEXES on failure, so every incremental analyze on a machine without VECTOR paid a bounded out-of-process INSTALL attempt plus an 'extension unavailable' warning — including repos that never built an embedding index and can never hit this bug. One local catalog read settles that case first; the load is attempted only when an index actually gates DML, or when the catalog cannot be read. 3. Dead branch. targetConn is always the module singleton there, so the isSharedSingletonConn ternary could never take its second arm. Collapsed to withConnLock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(doctor): live-probe the VECTOR extension instead of printing the static platform capability Review finding on #2624 (MEDIUM), and exactly what #2623's reporter hit: doctor printed 'VECTOR index: available' — derived from a static platform check — while every incremental analyze on the same machine was dying on an unloaded VECTOR extension. The FTS line was switched to a live LOAD probe for the identical contradiction under #2374; VECTOR now gets the same treatment. probeVectorExtensionLoad shares the FTS probe's implementation (bounded, offline-safe, never runs the installer) and doctor's semantic-mode line now follows the probe, not the platform: without a loadable extension the vector index can be neither built nor queried, so search really is on exact scan. The load-error classifier's remedies are label-parameterized so the VECTOR row stops dispensing FTS-specific advice — 'run analyze --repair-fts' repairs FTS indexes only and was actively wrong for a missing vector extension. Default label stays 'FTS'; every existing caller and pinned remedy string is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lbug): remove the stale Windows VECTOR gate — the extension ships for win_amd64 The codebase categorically refused VECTOR on Windows (platform !== 'win32' in isVectorExtensionSupportedByPlatform, plus a hard early-return in loadVectorExtension) on the strength of an early-era report that in-process INSTALL VECTOR could SIGSEGV (#1365). That belief is stale, verified directly: - the extension server hosts win_amd64 VECTOR artifacts for every 0.18.x extension version — v0.18.0 and v0.18.1 both serve a real 14 MB PE32+ DLL (curl-probed; 'file' confirms PE32+ x86-64) - the pinned 0.18.2 core resolves its extension directory to 0.18.1 (strace-verified LOAD open()), so the pinned version's Windows artifact exists too - INSTALL now runs in a spawned child (installDuckDbExtensionOutOfProcess), so even a crashing installer kills only the child and degrades to unavailable — the original hazard cannot reach the parent process any more Windows now takes the same runtime path as every other OS: try LOAD, install out-of-process when policy allows, degrade to exact scan when it truly fails. The MCP semantic-search lane loses its static platform gate too — it always attempts the vector index and falls back to the exact scan on runtime failure, with a once-per-backend diagnostic naming the real error instead of a platform-policy message. isVectorExtensionSupportedByPlatform is deleted; getRuntimeCapabilities reports the platform capability as available everywhere and defers machine truth to the live probe. Windows CI is the enforcement: the vector suites skip visibly only when the extension genuinely cannot load, so green Windows lanes now actually exercise VECTOR instead of silently skipping by policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the catalog-read-failure fallback in ensureEmbeddingRowDmlSafe Review finding on #2624 (LOW): the one branch where the gate cannot cheaply prove safety — SHOW_INDEXES itself erroring — was exercised only by inference. Force it with a Connection.prototype.query spy over the real DB: the catalog read fails, and the gate must fall through to actually attempting the extension load (asserted via the recorded statement stream) rather than guessing, returning true here because the extension is loadable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): load VECTOR on the pool's shared Database so the semantic vector lane actually works Review finding on #2624 (MEDIUM): extension load scope is per-Database (probe-verified — LOAD on one connection enables QUERY_VECTOR_INDEX on every connection of the same Database), and the pool pre-warm loaded only FTS. So LocalBackend's vector lane has ALWAYS raised 'Catalog exception: function QUERY_VECTOR_INDEX is not defined' through the pool and silently fallen back to the exact scan — repos above the 10k exact-scan cap got empty semantic results. The serve path was unaffected (the embedding pipeline loads the extension itself). Mirror the FTS line at BOTH load sites — doInitLbug's pre-warm and initLbugWithDb's external-Database adoption — under the same load-only contract (the read pool never triggers a network install), tracked by a new SharedDB.vectorLoaded flag reset where ftsLoaded resets. The new pool test is discriminating and deliberately closes the writable core adapter before the pool opens: a shared/injected Database would inherit the VECTOR load from test seeding and pass either way, so the case forces the pool onto its OWN fresh read-only Database where only the pre-warm can make the lane legal. Verified: fails at the pre-fix tree with the exact Catalog exception, passes with the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run the #2623 ordering suite on Windows/macOS and pre-install VECTOR alongside FTS Two review findings on #2624, both landing in existing seams: - scripts/cross-platform-tests.ts gains incremental-vector-extension-ordering .test.ts: the win32 VECTOR gate is gone in this PR, so the #2623 drop-ordering + blocked-path escalation must be proven on the windows-latest native addon, not just Ubuntu. (The review's claim that lbug-delete-nodes-for-files.test.ts was also missing was wrong — it has been on the roster since #2409.) - scripts/ensure-fts.ts now pre-installs VECTOR under the same best-effort auto-policy contract, so every sharded CI process LOADs from ~/.lbdb instead of racing its own bounded out-of-process INSTALL; the workflow's extension cache already covers it (path is the whole extension dir — key kept for cache continuity). The cross-platform job sets GITNEXUS_REQUIRE_VECTOR=1 beside GITNEXUS_REQUIRE_FTS so a genuinely unavailable VECTOR is a loud failure, never a silent skip. Windows/macOS cannot be executed locally; the PR's CI lanes are the proof for this commit. Linux smoke: ensure-fts.ts reports both extensions ready; all 79 roster entries resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(pool): register loadVectorExtension in the pool unit-suite mocks The pool adapter's new loadVectorExtension import surfaced in four suites that mock lbug-adapter.js with explicit factories (vitest fails loudly on a missing mocked export). Register the export in each — resolving false where the suite's world assumes no vector, true where it mirrors FTS — and extend lbug-pool-fts-load.test.ts, the suite that owns pre-warm extension loading, with the vector pair: successful load cached per shared Database, failed load retried on the next open, both pinned to policy load-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): use POSIX literals for graph paths in the #2623 ordering suite First Windows CI run of this suite (it joined the cross-platform roster this PR) failed with 'Parser exception: Invalid input <MATCH (n:Function) WHERE n.filePath = '>' — path.join produces backslashes on Windows, and a backslash inside the seed helper's single-quoted Cypher literal breaks the parser. The graph stores repo-relative filePaths with forward slashes on every OS, so graph-side paths are POSIX literals now (the incremental-orchestration convention); path.join stays only for real filesystem access. The same Windows lane also proved the substance this suite exists for: lbug-vector-extension passed 7/7 on windows-latest — the extension installed, loaded, and built a real HNSW index there — and the pool vector-lane and DML gate suites passed too. This commit fixes the harness, not the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
5549403082
|
fix(eval): self-hosted skill-evolution runner + sandbox Python 3 trust fix (#2600)
* fix(eval): move skill-evolution to a self-hosted runner and fix the sandbox's Python 3 trust gap GitHub-hosted runners hard-cap job execution at 6 hours, which is too short once a benchmark session actually invokes Skill/MCP tools for real (the --bare fix in #2584 means sessions no longer no-op). Move the job onto a self-hosted runner (5-day cap instead) and document the activation step in the workflow's own checklist. Validating the self-hosted run surfaced a real bug: gitnexus-plan sessions inside the bwrap sandbox failed with "planning must create or modify exactly one plan artifact; observed 0". Root cause: evidence-provenance.mjs's atomic plan-writer only trusts a Python 3 binary owned by root or by the current process. Inside this --unshare-user sandbox only the calling uid is mapped (root isn't), so the real, root-owned /usr/bin/python3 surfaces as the kernel's overflow uid and gets correctly refused as untrusted. Fix: provision a small, self-owned wrapper script (same pattern already used for shell-prefix) that execs the real interpreter, so the sandbox has a Python 3 candidate the existing trust check can actually accept -- without touching that security-sensitive validation logic at all. Also add visibility so this class of failure isn't quiet next time: report.md now shows why each row failed (error_kinds), not just resolved 0/1, and the benchmark now exits non-zero when an incumbent arm -- the currently-shipped skill -- resolves zero across every task, since that reads as a broken harness rather than a normal candidate miss. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(eval): close the broken_incumbent_arms zero-valid-runs gap; document runner exposure tradeoff Addresses the two MEDIUM findings from the gitnexus-review-agent on this PR (https://github.com/abhigyanpatwari/GitNexus/pull/2600#issuecomment-5033363096). broken_incumbent_arms required valid_runs > 0 before flagging an incumbent, so an incumbent that fails every run with an excluded-but-non-systemic error_kind (e.g. evidence-unverified, which the outage-streak breaker explicitly resets on rather than accumulates) never accumulated a single valid run and sailed through silently -- the exact quiet no-promotion outcome this guard exists to catch, and arguably worse than the some-runs-resolved-zero case since here nothing completed at all. aggregate() never marks an excluded/unverifiable row resolved=True, so dropping the valid_runs requirement and checking resolved == 0 alone correctly covers both cases. Added a test for exactly this all-excluded scenario, which none of the existing three did. Updated the workflow's own activation checklist to reflect what's actually true now (the gitnexus-evolution environment's branch policy and the self-hosted runner are both live, codified in infra/gitnexus-evolution/ in a companion PR) and documented the exposure-window tradeoff the review flagged: the runner is stopped between runs but not destroyed/recreated per run, so it isn't fully ephemeral. Stopping already bounds the exposure window to the job's own runtime on one day out of seven; full per-job ephemeral provisioning is a deliberate non-goal for a job that runs at most weekly, revisit if that changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(eval): remove public infra/ pointers from the activation checklist PR #2603 (the Terraform codification this checklist pointed to) got closed -- publishing the exact IAM roles, security group rules, and self-hosted runner topology for a real, live AWS account isn't safe to do in a public repo, even with no literal secrets or resource IDs in the diff. The underlying AWS/GitHub setup is unaffected and still documented privately; this just removes the now-dangling references to a directory that won't exist in this repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ci(actionlint): register the gitnexus-evolution self-hosted runner label actionlint rejected `runs-on: [self-hosted, linux, x64, gitnexus-evolution]` in gitnexus-skill-evolution.yml because it can't discover custom runner labels. Register it in .github/actionlint.yaml so the Workflow Lint check passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Claude
|
eea9ac92dc |
ci: move Node pins to the 22.18 floor
With the supported minimum raised to Node 22.18, retarget every lane and pinned runtime that sat at a lower version so nothing builds or runs the package on an unsupported (EBADENGINE-warning) Node: - ci-tests.yml: node-floor-compat 22.14 -> 22.18.0 (name, comment, pin, version assertion) so the floor gate guards the new minimum; its #2372 registerHooks failure mode cannot recur above 22.15. Containment-canary pin 22.16.0 -> 22.18.0. - gitnexus-review-agent.yml + the pinned review/canary runtime: the reproducible runtime is version-locked in lockstep across .github/{gitnexus-review-runtime,claude-canary-runtime}/package.json and their lockfiles (engines), the workflow's node-version, its two 'node --version = v22.18.0' assertions, the lockfile-engines guard, and NODE_VERSION. Moved all of them 22.16.0 -> 22.18.0. - gitnexus-skill-evolution.yml: pinned runtime 22.16.0 -> 22.18.0. - CONTRIBUTING.md prerequisite floor updated. - review-agent-workflow.test.ts, which enforces the runtime lock, updated to expect 22.18.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Abhigyan Patwari
|
2ea00a2b22
|
fix(ci): install root and shared node_modules for the evolution benchmark (#2575)
* fix(ci): install root and shared node_modules for the evolution benchmark The first real workflow_dispatch of the skill-evolution loop failed at task binding: capture_task_dependency_binding aborted with SandboxError: sandbox_copy path is unavailable: node_modules: No such file or directory The benchmark tasks sandbox-copy node_modules from three locations (tasks.scenarios.yaml) — the monorepo root, gitnexus-shared, and gitnexus — mirroring a full dev checkout. The install step only ran `npm ci` in gitnexus/, so the root and gitnexus-shared node_modules never existed and the loop died before any agent ran. Install all three (root, then build gitnexus-shared, then build gitnexus), matching the per-package install in ci-tests.yml plus the root deps the tasks require. A new contract test pins all three installs so this fails in CI rather than on the next real run — the same guard the workflow's other two P1 fixes got. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): only add the missing root install; the subpackage steps already exist The initial fix redundantly rebuilt gitnexus-shared and gitnexus inside the gitnexus step — but the workflow already builds both in their own dedicated steps. Only the monorepo root node_modules was missing. Add a single "Install monorepo root dependencies" step and leave the two subpackage build steps untouched, so the benchmark's root sandbox_copy resolves without double-building. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Gergő Magyar
|
fd1e0a999c
|
feat(ci): review agent runs as a coordinated reviewer swarm (#2572)
* feat(ci): review agent on Sonnet 5 with structured, linked reviews Bump the pinned review model from claude-sonnet-4-5-20250929 to claude-sonnet-5 (verified against the pinned Claude Code 2.1.214 with subscription auth and --json-schema structured output). Restructure the published review body: verdict-first summary, findings ordered by severity, fixed section order, and every file or symbol reference as a GitHub permalink pinned to the analyzed head SHA (or the merge-base SHA for deleted and rename-old paths) instead of bare path:line text, so references are clickable and render inline previews. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * feat(ci): review agent runs as a coordinated reviewer swarm Implement the review skill's expert-lens section in CI: the main agent spawns four trusted lanes in parallel via the Task tool — correctness, security, blast-radius, and coverage — each a purpose-built persona restricted to Read/Glob/Grep plus the read-only graph MCP tools. Personas live in the canonical skill tree (mirrored to all shipped copies) and are installed into the reviewer's user-scope agents dir from the exact control SHA, so a hostile PR head can never define a lane. Lane reports are treated as unverified claims: the main agent re-anchors findings before publishing, and the publisher's context-evidence gate still requires the main conversation's own successful context call. Bash and the newer Agent tool remain disallowed for every context; the analyze timeout gets swarm headroom (45 -> 60 minutes). The workflow contract test now pins the swarm posture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): sidechain tool calls can no longer satisfy the evidence gate The review agent's own review of this PR found that proveGraphReview() walked the flat transcript without reading parent_tool_use_id, so a spawned lane's context call could satisfy the publisher's graph-evidence gate the prompt reserves for the orchestrator. Entries with a non-null parent_tool_use_id are still strictly validated (malformed linkage fails the transcript) but are excluded from both candidate context calls and qualifying results; a new fixture proves sidechain-only evidence is rejected while mainline evidence beside sidechain turns still passes. Also gives the orchestrator turn headroom for the four dispatched lanes (--max-turns 100 -> 150), addressing the review's LOW finding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * refactor(skills): swarm-lane dispatch belongs to the review skill Move the lane orchestration out of the workflow prompt and into the gitnexus-review skill itself: a new "Swarm lanes" section names the four ci-persona lanes, defines when and how to dispatch them (parallel, one message, per-lane context and file slices), and owns the verification contract (lane reports are unverified claims; re-anchor, dedup, drop unanchored findings; lanes structure the work but never gate it). Any runner of the skill — the CI workflow or a local harness — now triggers the lanes from one canonical definition. The workflow prompt keeps only its CI-specific deltas: the lanes' trusted-control-SHA install provenance, the Task-tool dispatch surface, and the publisher's orchestrator-only context-evidence gate. Mirrors synced; 122 contract tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * feat(skills): add adversarial finder lane and critic gate to the swarm ci-adversarial-lens joins the parallel finder wave: it assumes the change is broken and constructs reachable failure scenarios — interleavings, hostile inputs, state corruption, abuse of newly exposed surfaces — each verified to a concrete entry point before it may be reported. ci-critic-lens runs last as a gate on the orchestrator's finished draft: it audits anchoring, concreteness, severity calibration, format conformance, and honesty, returning PASS or a numbered defect list with the smallest repair per item. The skill bounds it to two passes and the critic hardens the review without ever blocking it; the workflow inherits both lanes automatically through the wholesale ci-personas install. Mirrors synced across all three shipped trees; 122 contract tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * refactor(ci): workflow defers the whole swarm contract to the skill Now that the skill's Swarm lanes section owns dispatch, verification, the critic gate, and the fallbacks, the workflow prompt stops restating any of it. It contributes only what CI alone knows: the lanes' control-SHA install provenance, the concrete environment mapping for lane inputs (diff, manifest, head and merge-base checkouts, exact SHAs), and the one CI override — the publisher's context-evidence gate remains orchestrator-only. Analyze timeout gains headroom for the critic's sequential rounds (60 -> 75 minutes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): dispatch swarm lanes via the Agent tool, not the renamed Task alias On the pinned Claude Code 2.1.214 the subagent-dispatch tool is `Agent` (`Task` was renamed to `Agent` in 2.1.63 and is now a legacy alias), and permission rules evaluate deny before allow. The workflow allowed `Task` and denied `Agent`, so the orchestrator could never dispatch a lane and every review silently fell back to the inline single-agent path while the text-only tests certified the broken config. Use `Agent` consistently: add it to --tools, allow it scoped to the six ci-personas (`Agent(ci-correctness-lens,...,ci-critic-lens)`), remove it from --disallowedTools, and update the prompt. Tests now match the scoped allowlist on the raw string (commas inside Agent(...) break a split) and assert Agent is no longer bare-denied. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): harden swarm permissions — allow Glob/Grep + merge-base reads, quarantine PR-head agents Three permission-hygiene gaps around the swarm dispatch: - Glob/Grep were in --tools but had no allow rule, so the lanes' declared tools could manufacture denied-tool errors; allow them (read-only, sandboxed by cwd + add-dir). - The prompt hands lanes the merge-base source checkout for deleted / rename-old symbols, but no Read rule covered it; add a scoped Read() allow (which grants access without triggering --add-dir agent discovery). - The --add-dir PR-head copy is scanned for spawnable agent definitions and the pinned runtime has no suppression env, so a PR could ship its own .claude/agents/*.md. Drop that subtree from the materialized copy after checkout-index (skills left intact), so only the trusted control-SHA personas can ever be dispatched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * test(ci): pin the Agent allowlist to the ci-personas; require a dispatch canary A text-only assertion cannot prove the pinned CLI actually dispatches the lanes (print mode silently ignores invalid settings and does not validate Agent(type) content at parse time) — that is what let the original Task/Agent inversion pass CI. Two mitigations for the class: - A cross-consistency test asserts the six names in the Agent(...) allowlist equal the six ci-personas filenames and each persona's frontmatter name, so a rename or typo in any of the three fails without auth. - The activation checklist now requires the post-merge canary to prove a positive dispatch AND an unlisted-type refusal before enabling the trigger. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): bound swarm transcript volume with per-persona maxTurns The six lanes stream into the single execution transcript the publisher validates, but the personas carried no turn budget, so a large-PR swarm run could overflow the (hard-throw) transcript caps and brick a valid review. Bound each lane deterministically — finders maxTurns 12, the critic maxTurns 6 — which keeps the worst case (~2×(150+5×12+2×6) ≈ 444 messages) under the unchanged 1_000 cap, so no cap needs raising. A new test encodes that invariant: it fails if a persona's maxTurns is bumped without revisiting the cap. Applied byte-identically across all four shipped skill trees. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * test(ci): independently pin both sidechain evidence-gate guards The sidechain-exclusion guards at candidate registration and result acceptance were mutually redundant on realistic transcripts (a real sidechain turn carries parent_tool_use_id on both its call and result), so deleting either guard alone still passed the whole suite. Add two asymmetric cross-wired fixtures — a mainline call with a sidechain result (pins the acceptance guard) and a sidechain call with a mainline result (pins the registration guard), both expecting missing_graph_evidence. Mutation-verified: deleting either guard alone now reddens the suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * docs(skills): require own evidence before dispatch; document critic fail-open and swarm naming Strengthen the gitnexus-review "Swarm lanes" contract (all four mirrors): - The orchestrator must make its own graph context call on a changed symbol before dispatching any lane, so a fully-delegated run cannot leave the publisher's evidence gate unsatisfied (mirrored into the workflow prompt, with a test pinning the ordering phrase). - Document that the critic's fail-open is deliberate (bounded to two passes, cannot deadlock, review still gated by evidence + schema), and distinguish it from the hard lane-7 gate in the separate gitnexus-pr-swarm-review skill. - Give a concrete local-harness registration pointer for ci-personas. - Add a reciprocal cross-reference in gitnexus-pr-swarm-review (single path — that skill is not part of the mirrored family). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * docs: record the review-agent swarm capability (AGENTS.md, CLAUDE.md, reviewer-swarm README) Reflect the shipped swarm in the standing docs: bump AGENTS.md to 1.14.0 and CLAUDE.md to 1.8.0 with changelog rows, extend the gitnexus-review description to mention the ci-personas swarm lanes, and refresh the reviewer-swarm README so its differentiator names the real distinction (interactive on-demand swarm vs the CI review agent's in-workflow lanes) now that both run swarms. No CHANGELOG.md edit (feature-PR rule). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): close pre-push review findings on the swarm permission change Adversarial review of the fix diff caught two issues introduced by the permission-hygiene commit: - Bare Glob/Grep in --allowedTools are separate tools that the Read()-scoped path denies (/proc, github.workspace, ...) do not cover, opening an undenied read path to the raw checkouts and host paths via a prompt- injected lane. Drop the bare allow — under dontAsk they stay denied by omission; lanes read via the scoped Read() rules and the graph MCP. - The agents quarantine removed only the add-dir root's .claude/agents; make it recursive so a nested (e.g. monorepo subpackage) .claude/agents cannot survive and be discovered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * feat(ci): post an "in progress" marker while the review swarm runs Swarm reviews can take up to 75 minutes, and until now the PR showed no sign a review was running. Add a dedicated write-scoped `acknowledge` job that, under the same authorization gate as analyze, upserts a per-PR "🔄 GitNexus review in progress" sticky comment linking to the live run (and reacts 👀 to the trigger comment); the publisher removes that marker when the review — or a clean failure — posts. The marker lives in its own job so the model-facing analyze job stays secretless and read-only: it cannot post to the PR, so per-lane live progress isn't exposed there — the marker is a binary "running" state with a link to the Actions run where lane-by-lane progress is visible. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Gergő Magyar
|
becac9a5d3
|
feat(eval): run the skill-evolution loop online (#2571)
* feat(eval): run the skill-evolution loop online Add a scheduled + dispatch-gated workflow that runs the offline propose -> benchmark -> gate loop (workflow_bench.evolve) in CI with the pinned Claude canary runtime and bubblewrap containment, uploads the benchmark evidence as an artifact, and on a gate-passed promotion opens a human-reviewed PR via the release App token. The applied overlay is bounded to the canonical skill tree and its shipped mirrors; any escape fails the run instead of reaching a PR. The scheduled lane ships disabled behind GITNEXUS_EVOLUTION_ENABLED and requires the new GITNEXUS_BENCH_AUTH_TOKEN secret (benchmark sessions bill real API usage), mirroring the review agent's staged rollout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): restructure promotion-PR script so no lint suppression is needed Replace the inline single-quoted credential helper with a GIT_ASKPASS file written via a quoted heredoc (the App token still reaches git only through step env at push time), and assemble the PR body from quoted heredocs plus double-quoted printf instead of a backtick-laden single-quoted template. Every run script in the workflow now passes shellcheck with zero findings and zero disables; the body and askpass rendering are smoke-tested. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): apply gate-passing overlays in the evolution loop The loop invoked workflow_bench.evolve without --apply, so apply_promoted_overlay (its only working-tree writer, gated by `if args.apply:`) never ran. git status stayed clean, promoted=false was emitted every run, and the App-token/PR-open steps were unreachable dead code — a gate-passing run went green as "No promotion this run". validate_promotion_for_apply already runs before the apply gate, so adding --apply lets a passing candidate reach the tree without weakening the deterministic gate; the boundary check then confirms it stayed in the skill trees. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): provision ~/GitNexus so the benchmark repo resolves on CI Every scenario in tasks.scenarios.yaml addresses the target repo as ~/GitNexus; runner_tasks.py resolves it with expanduser().resolve() then `git -C <repo> rev-parse`, which raises when the path is missing. On a hosted runner the checkout lands in $GITHUB_WORKSPACE and nothing created ~/GitNexus, so the first real run failed at task-binding. Symlink ~/GitNexus -> $GITHUB_WORKSPACE before the loop. The checkout uses fetch-depth: 0 (full history for the parentless clone), and the benchmark only clones the repo copy-on-write and mounts deps read-only, so the checkout is never mutated. GITNEXUS_BENCH_ORACLE_ROOT stays unset — it defaults to the in-repo oracles dir and is staged by the harness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): harden promotion summary output and PR branch recovery Three fixes to the promotion-detection and PR-open steps: - GITHUB_OUTPUT summary used a fixed `PROMOTION_EOF` heredoc delimiter; a value containing that marker on its own line could close the block early and inject output keys. Use a per-run random delimiter, matching the pattern already in tree-sitter-upgrade-readiness.yml. - The summary concatenated every generation's promotion.json (including rejected ones), so the PR body could show a losing generation's decisions. The loop returns on the first promotion, so emit only the highest-numbered gen-N/bench/promotion.json — the decision that fired. - The promotion branch name omitted the run attempt. GITHUB_RUN_ID is stable across re-runs, so a re-run after push-succeeds/PR-create-fails could never push. Include ${GITHUB_RUN_ATTEMPT} (the artifact name already does). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): least-privilege the promotion App token and gate on an Environment The Mint-App-Token step passed only app-id + private-key, so the minted token inherited every permission the Release App installation holds (including Workflows: write) — far more than "push a branch, open a PR". Switch to `client-id` (as publish.yml does) and request only permission-contents: write + permission-pull-requests: write. Bind the job to a protected Environment (gitnexus-evolution) so promotion runs can be gated server-side. workflow_dispatch runs the workflow and in-tree evolve.py from the *dispatched ref*, so a code-side ref guard is removable by the dispatched branch itself; an Environment deployment-branch rule (main only) is the boundary that holds. The admin steps to create it and scope the secrets are documented in the activation checklist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(ci): correct upload-artifact pin comment and add shell strict-mode - The upload-artifact SHA 043fb46d… is v7.0.1 (labeled so in the sibling workflows that pin it); the comment mislabeled it # v6.0.0. Correct the comment; the pin is unchanged. - Add `set -euo pipefail` to the two build steps that lacked it, matching every other run block in the file (GitHub's default shell already sets -eo pipefail; this adds -u and consistency). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * docs(ci): complete the skill-evolution activation checklist - Add RELEASE_APP_ID / RELEASE_APP_PRIVATE_KEY to the required-secrets checklist (the Mint step hard-fails without them on a promotion) and the App-install-scope verification. - Document the protected Environment admin step and why it is the real boundary for the workflow_dispatch ref-secret exposure. - Note that workflow_dispatch runs the billing loop regardless of GITNEXUS_EVOLUTION_ENABLED. - Justify the weekly cron against the README's ~90-day guidance and note the 355-minute timeout ceiling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * fix(eval): redact API tokens from diagnostic fields before artifact upload results.jsonl (runner.py) and proposer-session.json (evolve.py) serialize session records whose error_detail can carry a stderr_tail that echoed the API key. Transcripts are redacted before persistence, but these two sinks were not, and both land in the 14-day evolution artifact. Run each record's serialized JSON through the existing redact_text with the run's auth token before writing. Scoped to these diagnostic sinks only: the promoted overlay and proposal.md are left untouched (the overlay is the applied artifact and must stay byte-identical for apply and the shipped-skills-sync guard). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * test(ci): add a contract test for the skill-evolution workflow No test exercised this workflow's path, which is why both P1 blockers (missing --apply, unresolvable ~/GitNexus task repo) reached production. Parse the workflow YAML and assert the structural contract: --apply is passed, the task repo is provisioned, the promotion branch carries the run attempt, the App token is permission-scoped and the job is Environment- gated, the output summary uses a random delimiter and a single generation, the artifact pin is labelled correctly, and every multi-line shell step sets strict mode. Follows the review-agent-workflow.test.ts precedent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * feat(ci): run the proposer on its own (stronger) model One `model` input drove both the benchmark arms and the proposer/diagnosis session. Split them: `model` stays the benchmark arms (match the model your skill users run, so a promotion is valid for them and the tasks aren't ceiling-saturated), and a new `proposer_model` input runs the proposer — the harder meta-reasoning task that writes the candidate skill, and only one session per generation, so a stronger model is cheap here. evolve.py already supports --proposer-model; the workflow just didn't expose it. Defaults: arms = claude-sonnet-5, proposer = claude-opus-4-8 (both overridable via workflow_dispatch). The weekly cadence bounds the added spend. Contract test asserts the split stays wired. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Gergő Magyar
|
94a528f577
|
feat(ci): review agent on Sonnet 5 with structured, linked reviews (#2570)
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
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Skill copy sync / shipped skills drift guard (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
Bump the pinned review model from claude-sonnet-4-5-20250929 to claude-sonnet-5 (verified against the pinned Claude Code 2.1.214 with subscription auth and --json-schema structured output). Restructure the published review body: verdict-first summary, findings ordered by severity, fixed section order, and every file or symbol reference as a GitHub permalink pinned to the analyzed head SHA (or the merge-base SHA for deleted and rename-old paths) instead of bare path:line text, so references are clickable and render inline previews. Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Gergő Magyar
|
b3826d6b0e
|
fix(ci): unblock the review agent dispatch and publisher lanes (#2567)
* fix(ci): unblock the review agent dispatch and publisher lanes The first workflow_dispatch validation run surfaced two defects: - setup-node rejects `cache: false` (the YAML boolean arrives as the string 'false' and v6 fails with "Caching for 'false' is not supported"), killing the analyze job before the isolation preflight. Omitting the input is the supported way to disable caching. - The publisher held only `issues: write`, but GITHUB_TOKEN needs `pull-requests: write` to create issue comments on a pull request, so even the safe-failure comment died with "Resource not accessible by integration". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ * test(ci): align the publisher permission contract with PR commenting The workflow contract test pinned the publisher to pull-requests: read, which is exactly the permission set that made comment publication fail. Encode the corrected scope and assert the publisher still cannot write repository contents. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Va5uu9Ar3e45QZ5xFsG4AZ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Gergő Magyar
|
8b5057f325
|
feat(skills): GitNexus Engineering Tool Kits (#2566)
* feat(skills): add ce-plan — GitNexus+PDG implementation-planning skill Adds .claude/skills/ce-plan: a planning-only skill that builds implementation-ready plans from GitNexus graph navigation (query/context/ impact/trace), bounded statement-level PDG slices (pdg_query, impact mode:pdg, explain), and targeted source verification, with a context ledger to prevent repeated reads and a machine-readable implementation context pack (stable contract for a future ce-implement). Whitelisted in .gitignore and registered in AGENTS.md and CLAUDE.md outside the auto-managed gitnexus block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(skills): apply ce-plan validation findings (tool contract, consistency, conventions) Tool contract: impact mode:'pdg' shape now includes the schema-required direction param; CDG branch sense documented as the result 'label' field (reason is cypher/raw-edge only); explain caveats corrected to its real false-negative classes (cross-function TAINT_PATH is modeled). Consistency: PDG slice homed in working memory (ledger keeps one-liners); depth knob defined and category-overrides-baseline ordering stated; call_depth (consumed by nothing) and content-hash bookkeeping dropped; Never section folded into Hard rules; Phase 3 deduplicated to a pointer; allowed-repeat escalations defined; budget/discard accounting clarified; verification-commands gathering added to Phase 4; open_questions added to the context pack. From scenario runs: plans now pin the verified-at HEAD commit and index freshness in a header, tag claims [verified]/[graph]/[inferred]/[assumed], quote load-bearing tool output, prefer pre-hook-carrying npm scripts, and support an out:<path> destination override; output path defined as the Phase 1 target repo root. Conventions: AGENTS.md 1.9.0 / CLAUDE.md 1.4.0 changelog rows + metadata bumps; future ce-implement qualified as future. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): rename ce-plan → gitnexus-plan; add cross-CLI (Codex) entrypoints Renames the skill dir, frontmatter, output filename convention, plan H1 (GitNexus Engineering Plan), the future executor handle (gitnexus-implement), the .gitignore whitelist entry, and all AGENTS.md/CLAUDE.md references. Follows the pr-swarm-review cross-CLI pattern: SKILL.md is the canonical CLI-neutral spec, AGENTS.md § Engineering planning is the Codex/any-agent entrypoint, and the README documents the optional user-level ~/.codex/prompts/gitnexus-plan.md slash command plus an invocation matrix. Skill prose de-branded from Claude Code (agent-neutral verification layer). Also fixes two post-review README contradictions: the anti-reread claim now names the ledger's allowed escalations, and 'read-only by contract' is now 'planning-only' (the skill writes exactly one repo file — the plan); the scope-creep rule and template §12 now agree on where deferred follow-ups land. Drops the stale plugin-collision limitation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(skills): document Codex user-level install path for gitnexus-plan Codex discovers SKILL.md skills from ~/.agents/skills (same path the other gitnexus-* skills install to); README now documents the cp install plus the optional ~/.codex/prompts slash-command file, with the prompt body preferring the repo copy and falling back to the user-level install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): gitnexus-plan freshness gate + active PDG-layer refresh Freshness is now a Phase 1 gate, not advisory: under the default freshness:strict, a stale index is refreshed once per planning session via node .gitnexus/run.cjs analyze --index-only (appending --pdg when the task will reach the PDG phase), then the context resource is re-read. A missing PDG layer likewise triggers the one permitted --index-only --pdg refresh and re-probe instead of a passive recommendation. freshness:accept (or a failed/impractical refresh) preserves the old behavior: plan on the stale graph, source-weighted, labelled in the plan header. --index-only is the load-bearing flag choice — it suppresses all file generation, so the planning-only contract holds (only the .gitnexus store changes). Ledger gains an index_refresh record; plan header states fresh / refreshed / refresh-skipped-with-reason. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): gitnexus-plan runner build check before freshness refresh When the target repo builds the analyzer from its own source (bin → dist/ mapping, as gitnexus/ does), the Phase 1 freshness gate now verifies dist/ is current before running the analyze refresh — rebuilding via the package's build script when any analyzer source file is newer than the built entrypoint — and prefers that freshly built CLI. Otherwise a stale dist re-indexes with outdated extraction logic and the 'fresh' index lies. Rebuilds are recorded in the ledger's index_refresh; the PDG-phase refresh inherits the same check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): add gitnexus-work executor and gitnexus-lfg pipeline gitnexus-work executes a gitnexus-plan as verified atomic commits: consumes the §11 implementation_context pack, drift-checks the plan's evidence pin against HEAD, re-verifies assumptions before relying on them, runs impact before every symbol edit and detect_changes before every commit (repo mandates), builds tests from the plan's scenarios, and routes structural drift back to gitnexus-plan Deepen mode instead of coding around it. gitnexus-lfg is a thin orchestrator: gitnexus-plan → blocking user gate (deepen / proceed / stop, deepen loops allowed) → gitnexus-work → review via the existing gitnexus-pr-review skill (open PR, else branch diff vs default). One bounded fix cycle for review findings; never pushes or opens a PR on its own. gitnexus-plan gains a Deepen mode (re-run freshness gate, escalate to depth:deep, re-verify graph/inferred/assumed claims toward verified, rewrite the same file); its 'future gitnexus-implement' placeholder is retired in favor of gitnexus-work. Registered via .gitignore whitelists, AGENTS.md 1.10.0 (section renamed to Engineering planning & execution), CLAUDE.md 1.5.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(skills): apply cross-skill review findings to the gitnexus skill family Two P1s: gitnexus-plan Deepen mode now re-anchors before re-pinning (diffs the old evidence pin over every [verified]-claim file and re-reads or downgrades before the header moves — moving the pin without this laundered stale claims as verified); the index-refresh budget is stated once in Phase 1 (one --index-only refresh plus at most one Phase 3 --pdg upgrade per session, Deepen = its own session) with ledger and pdg-slice deferring to it. Contract fixes: gitnexus-work's drift check now covers every file the pack cites (not just files_to_modify) and parses the full pack incl. primary/related symbols and acceptance_criteria (walked in Phase 4 alongside §13); a pre-completed check skips §7 steps already landed and Deepen gains a reconcile-execution-state step, closing the mid-execution route-back loop; pack assumptions must name what to check and how. lfg: Lane 4 passes the merge-base to detect_changes compare (two-dot diff misattributes upstream commits when default advanced), branch-diff is the stated normal case, oversized review findings route to the plan gate instead of overflowing direct mode, the one-fix-cycle cap is explicit on re-run, and headless runs end at the plan gate with the plan as deliverable. work: blank mode narrowed to *gitnexus-plan*.md with a re-execution guard, direct-mode discipline spelled out, branch meaningfulness defined against the plan slug, and the plan document is committed as the branch's docs commit (review diff includes it). Planning-only contract now names the dist/ rebuild as the second permitted state change; Phase 5.1 names the four claim tags; stale AGENTS.md anchors fixed. Known latent issue left untouched: gitnexus/gitnexus-pr-review pairs a three-dot example with a two-dot detect_changes compare — that skill is also shipped by the plugin, so fixing it here would drift the copies; lfg compensates by passing the merge-base. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): ship the engineering skill family with the gitnexus package npm i -g gitnexus users now get gitnexus-plan / gitnexus-work / gitnexus-lfg: the three skills are added to gitnexus/skills/ in directory form (SKILL.md + references/), which installSkillsTo already enumerates dynamically and copies recursively to every editor target (~/.agents/skills for Codex, Cursor, OpenCode, Qoder, ...) on gitnexus setup — uninstall enumerates the same root, so removal stays clean. The Claude Code plugin channel (gitnexus-claude-plugin/skills/) carries the same copies plus the standard per-skill mcp.json. Global-install support in the skill text: gitnexus-plan Phase 1 now resolves the analyzer runner explicitly — node .gitnexus/run.cjs analyze when the project has a runner, else gitnexus analyze (installed CLI), else npx gitnexus analyze — and all analyze mentions route through it, satisfying the skills-steering policy (#1939/#1945) which sweeps the plugin copies. New drift guard test/unit/shipped-skills-sync.test.ts asserts the npm and plugin copies stay byte-identical to the canonical .claude/skills/ family (plugin = canonical + mcp.json), same discipline as run.cjs ↔ resolve-invocation.ts. skills-steering + shipped-skills-sync: 11/11 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): workflow_bench — measure the skill workflow's token savings Benchmarks gitnexus-plan → gitnexus-work against a baseline agent (--disallowedTools Skill) on identical tasks, in fresh detached worktrees, using real headless Claude Code sessions; every number comes from the CLI's --output-format json usage report (field names validated against a live 2.1.207 session). Reports per-arm medians (input/cache/output tokens, cost, wall time, turns), a savings row, and resolve status from a per-task verify command — savings on failed tasks are flagged, not celebrated. Per-task setup hook prepares fresh worktrees (deps); --permission-mode bypassPermissions (default) lets sessions run unattended in the throwaway trees. Free-model support: --base-url/--auth-token/--model route headless sessions through any Anthropic-compatible endpoint; free-model.litellm.yaml is a ready litellm-proxy template for OpenRouter :free variants or local Ollama, so benchmarking burns no paid tokens (README documents rate limits and the small-model skill-following caveat). Harness validated end-to-end with a stub CLI (worktree lifecycle, both arms, plan→work chaining, verify, aggregation, report) and 4 pytest units for the pure aggregation/savings/report helpers. AGENTS.md 1.11.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): record first workflow_bench calibration run Trivial-task calibration (add -V alias): both arms resolved; workflow arm ~4.3x baseline cost — the documented overhead-dominated regime, recorded so the regime boundary is empirical rather than asserted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): workflow_bench scenario matrix — arm variants, task classes, churn Ground-base measurement across scenarios: tasks.scenarios.yaml spans four labeled classes (trivial → investigation-bug → investigation-feature → cross-module) with deterministic verifies (prescribed test files). New arms: workflow_direct (gitnexus-work direct mode — the middle option that locates the routing boundary lfg's gate and work's triage encode) and baseline_nomcp (no skills AND no graph tools — separates workflow-discipline value from GitNexus-tool value; off by default). Records now carry task class and diff churn (files/+ins/−del vs the starting commit) as an over-engineering proxy; the report renders a class column and per-arm savings rows vs baseline. 5 pytest units + stub-CLI e2e of the full three-arm matrix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): record workflow_bench ground base; fix churn measurement bias Ground base (3 classes x 3 arms, n=1/cell): every arm resolved every task — pass/fail quality saturates at this difficulty, making the comparison pure cost. Full plan→work never amortized its ~$9-11 fixed cost on tasks a baseline finishes in ≤35 turns (−211% to −333% cost); workflow_direct sits near baseline (−15% to −55%, once faster wall) with more test coverage. Routing implication recorded: direct mode/plain agent below this scale, full workflow for cross-module / multi-session / plan-as-deliverable work. The cross-module cell and multi-run variance are the next measurements. Churn fix: git add --intent-to-add -A before diffing (arms that never commit no longer undercount new files) and :(exclude)docs/plans (the committed plan doc no longer inflates workflow churn); this run's churn numbers predate the fix and are omitted from the recorded table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(skills): cost-optimize the workflow from measured ground base Every optimization targets a measured fixed-cost component (eval/workflow_bench ground base: workflow arm −211% to −333% vs baseline, all tasks resolved): - Plan form is category-priced: compact form (core sections w/ § anchors preserved, ≤80 lines excl. pack, mini-pack subset of the context pack) for narrow/default categories; the full 13 sections only for deep work (refactor/security/performance/concurrency/architecture). A compact plan outgrowing its cap reclassifies to full rather than overflowing. - Freshness gate is category-priced: compact categories default to accept (source-weighted, refresh only when a graph claim becomes load-bearing); strict stays the default for full-plan categories — the rebuild+re-index was the largest single fixed cost. - Turn economy: per-category tool-call budgets (~10 to ~45; architecture uncapped); budget exhaustion routes open questions to §12 instead of more digging. - gitnexus-work fast path: HEAD == evidence pin → skip all citation re-reading (the pin's entire point); mini-pack fields tolerated. - lfg Lane 1 boundary triage: tasks below the measured ~35-turn boundary get offered gitnexus-work direct mode before the plan lane is spent. Copies re-synced (npm skills/, plugin, ~/.agents); steering + sync guards green. Re-measurement of the workflow arm follows to verify the numbers actually improve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): record optimization re-measurement — inv-bug workflow cell −20% cost Same task, same conditions, post-830a0459 skills: $14.56→$11.70 (−20%), 83→72 turns, cache_read −24%; verified in-transcript that the compact form, turn budget, and skipped rebuild/re-index all fired. Wall +15% from a work- session test-debugging tail (n=1 variance). Regime unchanged (~3.5x baseline on this class) — routing rule stands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(eval): per-arm clone isolation — worktree ref-namespace leak contaminated an arm The cross-module workflow_direct cell reported an impossible 28-turn solve with churn byte-identical to the workflow arm: git worktree add shares the repo's ref namespace, so the workflow arm's slug branch (created by gitnexus-work Phase 2) survived worktree removal and the direct arm found and adopted the completed work. Arms now get isolated git clone --shared copies (object store via alternates, refs clone-local — agent branches and stashes die with the clone; origin/<ref> fallback for non-default refs). Leaked branch deleted; baseline arm verified clean (0 branch references in its transcript); cell marked invalidated pending re-run. Records the valid cross-module cells: workflow $18.32 vs baseline $18.03 (premium −1.6%, vs −211%..−333% on smaller classes) — fixed costs amortize at this scale, with a less destructive diff and a plan artifact as bonus; resolve rate still tied. Churn fingerprinting is what caught the contamination — noted in the README as an integrity check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): complete cross-module cell — direct mode wins 47% cost / 56% wall Clean clone-isolated re-run: workflow_direct resolved the hardest class at $9.53/52 turns/15m vs $18.03/98/34m baseline and $18.32/107/37m full workflow. The measured story across all four classes: the execution discipline (gitnexus-work) is the consistent sweet spot and delivers real token savings on hard tasks; the planning pass buys its artifact, not same-session savings. Resolve rate tied everywhere (n=1/cell caveat). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): add trajectory-gated skill evolution (#2431) - Pair prompt candidates with incumbent workflow arms - Gate promotions on pinned-model quality and efficiency - Expire router evidence and document its lifecycle * fix(eval): allow pr-review skill candidates * feat(skills): rename and generalize GitNexus review * feat(eval): external-comparator and review arms for workflow_bench - ce_workflow / ce_workflow_direct: compound-engineering ce-plan/ce-work arms prompted with the same structure as the gitnexus arms - review / ce_review: gitnexus-review vs ce-code-review on an identical diff applied by the task's setup - plan handoff is snapshot-based: committed example plans in docs/plans/ tie on clone mtimes and broke the name-glob pick (executed a stale plan) - verify output tail is recorded per run and the final working-tree patch is kept, so failed rows are diagnosable after the clone is destroyed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(skills,eval): address #2431 review — data-safe rename migration, fail-closed bench evidence - setup: never delete a legacy renamed skill dir — the installer cannot prove ownership (users customize or hand-write skills under these names); warn with the path instead, and the test now asserts survival - workflow_bench: fail closed when a session's --output-format json report is empty, malformed, or missing usage fields — an exit-0 shell with no parseable usage no longer counts as measured evidence (5 parametrized regression tests) - workflow_bench: document the trust model prominently (task setup/verify are shell-executed, sessions run bypassPermissions with the parent env, candidate overlays are prompt injection surface) in README + docstring - free-model.litellm.yaml: master_key from LITELLM_MASTER_KEY env instead of a static token; loopback-binding warning - ci: run the eval workflow_bench pytest suite on ubuntu (pytest+pyyaml only — no full eval stack) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(eval): demand observed foreground verification in headless work-arm prompts In a headless -p session there is no later turn: a work arm backgrounded its slow test run, scheduled wakeups that can never fire, and reported done while two of its tests failed. All four work-arm prompts (both skill families, symmetric) now require verification output to be observed inside the session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): ask plan depth up front instead of offering deepen afterwards gitnexus-plan Phase 0 now asks one blocking question in interactive sessions — quick / standard / deep, mapped onto the existing depth/form/ freshness knobs — when the invocation carries no explicit depth signal. Explicit knobs and headless runs skip the question (category posture unchanged, so benchmarks and automation behave as before). gitnexus-lfg's plan gate slims to proceed/stop: depth was already the user's up-front choice, so deepening is no longer offered by default — an explicit deepen request at the gate and executor route-backs still run Deepen mode, which remains the mechanism for strengthening an existing plan document. All shipped copies resynced (npm skills/, Claude plugin); AGENTS.md 1.13.0 and CLAUDE.md 1.7.0 pointers updated, including the analyzer's regenerated index-stats block at this branch's head. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): taint pass, expert lenses, and post-work index refresh gitnexus-review gains a PDG-backed taint-and-dependence pass (explain + pdg_query, --pdg folded into the stale refresh on trust-boundary diffs) and an Expert lenses section: domain reviewers derived from the graph's clusters plus four cross-cutting lenses (architectural fit, language conformance per the repo's own contract, Definition of Done, simplicity), dispatched once after the evidence-gathering steps and scaled to the diff. gitnexus-work Phase 4 now refreshes the knowledge graph after the DoD walk via the resolved-runner ladder with analyze --index-only, so the lfg review lane and later sessions query the finished work without dirtying the tree. lfg's threshold-governance paragraph moves to its README; eval citations are tagged as measured in the GitNexus repo. All shipped copies re-synced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): remove legacy gitnexus-pr-review on uninstall; cover the rename migration uninstall's removal set now includes LEGACY_SKILL_DIR_NAMES derived from RENAMED_SKILL_DIRS, so a pre-rename install is cleaned up instead of orphaned. The rename warning gains behavioral coverage (fires with a legacy dir present, silent without), and shipped-skills-sync asserts legacy names stay absent from every shipped tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(eval): metric provenance, error-kind rows, skill-invocation verification, gate noise floor The promotion gate defaults to cost_usd (the only metric that includes subagent spend); token metrics carry an explicit main-loop-only warning in the report and promotion.json. Rows are classified by error_kind (session-error / verify-failed / infra-error), excluded from efficiency medians, and the gate requires equal valid-run counts. Each session's transcript is scanned for the expected Skill invocation and fails closed on a verified miss; a one-run resolution edge no longer promotes (noise floor). Per-run timeouts and setup failures record an infra-error row instead of aborting the sweep. Overlays touching skills no candidate arm exercises are rejected up front. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: fix skill routing paths, version headers, and skill rosters Routing tables point at the tracked direct skill paths (matching the post-#2434 generator output), AGENTS.md/CLAUDE.md headers match their latest changelog rows, the 1.12.0 row describes what the migration actually does, package/cursor READMEs list the full shipped skill roster, and the swarm READMEs describe /gitnexus-review's expert lenses instead of calling it single-agent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: drift-guard workflow for skill copies; pin eval pip deps; track docs/plans ci.yml ignores '**.md', so an md-only skill edit would merge without the shipped-skills-sync test running — skill-sync.yml triggers exactly on the guarded trees. The eval job's pip install is version-pinned, and docs/plans/ is unignored so gitnexus-plan output can be committed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): keep the runner-invocation literal in gitnexus-review; add concurrency block to skill-sync skills-steering requires skills with a stale-index hint to carry the exact 'node .gitnexus/run.cjs analyze' form — restore it with the fallback ladder as a parenthetical instead of replacing it. skill-sync.yml gains the top-level concurrency block the workflow-convention check enforces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): token-economy guidance for expert lenses Merge lenses that ground in the same material into one reviewer, and use cheaper model/effort tiers for mechanical lenses where the harness offers them, reserving the strongest engine for adversarial judgment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(eval): isolate transcript home on Windows Ensure workflow_bench transcript tests set USERPROFILE alongside HOME so Path.home() resolves to the temporary test home on Windows. * docs(skills): fold PR #2522 execution learnings into review/work/plan Eight incident-backed hardenings from running the full skill cycle (review -> plan -> work, 28-finding fix series) on PR #2522: gitnexus-review: - Expert lenses execute the code under review on candidate failing shapes (empirical probe outranks source reading — every HIGH the language lenses found came from a probe, not a read). - Step 7 re-runs the exact CI check for refreshed baselines/fingerprints (a stale committed artifact is invisible in the diff; caught a red benchmarks arm). - Step 8 treats version/invalidation constants as review surface (INCREMENTAL_SCHEMA_VERSION class recurred verbatim from #2494). gitnexus-work: - Step 4 proves regression tests discriminate against the pre-fix tree. - Step 5 rebuilds executed build output before every verification run (parse workers load dist/; a correct fix 'failed' until rebuilt). - Step 6 makes stage -> detect_changes -> commit one unbroken sequence. gitnexus-plan: - Phase 0 seeded-evidence mode: plan FROM a completed review's verified findings instead of re-running the graph ladder. - Template §7: fingerprint/golden-guarded output rebaselines once, at the series tip. All distribution copies resynced; shipped-skills-sync + skills-steering 24/24 locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): close the skill-evolution loop with an automated proposer driver workflow_bench.evolve adds the three arrows the README described as manual: a proposer session that turns loser trajectories (results.jsonl rows, transcripts, patches, the learning queue) into ONE bounded candidate overlay, a driver that iterates propose -> paired benchmark -> deterministic gate up to --generations, and an --apply step that copies a promoted overlay onto the canonical skills and shipped mirrors as a working-tree diff. The trust boundary is unchanged: overlays re-validate through candidate_overlay_files before any benchmark or apply consumes them, and committing, CI, and the PR merge stay human. learnings.jsonl is gitignored: it is machine-local evidence, like the session transcripts it complements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(skills): route live-task friction into the evolution learning queue Each family skill gains a short 'Skill feedback' section: on friction with the skill's own instructions, append one JSON line to eval/workflow_bench/learnings.jsonl (GitNexus repo only) — never self-edit the skill from a live task. The proposer in workflow_bench.evolve consumes the queue as hints; a learning reaches a shipped skill only by beating the incumbent on the paired benchmark. All shipped mirrors re-copied byte- identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(tests): run the evolve helper tests in the eval pytest job test_evolve.py needs only pytest+pyyaml, same as the harness tests the job already runs — without this line the new module had no CI coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(ci): comment-triggered GitNexus review agent for PRs '@gitnexus review' from a maintainer (OWNER/MEMBER/COLLABORATOR; the action re-validates write access) runs the repo's gitnexus-review skill headlessly against the PR and posts the review as a sticky comment — remote triggering with no local setup. Read-only by construction: contents: read token, Write/Edit and web tools disallowed, Bash allowlisted to git reads and the gitnexus CLI; analyze parses PR code with tree-sitter, never executes it. Requires the ANTHROPIC_API_KEY repository secret; activates once the file is on the default branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(ci): dispatch lane + existing OAuth secret for the review agent Align with claude.yml: same action pin and the CLAUDE_CODE_OAUTH_TOKEN secret the repo already carries — no new secret to configure. Add a workflow_dispatch lane (PR number input) so the agent can be triggered from the Actions UI and tested before the issue_comment trigger reaches the default branch. Allowlist gh pr view/diff and gh api, which the review skill uses to pin PR SHAs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): close a fork-PR RCE vector in the review agent's tool allowlist A live headless run of the exact workflow session against PR #2431 (66 turns, full gitnexus-review pass) surfaced a real HIGH-severity confused deputy: .gitnexus/ is gitignored, not blocked — a fork PR can commit its own .gitnexus/run.cjs, issue_comment checks out PR-head content, and the skill's runner ladder tries 'node .gitnexus/run.cjs analyze' first. That would execute fork-controlled JS inside a job holding CLAUDE_CODE_OAUTH_TOKEN and a write-scoped GITHUB_TOKEN — the opposite of the 'PR code is read, never executed' claim in the workflow's own header. Fix: drop the run.cjs allowlist entry so analyze always resolves through npx gitnexus (npm registry, not the checked-out tree); the skill's documented fallback mode covers the resulting graceful degradation. Also drop 'gh api' (not read-only — accepts -X POST/PATCH/DELETE) and downgrade pull-requests: write to read (comment posting only needs issues: write; the prompt already forbids formal review submission). Same session flagged a latent evolve.py bug: select_evidence's cost sort used dict.get's missing-key default, which doesn't cover an explicit JSON null in a foreign --seed-results row and crashes proposer setup with TypeError. Guarded with 'or 0.0' and added a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: harden PR review and evolution trust boundaries * ci: follow workflow concurrency convention * fix(eval): make terminating error paths explicit * fix: unblock hardened review runtime checks * test: make containment canaries deterministic * test: expose Claude canary tool failures * fix: adapt clean shell environment for Claude * fix(eval): accept the runner's transcript source key in evidence preflight The proposer evidence preflight required transcript-artifact metadata to be exactly {path, sha256, bytes}, but the runner stamps a fourth provenance key (source=parent-captured-stream-json). Any --seed-results or generation>=2 run therefore aborted with SandboxError before proposing or promoting. Pin the producer literal as PARENT_EVENT_STREAM_SOURCE and validate it in the metadata check, and round-trip real producer output through sum_sessions into the preflight so the schema can't drift again. * fix(eval): treat an unmeasured session cost as unavailable, not $0 well_formed validated only the nested usage block, so an otherwise-successful session missing total_cost_usd was recorded as cost_usd=0.0 — and cost_usd is the default promotion metric (lower wins), so a cost-less session scored as free and could win promotion it never earned. Extract cost via measured_cost() (None on absent/garbage, a measured 0.0 preserved), propagate None through sum_sessions/aggregate/savings/report, and have the gate refuse to rank on a metric that was not measured on every run in both arms. * fix(eval): warn when ranking on the main-loop-only num_turns metric num_turns comes from the CLI's top-level usage (main-loop session only), like output_tokens, but selecting it emitted no metric_warning — so a subagent-heavy candidate could look artificially efficient. Add num_turns to MAIN_LOOP_ONLY_METRICS and broaden the warning to cover turns. * fix(eval): fail closed when an overlay adds a file with no committed base An overlay adding a new .md under gitnexus-{plan,work} passes the structural overlay checks but has no committed base for committed_destination_base_digests to bind against, so it raised an uncaught ValueError that crashed the evolve driver (and runner --candidate-overlay) mid-run. Catch it at both call sites: evolve reports NOT PROMOTED and exits, runner routes it through parser.error. * feat(eval): circuit-break the runner sweep on a systemic outage A sustained upstream outage used to pay out every remaining --timeout window one session at a time. Track consecutive session/infra/cleanup failures via a pure systemic_outage_streak helper; after --outage-streak (default 5) in a row, stop the sweep, still write report.md/promotion.json from partial evidence, and exit non-zero so evolve.py halts instead of proposing from truncated evidence. A task's own resolved=False never trips the breaker. * fix(cli): report a dirty working tree as stale in gitnexus status status --json (and the human output) computed up-to-date from commit + runner identity + completeness only, so a repo with uncommitted source changes at a matching HEAD was reported up-to-date while analyze would still re-index it. A graph-backed agent gating on that JSON could skip re-analysis on a stale graph. Extract analyze's dirty-tree check into a shared isWorkingTreeDirty() in storage/git and fold it into the status freshness decision. * fix(ci): use single-slash deny globs in the review agent's disallowedTools github.workspace already expands to an absolute path, so Read(/${{ github.workspace }}/**) and Read(//proc/**),(//sys/**),(//dev/**) produced double-slash patterns that a normalizing matcher may not match — silently no-opping the deny layer. Not exploitable (the allowlist is the primary control and never grants those paths), but the globs should be well-formed. Update the pinned test strings. * ci: install gitnexus-shared with npm ci from the committed lockfile The gitnexus-shared build floated its deps via npm install in three workflows (skill-sync, ci-tests, and — most importantly — the release publish.yml) while every other install step uses npm ci. The lockfile is committed and in sync, so switch all three to npm ci for reproducible, locked installs. * test(cli): make the shipped-skills drift guard reject symlinks listFilesRecursive walked with readdirSync and snapshotDir read with readFileSync, both of which follow symlinks — so a mirror file symlinked to the canonical tree passed the byte-compare (and a symlinked mirror dir would be followed too). Reject a symlinked root via lstat and any symlinked entry via Dirent.isSymbolicLink, with negative tests (skipped on Windows). * test(eval): guard the candidate-skill vs mirror-root coverage invariant MIRROR_SKILL_ROOTS omits the Cursor tree, safe only because no candidate skill is cursor-shipped. Pin that invariant: every CANDIDATE_SKILLS entry must exist under canonical + every mirror root and must not ship to Cursor, so adding a cursor-shipped skill to the candidate set (the PR #2488 asymmetric-sync class) fails loudly instead of syncing three of four trees. * docs(ci): describe the review agent's staged post-merge rollout The DoD asked for a dry-run or triggered run before merge, but an issue_comment (or newly added workflow_dispatch) workflow only ever executes the default-branch copy, so it cannot be exercised from the PR that introduces it. Reword the DoD and the activation checklist to a staged rollout: merge registered-but-disabled, validate same-repo and fork execution post-merge, then enable the variable. * fix: pin plugin skill mcp.json to the release version via #2445 tooling The ten plugin skill mcp.json launched `npx -y gitnexus@latest mcp` on every skill connect — non-reproducible and a supply-chain surface, and (unlike the persisted setup config) never pinned. Extend sync-plugin-manifests.mjs with an mcp surface kind that stamps the gitnexus@<version> launch arg, pin all ten to 1.6.9 now, and keep them byte-identical so the drift guard stays green. The release lifecycle + publish.yml --check now re-stamp them like the four manifest surfaces; only READMEs stay on @latest as docs. * test(eval): prove the proposer's built-in file tools are confined The real-Claude canary only exercised Bash + MCP, so it proved process/MCP containment but not that the proposer's built-in file tools stay inside their mounts. Add a canary over the exact PROPOSER_ALLOWED_TOOLS surface and the same read-only /evidence mount as run_proposer (allowlist extracted to a shared constant so it can't drift): Read reaches /evidence, a Write into the read-only evidence mount is denied, and a Write lands in the output tree. * fix(eval): apply the candidate overlay after task setup for fair arms The candidate overlay was applied before the task's untrusted setup ran, so setup could observe candidate prose and the incumbent/candidate arms started from different pre-overlay state. Reorder within the sandbox: capture the base (pre-overlay) skill digest, run setup against the base skills, verify setup did not tamper them, then apply the overlay and capture the post-overlay digest the model must preserve. apply_candidate_overlay stages path-specific overlay files, so setup's uncommitted changes stay out of the baseline and churn is unchanged. Graph freshness for the review arm is handled by the status dirty-tree fix plus the review skill's stale-triggered re-index, not by reordering the cached per-task-sha graph materialization (which is mechanically blocked). * test(eval): end-to-end containment proof of the autonomous proposer Drives the real run_proposer through bubblewrap with a deterministic scripted model (no paid API): it reads the read-only evidence bundle and writes a candidate gitnexus-plan skill edit plus a rationale into the sandbox output tree; run_proposer enforces the trust boundary and copies only the validated overlay + proposal out. This exercises the autonomous-proposal stage of the self-evolution loop end-to-end in the eval/containment CI job (the gate and apply stages are covered by test_workflow_bench_evolution and test_promotion_apply). Env-gated on GITNEXUS_REQUIRE_CLAUDE_CANARY, so it runs only where the pinned Claude binary and user namespaces are available. * fix(eval): let the proposer author its overlay via Bash Running the end-to-end proposer canary in the containment CI job surfaced a real bug: run_proposer starts the session with --bare, which hard-disables the Write/Edit tools ("Write exists but is not enabled in this context"), yet allowlisted Edit/Write and omitted Bash. The proposer therefore had no working way to write its candidate overlay — the self-evolution loop could never produce a candidate. The sandbox settings already pre-authorize Bash (autoAllowBashIfSandboxed) and confine writes to workspace/tmp/home, so switch PROPOSER_ALLOWED_TOOLS to Read/Grep/Glob/Bash and tell the proposer to author files with Bash. The end-to-end test now drives the real run_proposer through bubblewrap and asserts a validated overlay + proposal are produced (this also replaces the earlier file-tool canary, whose Write/Edit premise was moot). * test(eval): author the proposer overlay with newline-free Bash content The nested shell-sandbox prefix mangles embedded newlines, so the multi-line overlay content never landed. Use single-line content for the deterministic proposer canary. * test(eval): drop the unverifiable end-to-end proposer canary The scripted proposer overlay never materialized in the containment job across runs, and the model tool-result content is not visible in CI logs, so the test cannot be finalized without an environment where the sandbox can actually run. Keep the verified production fix (Bash-authoring in run_proposer); the proposer sandbox/containment stays covered by the existing Bash+MCP and process-tree canaries. * test(cli): drop run-analyze.ts from the windowsHide spawn-family list U7 moved run-analyze.ts's only child_process call (the git status --porcelain dirty check) into storage/git.ts (already covered by this test, with windowsHide). run-analyze.ts no longer imports a spawn-family function, so the windowsHide-regression test's 'must have >=1 spawn call' invariant failed for it. Remove it from SRC_FILES. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Zander Raycraft <zanderjraycraft@gmail.com> Co-authored-by: Azizur Rahman <azizur100389@gmail.com> |
||
|
dependabot[bot]
|
e2e9254938
|
chore(deps): bump github/codeql-action/upload-sarif (#2535)
Bumps the codeql-action group with 1 update: [github/codeql-action/upload-sarif](https://github.com/github/codeql-action).
Updates `github/codeql-action/upload-sarif` from 4.36.2 to 4.37.0
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](
|
||
|
dependabot[bot]
|
0656099332
|
chore(deps): bump docker/metadata-action from 6.1.0 to 6.2.0 (#2536)
Bumps [docker/metadata-action](https://github.com/docker/metadata-action) from 6.1.0 to 6.2.0.
- [Release notes](https://github.com/docker/metadata-action/releases)
- [Commits](
|
||
|
dependabot[bot]
|
731ab6f512
|
chore(deps): bump marocchino/sticky-pull-request-comment (#2537)
Bumps [marocchino/sticky-pull-request-comment](https://github.com/marocchino/sticky-pull-request-comment) from 3.0.4 to 3.0.5.
- [Release notes](https://github.com/marocchino/sticky-pull-request-comment/releases)
- [Commits](
|
||
|
dependabot[bot]
|
dc993a6d43
|
chore(deps): bump github/codeql-action/analyze from 4.36.2 to 4.37.0 (#2506)
* chore(deps): bump github/codeql-action/analyze from 4.36.2 to 4.37.0
Bumps [github/codeql-action/analyze](https://github.com/github/codeql-action) from 4.36.2 to 4.37.0.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](
|
||
|
Gergo Magyar
|
573a777ef5 |
ci(tests): widen the Windows shard watchdog and keep exit diagnostics (#2449)
The busiest Windows platform shard reached 14m57s against the 15 minute watchdog on the rc.19 green run and has timed out once since. CI now sets GITNEXUS_CROSS_PLATFORM_TIMEOUT_MINUTES=20 (the job timeout stays 25), the stale comfortably-under comment reflects reality, and the runner always logs status, signal, spawn code and elapsed time so the next status-null death is diagnosable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Gergo Magyar
|
42de243e9a |
ci(release): sync plugin manifests on every version bump (#2445)
The RC path bumped only gitnexus/package.json, so every v1.6.10-rc tag through rc.28 shipped the four plugin manifest surfaces frozen at 1.6.9 and failed its own unit suite. The npm version lifecycle script now runs a fail-closed sync whenever npm version executes, in CI or on a maintainer's laptop; publish.yml verifies the result and stages the surfaces into the detached release commit, and the stable path refuses to publish a tag whose manifests drifted. The sync is textual so a release commit carries a one-line change per surface instead of reformatting churn. Design follows the proposal by @100yenadmin in #2445, moved onto the standard npm version hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
dependabot[bot]
|
40f7502370
|
chore(deps): bump docker/setup-buildx-action from 4.1.0 to 4.2.0 (#2500)
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 4.1.0 to 4.2.0.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](
|
||
|
dependabot[bot]
|
7edf6236b5
|
chore(deps): bump docker/login-action from 4.2.0 to 4.4.0 (#2507)
Bumps [docker/login-action](https://github.com/docker/login-action) from 4.2.0 to 4.4.0.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](
|
||
|
dependabot[bot]
|
76c61bed49
|
chore(deps): bump dorny/paths-filter from 4.0.1 to 4.0.2 (#2505)
Bumps [dorny/paths-filter](https://github.com/dorny/paths-filter) from 4.0.1 to 4.0.2.
- [Release notes](https://github.com/dorny/paths-filter/releases)
- [Changelog](https://github.com/dorny/paths-filter/blob/master/CHANGELOG.md)
- [Commits](
|
||
|
Eva
|
e3136f593f
|
ci: update setup composites to setup-node v6 (#2451)
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
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
|
||
|
dependabot[bot]
|
3e38cd0eb5
|
chore(deps): bump docker/setup-qemu-action from 4.1.0 to 4.2.0 (#2408)
Bumps [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) from 4.1.0 to 4.2.0.
- [Release notes](https://github.com/docker/setup-qemu-action/releases)
- [Commits](
|