mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-07 08:26:11 +00:00
85 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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> |
||
|
Sparsh
|
028bd11053
|
fix(group): cache read-only bridge handle to fix Windows @group reopen (#2274) (#2313)
* fix(group): cache read-only bridge handle to fix Windows @group reopen (#2274) A long-lived MCP server opened bridge.lbug read-only, queried, and closed it on every @group trace/impact call. On Windows the in-process reopen of the same file fails (the OS handle is not fully released before the next open races in), so repeated @group calls broke. #2269 fixed Linux/macOS by skipping CHECKPOINT on read-only handles; Windows stayed broken. Instead of fighting LadybugDB's Windows close/reopen timing: cache one read-only handle per groupDir and reuse it across calls (open-once-per-process already works on Windows). getCachedBridgeReadOnly: - reuses a single handle keyed by resolved groupDir, - invalidates on mtime change (external writer / re-sync), - invalidates explicitly before same-process writes (writeBridge), - guards concurrent first-open with an in-flight promise (no handle leak), - closes all handles on process exit. closeBridgeDb now no-ops for the cached handle (cache owns its lifetime); uncached/writable handles are unaffected. ensureBridgeReady uses the cache. The in-process write->read reopen of the same bridge.lbug file remains a known LadybugDB Windows limitation, so the existing reopen tests stay win32-skipped. A new cache-aware itCacheReopen gate applies to the 3 new tests whose setup requires write-then-read in the same process (same class as itLbugReopen). The cache itself exercises read->read reuse and is unaffected. * fix(group): harden bridge RO-handle cache for concurrency, lifetime & Windows (#2313 review) Addresses the tri-review + Copilot findings on the read-only bridge-handle cache: - P1 (F2): serialize queryBridge per cached handle via a per-handle FIFO lock (the conn-lock.ts chain mechanic, keyed per cache entry, not the global lock). Two concurrent @group callers sharing one lbug.Connection can no longer dispatch two queries at once (the heap-corruption hazard). Uncached/writable handles skip the lock at zero cost. - P1 (F3): refcount lease — getCachedBridgeReadOnly acquires, closeBridgeDb releases (no caller change). The native close is deferred until in-flight readers drain (refs===0) and runs exactly once (closeStarted guard). invalidateBridgeCache and the mtime-evict path share one evict/close path. - Windows: bounded drain in evictBridgeEntry — a concurrent group_sync waits (<= WINDOWS_DRAIN_TIMEOUT_MS) for readers to release before the atomic rename on win32 so it stays clean; POSIX remains fully non-blocking; single-threaded sync still closes-before-rename on all platforms. - P0 (F1/F6): gate the mtime cache test with itCacheReopen (win32-skipped) and drop the manual invalidate so writeBridge self-invalidation is under test; add an external-writer (fsp.utimes) reopen case. - Windows coverage (F9): new cross-process integration test seeds bridge.lbug in a separate tsx process, so read->read handle reuse is proven on win32 CI (not skipped). Plus concurrent cold-open dedupe coverage. - P2/P3: scope the Windows NOTE to read->read (F4); JSDoc the closeBridgeDb release/close contract (F5); drop the if-branch in the B2 probe (F7); revert incidental Prettier churn in cross-impact.ts (F14); fix the stale describe header (F15); document the beforeExit/signal and ENOENT-mtime behavior (F11/F13). tsc clean; group unit + integration suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): run the B2 rename-clash probe on win32 via cross-process seed (#2313 review) Moves the B2 "external rename while a cached RO handle is held" probe out of the unit suite (where it was win32-skipped, because its in-process writeBridge->RO-open is the unfixed Windows reopen) into the cross-process integration test, where a separate-process seed makes the RO open clean. The probe now RUNS ON WIN32 CI and empirically answers whether an open RO handle blocks an external atomic rename over bridge.lbug — the assumption under writeBridge's invalidate-before-rename and the win32 drain. Hardened (per adversarial review) so a win32 RED is the real steady-state share-mode signal, not an artifact: - use production retryRename (not bare fsp.rename) so transient EBUSY/EPERM from the Windows AV/indexer scanning the fresh temp file is absorbed; a RED then means the rename is blocked even after retries (FILE_SHARE_DELETE absent -> invalidate-before- rename is load-bearing). - stage the byte-identical replacement BEFORE opening the RO handle, so no second OS handle touches bridge.lbug while LadybugDB holds it (avoids a FILE_SHARE_READ red for the wrong question). - drop the post-rename query (handle survival is covered by the reuse test); the probe's sole verdict is whether the rename is blocked. Removes the old win32-skipped unit B2 (a strict subset of the new probe). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
azizur100389
|
8ad4469e96
|
fix(test): stabilize local Windows gate baselines (#2314) | ||
|
Parafee41
|
7ca7166b8e
|
fix(fastapi): apply APIRouter constructor prefixes (#2312)
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
|
||
|
henry201605
|
0936553d63
|
fix(ingestion/routes): recognise Spring method-level array-form route mappings (#2281)
* feat(routes): extract Spring method-level array-form routes in ingestion + extractor parity test (#2138 follow-up) ingestion's `extractSpringRoutes` (route-extractors/spring.ts) matched only a single string literal on `@(Get|...)Mapping`, so the array form `@GetMapping({"/a","/b"})` produced no graph Route node — while the group-layer `java.ts` scan did match it. That divergence was the root of the #2265 array-form parse-skip gap. - spring.ts: add the array-form alternation `[(string_literal) @value (element_value_array_initializer (string_literal) @value)]` to the two method-declaration query branches (positional + `path=`/`value=`), mirroring the group query. A multi-element array yields one match per element, so the Phase 2 loop emits one route per path with no other change. Class-level `@RequestMapping` array prefixes remain single-literal (rare; left to a follow-up). - test: spring-route-parity runs one shared Java fixture through BOTH extractors (ingestion `extractSpringRoutes` + group `JAVA_HTTP_PLUGIN.scan`) and asserts identical provider {method,path} sets — the parity guard the maintainer asked for in #2078, so the two Spring extractors can't silently drift again (verified: reverting the array branch turns the parity test red). * fix(ingestion/routes): suppress wrong unprefixed route under class-array @RequestMapping; cover named-array + class-array parity Addresses PR review on #2281: - P2 class-array wrong-route: class branches now match the array form only to detect it; a method-level array route under a class-level array-form @RequestMapping is suppressed rather than emitted with a dropped prefix, so ingestion stays a strict subset of the group scan. Scalar method paths under an array class prefix are unchanged (pre-existing). Full class-array cross-product support tracked in a follow-up. - P2 named-array coverage: added value={...}/path={...} parity cases, a consumes/produces array false-positive case, and a dedicated empty-provider-set assertion. - P3 stale comments: updated the routeCoverage comment in java.ts and the route-parse-skip test note; narrowed the parity test drift claim. routeCoverage stays 'partial'. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
698f5efc82
|
feat(group): resolve inline HTTP provider handlers via call-site line (#2276) (#2282)
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(group): resolve Go inline provider handlers via line containment (#2276) Widen the Go HandleFunc + framework-route handler capture to match func literals and emit name:null + call-site line for them, so an inline handler resolves to its containing/closure symbol instead of file-level. Named identifier handlers keep resolving by name. * feat(group): resolve Laravel closure provider handlers via line containment (#2276) Capture the Laravel route handler argument; a closure (anonymous function or arrow fn) now emits name:null + the registration line so it resolves to its containing symbol (service-provider boot, controller method) by containment. Named-controller routes keep the 'route' label. File-scope closures stay file-level (PHP closures not yet indexed). * feat(group): wire call-site line on FastAPI provider emits (#2276) Set line on the FastAPI @app/@router provider detections (already name:null) so the source-scan fallback resolves the decorated handler by line-span containment. Best-effort: FastAPI routes are graph-backed and the function span starts at def, so this lands the single-decorator case. Flask add_url_rule already carried line. * feat(group): wire call-site line on Kotlin/Java Spring provider emits (#2276) Add line to the Kotlin and Java Spring @*Mapping provider detections for parity with the consumer emits and a future inline DSL. Inert for current resolution: a named Spring controller method resolves by name and never falls through to line-span containment. * fix(review): apply autofix feedback Pin two documented limitations with tests: a file-scope Laravel closure and a multi-decorator FastAPI handler both degrade to file-level rather than mis-attributing (#2276 ce-code-review autofix). * test(group): lock named gin framework-route resolves by name not registrar (#2276) Reviewer verified named Go handlers still resolve by name across the widened queries; the HandleFunc path was already pinned, this adds the framework-route (gin/echo) path with a DB + enclosing registrar whose span covers the registration line, proving the emitted line never diverts a named provider to its registrar via containment. * test(group): end-to-end inline Go provider resolution against real LadybugDB (#2276) Closes the validation gap that all prior coverage mocked CONTAINING_QUERY: runs the real pipeline over a Go file with an inline http.HandleFunc func-literal handler, persists into a real LadybugDB, and runs the production HttpRouteExtractor against the real executor — proving the emitted call-site line lands inside main()'s real 0-based span and yields source_scan_resolved, not the file-level fallback. * fix(test): use fs.mkdtemp to satisfy CodeQL insecure-temporary-file gate (#2276) The new integration test created its temp base via a predictable os.tmpdir()+name join, which CodeQL flags as js/insecure-temporary-file (1 high). Switch to fs.mkdtemp for an atomic, randomly-named base dir. * fix(group): anchor Go provider @handler to the trailing argument (#2276) The widened framework-route and HandleFunc handler captures (`[(identifier) (func_literal)] @handler`) were unanchored, so a variadic middleware route `r.GET("/x", mw, func(){})` produced two provider detections — one for the middleware identifier and one for the closure. The contractId-only merge then kept the middleware detection and mis-attributed the route to it (and the pre-existing `mw, namedHandler` shape had the same defect), silently neutralizing the inline-handler containment resolution from #2276. Add a trailing tree-sitter anchor (`@handler .`) so the handler binds the LAST argument of the call, leaving middleware args before it unconstrained. Verified against tree-sitter-go: the multi-arg shapes now yield exactly one detection (the real handler) while every 2-arg case is unchanged. Adds two regression tests pinning that a middleware + inline closure resolves to its containing function and a middleware + named handler resolves by name. * test(group): cover FastAPI @router inline-handler containment (#2276) The @router/APIRouter provider emit gained a call-site `line` in #2276 but only the @app path was tested; the existing @router tests call `extract(null, …)` so the resolver/containment path never ran for @router. Add two tests mirroring the @app cases: a single-decorator @router handler resolves to its function via source_scan_resolved (which fails if `line` is dropped), and a multi-decorator one degrades to file-level. * fix(group): treat synthetic 'route' label as anonymous in cross-trace (#2276) After #2276 an unresolved file-scope Laravel closure emits name:null, so its persisted symbolName falls back to 'handler' — which providerLabel already anonymizes to '<contractId handler>'. But an unresolved named-controller route still carries the synthetic 'route' placeholder, which the sentinel did NOT cover, so group_trace/group_cross_impact rendered it as the literal 'route' while equivalent closures showed '<... handler>'. 'route' is only ever the synthetic Laravel placeholder (php.ts), never a resolved handler name, so add it to the unresolved-generic sentinel set alongside 'handler'/'fetch'. The resolved branch is untouched, so a real symbol genuinely named 'route' still displays its name. Adds a cross-trace test pinning the anonymized label. * fix(group): gate Spring provider line on a present method name (#2276) The Java/Kotlin Spring @*Mapping provider emits set `line` unconditionally while the method name is typed string|null. The 'a named provider never reaches containment' guarantee held only because the grammar always captures a method name — the type did not enforce it. A (grammar-impossible) null name would emit name:null + line and resolve by containment to the enclosing class body instead of staying file-level. Emit `line` only when the method name is truthy, so a nameless provider degrades to file-level (the safe no-mis-attribution outcome). Behavior is unchanged for every real Spring route (name is always present), but the inertness is now enforced rather than incidental. |
||
|
Gergő Magyar
|
49ffd8e316
|
feat(group): resolve cross-file named HTTP handlers (#2275) (#2277)
* feat(group): resolve cross-file named HTTP handlers via unique repo-wide lookup U1 of #2275. When a provider's named handler is defined in a file other than its route registration (e.g. router.get('/x', listUsers) with listUsers imported), the registration file's symbols don't contain it, so resolution fell back to the file-level boundary. Add a repo-wide name query (RESOLVE_BY_NAME_QUERY, the label-union pattern from manifest-extractor) consulted only after the file-scoped lookup misses, and honored ONLY when exactly one Function/Method/CodeElement carries that name (zero/many → keep the file fallback, no wrong-symbol attribution). Provider-only, cached by name. 4 unit tests; 743 group tests pass. * test(bench): cross-file named handler scenario (end-to-end proof of #2275) U2 of #2275. Adds a fifth bench scenario: a backend route whose handler (listUsers) is imported from another file than its registration, with a frontend consumer. Asserts the provider resolves to the handler via the repo-wide unique name lookup (sym=listUsers, uid set) and that the cross-repo trace is symbol- precise (no file-level fallback). verify.mjs now 12/12 on the real pipeline. * fix(review): apply autofix feedback ce-code-review (autofix) — no correctness/security findings; applied test-coverage + robustness fixes: repo-wide query throw -> empty (no exception); by-name lookup cache fires once across same-named handlers; consumers never consult the repo-wide lookup; same-file-wins now asserts the global path is bypassed; bench provider find scoped by contractId; clarified the uniqueness-guard comment. 167 extractor tests. * fix(group): tri-review fixes for cross-file handler resolution Two-engine PR tri-review (Claude swarm+ce, Codex gpt-5.5 swarm+ce+adversarial) on #2277. Correctness/security clean (injection refuted, bind-param). Fixes: - Named-provider wrapper-attach (Codex swarm P1 + Claude ce-adversarial, cross-engine): a named handler that fails both name lookups no longer falls through to line-span containment, which attached the route to the enclosing registrar (e.g. a setupRoutes() wrapper) instead of leaving it empty. Containment now applies only to consumers and inline-arrow providers. - CodeElement/ORM empty-file nodes (Claude ce-adversarial reproduced + ce-maintainability): RESOLVE_BY_NAME_QUERY gains 'AND n.filePath <> ""' so a handler name colliding with a synthetic ORM model node (orm.ts emits filePath:'') neither resolves to an edge-less node nor inflates the uniqueness count and masks the real handler; + a defensive empty-filePath guard in resolveSymbolByNameUnique. Added LIMIT 2 (Codex swarm P3 + ce-maintainability) to bound homonym materialization (count guard stays exact). - Documented the aliased-import limitation (Codex adversarial): the route-site identifier is the local alias, fix deferred to #2275 import narrowing. - README expected verdict 9/9 -> 12/12 (Codex swarm+ce P3). Tests: +3 (wrapper-no-attach, empty-filePath reject, empty-registration-file resolves) covering the cross-engine gaps. 170 extractor / 748 group+integration pass; bench 12/12 end-to-end. * feat(group): import-pinned handler resolution (fixes deferred alias case) Resolves the tri-review's deferred item: cross-file named handlers are now pinned to their import's target module instead of resolved by name alone, so aliases and names that collide with a local symbol resolve correctly. - node.ts builds a local-binding -> {declared name, module} map from the file's named imports; the express handler emits the DECLARED name + a handlerImport {name, module} (HttpDetection gains the optional field). - resolveDetectionSymbol gains an imported-handler rung: resolveImportedSymbol pins to the import's target file via RESOLVE_IN_MODULE_QUERY (n.name= AND filePath STARTS WITH the resolved module path), unique-match only. An imported handler never uses file-scoped lookup (it is defined elsewhere); on a module miss it falls back to a unique repo-wide name match on the DECLARED name, then null. Relative imports only; bare/non-relative imports keep the repo-wide fallback. Cached by (module-prefix, name). - Closes the Codex-adversarial alias finding: import { listUsers as handleUsers } + an unrelated handleUsers no longer mis-resolves — the route resolves to the imported listUsers in its module, and the alias is never looked up. - Shared toResolvedSymbol helper (dedups the row->symbol + empty-filePath guard). Tests: alias-resolves-to-declared-name + module-pin-resolves-ambiguous-name unit tests; same-file-wins reworked to a genuinely LOCAL handler. Bench scenario 6 (aliased import with a decoy) proves it end-to-end. 172 extractor / 751 group+integration pass; bench 14/14. * feat(group): import-pinned resolution for Python aliased handlers Extends the JS/TS import-pinning to Python. The Python analog of express router.get(path, handler) is Flask's imperative add_url_rule(view_func=...), whose view is often an imported (aliased) symbol. - New Flask add_url_rule provider pattern (path + view_func handler + methods; default GET, methods=[...] honored). High Flask-specificity keeps false positives low — unlike bare path()/Route(), which the plugin deliberately leaves to graph Route nodes. - buildPythonImportMap resolves 'from .mod import name as alias' (and plain 'from mod import name') to the declared name + raw module spec. - resolveModuleBase generalized to two relative-import dialects: path-style (JS './h/users') and dotted (Python '.handlers.users', '..pkg.users' — leading dots are package levels). Bare/absolute imports keep the repo-wide fallback. - Django stays graph-resolved (handlerSymbolId); FastAPI/Flask decorators stay same-file (decorated function). This only adds the imperative imported-view case Python lacked. Tests: Flask aliased add_url_rule unit test (relative dotted module pinned, alias never queried) + bench scenario 7 (end-to-end, 16/16). 173 extractor / 752 group+integration pass. |
||
|
Gergő Magyar
|
1a03c8527a
|
feat(group): cross-repo call trace using PDG (#2269)
* refactor(group): extract shared resolveBridgeNeighbors from cross-impact
Lift the uid-filtered consumer<->provider ContractLink join (direction +
queryBridge + row normalization + confidence sort) out of runGroupImpact's
inline Phase-2 block into an exported resolveBridgeNeighbors helper. Behavior
is unchanged for impact; the helper becomes the single shared bridge join so
the upcoming cross-repo trace path never forks its own copy of the neighbor
Cypher. Empty uid sets short-circuit without a DB round-trip.
Adds direct coverage (real bridge via writeBridge/openBridgeDbReadOnly) for
both directions plus the empty-set and unknown-uid edges.
* feat(group): cross-repo trace stitching (groupTrace + runGroupTrace)
Add GroupService.groupTrace and the pure runGroupTrace engine that stitches
per-repo CALLS/HAS_METHOD trace segments across one ContractLink boundary in
the group bridge:
from --(local trace)--> consumer --(ContractLink)--> provider --(local trace)--> to
- Resolves from/to across all members (symbol node id == bridge symbolUid);
same-repo endpoints delegate to a single local trace with no crossing.
- Single boundary crossing (MAX_SUPPORTED_CROSS_DEPTH); deeper crossDepth is
clamped with a note, mirroring cross-impact.
- Discriminated GroupTraceResult union (ok|not_found|ambiguous|error) with
per-hop repo tags, a typed crossings[] entry, and centralized degraded-state
note constants (TRACE_NOTES). No .
- Trace-specific pair query (keeps BOTH crossing endpoints) lives in this
module; the uid-filtered neighbor join (resolveBridgeNeighbors) is reused
where it fits. ensureBridgeReady exported for reuse.
- New GroupToolPort methods (trace/resolveSymbol/pdgFlows) are optional so
existing port mocks keep type-checking; runGroupTrace guards on presence.
PDG enrichment is wired as an opt-in hook (enrichSegment) — the port method is
stubbed until U4. Covered by unit tests over a real bridge + mocked port.
* feat(group): route trace tool to groupTrace on @group syntax
Wire the cross-repo trace through the existing @group dispatch:
- callTool routes trace with an @-prefixed repo to callToolAtGroupRepo, which
forwards from/to/uid/file/maxDepth/includeTests plus the experimental
pdg/crossDepth flags to GroupService.groupTrace. Member path in @group/path
is advisory for trace (resolution is whole-group).
- Port gains trace/resolveSymbol/pdgFlows adapters. resolveSymbolForGroup wraps
the shared resolveSymbolCandidates so groupTrace can locate the member repo
and recover each endpoint node id (== bridge symbolUid). pdgFlowsForGroup is
a degraded stub here (call-level only); U4 implements the REACHING_DEF walk.
- trace tool schema documents the @group entry point, pdg, and crossDepth.
Single-repo trace is untouched. Covered by dispatch-routing tests (@group ->
groupTrace, non-group stays local) and tool-schema assertions.
* feat(group): opt-in PDG data-flow enrichment for cross-repo trace
Implement _pdgFlowsForGroupImpl: the real REACHING_DEF anchor walk that backs
the port pdgFlows adapter (replacing the U3 call-level stub). When pdg:true and
the segment repo has a flows PDG layer, the boundary-adjacent segments carry
their intra-procedural def->use hops:
- Anchors by the boundary symbol UID (precise; avoids the by-name ambiguity the
resolveBlockAnchor path can hit), then reuses the same span-anchored,
bind-param-only flows query as pdg_query (BasicBlock id-prefix + [start+1,
end+1] line window; no rel-property index, so the anchor IS the bound).
- Stays intra-procedural: data flow never crosses the repo boundary.
- pdgStampForMode probe: false -> available:false (degrade with note); the
trace stays ok. Any query failure is swallowed (enrichment is auxiliary).
Covered by runGroupTrace enrichment tests: dataFlow attached on opt-in,
degraded note when no layer, and no pdgFlows call when pdg is omitted.
* test(group): evaluation-first cross-repo trace e2e (two real indexes)
End-to-end gate for the cross-repo trace: stands up two real LadybugDB indexes
(consumer 'frontend' + provider 'backend'), a real ContractLink bridge, and a
real LocalBackend with both repos registered, then drives the public
callTool('trace', { repo: '@grp', pdg: true }) and asserts:
- the stitched checkout -> callUsers -(CONTRACT_LINK)-> handleUsers -> getUsers
path, each hop tagged with its member repo
- real REACHING_DEF data-flow enrichment of the consumer segment (userId)
- a degraded 'No PDG layer in app/backend' note (provider has no PDG layer)
- single-repo trace against one member is unchanged (no crossings)
Hand-persists the minimal real graph (deterministic; a full two-repo analyze is
heavier than this gate needs) and exercises real Cypher across
resolveSymbolCandidates, _traceImpl, the bridge pair query, and
_pdgFlowsForGroupImpl. Windows-skipped (describeReopen) and registered in the
cross-platform native-lbug set.
Scoped to a single @group call: opening bridge.lbug read-only a SECOND time in
one process currently fails (shared bridge open/close lifecycle, also affects
impact @group) — the pdg-omitted/clamp variants are unit-covered.
* docs(group): document cross-repo trace + PDG enrichment
ARCHITECTURE.md: trace is now group-aware; describe the @group cross-repo
stitch over a single ContractLink boundary (CONTRACT_LINK hop, crossings[],
crossDepth clamp), the opt-in experimental PDG REACHING_DEF enrichment of
boundary-adjacent segments, the symbolUid-grain join between the two stores,
and the deferred full cross-program (SDG-like) data flow. PIPELINE.md: add the
cross-trace consumer of the bridge with its pair-query rationale.
Does not touch gitnexus/CHANGELOG.md (release-owned).
* fix(review): apply autofix feedback
Apply safe_auto findings from ce-code-review (run 20260622-094243):
- local-backend.ts: drop (r: any) in _pdgFlowsForGroupImpl row map; coerce
hop line via Number() so a nullish LadybugDB cell can't surface NaN.
- tools.ts: advertise the forwarded param in the trace schema and add
crossDepth maximum:10 (schema now matches what groupTrace reads).
- cross-trace.ts: parallelize per-member resolveSymbol/resolveRepo with
order-preserving Promise.all (matches groupContext/groupQuery); add a note
when pdg:true is passed to a same-repo trace (PDG only enriches at a
cross-repo boundary).
- tests: remove / tighten (no-any rule).
Residual gated_auto/manual findings (unbounded crossing query + loop,
whole-file PDG widening on absent span, error-vs-no_path masking, top-level
try/catch parity, helper dedupe, branch-coverage gaps) are recorded in the run
artifact for the PR body.
* fix(group): skip CHECKPOINT on read-only bridge close so it can reopen
Root cause of the in-process bridge.lbug reopen failure (which broke repeated
@group impact/trace calls in a long-lived MCP server): closeBridgeDb issued
CHECKPOINT on EVERY handle, including read-only ones. A CHECKPOINT on a
read-only connection has nothing to flush but leaves a WAL/shadow lock artifact
that makes the next read-only open of the same path fail (openBridgeDbReadOnly
returns null -> 'Could not open bridge.lbug read-only'). Reproduced: open ->
query -> closeBridgeDb -> open again returned null only when the close ran
CHECKPOINT; a non-checkpoint close reopened fine, and the raw native
open/close cycle was never the problem.
Fix: tag read-only handles (BridgeHandle._readOnly, set by openBridgeDbReadOnly)
and skip CHECKPOINT for them in closeBridgeDb. Writable handles are unchanged
(they still flush before close). This is the shared bridge-db close path, so
impact @group benefits identically.
- Regression test in bridge-db.test.ts: open/query/close/open/query/open in one
process now succeeds.
- Re-enabled the second @group call in cross-trace-e2e.test.ts (was scoped to a
single call for this very limitation).
* fix(group): bring bridge-db close to parity with the core adapter safeClose
The bridge open/close cycle was less robust than the main graph DB's: closeBridgeDb
closed the connection/database but skipped the post-close steps the core adapter's
safeClose performs, so a rapid in-process reopen could race the OS handle release
(Windows) or an orphaned WAL sidecar. That gap is why the close-then-reopen tests
had to skip Windows.
closeBridgeDb now mirrors safeClose after closing the handle:
- waitForWindowsHandleRelease(dbPath): probe the file (+ .wal) until the residual
Windows lock clears, so the next open does not race (warns if the budget is
exhausted, matching the core adapter).
- finalizeLbugSidecarsAfterClose(dbPath): quarantine an orphaned WAL (shadow
missing) so the next open replays a consistent file.
Both helpers are the same ones safeClose uses (Windows-proven via the core adapter
CI), and the bridge read open already retries transient locks. Combined with the
read-only CHECKPOINT skip, the bridge reopen is now robust on every platform, so
the close-then-reopen tests run on all platforms (Windows CI exercises them via the
cross-platform subset). No write-path behavior change; Linux/macOS unaffected.
* fix(group): bound cross-repo crossing fan-out (LIMIT + segment memoization)
Address the top review residual: the bridge crossing query was unbounded and the
crossing-selection loop could run an O(2*N) sequential trace-BFS over every
ContractLink between a repo pair.
- CY_CROSSINGS_BETWEEN now ORDERs BY confidence DESC and LIMITs to
MAX_CROSSINGS_TO_TRY + 1; listCrossingsBetween slices to the cap and reports
truncation. Exceeding the cap surfaces a note (no silent truncation), keeping
the highest-confidence crossings. Aligns with the repo's anchored+LIMIT-bounded
query discipline (LadybugDB has no rel-property index).
- The home-repo segment (from -> consumer) depends only on the consumer uid and
the target-repo segment (provider -> to) only on the provider uid, so each is
memoized by that uid. Many crossings sharing a consumer/provider (one client
call linked to several providers) now cost one trace per distinct endpoint
instead of one per crossing. A consumer whose segment already failed is skipped
for every later crossing that shares it.
Test: two links sharing a consumer (first provider unreachable, second reachable)
assert the from->consumer segment is traced exactly once and the second crossing
wins.
* fix(group): restore Windows skip for bridge reopen tests; drop ineffective close-side probe
The previous commit flipped the bridge close-then-reopen tests to run on Windows,
betting that a close-side waitForWindowsHandleRelease + finalizeLbugSidecarsAfterClose
probe (mirroring the core adapter safeClose) would make the in-process reopen work
there. Windows CI proved otherwise: 4 writeBridge->openBridgeDbReadOnly tests fail
('expected null not to be null' — the read open returns null). The writable-close ->
read-open handoff plus writeBridge's atomic sidecar rename does not release the OS
file handle before the read open races, and the existing open-side LBUG_OPEN_RETRY
only retries lock-pattern errors, not the post-rename sidecar database-id mismatch.
macOS passes; the core adapter's own reopen also passes — this is bridge+Windows
specific.
- Revert itLbugReopen to the Windows skip (the pre-existing, correct state).
- Remove the close-side probe + finalize from closeBridgeDb: it did NOT close the
Windows gap, and reviewers flagged it for hot-path latency (finalize ran on every
close, all platforms) and safeClose duplication.
- KEEP the load-bearing fix — skipping CHECKPOINT on read-only handles — which fixed
the reproduced Linux/macOS in-process reopen artifact (the real bug).
Net: Linux/macOS repeated @group impact/trace works in-process; Windows in-process
bridge reopen remains a documented limitation (unchanged from before this PR).
* fix(group): surface degraded members + cap truncation; honest crossDepth schema
Address the cross-engine-corroborated tri-review findings (Codex + Claude):
- resolveAcrossMembers / runGroupTrace now track member repos that could NOT be
queried (resolveRepo or resolveSymbol threw) and, when the result is not_found,
attach a degraded-member note. A transient/corrupt member DB is no longer
silently reported as a clean 'symbol absent' not_found. (Codex B1+B3 + ce-reliability.)
- The cross-repo not_found now carries a programmatic truncated:true flag (and a
clearer suggestion) when the MAX_CROSSINGS_TO_TRY cap was hit, so a consumer can
distinguish 'no path' from 'cap may have hidden a connecting ContractLink'.
(Codex B3 + ce-adversarial + ce-api-contract.)
- trace tool schema: crossDepth maximum 10 -> 1 to match the implementation's
single-hop clamp (the schema previously advertised an unsupported 2-10 range).
(ce-api-contract, conf 100.)
Test: a member whose resolveSymbol throws yields not_found WITH a degraded note
naming the unreachable repo (if-free responder map).
* docs(group): clarify trace @group/memberPath is advisory (resolves all members)
Tri-review (Codex ce, conf 100) caught a doc/impl inconsistency: ARCHITECTURE.md
lumped trace with query/context/impact as honoring @group/memberPath member
scoping, but cross-repo trace resolves from/to across ALL members (the member
path is advisory). Clarify the behavior and point to from_uid/to_uid for
disambiguating same-named symbols across members.
* feat(group): file-level boundary fallback so cross-repo trace works on HTTP contracts
Benchmark (bench/cross-repo-trace/) running the REAL pipeline (runFullAnalysis
--pdg -> real syncGroup -> trace @group) found that cross-repo trace returned
not_found for real HTTP links even though sync built the correct ContractLinks:
HTTP (and other source-scan) contracts hardcode symbolUid:'' (http-route-extractor),
and both cross-trace AND cross-impact join crossings by Contract.symbolUid, which
never matches an empty uid. (Pre-existing — impact @group has the same gap.)
Fix: when a crossing's symbolUid is empty, fall back to the contract's FILE — if
the user's from/to resolves into the contract file, that endpoint anchors the
boundary. CY_CROSSINGS_BETWEEN now returns consumer/provider filePath; a crossing
is kept if it can be anchored by uid OR file on each side; a fileBoundaryFallback
note flags that the boundary is file-level, not symbol-precise. This makes the
common 'trace from=<calling fn> to=<handler fn>' case work end-to-end (verified:
fetchUsers -> listUsers stitches with a CONTRACT_LINK hop + PDG enrichment, 2/2).
Limits (documented in the bench README + the note): anonymous handlers have no
named target; when several contracts share files the file fallback may attach the
wrong contractId to a correct path. The proper upstream fix is to populate
symbolUid in the HTTP extraction (benefits impact too) — the bench is its gate.
Adds a unit test pinning the empty-symbolUid file-fallback stitch.
* fix(group): resolve HTTP contract symbolUid by containment (fixes cross-repo trace + impact)
Addresses the root cause behind the cross-repo trace file-fallback: HTTP
contracts hardcoded symbolUid:'' (http-route-extractor), so both cross-trace and
cross-impact — which join crossings on Contract.symbolUid — could not traverse
HTTP links. (Also found: the pre-existing graph-assisted resolution queried the
wrong edge, CONTAINS instead of DEFINES, so it never resolved a uid either.)
Now the extractor resolves each detection to a real symbol:
- HttpDetection carries the call-site line (node.ts sets it on every express/
fetch/axios/jquery/nest detection; express also captures the handler arg).
- resolveDetectionSymbol resolves the named handler first, else the innermost
Function/Method whose line span encloses the call (consumer = the function
containing the fetch; provider = the named/inline handler), over the correct
File-[DEFINES]->symbol edge. Base-tolerant (0- vs 1-based startLine).
- Wired into both source-scan and graph-assisted provider/consumer paths.
Verified end-to-end (bench/cross-repo-trace): all 4 contracts now carry real
uids, trace is symbol-precise (GET pair -> http::GET, POST -> http::POST, no
file-fallback note), and impact @group fans out (cross_repo_hits 0 -> 1). The
cross-trace file-level fallback remains as the secondary path for truly
anonymous handlers. Adds 2 containment unit tests; 738 group/integration pass.
Languages other than JS/TS still resolve providers by handler name; their
consumers fall through to the file fallback until their plugins set the line.
* fix(group): extend HTTP symbolUid containment to all languages + nested methods
Completes the symbolUid resolution across every bundled HTTP plugin: Python, Go,
PHP, Kotlin and Java now set the call-site line on their consumer (and Feign/
named) detections, so their HTTP contracts resolve to the containing function
the same way Node/TS already did.
Also generalizes the containment query: it now matches Function/Method/CodeElement
by filePath (UNION ALL) instead of File-[DEFINES]->symbol. The DEFINES edge only
reaches a file's TOP-LEVEL symbols, so methods nested in classes (Java/Kotlin —
File defines the class, the class defines the method) were invisible; matching by
filePath reaches them. Verified against a real index (LadybugDB supports the
UNION); JS/TS still fully symbol-precise (bench 2/2), 709 group tests pass.
Residual is now only the inherent case — a fully anonymous handler with no named
callee — which keeps the cross-trace file-level fallback.
* feat(group): destination trace — follow a consumer to an anonymous handler
Handles the one inherent residual: an anonymous route handler
(`router.get('/x', (req,res) => …)`) has no symbol node at all (the file holds
only a Const + PDG BasicBlocks), so it can never be named as a trace `to`.
Adds a DESTINATION TRACE: omit to/to_uid/to_file on an @group trace and
`trace from=<consumer>` follows the consumer's outgoing HTTP call across the
bridge and reports where it lands — by route + file:line, with a notes[] entry
flagging the handler as anonymous. Implemented as a new branch in runGroupTrace
(p.destination) backed by CY_CROSSINGS_FROM (all ContractLinks leaving the
consumer repo) + stitchToDestination; the provider endpoint is labelled
'<METHOD /path handler>' when its symbolName is a generic token/file basename.
The MCP routing already omitted an absent `to`, so only the schema docs changed.
parseTraceParams now treats a missing `to` as a destination trace instead of an
error. Verified end-to-end: anonymous fixture reports
'app/frontend:fetchUsers -> app/backend:<http::GET::/api/users handler>'; named
fixture lands at the real function. Adds 2 unit tests; 915 group tests pass.
* fix(group): tri-review fixes for cross-repo trace + symbolUid resolution
Two-engine tri-review (Claude swarm+ce + Codex GPT-5.5 swarm+ce+adversarial)
surfaced these; cross-engine-corroborated unless noted.
Correctness (P1, all four lanes): destination trace reported the WRONG endpoint
— an empty-uid consumer made trace(from->from) trivially succeed, so the highest-
confidence same-file crossing won regardless of which call `from` makes.
stitchToDestination now collects ALL connecting crossings, prefers symbol-precise
hits, and returns `ambiguous` (with candidates) when it cannot disambiguate.
Correctness (P1, Codex): resolveDetectionSymbol early-returned null when
d.line==null, blocking NAME resolution for named providers that set no line
(Spring/Go/etc.). Name resolution now runs first; only containment needs a line.
Correctness (P2): resolveContainingSymbol OR-ed `line` and `line-1`, which could
mis-pick a one-line sibling. It now probes the base-correct `line-1` first and
falls back to `line` only if nothing matches.
Correctness (Codex): anonymous Express handlers emitted name:'handler' and could
attach to an unrelated fn literally named `handler`. node.ts now emits name:null
for non-identifier handlers (containment-only).
Robustness: drop the first-symbol-in-file pickSymbolUid guess from the graph
consumer/provider paths (a wrong uid would win the contractId merge); remove the
dead CONTAINS_QUERY fallback (CONTAINS is File->Folder, never a symbol) + the now
-unused pickSymbolUid/handlerName; seed destination notes with degraded-member
notes so a successful trace still surfaces them; providerLabel takes providerUid
so a resolved fn named `handler` is not mislabeled anonymous, and only true file
basenames (known extensions) — not any dotted name — count as anonymous.
API contract: a single-repo trace with no `to` now returns an actionable error
(destination trace is @group-only) instead of "symbol 'undefined' not found".
Maintainability/tests: narrow asLocalTrace per-field (drop as-unknown-as); fix the
PR's lone as-any (vi.mocked); if-free e2e teardown; qualify the bench README.
Adds ambiguous-destination, anonymous-handler-no-false-name, and single-repo-no-to
tests; redirects graph mocks CONTAINS->UNION ALL. 918 group/integration pass.
* fix(group): carry degraded-member notes through SUCCESSFUL group traces
A reviewer (koriyoshi2041, PR #2269) correctly flagged that degraded-member
resolution was surfaced only on not_found, not on a successful ok result. Group
trace resolves names across ALL members, so an ok is 'unique among the members
we could query' — if a member that threw during resolveSymbol also holds from/to,
the real answer could be ambiguous. The destination path already seeded the note
(prior commit); this extends it to the same-repo and cross-repo success paths by
seeding the dispatch notes with degradedNotes([...fromRes.degraded, ...toRes.degraded]).
Adds a regression test: reg-be throws while a same-repo trace succeeds in reg-fe;
the ok result now carries the 'could not be queried' degraded note (app/backend).
* test(bench): cover all implemented cross-repo trace cases in one runner
Replace the single named-handler script with a self-contained verify.mjs that
generates each fixture inline and exercises every implemented end-to-end case
against the real analyze -> sync -> trace/impact pipeline, asserting PASS/FAIL
(exit non-zero on failure). 10 checks across 4 scenarios:
- named handlers: 4/4 symbolUid resolved; symbol-precise GET vs POST crossing
selection; destination trace lands at the named handler.
- anonymous handler: empty symbolUid; destination trace reports it by route with
the anonymous note.
- impact @group fan-out (cross_repo_hits >= 1).
- multi-language (Python Flask + requests): link built, cross-repo trace stitches,
and the file-level boundary fallback is exercised when the provider has no uid.
Ambiguous-destination and degraded-member paths need synthetic inputs the real
analyzer cannot produce, so they stay in the unit suite (documented in the README
+ script header). Removes verify-named.mjs + fixtures-named/ (folded inline).
* test(group): pin destination degraded-success + precise-tier ambiguity
Adds the two regression guards koriyoshi2041 requested on PR #2269 after the
degraded-on-success fix:
- destination trace success with a degraded member: reg-fe resolves from and
follows the link to an anonymous handler while reg-be throws; the ok result
carries the anonymous endpoint AND the 'could not be queried' degraded note, so
the no-to path stays aligned with explicit to traces.
- multiple PRECISE destination hits: one from reaches two consumers with resolved
uids linked to different routes; the result is ambiguous (role: to) with both
route candidates. Distinct from the existing file-level ambiguous test, this
pins the stronger precise tier against a future change silently picking the
highest-confidence destination.
Both already pass against current behavior; 716 group tests pass.
|
||
|
henry201605
|
b16ec344f7
|
perf(group/http): skip source parse for graph-covered route files (#2138 Part 2) (#2265)
* feat(routes): resolve + persist handler symbol on Route nodes (#2138 part 2, WIP) Part 2 groundwork for #2138: give the graph-assisted HTTP provider path the handler symbol directly, so it no longer re-parses source to recover the handler name. (The remaining parse-skip in extract() + a call-count benchmark land in a follow-up commit.) - ExtractedDecoratorRoute gains `handlerName`; the Spring extractor captures the decorated method's name (the method_declaration node is in hand). - New `resolveRouteHandlerSymbols` (call-processor) resolves each route's handler to a real symbol UID, keyed by normalized route URL — Laravel framework routes (controller + method) and decorator routes (Spring/FastAPI) both reduce to `(filePath, name) -> nodeId`. Threaded through the parse phase onto `ParseOutput.routeHandlerSymbols`. - routes phase stamps `Route.handlerSymbolId`; persisted end-to-end (schema + Route CSV row + getCopyQuery COPY columns), mirroring Part 1's `method`. - HttpRouteExtractor: `HANDLES_ROUTE_QUERY` returns `handlerSymbolId`; `extractProvidersGraph` uses it as the authoritative symbol and SKIPS `getDetections()` for resolved rows (CONTAINS is a cheap graph lookup for the display name only — no tree-sitter parse). Fully backward compatible: an unresolved/old-index route with no `handlerSymbolId` keeps the source-scan fallback. - Extracted `normalizeExtractedRoutePath` to `route-extractors/route-path.ts` (shared by routes phase + resolver without an import cycle). - SCHEMA_BUMP 6->7 (ParseWorkerResult gained `handlerName`); regenerated the emit-persistence byte-identity baseline (route.csv header gained two columns). - Tests: Spring pipeline asserts the Route node carries a handlerSymbolId resolving to the handler method; extractor fast-path test proves the handler resolves with zero source detections. Refs #2138 * perf(group/http): skip source parse for graph-covered route files (#2138 Part 2) Builds on the persisted Route.handlerSymbolId (U0–U3a). When a file's HANDLES_ROUTE rows all resolve a handler symbol AND its language plugin declares routeCoverage: 'complete' (Java/Python/PHP), the graph is authoritative for that file's providers, so the source scan + tree-sitter parse can be skipped — the scan would only re-discover routes the graph already has. This is the measurable parse reduction #2167 could not show. Consumer safety: routeCoverage: 'complete' asserts *provider* Route-node completeness only. The scan() of those same languages also emits consumer detections (RestTemplate/WebClient/OkHttp/Feign, Guzzle/Http::, requests/httpx), and ingestion's FETCHES edges are JS/TS-only — so the graph cannot back up server-side consumers. A provider-covered controller that also calls out would otherwise lose its consumer contract. Guarded by a cheap, parse-free text gate. - types: HttpLanguagePlugin gains - routeCoverage?: 'complete' | 'partial' (default 'partial') - hasConsumerSignals?(content): false only when the raw source provably has no outbound-HTTP call this plugin detects (conservative). - java/python/php: mark routeCoverage 'complete' + implement hasConsumerSignals with a token regex over their consumer idioms. - http-route-extractor: run the graph provider pass first to build a coveredFiles set; then keep a file covered only when hasConsumerSignals(content) === false (read via readSafe, no parse). scanFiles = files not covered → drives collectProjectDetections + both source scans. Fail-open per file: any unresolved row, a 'partial' language, a positive consumer signal, a missing hook, or an unreadable file leaves the file in the scan set. The orchestrator names no languages — token knowledge stays in the plugins. Net: pure-provider controllers skip the parse (the win); controllers that also call out are still parsed (no consumer loss); partial-coverage languages and graph-less runs are unchanged. - test: route-parse-skip integration test spies the real parseSourceSafe to COUNT parses over a temp repo of Spring controllers with a mock DB — baseline (every file parsed), fully-covered (0 parses), mixed (unresolved file falls back, resolved stays skipped), and provider+consumer (a covered controller that also calls restTemplate is parsed; its consumer contract survives). * fix(group/http): cover Spring HTTP Interface @*Exchange in Java consumer-signal gate #2254 (merged) added Spring 6 HTTP Interface `@(Get|...)Exchange` / `@HttpExchange` as a new Java *consumer* idiom. The #2138 parse-skip consumer-safety gate must recognize it, or a provider-covered file carrying an `@GetExchange` could be parse-skipped and lose that consumer contract. Add `Exchange` to JAVA_HTTP_PLUGIN.hasConsumerSignals (conservative; also matches `restTemplate.exchange(`). * style(group/http): prettier formatting for #2138 Part 2 files * style(ingestion): prettier formatting for call-processor.ts (#2138 Part 2) * fix(group/http): P1 (Java over-claim) + P2 (handler mis-attribution) on top of #2268 (#2138 Part 2) Re-applied on the maintainer's #2268 (expanded Java/Kotlin consumer extraction) base. P1 — `routeCoverage: 'complete'` over-claimed for Java: the graph provider set is a strict subset of the group scan (array-form `@GetMapping({...})`, interface-inherited routes, same-URL multi-verb have no graph Route node), so parse-skip could drop those group-only providers. - java/python → default 'partial' (always source-scanned). Java flips to 'complete' only once ingestion provider extraction matches the group scan (a separate follow-up). Python was a no-op anyway (no handlerName resolved); 'complete' was a latent trap. PHP stays 'complete' (Laravel ingestion ⊇ the group scan, the one language the skip engages for). - python hasConsumerSignals widened to a true superset of scan() (uri=/url= wrapper, aiohttp, urllib). Java's gate already covers #2268's consumer set (same receivers; the @*Exchange token is present). P2 — resolveRouteHandlerSymbols: reserve the URL slot on first encounter even when unresolved (mirrors addRoute first-writer-wins, so a later same-URL route can't stamp the node-winner's slot); refuse to guess on an ambiguous same-name lookup (exactly one match → use it; zero/many → fail-open, never a wrong handler). The cross-source case (filesystem route winning a URL a framework route also normalizes to) is unchanged — the resolver never receives filesystem routes — and stays fail-open. Tests: - route-parse-skip rewritten: the parse-skip win is proven on PHP (fully covered → 0 parses; mixed fallback; consumer-covered file still parsed), plus three Java P1 regression guards (array-form / interface-inherited / multi-verb) asserting the group-only routes survive — verified they go red if Java is flipped back to 'complete'. - resolve-route-handler-symbols: direct unit tests (the fn had none) — unique resolve, ambiguous/unknown fail-open, same-URL reservation, first-writer-wins. - http-consumer-signals: each plugin's hasConsumerSignals is a superset of its scan() consumer idioms; pure providers return false. - route-handler-symbol-roundtrip: real-LadybugDB CSV→COPY→query for Route.handlerSymbolId. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
77741fe13a
|
feat(group): expand Java and Kotlin HTTP consumer extraction (re #1888) (#2268)
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
* feat(group): extract RestTemplate URI.create(...) static paths (Java)
Widen the RestTemplate query @path captures to (_) and resolve
URI.create("/x") arguments via a new extractStaticPathExpression
helper. Variable-bound paths stay unresolved (no consumer).
* feat(group): resolve RestTemplate UriComponentsBuilder chains (Java)
Add appendPath + recursive extractUriComponentsBuilderPath (fromPath/
fromUriString/fromHttpUrl seeds, path/pathSegment append, build/query
passthrough). Host-bearing seeds are normalized downstream. Non-literal
segments stay unresolved.
* feat(group): infer OkHttp request verb from builder chain (Java)
Walk up from the matched .url(...) call to the sibling verb helper
(.post()/.method("X")), defaulting to GET. Variable-bound verbs stay
GET. Re-document the Kotlin OkHttp GET-default pin as an accepted
Java/Kotlin asymmetry (Kotlin verb-walk is a tracked follow-up).
* feat(group): Java HttpClient HEAD, .method("X"), and default-GET
Add HEAD to the verb-helper regex and two dedicated pattern families:
.method("VERB", body) (covers PATCH) and a bare .build() defaulting to
GET. The three families terminate at distinct calls, so each chain
matches exactly one (no double-emit); variable-bound verbs stay unresolved.
* feat(group): extract fully-qualified Java route annotations
Widen JAVA_ROUTE_ANNOTATION_PATTERNS @ann to match scoped_identifier
(predicate-free node-type change), normalizing to the trailing segment
via simpleName in the scan loop. Snapshot-verified: only previously
unmatched FQN annotations gain routes; existing contracts unchanged.
Brings Java to FQN parity with the Kotlin plugin (closes #2254 limitation).
* refactor(group): reuse simpleName helper in hasAnnotation
Drop the inline split('.').pop() (which shadowed the new module-level
simpleName) and call the helper. Note appendPath's deliberate divergence
from the shared joinPath so it is not accidentally unified.
* docs(test): fix reversed Java/Kotlin OkHttp asymmetry comment
The comment stated .kt->POST / .java->GET; it is the opposite — Java
infers the verb (inferOkHttpMethod) so .java emits POST, while Kotlin
still defaults so .kt emits GET. Aligns the prose with the assertion
below it.
* docs(group): correct stale kotlin.ts OkHttp parity comment
The comment claimed Kotlin's GET-default 'mirrors java.ts:OK_HTTP_PATTERNS'
and is 'the same trade-off Java has accepted'. The Java plugin now infers
the verb (inferOkHttpMethod), so this is now a documented Java/Kotlin
asymmetry; the Kotlin verb-walk is the tracked follow-up.
* test(group): pin OkHttp default-GET branch on a distinct path
The bare `.build()` (no verb call) now uses /api/bare-build and is
asserted individually, so the default-GET branch of inferOkHttpMethod
is no longer masked by the explicit .get() case collapsing into the
same {param} slot.
* fix(group): skip OkHttp emission for variable-bound .method(verb)
inferOkHttpMethod now returns string|null: an explicit .method(verb, …)
with a non-literal verb returns null and the loop skips it, instead of
asserting a wrong GET contract. A bare .url().build() with no verb call
still defaults to GET (OkHttp's real default). Matches WebClient
long-form, which also skips variable-bound verbs.
* fix(group): strip query from UriComponentsBuilder seed literal
A query baked into the seed (fromUriString("/base?x=1")) was returned
verbatim, so a later .path("/sub") glued onto it (/base?x=1/sub) and
normalizeHttpPath truncated the tail at ? to /base. Strip ?query at the
seed so .path() appends to a clean base → /base/sub. Host prefixes are
preserved and stripped downstream by normalizeConsumerPath.
* fix(group): add recursion depth guard to extractUriComponentsBuilderPath
The recursive builder-chain walk was unbounded; a pathological or
machine-generated chain could overflow the stack. Cap recursion at
MAX_BUILDER_DEPTH (100) and return null past it — consistent with the
project's other AST-depth guards.
* docs(group): document accepted FQN simple-name collision trade-off
The route discriminator matches on the trailing annotation segment, so a
non-Spring annotation sharing a route name (@com.evil.GetMapping) is
treated as a route — the same trade-off hasAnnotation makes and the
intended Kotlin parity. Note why package-origin gating is deliberately
not added.
* refactor(group): extract static-path helpers to java-static-path.ts
Move the URI.create / UriComponentsBuilder resolution helpers
(methodInvocation*, firstLiteralArgument, appendPath, extractUri*,
extractStaticPathExpression) out of java.ts (back under ~1000 lines).
java.ts imports the four it consumes; inferOkHttpMethod stays. Pure
move, behavior-preserving — full group suite unchanged.
* fix(group): walk builder chain for Java HttpClient verb (#2268)
Replace the three rigid JAVA_HTTP_CLIENT_* pattern families with one
.uri()-anchored query plus inferHttpClientMethod, which walks up the
fluent chain for the verb (mirroring inferOkHttpMethod). The walk is
transparent to intervening .header()/.timeout()/.version() calls, so a
header/timeout hop before the terminal no longer silently drops the
consumer contract.
Relocate both verb-walks onto a shared inferBuilderVerb in
java-static-path.ts and de-export the now-internal methodInvocation*
primitives; java.ts drops 1015 -> 910 lines.
* fix(group): append UriComponentsBuilder .path() verbatim (#2268)
Spring's UriComponentsBuilder.path(p) appends p as-is without inserting
a slash (then collapses duplicate slashes), unlike .pathSegment() which
slash-joins. The resolver used the always-one-slash appendPath for both,
so fromPath("/api").path("users") resolved to /api/users instead of
Spring's /apiusers. Switch the .path() branch to verbatim append plus a
colon-aware duplicate-slash collapse (preserving a host seed's ://);
.pathSegment() keeps appendPath.
* fix(group): skip empty-string verb literal in builder verb-walk (#2268)
`.method("", body)` produced a malformed `http::::/path` consumer:
unquoteLiteral('""') returns "" (not null), so the `=== null` guard
let an empty method through. Treat a falsy literal verb as unresolvable
(return null from the shared inferBuilderVerb) and switch the OkHttp and
HttpClient emission guards to falsiness, so an empty verb skips like a
variable-bound one.
* test(group): harden Java HTTP consumer coverage (#2268)
Add coverage beyond the tri-review findings: a count guard on the
UriComponentsBuilder query-seed test (so a double-emit can't slip past
the two find assertions), an exchange()+UriComponentsBuilder end-to-end
case (the widened (_) @path exchange capture was only covered with
URI.create), and an HttpClient .method("REPORT") custom-verb
pass-through pin.
* docs(group): document pre-path builder rigidity + fix stale refs (#2268)
Document the OkHttp pre-.url() limitation (a builder call before .url()
is missed) at OK_HTTP_PATTERNS, cross-referencing the Java-HttpClient
pre-.uri() dual the verb-walk rewrite leaves in place — so neither
comment overclaims that the chain is walked before the path call. Update
the now-stale 'inferOkHttpMethod in java.ts' references in kotlin.ts and
the test to point at java-static-path.ts after the relocation.
* feat(group): match Java HTTP consumer chains with a pre-path builder call (#2268)
The OkHttp .url() and HttpClient .uri() queries required the path call to
sit directly on the construction, so a builder call BEFORE it —
new Request.Builder().addHeader(...).url(...) or
HttpRequest.newBuilder().version(v).uri(...) — silently dropped the
consumer contract. Match the path call on any receiver and re-impose the
framework anchor in JS (okHttpUrlRootsAtBuilder / httpClientUriRootsAtNewBuilder:
the chain must root at new Request.Builder() / HttpRequest.newBuilder()), so a
preceding call is captured while an unrelated .url()/.uri() is rejected. The
verb-walk now scans the whole chain, so a verb set before the path call also
resolves. Also extract the HttpRequest.newBuilder(URI.create(...)) constructor-arg
form (skipped when a later .uri() overrides it). Resolves the deferred
follow-ups from the round-2 tri-review.
* feat(group): Kotlin OkHttp verb-walk parity with Java (#2268)
The Kotlin OkHttp consumer always emitted GET while the Java side walks
the builder chain to recover the verb — a documented Java/Kotlin
asymmetry. Mirror the verb-walk into kotlin.ts, adapted to the
tree-sitter-kotlin call_expression/navigation_expression grammar: match
.url("literal") on any receiver, gate to chains rooting at
Request.Builder() (kotlinUrlRootsAtRequestBuilder), and scan the whole
chain for the verb (inferKotlinOkHttpMethod — last-wins, null-skip for a
variable/empty .method(verb), resolves a named-argument .method(method="X")).
This brings .kt to full parity with .java — verb inference, a builder
call before .url(), and verb-before-url — pinned by two new Java<->Kotlin
parity-harness rows. Flips the former GET-default asymmetry test.
|
||
|
Dinh Huy
|
dbd4e1c9fb
|
feat(group): Support Django route extraction for multi-repo (#1836)
* [+] Add django route discovery to create cross-link for multi-repo * [+] Update ingestion * [~] Fix bugs and abstraction violation * feat(python-http): add keyword url= and variable propagation for consumer detection - Add REQUESTS_KEYWORD_URL_PATTERNS for requests.get(url='...') keyword args - Add WRAPPER_URI_PATTERNS for generic wrapper.fetch(uri='...') calls - Add WRAPPER_URI_VAR_PATTERNS + buildLocalStringMap for uri=variable propagation - Add LOCAL_STRING_ASSIGNMENTS to track uri='...' assignments - Wire both direct-string and variable-propagation loops in scan() - Add normalizeConsumerPath() helper Note: Automatic cross-link detection remains limited for runtime-computed URLs (URLs built via .format(), string concat, or module constants). Manual manifest links needed for known cross-repo contracts. * [+] add extract uri and url keywork pattern for request http * feat(python-http): add variable propagation for uri=/url= consumer patterns Re-add LOCAL_STRING_ASSIGNMENTS, WRAPPER_URI_VAR_PATTERNS, buildLocalStringMap(), and normalizeConsumerPath() lost during cherry-pick merge of upstream keyword-URL commit. Together with the upstream WRAPPER_URI_PATTERNS and REQUESTS_KEYWORD_URL_PATTERNS, we now detect: - requests.get(url='literal') keyword args - wrapper.fetch(uri='literal') keyword args - wrapper.fetch(uri=variable) where variable was assigned a string literal * fix(group): discover Django roots relative to manage.py dir + multi-project (#1836 R1) A Django project not at the repo root (e.g. backend/manage.py) discovered zero routes: the settings module path was resolved repo-root-relative only, so backend/myproj/settings.py was never found and discovery returned null. Resolve settings, star-imported base settings, ROOT_URLCONF, and the root urls.py against the manage.py's own directory first, then the repo root (resolvedSettingsPath is now project-dir-aware so relative imports anchor correctly). Iterate every manage.py so a monorepo with several Django projects yields each project's root — the provider hook becomes plural (discoverRootRouteFiles → string[]) and the main-thread pass loops over all roots (inner-scoped continues, parser hoisted once per language). Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(group): remove dead code in Django root discovery (#1836 R9) - Collapse the identical if/else in extractStarImports to one push. - Drop the unreachable baseModule.startsWith('.') branch (baseModule is always a resolved slash-path or a bare absolute module — never dot-prefixed). - Import DjangoFileReader from django.ts instead of re-declaring the type. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): walk Django includes once per prefix, not per file (#1836 R2) The include() recursion guard was keyed on file path alone and shared across the whole walk, so a urlconf included under two prefixes (a "diamond" — the same app mounted at /v1/ and /v2/) emitted routes for only the first mount. Key the guard on (resolvedFilePath, accumulatedPrefix) at all three sites (function entry, path()-wrapped include, bare include) so a file reached under two distinct prefixes is walked once per prefix while a genuine cycle (same file + same prefix) still terminates — null/'' prefixes collapse to one key so a no-prefix re-entry is treated as a cycle. MAX_INCLUDE_DEPTH remains the backstop. Adds diamond + self-include-cycle tests. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): extract Django routes from non-list urlpatterns (#1836 R3) findUrlpatternsLists only accepted a list-literal RHS, so common shapes yielded zero routes: concatenation (urlpatterns = a + b), wrapper calls (format_suffix_patterns([...]), i18n_patterns, staticfiles_urlpatterns), and tuples. Add collectUrlpatternContainers to descend binary_operator operands, known wrapper-call list arguments, and tuples. Inherently-dynamic forms (DRF router.urls, comprehensions, bare names) still yield nothing but now emit a debug log so the silent-zero case is observable rather than mysterious. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(group): thread Django parser explicitly, drop module singleton (#1836 R4) extractDjangoRoutes relied on a module-level _djangoParser set via setDjangoParser before each call — hidden state that would break if a second language ever used the include re-parse path, and an easy-to-forget contract. Pass the tree-sitter parser as an explicit parameter of extractDjangoRoutes (the extractRoutes provider hook already receives it) and delete the global plus its setter. The Python provider wires it directly; tests pass the parser in place of the removed setDjangoParser() call. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ingestion): isolate a throwing extractRoutes in the cross-file route pass (#1836 R5) The main-thread cross-file route pass called provider.extractRoutes without a guard, so a throw (e.g. a future grammar edge case in the include() walk) would propagate out of the parse phase and abort the entire analyze — unlike the worker, which isolates per-file failures. Wrap the per-root extractRoutes call in try/catch that logs a warning and continues to the next root. Export extractCrossFileRoutes and add a unit test driving a stub provider whose extractRoutes throws, asserting the pass returns [] and does not propagate. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(ingestion): bucket only route-capable languages in cross-file pass (#1836 R6) extractCrossFileRoutes runs in the deferred band on every analyze (incl. warm all-cache-hit runs). It now derives the set of languages whose provider exposes the cross-file route hooks once, returns early if none do, and buckets only those languages' paths — so a non-framework repo no longer pays to bucket the languages it doesn't use here. Route results are intentionally not persisted across runs, so a Django repo still re-derives its routes each analyze; documented inline that cross-run route caching is a deliberate follow-up rather than implemented here. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(group): prettier-format http-patterns/python.ts (#1836 R7) The file was not formatted to the root .prettierrc (the consumer-path normalizer used single-line try/catch and method chains), so the CI quality/format check (`prettier --check .`) failed. Reflow only — no logic change (`git diff -w` confines the change to normalizeConsumerPath's layout). Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): dedup Python URI detections by byte offset, not line arithmetic (#1836 R8) The wrapper-URI dedup key was lineNum*1000+methodRow, which can collide for distinct calls in files over 1000 lines (carry into the row term) and can fail to dedup a genuine duplicate when a node straddles a line boundary. Key on node byte offsets (`${pathNode.startIndex}:${methodNode.startIndex}`), matching the sibling seenVarDetections dedup a few lines below. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ingestion): end-to-end Django cross-file route extraction (#1836 R10) Adds an integration test that runs runPipelineFromRepo against a Django fixture whose project lives under backend/, asserting the resulting Route graph nodes (/health, /api/items, /api/items/<int:pk>). This exercises the previously-untested main-thread orchestration glue (discovery → parse → extractRoutes → allExtractedRoutes → Route nodes) and, because the project is in a subdirectory, regresses the subdir-discovery fix (R1) — a repo-root-only resolver would discover nothing and emit zero Route nodes. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): anchor Django include() resolution at the project root (#1836 review F1) resolveIncludedFile tried the bare repo-root candidate (app/urls.py) before the project-relative one, so in a monorepo with both a repo-root app/ and a backend/ Django project that also has an app/, include('app.urls') from the backend project resolved to the WRONG service's routes. Probe up-tree from the root urls.py for the nearest manage.py (the Django project root / sys.path entry) and try that-anchored candidate first. Absolute module paths like `app.urls` now resolve to <projectRoot>/app/urls.py unambiguously. When no manage.py is reachable (e.g. unit tests with a urls-only reader) the prior strategy order is preserved. Adds a monorepo wrong-app test. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): drop bogus Django provider source-scan, use graph routes (#1836 review F2) The DJANGO_PATH_PATTERNS / DJANGO_URL_PATTERNS source scan emitted an HTTP provider contract for every path()/re_path()/url() string literal, without checking it was inside urlpatterns, without skipping include() mount points, and without composing the include() prefix across files. For `path('api/', include('app.urls'))` + child `path('items/', view)` it emitted providers for `/api` (a mount, not a route) and `/items` (un-prefixed) — which survived the exact-contract-ID dedup alongside the correct graph route `/api/items`, polluting cross-repo matching with false providers. Remove the Django provider patterns and their scan blocks. Django provider contracts come from the graph Route nodes, which the ingestion route extractor builds with includes already composed (and now correctly, per the other fixes). Python HTTP *consumer* patterns (requests/wrapper) are unaffected. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): match method-agnostic Django providers to any-method consumers (#1836 review F3) Django function views are method-agnostic, so extractDjangoRoutes emits httpMethod '*'. That '*' was dropped by normalizeRouteMethod and then defaulted to GET by the contract extractor, while the matcher only expanded wildcard *consumers* — so a `POST /api/items` consumer never matched the Django provider that was silently narrowed to GET. - routes.ts: preserve '*' as a method-agnostic marker on the Route node, so the contract layer emits a wildcard provider (http::*::path) instead of GET. - matching.ts: make findMatchingKeys symmetric — a specific-method consumer now matches an exact-method provider OR a wildcard (http::*::) provider on the same path, mirroring the existing wildcard-consumer expansion. Co-Authored-By: HuyNguyenDinh <61400397+HuyNguyenDinh@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dinh Huy <huynd86@fpt.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
glier
|
6a571570f2
|
feat(group): Kotlin Spring HTTP consumer extraction + provider parity with Java (#2254)
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(group): Kotlin Spring HTTP consumer extraction + provider parity with Java Brings the Kotlin group/contract HTTP extractor up to parity with Java for inter-service contract detection, and unifies the language-agnostic consumer logic so it is not duplicated. Consumers (new for Kotlin): - @FeignClient interface @(Get|...)Mapping methods are emitted as OpenFeign consumers (a remote call), not providers — previously mis-classified because tree-sitter-kotlin models an interface as a class_declaration. - Spring 6 HTTP Interface @(Get|...)Exchange (with optional class-level @HttpExchange(url) prefix) — added for BOTH Java and Kotlin. - Native OpenFeign @RequestLine, gated to interfaces (Feign proxies are interfaces only), mirroring java.ts's findEnclosingInterface check. Providers (Kotlin parity with java.ts scanSpringProject): - A @(Get|...)Mapping on a non-Feign interface is a route *contract*, not a served route; it is skipped in scan() so the implementing controller is the sole provider (Java drops these implicitly via interface_declaration). - scanProject inherits interface routes onto the implementing class, gated on the class being a @RestController/@Controller (kotlinClassIsController handles both the attached `modifiers` shape and the detached leading-arg-form prefix_expression shape) so non-controller implementers don't emit phantom providers. Shared module: - New spring-consumer-shared.ts holds the language-agnostic primitives (REST_TEMPLATE_/WEB_CLIENT_/EXCHANGE verb maps, joinPath, parseRequestLine, framework + confidence constants); java.ts and kotlin.ts both import it. Array-of-paths (both languages): - Route/Feign/Exchange annotation paths are `String[]`; a multi-element array registers the route under EVERY element. The class/Feign/HttpExchange prefix maps now accumulate all elements (were last-write-wins) and emission cross-products prefixes × method paths, so `@RequestMapping(["/a","/b"])` + `@GetMapping(["/x","/y"])` yields all four contract IDs. Array form is matched via a predicate-free alternation over Kotlin `collection_literal` / Java `element_value_array_initializer`. Tests: comprehensive Java + Kotlin cases incl. consumer-vs-provider classification, @*Exchange, @RequestLine (interface-only + plain-interface), interface-based controller inheritance, non-controller negative case, detached @RestController, single- and multi-element array paths (method-level and class-prefix cross-product). 109 http-route + group tests pass; tsc/eslint/ prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): apply @RequestMapping prefix to Kotlin @RequestLine consumers (#2254 P2) A @RequestLine method on an interface with a class-level @RequestMapping prefix but no @FeignClient(path) dropped the prefix in Kotlin while Java applied it (java.ts merges the fallback into feignPrefixByInterfaceId). Mirror the feignPrefixByClassId ?? prefixByClassId ?? [''] chain already used by the @GetMapping-in-Feign path. Adds Kotlin twins for the @RequestMapping-prefix and @FeignClient(path)-wins cases. * fix(group): accept named-arg Kotlin @RequestLine(value=...) (#2254 P2) The positional pattern's '.' anchor only matched @RequestLine("VERB /x"), silently dropping the named @RequestLine(value = "VERB /x") form that java.ts accepts. Add a dedicated named pattern constrained to #eq? @key "value" (Java parity: non-value keys stay dropped). Adds Kotlin twins for the named-value and non-value-key cases. * fix(group): resolve Kotlin FQN annotations/supertypes by trailing segment (#2254) A fully-qualified @org…RestController / supertype : a.b.Api parses to a user_type with one type_identifier per dotted segment; kotlinAnnotationName and collectKotlinSupertypes took the FIRST ("org"/"a"), so FQN controllers were not recognised and FQN supertypes never matched their interface. Take the trailing segment. Adds FQN controller + FQN supertype inheritance twins. * refactor(group): remove dead prefix_expression branch in kotlinClassIsController (#2254) AST probe (bare and realistic package+constructor forms) confirms the arg-form @RestController("bean") attaches under the class `modifiers` as an annotation/constructor_invocation, caught by the modifiers loop — the prefix_expression sibling branch was unreachable. Remove it and correct the false grammar comments (source + the arg-form test). The existing arg-form test stays green via the modifiers branch, confirming no behavior change. * feat(group): support Kotlin arrayOf(...) annotation arrays (#2254 P3) arrayOf("/a","/b") (the explicit String[] form) parses to a call_expression, not a string_literal/collection_literal, so it was missed across all five annotation-array families. Add dedicated arrayOf query patterns (positional + named) per family via a shared arrayOfArg fragment — kept out of the existing [(string_literal) (collection_literal …)] alternation to avoid the tree-sitter 0.21.x predicate-bucket hazard. Verified one match per element (multi-element accumulates) with buildPath/produces/empty anti-overreach. * feat(group): detect WebClient long-form in Java for Kotlin parity (#2254 P3) Java deliberately deferred webClient.method(HttpMethod.X).uri(...); the Kotlin plugin proves a single structural query suffices (same field-access shape as REST_TEMPLATE_EXCHANGE). Add WEB_CLIENT_LONG_FORM_PATTERNS + scan loop so .java and .kt detect it identically. Move WEB_CLIENT_LONG_VERB_RE to the shared module (single source for both). Flip the now-obsolete java :1741 negative test to positive (verbs + no-double-emit) and add a Java var-verb anti-overreach twin. * refactor(group): share pushPrefix between java.ts and kotlin.ts (#2254) The de-duping prefix accumulator was duplicated as kotlin.ts pushKotlinPrefix and a java.ts closure. Hoist a single export pushPrefix into spring-consumer-shared.ts; both plugins import it. No behavior change. * test(group): add Kotlin interface-inheritance boundary twins (#2254) Twins for the Java inheritance-boundary cases that had no Kotlin counterpart: shared-leading-segment combine, prefix-less method overlap, ambiguous duplicate-interface-name suppression, plus a positive multi-interface implementer. These pin Kotlin's scanProject behavior before U8 extracts the shared inheritance algorithm. * refactor(group): share the Spring interface-inheritance scanProject algorithm (#2254) scanKotlinProject and scanSpringProject were ~80-line near-duplicates over structurally identical type records. Extract scanSpringInheritanceProject + SharedSpringType into spring-consumer-shared.ts; collapse KotlinTypeInfo and SpringTypeInfo into the shared type; both plugins' scanProject become thin collect-and-delegate wrappers. The ownerPrefix-carrying intermediate is owned by the shared function. Behavior-preserving — Java and Kotlin inheritance suites (incl. the new Kotlin boundary twins) byte-identical; tsc clean. * test(group): close Kotlin↔Java consumer test-parity gaps + assert confidence (#2254) Add Kotlin twins for Java-tested consumer scenarios with no Kotlin coverage: @RequestLine query-strip, mixed @RequestLine+@GetMapping, malformed-value rejection, and @FeignClient(path)-wins-when-@RequestMapping-first. Add the Java dual-role twin (interface as consumer + implementing controller as provider). Add two-sided provider confidence (0.8) assertions on the canonical Java and Kotlin interface-inheritance tests. * docs(group): document Java FQN route-annotation limitation + pin it (#2254) Per KTD6, the Java FQN route-annotation gap is documentation-first: the gap is route-string-only (FQN controllers are already recognised via hasAnnotation) and FQN-written annotations are vanishingly rare. Document the asymmetry with Kotlin in JAVA_ROUTE_ANNOTATION_PATTERNS and pin current behavior with an anti-overreach test. The scoped_identifier query change is deferred to avoid re-keying existing contracts via the predicate-bucket hazard. * test(group): add Java↔Kotlin contract set-equality parity harness (#2254) Independent per-side twins can both pass while the emitted contract SETS differ. Add a table-driven harness over the parity-critical families (@RequestLine prefix-fallback, named @RequestLine, @FeignClient(path)+@GetMapping, @HttpExchange+@GetExchange, WebClient long-form, interface inheritance) that runs matched .java/.kt fixtures through both plugins and asserts the full projected contract set (role+contractId+framework+confidence) is equal across languages AND equal to the expected set — the durable guard for the byte-identical goal. Gated on kotlinConsumerAvailable. * style(group): apply prettier formatting to #2254 changes --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
henry201605
|
a691dcb320
|
feat(routes): persist HTTP method on Route nodes (#2138 part 1/2) (#2234)
* feat(routes): persist HTTP method on Route nodes Part 1 of 2 for issue #2138 (skip redundant HTTP provider source-scan). The ingestion routes phase already knows each route's HTTP verb — `ExtractedRoute.httpMethod` (Spring/Laravel framework routes) and `ExtractedDecoratorRoute.httpMethod` (decorator routes) — but dropped it when creating the Route graph node. As a result `HttpRouteExtractor`'s graph-assisted path could not recover the verb for `framework-route` sources (whose edge `reason` is undecodable by `methodFromRouteReason`) and had to fall back to re-scanning the handler source. Changes: - routes phase: carry `httpMethod` into `RouteEntry` and persist it as `Route.method` (filesystem-derived Next.js/Expo/PHP routes have no structural verb, so they stay method-less). - HttpRouteExtractor: HANDLES_ROUTE query now returns `route.method`; `extractProvidersGraph` prefers it and falls back to the edge reason for older indexes / method-less routes (fail-open, fully backward compatible). - tests: graph-method precedence, multi-verb handler disambiguation via the persisted verb, case normalization, and old-index fallback. This change is intentionally NOT a performance optimization on its own: the graph path still parses handler files to recover the handler *name*. Eliminating that parse (and thus the redundant source-scan #2138 targets) requires linking HANDLES_ROUTE to the handler symbol, which lands in Part 2. This PR is the data-completeness groundwork for that. Refs #2138 * test: account for new Route.method in blade route-registry assertion The routes phase now persists httpMethod onto RouteEntry/Route nodes, so the strict toEqual on the framework-route registry entry must include the new method field. * fix(routes): persist Route.method end-to-end + real-lbug round-trip test Addresses review on #2234 (magyargergo + tri-review): the prior commit read `route.method` in HANDLES_ROUTE_QUERY but never added the column to the schema/persistence path, so against a real LadybugDB the query failed to bind (`Cannot find property method for r.`) and the `catch { return [] }` silently swallowed it — regressing the graph-assisted HTTP provider path. - schema: add `method STRING` to ROUTE_SCHEMA. - csv-generator: write `method` in the Route CSV row (header + row, column order aligned with the COPY statement). - lbug-adapter: add `method` to getCopyQuery('Route'). - routes phase: normalizeRouteMethod() canonicalizes the verb to upper-case and skips non-verbs — Laravel resource/apiResource carry httpMethod values like `resource`/`apiResource`, which must not land a junk method. - http-route-extractor: log at debug when the HANDLES_ROUTE / FETCHES graph query throws, so a total graph-provider outage is observable instead of silently swallowed. Export HANDLES_ROUTE_QUERY for the round-trip test. - tests: add a real-lbug round-trip (graph -> CSV -> COPY -> HANDLES_ROUTE_QUERY) asserting the verb persists and reads back; update the blade registry assertion for the normalized (upper-case) method. Refs #2138 * fix(csv): coerce Route.method to string for escapeCSVField typecheck node.properties.method is typed unknown (not a declared property), so `x || ''` stayed unknown and failed tsc against escapeCSVField's string|number param. Coerce explicitly with String(... ?? ''). * test(bench): regenerate emit-persistence fingerprint for Route.method column Adding the method column to route.csv changes the byte-identity fingerprint of the emit-persistence benchmark (the synthetic graph's route.csv header now includes 'method'). scaling_ratio unchanged (~0.9, linear); this is the documented regenerate-on-legitimate-emit-change path. Streaming baseline (BasicBlock/PDG) is unaffected. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
azizur100389
|
ff0124e067
|
feat(cpp): parse CUDA source extensions (#2213)
* feat(cpp): parse CUDA source extensions * test(cpp): characterize CUDA parser limitations --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
fb068a9480
|
fix(group): pin repos during sync so large groups resolve cross-links (#2191)
* fix(lbug): pin repos to exempt them from automatic pool eviction [#2189] Add a pinnedRepos set and pinRepo/unpinRepo to the LadybugDB pool adapter. evictLRU and the idle-timeout sweep skip pinned repos; closeOne clears the pin on teardown so explicit close always wins and pins never leak across operations. Behavior is byte-identical when nothing is pinned. Bounded multi-repo callers (group sync) can now keep more than MAX_POOL_SIZE repos resident through deferred cross-repo resolution. * fix(group): pin repos during sync so >MAX_POOL_SIZE groups resolve [#2189] syncGroup now pins each repo immediately after initLbug and releases the pin (unpin then close) in the finally. This keeps every group member resident through the deferred manifest/workspace resolution that runs after the init loop, so cross-links anchor to real graph symbols instead of falling back to synthetic UIDs when a group has more than MAX_POOL_SIZE repos. Release is unpin-before-close plus closeOne's own pin-clear, so pins never leak across syncs in the long-lived MCP server even on error. * style(test): apply prettier formatting to #2189 test files * fix(review): apply autofix feedback Clarify the pinRepo docstring: the pin does not survive teardown (closeOne clears it) and the repoId must match the key passed to initLbug. Addresses a code-review finding that the prior 'or later holds' wording contradicted closeOne's unconditional pin-clear. * refactor(lbug): reference-count pool pins so overlapping holders are safe [#2189] Change pinnedRepos from Set<string> to Map<string,number>. pinRepo increments the lease count; unpinRepo decrements and deletes the key at 0 (flooring at zero, unknown-id no-op). evictLRU, the idle sweep, and closeOne are transparent to the swap (has()/delete() keep their semantics: skip while count>=1, force-clear on teardown). A boolean Set could not represent two simultaneous holders, so the first release wrongly cleared a pin another holder still needed — the concurrent overlapping group_sync teardown race from the PR #2191 review (Finding 1). Reference counts let two windows of one sync, or two concurrent syncs sharing a repo, coexist safely: the repo stays exempt until the last lease releases. * refactor(lbug): pinRepo returns a leak-proof release disposer [#2189] pinRepo now returns a release() disposer (mirroring addPoolCloseListener) that releases its own lease exactly once — a double-call is a guarded no-op, so it can never over-decrement a sibling holder's reference count. Callers can use the leak-proof pattern `const release = pinRepo(id); try { … } finally { release(); }`. unpinRepo stays exported for explicit pairing. Addresses the PR #2191 review's P3: the exported pin primitive had no built-in pairing, so a caller that forgot to unpin would disable eviction for a repo permanently. * refactor(group): windowed manifest resolution bounds sync pool residency [#2189] Replace whole-sync pinning with windowed deferred resolution. The init loop extracts contracts without pinning (repos evict naturally); manifest links are pre-sorted and partitioned into windows whose referenced in-group repos number <= getMaxResidentRepos(), and each window re-inits + leases only its own repos, resolves, then RELEASES the leases (not closeLbug — released repos stay evictable for the LRU, which avoids stomping a concurrent MCP reader). Peak per-sync pool residency is now bounded by getMaxResidentRepos() distinct repos regardless of group size, removing the unbounded-mmap crash risk the PR #2191 review flagged (Finding 3) — without a new magic-number threshold (it reuses MAX_POOL_SIZE via an intent-named accessor). #2189 stays fixed: each window resolves against live, freshly-leased pools, so cross-links anchor to real graph symbols. partitionManifestWindows is a pure, unit-tested function (every link in exactly one window — the contract-dedup invariant). New sync-windowed-resolution.test.ts asserts the partition bound and, through the real pool, that concurrently-open Databases never exceed the resident cap for a group larger than it. Rewrote the sync.test.ts pinning block (init loop no longer pins; per-window lease/release; release-not-close). |
||
|
azizur100389
|
9a40af3d79
|
fix(java): dedupe inherited RequestMapping prefixes (#2057) | ||
|
henry201605
|
a93ecee068
|
fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) (#1917)
* fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) PR #1904 gated @RequestLine consumer extraction on the enclosing interface also carrying @FeignClient. That guard is wrong: @RequestLine is a core feign.* annotation used with Feign.builder(), while @FeignClient is the Spring Cloud variant that uses Spring MVC annotations (@GetMapping etc.) — the two are effectively mutually exclusive. Requiring @FeignClient therefore excluded the annotation's primary, canonical usage, so the feature recognized nothing on real core-Feign client interfaces. Fix: drop the @FeignClient requirement for @RequestLine. The match still requires an enclosing interface (Feign proxies are always interfaces), and the `RequestLine` annotation name is itself a strong, framework-specific signal, so false-positive risk stays low. A @FeignClient(path=...) prefix is still applied when present. The @(Get|Post|...)Mapping consumer path keeps its @FeignClient requirement: those annotations are generic Spring MVC and need the Feign context to be disambiguated from provider routes. Verification (real-world, not just synthetic fixtures): - A real client-jar consumer (BigModeClientService.java: a plain interface with 12 @RequestLine methods, no @FeignClient) now yields 12 openfeign consumer contracts; it yielded 0 before this change. - End-to-end `group sync` over that consumer repo + its FastAPI provider repo (with zero hand-written links) produces 12 exact cross-links (confidence 1.0), Java @RequestLine consumer → Python route provider. - The prior test that asserted the wrong behavior ("ignores @RequestLine on interfaces without @FeignClient") is reversed into a realistic core-Feign fixture. - Full test/unit/group suite (579) green; tsc and prettier clean. * test(group): add negative cases for relaxed @RequestLine matcher Per review on #1917 — guard the no-@FeignClient relaxation with explicit negative tests: malformed @RequestLine values (no verb / no leading-slash path / unknown verb) yield no contract, and @RequestLine on a concrete class method (not an interface) is not emitted as a consumer. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
f18ff521fc
|
fix(group): stop Node gRPC loadPackageDefinition gate from matching every member call (#1916)
LOAD_PACKAGE_DEFINITION_SPEC matched `loadPackageDefinition` via a single `function: [ (identifier) @fn (#eq?) (member_expression property:(property_identifier) @fn (#eq?)) ]` alternation. Under the pinned tree-sitter@0.21.1 binding a top-level alternation whose branches reuse one capture name collapses to a single pattern with a shared predicate bucket: the member-expression branch's `@fn` is left unbound and its `#eq?` is never enforced, so that branch matches EVERY `obj.method(...)` call (`console.log(...)`, `logger.info(...)`, …). Since virtually every TS/JS file has some member call, the `usesLoadPackage` gate was effectively always-open and `new pkg.<Capitalized>Service(...)` was emitted as a spurious gRPC consumer — the exact false positive the gate was added to prevent. Split the spec into two single-branch PatternSpecs; each compiles to its own Parser.Query with an independent predicate bucket where the `#eq?` is enforced correctly. `runCompiledPatterns` concatenates their matches, so the `.length > 0` gate is unchanged. `mk` now accepts a spec or a spec array. Adds test_extract_ts_qualified_ctor_without_loadPackageDefinition_is_ignored, a negative regression test verified to FAIL on the pre-fix code and PASS with the fix: a file with no loadPackageDefinition but an unrelated member call + `new authProto.auth.v1.AuthService(...)` must emit no consumer. grpc-extractor suite 65/65; tsc + prettier + pre-commit hook clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
henry201605
|
5d710413d7
|
feat(group): extract OpenFeign @RequestLine consumer contracts (#1904)
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(group): extract OpenFeign @RequestLine consumer contracts
Adds Java HTTP plugin support for the native OpenFeign annotation
`@RequestLine("METHOD /path")`. Previously only `@FeignClient` interfaces
using Spring MVC method annotations (`@GetMapping` etc.) were detected;
the native annotation form — required by Feign Builder users and
non-Spring Feign deployments — was silently ignored.
Implementation:
- New `FEIGN_REQUEST_LINE_PATTERNS` covers both positional and named-arg
(`value =`) forms.
- New `parseRequestLine()` parses the verb+path string and drops any
query string (consistent with how RestTemplate/WebClient consumers
handle inline literal URLs).
- The enclosing interface MUST carry `@FeignClient`; otherwise the
detection is dropped to avoid false positives from same-named
annotations in unrelated libraries.
- Reuses the existing `feignPrefixByInterfaceId` map so
`@FeignClient(path=)` and `@RequestMapping` interface prefixes apply
uniformly across both Spring MVC and `@RequestLine` methods.
- Confidence 0.75 — slightly higher than the 0.7 used for Spring MVC
annotations because the verb is a string-literal value, not inferred
from the annotation name (less ambiguous).
Six new unit tests cover: basic two-method extraction; `@FeignClient(path=)`
prefix joining; query-string stripping; rejection of `@RequestLine` on
non-Feign interfaces; mixing with `@GetMapping` on the same interface;
named-argument form (`value = "..."`).
Verification: `npx tsc --noEmit`, full `test/unit/group` (31 files / 563
tests), `http-route-extractor.test.ts` (83/83 incl. 6 new), `prettier
--check` and `eslint` on touched files all pass.
* refactor(group): collapse @RequestLine positional + named-arg into one query
Per @magyargergo's review on PR #1904 — uses tree-sitter alternation
`[(...) (...)]` so the positional and named-argument forms of the
`@RequestLine` annotation are matched by a single compiled query and
invoked through one `runCompiledPatterns` pass instead of two.
* refactor(group): drop framework prefixes from java http pattern constant names
Per review feedback on #1904 — renames the four route-mapper pattern
constants to framework-agnostic names (the per-constant comments already
document which framework each targets):
SPRING_TYPE_PREFIX_PATTERNS -> TYPE_PREFIX_PATTERNS
FEIGN_REQUEST_LINE_PATTERNS -> REQUEST_LINE_PATTERNS
FEIGN_INTERFACE_PREFIX_PATTERNS -> INTERFACE_PREFIX_PATTERNS
SPRING_METHOD_ROUTE_PATTERNS -> METHOD_ROUTE_PATTERNS
* refactor(group): collapse Java route-mapper annotations into one query
Merge the four annotation pattern bundles (Spring @RequestMapping type
prefix, @FeignClient(path) prefix, @(Get|Post|Put|Delete|Patch)Mapping
method routes and native @RequestLine) into a single
JAVA_ROUTE_ANNOTATION_PATTERNS query, read by scanRouteAnnotations() in
exactly one matches() pass per file. Variants are tagged by branch-local
captures and discriminated in JS (METHOD_ANNOTATION_TO_HTTP,
isRouteMemberKey), per review feedback. This drops the per-file annotation
passes from 4->1 in scan() and 2->1 in collectSpringTypes(), and removes
the interface-@RequestMapping / @FeignClient prefix redundancy.
Verb and path/value key filtering stay in JS rather than in-query: under
the pinned tree-sitter 0.21.1 binding a top-level [...] alternation
compiles to one pattern whose text predicates share a single bucket keyed
by capture name. A #match? against a capture absent from the matched
branch evaluates FALSE and silently drops every sibling-branch match,
whereas #eq? against an absent capture is vacuously true. So only fixed
annotation names use in-query #eq? (on branch-local captures); the
variable verb name and member key carry no in-query predicate.
Behaviour is unchanged for all compilable Java; existing http-route tests
(93) and the full group suite remain green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(group): make Java route-annotation query generic, match name in loop
Collapse JAVA_ROUTE_ANNOTATION_PATTERNS from 9 annotation-name-pinned
branches to 6 generic structural branches (class/interface/method x
positional/named) that capture the annotation name (@ann), declaration
(@node), argument (@value) and member key (@key) generically. The query
now carries NO #eq?/#match? predicates at all; scanRouteAnnotations reads
@ann.text and @node.type in its for-loop to decide what each match means
(RequestMapping prefix, FeignClient(path) prefix, @(Get|...)Mapping route,
or @RequestLine), ignoring unrecognised annotations.
This makes the query framework-agnostic and extensible — adding a new
route annotation is a change to the loop and the lookup maps, not the
query — and removes the last tree-sitter-0.21.1 shared-predicate-bucket
footgun, since a predicate-free alternation cannot drop sibling branches.
Behaviour is byte-identical: 93 targeted http-route tests and the full
569-test group suite stay green; tsc and prettier clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(group): pin newly-reachable Java route-annotation JS branches; clarify invariants
Code-review follow-up to the route-annotation query consolidation. No
behaviour change to the extractor:
- Add two regression tests for branches the generic predicate-free query
made reachable in scanRouteAnnotations: (1) a @RequestLine whose named
argument is not `value` must be dropped (the in-query `#eq? @key "value"`
guard now lives in JS); (2) @FeignClient(path) must win over @RequestMapping
even when @RequestMapping is the first annotation in source order, covering
the deferred interfaceRequestMappingPrefixes apply (the existing precedence
test only covered @FeignClient-first).
- Document two invariants flagged in review: why prefixByTypeId and
feignPrefixByInterfaceId intentionally diverge for the same interface node
(Spring provider vs OpenFeign consumer prefix), and that the query's
single-string-argument shape excludes array-valued annotations.
http-route-extractor + multi-verb suites: 95/95 (was 93); tsc + prettier clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
henry201605
|
4bc8622642
|
fix(group): derive grpc consumer FQN from java imports for client-jar consumers (#1889)
* fix(group): derive grpc consumer FQN from java imports so client-jar consumers don't fall back to short names
Java gRPC microservices commonly follow the "client-jar" pattern: the
service owner publishes a pre-compiled stub jar to a Maven repository
and consumer repos depend on the jar instead of carrying the
originating `.proto` files. gRPC's official Java quickstart, Alibaba
HSF, ByteDance KiteX-Java and google-cloud-java all document this
shape.
Before this commit, `GrpcExtractor` resolved a fully-qualified
contract id (`grpc::<package>.<Service>/*`) only when the consumer
repo also carried a matching `.proto`. Client-jar consumers had no
proto, so they fell back to a short-name contract id
(`grpc::<Service>/*`) that never matched the provider's contract id.
Cross-repo grpc cross-link counts dropped to zero on every realistic
Java microservice group — including all of crsdp's `crsdp-backend →
unipus_cloud_framework` connections.
Fix: derive the proto package directly from each consumer file's
`import <pkg>.<XxxGrpc>;` statement. The package from the import is
exactly the proto package, so the contract id matches the provider's
verbatim — no `.proto` lookup needed in the consumer repo.
Implementation
--------------
* `grpc-patterns/types.ts` — `GrpcDetection` gains an optional
`protoPackage` field. Plugins set it when the package can be
derived from the source file alone.
* `grpc-patterns/java.ts` — adds `GRPC_CLASS_IMPORT_PATTERNS`, a
tree-sitter query that captures every
`import_declaration > scoped_identifier { scope, name }` pair where
the imported name ends in `Grpc`. `import static …` and
`import w.x.*;` are excluded by tree-sitter shape: the `name:` field
is only present on the non-static, non-wildcard form. The plugin
builds a per-file `XxxGrpc → fullPackage` map and tags every
provider / consumer detection it emits.
* `grpc-extractor.ts` — `detectionToContract()` now resolves the
contract id in three steps:
1. detection-supplied `protoPackage` wins (skips the proto map
entirely so an unrelated same-name service in the consumer
repo can't blur the FQN);
2. otherwise consult the legacy per-repo proto map;
3. otherwise fall back to a short-name contract id, preserving
pre-fix behaviour.
Confidence stays at the "with proto" tier when the import path
resolves: an import statement in real source is at least as
authoritative as a per-repo proto map.
Same-short-name disambiguation
-------------------------------
The motivating case `unipus_cloud_framework` defines two distinct
`ContentRpcService` services in different proto packages
(`cn.unipus.ucf.api.proto.client.service.ContentRpcService` vs
`cn.unipus.ucf.admin.proto.client.service.ContentRpcService`). Two
consumer files importing the two flavours now emit two distinct FQNs;
neither could be told apart from the other under the legacy short-
name fallback.
Out of scope
------------
`import w.x.*;` (wildcard service imports) are left to the legacy
short-name fallback. Wildcard imports are discouraged by Google's
Java style guide and IntelliJ's defaults, and resolving them
unambiguously would require either group-level proto-package
catalogs or per-class disambiguation, both of which are larger
follow-ups. This commit only changes behaviour for the dominant
specific-import case.
Tests
-----
`test/unit/group/grpc-extractor.test.ts` adds a new "Java client-jar
consumer (import-derived FQN)" describe block with 9 cases covering
both the happy paths (consumer/provider FQN derivation, same-short-
name disambiguation, import-vs-local-proto precedence) and the
regression-protection paths (no import + no detection emitted, static
imports / wildcards ignored, mixed-file repos preserved).
End-to-end verification
-----------------------
Ran the patched cli on the real `crsdp-backend` (consumer, no
`.proto`) and `unipus_cloud_framework` (provider, has `.proto`)
repos. Synced as a two-repo group, every `XxxGrpc` referenced via a
specific import in `UcfAdminGrpcClientService.java` produced an FQN
contract id that exact-matched the provider repo's FQN — 9 grpc
cross-links surfaced where there were 0 before.
Verification
------------
* `npx tsc --noEmit`: pass
* `npx tsc` (dist rebuild): pass
* `test/unit/group/grpc-extractor.test.ts`: 60/60 pass (51 existing
+ 9 new)
* `test/unit/group/`: 30 files / 545 tests all green
* `npx prettier --check` on touched files: pass
* `npx eslint` on touched src files: 0 errors / 0 warnings
* fix(group): handle option java_package and proto-map disagreement in grpc detection
Addresses Claude bot review on PR #1889:
- Finding 1: parse `option java_package` when building proto context;
add a reverse index so an import-derived package can be translated
back to the proto package.
- Finding 2: when same-repo proto map has the service, use the proto
package; warn and record `meta.importPackage` if the import disagrees.
- Finding 3: add an end-to-end wildcard match test (provider+consumer
fixture, runs `buildProviderIndex`+`runWildcardMatch`).
Client-jar consumer + diverging `java_package` (no local proto)
remains a known limitation; pinned by a dedicated test.
---------
Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
|
||
|
henry201605
|
2f15c1ece1
|
feat(group): add Kotlin Spring WebClient long-form HTTP consumer extraction (#1884)
* feat(group): add Kotlin Spring WebClient long-form HTTP consumer extraction Follow-up to #1855. Extends `kotlin.ts` with the long-form WebClient fluent chain that #1855 explicitly deferred: webClient.method(HttpMethod.GET).uri("/x").retrieve().awaitBody<T>() This pattern remains common in Kotlin Spring 4 → 5 migrations and in codebases that prefer the fluent verb-as-enum style. The short form (`webClient.get().uri("/x")`) was already supported in #1855. Approach: - Single deeper tree-sitter query (`WEB_CLIENT_LONG_PATTERNS`) that matches the full chain structurally — both `.method(HttpMethod.X)` and `.uri("...")` in one pattern. Verb is captured as the `simple_identifier` of the `HttpMethod.X` field access. - Verb is whitelisted to GET/POST/PUT/DELETE/PATCH (consistent with the short-form's `WEB_CLIENT_SHORT_TO_HTTP` map). - Receiver constraint `(#eq? @obj "webClient")` mirrors the short form and Java plugin heuristic. Out of scope (intentional): - Variable-bound verbs: `val verb = HttpMethod.PATCH; webClient.method(verb)...` Source-scan can't follow the binding without graph context. Pinned by an anti-overreach test. - HEAD/OPTIONS/TRACE: not in `WEB_CLIENT_SHORT_TO_HTTP` either — keeps polyglot symmetry with java.ts and the short form. Tests: 4 new cases under `consumer extraction — fetch patterns`, gated by tree-sitter-kotlin grammar availability. positive (3) - long form GET - long form POST / PUT / DELETE / PATCH (4 verbs in 1 fixture) - no double-emit pin (long-form chain produces exactly one consumer, not one from each query) anti-regression (1) - variable-bound verb does NOT match (graph-aware concern) The previous `'does NOT match Kotlin WebClient long form (deferred to follow-up)'` test from #1855 is replaced by these — the deferred state is now resolved. Reverse-validated: temporarily disabling the long-form emit makes exactly the 3 positive tests fail; the variable-bound-verb anti- regression test continues to pass (it pins behavior independent of the emit being on or off). Local validation: - test/unit/group/http-route-extractor.test.ts: 66/66 ✅ - test/unit/group: 546/546 ✅ - npx prettier --check (changed files): clean ✅ * test(group): address Claude review findings F1 and F2 on PR #1884 Two minor follow-ups from the production-readiness review: F1 — Stale block comment at the top of the Kotlin consumer suite (was: "Three consumer flavors covered here ... long-form deferred to a follow-up"). Updated to "Four consumer flavors" and removed the deferred sentence — the deferral is resolved by this PR. The kotlin.ts file header was already updated; this brings the test file comment in sync. Per DoD §2.3 (no stale comments). F2 — Replaced `expect(wcConsumers.length).toBeGreaterThanOrEqual(4)` with `expect(wcConsumers).toHaveLength(4)` in the multi-verb test. The fixture is fully deterministic — exactly 4 long-form calls, no other consumer types — so an exact count assertion is the right shape per DoD §2.7 ("use toBe / toEqual for exact expectations"). Added a comment explaining what the assertion catches that the existing per-verb toBeDefined() checks would miss (accidental 5th consumer from a duplicate query firing or a regressed receiver constraint). F3 (HEAD/OPTIONS/TRACE negative test) is intentionally not added in this PR — same precedent as #1855 where HEAD/OPTIONS/TRACE on the short form are also implicitly excluded without a pinning test. Happy to add one in a separate PR if maintainers want explicit pinning across both forms. F4 (CI on pre-merge SHA) is the maintainer's call — the merge from main is theirs to re-trigger CI on. The merge brings only Java consumer changes (PR #1872) and Go provider changes (PR #1886), both in entirely separate files from this PR's Kotlin work. Local validation: - test/unit/group/http-route-extractor.test.ts: 73/73 ✅ (66 from this PR pre-merge + 7 from PR #1872 merged via main) - npx prettier --check (changed files): clean ✅ * refactor(group): hoist Kotlin WebClient long-form verb regex to module scope Address @magyargergo's review request on PR #1884: > Can you please extract the regexp from the for loop? 🙏 (kotlin.ts:510) Compiles the verb whitelist `^(GET|POST|PUT|DELETE|PATCH)$` once at module load instead of every iteration of the long-form scan loop. Mirrors the placement and JSDoc style of the sibling `WEB_CLIENT_SHORT_TO_HTTP` constant. Behavior is unchanged — same verb whitelist, same exclusion of HEAD/OPTIONS/TRACE for symmetry with the short form. The 4 itKotlinConsumer long-form tests added in this PR continue to pass, and the variable-bound-verb anti-overreach test continues to pin the deliberate non-match. Local validation: - test/unit/group/http-route-extractor.test.ts: 77/77 ✅ - test/unit/group: 557/557 ✅ - npx prettier --check (changed file): clean ✅ --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
JaysonAlbert
|
7dae4fcc41
|
fix(group): attribute Spring interface routes to controllers (#1743)
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): attribute Spring interface routes to controllers * test(group): normalize Spring route fixture paths --------- Co-authored-by: gfwangjie <gfwangjie@gf.com.cn> |
||
|
MyShining
|
7b38b8aae2
|
feat(java): add HTTP consumer contract extraction (#1872) |