scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-10 03:27:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix(group): fold Kotlin route constants on Windows and when they fold to ""

Browse source 
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>
This commit is contained in:
Borozdenets Ilya 2026-08-28 11:08:29 +03:00
parent 8fd58b490d
commit 7ccc344ec4
4 changed files with 227 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

43
gitnexus/src/core/group/extractors/http-patterns/kotlin.ts
View file

@ -173,6 +173,31 @@ const FOLDABLE_PATH_EXPRESSIONS: ReadonlySet<string> = new Set([
'additive_expression',
]);
/**
* Repo-relative path in the POSIX form the Kotlin constant map is keyed by.
*
* The orchestrator's file list comes from glob v13, which has no `posix: true`
* option and joins with the platform separator, so on Windows `prepareRepo`
* receives `src\main\kotlin\com\example\ApiPaths.kt` and `scan` receives the
* same for `fileRel`. `resolveKotlinImport` turns an import specifier into
* `com/example/ApiPaths.kt` and asks whether a key ENDS WITH it — a test no
* backslashed key can pass. Left unnormalized, every cross-file constant fold
* returns null on Windows and on Windows only: the pre-pass still runs, the
* context is still built, and the feature is simply, silently absent. The unit
* fixtures build POSIX keys by hand, so CI cannot see it.
*
* Normalizing at this boundary — write side (the map keys below) and read side
* (`fileRel`) — is the same fix `node.ts` (`normalizeRel`) and `python.ts`
* (`fileShortKey` / `fileLongKey`) already apply for the same reason, and it is
* the only coherent place: the resolver returns the key it matched, so
* normalizing inside it would hand back a value that misses in a map nobody
* normalized. `readFile` still receives the ORIGINAL `rel`, since the filesystem
* wants the platform's own spelling.
*/
function normalizeRel(rel: string): string {
return rel.replace(/\\/g, '/').replace(/^\.\//, '');
}
/**
* The path expression carried by one route-annotation argument, or null when the
* argument does not designate a path.
@ -1258,7 +1283,8 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
if (!tree) continue;
const mc = extractKotlinModuleConstants(tree);
if (mc.literals.size > 0 || mc.exprs.size > 0 || mc.imports.size > 0) {
constants.set(rel, mc);
// POSIX key (see `normalizeRel`); `readFile` above got the raw `rel`.
constants.set(normalizeRel(rel), mc);
}
} catch {
// Per-file resilience: one unreadable/oversized/ill-formed file must
@ -1272,6 +1298,11 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
const out: HttpDetection[] = [];
const kotlinCtx = repoContext as { constants: RepoConstants } | undefined;
// Read side of the POSIX keying (see `normalizeRel`): the map `prepareRepo`
// built is keyed by normalized path, so every lookup and every fold entry
// point below uses `fileKey`, never the raw `fileRel`.
const fileKey = fileRel === undefined ? undefined : normalizeRel(fileRel);
// Lazy per-file constants view. `prepareRepo` only indexes constant-
// DEFINING files, so an importing controller is absent from that map.
// When a route actually references a constant, extract THIS file's import
@ -1282,13 +1313,13 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
const getFoldConstants = (): RepoConstants | undefined => {
if (foldConstants !== undefined) return foldConstants;
foldConstants = kotlinCtx?.constants;
if (!kotlinCtx?.constants || !fileRel) return foldConstants;
if (kotlinCtx.constants.has(fileRel)) return foldConstants;
if (!kotlinCtx?.constants || !fileKey) return foldConstants;
if (kotlinCtx.constants.has(fileKey)) return foldConstants;
try {
const mc = extractKotlinModuleConstants(tree);
if (mc.imports.size > 0) {
const merged = new Map(kotlinCtx.constants);
merged.set(fileRel, mc);
merged.set(fileKey, mc);
foldConstants = merged;
}
} catch {
@ -1377,12 +1408,12 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
if (!expr || !FOLDABLE_PATH_EXPRESSIONS.has(expr.type)) continue;
// No repo context (context-less fallback scanning) means no constant map
// and therefore no honest answer — skip rather than guess a path.
if (!fileRel) continue;
if (!fileKey) continue;
const constants = getFoldConstants();
if (!constants) continue;
const operands = parseKotlinConstOperands(expr);
if (operands === null) continue;
const rawPath = foldKotlinOperands(fileRel, operands, constants);
const rawPath = foldKotlinOperands(fileKey, operands, constants);
if (rawPath === null) continue;
methodRoutes.push({
httpMethod,

107
gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts
View file

@ -1,13 +1,31 @@
/**
* Kotlin binding for the language-agnostic constant resolver (#2391 core).
*
* Supplies the two Kotlin-specific pieces the shared fold in
* `constant-resolver.ts` needs — {@link resolveKotlinImport} (import specifier →
* file, honoring JVM package rules) and {@link extractKotlinModuleConstants}
* (tree → {@link ModuleConstants}) — plus a pre-bound
* {@link resolveKotlinConstant} wrapper so callers stay language-oblivious. The
* reusable fold, the cycle guard, and the depth cap all live in the agnostic
* core.
* Supplies the two Kotlin-specific pieces — {@link resolveKotlinImport} (import
* specifier → file, honoring JVM package rules) and
* {@link extractKotlinModuleConstants} (tree → {@link ModuleConstants}) — plus
* the folding entry points {@link resolveKotlinConstant} and
* {@link foldKotlinOperands}, so callers stay language-oblivious.
*
* WHAT IS ACTUALLY SHARED WITH THE AGNOSTIC CORE. One value —
* {@link MAX_FOLD_LENGTH} — and five types. The core's own `resolveConstant` /
* `resolveOperands` are NOT called: the fold state machine below (cycle guard,
* success memo, depth caps, operand concatenation — roughly 200 of this file's
* lines) is a local fork, close enough to `java-const-resolver.ts`'s already
* forked copy that the two read as the same code with the language name
* swapped.
*
* That fork is a consequence, not an oversight. The core keys its maps by
* SIMPLE name, and a Kotlin operand can be a QUALIFIED reference at any
* position (`X = ApiPaths.Y + "/tail"`); handed to the core, `ApiPaths.Y` misses
* every map and floors the whole chain to null — see {@link computeKotlinFold},
* which resolves operands through the qualified-aware walk for exactly this
* reason. The import chase is Kotlin-specific too: a member import is spelled
* identically to a type import, so {@link resolveImportedName} has to try both
* readings, and the core exposes no hook for that. Java forked first on the same
* grounds. Teaching the core qualified names, and retiring both copies against
* it, is the standing follow-up; until then the honest description of this file
* is "a second fork", not "a binding over a shared fold".
*
* Kotlin shares the JVM package/import model with Java, so this binding mirrors
* `java-const-resolver.ts` in structure, naming and skip-floor discipline. The
@ -54,6 +72,17 @@
* file returns null (skip floor), never a wrong path. A missing route is a
* missing fact; a wrongly folded one is a false edge in the graph.
*
* POSIX keys are a PRECONDITION this module cannot check cheaply, so it is
* enforced at the one boundary that produces them: `http-patterns/kotlin.ts`
* normalizes separators on both the write side (the `prepareRepo` map keys) and
* the read side (`scan`'s `fileRel`). It has to, because the orchestrator's file
* list comes from glob v13, which has no `posix: true` and joins with the
* platform separator — so on Windows the keys arrive backslashed and every
* `<pkg>/<Name>.kt` test in {@link resolveKotlinImport} would miss, silently
* disabling cross-file folding on that platform alone. Normalizing INSIDE this
* module instead cannot work: the resolver returns the key it matched, and a
* normalized return value would then miss in a map that was never normalized.
*
* WHERE THIS IS WIRED. Java reaches its binding from BOTH layers: the group
* extractor (`group/extractors/http-patterns/java.ts`) and the ingestion
* provider (`languages/java.ts`, via `extractModuleConstants` +
@ -89,6 +118,30 @@ export type {
/** Source extensions a Kotlin declaration can live in. */
const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const;
/**
* Recursion ceiling for {@link parseKotlinConstOperands}, counted in `+` links.
*
* This bounds SYNTAX depth, not resolution: `A + B + C` nests one
* `additive_expression` per link, so the cap is really "how long a concatenation
* may one initializer be". Deliberately loose — generated route tables do
* concatenate a dozen fragments, and overrunning costs a skipped route, so the
* cap is a guard against pathological input rather than a statement about
* reasonable code.
*/
const MAX_OPERAND_PARSE_DEPTH = 64;
/**
* Recursion ceiling for the fold, counted in REFERENCE hops (`A = B`, `B = C`).
*
* Larger than the agnostic core's own `MAX_RESOLVE_DEPTH` (8), which is
* module-private in `constant-resolver.ts` and therefore cannot simply be
* reused, and equal to the value the Java binding spells inline. It backstops
* the cycle guard, which terminates loops but not a long acyclic chain; the
* memo makes reaching it cheap. Both caps floor to null, i.e. to a skipped
* route.
*/
const MAX_FOLD_DEPTH = 32;
/**
* Cheap content gate: can this Kotlin file DEFINE a string constant that a route
* annotation might reference?
@ -97,8 +150,9 @@ const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const;
* {@link extractKotlinModuleConstants} about which files carry constants — the
* defect class the Java binding's shared `isJavaConstantFile` exists to prevent.
*
* Arms, both deliberately WIDER than the extractor (a gate may over-admit — it
* only costs a parse — but must never reject a file the extractor accepts):
* Arms, both intended to be WIDER than the extractor (a gate may over-admit — it
* only costs a parse — while rejecting a file the extractor would accept costs a
* fact):
* - `const val NAME [: T] =`. `const` is legal only at a file's top level or in
* an `object`/`companion object`, i.e. exactly the carriers the extractor
* harvests, so this arm needs no scope check.
@ -108,6 +162,23 @@ const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const;
* whose only `val`s are function locals from costing a parse. It still admits
* a top-level `val` in a file that happens to declare an object elsewhere,
* which is the harmless direction.
*
* KNOWN GAP — the "never rejects what the extractor accepts" property does NOT
* hold, and claiming it did was wrong. {@link extractKotlinModuleConstants}
* calls `collectProperties(tree.rootNode, null)`, so it harvests a TOP-LEVEL
* non-`const` `val`; a file whose only constant has that shape and declares no
* `object` fails both arms above (`const val` absent, `object` absent) and is
* never parsed.
*
* The direction is safe — the constant is simply missing from the map, so a
* reference to it floors to skip, never to a wrong path — but the cost is not
* nil. Measured on both sides of this change: with the declaration in the SAME
* file as the route it still folds, because `scan` re-extracts that file's tree
* on demand and bypasses the gate; with the declaration in its OWN file the
* route is silently dropped. Closing the gap means admitting every file that
* contains any `val … =`, function locals included — very nearly the whole
* repository, in a pass whose entire purpose is to avoid parsing it. That trade
* has not been measured, so the gap is recorded here rather than papered over.
*/
const CONST_VAL_RE = /\bconst\s+val\s+\w+\s*(?::[^=\n{}()]{0,60})?=/;
const OBJECT_DECL_RE = /\bobject\b/;
@ -272,7 +343,7 @@ export function parseKotlinConstOperands(
depth = 0,
): Operand[] | null {
if (!node) return null;
if (depth > 64) return null;
if (depth > MAX_OPERAND_PARSE_DEPTH) return null;
if (node.type === 'string_literal') {
const value = stringLiteralValue(node);
return value === null ? null : [{ kind: 'literal', value }];
@ -523,7 +594,7 @@ function resolveWithState(
state: KotlinFoldState,
depth: number,
): string | null {
if (depth > 32) return null;
if (depth > MAX_FOLD_DEPTH) return null;
const guard = `${fileKey}::${name}`;
const memoized = state.memo.get(guard);
if (memoized !== undefined) return memoized;
@ -667,12 +738,22 @@ function foldOperands(
/**
* Fold an inline operand list (e.g. `ApiPaths.BASE + "/orders"`) against
* `fileKey`, or null when any piece is unresolvable (skip floor).
*
* An empty result is a SUCCESS, not a skip. `const val ROOT = ""` folds to `""`,
* which `joinPath` then resolves against the class-level prefix exactly as it
* resolves the literal `@GetMapping("")` — both mean "the prefix itself", the
* Spring idiom for a collection root. Collapsing it into `null` would make a
* resolved-empty path indistinguishable from an unresolvable one — the skip
* floor is reserved for "could not fold", and nothing else in the resolver
* conflates the two: {@link resolveKotlinConstant} returns `''` for an empty
* constant, and `resolveOperands` in the shared core returns its fold
* unfiltered. Matches `foldJavaOperands`, so the two JVM bindings do not
* diverge on the same input.
*/
export function foldKotlinOperands(
fileKey: string,
operands: readonly Operand[],
repo: RepoConstants,
): string | null {
const out = foldOperands(fileKey, operands, newFoldState(repo), 0);
return out === '' ? null : out;
return foldOperands(fileKey, operands, newFoldState(repo), 0);
}

74
gitnexus/test/unit/group/kotlin-const-route-fold.test.ts
View file

@ -31,6 +31,12 @@
* an interface whose `@RequestMapping` is a constant, and an unresolvable
* `path` is fatal on its own;
* • an unresolvable constant emits nothing rather than a guessed path;
* • a cross-file fold survives BACKSLASHED repository keys — the shape glob
* v13 hands the orchestrator on Windows, and the one every other fixture
* here misses by writing POSIX string literals;
* • a constant that folds to `""` publishes the class prefix, exactly as the
* literal `@GetMapping("")` beside it does — an empty fold is a success,
* not the skip floor;
* • without a repo context the plugin emits nothing (the documented skip
* floor, and the branch the 1-argument guards cannot reach);
* • literal routes are untouched and are not emitted twice.
@ -532,6 +538,74 @@ interface OrderClient {
).toEqual([]);
});
it('folds across files when repository keys use Windows separators', () => {
// The orchestrator's file list comes from glob v13, which has no
// `posix: true` and joins with the platform separator, so on Windows both
// `prepareRepo({files})` and `scan(tree, ctx, rel)` see
// `src\main\kotlin\…`. `resolveKotlinImport` asks whether a key ends with
// `com/example/app/api/ApiPaths.kt` — a test no backslashed key can pass —
// so EVERY cross-file fold returned null on Windows and on Windows only:
// the pre-pass still ran and the context was still built, the feature was
// just silently absent. Every other fixture in this file is a POSIX string
// literal, which is exactly why CI stayed green.
//
// The keys are backslashed HERE rather than derived from `path.sep`, so the
// regression is pinned on every runner instead of only on the Windows
// matrix — the plugin reads keys, not the host OS, so simulating the keys
// simulates the whole bug.
const winKey = (rel: string): string => rel.replace(/\//g, '\\');
expect(
providers({
[winKey(CONSTS)]: CONSTS_SRC,
[winKey(CONTROLLER)]: `package com.example.app.web
import com.example.app.api.ApiPaths
@RestController
class OrderController {
@GetMapping(ApiPaths.ORDERS)
fun list() {}
}
`,
}),
).toEqual(['GET /api/v1/orders']);
});
it('treats a constant that folds to "" as the class prefix itself', () => {
// `const val ROOT = ""` is Spring's idiom for "the collection root", and
// `joinPath` resolves it against the class prefix exactly as it resolves the
// literal `@PostMapping("")` beside it. The fold used to collapse `''` into
// the skip floor, so the two annotations below — the same path, written two
// ways — disagreed: the literal published `POST /api`, the constant
// published nothing. Asserting BOTH in one class is the point; a test on the
// constant alone would pass against any chosen convention rather than
// pinning the two spellings together.
expect(
providers({
[CONSTS]: `package com.example.app.api
object ApiPaths {
const val ROOT = ""
}
`,
[CONTROLLER]: `package com.example.app.web
import com.example.app.api.ApiPaths
@RestController
@RequestMapping("/api")
class OrderController {
@GetMapping(ApiPaths.ROOT)
fun list() {}
@PostMapping("")
fun create() {}
}
`,
}),
).toEqual(['GET /api/', 'POST /api/']);
});
it('leaves literal routes unchanged and emits each exactly once', () => {
expect(
providers({

22
gitnexus/test/unit/kotlin-route-const-resolver.test.ts
View file

@ -296,6 +296,28 @@ import com.example.app.api.*
expect(foldKotlinOperands(CONSTS_KEY, [{ kind: 'ref', name: 'MISSING' }], repo)).toBeNull();
});
it('folds to the empty string as a SUCCESS, not a skip', () => {
// The counterpart of the test above, and the distinction it depends on:
// `null` means "could not fold", `''` means "folded, and the answer is
// empty". `const val ROOT = ""` is Spring's spelling for "the class prefix
// itself", so collapsing it into null loses a route the literal
// `@GetMapping("")` publishes from the same class. `resolveKotlinConstant`
// already returned `''` here; `foldKotlinOperands` did not, which made the
// two entry points disagree about the same constant.
const key = 'src/main/kotlin/com/example/app/api/Root.kt';
const repo = repoOf({
[key]: `package com.example.app.api
object ApiPaths {
const val ROOT = ""
}
`,
});
expect(resolveKotlinConstant(key, 'ApiPaths.ROOT', repo)).toBe('');
expect(foldKotlinOperands(key, [{ kind: 'ref', name: 'ApiPaths.ROOT' }], repo)).toBe('');
expect(foldKotlinOperands(key, [{ kind: 'literal', value: '' }], repo)).toBe('');
});
it('terminates on a self-referential constant', () => {
const key = 'src/main/kotlin/com/example/app/api/Cycle.kt';
const repo = repoOf({