mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-28 06:22:22 +00:00
107 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gergő Magyar
|
01af833875
|
fix(scope): preserve lexical bindings in HTTP wrapper detection (#3632)
* docs(scope): clarify lexical binding ownership and wrapper provenance Checkpoint for #3630. Publishing the implemented source, regression tests, and benchmark harness as a draft PR; full-suite and final scaling validation remain in progress. * fix(scope): publish #3630 docs/plans * fix(scope): publish #3630 gitnexus/bench/lexical-binding-provenance * fix(scope): publish #3630 gitnexus/bench/lexical-binding-provenance/fixtures * fix(scope): publish #3630 gitnexus/bench/lexical-binding-provenance/results * fix(scope): publish #3630 gitnexus/src/core/ingestion * fix(scope): publish #3630 gitnexus/src/core/ingestion/languages/typescript * fix(scope): publish #3630 gitnexus/src/core/ingestion/languages * fix(scope): publish #3630 gitnexus/src/core/ingestion/pipeline-phases * fix(scope): publish #3630 gitnexus/src/core/ingestion/route-extractors * fix(scope): publish #3630 gitnexus/src/core/ingestion/workers * fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/lib * fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity * fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/packages/http * fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/packages/http/src * fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/server * fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/web * fix(scope): publish #3630 gitnexus/test/integration/resolvers * fix(scope): publish #3630 gitnexus/test/unit * fix(scope): publish #3630 gitnexus/test/unit/group * fix(scope): publish #3630 gitnexus/test/unit/scope-resolution * fix(scope): publish #3630 gitnexus/test/unit/scope-resolution/typescript * fix(scope): publish #3630 gitnexus-shared/src/scope-resolution * fix(scope): treat wrapped assignment targets as writes `(app) = new Map()`, `(app as T) = x` and `app! = x` assign the same binding as `app = x`, but route-fact invalidation and the shared pattern walker stopped at the wrapper, so Express provenance survived the write and produced a false route. Recurse through parenthesized, as/satisfies and non-null targets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(routes): resolve wrappers through overloads, declarator lists and barrels Three wrapper shapes lost their identity: - Bodyless overload signatures were indexed as separate definitions sharing the implementation's binding, so the binding looked ambiguous and both local and imported calls were rejected. Signatures now declare no callable. - Facts for `const base = 0, request = (url) => fetch(url)` followed the first declarator. The captured name now selects its own declarator. - An import resolved to a workspace barrel stopped at index.ts. Workers now record named, star and imported-then-exported re-exports, and resolution follows them to the declaring module: local and named exports shadow `export *`, star exports never carry `default`, and conflicting star exports block the name. No repository-wide name fallback is restored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(scope): assert closure ownership against the declaration The closure-write assertion compared one lookup with itself, which also passes when both are undefined. Compare it with the outer declaration and the shadowing parameter, and keep the memoization check explicit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(routes): share lexical helpers and release per-file HTTP contexts - Share binding keys, parameter patterns and transparent expression wrappers from lexical-bindings instead of four local copies; drop the unused moduleStem export; name the inferred-source predicates in lookup. - Key the per-file HTTP context by its tree rather than its ParsedFile. Workers retain ParsedFiles for the whole chunk, which kept every file's AST and traversal maps alive until the chunk flushed. - Index re-exports by name, memoize barrel walks per starting export, and keep facts without import targets as-is instead of copying them. - Move TS-only wrapper cases into the TS describe.each instead of branching inside the per-language loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(routes): keep wrapper identity for HOCs, CommonJS, object keys and fetch packages Four definition shapes that main linked by name lost their wrapper identity: - `const load = useCallback((url) => fetch(url))` and curried HOCs produced no facts because definition lookup stopped at the call; it now follows the callable argument, as the structural definition capture does. - `module.exports = { request }`, `exports.x = …` and `module.exports.x = …` were not exports, so destructured `require` callers missed the wrapper. CommonJS export assignments are recorded when `module`/`exports` are the unshadowed globals, including functions defined inline in the export object. - An object-literal key or method name was resolved as a lexical binding and collided with a same-named module wrapper. Only binding identifiers now name a definition's binding. - `import fetch from 'node-fetch'` no longer counted as fetch. Imports of known fetch implementations (node-fetch, cross-fetch, isomorphic-fetch, the fetch member of undici and whatwg-fetch) are fetch evidence again; parameters and other local names still are not. Reference identifiers are also resolved only when their name is bound to a callable or class region; other identifiers cannot add a reachability edge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(routes): drive the forwarding fallback case from data and cover re-export merges Replace the boolean-switched forwarding fallback case with table rows instead of branching inside the test, and assert that barrel re-export facts survive worker sub-batch merges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(bench): reject stale or dirty call-graph exports The committed comparison named a candidate revision that is not on this branch, and nothing stopped it. Comparison now requires both exports to come from clean checkouts and the candidate export to come from the checkout running the comparison. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(bench): regenerate the call-graph comparison on the reviewed head Clean baseline |
||
|
Abhigyan Patwari
|
89602560d3
|
fix(group): read Python prefixed literals and f-string URLs as paths (#1675) (#3626)
Some checks failed
Skill copy sync / shipped skills drift guard (push) Has been cancelled
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
|
||
|
Abhigyan Patwari
|
d4dc797054
|
fix(group): read net/http HandleFunc and Handle on proven routers as method-agnostic routes (#3627)
* fix(group): read net/http HandleFunc and Handle on proven routers as method-agnostic routes (#1668, #1670) The Go provider query only matched the literal `http.HandleFunc(...)` with an identifier or inline handler, so a local `http.NewServeMux()`, a gorilla or chi router, `Handle(...)`, a method-value handler (`api.ListUsers`) and a raw string path produced no provider at all (#1668). It also hard-coded `GET`. A ServeMux registration serves every method unless its pattern names one, so a POST consumer never paired with the route (#1670). The provider is now method-agnostic (`*`), the convention Django, Nest `@All` and Hono `.all` already use and the matcher already pairs with consumers of every verb. A Go 1.22 pattern (`"POST /p"`, `"GET host/p/{id}"`) gives its method and drops the host and the `{$}` anchor, and a gorilla `.Methods(...)` on the route chain gives its verbs. `HandleFunc`/`Handle` are common names outside HTTP (telebot's `b.Handle("/start", fn)`), so the receiver must be a proven router: the net/http package, a binding to `http.NewServeMux()`, gorilla `mux.NewRouter()` or chi `chi.NewRouter()`, a gorilla `PathPrefix("/p").Subrouter()` or chi `Route("/p", func(r chi.Router))` (joining the prefix), or a parameter typed as one of those routers. A router the file mounts under a stripped or prefixed path (`Mount`, `StripPrefix`, gorilla `.Handler`) is declined, as is a struct field, since their registered paths are not their URLs. Routers in `_test.go` files are httptest mocks and are skipped. Handler naming is shared with the gin/echo loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(group): decline mounted router parameters and read HEAD routes as GET A router parameter passed to `StripPrefix`/`Mount` was never marked mounted, because the mount scan resolved identifiers through `findBinding`, which returns nothing for a parameter, so its unprefixed paths were emitted. The scan now records a parameter by its declaration and the router proof checks it. A `"HEAD /x"` pattern or gorilla `.Methods("HEAD")` produced an `http::HEAD::` provider, while this plugin's `http.Head` consumers are keyed as GET. Providers now follow the same HEAD-as-GET convention. The plugin docstring no longer says any receiver is accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(group): read http.HandlerFunc conversions and hyphenated method tokens `http.Handle("/x", http.HandlerFunc(h))` was dropped because the handler argument is a conversion call; the converted function is now the handler. A Go 1.22 pattern whose method token contains characters other than letters (`"M-SEARCH /discover"`) produced no provider; the method now accepts any HTTP token that starts with a letter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(group): keep HEAD-only Go routes apart from GET consumers Reading a `"HEAD /x"` pattern or gorilla `.Methods("HEAD")` as GET let every ordinary GET consumer pair with a route that does not serve GET. The provider now stays HEAD. A HEAD consumer, which this plugin records as GET, then misses a HEAD-only route; a missed link is preferred to a false one. Repeated verbs still collapse. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
f8209df2e2
|
fix(routes): resolve Express receivers and router mount paths (#3603)
* fix(routes): resolve Express receiver provenance and mounted paths Require statically proven framework receivers, compose Router mount paths, and share the same provider resolution between ingestion and group routes. Preserve worker and parse-cache parity with per-file facts. * fix(review): apply review findings Invalidate exported Routers through namespace property mutations and retain uncertain incoming mounts so invalid parents cannot revive local paths. * refactor(routes): reuse export checks and fixture writer * fix(routes): evaluate fluent call receivers once * fix(review): preserve provider paths and uncertain mounts * fix(test): distinguish parameterized router mutation cases * docs: remove Express route implementation plan |
||
|
Gergő Magyar
|
b37d4f246d
|
fix(group): release bridge database before publishing metadata (#3594) | ||
|
Gergő Magyar
|
999cc9f913
|
fix(deps): upgrade native tree-sitter to 0.25.1 (#3531)
* fix(deps): upgrade native tree-sitter to 0.25.1 * fix(tree-sitter): preserve sync loading and update verified capture baselines Route C# scope queries through the synchronous native grammar loader so the compiled analyze module remains require-compatible on Node 22.18. Refresh only the audited empty-Python-module and renamed-PHP-fixture captures, and pin the runtime assertion to 0.25.1. Validated 479 targeted tests, all 16 scope-capture benchmark cases, Python capture/import guards, compiled C# worker parsing, build, typecheck and formatting. Performance budgets are unchanged. * fix(tree-sitter): refresh upstream vendors and preserve parser semantics * fix(ci): refresh prebuilds and support Windows ARM Refresh macOS, Linux ARM, and Windows x64 grammar binaries from the successful native CI builds. Pin node-gyp 12.4.0 so the Windows ARM jobs recognize Visual Studio 2026. Use local Git filtering for E2E to avoid PR-files API timeouts on generated parser diffs. Validation: 143 focused tests, Actionlint, and all 16 scope-capture benchmark checks passed. Full npm test run started; native database suites still running. * fix(ci): refresh Windows ARM grammar prebuilds Import all seven Windows ARM grammar artifacts from successful native build run 37965761760. Artifact filenames and SHA-256 hashes verified; all 42 native build jobs passed. Validation: 31 prebuild coverage tests passed. * fix(bench): stabilize scope capture timing guard Warm each input size seven times and use 21 median samples while retaining isolated timing blocks. Yield between languages so native finalizers release allocations outside the measured sections. Keep all fixture sizes, fingerprints, scaling formulas, and budgets unchanged. Validation: all 16 language guards passed with a 2 GB heap limit (1.38 deep-chain ratio; 1518 MB peak RSS). Linear and quadratic controls passed their expected guard outcomes. Formatting and graph change analysis passed. Full npm test is running without failures so far. * fix(parser): reduce TypeScript capture traversal cost * fix(test): await parser worker readiness before teardown * fix(parser): load C# synchronously in workers * fix(grpc): resolve native proto detections against their own file With the native proto grammar active, sibling .proto files that declare the same short service name under different packages tied in the directory-based resolveProtoConflict heuristic and both providers were dropped. A proto-origin detection already knows its defining file, so use that file's entry before falling back to the heuristic. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * ci(packaging): assert native grammars load from the installed tarball The packaged-install smoke only ran `gitnexus --version`, so a grammar that failed to load from the published package (bundled runtime, patched peer manifests, vendored prebuilds, C# sync loader, Protobuf wrapper) would ship silently; the dev-tree ABI smoke also accepts vendored grammars that report unavailable. Parse a sample for every registered grammar plus the native Protobuf plugin from the installed package, on the main thread and in a worker thread. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * ci(tree-sitter): track compatibility workarounds instead of 0.25 readiness The daily readiness report still targeted tree-sitter@0.25.0, so after the upgrade it would keep reporting "Blocked" and never close its tracking issue. Repurpose it to track what we carry for the bundled runtime: derive the target from the package.json pin, list each pack-time peer patch from prepare-tree-sitter-bundle.cjs with its drop condition (bump, wait for an upstream release, transitive, or intentionally pinned), and reword the buckets. The workflow adopts the issue opened under the old title. Also update the grammar-update-monitor header and auto-PR text, which still described the 0.21.1 runtime and a tree-sitter-c hold. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(dart): assert both nested labels target the loop; fix Zig call-shape doc The nested-label CFG test only jumped to `outer` and checked reachability, which still passes when a jump is retargeted to the inner loop (its exit reaches the outer header). Jump to both labels and assert the exact `continue` edges; dropping the inner label now fails the test. Update the Zig callable-flow comment: call_expression now wraps actuals in a required `arguments:` field. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Codex <codex@openai.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> |
||
|
rgb-vgx
|
dbcfa63d40
|
fix(group): match Go framework imports exactly and follow bare-block group writes (#3473) | ||
|
Gergő Magyar
|
c474a811ba
|
fix(test): align fixtures and mocks with current types (#3471) | ||
|
rgb-vgx
|
504bff7102
|
fix(group): join gin/echo route-group prefixes and accept method-value handlers in Go HTTP providers (#3458)
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
|
||
|
Ankit Verma
|
4f298d0ac0
|
fix(staleness): detect rollback with one Git query (#3445) | ||
|
azizur100389
|
acb65b95b6
|
fix(fastapi): propagate package router mount prefixes (#3408)
Some checks failed
CodeQL / Analyze (javascript-typescript) (push) Has been cancelled
CodeQL / Analyze (python) (push) Has been cancelled
Gitleaks / gitleaks (push) Has been cancelled
Publish / Classify release event (push) Has been cancelled
Scorecard / Scorecard analysis (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-cli) (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-web) (push) Has been cancelled
Publish / RC guard (marker + release-PR skip) (push) Has been cancelled
Publish / ci (push) Has been cancelled
Publish / Publish to npm (push) Has been cancelled
Publish / Build & Push RC Docker images (push) Has been cancelled
* fix(fastapi): carry package router mount prefixes to child routes * fix(fastapi): address review feedback on nested router prefixes (#3408) - Skip unprefixed includes in the parse-impl legacy loop so a bare include_router in another file no longer shadows the real prefix. - Union exact-file prefixes with legacy long/short prefixes via a shared mergeMountPrefixes helper in both ingestion and the group extractor. - Join the parent APIRouter(prefix=...) between the mount prefix and the child include prefix. - Resolve the group layer over every repo path (empty files included) so absolute-import ambiguity matches ingestion. - Memoize (file, prefix) frames so diamond-shaped include graphs stay linear; drop the stack.pop() non-null assertion. - Accept extra keyword arguments and a trailing comma in unprefixed include_router calls without double-firing on prefix= calls. - Document that pass-through is limited to a host named `router`. - Bump parse-cache SCHEMA_BUMP to 123 for the new capture fields. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(fastapi): seed prefix propagation from bare router mounts (#3408) - A router mounted without a prefix now seeds traversal with an empty prefix (only when no prefixed mount targets the same file), so its own APIRouter(prefix=...) reaches unprefixed children on both surfaces. - An all-empty chain records nothing and leaves the child on its legacy fallback. - The bare-mount integration test no longer asserts that the test app's unprefixed mount is absent; it pins only that the real prefix survives. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(fastapi): capture include prefixes after nested-call arguments (#3408) - Let the Shape A/B and unprefixed include_router patterns step over one level of nested calls such as dependencies=[Depends(auth)], so a prefix= written after them is captured by the worker (the group layer's tree-sitter patterns already handled this shape). - Replace the unit test that pinned the dropped prefix with one that pins the captured prefixes and the unprefixed Depends-only edge; add a group-layer parity test. - Correct the diamond test comment to 2^39 root-to-leaf paths. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
52581cd4e9
|
test(group): make bridge mtime fixture deterministic
Fixes #3397 |
||
|
Gergő Magyar
|
aa2d6aaf7d
|
fix(query): keep process_symbols attaches per (id, process_id) (#3353)
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 / ci (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (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 / Publish to npm (push) Blocked by required conditions
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* fix(query): keep process_symbols attaches per (id, process_id) Hubs that belong to several processes were collapsed to one row by unique-id first-wins, so later process cards advertised symbol_count with nothing to expand. Slice, pair-key, and recount now live in one shaper so each listed process can open its own attaches. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(query): drop leftover unique-id attach bind name The shaper already returns process_symbols; keep that name through the query response so unique-id vocabulary does not linger at the call site. Co-authored-by: Cursor <cursoragent@cursor.com> * style(query): wrap process attach call for prettier CI quality/format rejects the previous call shape. Co-authored-by: Cursor <cursoragent@cursor.com> * test(query): assert hub lines under each process card A global duplicate count still passes when both lines render under one process. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(query): recount group symbol_count after the service prefix The single-repo (id, process_id) join was readable as if @group results included process_symbols. Scope that sentence, and set symbol_count from the filtered attaches when a service prefix is set. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(query): document the attach contract and count prefix rows once The Unreleased notes and query tool text now say how to join process_symbols, including the @group follow-up. Service-prefix filtering counts those rows in one pass. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Adam B.
|
7376347058
|
feat(ingestion): add tRPC procedure detection and MCP chain/route surfacing (#3339)
* feat(ingestion): tRPC pattern detection — procedures, curried calls, route extraction
Three changes to properly index tRPC router files:
1. HOC-in-pair patterns: detect procedures like
'create: procedure.mutation(async ({ input }) => {...})'
as named Function nodes (pair > call_expression > arguments > arrow_function)
2. Curried call detection: capture 'workflow(db)(input)' chained calls
where call_expression.function is itself a call_expression
3. tRPC route extraction: new route-extractors/trpc.ts detects
.query()/.mutation()/.subscription() procedures, maps to /trpc/* routes
with prefix inference from router variable names
4. Function/Const dedup: structural check skips Const nodes when
variable_declarator value is arrow_function/function_expression
5. Process-route linking: match by (filePath, methodName) instead of
filePath only, preventing shared flows across procedures in same router
(cherry picked from commit eced15e60a39a181a48bff8c5bbe278d26aa53b8)
* fix(route-extractors): line-by-line scanner for chained tRPC procedures
Original regex only matched direct patterns (list: proc.query()) but not
chained patterns (create: proc.input(z.object({...})).mutation()). Only
41/256 routes were detected on Jurialis.
Rewrote extractTrpcRoutes() as a line-by-line scanner that:
- Detects procedure keys starting with *Procedure builders
- Tracks currentProcedure forward until terminal .query()/.mutation()
- Handles .input() chaining naturally
- Deduplicates via seen Set on procedurePath
Result: 245 routes from 30 routers (was 41), 256 total after re-index.
(cherry picked from commit 49edf953e93e6b67b77932e866fcbfb8d089b1f3)
* feat(mcp): expose tRPC chains via context/query tools
Eight fixes to make the tRPC route->procedure->workflow->sub-workflow chain
visible through the standard MCP tools (context, query) that AI agents use,
instead of requiring raw Cypher queries.
- A context: order incoming/outgoing CALLS test-last, raise LIMIT 30->100
- B query: batched ENTRY_POINT_OF lookup, surface route URL on processes
- C query: mark process_symbols entry point with is_entry_point=true
- D entry-point-scoring: skip UTILITY_PATTERNS (get*/set*) penalty for
symbols in tRPC router files so framework boost (3.0x) is preserved
- E fts-schema: index Route nodes so /trpc/* URLs are keyword-searchable
- F tools: bump max_symbols default 10->25 to fit procedure->workflow chain
- G context: optional chain_depth (0-3) param walks CALLS edges in both
directions and returns layered chain field
- H LadybugDB bug workaround: WHERE r.type IN [...] silently drops edges
on relationship properties; replace with OR chains (7 occurrences in
local-backend.ts, pdg-impact.ts, graph-queries.ts)
Validated on Jurialis: setProviderCap procedure now appears as caller of
setProviderCapWorkflow, /trpc/cabinet.setProviderCap Route is searchable,
chain_depth=3 returns the layered call graph.
(cherry picked from commit c24df0adce91e80f191a5c5d2e8b14e9da677366)
* feat(mcp): surface is_entry_point + routes in context, raise query limit
context() is the mandatory pre-edit tool (AGENTS.md). Until now an agent
had to issue a separate query() call just to learn whether its symbol is
a process entry point or which HTTP route it handles. These two fields
make context() self-sufficient for the bmad-dev flow.
- context: query STEP_IN_PROCESS now returns p.entryPointId; new
ENTRY_POINT_OF lookup attributes routes only from processes where the
symbol is the entry point (middle steps do not own the route) plus any
direct Route->symbol handler edge (tRPC procedures outside any Process)
- context: new top-level is_entry_point (true only) and routes[] fields
({url, method?}), emitted only when non-empty to keep the diff additive
- query: raise default limit 5->10 so dense domains (tRPC action router
with 20 chains, data-export with 9 flows) surface more of their flow
set without an explicit param
(cherry picked from commit 004f5b0ba985fcef3dfac5e67a2f2f7262184215)
* docs(mcp): document chain_depth, routes, and entry-point flag; neutralize examples in comments
* fix(mcp): address code-review findings on tRPC entry points, queries, and dispatch
- entry-point scoring: optional-chain framework detection and normalize
path separators before router-pattern matching (P1 crash on .js routers)
- tRPC extractor: emit controllerName as null (callers resolve via
lookupClassByName; a router object stringified as name corrupted lookups)
- route dedup: key seenRoutes by method:url so GET/POST pairs survive
- legacy TS queries: drop curried-call patterns (arity-corrupting for
overload resolution) and require non-array callee on pair member calls
- registry-primary TS query: mirror the non-array-callee predicate on the
new pair member-expression patterns (fixes query compile error)
- group tool port: forward chain_depth into per-tool context args
* fix(mcp): nested tRPC router paths, controller-less route binding, query chain_depth
- trpc extractor: brace-depth nesting stack composes sibling router paths (bare router() import style included); merge-prefix dot normalization; strict publicProcedure allowlist gate; drop phantom router metadata
- call-processor: bind controller-less tRPC routes to same-file handlers via exact single-match symbol lookup; ambiguous or missing handlers skipped
- query/group query: optional chain_depth (0-3) enriches ranked processes with their context chain; fix _computeContextChain layer docstring
- tree-sitter TS/JS scope queries capture string-key function declarations; tRPC pattern scoring narrowed to server router files
- parse-cache schema bump to v103 for extractor and route-binding changes
- add trpc route extractor regression tests (9 cases incl. sibling nesting and merge prefix)
* Address PR review feedback (#3339)
Close remaining review threads: compact tRPC keys, comment-safe terminals,
quoted-key identifier HOC captures, group-query default alignment, and
LadybugDB label scalars on context chains.
* chore(autofix): apply prettier + eslint fixes via /autofix command
* test: pin PARSE_CACHE schema bump to 103 (PR #3339 review fixes)
* fix(ingestion): bind same-name tRPC handlers and surface HANDLES_ROUTE
Same-name procedures resolve by startLine, the extractor keeps nested paths
through multiline schemas, and context/query UNION HANDLES_ROUTE for leaf
procedures that never become Process entries.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Clamp group query bounds, drop HOC pair false positives, emit tRPC
terminal lines and hyphenated quoted keys, cap chain BFS concurrency,
and restrict the router utility exemption to accessor names.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Score JS/JSX tRPC routers like TypeScript, ignore inner db.query and
unrelated .merge calls, mask regex braces, and assert clamp tests invoke
the query mock.
Co-authored-by: Cursor <cursoragent@cursor.com>
* chore(autofix): apply prettier + eslint fixes via /autofix command
* fix(ingestion,mcp): emit-side HOC callback guards + MCP schema prettier (PR #3339 round 2)
* Address PR review feedback (#3339)
Treat `/` after return-style keywords as a regex, drop the synthetic
appRouter path prefix, and bind the create mutation via an inline callback.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Qualify query() docs so is_entry_point is promised only when the entry
symbol is among the search hits.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Emit tRPC terminals only at procedure paren depth, drop the filename
prefix on bare appRouter = router(), mask regex after if (), and cap
groupQuery member fan-out.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
De-duplicate context-chain BFS nodes reached through multiple frontier
edges, and bind tRPC identifier callbacks (`.mutation(handler)`) to the
handler symbol instead of the procedure key.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Apply LIMIT 50 after DISTINCT neighbors in the context-chain BFS, and
treat the official lowercase `procedure` builder as a tRPC procedure key.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Keep the later duplicate tRPC object-literal key so route binding matches
the handler JavaScript actually ships.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Compose same-file identifier-mounted tRPC subrouters so admin: adminRouter
emits the live admin.list path instead of an unprefixed /trpc/list.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Memoize tRPC identifier-mount paths so a depth-N chain stays linear, and
gate it with the build-free measure.mjs / baselines.json harness.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Compose identifier mounts without phantom /trpc URLs, bind wrapped and
multiline handlers, fail-close missing bench budgets, and reject query
page bounds before search.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Reject advertised MCP query bounds in group mode and chain_depth
instead of clamping or accepting non-integers.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Reject invalid groupContext chain_depth and ignore non-callable
same-file tRPC handler candidates.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Memoize live tRPC mount paths so the depth-N chain bench stays linear,
and render invalid group/MCP bounds without throwing.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Keep t.merge('prefix.', namedRouter) procedures live by recording a
zero-hop mount so the unmounted-router drop does not hide them.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3339)
Recognize type-annotated `const adminRouter: AppRouter = t.router(`
bindings so identifier mounts still compose.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
||
|
Gergő Magyar
|
56feb85c97
|
chore: compile first-party packages with TypeScript 7 (#3311)
Some checks are pending
CodeQL / Analyze (python) (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
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
* fix(web): drop TypeScript 7-incompatible tsconfig paths Remove baseUrl and the dead ../shared include so web project references typecheck under TypeScript 7. Co-authored-by: Cursor <cursoragent@cursor.com> * test(cli): parse TypeScript with a TypeScript 6 API package Keep AST guards working after the named typescript package becomes 7, which no longer ships the Compiler API. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(lint): pin root TypeScript to the 6 API package Give typescript-eslint a TypeScript 6 peer so syntax-only lint still installs after CLI and web move to TypeScript 7. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(deps): compile first-party packages with TypeScript 7.0.2 Unify CLI and web on the same native compiler line as gitnexus-shared so typecheck and emit no longer split 5.x versus 7.x. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(ci): describe parent TypeScript 7 as the shared compiler Stop saying web compiles shared with TypeScript 5 now that the parent lockfile is 7. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ci): compile shared from parent TypeScript on Vercel and skill-evolution Stop isolated npm installs in gitnexus-shared so those paths do not pull a second TypeScript 7 optional-platform tree. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: record TypeScript 7 typecheck and Dependabot major-split policy Keep contributor typecheck commands, and stop Dependabot from bumping shared onto a different TypeScript major than CLI and web. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lint): pin root TypeScript to 5.9 so npm ci satisfies eslint peers typescript-eslint 8 peers typescript below 6.0.0, so the typescript6 alias made quality lint npm ci fail with ERESOLVE. Co-authored-by: Cursor <cursoragent@cursor.com> * test(cli): drop the TypeScript 6 Compiler API package TypeScript 7.0 has no classic createProgram surface, so parse-only guards now use Babel and Mode 4 uses the TypeScript 7 Checker. Co-authored-by: Cursor <cursoragent@cursor.com> * docs: align contributor setup with parent TypeScript 7 compile Stop telling clones to npm-install gitnexus-shared; CI and Vercel already emit that package from a parent lib/tsc.js shim. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(web): typecheck React JSX on TypeScript 7 with explicit DOM libs TypeScript 7 no longer implies DOM or auto-includes @types, so the web app must declare React/JSX settings while Vite keeps plugin-react. Co-authored-by: Cursor <cursoragent@cursor.com> * test: pin Vercel --include=dev and share parse-only string helpers Production npm ci omits the web TypeScript unless --include=dev is on that install. Move staticStringValue next to the other Babel walk helpers so CLI help and contract tests share one source. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Gergő Magyar
|
21a52af1d4
|
fix(lbug): ship FTS per-platform and recover in-place native aborts (#3274)
* fix(lbug): pin Ladybug core so Dependabot cannot ship a skewed FTS artifact The extension version is a separate upstream constant. Ignore daily core bumps and fail the pairing gate when the committed manifest does not name the installed core. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): make doctor and CI FTS gates resolve the packaged artifact Doctor and the REQUIRE_FTS file gates still treated an empty ~/.lbdb as unavailable, which would turn three CI jobs red once analyze stops installing into that tree. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): name native-abort and tuple-missing so analyze cannot mis-advise The CLI summary's trailing else treated every unknown skip reason as a missing extension. New crash and platform causes must get their own remedies, not a network-install hint. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): delete the dead read-path FTS index create ensureFTSIndex had no production callers and swallowed read-only CREATE_FTS_INDEX failures, which hid the only signal that a reader tried to write. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): vendor per-platform FTS artifacts so analyze needs no host install Keyword search depended on a CDN fetch into ~/.lbdb. Shipping the five published tuples inside the package makes air-gapped and ignore-scripts installs load the same artifact the publish gate checksums. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): load the packaged FTS artifact before any network install Analyze still required a CDN fetch into ~/.lbdb even when the package already shipped the file. FTS now path-loads the vendored tuple first and records source labels so a later truncated home copy cannot steal the diagnosis. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): diagnose a core/extension version skew instead of a missing runtime A structurally valid FTS artifact whose path version disagrees with the packaged pin must name both versions, not prescribe VC++ or OpenSSL. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): stamp an FTS phase so repair stays usable after an in-place abort A native CREATE_FTS_INDEX abort leaves no skip reason; the next run infers it from the dirty flag, and --repair-fts must not treat that phase as a half-written graph. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): park an in-place FTS crash WAL without wiping the graph An FTS abort after a successful checkpoint must reopen the live index on macOS, Windows, and Linux. Staging never parks the live WAL; readers keep today's large-WAL refusal. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): refuse read-only opens of an FTS-poisoned WAL MCP and serve cannot repair a leftover in-place abort. Fail before the native open and name --repair-fts, on macOS, Windows, and Linux. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): name a vendor-neutral Windows OpenSSL prerequisite OQ1 is unanswered here so GitNexus does not ship OpenSSL DLLs. Windows FTS now asks for a system OpenSSL 3 runtime instead of Git Bash PATH. Co-authored-by: Cursor <cursoragent@cursor.com> * test(lbug): inject the FTS vendor root and redact it on HTTP and MCP Path-loaded artifacts no longer vary with HOME. Tests pass an injected vendor tree and assert search warnings never leak a filesystem path. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(lbug): document load-only as the global FTS install default Analyze still overrides to auto. Packaged per-platform artifacts load before any network install on macOS, Windows, and Linux. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(lbug): format the FTS install-policy README table Prettier does not run on Markdown in pre-commit, so the U10 table wrap needs its own formatting commit. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): skip FTS CREATE after a persisted native abort A recovered analyze run was retrying CREATE_FTS_INDEX from skipReason alone. Keep that skip until --repair-fts, fail closed on unsupported tuples, and honor the checkpoint warrant for park/repair. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): honor checkpoint flushed warrant and align FTS tests with packaged vendor A no-op CHECKPOINT must not satisfy the FTS park warrant, and CI still asserted HOME-only FTS isolation after analyze started path-LOADing the packaged artifact. Co-authored-by: Cursor <cursoragent@cursor.com> * test(lbug): accept a nonempty incremental write set in the #2790 recovery check FTS-phase recovery can incremental-add files (changed=0, added=1). That is not the #2790 empty-diff wipe skip. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): compare FTS home versions to the core pin and tighten the publish filename gate Ladybug's ~/.lbdb/extension directory is the runtime/core version; treating it as the artifact version false-diagnosed skew. The publish guard now rejects a path-escaping filename the same way the fetch script does. Co-authored-by: Cursor <cursoragent@cursor.com> * test(lbug): seed FTS e2e fixtures from the packaged vendor artifact A machine with no ~/.lbdb copy should still run the vendor-survivorship cases; the seed no longer depends on HOME or a network install. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3274) Keep in-place FTS abort evidence after persist so a second CREATE abort cannot fail-open readers, and close the CLI, loader, embed, and e2e gaps the review called out. Note: full npm test hit Ladybug worker-pool startup failures under memory pressure; tsc and 180 targeted unit tests passed. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3274) Run the vendored-path symlink guard on the OS matrix, put e2e HOME fixtures on Ladybug's real extension layout, pin the embed crash-WAL gate before the writable open, and let analyze writers park through missing-shadow recovery. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): keep --repair-fts CI green after vendored-first FTS Never-installed warning fixtures must not inspect a packaged vendor binary, and a failed dirty restamp must not abort an otherwise successful --repair-fts run. Co-authored-by: Cursor <cursoragent@cursor.com> * test(cli): give the #1169 analyze e2e the same 90s Windows budget as its sibling The first #1169 persist-meta case was still on a 60s spawn/it budget and was killed banner-only on windows-latest after the FTS warning fixture no longer failed the shard first. Co-authored-by: Cursor <cursoragent@cursor.com> * test(ci): reweight Windows shards after the FTS e2e grew Vendored-first HOME fixtures pushed fts-extension-e2e to ~6 minutes on windows-latest, so the old 146s weight packed it with skills-e2e and blew the 20-minute watchdog. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
mengkaka
|
79543c8f83
|
feat(storage): add configurable index storage and content retention tiers (#3060)
* feat(storage): add configurable index storage and content retention tiers Rebase #3060 onto current origin/main. Keep GITNEXUS_STORAGE_PATH, GITNEXUS_STORAGE_ROOT, and GITNEXUS_CONTENT_RETENTION, and fold in main's FTS skip, embed-session, and help-text updates. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Keep legacy registry rows on the local storage fallback, resolve symlinks before the destructive-path guard, and align hook lookup with CLI branch slugs, branch-slot metadata, and longest-path match. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Only list swept upload directories after a successful removal so callers cannot treat a permission or transient rm failure as gone. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Document that getStoragePath may consult registered storage while this module still does not mutate the global registry. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(storage): close review findings for external indexes and retention Re-inspect ownership under the analyze lock, fail-closed when the registry file is missing, and keep skip-git hook discovery plus retention fields on HTTP/MCP list surfaces. /api/file stays 410 unless contentRetention is full. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * Address PR review feedback (#3060) Treat lock-only index dirs as empty, honor HTTP --force storage policy, and prefer registered plus branch-aware slots in hooks and augment. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3060) Keep hook fallbacks inside the current worktree, compare foreign-local slots canonically, and make storage fixtures survive ownership validation. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix macOS hook test expecting realpath'd registry paths. resolveHookRepo returns the written registry path, not a filesystem realpath, so the assertion must match that. * Address gitnexus-check warnings on hook install docs and slot tests. The Cursor troubleshooting list omitted registry-query.cjs, and the writable-slot test only checked that isDirectory exists instead of that the path is a directory. * Align the HTTP catalog source-scan with skippable resolveRepo validation. resolveRepo lists fresh repos with validate: options.validateStorage !== false so DELETE can skip prune; the test still required a literal validate: true. * Harden storage path sinks so CodeQL path-injection and ReDoS alerts clear. Contain every filesystem probe inside the resolved storage slot with the inline path.relative idiom, reject filesystem-root slots, and trim slot basenames in linear time. * Settle bridge stamps before writing so CI size/mtime matches stay stable. LadybugDB can still flush into bridge.lbug after close+rename; persist whole-millisecond mtimes and wait for consecutive stats to agree so a freshly written pair matches. * Type the settled bridge stat as fs.Stats so tsc does not see bigint. Awaited<ReturnType<typeof fsp.stat>> collapsed the bigint overload and broke prepare/typecheck on CI. * Keep the bridge mtime stamp exact so same-size swaps still fail the pair check. Co-authored-by: Cursor <cursoragent@cursor.com> * Wrap the bridge stamp predicate so prettier --check stays green. Co-authored-by: Cursor <cursoragent@cursor.com> * Require a quiet interval before stamping a settled bridge file. Co-authored-by: Cursor <cursoragent@cursor.com> * Reuse shared storage and settle helpers instead of local copies. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
pcaceres-hazloagil
|
a8736a07d0
|
fix(lbug): checkpoint race in pool-adapter.ts + pin @ladybugdb/core to 0.18.3 (#3189)
* fix(lbug): await evict-then-reopen so it can't race the checkpoint
closeOne() closed the evicted repo's shared Database with a
fire-and-forget `db.close().catch(() => {})` (no await). Both call
sites that evict-then-reopen — evictLRU() right before doInitLbug
opens the new connection, and the "idle & changed" path in initLbug —
proceeded to open the next repo's connection immediately after,
without waiting for the evicted repo's close (and the checkpoint it
triggers) to finish. On the real engine the new open can then collide
with that still-in-flight checkpoint, surfacing on any read as:
Runtime exception: Cannot open database in read-only mode while
checkpoint is in progress. Please retry later.
This reproduces reliably once more than MAX_POOL_SIZE (5) distinct
repos are queried within a short window (self-hosted deployments with
more than a handful of active repos hit it routinely), and gets worse
under genuinely concurrent requests for different repos, since nothing
serialized pool mutations across callers either.
Fix:
- closeOne / evictLRU are now async and await their internal work
(closeOne's own close() call; evictLRU's call to closeOne), closing
the race within a single initLbug call.
- The exported initLbug is wrapped in a small async mutex
(initLbugInner does the real work) so concurrent initLbug calls for
different repos serialize instead of each racing their own
evict-then-reopen against the others.
- closeLbug's two closeOne() calls are now awaited too — closeOne
becoming async meant closeLbug could resolve before pool.delete()
had actually run, which a repo-pinning test caught (isLbugReady()
briefly still true right after a resolved closeLbug()).
- closeOne now deletes the pool entry (and clears its pin, and
notifies pool-close listeners) BEFORE the awaited db.close(), not
after. Review caught that the previous order left a "zombie" entry
reachable via pool.get(repoId) — closed=true, available emptied, but
still present — for the duration of that await; a same-repo
query/init landing in that window would see isLbugReady() as true
and hit a "Connection pool integrity error" in checkout() instead of
just reopening. Deleting first removes the entry entirely, so a
concurrent caller takes the normal fresh-open path instead.
Verified two ways:
- Against the compiled bundle (`ghcr.io/abhigyanpatwari/gitnexus`,
1.6.10/1.6.11 — pool-adapter.js is byte-identical between them): an
A/B docker build with 7 tiny local repos and genuinely concurrent
(parallel, not sequential) /api/graph requests goes from 7/7 failing
to 7/7 succeeding on a freshly-analyzed pool.
- Unit tests here (mocks @ladybugdb/core the same way as
lbug-pool-pinning.test.ts): one asserts the evicted repo's close()
completes before the initLbug call that triggered the eviction
settles; another asserts closeLbug's own promise doesn't resolve
before the underlying close() does. Both gate their mock's close()
on a real short delay and were confirmed to fail against code that
drops the corresponding await.
Note: a second, deeper issue was also observed in the docker A/B
setup — repeated rounds of concurrent access show a repo that has
gone through one evict+reopen cycle can become permanently unable to
reopen for reads, identically with and without this fix. That did not
reproduce with mocks and isn't understood yet; filed separately as
#3186, which stays open and untouched by this PR — this fix closes a
real, root-caused bug on its own but does not resolve #3186 by itself.
Second review round caught a follow-up: the idle-timeout sweep calls
closeOne(repoId) directly, outside of initLbug's poolLock. Now that
closeOne deletes the pool entry before its awaited close(), an
unsynchronized idle close racing a same-repo initLbug could let that
initLbug treat the repo as absent while the idle close (and its
checkpoint) is still in flight — reopening the same class of race this
PR exists to close, just via the idle path instead of LRU eviction.
Routed the idle sweep's closeOne call through withPoolLock too, so it
serializes against initLbug the same way evictLRU already does.
(Tried to add a mocked regression test for this specific interleaving;
dropped it — the mock's dbCache-reuse path masks the difference
regardless of the fix, so it could not be made to discriminate
reliably. Fixed by direct code review instead, same as the note below
already does for the native-engine-specific checkpoint collision.)
Also removed the initLbugInner per-repoId initPromises dedup map: with
every initLbug call now serialized through poolLock, a second call for
a repoId already being initialized cannot observe a pending promise in
initPromises (the first call always fully completes, including its
finally-block cleanup, before the lock releases) — the branch was dead
code the bot correctly flagged twice.
Third review round caught two more follow-ups on the same theme (both
introduced by making the idle sweep route through poolLock):
- closeLbug()'s no-arg ("close everything") branch still calls closeOne
directly in a loop over a snapshotted pool.keys(), without the lock —
an initLbug racing that loop could register a fresh entry the
snapshot never saw, leaving it resident after a call meant to empty
the pool. Wrapped the snapshot+loop in withPoolLock.
- The idle timer callback can now sit queued behind an in-progress
initLbug before its turn arrives, and that init (or a concurrent
touchRepo()) can refresh lastUsed in the meantime — so the pre-lock
idleness check taken when the timer fired can be stale by the time
it actually runs. Re-check lastUsed/checkedOut again inside the lock,
right before closing, instead of trusting the outer snapshot.
* fix(deps): pin @ladybugdb/core back to 0.18.3
Bisected the "checkpoint is in progress" symptom (root cause #2, not
touched by the pool-adapter.ts fix in the previous commit) down to a
single dependency-version-bump commit with zero application code
changes:
|
||
|
azizur100389
|
3f5ca8cdb7
|
fix(storage): guard stale file-lock reclamation (#3234)
* fix(storage): guard stale file-lock reclamation * fix(storage): close lock recovery failure paths * Address PR review feedback (#3234) - Flush the lock-child stderr diagnostic before process.exit - Treat explicit NaN timeouts as the default ceiling - Document non-retryable guard timeouts on the worker IPC contract Co-authored-by: Cursor <cursoragent@cursor.com> * fix(storage): stop mislabeling live lock waits as orphan recovery A brief peer inspect must not attach guardPath or send operators to RUNBOOK delete steps. Refuse lock-free embeddings sync, stop --watch only on a true guard timeout, and drop an unreadable self-created guard before failing closed. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * Address PR review feedback (#3234) - Verify lock/guard absence before degrading a denied main-lock create - Launch the third contender from unlinkSync, not the dead rename path - Document group-lock timeouts for unrecoverable guard leftovers Co-authored-by: Cursor <cursoragent@cursor.com> * style: prettier index-lock reclaim guard tests Co-authored-by: Cursor <cursoragent@cursor.com> * fix(storage): treat O_EXCL as the lock-file presence check CodeQL flagged existsSync-then-wx on analyze.lock. Create with wx first and only reclaim unreadable leftovers after grace, so a successor is never unlinked from a lost race. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Ankit Verma
|
8bd71c8335
|
feat(staleness): report diverged and unknown index state instead of fresh (#3257)
* feat(staleness): report diverged and unknown index state instead of fresh
A staleness check collapsed every git failure into { isStale: false,
commitsBehind: 0 }. On a branch-pinned serve clone, a failed re-index
leaves the recorded commit orphaned by the --depth 1 fetch; once the
reflog expires and gc prunes it, rev-list fails and the index silently
reads as fresh while still behind.
checkStaleness / checkStalenessAsync now return an additive status:
current, behind, diverged (rev-list failed but HEAD resolved and differs
from lastCommit) or unknown (HEAD unresolvable, no lastCommit, timeout).
isStale and commitsBehind keep their values in every case.
One payload builder (core/staleness-status.ts) feeds MCP list_repos, the
hot read tools and /api/repos + /api/repo. diverged carries a hint and no
invented count; unknown appears on listings only. The helpers live in a
pure module so existing vi.mock stubs of git-staleness stay valid.
gitnexus group status no longer prints "-1 commits behind".
Fixes #3256
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Address PR review feedback (#3257)
- Document the `current` arm `fromHead` can return after a failed `rev-list`.
The `staleness-status.ts` status list, the `fromHead` docstring and the
`stalenessForTool` comment each named only `diverged`/`unknown`, so all three
described a contract the helpers do not have.
- Document both `unknown` rows on the group status type: no recorded commit
(`indexStale: true`, `commitsBehind: -1`) and a git probe that could not
answer (`indexStale: false`, `commitsBehind: 0`), which do not agree on
either field.
- Update both shipped `gitnexus-guide` copies to the new wire shape
(`{ status, commitsBehind?, hint? }`), add a `diverged` example carrying no
count, and state `unknown` is reported only by the `list_repos` listing.
Presence now means "not `current`", not "behind N". Pinned in
shipped-skills-sync.
- Lock the timed-out `rev-list` short-circuit with staleness-timeout.test.ts:
it asserts `unknown` from exactly one git spawn, so removing the `killed`
guard (which would probe HEAD again and double the #3232 bound) now fails.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Claim only the uncountable gap in the diverged hint (#3257)
`fromHead` reaches the `diverged` branch on any non-timeout `rev-list` failure
where `rev-parse HEAD` resolves to a different SHA. It compares the two SHAs and
runs no reachability check, so a transient object-read failure lands there while
`lastCommit` is still an ancestor of HEAD — and the hint told the operator the
commit was gone from history. `staleness-status.ts` already documents `diverged`
as only "provably not at HEAD; the count is unknown", so the string was also the
one place contradicting its own contract.
The hint now states what the check established and hedges the usual cause:
"Index is not at HEAD and the commit gap could not be counted — the recorded
commit may no longer be in this clone's history."
Updated with it: the `staleness.test.ts` diverged assertion, and the `diverged`
example plus its lead-in sentence in both shipped `gitnexus-guide` copies (still
byte-identical). `mcp/resources.ts` needs no change — it interpolates
`staleness.hint`, holding no copy of the text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(staleness): cover the remaining #3256 review test gaps
The tri-review's lower-priority gaps, each now locked:
- staleness-fallback.test.ts: after a failed (not timed-out) rev-list,
both helpers report `current` when HEAD alone still resolves to the
indexed commit, `diverged` when it resolves elsewhere, and `unknown`
when it cannot be read, each from exactly rev-list + rev-parse. This
complements staleness-timeout.test.ts: a timeout spawns once, any other
failure probes HEAD.
- list_repos carries the listing-only `unknown` and a count-free
`diverged`; `current` carries nothing.
- group status: a resolvable repo with no recorded commit composes to
indexStale: true, commitsBehind: -1, status: unknown and renders as
"STALE (? commits behind)", through the real groupStatus path rather
than a hand-built row.
Each test was checked against a mutation of the code it guards (the
fromHead current arm, the group no-commit literal, list_repos
includeUnknown); every mutation fails its test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: tech-admin3 <tech-admin@kodnest.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
|
||
|
Parafee41
|
c9b391357e
|
fix(group): detect function-local Python imports (#3254)
* fix(group): detect function-local Python imports * fix(group): parse Python imports structurally * fix(group): use safe Python parser wrapper * fix(group): isolate Python parse timeouts * fix(group): skip recovered indented Python from-imports Error-recovered trees can promote unclosed-docstring lookalikes into real import_from_statement nodes. Drop those indented imports, bound file reads, log parse timeouts, and add fingerprint floors for the tree-sitter scan. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Gergő Magyar
|
b60c21d05d
|
fix(group): extract NestJS GraphQL contracts on real indexes (#3201) (#3227)
* fix(group): extract NestJS GraphQL contracts against real 0-based indexes (#3201) Provider lookup used 1-based startLine while the graph stores tree-sitter rows, so every resolver missed. Also try PascalCased Document names and inline sibling FragmentDoc interpolations from graphql-codegen output. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): bind arrow-field providers and fail closed on interpolations Match Method startLine to the public_field_definition wrapper, decode template escape sequences, and reject FragmentDoc names that mix static and dynamic declarators. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): only inline interpolated templates under a gql tag Cooked reconstruction is not the runtime value for String.raw or unknown tags, so those interpolations stay fail-closed. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): tighten gql-tag trust and PascalCase Document lookup Only the identifier `gql` is a trusted interpolating tag. Underscored operation names now try the full pascal-case Document candidate graphql-codegen emits. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): memoize GraphQL interpolation source resolution Avoid exponential re-walks when the same fragment name is declared twice at each layer of a ${FragmentDoc} chain. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): fail closed on invalid tagged-template escapes Treat line continuations as empty cooked text and reject \8/\9 plus legacy octals so reconstructed gql source matches runtime. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
ChunxueLi
|
b9613ee86b
|
feat(node): wrapped-client HTTP consumers + leading-prefix template stripping (#3111)
* feat(http-patterns): Patch 8 - extract enterprise wrapped-client HTTP consumers
Recognize the enterprise axios-wrapper call shape X.request({ url, method })
(e.g. httpClient.request from @winex-plugin/win-request) as a consumer
contract source, and strip the leading gateway/service-prefix template
variable so consumer paths align with backend provider routes.
Effect on sr-next group: 1740 contracts / 0 cross-links ->
3540 contracts / 883 exact cross-links (14 frontend repos <-> backend/opt).
Not submitted upstream yet; see CUSTOM_PATCHES.md Patch 8 for details.
Co-Authored-By: Claude <noreply@anthropic.com>
* feat(node): wrapped-client HTTP consumers + leading-prefix template stripping
Consumer extraction for enterprise axios-wrapped clients:
- X.request({ url, method }) member form (httpClient.request from
win-request and friends): any .request(options) call with url/method|type
string props; the url template literal is split on ${...} spans and the
longest /-leading literal segment is kept.
- Leading ${...} gateway/service-prefix variables are stripped as
consumer-path normalization semantics in normalizeHttpPath
(stripLeadingTemplatePrefix): `${client}/api/v1/x` → /api/v1/x for
fetch/axios/wrapped shapes alike. Mid/tail interpolations still round-trip
through {param}; a stripped remainder not starting with / is dropped
(same rejection static relative urls get at scan time).
- %7B/%7D unescaping for absolute-URL branches.
On our 16-repo frontend monorepo this took group sync from 1740
contracts / 0 cross-links to 5111 / 2097 (exact links, 16/16 repos linked).
* fix(route): preserve upstream symbol-resolution machinery; strip only
Rebasing correction: an earlier iteration of this change simplified the
symbol probing to a bare line-1 offset and dropped the exact-module match,
which mis-attributed data-table handlers to decoy same-name symbols
(6 data-route-table regressions). Restore the upstream probing
(toZeroBasedLine + exact-module resolution) wholesale; the deltas this
change actually needs are the pure ones: stripLeadingTemplatePrefix,
normalizeConsumerPath returning null for un-reducible urls (callers
drop), and the %7B/%7D brace restoration in the absolute-URL branch.
* style: prettier
* Address PR review feedback (#3111)
Pass wrapped-client URLs through shared consumer-path normalization instead of the longest-segment reducer, emit * for present-but-non-literal methods, restore {param} via a sentinel so literal %7B segments stay encoded, and drop unused decorator helpers.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3111)
Gate wrapped X.request({url}) on axios-proven receivers or a small wrapper allowlist so cy.request/queue.request cannot mint HTTP consumers.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#3111)
Use a private-use sentinel so literal path segments are not rewritten, trim wrapped URLs before the scan-time path gate, and treat interpolated/shorthand methods as *.
Co-authored-by: Cursor <cursoragent@cursor.com>
* chore(autofix): apply prettier + eslint fixes via /autofix command
* fix(http): honest wrapped-request methods and tighter admission (#3111)
Quoted keys and object spreads were minted as GET; drop spelling-only `api`,
align gateway-prefix member verbs with prefix-strip, and keep absolute URLs.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: l.cx <l.cx@winning.com.cn>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: ChunxueLi <mecoloud@users.noreply.gitee.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
||
|
Carter LaSalle
|
f34daea86a
|
fix(routes): connect decorator routes to their handler function (#2865)
* fix(routes): connect decorator routes to their handler function
A Route node's only relationship was HANDLES_ROUTE from its FILE. The graph knew
a route existed and which file declared it, but not which function implemented
it. Two consequences on a 12.4k-file repository with 162 FastAPI routes:
- Every decorated handler was indistinguishable from dead code. Its sole edge
was DEFINES, so a reachability query reported it unreferenced even though the
framework invokes it on every request.
- `route_map` / `api_impact` could only answer at file granularity, and
`processes.ts` routed every route through its `routesWithoutHandlerByFile`
fallback instead of keying by handler.
Two halves of one gap, both already designed for and neither wired:
1. `ExtractedDecoratorRoute.handlerName` is documented as "captured at extraction
where the decorated definition node is in hand", and `resolveRouteHandlerSymbols`
already consumes it to stamp `handlerSymbolId`. Only the Spring extractor ever
set it, so for every decorator-routed framework — FastAPI, Flask, NestJS — it
arrived undefined and 0 of 162 routes carried a handler. A route decorator's
parent IS the decorated definition, so the name is in hand: add
`decoratedDefinitionName` and thread it through. It climbs consecutive
decorators so stacked forms (`@router.get(...)` over `@requires_auth`) resolve,
caps the climb so a malformed tree cannot loop, and returns undefined rather
than guessing — the routes phase already treats a missing name as
"fall back to file-level".
2. With a handler symbol resolved there is finally something to point an edge at.
Emit a definition-level HANDLES_ROUTE alongside the file-level one. The sibling
decorator overlay already does exactly this: `pipeline-phases/tools.ts` anchors
HANDLES_TOOL on the definition the decorator sat on, not its file. Routes were
the outlier.
Kept as one change because the edge is inert without the symbol — emitted from a
branch lacking part 1 it produces zero edges, since `handlerSymbolId` is empty.
Additive, and both existing consumers are unaffected:
`group/extractors/http-route-extractor.ts` types its query `(handlerFile:File)`;
`manifest-extractor.ts` matches an untyped `(handler)` but takes `LIMIT 1` ordered
by `handler.id`, and `File:…` sorts before `Function:…`, so its selected row is
unchanged.
Direction is Function → Route, matching how every other overlay attaches
(MEMBER_OF → Community, STEP_IN_PROCESS → Process, HANDLES_TOOL → Tool: the symbol
is the source). That also keeps it free of schema risk — `Function|Route` is
already declared by the ATTACHMENT rule in `lbug/schema.ts`
(`DEFINITION_ANCHOR_LABELS × ATTACHMENT_TARGET_LABELS`), which that file documents
as deliberate headroom for this case. Route → Function would have needed a new
hand-listed pair, and an undeclared pair aborts `analyze` outright — a failure
that file records having hit four separate times.
Verified on a FastAPI fixture (edges 9 → 11):
api.py (File) -> GET /widgets, POST /widgets [unchanged]
list_widgets (line 10) -> GET /widgets [new]
create_widget (line 15) -> POST /widgets [new]
On the 12.4k-file repository: 161 of 162 routes now resolve to their handler
function, up from 0. The single abstention is `uniqueSymbolId` correctly refusing
to guess where the name is not uniquely resolvable in its file.
`npx tsc --noEmit` clean; schema-pair coverage and route suites pass (196 tests).
* fix(routes): harden decorator handler attribution (#2865)
Keep definition-level route links correct across warm caches and malformed symbol lookups, and avoid per-route group-sync scans. Move Python AST ownership behind the language provider and add end-to-end regression coverage.
Note: full npm test could not complete in this container due unrelated worker startup failures and a stalled retry; targeted route suites, typecheck, format, and lint passed.
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(routes): reuse per-file symbol lookup and drop duplicate warm-cache test
Share extract()'s CONTAINING_QUERY memo with the graph provider path, resolve each route handler once, and fold the decorator-edge warm-cache assertions into the existing FastAPI composed-route round-trip.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Carter LaSalle <carterlasalle@gmail.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
||
|
ChunxueLi
|
72ff2b9453
|
feat(jvm): fold Java static wildcards and Kotlin star imports for route constants (#3110)
* feat(java): expand wildcard static imports + POSIX-space import resolution - `import static a.b.C.*` records the class FQN at extract time (ModuleConstants.wildcardImports) and expandJavaWildcardStaticImports materializes the bare-name bindings once the repo constants map exists, mirroring explicit single-member static imports. Wired in both the group-side prepareRepo fold and the ingestion-side parse-impl pass so the two query surfaces cannot diverge. Without this, wildcard-imported route constants (~693 routes on our monorepo) silently failed to fold and their provider contracts were dropped. - resolveJavaImport compares in POSIX space (backslash-normalized repo keys) — on Windows the '/'-joined class file never matched a backslash-keyed repo (observed: 675 calls, zero hits). - Wildcard bindings never overwrite single imports (a member shadowing its own wildcard is honored); unresolved wildcards degrade to the existing skip floor. Rebased onto current main: the parse-worker gate this originally carried is superseded by the provider moduleConstantHeuristic architecture; only the resolver-side wildcard expansion and POSIX tolerance remain. * chore(cache): claim SCHEMA_BUMP 83 (82 taken upstream by Spring lookup facts) * style: prettier * fix(java): make wildcard static imports actually resolve route constants extractJavaModuleConstants never populated wildcardImports, so `import static a.b.C.*;` was inert: the asterisk is a sibling of scoped_identifier in tree-sitter-java, not a path segment. Record the class FQN there and stop binding the class simple name as a field. Wildcard-only files also fell through every harvest gate (the java provider heuristic regex, the parse worker emit check, and the group prepareRepo filter), so the constants never reached either layer. Expansion now resolves targets against constant-defining files only, keeping ingestion and group in parity (#2980 R4). Adds unit coverage for extraction, expansion/shadowing, the unresolved skip floor, the harvest heuristic, Windows path keys, and group <-> ingestion parity. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(kotlin): fold package-star imports for route constants Kotlin `import pkg.*` now records star scopes and resolves unique top-level names after local and explicit imports, matching the Java wildcard path without accepting invalid object-star imports. Reuse a Java constant-file suffix index across expansion so wildcard materialization stays linear as both constants and importers scale. Add named-vs-star benches and CI --check gates. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(jvm): fold wildcard imports without shadowing locals or skipping same-package names Keep Java unfoldable declarations from being resurrected by static wildcards, bind only the target type's members, and prefer Kotlin same-package names over package-star imports. Move repo-wide preparation behind a language-provider hook so the shared parse phase stays language-agnostic. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3110) - Stop treating a Java static wildcard as a type import for qualified refs. - Harvest only class and static (including on-demand) imports, not import pkg.*. - Let harvested Kotlin top-level names shadow same-package star imports. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3110) - Fold Kotlin classifier-star imports (`import Type.*`) the same way package stars already fold. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3110) - Rebuild the Kotlin constant index when overlay replaces a contributing file. - Document the wildcard shape in the Java pipeline e2e fixture. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: l.cx <l.cx@winning.com.cn> Co-authored-by: ChunxueLi <mecoloud@users.noreply.gitee.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Gergő Magyar
|
66b44afe8c
|
fix(group): make degraded links, sync warnings and UID-only impact actually work (#3113)
* feat(group-surface): impact selector pass-through + degraded links + sync hygiene
- @group impact forwards target_uid/file_path/kind through service port
and cross-impact impactParams (was dead-wired: params accepted at MCP
boundary then dropped at validation).
- crossLinks with unresolved provider symbols carry degraded: true,
derived at the persistence boundary after merge/dedupe; sync reports
'degraded links: N' and per-repo extraction failures instead of
swallowing them; bridge write failures surface as sync warnings;
contracts.json passes through dedupeContracts.
- Absolute-URL branch restores %7B/%7D around {param} after URL parsing.
- tests: consumer matrix + wildcard folding + degraded pins (261 new);
SCHEMA_BUMP pin 47 -> 48 (wildcardImports cache shape); sync.ts NUL
byte rewritten as text escape (no longer binary to git).
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(group): impact selector pass-through, degraded links, sync failure hygiene
- @group impact forwards target_uid/file_path/kind through the service
port into cross-impact impactParams. These were accepted at the MCP
boundary and then dropped in validation — a dead wire: disambiguating
an ambiguous impact target never actually reached the per-member impact.
- Cross-links whose provider endpoint never resolves to a graph symbol are
marked degraded: true at the single persistence boundary (post
merge/dedupe, before re-export), counted as SyncResult.degradedLinks,
and surfaced by the sync summary ('degraded links: N') — the remedy
(re-analyze the provider repo) is documented on the field.
- Sync failure hygiene: a repo whose per-repo extraction throws records
its reason in SyncResult.failedRepos (still lands in missingRepos, so
downstream semantics are unchanged) instead of the old silent swallow
that could persist half a repo's contracts; operator warnings
accumulate in SyncResult.warnings.
Tests: cross-impact selector threading, degraded-link marking, per-repo
failure reporting.
* style: prettier
* fix(group): make degraded links, sync warnings and UID-only impact actually work
The three fixes this branch claims were wired at the type and payload level
but never at the boundary that produces the values:
- `degraded` was only ever cleared by the exported `dedupeCrossLinks`, which
the sync path does not use, so `degradedLinks` was always 0. Derivation now
lives in one exported `applyDegradedFlag` that both the sync finalize and
the post-merge re-derivation call.
- The bridge-write catch logged an operator warning and dropped it, leaving
`warnings` permanently `[]`.
- `@group impact` rejected a UID-only call before it parsed `target_uid`, so
the documented "re-call with target_uid" disambiguation loop was
unreachable in group mode even though the selectors were forwarded.
- `failedRepos[].repo` reported the registry display name while the repo
landed in `unreadableRepos` under its group path, so the two lists could
not be joined; the JSDoc also pointed at the wrong list.
- Restored the truncated `READ THE RESULT:` heading in the group_sync tool
description and documented degradedLinks / failedRepos / warnings.
Tests pin each value at the boundary that produces it, including the exact
group_sync wire shape, which previously omitted all three new fields.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: l.cx <l.cx@winning.com.cn>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: ChunxueLi <mecoloud@users.noreply.gitee.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
||
|
Gergő Magyar
|
e04e1ecc65
|
fix(group): parse Maven child coordinates independently of parent POMs (#3108)
* fix(group): parse Maven child coordinates independently of parent POMs Stop treating inherited parent groupId/artifactId as the child's identity so sibling repos no longer collide and workspace manifest links can resolve. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): parse Maven POMs with fast-xml-parser Replace the hand-rolled tokenizer so child identity and CDATA/namespaces stay accurate, and collect only project.dependencies so BOM, profile, and plugin entries cannot create workspace links. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): parse Gradle identity, catalogs, and named coordinates Read gradle.properties, settings.gradle, and the default libs.versions.toml catalog so workspace links work without executing Gradle, matching the static POM contract. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): resolve Kotlin Gradle DSL workspace coordinates Honor Kotlin named arguments, catalog get()/asProvider(), type-safe projects.* accessors, and ksp/kapt/commonMain configs without executing Gradle. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3108) Recognize Gradle group inside allprojects { } and Groovy name-first map coordinates so workspace identity and deps match common DSL forms. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * Address PR review feedback (#3108) Restore XMLParser.parse for POMs after /autofix swapped in tree-sitter parseSourceSafe, and match underscore catalog aliases from Gradle files. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Gergő Magyar
|
7e993ab897
|
fix(group): fail ambiguous sync names and honor analyze --name (#3094)
* fix(group): fail sync when a member name is ambiguous Silent first-match bound the wrong clone when --allow-duplicate-name registered two paths under one alias. Refs #3028. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(analyze): apply --name on the already-up-to-date path A rename should not require --force when the index is already current. Register before the same-commit branch restamp. Refs #3028. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): hint member path when impact --repo is an alias $localRepo stays the yaml key; joining on the registry alias is a non-join. List matching keys so operators can retry. Refs #3028. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(group): keep injected sync and alias hints consistent Workspace-deps path maps reuse the resolved handle so duplicate names cannot throw after an injected resolver. Alias hints match case-insensitively. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Gergő Magyar
|
54f97c86c7
|
fix(impact): make File risk comparable via shared axes (#3075) (#3082)
* docs(plans): add impact file risk plan Capture the evidence, constraints, and verification path for fixing incomparable File and symbol impact risk. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(impact): centralize risk scoring Keep the existing thresholds in one shared scorer and expose a common-axis comparison for targets with unavailable enrichment axes. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(impact): expose incomparable file risk scale Mark File impact results when process and module axes are unavailable, and provide a common-axis score for honest cross-kind comparisons. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(impact): explain cross-kind risk comparisons Surface the common-axis score in CLI and agent guidance while reusing the shared threshold ladder in the web impact tool. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(impact): fail closed when enrichment is incomplete Preserve proved HIGH/CRITICAL process counts, treat failed queries as UNKNOWN, and surface riskScale metadata on MCP, group, CLI, and Graph-RAG File walks. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
azizur100389
|
f64cc8b7a8
|
feat(group): add GraphQL cross-repo contracts (#3070)
* feat(group): add GraphQL contract extraction * fix(group): tighten GraphQL contract guards * fix(group): complete GraphQL review hardening * fix(group): isolate bounded GraphQL reads * fix(group): harden GraphQL contract extraction |
||
|
Przemek Poppe
|
b059ab3541
|
PHP: detect generated-client Request(method, host . resourcePath) consumer shape (#3079)
* feat(group/php): detect generated-client Request(method, host . resourcePath)
openapi-generator-php / swagger-codegen PHP clients build every operation as
`$resourcePath = '/foo/bar'; ...; new Request($method, $host . $resourcePath);`
— a shape the PHP consumer patterns didn't cover (only `$client->verb('/path')`
literal calls were matched), documented in the module's own docblock as a
follow-up ("constant-folding the surrounding scope").
Adds a pattern for `new [Qualified\]Request(...)` constructor calls, with a
conservative, single-scope backward constant fold: the last variable in the
path argument's concatenation chain (generated clients build
`<host> . <resourcePath>`) is resolved to a `$var = '<literal>';` assignment
in the same enclosing function/method body (or file scope) if one exists
earlier in the same scope. No interprocedural resolution — a miss just
leaves the endpoint undetected, never a wrong one.
The HTTP verb is often itself a parameter in these generated clients (not a
literal at the call site), so when it can't be resolved to a literal the
detection reports a wildcard method (`'*'`), consistent with this project's
existing manifest-link convention for a contract whose verb isn't pinned.
`hasConsumerSignals` is widened to stay a proven superset of what `scan()`
now detects (required by its own contract, checked by
`http-consumer-signals.test.ts`).
The docblock notes this is a deliberately narrow, single-scope fallback, not
this language's entry into the shared cross-file constant-fold the other
languages use (`constant-resolver.ts`, wired in via `java-const-resolver.ts`
/ `python-const-resolver.ts` / `js-const-resolver.ts`) — PHP has no such
binding yet; adding one is a separate, larger project (this repo's PHP
import resolution for `use`-statements is its own multi-file subsystem built
for symbol/scope resolution, not constant extraction) and is out of scope
here.
Tests: 7 scan()-level cases (resolution across a member-access host,
fully-qualified class name, purely literal call, negative — different
function scope, negative — non-Request constructor, negative — non-HTTP
literal, picking the LAST var in a 3-part concatenation), plus 2 new
hasConsumerSignals cases and a negative. `tsc --noEmit` clean, `eslint`
clean, full test/unit/group green (1035+/1037; 2 pre-existing native EBUSY
failures on a `.lbug` file in bridge-meta-swap-window.test.ts, unrelated
subsystem, reproduces in isolation on unchanged upstream too).
* fix(group/php): fix two code-review findings in guzzle-request-ctor
1. lastConcatVariable silently returned the WRONG variable for a
parenthesized right operand: `$host . ($resourcePath . $suffix)` fell
through to the left operand (unhandled parenthesized_expression) and
returned $host instead of looking inside the parens. Restored parenthesis
unwrapping (present in an earlier draft, dropped during a simplification
pass that didn't account for this fallthrough).
2. resolveLocalStringLiteral stopped at the nearest compound_statement, so
a `new Request(...)` call nested in `if`/`try`/`foreach` inside the same
function couldn't see an assignment made just above that block — despite
the docblock's claim of covering the "enclosing function/method body".
Now widens level by level (search the immediate block's preceding
statements, then its own enclosing block, and so on), stopping at
`program` so it still never crosses into a different function or the
containing class body — verified by a regression test asserting exactly
that boundary.
Also documents the line-number choice (path argument, not the `new Request(`
call site — the two differ for this pattern's characteristically
multi-line calls) inline, matching the other three consumer patterns'
convention in this file.
4 new regression tests (35 total in this file's suite): parenthesized
right operand no longer mismatches, enclosing-block resolution across an
`if`, and a negative case proving the widened search still respects the
function boundary. tsc --noEmit clean, eslint clean.
* fix(group/php): address gitnexus-check bot review on PR #3079
1. resolveLocalStringLiteral fell through an intervening non-literal
reassignment: `$v = '/old'; $v = buildPath(); new Request(..., $v)`
resolved to '/old' even though $v never holds that literal at the call
site. The NEAREST assignment to the target variable now decides the
outcome unconditionally — a non-string RHS stops the search (returns
null) instead of letting the scan continue past it to an older,
shadowed literal. This was a real "wrong answer", not a miss, directly
contradicting the function's own documented invariant.
2. lastConcatVariable recursed into every binary_expression regardless of
operator, so `$host && $resourcePath`, `$host + $resourcePath`, and
`$host ?? $resourcePath` were walked exactly like `.` concatenation.
Now checks operator === '.' before recursing.
3. hasConsumerSignals matches case-insensitively (`/i`), correctly, since
PHP class names are case-insensitive at the language level — but scan()
compared the resolved class name to 'Request' case-sensitively, so a
valid `new request(...)` / `new \NS\REQUEST(...)` call would pass the
parse-skip gate as a signal and then be silently dropped by scan()
itself. Both sides now agree (case-insensitive compare in scan() too).
4. The first test's own PHP source assigned `$method = 'POST';` as a local
variable but asserted `method: '*'` with a comment calling it "a
parameter" — it wasn't; it was exactly the same locally-resolvable shape
as $resourcePath. Fixed by (a) rewriting that test's source to show
$method as a genuine function parameter (the shape generated clients
actually use — the verb is fixed by the caller of the builder method),
which is what the test intended to demonstrate, and (b) actually
implementing symmetric resolution: method now resolves through the same
resolveLocalStringLiteral fold as path when it IS a local variable,
with a new test proving that case resolves to a literal method instead
of a wildcard.
5 new regression tests (39 total in this file's suite, up from 35):
non-literal-reassignment shadowing, non-concatenation operator rejected,
case-insensitive class name match, and local-variable method resolution.
tsc --noEmit clean, eslint clean.
* fix(group/php): address second round of gitnexus-check bot review
1. Backward fold missed reassignments nested inside a preceding if/foreach/
try/switch: the scan only recognized direct expression_statement
siblings as candidate assignments, so `if ($cond) { $v = '/new'; }`
right before the call was invisible, and an OLDER, now-shadowed literal
outside that block was returned instead — a real wrong answer whenever
that branch runs. Any non-assignment sibling that contains an assignment
to the target ANYWHERE inside it now stops the search (miss) rather than
being skipped over, since whether that branch ran is unknown.
2. Level-by-level scope widening crossed anonymous-function boundaries
without checking PHP's actual capture rule: closures capture NOTHING
automatically, only variables listed in `use (...)` are visible inside
— unlike arrow functions, which auto-capture everything and have no
`compound_statement` body (never seen as a scope by this walk at all).
Widening past a closure's body now checks its `use (...)` clause first;
real PHP would throw "Undefined variable" for anything not captured,
not resolve to a value from the enclosing scope.
3. lastConcatVariable still fell through to the LEFT operand whenever the
right one wasn't a variable-or-nestable-expression — `new Request($m,
$host . '/users')` (a trailing string literal, not a variable) resolved
to $host instead of recognizing there's simply nothing to resolve at
that position. Removed the left-operand fallback entirely: the
rightmost position decides, full stop, matching the function's own
"single lookup, not a fallback list" docblock (which the previous round
already stated but the code didn't yet fully honor for this case).
Also strengthened a test that the bot correctly flagged as non-diagnostic:
"ignores an unrelated constructor" used an unresolvable $resourcePath, so
it would have passed even with the class-name filter deleted. Now uses a
fully resolvable path so the class-name filter is what the assertion
actually exercises.
4 new regression tests (43 total, up from 39): shadowed-by-conditional-
reassignment, closure boundary without use()-capture (negative), closure
boundary WITH use()-capture (positive control), and trailing-literal
concatenation no longer mistaken for the host variable.
tsc --noEmit clean, eslint clean.
* chore: trigger re-review (previous gitnexus-check report cited stale line numbers)
* fix(group/php): stop scope widening at a function/method boundary
The digest posted on PR #3079 (verified against the current file, not the
stale HEAD it was generated from — three of its four findings were already
fixed in prior commits) reproduced a real, still-present fourth issue:
after exhausting a method's own body, widening continued straight to
`program` (file/script scope) and could resolve a top-level variable into
a class method — but PHP methods (and plain functions) have NO access to
file-level variables without an explicit `global $v;`, which this resolver
intentionally never adds support for. A file-level `$resourcePath = '/x';`
could therefore leak into an unrelated method's `new Request(...)` as a
real, wrong answer.
Widening now stops unconditionally at a `function_definition` or
`method_declaration` boundary — these get no automatic capture and no
implicit global in PHP, unlike closures (already handled: an
`anonymous_function` boundary stops unless `$target` is `use()`-captured).
The call-site-at-file-scope case still resolves correctly, since `program`
is reached directly there with no boundary to cross.
3 new regression tests (46 total): file-scope variable does not leak into
a class method, does not leak into a plain top-level function either, and
a positive control confirming file-scope-to-file-scope resolution still
works when there's no function boundary at all.
tsc --noEmit clean, eslint clean.
---------
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
|
||
|
glier
|
880f94d147
|
feat(group): resolve Kotlin constant-based route paths (@PostMapping(ApiPaths.X)) (#3059)
* feat(ingestion): resolve Kotlin constant-based route paths
`route-extractors/constant-resolver.ts` is the language-agnostic core for
folding constant references into literal route paths. Bindings existed for
Java, JavaScript/TypeScript and Python, but not Kotlin — so
`@PostMapping(ApiPaths.ORDERS)` resolved on Java sources and silently
produced no route on the identical Kotlin code.
Add `route-extractors/kotlin-const-resolver.ts` as the fourth binding,
mirroring `java-const-resolver.ts` (Kotlin shares the JVM package/import
model) in structure, naming and skip-floor discipline:
* `resolveKotlinImport` — import specifier -> file path. Tier 1 matches the
`<package>/<Name>.kt` convention; tier 2 falls back to the unique
constant-defining file in the package directory, because Kotlin does not
require a file to be named after the declaration it holds. Each tier is
unique-or-nothing.
* `extractKotlinModuleConstants` — parse tree -> `ModuleConstants`.
* `parseKotlinConstOperands` / `foldKotlinOperands` — pre-bound wrappers so
the calling side stays language-neutral, matching how `spring.ts`
consumes `parseJavaConstOperands`.
Kotlin-specific forms handled explicitly rather than translated from Java:
top-level `const val`, `object` members, `companion object` members (keyed
under the enclosing class, since `Companion` never appears in a reference),
import aliases (`import a.b.C as D`), and the absence of a `String` type gate
(Kotlin infers property types, so the initializer decides). String templates
and multi-line raw strings are refused rather than folded with the
interpolation dropped, which would publish a path the application does not
serve. `var`, custom getters and delegates are not constants and are skipped.
Wiring: `group/extractors/http-patterns/kotlin.ts` gains a `prepareRepo`
pre-pass that builds the repo-wide constant map once per `extract()` run, and
`scan` now folds constant-valued `@(Get|Post|Put|Delete|Patch)Mapping`
arguments against it — the same shape the Java plugin already implements. A
constant-valued class-level `@RequestMapping` prefix suppresses the routes
under that class, as in `java.ts`: emitting them unprefixed would turn a
missing fact into a wrong one.
An ambiguous import returns null, never a guess — a duplicate fully-qualified
name across modules, a package with two constant files, and a wildcard import
all floor to skip. A wrong resolution is a false edge in the graph; a missing
one is only a missing fact.
The ingestion provider (`languages/kotlin.ts`) is deliberately left alone: the
ingestion fold runs only over `decoratorRoutes`, and Kotlin declares no
`extractDecoratorRoutes` because `spring.ts` is bound to tree-sitter-java.
Declaring the constant hooks there today would harvest a map nothing consumes.
The reasoning is recorded in the new module's header.
Tests cover each reference form (qualified, fully-qualified, single-name
import, concatenation incl. 3+ operand chains) plus every ambiguity case, at
both the resolver layer and through `KOTLIN_HTTP_PLUGIN.prepareRepo` + `scan`.
* test(group): cover the Kotlin route-fold guards left unpinned
Follow-up to the Kotlin constant-route binding, closing the test gaps a
review found. No behavior change: the only source edit is a comment.
* Class-prefix suppression is now asserted for the NAMED spelling
(`@RequestMapping(value = ApiPaths.BASE)`) as well as the positional
one. The two take different branches of `kotlinRouteArgumentExpression`,
and only the method-path side of the named branch was covered; a
regression there would let a constant-prefixed class escape suppression
and publish every method under it at an unprefixed path.
* `MAX_FOLD_LENGTH` and both recursion caps are pinned from both sides.
Output doubles per level while depth only increments, so 13 doublings of
a one-character leaf land exactly on the limit and 14 overrun it; a
30-link reference chain resolves where a 40-link one hits the
cross-file cap; an 80-term `+` chain hits the operand-parse cap where a
60-term one folds. A 30-level shared-descendant DAG folding inside a
5 s budget pins the success memo that keeps it out of O(2^depth).
These are the guards that keep a pathological constant graph from
building a gigabyte-scale string or recursing without bound during a
group sync; they were inherited from the audited Java binding but
nothing held them in place.
* OpenFeign consumers are covered on both paths: a constant method path
folds, and a constant interface-level `@RequestMapping` prefix
suppresses the consumer. The latter is deliberate — Spring Cloud
prepends a type-level `@RequestMapping` to every method of the client,
so an unfoldable prefix makes the remote URL unknowable whether or not
`@FeignClient(path)` is present, and a dropped edge beats a wrong one.
The suppression reaches an interface because tree-sitter-kotlin models
`interface` as a `class_declaration`; `java.ts` misses this case only
because its `findEnclosingClass` skips `interface_declaration`, and
aligning Java changes Java's behavior, so it is left to its own change.
Documented at the guard so the divergence is not read as an oversight.
Note for reviewers of the parent change: re-indexing a Kotlin Spring
service will REMOVE routes that were previously emitted, unprefixed, from
classes whose `@RequestMapping` prefix is a constant. Those paths were
never served by the application; the drop is the fix, not a regression.
* fix(group): suppress Kotlin routes only when the class prefix resolves to no literal
Class-prefix suppression decided "is this prefix unresolvable?" from a
three-element allow-list of node types (`simple_identifier`,
`navigation_expression`, `additive_expression`). An allow-list is safe for
FOLDING, where a forgotten shape yields no route, but it is the wrong shape
for SUPPRESSION, where a forgotten shape means "emit unprefixed" — a route
the application does not serve. `java.ts` gates on the ABSENCE of a literal
(`if (!valueNode)`) for exactly this reason.
The predicate is now inverted: a class is marked unless its `path`/`value`
argument is provably literal, recursing into `[…]` and `arrayOf(…)`
elements and refusing an interpolated `string_literal`. Measured against
the previous behavior, with `@PostMapping(ApiPaths.ORDERS)` under each
class prefix, on an app serving `/api/v1/orders`:
* `[ApiPaths.BASE]` `POST /orders` -> dropped
* `arrayOf(ApiPaths.BASE)` `POST /orders` -> dropped
* `value = [ApiPaths.BASE]` `POST /orders` -> dropped
* `buildPath()` `POST /orders` -> dropped
* `if (USE_V2) "/api/v2" else …` `POST /orders` -> dropped
* `"${ApiPaths.BASE}"` `POST /${ApiPaths.BASE}/orders` -> dropped
The last one published raw source text as a served path; refusing an
interpolated literal also fixes it for LITERAL method routes, which emitted
`/${ApiPaths.BASE}/list` before this branch existed.
Two regressions this suppression had introduced are repaired, both by
consulting the literal-prefix map that the pass above already built and
declining to mark a class that has an entry in it:
* `@RequestMapping("/lit", ApiPaths.BASE)` + `@GetMapping("/list")` lost
`GET /lit/list` entirely. Kotlin's vararg spelling leaves a resolvable
arm behind, and suppression exists to avoid wrong routes, not to
discard right ones.
* `@FeignClient(path = "/api")` + `@RequestMapping(ApiPaths.BASE)` lost
its consumer, though `path` outranks `@RequestMapping` when the URL is
assembled and made the prefix perfectly knowable.
Two Feign emission paths never consulted the unfoldable set at all:
* `@FeignClient(path = CONST)` was invisible to the analysis, which
matches `@RequestMapping` only, so the client fell through to the
no-prefix fallback and published `GET /orders` for a call the service
makes to `/api/v1/orders`. Collected as its own set, kept separate
because `path` outranks `@RequestMapping` in both directions.
* The `@RequestLine` loop resolves through the identical "path wins"
fallback chain but had no guard, so one interface could suppress its
`@(Get|…)Mapping` route and publish its `@RequestLine` route under the
very same unresolvable prefix. Both lanes now judge alike.
Note for reviewers: the `@RequestLine` guard is not a regression fix — that
lane emitted a wrong unprefixed consumer before this branch too. It moves a
wrong route to no route, on both sides of the change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(group): fold Kotlin route constants on Windows and when they fold to ""
Two defects in the Kotlin constant-path fold, plus the documentation
corrections review asked for.
Windows keys. `resolveKotlinImport` turns an import specifier into
`com/example/ApiPaths.kt` and asks whether a repository key ends with it.
The orchestrator's key list comes from glob v13, which has no `posix: true`
and joins with the platform separator, so on Windows every key arrives
backslashed and that test can never pass: the pre-pass still ran, the repo
context was still built, and every cross-file fold returned null — the
headline feature silently absent on one platform. Every unit fixture spelled
its keys POSIX, so CI could not see it. Normalized at the one boundary that
produces the keys — `prepareRepo`'s map keys and `scan`'s `fileRel` — which
is the fix `node.ts` and `python.ts` already apply for the same reason.
`readFile` still receives the raw path. Normalizing inside the resolver
cannot work: it returns the key it matched, so a normalized return value
would miss in a map nobody normalized.
Empty fold. `foldKotlinOperands` collapsed `''` into `null`, conflating
"folded to the empty string" with "unresolvable". `const val ROOT = ""` is
Spring's spelling for the class prefix itself, so under
`@RequestMapping("/api")` the literal `@PostMapping("")` published `POST
/api/` while `@GetMapping(ApiPaths.ROOT)` published nothing. Return the fold
unfiltered: callers already guard on `=== null`, `resolveKotlinConstant`
already returned `''` for the same constant, and this matches
`foldJavaOperands`.
Docs. The module header claimed the fold, the cycle guard and the depth cap
all live in the agnostic core. They do not — roughly 200 lines are a local
fork of the Java binding's already forked state machine, because the core
keys its maps by simple name while a Kotlin operand can be qualified at any
position. Say that, with the reason and the follow-up. The stated
`isKotlinConstantFile` invariant ("never rejects a file the extractor
accepts") is false: the extractor harvests a top-level non-`const` `val`
that fails both gate arms. The cost is not nil, either — measured, such a
constant in its own file loses every cross-file route, while the same
declaration beside the route still folds through `scan`'s on-demand
re-extract. Both recorded on the gate. The depth caps are now
`MAX_OPERAND_PARSE_DEPTH` (64) and `MAX_FOLD_DEPTH` (32); the core's own
`MAX_RESOLVE_DEPTH` is 8 and module-private, so it cannot simply be reused.
Measured with a differential probe over 41 Kotlin fixtures against the PR
base, in both key styles. Exactly one POSIX row moves — the empty fold —
and every other row, controls included, is byte-identical to before. POSIX
and Windows keys now yield identical detections on every fixture, on both
sides.
Deliberately not done: the `isKotlinConstantFile` gap is documented, not
closed, because closing it means parsing every file that contains any `val`.
`java-const-resolver.ts` still spells 64 and 32 inline. The PR body's
rollout note still says re-indexing activates the change — `HttpRouteExtractor`
runs during `group sync` (`sync.ts:297`), so that is a PR-body fix, not a
code one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(group): key Kotlin constants by visibility and resolve imports on the declared package
Two ways the Kotlin route fold could publish a path the application does not
serve. Both were inherited from the merged Java binding, which documents each as
accepted; the notes were wrong, not merely conservative, and both are left open
as a Java follow-up rather than changed here.
Simple-name flattening. Every `object`/companion member was recorded under BOTH
its qualified name `Owner.NAME` and its bare `NAME` in one file-level namespace,
so an initializer naming a sibling resolved through whichever object was walked
LAST:
object A { const val BASE = "/right"; const val ROUTE = BASE + "/m" }
object B { const val BASE = "/wrong" }
@GetMapping(A.ROUTE) // Kotlin serves /right/m; this emitted /wrong/m
Swapping the two objects flipped the answer back — the same source, merely
reordered, changed the emitted route. The bare key is also a binding Kotlin does
not have: `BASE` alone never names `A.BASE` from outside `object A`'s body, and
because the fold consults literals before imports, that fabricated key outranked
a genuine `import com.example.api.Paths.ORDERS` and published the local object's
value instead of the imported one.
Keys now follow Kotlin's own visibility. A member of a named `object` gets only
`Owner.NAME`; the simple name is recorded for a top-level `val` and for a
companion member, which really is in scope unqualified throughout its enclosing
class. Initializers resolve against their scope chain, innermost first, so `BASE`
inside `object A` means `A.BASE` — collecting every declaration before recording
any is what makes that independent of declaration order. An unfoldable object
member no longer drops a same-named import either, since it shadows nothing.
The known limit is now stated rather than argued away: a companion's bare key is
still file-wide, so two companions in one file whose members collide still
resolve last-wins for an unqualified reference. Kotlin scopes that to the
enclosing class and this map cannot express it — the fold is entered with a file
key and a name, and nothing says which class body the annotation sat in.
Initializers are unaffected; only a bare annotation reference can land wrong.
Import binding never read the `package` header. Both tiers picked candidates
purely from the path, so a file whose PATH ended with the imported FQN beat the
real declaration — and when the decoy declared the same constant the fold did not
skip, it invented a value. Measured: `object ApiPaths { const val ORDERS = "/right" }`
in `src/generated/Constants.kt` (`package com.example.api`) plus a decoy at
`src/x/com/example/api/ApiPaths.kt` (`package x.com.example.api`) emitted
`GET /wrong`. This falsifies the old docstring's safety argument, which only
covered a wrong file that LACKS the name. Two further triggers: a root-level
`package data` was impersonated by `com/example/data` on a path-suffix test,
while the real root-level file was invisible to the package-directory tier at
all; and a unique constant file under a test source tree folded into a
production route.
The declared `package` is now recorded per file and matched exactly. Candidates
that declare a different package are rejected rather than guessed at, an entry
with no recorded package is rejected too, and two files declaring the same
fully-qualified name resolve to nothing — a duplicated FQN names no single
declaration, so the test-source copy of a production constant is a skip, not a
guess about build configuration this layer cannot see. The file-name convention
survives only as a tie-break among candidates that already declare the right
package. `packageName` rides on a Kotlin-local `KotlinModuleConstants` rather
than widening the agnostic `ModuleConstants`, which Java, JS and Python share and
none of them needs it.
Measured with a differential probe over all 41 Kotlin fixtures, in both key
styles. Seven rows move, all of them from a wrong route:
* sibling shadow, A first /wrong/m -> /right/m
* bare key beats import /wrong -> /right
* path-suffix decoy /wrong -> /right
* root-package suffix match /wrong -> /right
* root-package suffix only /wrong -> (skip; not in the repo)
* test copy into production /test-only -> (skip; FQN declared twice)
* wrong file lacks the name (skip) -> /right
The last row is the one control that changes, and it changes from emitting
nothing to emitting the route Kotlin serves: its decoy declares a different
package, so the unconventionally named real file is now the sole candidate.
Every other row, all six remaining controls included, is byte-identical to
before, and POSIX and Windows keys still agree on every fixture.
Deliberately not done: `resolveKotlinImport` does not PREFER the candidate that
declares the sought name when several share the package — it only rejects when
two do. Preferring it would resolve more imports correctly (a package holding
`ApiPaths.kt` that declares something else and `Constants.kt` that declares
`ApiPaths`), but it is a separate skip-to-route improvement that would rewrite an
assertion this suite already pins, and the review round did not ask for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(group): scope Kotlin companion constants to their class, and stop an empty path array suppressing routes
Three ways the Kotlin route fold still published the wrong answer, all measured
on fixtures rather than reasoned about, plus one comment that was false.
Empty path array. `hasResolvableLiteralPathElement` answered "does any element
resolve to a literal?", and `[].some(...)` is `false`, so `@RequestMapping(arrayOf())`
read as an UNRESOLVABLE prefix and suppressed every route under the class —
including a plain `@GetMapping("/lit")` that no constant fold ever touched.
Spring treats an empty array as NO prefix, so `/lit` is genuinely served. The
same arithmetic hit `@FeignClient(path = arrayOf())`, dropping a consumer.
Measured against the pre-suppression branch point:
* `@RequestMapping(arrayOf())` + `@GetMapping("/lit")` nothing -> GET /lit
* `@FeignClient(path = arrayOf())` nothing -> consumer GET /orders
The predicate is now a three-valued `classifyPathArgument`: `'literal'`,
`'none'` (an empty array — no prefix), `'unresolvable'`. Only the last may
suppress, because "no prefix" is not "an unresolvable prefix" and only one of
them makes the served path unknowable. `@RequestMapping([])` is the same idea
spelled differently, but tree-sitter-kotlin does not parse the class carrying it
as a `class_declaration` at all, so no class-prefix pattern matches it and no
arm here can be reached — recorded rather than guarded against.
Companion scope. A companion member's simple name was recorded into the SAME
file-level namespace as top-level constants, and companions are recorded last,
so it won every unqualified reference in the file — including from a class that
is not its own. Kotlin binds it unqualified inside its enclosing class body and
nowhere else:
* top-level `/top` vs an unrelated `Holder`'s companion `/companion`,
referenced from a third class `/companion` -> `/top`
* two companions colliding on one name `/h2` -> `/h1`
* `const val ROUTE = BASE + "/m"` at file level beside a companion `BASE`
`/comp/m` -> `/top/m`
* a single-name import losing to a same-named companion outside its class
Only a TOP-LEVEL `val` now writes a bare key. The unqualified binding is reached
from the reference site instead: `scan` collects the enclosing type chain of the
annotation and `foldKotlinOperands` rewrites a bare operand to `<Owner>.<NAME>`
when an enclosing type declares it — innermost first, before the file-level maps
and before imports, which is Kotlin's own order. So the companion still wins
inside its own class (the control that pinned this behavior keeps passing) and
loses everywhere else. Nothing was skipped to get there: every one of the four
cases now emits the route the application serves.
An unfoldable companion member still drops a same-named import file-wide. The
import map has no scopes, and over-deleting costs a route while under-deleting
publishes the imported value at a reference the compiler binds to the unfoldable
member.
Backtick quoting. `` package com.example.`api` `` and `package com.example.api`
are the same package to the compiler — the quotes are lexical syntax, not part
of the name — but the grammar keeps them in the node text, `declaredPackage`
joined them verbatim and `resolveKotlinImport` required an exact match, so the
sole real candidate was rejected and `GET /right` was lost. Every identifier
that becomes a map key or a lookup name is now read through
`unquoteKotlinIdentifier`: package segments, import specifiers and aliases,
declaration and member names, and references. Both directions matter — an import
may quote a segment the declaration spells plainly, and the reverse — and a
KEYWORD segment, which can only be spelled quoted, still folds.
Comment correction. The previous commit's note claimed "Sibling INITIALIZERS are
unaffected (they go through the scope chain above); only a bare reference from a
route annotation can land on the wrong companion." That is false, and the
`/comp/m` case above is the counterexample: a top-level initializer has an EMPTY
scope chain, so `qualifyRef` leaves its operand bare and the file-wide companion
key answered it. The source comment now describes what the code does; the claim
also appears in the `ef402a4a` commit body, which is already published and is
left as written.
Not changed. `resolveKotlinImport` computes `declaring` — the unique in-package
file that declares the sought name — and uses it only to REJECT when two files
declare it, never to resolve. With two or more in-package candidates it falls
through to the file-name convention and returns null, dropping a case Kotlin
resolves unambiguously. Returning it would flip the pinned assertion "returns
null when the package holds two constant files and no name matches" from skip to
route (that fixture's `Paths.kt` does declare `object ApiPaths`, so `declaring`
is not null there despite the test's title), so it is left open as a follow-up
rather than traded against a skip-floor assertion.
Fixture sweep: 82 cases in both POSIX and Windows key styles. Six move, all
listed above; the other 76 are byte-identical on both key styles, including the
companion-inside-its-own-class control, the qualified-reference cases, and the
pre-existing interface-inheritance gap on a constant controller prefix, which is
unchanged and remains a separate follow-up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(group): admit backtick-quoted constants, and overlay a same-file val that imports nothing
Two gate defects, both reported by the review bot and both reproduced before
being fixed.
`isKotlinConstantFile` matched only `\w+` for a declaration's name, so a file
whose constants are backtick-quoted — `const val ` + "`ORDERS`" + ` = "/orders"` — failed both
arms and was never parsed into the repo constant map. The resolver supports
backtick identifiers everywhere else: `unquoteKotlinIdentifier` strips the
quoting at every point a name becomes a key or a lookup. So the gate was
NARROWER than the extractor, which is the one direction its arms exist to
exclude, and a cross-file reference to such a constant floored to skip.
Measured: the route emitted nothing, and emits `GET /orders` now.
The on-demand overlay in `scan` admitted the file's extraction only when it had
imports. A file declaring a top-level non-`const` `val` is already excluded from
the pre-pass map (no `const`, no `object`), so this branch is its only chance,
and an import-only test discarded exactly the constants the route needed. The
guard now matches the admission test the pre-pass itself applies.
The bot stated this second one more broadly than it holds. Measured, any import
at all masks it — a realistic Spring controller always has one — so the failure
needs all three of: a top-level non-`const` `val`, no `object` in the file, and
no imports. Narrow, but real, and the fix costs one predicate.
Verified with the differential probe: both cases go from no detection to the
correct route, and all 41 existing fixtures are byte-identical before and after.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(group): let the resolver own Kotlin enclosing-type qualification
The route caller gated kotlinEnclosingTypeNames on its own copy of
foldKotlinOperands' bare-ref predicate. That gate could not change the
result — qualifyKotlinRefInEnclosingTypes returns a dotted name unchanged —
so it only spread one rule across two modules that can drift apart.
Also corrects a trimmed comment that claimed a collection_literal never
reaches classifyPathArgument, which the non-empty branch there disproves.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(group): keep unfoldable Kotlin constants as skip, not a wrong route
Record declared-but-unfoldable names so a companion or duplicate FQN cannot fall through to a foldable twin, and treat empty [] as no prefix on parsed RequestMapping arrays. Prefer the unique declaring file before package filename fallbacks so extra unfoldable files in the same package do not drop a real route.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(group): preserve full paths for nested Kotlin constants (#3059)
Key nested objects and companions by their full enclosing type path so same-file, imported, and bare nested references resolve consistently.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(group): qualify a PARTIALLY qualified Kotlin reference, not just a bare one
Both qualification points short-circuited on `name.includes('.')` — "already
carries its owner". A dotted reference carries AN owner, not necessarily its own
full one, and Kotlin resolves a partially qualified name against the enclosing
scopes exactly as it resolves a bare one.
Measured on the branch before this change:
* `object Outer { object Inner { const val Q = "/orders" }
@GetMapping(Inner.Q) … }`
emitted NOTHING. The key is `Outer.Inner.Q`; left unchanged, `Inner.Q`
matches nothing.
* a top-level `object ApiPaths { ORDERS = "/orders" }` beside
`class OrderController { object ApiPaths { ORDERS = "/inner" } }`, with
`@GetMapping(ApiPaths.ORDERS)` inside that class, emitted `/orders`.
The compiler binds the NESTED object, so the application serves `/inner`.
That is a wrong route, not a missing one.
* the same defect on the initializer side: `const val ROUTE = Inner.Q + "/m"`
inside `object Outer` emitted nothing, where Kotlin gives `/orders/m`.
Fixing only one side would leave the two halves disagreeing about what a dotted
name means, which is the asymmetry the earlier defects in this file came from,
so both move together:
* `qualifyKotlinRefInEnclosingTypes` drops the early return. The scopes it
walks are already qualified, so prefixing them onto whatever the reference
spells is the whole rule.
* `qualifyRef` splits at the last dot and prefixes the scope onto the OWNER,
so the bare case is byte-for-byte what it was.
The allocation gate in `foldKotlinOperands` loses its `!includes('.')` clause
for the same reason. It was not merely a missed optimization: it decided the
result per OPERAND LIST, so the same `Inner.Q` folded or not depending on
whether a sibling operand happened to be bare.
Verified with the differential probe: the three cases above go from wrong or
missing to correct, and all 41 existing fixtures are byte-identical to
|
||
|
DuduPhudu
|
0f793558ad
|
fix(group)!: stop group sync claiming matching it never did (#3020)
* fix(group)!: remove the matching cascade that was advertised but never built `gitnexus group create` wrote `matching.bm25_threshold` and `matching.embedding_threshold` into every generated group.yaml, and no matcher ever read either one. That was not the whole of it — an entire feature surface described a BM25/embedding cascade that does not exist: - `matching.bm25_threshold` / `matching.embedding_threshold` — parsed, persisted, unread - `detect.embedding_fallback` — defaulted and templated, unread - `MatchType` declared `'bm25' | 'embedding'`; both variants unreachable - `SyncOptions.skipEmbeddings` — declared in sync.ts and never read - `gitnexus group sync --skip-embeddings` — accepted, threaded through GroupService, ignored - CLI help in en and zh-CN promised "Exact + BM25 only (no embedding fallback)" - the MCP `group_sync` schema exposed `skipEmbeddings`, described as "Exact + BM25 only (Demo PR: same as default exact path)" `sync.ts` imports exactly `buildProviderIndex`, `runExactMatch` and `runWildcardMatch`, and the printed cascade has one stage. An operator whose links do not match reaches for those thresholds first, and turning either knob changes nothing — config that silently does nothing is how people conclude a feature is broken. Evidence that the cascade should be deleted rather than implemented, from a real backend/frontend pair: of 165 consumer contracts, 149 link exactly and 16 do not. Nine of the sixteen are third-party APIs (Google OAuth, Apple public keys, PostHog, image annotation) with no in-group provider by construction — similarity matching cannot recover them, it can only invent false links. Two are verb mismatches: the frontend calls `POST /links` and `GET /links/check-exists` while the backend declares `GET /links` and eleven other `/links/*` routes but neither of those, so a fuzzy path match would link a POST consumer to a GET provider. The rest are path-extraction artifacts. Roughly none of the sixteen would be correctly recovered, and several would be actively mis-linked. BREAKING CHANGE: `gitnexus group sync --skip-embeddings` and the MCP `group_sync` `skipEmbeddings` parameter are removed. Both were accepted and ignored, so no behavior changes — but a script passing the flag now fails with `unknown option` instead of being silently misled. Existing group.yaml files keep loading: the removed keys are simply no longer part of the schema, and a regression test pins that a legacy config carrying all three still parses. Closes #3006 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(group)!: honour --exact-only, drop inert --allow-stale, report every matching stage Addresses the review findings on #3020, all of which are the same defect the PR itself is about: group-sync surface that describes behaviour the pipeline does not have. `exactOnly` was inert in exactly the way `skipEmbeddings` was — declared on `SyncOptions`, threaded through the CLI and the MCP tool, and read by nothing — and strictly worse, because the stage it promised to suppress DOES run and DOES write `matchType:'wildcard'` links into contracts.json and the bridge, which `group impact` and cross-repo `trace` then traverse. It is now honoured rather than deleted: unlike the never-built BM25/embedding stages, the stage it names exists, so the flag describes a real choice. The substituted result is `{ matched: [], remaining: unmatched }`, not an empty result — `wildcard.remaining` IS `SyncResult.unmatched`, so skipping the stage has to leave its input unmatched rather than dropping it from the count an operator reads. `allowStale` had no such stage to gate: `syncGroup` emits no stale warning at any point (the `checkStaleness` call lives in `groupStatus`, a different path), so it is removed under the same rationale as `skipEmbeddings`. `group sync` now prints every matching stage instead of `exact` alone. The old block printed a `Matching cascade:` header and counted only exact links while the next line reported `result.crossLinks.length` — which also includes `manifest` and `wildcard` — so for any group with those the two numbers disagreed with nothing on screen explaining why. Counting is an exhaustive `Record<MatchType, number>`, so a new MatchType fails the build here instead of going silently uncounted, and reads through `?? 0` so a legacy registry carrying a removed matchType prints an honest count rather than `NaN`. Also: the MCP `group_sync` description no longer omits the wildcard stage that always runs, `exactOnly`'s description no longer refers to a "cascade", and bench/cross-repo-trace/verify.mjs no longer generates the removed threshold keys into a fresh group.yaml. Tests: `sync-exact-only.test.ts` pins both directions of the gate (mutation-verified: removing the gate, or returning `remaining: []`, both go red). `group-tools.test.ts` pins that the MCP schema dropped `skipEmbeddings` and kept `exactOnly`. `group-cli.test.ts` pins that both removed flags are rejected, with `--exact-only` as an accepted-flag control. `config-parser.test.ts` now pins that legacy keys are PRESERVED (measured, not assumed) rather than only that parsing does not throw. The type narrowing's fallout in test files is cleared: `tsc -p tsconfig.test.json` is 987 errors at head against 987 measured on origin/main, with the two error sets identical — zero net, zero new, zero masked. Verification: `tsc --noEmit` exit 0; prettier clean; eslint 0 errors (2 warnings, both pre-existing on base); 69 test files / 1169 tests green across test/unit/group, test/integration/group, tools, cli-i18n and cli-index-help. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(group): reject malformed and retired group_sync parameters (U1) `GroupService.groupSync` read `exactOnly` off an untyped MCP payload with `Boolean(params.exactOnly)`. While the flag was inert that coercion was harmless; now that it gates the wildcard matching stage, the string "false" -- a routine shape for an LLM caller emitting JSON -- is truthy, so a caller that asked to KEEP wildcard matching got it suppressed and a registry with fewer cross-links persisted to disk. The opposite of the request, written down. Validate instead of coercing, at the service boundary: the MCP SDK does not enforce a tool's advertised inputSchema and `callTool` is reachable directly, so this method is the real gate. The validator mirrors `validateImpactMode`'s `{ value } | { error }` shape -- the established idiom for this boundary, and the one groupSync's other guards already return through. Also refuse `skipEmbeddings` and `allowStale` by name. The CLI rejects them outright because commander errors on an unknown option; the MCP path accepted and silently dropped them, so an agent working from a cached tool schema was never told. Removing them took away discoverability, not acceptance. Both guards run before the group is read off disk, so a rejected call performs no work. Every test asserts the sync did NOT run -- an error string alone cannot distinguish "refused" from "refused but synced anyway". The tool description gains the validation note AFTER the registryOutcome paragraph: `tools.test.ts` slices that description by ordinal position of the 'preserved' / 'superseded' / 'no-prior-registry' literals, so appending past all three leaves those slices intact (verified, 44/44). tsc clean; 1039/1039 group unit tests pass. * fix(group): record the matching stages a sync was told to skip (U2) An `--exact-only` sync wrote a contracts.json and bridge with fewer cross-links and nothing recorded that the wildcard stage had been suppressed by request. `group_impact` and cross-repo `trace` read that registry as authoritative, so a narrowed graph was indistinguishable from a complete one -- and because `group_sync` is MCP-exposed, one agent call durably narrowed the shared answer for every later reader with no signal at all. Add `suppressedMatchStages` to ContractRegistry and SyncResult, following the `unreadableRepos` tri-state end to end: absent means a registry written before the field existed, `[]` is the measurement "this run suppressed nothing", and a populated list names the stages. The writer always emits it, because omitting the empty case is what made "measured, none" unreachable for `unreadableRepos`. Two properties that are easy to get backwards, and are why the split matters: - SyncResult carries the marker on EVERY outcome. The sync genuinely did skip the stage whatever happened to the file afterwards, and the CLI summary (U3) renders from this rather than re-deriving it from the caller's options. - The PERSISTED registry stamps it only on the `written` outcome. The preserve path re-writes `{ ...prior }`, so a carried-forward registry keeps the marker of the sync that actually produced its contracts instead of being relabelled with this run's request. That holds by construction: the registry literal carrying the field is only reachable on the written path. `loadContractRegistryResilient` gains an explicit line, because it rebuilds the envelope field by field with no spread of the parsed root -- a new on-disk field is silently dropped unless named there. Its reader is `recordedMatchStages`, not the existing `recordedRepoList`: that one validates `string[]`, which is right for repo names and one notch too weak here. This repo has already retired MatchType members ('bm25', 'embedding'), so a stale value on disk is a real shape, and dropping non-members keeps an unknown stage name from reaching a caller typed as a live one. Surfaced on `group_contracts` and on `group_sync`'s own return -- deliberately kept separate from the truncated/truncationReason/riskEpistemic triple. That triple reports limits a run hit by accident, whose remedy is to fix the repo; a suppressed stage was asked for, and its remedy is to re-sync without the flag. Conflating them would tell an agent to retry something that returns identically. tsc clean; 1043/1043 group unit tests pass. * fix(group): name a skipped matching stage as skipped, and pin it (U3, U4) Two facts were printing as the same line. `wildcard: 0 cross-links` meant both "the stage ran and matched nothing" and "the stage never ran because you passed --exact-only" -- the same conflation this summary block was introduced to remove one line up, reintroduced by the flag that made the block necessary. Render a suppressed stage as `skipped (--exact-only)`, driven by the sync's own `suppressedMatchStages` rather than by `opts.exactOnly`. The renderer reports what the sync did, not what the caller asked for, so it stays correct on the outcomes where the run ended without writing a registry -- which is where the summary is least legible and a re-derivation from the options would have been wrong. Also drops the `?? 0` fallback and the comment justifying it. The comment claimed a legacy registry could carry a retired matchType into this loop. It cannot: `syncGroup` returns a freshly computed `crossLinks` array on every outcome, and even on the preserve path the prior links go to disk while the fresh array is returned. The code was harmless; the stated reason was false, and a comment that explains an unreachable path is worse than no comment. U4 pins both halves through the CLI. A manifest fixture is sufficient: the stage counts must sum to the total on the `Wrote contracts.json (…)` line, and the skipped rendering does not need a stage to have matched anything, because --exact-only records the suppression whatever the fixture holds. That is why this coverage did not need indexed gRPC/Thrift fixture repos. Verified by mutation, not assertion: removing the skipped-rendering branch turns `names a stage it was told to skip as skipped` red and leaves the other 22 green. A control case pins the opposite direction -- the same group without the flag still reports the stage as zero -- so `skipped` cannot be printed unconditionally and pass. U3 and U4 land together: the test has no value without the renderer, so one commit keeps a revert clean. Both depend on U2, which introduced the field they read. tsc clean; 1066/1066 across the group unit and CLI integration suites. * fix(group): make every description of --exact-only match what it does (U5) Two descriptions this branch wrote or touched still misstated behavior. The MCP `exactOnly` description carries "Manifest links still apply." The CLI help and both locale strings, rewritten in the same commit, omit it -- so the surface most operators read understated what still runs. Manifest cross-links are computed before the gate and are genuinely unaffected by the flag, so the caveat is the accurate half and the CLI now says it too. The `group_sync` tool description opened with "extract HTTP contracts". That clause was carried forward byte-identical while only the trailing cross-linking half was rewritten, and it is wrong: the detect config has six non-HTTP extraction toggles, and this branch's own new test fixture is Thrift. `help-i18n.ts` is deliberately untouched. It maps an option to its translation key and that key already exists; only the commander string and the two locale values carry text, so a text-only change does not reach it. The tool-description edit sits ahead of the registryOutcome paragraph, leaving the relative order of the 'preserved' / 'superseded' / 'no-prior-registry' literals intact -- `tools.test.ts` slices that description by their positions. tsc clean; 64/64 across the locale-parity, help-registration, tool-schema and group-tool suites. * fix(group)!: remove max_candidates_per_step and shared_libs (U6) Both keys were declared, defaulted, written into every generated group.yaml, and read by nothing -- the same three-station dead surface this PR removed for bm25_threshold, embedding_threshold and detect.embedding_fallback. Every other DetectConfig field gates a real extractor in sync.ts; shared_libs gates nothing, because 'lib' contracts come only from the operator-declared manifest extractor. MatchingConfig reaches matching.ts solely through buildNoisyContractFilter, which reads exclude_links_paths and exclude_links_param_only_paths and nothing else. Existing group.yaml files keep loading and keep their keys. parseGroupConfig spreads the raw block over its defaults, so a key the schema no longer knows about survives into the returned config -- which matters because `group add` and `group remove` round-trip the operator's file through loadGroupConfig -> yaml.dump -> write, so anything the parser dropped would be deleted from their checked-in file. The legacy-config test now pins both keys in the same cast form as its three siblings, and the fixture carries shared_libs so that assertion is not vacuous. Two stations that are easy to miss and are swept here: - gitnexus/bench/cross-repo-trace/verify.mjs GENERATES a fresh group.yaml. It is not a preserve-path fixture, so "leave YAML fixtures alone" does not cover it; the repo has two generators and both are updated. It is a .mjs file outside tsconfig's include, so no type gate would have caught it. - config-parser.test.ts asserted the removed default at runtime, which vitest DOES run. That assertion is gone from the defaults case (the key no longer has a default) and re-formed as a preserve assertion in the legacy case. Verification gate, corrected: "zero net new errors against origin/main" would have measured the whole branch delta and been red through no fault of this commit. Measured instead against the branch tip immediately before it -- tsc -p tsconfig.test.json --noEmit reports 989 before and 989 after. Twenty-four typed-literal sites across ten test files, none of them CI-gated, plus the two runtime sites above which are. Note the deliberate side effect: removing a key from the defaults also stops the group add round-trip from re-adding it to a file that never carried it. Nothing in src reads either key, so no behavior changes. BREAKING CHANGE: `matching.max_candidates_per_step` and `detect.shared_libs` are no longer part of the group.yaml schema and are no longer written into generated templates. Existing files carrying them continue to parse and retain them. src tsc clean; 1189/1189 across the group unit, group integration, locale-parity, help-registration and tool-schema suites. * docs(group): map PR #3020 review findings to the commits that close them Retitles the ledger to hold one section per reviewed PR and adds #3020's ten findings. Two things are stated rather than claimed away: `abda0d041` closes three findings because they are one code block plus the test that pins it, and the suppressed-stage marker is a coupled set because the renderer consumes the field the earlier commit introduces. Also records what is NOT closed here -- the PR description's false claim about `max_candidates_per_step` lives outside this branch. * refactor(group): apply simplify-pass findings Four cleanup agents (reuse, simplification, efficiency, altitude) over this run's diff. Efficiency was clean. The rest found five things worth fixing, two of which were real gaps rather than style. `recordedMatchStages` filtered unknown values instead of rejecting the list. That inverted the tri-state on the one field built to prevent exactly this conflation: a stale `['bm25']` -- the scenario its own comment cites as the motivation -- survived as `[]`, which on this field MEANS "measured, nothing was suppressed". A confident clean answer manufactured from a value we could not read. Now all-or-nothing, matching `recordedRepoList`. `gitnexus group contracts` showed nothing after an exact-only sync. The human renderer destructures a fixed field list and gates its incompleteness warning on `truncated`, so the marker reached the MCP payload and the JSON output but not the listing an operator actually reads. It now warns, separately from the `truncated` warning, because the remedies differ: one says fix the repo, this one says re-run without the flag. `verbose` was still coerced with `Boolean()` in the same call whose tool description this branch changed to promise "PARAMETERS ARE VALIDATED". Validated now, and added to the tool schema -- it was read by the backend and advertised nowhere. Reuse: the thrift wildcard-matchable pair existed twice, near-verbatim, in `sync-exact-only` and `registry-suppressed-stages`. Both now call a shared `makeWildcardPair` fixture, so the shape `runWildcardMatch` fires on is defined once. Simplification: dropped a `Set` built per sync over a list that only ever holds zero or one entries; iterating `Object.keys(STAGE_COUNTS) as MatchType[]` also keeps the exhaustiveness the `Record` was built for, which `Object.entries` had discarded. Deliberately not done, with reasons: a schema-driven unknown-parameter layer at the MCP chokepoint (five parameters are read by backends and declared in no schema, so a strict layer rejects working calls today, and it cannot produce the "was removed" message finding 3 is about); folding the marker into `GROUP_IMPACT_TRUNCATION_REASONS` (reverses a recorded plan decision and the bridge scope is an open question for the maintainer); a per-stage suppression cause `Record` (no second suppressor exists -- speculative); collapsing the six `detect` extractor branches into a table (a real generalization, but a refactor outside this diff); and converging an untouched pre-existing CLI test onto the new manifest helper (it captures a value the helper does not return, so the change risks more than the duplication costs). tsc clean; eslint 0 errors (1 pre-existing warning); 1085/1085. * docs(group): remove REVIEW-FINDINGS-MAP.md Removes the findings-to-commits ledger from the source tree. Note for anyone reading this in history: the file was introduced on main by #3012 and carried that PR's findings map; this branch had appended a #3020 section. Deleting it drops both. #3012's content is recoverable with `git show 2c0fb7753:gitnexus/src/core/group/REVIEW-FINDINGS-MAP.md`. * fix(group): stop cross-repo impact and trace claiming a narrowed graph is complete Closes the half of the suppressed-stage finding that was deferred. The reviewers were right that deferring it was the weak point: the motivating harm was named as `group_impact` and cross-repo `trace` traversing a graph missing real edges, and those were exactly the surfaces left uncovered. The deferral rested on an assumption that does not hold. "It is already blind to this, so we do not make it worse" is false: `--exact-only` was inert before this PR, so the number of narrowed registries in the world goes from zero to nonzero exactly when this lands. The blindness was harmless only while narrowing was impossible. And silence there is not neutral -- `cross-impact.ts` documents `truncated: false` as an affirmative completeness claim, so those tools were about to start asserting a complete answer over a knowingly short graph. `suppressedMatchStages` now rides the bridge the same way `unreadableRepos` does: persisted in meta.json (no BRIDGE_SCHEMA_VERSION bump -- meta fields have this precedent), read back all-or-nothing, and carried across the preserve path through `refreshPreservedBridgeMeta`'s diagnostics so a preserved bridge keeps the marker of the sync that actually built it. `crossRepoCompleteness` folds it in, which is what makes this one change reach all three surfaces -- that function is by design the ONE computation behind the truncation triple. Precedence is explicit: an unreadable or unaccounted repo outranks a suppressed stage, because it is the more serious structural gap and its remedy has to be the one reported. `'suppressed-stage'` is a new member of the truncation-reason union rather than a reuse of `'incomplete-sync'`. The earlier decision not to touch that union was about not conflating remedies -- telling an agent to repair a repo that read fine, for a narrowing it requested. A distinct member preserves that reasoning while letting the answer stop claiming completeness, which is what reusing the existing member would have destroyed. The union's guard test did its job: adding a member failed the check that every reason is explained on the agent-facing surface, so the impact tool description now names this one and its distinct remedy (re-run WITHOUT the flag; nothing failed to read). `group status` and its CLI renderer surface it too, on the populated case only -- absent is a registry predating the field and empty is the ordinary clean sync; neither earns a line. Deliberately still not done, and why: a repo-wide unknown-parameter layer for every MCP tool. Five parameters are read by backends and declared in no schema (`subgroupExact`, `unmatchedOnly`, `showClusters`, `showProcesses`, and `verbose` until this branch declared it), and three tools dispatch with no schema entry at all, so a strict layer rejects working calls until each is reconciled. That reconciliation is the work; the layer is the cheap part. It also cannot produce the "was removed and is no longer accepted" message the retired-parameter guard exists to give. tsc clean; eslint 0 errors (2 pre-existing warnings); 1159/1159 across the group unit, group integration and tool-schema suites. * fix(group): make the suppressed-stage signal actually reach its readers Applies the mechanical findings from the code review of the previous commit. That commit claimed cross-repo impact and trace stop reporting a narrowed graph as complete. Trace did; impact did not, and two operator-facing messages said something false. Four reviewers plus the cross-model pass converged on the same two defects, and the untested seams were exactly where they were. `runGroupImpact` recomputed the truncation reason and hardcoded its fallback, so it could never emit 'suppressed-stage' -- the value the previous commit added to the union and documented in the tool description. Every narrowed-but-readable bridge was reported as 'incomplete-sync', telling the caller to repair a repo that read fine. It now propagates the bridge's own reason, as cross-trace.ts already did. The preserve path stamped this run's request onto an older bridge. When no repo can be read the database and registry are kept from an earlier sync, so meta.json has to keep describing that sync; instead `{ ...existing, ...diagnostics }` overwrote its marker, leaving contracts.json, meta.json and bridge.lbug describing three different runs. Currently masked by unreadable-repo precedence, one loosened condition from a live wrong verdict. `group contracts` printed "the last sync did not record which repos it could read" after any exact-only sync: truncated was set with both repo lists empty, so the message fell through to the wrong branch. It is now gated on the reason, not the flag. `group impact` likewise blamed the local walk for a floor the flag caused. The tri-state reader is now defined once, in the leaf module whose own comment says it exists so this exact duplication cannot recur -- it had been copied into bridge-db.ts within one commit of that comment being true. Both agent-facing descriptions now name the field. The previous commit added it to three payloads and documented it on none. Tests cover what shipped green: the preserve path for both artifacts (verified by mutation -- reintroducing the stamp turns exactly one test red), and the reason's REACHABILITY. The existing guard only asserted each reason is described, which is why a documented-but-unemittable value passed it. Also corrects a comment that said the marker is deliberately not folded into the truncation triple. True when written; false one commit later. tsc clean; eslint 0 errors; 1163/1163 across the group unit, group integration and tool-schema suites. * fix(group): drop verbose from the MCP surface, fail a superseded bridge closed Two maintainer-directed findings from the review. verbose is removed from the group_sync MCP schema and from GroupService, and kept on the CLI. The parameter never did what either description claimed: the gates emit workspace-dependency discovery stats and one aggregate manifest line, not "each cross-link". Worse, they emit them through the server's logger, which an MCP caller cannot read at all -- so advertising it introduced precisely the kind of knob this PR exists to delete, in the PR that deletes them. SyncOptions keeps the field and the CLI keeps --verbose, because a CLI user really can see that output; its help now says "Show additional sync diagnostics", which is what it shows. It was added to the MCP schema earlier in this same PR, so there is no published compatibility burden in taking it back out. A caller that still sends it is ignored rather than refused: it was never a documented parameter, and the retired-name guard is reserved for ones this tool actually withdrew. The second fixes a split-brain the completeness work made materially worse. When contracts.json commits and the bridge write then fails, the previous database stays in place describing an EARLIER sync. Until now it kept vouching for itself, so group_impact could traverse the superseded graph and call its answer complete while group_contracts reported the advanced registry -- two public surfaces making contradictory epistemic claims out of one sync. That was tolerable when the disagreement was about counts. It is not, now that suppressed-stage makes completeness a correctness property. markBridgeProvenanceUnknown withdraws the claim without touching the database: bridgeMetaMatchesFile already gives provenanceUnknown highest precedence and refuses to vouch for the pair, so cross-repo answers downgrade to a floor until a sync succeeds. Deliberately not a re-stamp -- the metadata still describes the database it was written for, and saying otherwise recreates the mis-pairing the preserve path avoids. Deliberately not a delete -- the old graph is still worth having as a floor, it just stops being called complete. Best-effort, because it runs inside a failure handler and must not replace a reported bridge failure with an unrelated one; the warning now states which of the two happened. Shared registry+bridge generation identity is the architectural fix and is deliberately NOT attempted here. This is the PR-sized containment. Verified by mutation, both directions: neutering the withdrawal turns the new test red, and a control pins that a healthy sync does not withdraw provenance -- otherwise every successful run would report its own answers as a floor. tsc clean; eslint 0 errors; 1185/1185. * refactor(group): apply simplify-pass findings Four cleanup agents over the last five commits. Efficiency was clean and traced why: the containment helper is failure-path only, the reason ternary sits after the fan-out loop, and the tri-state readers run once per artifact read. The strongest finding was one the diff itself proved. `refreshPreservedBridgeMeta` enforced the never-persisted rule for `repoListsUnreadable` and `pairedWithDatabase` with two deletes in its own body, under a comment noting it was the only code that read metadata and wrote it back. That held exactly as long as there was one such caller. `markBridgeProvenanceUnknown` made it two, and inherited nothing. The strip now lives in `writeBridgeMeta`, so every writer gets it and no future one can forget; `pairedWithDatabase` is the dangerous one, because persisted it tells every later reader the pair was verified when nothing verified it. `group impact` still printed "fan-out stopped early" whenever `truncatedRepos` was non-empty — but the bridge's incomplete repos are unioned into that list even when zero crossings were attempted, so a structural gap was reported as a runtime one, with the only working remedy omitted. That is the same false-cause shape the contract listing was re-gated for one commit ago, left live one command over because the new reason was bolted in front of the old branch rather than replacing the thing it branched on. Now keyed on the reason. The `?? 'incomplete-sync'` arm in cross-impact was unreachable: reaching it needs `truncated` true with all three of its inputs false, which `truncated = runtimeTruncated || bridge.truncated` forbids. Flattened. Also: a `recordedMatchStages` insert had split `crossRepoCompleteness` from its own JSDoc; one new test was a strict subset of another; and the bridge-failure warning interleaved concatenation with a mid-chain ternary. The new invariant assertion was caught being VACUOUS by mutation before it shipped — seeded with a valid repo list, `readBridgeMeta` never sets the reader-only field, so it passed with or without the strip. The fixture now seeds an unreadable list, and both it and the pre-existing assertion go red when the strip is removed. Deliberately skipped, with reasons: a shared `firstTruncated` fold over `TruncationFields` (the right altitude, but it changes cross-trace's return assembly and that surface separately documents a 'timeout' rung it cannot emit — a behavior change, not a cleanup); a reason-keyed `explainFloor` helper across all four CLI renderers (real, but a four-site refactor); narrowing the persisted stage vocabulary to a `SuppressibleStage` alias (would be undone by the very extension the field was modelled as a list to allow); moving `verbose` to `logger.debug` and deleting `SyncOptions.verbose` (the maintainer explicitly directed keeping both); and merging the two tri-state readers behind a predicate (they are adjacent in one file now, so a tightening applies to both by inspection — the duplication the comment warned about was cross-FILE). tsc clean; eslint 0 errors; 1164/1164. * fix(group): address gitnexus-check findings Seven bot comments across two review rounds; five distinct after dedup. Four were valid and are fixed, two were already resolved by later commits the bot had not seen. The validator could throw from its own error path. `JSON.stringify` is the right renderer there — it is what distinguishes the string "false" from the boolean, which is the entire point of the message — but it throws on a BigInt and on a cyclic object. So a validator promising a structured `{ error }` instead rejected, and `callTool` is reachable directly, so neither input is hypothetical. Guarded, keeping the distinction and falling back for the shapes that cannot serialize. An unreadable suppression record read as "nothing was suppressed". `recordedMatchStages` is all-or-nothing by design, so garbage collapses to `undefined` — and the consumer treated `undefined` as an empty measurement, throwing that safety away and reporting a registry it could not parse as complete. Present-but-unreadable now forces the floor, while absent stays legitimate: a registry written before the field existed has no opinion and should not be dragged to a floor for it. Two test-side findings, both real and both invisible to CI because `tsconfig.json` is src-only. Three `mock.calls[0][1]` accesses did not type-check against a zero-arg mock, and four assertions read `truncationReason` / `riskEpistemic` straight off `CrossRepoCompleteness`, which is a discriminated union carrying them on one arm. Also removed a `StoredContract` import that went dead when those fixtures moved to `makeWildcardPair`. Worth recording: U6 set a test-config gate at 989 errors and later commits walked it to 994 without anyone re-measuring — the bot caught three of the five. Now 987, below the original baseline. Already fixed, not by this commit: the preserve-path stamp the bot flagged against |
||
|
DuduPhudu
|
48106d3c00
|
fix(ingestion): index NestJS decorator routes so api_impact and route_map stop reporting live endpoints as non-existent (#3017) | ||
|
azizur100389
|
88df18b829
|
fix(ingestion): discover nested source directories (#3043) | ||
|
DuduPhudu
|
2c0fb7753c
|
fix(group): stop reporting what could not be measured as a measurement of zero (#3012)
* fix: surface unreadable group indexes and escape raw NUL bytes in source Two independent diagnostics failures, both of which turn a real error into a confident, benign-looking answer. **Unreadable member repos (#3011).** `syncGroup` wrapped `initLbug` plus all contract extraction for each member in a bare `catch {}` that pushed the repo onto `missingRepos` and discarded the error. A LadybugDB storage-version mismatch therefore surfaced as "repo not found", `group sync` printed `0 contracts, 0 cross-links` and exited 0, and the existing contracts.json was overwritten with an empty registry. The two states need different answers from the operator — a missing repo must be indexed, an unreadable one is usually version skew or a lock — so they are now separate: - the caught error is logged with the repo, group path and lbug path - `unreadableRepos` is tracked alongside `missingRepos` on `SyncResult`, persisted (optionally, so older registries still parse) on `ContractRegistry`, and threaded through `GroupService` sync/status - `group sync` reports both before the cascade counts, since an unread repo is the likely explanation for a small or empty count - `group status` reports unreadable repos separately; calling them "missing" actively misdescribed them - when EVERY configured repo fails to open, the write is skipped: an extraction that read nothing is not evidence the group has no contracts, and replacing a good registry with an empty one loses data while reporting success **Raw NUL bytes (#3010).** `sync.ts` and `free-call-fallback.ts` each used a NUL as a join delimiter, written as a literal 0x00 instead of `\0`. Identical at runtime, but it makes the file test as binary: `file(1)` reports `data`, ugrep returns empty with exit 1 — indistinguishable from "no match", with no message — and BSD grep replaces matching lines with "Binary file ... matches". A search that should hit comes back as a confident "not present". Both now use the escape, and a unit test fails on any raw control byte in src/ so it cannot silently return. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(hygiene): guard every tracked source file against a raw NUL, not just src/ The guard added with the NUL escapes only scanned gitnexus/src for .ts/.tsx. Neither prior recurrence of this defect in this repo was in that scope: |
||
|
DuduPhudu
|
031e123731
|
fix(group): resolve HTTP consumers through configured clients and constant route tables (#3008)
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
* fix(group): resolve HTTP consumers through configured clients and constant route tables
Cross-repo linking found almost no frontend consumers because the Node/TS
consumer pattern required two things application code never has: a receiver
literally spelled `axios`, and an HTTP path that is a string literal at the
call site. Real apps call a configured instance and pass the path by reference
from a shared route table, so both halves of every call live in other files.
Widen the pattern to any identifier receiver with an HTTP-verb method, then
admit the match only after PROVING the receiver is an axios instance —
following local aliases, default/named imports and `export *` barrels back to
an `axios.create(...)`, including when that call is an argument to a factory
that decorates and returns the instance. The proof gate is load-bearing:
EXPRESS_SPEC matches `router.get('/x', handler)` as a provider, so admitting a
receiver on spelling alone would re-emit every Express route as a consumer of
itself.
Resolve the path argument through the existing language-agnostic constant fold
(`constant-resolver.ts`, #2391) via a new JS/TS binding, mirroring how
`python-const-resolver.ts` binds the same core. The binding adds the two
JS-shaped facts Python has no analogue for: object-literal route tables
flattened to dotted literal keys (`API_ROUTE_PATH.LINKS`), and export aliasing
(`export default`, `export { a as b }`, `export *`). Templates and `+` concats
fold partially, so a mixed path keeps its known prefix instead of collapsing to
`{param}/{param}/...`.
Cross-file facts come from a `prepareRepo` pre-pass, the hook FastAPI prefix
resolution already uses. The three JS/TS plugins share one pass via a WeakMap
keyed on the orchestrator's memoized file list.
Every resolution floors to `null` (skip) rather than a guess: an ambiguous
import specifier, an unprovable receiver, or a fold that overruns its depth
leaves the call site exactly as unmatched as before. An unresolved path is a
missing contract; a wrong one is a false cross-repo link.
Measured on a real Next.js frontend (874 source files): consumer contracts
7 -> 160, none lost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(group): tighten the JS/TS HTTP consumer proof gates and bound the fold
Addresses the review findings on #3008. Widening the axios consumer query
moved precision out of the tree-sitter pattern and into runtime gates; most
of these are one of those gates leaking.
Keying
- scanBundle normalizes fileRel ONCE and uses that key for both the receiver
gate and the path fold. isHttpClientRef read the raw value while the fact
map is written under normalizeRel(rel), so any non POSIX path returned zero
consumers and a key miss is indistinguishable from "not a client".
Proof
- containsAxiosCreate (subtree containment) becomes bindsAxiosClient: the
instance must be the bound VALUE, or reachable inside the arguments of a
wrapping call whose result is bound. An object literal, ternary, array or
new X(...) binding no longer makes a cache or registry an HTTP consumer.
- A folded first argument must look like a path: no whitespace, not wholly
numeric, and not starting with an unresolved term. The check runs on the
${...} to {param} normalized shape, so a placeholder whose source contains
spaces does not drop an otherwise anchored path.
- A template or concat whose LEADING term never resolved returns null, which
is what the docstring always claimed.
- The literal receiver axios with a literal or template argument keeps its
pre-PR output verbatim, so the widening only adds detections.
Resolution
- resolveJsImport checks ambiguity across ALL candidate extensions, not within
one, so a .ts/.tsx or .ts/index.ts collision skips instead of picking a
winner. Two spellings of one module still resolve by precedence.
- A single segment bare specifier with no alias sigil never binds to a repo
file, so a Node builtin or npm package cannot be "proven" an axios client.
- resolveExportedMember walks every export * edge and returns null when two
barrels answer differently.
- Imports are collected in a hoisting pre-pass, so a client bound above its
own import statement is still proven.
Termination and cost
- MAX_EXPR_DEPTH and MAX_CONCAT_TERMS bound the path fold, flattenConcat walks
the left spine iteratively, and buildImportMap is explicit stack. A file
nesting template substitutions 4000 deep threw RangeError out of scan, which
sync.ts records as an unexplained missing repo with every contract dropped.
- MAX_FOLD_LENGTH applies to accumulated output, not per term, and to the raw
literal fallback. The per term cap was a 2048x amplifier and the result is
persisted into contractId.
- resolveJsImport is backed by a basename index and memoized per repo, and
resolveConstant accepts the key set instead of rebuilding it per fold.
2000 file repo with one bare npm import: 11074 ms to 1250 ms.
- prepareRepo measures its ceiling in bytes, parses inside the try, and skips
the parse pass entirely when the string axios appears in no candidate file.
It carries only file identities between its two passes, never their text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV
* fix(group): let a path-shaped all-numeric consumer path through the gate
The shape gate rejected any wholly numeric path, which also dropped
`client.get('/123')`. The leading slash is the evidence that separates a
route from a constant that merely folded to digits: a bare "5000" out of
`CONFIG.TIMEOUT` still matches every one-segment provider route and is still
refused, while a path written as a path is kept and normalized to {param}
the same way it always was.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV
* style: apply prettier to the changed files
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV
* fix(group): decide the axios receiver on evidence, not only on its spelling
The bare name `axios` was trusted with no proof, which is right for the
convention and wrong for a file that binds that name itself:
`const axios = fakeFactory; const api = axios.create(); api.get('/x')` was
admitted as an HTTP consumer, and so was a test file whose `axios` is a mock
object with a `create` method.
extractJsModuleFacts now records whether the file declares its own top-level
`axios` binding, and the spelling is trusted only when it does not. The other
half of the same fact is that CommonJS was invisible: `const ax =
require('axios')` resolved to nothing at all, and the un-aliased form worked
only because `axios` happened to be the name the spelling shortcut trusted.
Requires are collected alongside imports now, so a receiver is admitted when
it IS the axios module (the bare spelling, or a declared import or require of
'axios' under any name) or when it traces to an `axios.create(...)` instance.
Verified across the receiver matrix: shadowed local, shadowed mock object,
CJS require aliased and not, ESM import aliased and not, express router and a
plain Map all land where they should.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV
---------
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>
|
||
|
ChunxueLi
|
3f5fbb05e0
|
feat(group+ingestion): resolve Java constant-based route paths (@PostMapping(ApiPathConstants.X)) (#2980)
* feat(group): resolve Java constant-based route paths via repo constant map - prepareRepo builds repo-wide Java constant map (constant-definition files only, cheap regex gate; per-file try/catch so one bad file degrades not forfeits) - bind parser language in prepareRepo (orchestrator hands over a bare Parser) - scan() lazily overlays the importing file's own import table (extracted from the tree already in hand, zero extra parses) before folding operands - foldJavaOperands resolves qualified refs (Class.CONST) + static imports + string concatenation against the merged view; unresolved refs are skipped, never guessed Real-repo validation (winning-winex-opt, 23k Java files): providers 2 -> 1701 (1700 source_scan_resolved), cross-links 0 -> 589 exact Unit: 14/14 (java-route-const-resolver.test.ts) * fix(review): address bot review findings on PR #2980 - P2-1 (real): spring.ts route loop dropped every @value_expr match — the '!valueNode' guard ran before the operand branch, so ingestion emitted zero constant-referencing routes. Guard now accepts @value_expr when @value is absent; two downstream valueNode dereferences made conditional. Added 2 extractor-level regression tests (16 total). - P2-2 (real): collectSpringTypes copied rawPath:'' for constant routes into the shared Spring inheritance view — now skipped there (fold happens in scan(); empty-path noise would leak into inheritance-based providers). - P1-1 (false positive): Java 'static final' allows exactly one initializer (duplicate declarations are compile errors), so the Python-style rebinding shadowing cleanup does not apply — documented at the site. - P1-2 (false positive): constant-resolver.ts and prepareDurableParsedFileChunk both exist on upstream main (#2391 / parsedfile-store.ts:562); the bot's 'repository lookup' appears to have compared against a stale index. - P3: removed dead FQN_CONTROLLER fixture. Real-repo regression: 589 cross-links / 2423 contracts (was 2424 — the dropped contract is the empty-path inheritance artifact fixed above). * docs(cache): note Java constant-route capture set in the SCHEMA_BUMP ledger The Java constant-route harvest (route-extractors/java-const-resolver.ts + the spring.ts operand branch + the parse-worker Java constant harvest) changes the worker capture set: a warm pre-feature cache replays moduleConstants=0 captures verbatim and silently drops every constant-based Spring route on unchanged files. After rebasing onto current main the ledger already sits at 70, whose capture set post-dates and includes this harvest, so v70 invalidates those caches — no additional bump is needed. * fix(feign): guard @RequestLine against the constant-valued shape A constant-valued `@RequestLine(SOME_CONST)` is captured as @value_expr, not @value, so `valueNode` is undefined in that shape and the literal dereference crashed the scan. Skip instead — folding verb+path literals through the constant map is out of scope for this PR. Found in maintainer review of #2980. * fix(resolver): bound qualified-ref recursion depth for self/mutual import cycles Maintainer review point: the qualified branch of resolveJavaConstant recurses through resolveJavaImport without a guard — a self-import (X = SelfConsts.X + ...) or a pair of mutually-importing constants would recurse without bound before reaching the shared fold's visited-stack, which only guards the bare-name path. Bound the Java-qualified walk with a depth cap (32) and thread it through every recursive call. Two regression tests use real repo shapes (repoOf fixtures): self-import and mutual-import cycles both terminate with null (skip floor), as before, but promptly. Also drops the stray machine-local .gitignore entry that rode along from the fork's dev branch. * fix(routes): address round-2 review — provider hooks, FQN fold, interface nesting F1 (High): production harvest silently dropped routes when the constants class is not named *Constants (e.g. ApiPaths). The content gate is now SYNTAX-driven (static-final String field or any class import) and lives in the provider (moduleConstantHeuristic), not a shared-layer regex. F2: shared ingestion layers no longer branch on language. The harvest and the qualified-ref fold run through new provider hooks (extractModuleConstants / foldRoutePathOperands); parse-impl resolves the provider by filePath (getProviderForFile). Python wires the same hooks for architecture parity. F3: multi-segment FQN chains (com.example.ApiPaths.USERS) now flatten recursively; verified via tree-sitter that the existing query already captures the whole nested field_access — the gap was resolver-side only. F4: implicit-final interface semantics no longer leak into nested classes at type boundaries (JLS 9.5). F5: nested same-name shadowing now drops the stale entry (rebind-drop, matching Python #2391 semantics) instead of keeping the first binding. Tests: 9 new unit tests (27/27) + real-pipeline e2e over a reviewer-shaped fixture (non-*Constants class, cold run + warm parse-cache replay) — the exact production gap unit tests missed. * style: prettier --write on the two touched test files (CI format gate) * fix(routes): address the open review findings on Java constant route folding Answers every reproduced finding still open on #2980, plus the defects an adversarial pass found in the first round of those fixes. The wrong-path group each turned a *missing* fact into a *wrong* one, which is what this module's skip-or-correct contract exists to prevent. Wrong-path fixes * Escapes were deleted from constant values. tree-sitter-java splits a `string_literal` around its `escape_sequence` children, so joining `string_fragment`s alone folded `"/user/{id:\\d+}"` — the standard Spring path-variable constraint — to `/user/{id:d+}`, and a pure-escape literal to the empty string. Worse, the LITERAL path keeps escapes verbatim, so one Java route had two irreconcilable spellings. `stringLiteralValue` now reuses `unquoteSpringLiteral`, the helper that literal path already uses. Java text blocks are excluded: that helper's `"""` arm would hand back the raw block, newline and incidental indentation included, so they keep the old skip. * A constant-valued class prefix produced a truncated route. The new `@value_expr` query branches were `method_declaration`-only, so `@RequestMapping(ApiPaths.BASE)` left the prefix empty and the method route was emitted unprefixed — a path the application does not serve, where the base emitted nothing at all. Both subsystems now detect such a class and suppress its method routes, the rule `classesWithArrayPrefix` already encodes for the array form. The suppression covers ingestion's separate no-argument-mapping loop too, without which a bare `@GetMapping` under a constant prefix still shipped an empty-path Route while the group emitted nothing. * A shadowed static import survived a non-foldable rebind. The rebind-drop deleted `literals`/`exprs` but not `imports`, so a name both static-imported and locally redeclared resolved through the stale import to the imported value instead of skipping (#2393's Python defect, reproduced for Java). * `resolveJavaImport` guessed where its own docstring promised null. The nearest-shared-directory tie-break is gone: javac resolves duplicate FQNs by classpath order, so proximity can return a src/test fixture copy. Parity and coverage fixes * One constant-file gate, exported as `isJavaConstantFile` and used by both the ingestion provider and the group `prepareRepo` pre-pass. The two spellings disagreed on a constant INTERFACE — implicitly `public static final`, so it carries neither keyword — which the group admitted and ingestion rejected, so the group published a contract while the graph got no Route node. It is also modifier-order agnostic now, and its interface arm requires a String assignment so a javadoc mentioning "interface" no longer costs a parse. * Import ambiguity is measured over constant-DEFINING files on both sides. Ingestion's harvest gate also admits import-only files, so handing `resolveJavaImport` every repo key let a duplicate FQN that defines nothing make ingestion alone floor to skip — reopening the same parity break in the same losing direction. * Python's constant harvest is unconditional again. The gate added here required NAME immediately followed by `=`, so it dropped `API: str = "/api"`, `API: Final[str] = "/api"` and every composed constant whose RHS starts with an identifier — routes that already resolve on main. The worker now treats a missing heuristic as "harvest" rather than "skip". * Enum and record declarations were traversed but never collected, so a `static final String` declared in one was absent from the map. The walk still descends the whole body, so a type nested in an enum-constant body is kept. * Constants composed across files through a qualified ref never resolved: operands found inside an initializer went to the agnostic core, which only knows bare names, so `X = BConsts.Y + "/tail"` floored to null even acyclically. The Java binding now folds its own expressions — and carries the core's guards with them: a `visited` stack popped on unwind, a memo of successes, and `MAX_FOLD_LENGTH`. Without the memo a shared-descendant DAG re-folds each child per reference; because a chain of empty strings never accumulates output, the length cap could not stop it, and one route over a 31-line constants file took 11 s at 28 levels on the main thread. * Dropped the dead `com.java.lang.` type normalization. Cache * `SCHEMA_BUMP` 70 -> 72. Leaving it at 70 was justified by "the ledger already sits at 70, whose capture set post-dates and includes this harvest" — it does not: 70 was cut by |
||
|
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
|
||
|
Gergő Magyar
|
414c1a5693
|
fix(storage): give every registry write its own tmp path (#2888) (#2920)
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
* fix(storage): give every registry write its own tmp path (#2888) `writeRegistry` staged the global registry through a FIXED `~/.gitnexus/registry.json.tmp`. The rename is atomic with respect to readers, but the tmp path is not private to the writer, and that file is the one file every gitnexus process on the machine writes. Two of them starting together stage through the same inode: the second `writeFile` overwrites the first's bytes, the second `rename` moves that inode onto `registry.json`, and the first's own rename then finds nothing at the source and rejects with ENOENT: no such file or directory, rename '<home>/registry.json.tmp' -> '<home>/registry.json' which kills the MCP server, because it lands on the startup path (`mcpCommand` -> `LocalBackend.init` -> `refreshRepos` -> `listRegisteredRepos({validate:true})`) where nothing catches — the client just reports "Server disconnected". #2716's `withRegistryLock` serializes the callers and hides this in the normal path, but it deliberately degrades to UNLOCKED after a 5s `IndexLockTimeoutError` (availability over serialization), so the window is still live. Measured on this branch's parent with 12 concurrent processes pruning a stale registry while another process held the registry lock: 4/12 crashed with the trace above. Same harness with 24 processes and no lock contention: 0/24. So the write itself has to be collision-proof rather than relying on the lock. `writeMetaFile` (repo-manager), `writeBridgeMeta` (group/bridge-db) and `writeContractRegistry` (group/storage) already carried the correct shape — random tmp suffix, `'wx'` + `0o600`, `retryRename` — as three byte-identical copies, none of which cleaned up its tmp file on failure. Rather than adding a fourth copy, that sequence moves to `writeFileAtomic` in storage/fs-atomic.ts (beside `retryRename`, which it uses) and all four writers call it. The helper also unlinks the tmp before rethrowing: with a fixed name a leaked tmp was self-limiting because the next writer overwrote it, but a random suffix would drop a fresh orphan beside the target on every failed publish. Second half of the same crash: the prune write inside `listRegisteredRepos({validate:true})` is housekeeping, not the caller's request. Every caller consumes the returned `valid` array and the prune set is recomputed from scratch on the next validating read, so a failed write costs a retry, never correctness — while rethrowing it took down the whole MCP server. It is now caught and warned about, which also covers the read-only-home and full-disk variants of the same startup death. Note: `registry.json` is now created `0o600` (it inherited the umask before, typically `0o644`), matching what `gitnexus.json` has always used. A rewrite tightens the mode on existing installs. Verified: the five new tests in test/unit/repo-manager-registry-atomic-write.test.ts all fail on the parent commit — four with the exact ENOENT above — and pass here; the process-level repro goes 4/12 -> 0/12 crashes with the lock held. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VXSu2fTmm7EZGDVquWeBrL * refactor(storage): trim the atomic-write helper and its guards Follow-up polish on the #2888 fix, no behaviour change except where noted. - `writeFileAtomic` drops the `mode` parameter (no caller ever varied it) and inlines `0o600`, and gains an `attempts` pass-through to `retryRename`. The prune write in `listRegisteredRepos` now passes `attempts: 1`: it discards a failure anyway, so the 300ms of rename backoff bought nothing and was spent holding the registry lock, on a path with a sub-500ms cold-start budget (`gitnexus augment`) and on MCP startup. - `saveMeta` serialises `meta` once instead of once per written file. `meta` carries a `fileHashes` entry per file — 263KB and ~420us on this repo, linear in file count — and it was being stringified twice per save, several times per analyze. `writeMetaFile` was a one-line forwarder after the previous commit, so it folds into `saveMeta`. - Comments: the four writers were each restating the primitive's contract, and the #2888 narrative appeared in four files. Kept one authoritative copy in the helper, one registry-specific note at `writeRegistry` (why the lock is not enough), and deleted the rest. - Tests: new test/unit/storage/fs-atomic.test.ts covers the primitive behaviourally — published bytes, `0o600` on the result, three concurrent publishers to one target all resolving, no leftover tmp and intact previous content when the publish fails. That is what the source-text regexes in insecure-tempfile.test.ts were approximating, so those shrink to the one thing regex is good for: this module does not hand-roll a tmp path. The registry test drops the assertions the primitive now owns, an unused `fs.writeFile` capture, a type alias with two `as unknown as` casts the sibling harnesses do without, and moves its two path-only temp repos to `beforeAll`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VXSu2fTmm7EZGDVquWeBrL --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
azizur100389
|
1fa751d76d
|
fix(spring): extract method-level RequestMapping routes (#2857)
* fix(spring): extract RequestMapping route methods Co-authored-by: Cursor <cursoragent@cursor.com> * fix(spring): address RequestMapping review findings * fix(spring): accept trivia in request methods --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
990d79ba8c
|
fix(mcp): make impact/context reproducible — deterministic ordering on every capped query (#2787) (#2796) | ||
|
azizur100389
|
d268f351d3
|
fix(group): preserve manifest-only impact crossings (#2784)
* fix(group): preserve manifest-only impact crossings Keep proven manifest cross-repo hits when the far endpoint has no concrete graph symbol, avoiding a guaranteed failed UID fan-out. * fix(group): verify manifest-only neighbor repos Keep manifest-only crossings from bypassing neighbor repository resolution so unavailable repos still surface as truncated fan-out. * fix(group): distinguish boundary-only impact crossings Keep manifest-only boundaries visible without treating unattempted fan-out as completed impact or escalating risk, and cover service scope, deduplication, and real bridge persistence. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
7a064a1f2a
|
fix(storage): stop the Windows \\?\ long-path prefix from breaking repo path matching (#2667) (#2700)
* fix(lib): add stripWindowsLongPathPrefix for path comparisons (#2667) A caller can hand GitNexus a `\\?\`-prefixed path — the usual MAX_PATH workaround on Windows — and `path.resolve` preserves the prefix, so it reaches every string comparison GitNexus keys paths on. It also poisons relativization: `path.win32.relative` cannot express a relative path between a prefixed and an un-prefixed form of the same directory, so it returns the absolute target instead. That absolute string is the shape reported in #2667. The helper is deliberately scoped to the comparison domain. libuv's `fs__capture_path` does not re-add the prefix for over-MAX_PATH paths, so stripping a filesystem-facing path would break long-path access on hosts that have not opted into LongPathsEnabled. `\\?\Volume{GUID}\…` is left alone because the remainder is not a usable path. The test is fixture-free and takes an explicit `platform`, mirroring `normalizeAnalyzerRootPath`, and is registered on the cross-platform matrix since the whole transform is a POSIX no-op. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2 * fix(storage): normalize the `\\?\` prefix in canonicalizePath (#2667) `canonicalizePath` is the single comparison key for the repo registry, MCP repo resolution and the server repo routes, and `registryPathEquals` compares its output as a plain string. A caller-supplied `\\?\` prefix therefore matched nothing: a repo registered as `D:\repo` was invisible to a caller passing `\\?\D:\repo`, which surfaces as "repo not found" or a duplicate registration from `analyze`, `remove`, `clean`, the MCP `repo` parameter and the server routes. Both branches are normalized. The realpath branch was already safe — libuv's `fs__realpath` strips the prefix itself — but the `catch` fallback returns `path.resolve(p)` untouched, and that is exactly the branch a path which is not on disk takes. Safe despite the CRITICAL blast radius (27 impacted, 12 direct dependents) because the result is only ever compared, never opened: all 23 call sites feed `registryPathEquals` or a string comparison. Both operands are canonicalized, so the equality relation is preserved and behaviour is unchanged for every un-prefixed input. The two regression assertions run only on windows-latest, where the file already runs via the cross-platform matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2 * docs(core): correct two false comments about Windows paths (#2667) Both comments assert the opposite of how the platform and the analyzer actually behave, and both would send the next investigator of #2667 the wrong way. `analyzer-identity.ts` claimed the `\\?\` prefix is one that `realpathSync.native` "can emit for paths over MAX_PATH". libuv's `fs__realpath_handle` strips the prefix unconditionally and rewrites `\\?\UNC\` back to `\\`, erroring if neither is present, so realpath never returns one. The prefix can only arrive from caller-supplied input. The optional group in the regex stays as a labelled defensive no-op, and the function's behaviour is unchanged on purpose: these identity fields are compared between an `analyze` and a later `status` run, so this is not the place to reshape a path. `include-extractor.ts` claimed "gitnexus analyze stores absolute paths in the File.filePath column". A full self-index at |
||
|
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
|
5f4964b4e6
|
fix: resolve imported/composed FastAPI route path constants (#2391) (#2393)
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
* feat(routes): add pure Python string-constant resolver (#2391 U1) * feat(routes): extract Python module constants from tree (#2391 U2) * feat(routes): capture non-literal FastAPI decorator args + per-file constants, bump parse-cache schema (#2391 U3) * feat(routes): resolve composed decorator route constants in parse-impl + skip floor (#2391 U4) * feat(routes): resolve composed FastAPI route constants in group HTTP-contract layer (#2391 U5) * test(routes): multi-hop, ingestion↔group parity, and warm-cache regression locks (#2391 U6) * docs(routes): mark the language-agnostic seam for cross-language const resolution (#2391) * refactor(routes): extract language-agnostic constant-fold core; Python becomes a binding (#2391) The fold, cycle guard, and depth cap now live in constant-resolver.ts and take a pluggable ImportResolver. python-const-resolver.ts supplies the Python import semantics + tree extractor and re-exports the same surface, so no call site changes. A Spring/Kotlin/C# binding can now reuse the core with its own resolver (proven by constant-resolver.test.ts driving it with a Java-style resolver). * fix(routes): treat the constant-fold cycle guard as a recursion stack (#2391) The `visited` set in `foldName` was added-to but never removed on unwind, so a constant referenced more than once in a single fold — `A + A`, a reused separator (`SLASH + PATH + SLASH`), or a diamond `X = P + Q` where P and Q share a base — tripped the cycle guard on its second occurrence and the whole route was silently dropped by the skip floor. Pop the guard in `finally` so it tracks the ACTIVE resolution stack, not every name ever seen: a true cycle (a name still on the stack) is still caught, but a name that already resolved and popped folds again. Re-computation stays bounded by MAX_RESOLVE_DEPTH, so no blowup is reintroduced. Locked in constant-resolver.test.ts (A+A, reused separator, shared-base diamond); the pre-existing real-cycle and depth-cap cases still return null. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): make module-constant binding writes mutually exclusive (#2391) `extractPythonModuleConstants` kept `literals`, `exprs`, and `imports` as three independent maps: `setName` cleared literals+exprs but never `imports`, and an import never cleared a prior literal/expr. Since `foldName` checks literals > exprs > imports regardless of source order, a name that was both imported and locally (re)assigned kept both bindings and the wrong one won — `from .c import ROUTE; ROUTE = os.getenv(...)` resolved the STALE import instead of dropping, a confidently wrong route path (the exact skip-floor invariant this feature is meant to uphold). Treat the three maps as one logical namespace: any write to one clears the other two for that name (via `imports.delete` in `setName` and a `bindImport` helper), so last-binding-in-source-order wins, matching Python. An import both imported and dynamically rebound now drops. Folding `+=`/`+` onto an imported base remains deferred (it drops safely, never a stale value). Locked in python-const-resolver.test.ts: dynamic-rebind drops, literal-shadows- import, import-shadows-literal, and `+=`-on-import drops. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): widen the group cost-gate to catch literal-leading concats (#2391) `NONLITERAL_ROUTE_DECORATOR_RE` required the first decorator argument to START with an identifier, so a string-literal-leading concat like `@router.get("/api" + SUFFIX)` never tripped `hasComposedRoute`. When such a route was the ONLY composed shape in a repo, the group layer left `constantsByFile` empty and dropped the route, while the ingestion side (which has no gate) resolved `/api/users` and emitted a Route node — an R4 provider/graph parity break. Widen the gate to also fire on a string-literal-leading `+`-concat, detected by a `+` before the closing paren on the decorator line. Gating on the `+` (not merely a leading quote) keeps a plain literal route `@router.get("/x")` OFF the gate, so a literal-only repo still pays no parse pass. Locked in fastapi-composed-provider.test.ts: a sole literal-leading concat now resolves (parseCalls>0 + provider emitted), plus previously-uncovered `@app.<verb>(CONST)` EXPR-branch resolution; the literal-only no-parse gate case still passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): correct package-init and over-deep relative import resolution (#2391) Two edges in `resolvePythonImport`: - `from . import X` (empty module after the dots) resolved to a sibling `<dir>.py` instead of the package `<dir>/__init__.py`. Resolve the bare-package case to `__init__.py`. - An over-deep relative import (more extra dots than the importing file has directory levels) silently clamped `dirOf('')` to `''` and could match an unrelated root-level `<name>.py` — a wrong file. Guard with `walk > depth → null` so an import that escapes above the repo root drops (skip floor). Both preserve the exact-match / ambiguity→null behavior for ordinary relative and absolute imports. Locked in python-const-resolver.test.ts: `from . import` → `__init__.py` (and null when absent), and an over-deep import returns null even when the clamped target file exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): bound parseConstOperands recursion depth (#2391) `parseConstOperands` recursed on `binary_operator` children with no depth bound. A stack overflow is not currently reachable (tree-sitter caps expression nesting below the JS stack limit, so it throws on a deep `+`-chain before this runs), but add a depth guard (cap 64, mirroring the fold engine's MAX_RESOLVE_DEPTH) as defense-in-depth: a pathological chain now floors to null (skip) rather than relying on tree-sitter's limit. The `depth` parameter defaults to 0, so all existing callers are unaffected. Locked in python-const-resolver.test.ts: a 100-term `+` chain yields no binding (null) instead of throwing; ordinary short chains still fold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(routes): read each .py once in buildPythonRepoContext (#2391) The group repo-context builder read every `.py` file from disk twice: once in the `include_router` cross-file pre-pass and again in the #2391 constant cost-gate loop — an unconditional 2x read on every Python repo, on every group extraction. Hoist a single read pass that populates one `pyContents` map (and computes the composed-route cost gate); both the include_router pre-pass and the constant-map pass now consume the cached content. Behavior-preserving — a literal-only repo still does one read and zero parses. Covered by the existing group unit + integration suites (R4 parity and include_router prefix joins unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(routes): tidy constant-resolver docs and declaration order (#2391) Three no-behavior nits from the PR #2393 review: - Name `conditional_expression` (`x if c else y`) in the `parseConstOperands` jsdoc list of shapes that deferred to null. - Move `NONLITERAL_ROUTE_DECORATOR_RE` above `buildPythonRepoContext`, which references it — it read as a forward reference before (runtime-safe, but confusing). - Correct the integration-test comment that called `/v2/api/v1/widgets/get` "ingestion-only garnish": the group side emits it too (asserted separately); the four paths in that block are the shared-parity set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(routes): fold `X += "…"` onto an imported base constant (#2391) Previously `from .c import BASE; BASE += "/v1"` dropped (the extractor could not represent "the imported prior value" as an operand without self-referencing X and tripping the cycle guard). Preserve the imported prior under a synthetic `$imp$N` key — `$` can never appear in a Python identifier, so it cannot collide with a real name — and reference it, so the augmented assignment folds to `<imported BASE>/v1`. Extractor-only: no change to the `Operand` type, the fold core, or the cache shape, so no SCHEMA_BUMP. An imported base that is itself unresolvable still drops (skip floor preserved — never a wrong path). Locked in python-const-resolver.test.ts: single and chained `+=` fold onto an imported base; an unresolvable base still drops. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(routes): resolve bare decorator constants via the by-name entry (#2391) The group `resolveExprArg` hand-built `[{ kind: 'ref', name }]` and called `resolveOperands` for a bare-constant decorator argument — exactly what the language-agnostic core's `resolveConstant(file, name, repo)` seam does. Call it directly for the identifier case. This gives the previously test-only by-name entry point a real production caller (it is the documented reuse seam for future JVM/other bindings), drops the synthetic operand construction, and lets the now- unused `Operand` type import go. Behavior-identical — the `+`-concat path still parses to an operand list and folds via `resolveOperands`. Guarded by the existing group provider suite (bare-constant and concat cases). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(routes): parse each .py once in buildPythonRepoContext (#2391) The repo-context builder ran two parse loops — the include_router prefix pre-pass and the #2391 constant-map pass — so an include_router file in a composed repo was tree-sitter-parsed twice. Merge them into a single pass that parses each `.py` at most once and feeds both extractions from the same tree; a file that needs neither pass is still not parsed at all (cost gates unchanged). Complements the earlier single-read-pass change (this is the single-parse counterpart). Behavior-preserving (prefixes, R4 parity, and cost gates verified by the group + integration suites). Locked with a parseCalls assertion: a file needing both passes is parsed once, not twice. Note: a cross-run (cross-process) constant-map cache — the other deferred perf idea — remains out of scope; it needs disk persistence + invalidation and would add hashing/IO cost on the common path, so it fails the minimal-change bar this single-parse dedup meets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): bound constant-fold work and output to prevent OOM (#2391) The `finally`-popped cycle guard (recursion-stack semantics) correctly folds diamonds/repeated refs, but popping the guard removed the accidental work cap the old seen-ever set provided: a wide shared-descendant DAG re-folds each child once per reference, and a self-multiplying concat (`X = A + A; A = B + B; …`) builds a genuinely exponential string. Reviewers reproduced ~16.8M folds escalating to `RangeError: Invalid string length` and heap OOM — and neither fold call site is wrapped in try/catch, so it crashed the whole phase rather than dropping the route. Two complementary bounds, both flooring to null (skip), never a wrong value: - a never-popped `memo` in `foldName` caps recomputation at O(nodes) (successes only — a null may be transient on a cyclic branch); - a `MAX_FOLD_LENGTH` (8192) cap in `foldExpr` drops a fold whose output grows past any real route path, bounding the string size the depth cap does not. Corrects the prior "≤ 2^8 folds" comment (output grows multiplicatively, not additively). Locked with a 64^4-fanout construction that now drops in ~ms instead of OOMing; diamonds/cycles/depth-cap behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): snapshot assignment RHS refs at the assignment line (#2391) `ROUTE = BASE` was stored as a lazy `ref(BASE)`, resolved against BASE's FINAL binding. So `ROUTE = BASE; BASE += "/v1"` (or `ROUTE = API; API = "/other"`) resolved ROUTE to the MUTATED value — a confidently wrong path, since Python assigns by value at the `ROUTE =` line. This was latent for local constants at the base of this feature and the `+=`-on-import work extended it to imports. Snapshot each assignment/`+=` RHS reference to a bound name into that name's current frozen value at the assignment line (`freeze`/`snapshot`): a literal value, a copy of the current expr (whose refs are already frozen), or an import preserved under a `$imp$N` alias. Unbound refs (forward references) stay lazy. A later rebind of the aliased name can no longer change the earlier binding. `freeze` also unifies the previous `currentOps` + inline import-alias logic. Locked in python-const-resolver.test.ts: aliased-import-then-`+=`, aliased-local-then-`+=`, aliased-local-then-rebind all resolve to the pre-mutation value; normal reference chains still fold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): fold group identifier args via resolveOperands for parity (#2391) Resolving a bare-constant decorator arg through `resolveConstant` entered `foldName` at depth 0, whereas the ingestion side folds `routePathOperands` through `resolveOperands([{ref}])`, entering at depth 1. At the MAX_RESOLVE_DEPTH boundary the group tolerated one more hop than ingestion, so a deep alias/re-export chain resolved in the group provider set but dropped from the graph Route nodes — an R4 parity break. Restore the operand-list path in the group so both subsystems share identical fold-entry depth. (`resolveConstant` reverts to the documented agnostic-core seam.) Locked in constant-resolver.test.ts: a 4-hop chain that `resolveOperands([ref])` drops but `resolveConstant` resolves, documenting why the group must use the operand-list entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): match multiline literal-leading concats in the cost gate (#2391) `NONLITERAL_ROUTE_DECORATOR_RE` used `[^)\n]*` so it only saw a literal-leading `+`-concat when the `+` was on the same line as the opening quote. A Black-formatted `@router.get(\n "/api"\n + SUFFIX\n)` therefore failed the gate, and when it was the only composed route in a repo the group dropped it while ingestion (which parses the tree, not the raw line) resolved it — an R4 parity break. Drop the `\n` exclusion: `[^)]*` spans the wrapped argument but stays bounded by the decorator's own closing paren, so a plain literal route still never trips the gate. Locked in fastapi-composed-provider.test.ts with a multiline concat fixture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): bump SCHEMA_BUMP for changed extractor output + E2E snapshot lock (#2391) `extractPythonModuleConstants` now emits DIFFERENT `moduleConstants` for the same source (binding mutual-exclusivity clears stale imports; RHS refs are snapshotted; `$imp$N` aliases). That output is cached verbatim in the parse cache, so a warm shard built at the pre-fix version would replay stale — in one case actively wrong — folded values, and the correctness fixes would silently no-op on upgrade. Bump SCHEMA_BUMP 11→12 to force re-extraction (same warm-cache-replay class the original 10→11 bump addressed for the field addition). Also adds the first end-to-end coverage for the new behavior through the real ingestion pipeline: app/snapshot.py aliases a constant (`SNAP = API_V1`) then mutates the source (`API_V1 += "/mutated"`), and the test asserts the Route node is `/api/v1`, never `/api/v1/mutated` — a case the pure-function unit tests covered but the pipeline did not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
b98f6e458f
|
fix(lbug): recognize Windows missing-shadow error so serve repo-switch recovers (#2382) (#2387) | ||
|
Gergő Magyar
|
fbffa96554
|
fix(lbug/mcp): exact symbol content + 0-based line storage with 1-based MCP display (#2377, #2379) (#2380)
* fix(lbug): store exact symbol content snippets * fix(ingestion): emit 0-based line numbers for COBOL/JCL/scope/markdown nodes COBOL/JCL processors, the scope-graph emitter, and the markdown Section emitter stored 1-based startLine/endLine, unlike every tree-sitter node (0-based). The exact-content slice (#2379) then dropped each symbol's declaration line for those languages. Convert to 0-based at the graph-node emission boundary via toZeroBasedLine — leaving parser-internal .line values, L${line} node/edge IDs, and containment checks untouched. Refs #2377, #2379 * refactor(lbug): single source of truth for symbol-content labels Extract SYMBOL_NODE_LABELS so the exact-content label set can't drift the way the inline copy did in #2379. csv-generator derives EXACT_SYMBOL_CONTENT_LABELS from it; manifest-extractor's near-identical allowlist is left behavior-unchanged (intentional subset, #2325-test-locked) with a documented cross-reference. Refs #2379 * test(ingestion): cover 0-based emitter output and pin exact-content slicing - csv-pipeline: replace the blank-buffer fixture (a +/-1 shift silently passed) with directly-adjacent neighbors; add one-line-symbol and Section (+/-2 fallback) cases. - cobol resolver: assert COBOL Module and JCL job/step emit 0-based startLine. - markdown CRLF: update Section startLine/endLine expectations to 0-based. Refs #2377, #2379 * feat(mcp): present 1-based line numbers in context/query/impact tools GraphNode startLine/endLine are stored 0-based (tree-sitter rows), which surprised users querying them (they don't line up with editors/sed). Add toDisplayLine and apply it at the context/query/impact response boundaries so line numbers are editor/sed-aligned. Raw cypher stays 0-based (documented in the schema resource); BasicBlock/PDG statement lines (already 1-based) and internal join params are left untouched. Refs #2377 * test(mcp): assert 1-based tool exposure with raw cypher staying 0-based context() reports startLine+1 (editor/sed aligned); a raw cypher RETURN of the same node keeps the stored 0-based value. Guards against double-conversion and leaking the display shift into raw results. Refs #2377 * fix(mcp): stop query() double-converting BM25 line numbers bm25Search applied toDisplayLine to its result rows, and query()'s aggregation loop applied it again, so BM25-matched symbols reported lines shifted +2 (stored 0-based 41 read as 43, not 42) while semantic-matched symbols were correct. bm25Search is called only from query(); return raw 0-based rows and let the single aggregation-loop conversion handle both retrievers. Adds a query() BM25 regression test asserting stored 41 -> 42 (would be 43 if double-converted), which the prior mcp-line-display test — covering only context()+cypher — never exercised. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): use ?? not || so first-line symbols keep their line number `sym.startLine || sym[4]` treated a legitimate 0-based startLine of 0 as absent, so context()/query() dropped startLine/endLine for every symbol on line 1 of its file — every COBOL Module (toZeroBasedLine(1) = 0) and markdown h1. `??` only falls through to the positional fallback on null/undefined, preserving a real 0. This also repairs the rename definition-edit path, which consumes context()'s value. Adds a context() first-line (startLine:0 -> 1) assertion. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): make group/cross-repo trace line numbers 1-based consistently A group/cross-repo trace presented 1-based endpoints (via resolveSymbolForGroup) but 0-based hops (tagHops copies port.trace output verbatim), so one response mixed bases. Wrap the trace port adapter (traceForGroup) to convert hop lines to 1-based too, matching the endpoints. Single-repo trace dispatches directly (not through this port) and stays 0-based — full single-repo parity is a tracked follow-up. core/group stays display-agnostic (no mcp import). Extends the cross-trace e2e test to assert hops share the endpoints' base (checkout 10 -> 11, getUsers 1 -> 2). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): present explain/pdg_query anchor line 1-based resolveBlockAnchor converted its ambiguous-candidate lines to 1-based but left the resolved-target anchor raw 0-based, so the same tool reported two bases depending on whether the target was ambiguous. Convert the display anchor to 1-based via toDisplayLine. The BasicBlock join param (symStart: sym.startLine + 1) is untouched — it targets the 1-based BasicBlock id space, not display. Asserts the resolved anchor is 1-based (targetFn stored 10 -> 11). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): bump schema + PDG result versions for the line-number change The 0-based storage flip for COBOL/JCL/markdown/scope (#2377/#2379) changed on-disk line semantics, and the PDG result startLine is now 1-based (#2380). Neither shipped a version bump, so an incremental re-analyze would preserve old 1-based rows (mixed-base index rendered one line too high) and PDG consumers got no signal. - INCREMENTAL_SCHEMA_VERSION 5 -> 6 (forces a one-time full re-analyze) - PDG_RESULT_VERSION 1 -> 2 (result-shape discriminator) Updates the version-pinning tests, the pdgResultVersion result type, and the tools.ts PDG output-contract doc. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): guard manifest label list against SYMBOL_NODE_LABELS drift manifest-extractor's CUSTOM_CONTRACT_RESOLVE_QUERY hand-lists the contract-resolvable labels as a deliberate subset of the shared SYMBOL_NODE_LABELS, guarded only by a comment — the same drift class (#2379) the shared-set refactor eliminated elsewhere. Derive the query's label set and assert it is a strict subset whose difference is exactly {Namespace, Variable, Module}, so adding a symbol label without a conscious manifest decision fails. Query string stays literal (#2325-test-locked). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(mcp): document which tools present 1-based vs 0-based line numbers The schema-resource note listed only context/query/impact as 1-based. After the trace/anchor fixes it now enumerates the full set — context, query, impact, group/cross-repo trace, and explain/pdg_query anchors are 1-based; raw Cypher and single-repo trace stay 0-based (full single-repo-trace parity is a tracked follow-up); BasicBlock/PDG statement lines are separately 1-based. (#2377, #2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): pin impact() line-value display (close the coverage gap) The prior mcp-line-display test only asserted context() + raw cypher, which is why the query() double-conversion (#2380) shipped green. Adds an impact() line-value assertion via the ambiguous-candidate path (the only impact response that surfaces a per-candidate line): two same-name symbols force ambiguity and the candidate at stored 0-based 41 must read 42. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): fix stale rename #2283 mock after 1-based context display rename resolves its symbol via context(), which now presents startLine 1-based (#2377), then subtracts 1 to recover the 0-based file index. The #2283 mock stored startLine:1 but put `oldName` on the file's line 0, so after the 1-based shift the definition edit no longer matched and the write-failure path never fired — the test read 'success' instead of 'partial'. Align the mock content to its stored line (oldName on 0-based line 1). Pre-existing failure surfaced once ubuntu/coverage completed on this branch. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): consolidate line-display tests into one shared DB block The query()/BM25 case had spun up a second full LadybugDB + FTS setup; fold it into the single existing block (adding FTS + the Zqxwvbm seed there) so the file builds one DB, not two. Trims per-file setup cost — relevant to the Windows platform-sensitive suite's under-load 15-minute timeout. Same five assertions, all green. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kigland <shuaizhicheng336@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Livio Gamassia
|
d546fa3cce
|
fix(storage): rename index metadata to gitnexus.json with dual-write compatibility (#2363) | ||
|
Gergő Magyar
|
e148bc089a
|
fix(group): replace LadybugDB-incompatible multi-label Cypher (#2325) (#2327)
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
* fix(group): use labels(n) IN allowlist instead of LadybugDB-incompatible multi-label Cypher (#2325) manifest-extractor and http-route-extractor built Cypher with the openCypher label disjunction `MATCH (n:A|B|C)`, which LadybugDB's parser rejects. The error was swallowed by try/catch, so manifest contracts silently fell back to synthetic UIDs with empty filePath and http-route cross-file handler resolution silently returned null. Replace all 7 queries with `MATCH (n) WHERE labels(n) IN [...]`. LadybugDB returns labels(n) as a single string, so this is an exact allowlist — a 1:1 behavior-preserving syntax translation (validated against LadybugDB 0.17.1). Export the two http-route query constants so integration tests can run the exact production strings against a real DB, and add per-branch real-DB regression coverage (the bug shipped because no test exercised these queries). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): import CypherExecutor from contract-extractor in #2325 test The new manifest regression test imported `CypherExecutor` from `group/types.js`, which does not export it — the type is defined only in `group/contract-extractor.js` (as all production extractors import it). This was a real TS2305 under `tsc -p tsconfig.test.json`, masked from CI because the default tsconfig excludes `test/` and `import type` is erased at runtime. Split the import so the type resolves from its real module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): run #2325 native-LadybugDB tests in the lbug-db project Per TESTING.md, every test that opens a real `@ladybugdb/core` handle must be registered in the sequential `lbug-db` Vitest project (and excluded from `default`) to avoid native-mmap file-lock conflicts across parallel forks on Windows. The two new group integration tests use `withTestLbugDB`/pool-adapter but were in neither list, so they ran under the parallel `default` project. Add both to `lbug-db.include` and `default.exclude`, matching every sibling. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(group): export custom-contract resolve query for #2325 test The #2325 integration test hand-copied the 21-label `custom`-branch resolve query into a local `LABELS_CUSTOM_QUERY` constant, so editing the production allowlist would silently desync the canary. Promote the query to an exported `CUSTOM_CONTRACT_RESOLVE_QUERY` (mirroring http-route-extractor's exported query strings) and import it in the test, so the canary always runs the exact production query. Behavior unchanged — same query string. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): de-brittle the #2325 custom-query label assertion The unit test asserted a fixed 7-label ordered substring of the 21-label custom-branch allowlist, coupling it to label order and no-space formatting — a harmless reorder would have broken it. Replace with order/spacing-tolerant membership checks for a spread of individual labels, keeping the unconditional `not.toContain('Function|Method')` guard as the real regression check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): correct #2325 http-route docstring + add real-trigger canary The http-route test claimed `MATCH (n:Function|Method|CodeElement)` "which LadybugDB rejects" — but that 3-label disjunction actually PARSES. Verified against the real parser, the genuine #2325 trigger is a *reserved-keyword* label in the disjunction: `Macro` and `Union` both are, and only the manifest custom branch (21-label list) and the lib branch (missing `Package` table) actually threw. The http-route conversion to `labels(n) IN [...]` was a consistency change, not a parser fix. Correct the misleading docstring and add a rejection canary pinned to the real cause (`MATCH (n:Function|Macro|Union)` rejects), so a future query that reintroduces a reserved-keyword disjunction is caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): cover the thrift package-strip path against a real LadybugDB The thrift-only branch of resolveSymbol strips a `package.` prefix from the service name (`com.example.AuthService` -> `AuthService`) before the Class/Interface lookup — previously exercised only with a mocked executor. Add a service-contract integration case (no method, so it takes the package-strip path, not the grpc-identical method path) that resolves the real `cls:AuthService`. Without the strip the lookup matches nothing and falls back to a synthetic uid, so this is a non-vacuous guard for the strip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): drop vestigial 'Package' label from lib contract lookup The `lib` branch allowlisted `labels(n) IN ['Package','Module']`, but there is no `Package` node table (see NODE_TABLES) — the entry only ever matched nothing. Restrict to `['Module']`, the label libraries actually resolve to. Behavior-neutral: the lib integration case still resolves its Module symbol. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(group): update PIPELINE label-scoped queries to labels(n) IN form The resolveSymbol label-scoping bullets still showed the banned `MATCH (n:A|B)` disjunction; a contributor copying them would reintroduce #2325. Rewrite them in the actual `labels(n) IN [...]` form, note the real trigger (LadybugDB rejects a disjunction naming a reserved keyword such as `Macro`/`Union`), and reflect the lib allowlist as `['Module']` after dropping the vestigial `Package` label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(group): correct #2325 root-cause comments in the extractors The production comments claimed LadybugDB rejects the `MATCH (n:A|B)` disjunction "outright". Verified against the real parser, it rejects only when a label is a reserved keyword (`Macro`, `Union`) or names a missing node table. So only the manifest `custom` branch (reserved keywords in its 21-label list) and the `lib` branch (missing `Package` table) actually threw; the http-route/grpc/thrift/topic disjunctions parse fine and were converted to `labels(n) IN [...]` for consistency and future-proofing, not because they were broken. Rewrite the comments to say so accurately. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): make #2325 test prose name the real reserved-keyword trigger The manifest test docstring/title and the unit-test comment said LadybugDB rejects the `MATCH (n:A|B)` disjunction generally. It rejects only when a label is a reserved keyword (`Macro`/`Union`) or a missing table. Reword the docstring (custom + lib branches threw; others parsed), retitle the rejection canary to "its list names reserved keywords Macro/Union", and correct the unit-test comment. The rejection canary still passes — the custom 21-label list does contain Macro/Union. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |