mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-29 06:32:28 +00:00
* 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>
This commit is contained in:
Gergő Magyar
GitHub
parent
b3680d5e30
commit
492800863e
7 changed files with 364 additions and 31 deletions
|
|
@ -112,11 +112,14 @@ flowchart TD
|
|||
EMIT --> BRIDGE[(bridge.lbug<br/>#795)]
|
||||
```
|
||||
|
||||
Label-scoped queries in `resolveSymbol` keep accidental cross-matches
|
||||
out:
|
||||
- `topic` → `(n:Function|Method|Class|Interface)`
|
||||
- `grpc` method → `(n:Function|Method)`, service → `(n:Class|Interface)`
|
||||
- `lib` → `(n:Package|Module)`
|
||||
Label-scoped queries in `resolveSymbol` keep accidental cross-matches out.
|
||||
They use the `MATCH (n) WHERE labels(n) IN [...]` allowlist form, NOT the
|
||||
`MATCH (n:A|B)` disjunction — LadybugDB's parser rejects a disjunction that
|
||||
names a reserved keyword (e.g. `Macro`, `Union`), which is what broke the
|
||||
`custom` branch in #2325:
|
||||
- `topic` → `labels(n) IN ['Function','Method','Class','Interface']`
|
||||
- `grpc`/`thrift` method → `labels(n) IN ['Function','Method']`, service → `labels(n) IN ['Class','Interface']`
|
||||
- `lib` → `labels(n) IN ['Module']`
|
||||
|
||||
## Cross-impact query (PR #606)
|
||||
|
||||
|
|
|
|||
|
|
@ -80,10 +80,21 @@ WHERE sym.filePath = $filePath AND sym.startLine IS NOT NULL AND sym.endLine IS
|
|||
RETURN sym.id AS uid, sym.name AS name, sym.filePath AS filePath,
|
||||
sym.startLine AS startLine, sym.endLine AS endLine, labels(sym) AS labels`;
|
||||
|
||||
// Repo-wide lookup of a symbol by exact name (label-union, as in
|
||||
// manifest-extractor.ts). Used to resolve a provider's named handler when it is
|
||||
// defined in a file OTHER than its route registration — and only honored when
|
||||
// the result is unique (see resolveSymbolByNameUnique).
|
||||
// Repo-wide lookup of a symbol by exact name. Used to resolve a provider's
|
||||
// named handler when it is defined in a file OTHER than its route registration —
|
||||
// and only honored when the result is unique (see resolveSymbolByNameUnique).
|
||||
//
|
||||
// Label filtering uses `labels(n) IN [...]` rather than the openCypher
|
||||
// disjunction `MATCH (n:A|B|C)`. NOTE: this 3-label set (Function/Method/
|
||||
// CodeElement) actually PARSES — LadybugDB only rejects a disjunction that
|
||||
// names a reserved keyword (e.g. `Macro`, `Union`) or a missing node table,
|
||||
// neither of which applies here. So this query was NOT broken by #2325; it
|
||||
// uses the `labels(n) IN` form for consistency with the manifest custom-branch
|
||||
// fix (which WAS broken) and to stay immune if a reserved-keyword label is
|
||||
// added later. `labels(n)` returns the node's single label as a string here, so
|
||||
// `IN [...]` is an exact allowlist. (Exported so integration tests can run the
|
||||
// exact production query against a real LadybugDB — the bug shipped because no
|
||||
// test ran these strings against the real parser.)
|
||||
//
|
||||
// `n.filePath <> ''` excludes synthetic non-source `CodeElement` nodes that
|
||||
// carry no real file — ORM model/table nodes (orm.ts emits `filePath: ''`) and
|
||||
|
|
@ -91,9 +102,9 @@ RETURN sym.id AS uid, sym.name AS name, sym.filePath AS filePath,
|
|||
// degenerate edge-less node NOR inflates the uniqueness count and masks the real
|
||||
// handler. `LIMIT 2` bounds materialization: distinguishing unique (1) from
|
||||
// ambiguous (>=2) never needs more than two rows (the count guard stays exact).
|
||||
const RESOLVE_BY_NAME_QUERY = `
|
||||
MATCH (n:Function|Method|CodeElement)
|
||||
WHERE n.name = $name AND n.filePath <> ''
|
||||
export const RESOLVE_BY_NAME_QUERY = `
|
||||
MATCH (n) WHERE labels(n) IN ['Function','Method','CodeElement']
|
||||
AND n.name = $name AND n.filePath <> ''
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
LIMIT 2`;
|
||||
|
||||
|
|
@ -103,9 +114,9 @@ LIMIT 2`;
|
|||
// the precise rung — it survives aliases and local same-name collisions that a
|
||||
// repo-wide name lookup cannot, and only resolves on a unique match within that
|
||||
// module. `LIMIT 2` keeps the uniqueness count exact (see RESOLVE_BY_NAME_QUERY).
|
||||
const RESOLVE_IN_MODULE_QUERY = `
|
||||
MATCH (n:Function|Method|CodeElement)
|
||||
WHERE n.name = $name AND (n.filePath STARTS WITH $fileDot OR n.filePath STARTS WITH $fileSlash)
|
||||
export const RESOLVE_IN_MODULE_QUERY = `
|
||||
MATCH (n) WHERE labels(n) IN ['Function','Method','CodeElement']
|
||||
AND n.name = $name AND (n.filePath STARTS WITH $fileDot OR n.filePath STARTS WITH $fileSlash)
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
LIMIT 2`;
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,21 @@ export interface ManifestExtractResult {
|
|||
crossLinks: CrossLink[];
|
||||
}
|
||||
|
||||
// Repo-wide symbol lookup for `custom` workspace contracts. Exported so the
|
||||
// #2325 integration test can run the EXACT production query against a real
|
||||
// LadybugDB — a hand-copied query string in the test would silently drift
|
||||
// from this allowlist. Uses the `labels(n) IN [...]` allowlist form rather
|
||||
// than a `MATCH (n:A|B)` disjunction: this 21-label list contains the
|
||||
// reserved-keyword labels `Macro` and `Union`, and LadybugDB's parser rejects
|
||||
// a disjunction that names a reserved keyword (#2325) — which the resolver's
|
||||
// try/catch then swallowed. `labels(n) IN` has no such collision.
|
||||
export const CUSTOM_CONTRACT_RESOLVE_QUERY = `MATCH (n)
|
||||
WHERE labels(n) IN ['Function','Method','Class','Interface','Struct','Enum','Trait','Constructor','TypeAlias','Impl','Macro','Union','Typedef','Property','Record','Delegate','Annotation','Template','Const','Static','CodeElement']
|
||||
AND n.name = $symbolName
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
ORDER BY n.filePath ASC
|
||||
LIMIT 1`;
|
||||
|
||||
/**
|
||||
* Canonicalize an HTTP path for matching against Route.name in the graph.
|
||||
* Mirrors core/ingestion/pipeline.ts ensureSlash semantics:
|
||||
|
|
@ -189,6 +204,18 @@ export class ManifestExtractor {
|
|||
// Cross-impact still works: the bridge query joins on the synthetic
|
||||
// uid, and the local impact engine derives the same uid for the
|
||||
// unresolved symbol — name-based hints are the additional safety net.
|
||||
//
|
||||
// Label filtering uses `MATCH (n) WHERE labels(n) IN [...]`, NOT the
|
||||
// openCypher disjunction `MATCH (n:A|B|C)`. LadybugDB's parser rejects a
|
||||
// disjunction that names a reserved keyword (`Macro` and `Union` both are)
|
||||
// OR a label with no node table (e.g. the old `lib` branch's `Package`).
|
||||
// The `custom` branch (reserved keywords in its list) and `lib` branch
|
||||
// (missing `Package` table) genuinely threw (#2325) and the whole try/catch
|
||||
// below swallowed it; the other branches parsed but use the same form for
|
||||
// consistency and future-proofing. `labels(n)` returns the node's single
|
||||
// label as a string here, so `IN [...]` is an exact allowlist that includes
|
||||
// listed labels and excludes everything else — and is immune to both
|
||||
// failure modes (no keyword collision; an unknown label is just a non-match).
|
||||
try {
|
||||
let rows: Record<string, unknown>[];
|
||||
if (link.type === 'http') {
|
||||
|
|
@ -222,7 +249,7 @@ export class ManifestExtractor {
|
|||
// avoid cross-matching Files/Variables/Imports that happen to
|
||||
// share the topic name.
|
||||
rows = await executor(
|
||||
`MATCH (n:Function|Method|Class|Interface) WHERE n.name = $contract
|
||||
`MATCH (n) WHERE labels(n) IN ['Function','Method','Class','Interface'] AND n.name = $contract
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
ORDER BY n.filePath ASC
|
||||
LIMIT 1`,
|
||||
|
|
@ -246,7 +273,7 @@ export class ManifestExtractor {
|
|||
const methodName = parts[1]?.trim() ?? '';
|
||||
if (methodName) {
|
||||
rows = await executor(
|
||||
`MATCH (n:Function|Method) WHERE n.name = $methodName
|
||||
`MATCH (n) WHERE labels(n) IN ['Function','Method'] AND n.name = $methodName
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
ORDER BY n.filePath ASC
|
||||
LIMIT 1`,
|
||||
|
|
@ -254,7 +281,7 @@ export class ManifestExtractor {
|
|||
);
|
||||
} else if (serviceName) {
|
||||
rows = await executor(
|
||||
`MATCH (n:Class|Interface) WHERE n.name = $serviceName
|
||||
`MATCH (n) WHERE labels(n) IN ['Class','Interface'] AND n.name = $serviceName
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
ORDER BY n.filePath ASC
|
||||
LIMIT 1`,
|
||||
|
|
@ -266,11 +293,12 @@ export class ManifestExtractor {
|
|||
} else if (link.type === 'lib') {
|
||||
// Only exact match on the symbol's name. Previous fallback to
|
||||
// CONTAINS on n.filePath would promote "react" to "react-native"
|
||||
// or "@types/react" — silent wrong attribution. Restrict to
|
||||
// package-level labels so we don't return arbitrary symbols
|
||||
// named after a library.
|
||||
// or "@types/react" — silent wrong attribution. Restrict to the
|
||||
// package-level `Module` label so we don't return arbitrary symbols
|
||||
// named after a library. (There is no `Package` node table — see
|
||||
// NODE_TABLES — so a `Package` entry only ever matched nothing.)
|
||||
rows = await executor(
|
||||
`MATCH (n:Package|Module) WHERE n.name = $contract
|
||||
`MATCH (n) WHERE labels(n) IN ['Module'] AND n.name = $contract
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
ORDER BY n.filePath ASC
|
||||
LIMIT 1`,
|
||||
|
|
@ -291,14 +319,7 @@ export class ManifestExtractor {
|
|||
const symbolName = link.contract.includes('::')
|
||||
? link.contract.split('::').pop()!
|
||||
: link.contract;
|
||||
rows = await executor(
|
||||
`MATCH (n:Function|Method|Class|Interface|Struct|Enum|Trait|Constructor|TypeAlias|Impl|Macro|Union|Typedef|Property|Record|Delegate|Annotation|Template|Const|Static|CodeElement)
|
||||
WHERE n.name = $symbolName
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
ORDER BY n.filePath ASC
|
||||
LIMIT 1`,
|
||||
{ symbolName },
|
||||
);
|
||||
rows = await executor(CUSTOM_CONTRACT_RESOLVE_QUERY, { symbolName });
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,111 @@
|
|||
/**
|
||||
* Issue #2325 (http-route extractor half): `RESOLVE_BY_NAME_QUERY` and
|
||||
* `RESOLVE_IN_MODULE_QUERY` were converted from the `MATCH (n:A|B)` multi-label
|
||||
* disjunction to the `labels(n) IN [...]` allowlist form, for consistency with
|
||||
* the manifest custom-branch fix.
|
||||
*
|
||||
* IMPORTANT nuance (verified against the real parser): the actual #2325 failure
|
||||
* was triggered by *reserved-keyword* labels. LadybugDB's parser rejects a
|
||||
* disjunction that names a reserved keyword — `Macro` and `Union` both are — so
|
||||
* the manifest custom branch (whose 21-label list contains `Macro`/`Union`)
|
||||
* genuinely threw, and the resolver's try/catch swallowed it. The http-route
|
||||
* 3-label disjunction `(n:Function|Method|CodeElement)` contains no reserved
|
||||
* keyword and actually PARSES — so this half was a consistency conversion, not a
|
||||
* parser fix. The value of these cases is verifying the EXPORTED production
|
||||
* queries resolve and filter correctly against a real LadybugDB (the unit tests
|
||||
* cover the resolution *logic* with a fake executor; these cover *parsing +
|
||||
* filtering*). The last case pins the real reserved-keyword trigger so a future
|
||||
* query that reintroduces a `MATCH (n:…|Macro|…)` disjunction is caught.
|
||||
*/
|
||||
import { it, expect, afterEach } from 'vitest';
|
||||
import {
|
||||
RESOLVE_BY_NAME_QUERY,
|
||||
RESOLVE_IN_MODULE_QUERY,
|
||||
} from '../../../src/core/group/extractors/http-route-extractor.js';
|
||||
import { initLbug, executeParameterized, closeLbug } from '../../../src/core/lbug/pool-adapter.js';
|
||||
import { withTestLbugDB } from '../../helpers/test-indexed-db.js';
|
||||
|
||||
const SEED = [
|
||||
// BY_NAME: one real-file Function + a same-named File (label excluded) + a
|
||||
// same-named CodeElement with empty filePath (excluded by `n.filePath <> ''`).
|
||||
`CREATE (:Function {id:'fn:getOrders', name:'getOrders', filePath:'src/handlers/orders.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:File {id:'file:getOrders', name:'getOrders', filePath:'src/getOrders.ts'})`,
|
||||
`CREATE (:CodeElement {id:'ce:getOrders', name:'getOrders', filePath:''})`,
|
||||
// IN_MODULE: same name in two modules — only the prefixed one resolves.
|
||||
`CREATE (:Function {id:'fn:listUsers:handlers', name:'listUsers', filePath:'src/handlers/users.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:Function {id:'fn:listUsers:admin', name:'listUsers', filePath:'src/admin/users.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
// IN_MODULE label-allowlist decoy: same name, SAME module prefix, wrong label.
|
||||
// The STARTS-WITH prefix would match it, so only the `labels(n) IN [...]` filter
|
||||
// excludes it — drop the filter and this surfaces, flipping the row count.
|
||||
`CREATE (:File {id:'file:listUsers:handlers', name:'listUsers', filePath:'src/handlers/users.ts'})`,
|
||||
// LIMIT 2 cap: three same-named Functions — the uniqueness count must stay exact.
|
||||
`CREATE (:Function {id:'fn:dup:1', name:'dup', filePath:'src/a.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:Function {id:'fn:dup:2', name:'dup', filePath:'src/b.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:Function {id:'fn:dup:3', name:'dup', filePath:'src/c.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
];
|
||||
|
||||
withTestLbugDB(
|
||||
'issue-2325-http-route-resolveSymbol',
|
||||
(handle) => {
|
||||
afterEach(async () => {
|
||||
try {
|
||||
await closeLbug(handle.repoId);
|
||||
} catch {
|
||||
/* best-effort */
|
||||
}
|
||||
});
|
||||
|
||||
it('RESOLVE_BY_NAME_QUERY resolves a handler by name, excluding a File and an empty-filePath node', async () => {
|
||||
await initLbug(handle.repoId, handle.dbPath);
|
||||
const rows = await executeParameterized(handle.repoId, RESOLVE_BY_NAME_QUERY, {
|
||||
name: 'getOrders',
|
||||
});
|
||||
// File (wrong label) and the empty-filePath CodeElement are both excluded.
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].uid).toBe('fn:getOrders');
|
||||
expect(rows[0].filePath).toBe('src/handlers/orders.ts');
|
||||
});
|
||||
|
||||
it('RESOLVE_IN_MODULE_QUERY resolves only the handler in the target module prefix', async () => {
|
||||
await initLbug(handle.repoId, handle.dbPath);
|
||||
const rows = await executeParameterized(handle.repoId, RESOLVE_IN_MODULE_QUERY, {
|
||||
name: 'listUsers',
|
||||
fileDot: 'src/handlers/users.',
|
||||
fileSlash: 'src/handlers/users/',
|
||||
});
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].uid).toBe('fn:listUsers:handlers');
|
||||
expect(rows[0].filePath).toBe('src/handlers/users.ts');
|
||||
});
|
||||
|
||||
it('RESOLVE_BY_NAME_QUERY caps materialization at 2 rows (uniqueness count stays exact)', async () => {
|
||||
await initLbug(handle.repoId, handle.dbPath);
|
||||
const rows = await executeParameterized(handle.repoId, RESOLVE_BY_NAME_QUERY, {
|
||||
name: 'dup',
|
||||
});
|
||||
// Three matches exist; LIMIT 2 returns exactly two so the caller treats it
|
||||
// as ambiguous (>=2) without over-materializing.
|
||||
expect(rows).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('LadybugDB rejects a disjunction naming a reserved-keyword label (the real #2325 trigger)', async () => {
|
||||
await initLbug(handle.repoId, handle.dbPath);
|
||||
// The genuine #2325 failure: a `MATCH (n:A|B)` disjunction whose label set
|
||||
// includes a reserved keyword (`Macro`/`Union`) is a parser error the
|
||||
// resolver's try/catch silently swallowed. The exported queries avoid this
|
||||
// by using the `labels(n) IN [...]` allowlist form. (The http-route
|
||||
// `Function|Method|CodeElement` form parses — this guards the real cause.)
|
||||
await expect(
|
||||
executeParameterized(
|
||||
handle.repoId,
|
||||
`MATCH (n:Function|Macro|Union) WHERE n.name = $name RETURN n.id AS uid LIMIT 2`,
|
||||
{ name: 'getOrders' },
|
||||
),
|
||||
).rejects.toThrow(/Parser exception|Invalid input|Prepare failed/i);
|
||||
});
|
||||
},
|
||||
{
|
||||
seed: SEED,
|
||||
poolAdapter: true,
|
||||
},
|
||||
);
|
||||
|
|
@ -0,0 +1,173 @@
|
|||
/**
|
||||
* Regression test for issue #2325:
|
||||
* manifest-extractor `resolveSymbol` used a multi-label Cypher disjunction
|
||||
* (`MATCH (n:Function|Method|Class|...)`). LadybugDB's parser rejects such a
|
||||
* disjunction only when a label is a reserved keyword (`Macro` and `Union` both
|
||||
* are) or names a missing node table — so the `custom` branch (reserved
|
||||
* keywords in its 21-label list) and the `lib` branch (no `Package` table)
|
||||
* genuinely threw, silently falling back to a synthetic UID with empty
|
||||
* `filePath`; the other branches happened to parse. The fix uses
|
||||
* `MATCH (n) WHERE labels(n) IN [...]` uniformly — immune to both failure modes
|
||||
* (LadybugDB returns a node's single label as a string, so `IN [...]` is an
|
||||
* exact allowlist).
|
||||
*
|
||||
* These cases run the REAL production query (via `extractFromManifest`) against a
|
||||
* real LadybugDB — the only layer that can catch a parser rejection, since the
|
||||
* unit tests mock the executor. Each per-branch case seeds a wrong-label decoy
|
||||
* whose `filePath` sorts BEFORE the target so a widened/broken allowlist would
|
||||
* surface the decoy under `ORDER BY n.filePath ASC LIMIT 1` and flip the
|
||||
* assertion — making the exclusion check non-vacuous.
|
||||
*/
|
||||
import { it, expect, afterEach } from 'vitest';
|
||||
import {
|
||||
ManifestExtractor,
|
||||
CUSTOM_CONTRACT_RESOLVE_QUERY,
|
||||
} from '../../../src/core/group/extractors/manifest-extractor.js';
|
||||
import type { GroupManifestLink } from '../../../src/core/group/types.js';
|
||||
import type { CypherExecutor } from '../../../src/core/group/contract-extractor.js';
|
||||
import { initLbug, executeParameterized, closeLbug } from '../../../src/core/lbug/pool-adapter.js';
|
||||
import { withTestLbugDB } from '../../helpers/test-indexed-db.js';
|
||||
|
||||
// Targets + sort-first wrong-label decoys for each link-type branch.
|
||||
const SEED = [
|
||||
// custom → Class; decoy File (not in allowlist) sorts first.
|
||||
`CREATE (:Class {id:'cls:MyServiceFacade', name:'MyServiceFacade', filePath:'src/main/java/com/example/MyServiceFacade.java', startLine:1, endLine:42, content:'', description:''})`,
|
||||
`CREATE (:File {id:'file:MyServiceFacade', name:'MyServiceFacade', filePath:'aaa/MyServiceFacade.java'})`,
|
||||
// grpc/thrift method → Method; decoy Class (not in [Function,Method]) sorts first.
|
||||
`CREATE (:Method {id:'mth:Login', name:'Login', filePath:'src/auth_grpc.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:Class {id:'cls:Login', name:'Login', filePath:'aaa/Login.java', startLine:1, endLine:9, content:'', description:''})`,
|
||||
// grpc/thrift service → Class; decoy Function (not in [Class,Interface]) sorts first.
|
||||
`CREATE (:Class {id:'cls:AuthService', name:'AuthService', filePath:'src/auth_service.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:Function {id:'fn:AuthService', name:'AuthService', filePath:'aaa/AuthService.go', startLine:1, endLine:9, content:'', description:''})`,
|
||||
// lib → Module (there is NO Package node table); decoy Function sorts first.
|
||||
`CREATE (:Module {id:'mod:mylib', name:'mylib', filePath:'src/index.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:Function {id:'fn:mylib', name:'mylib', filePath:'aaa/mylib.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
// topic → Function; decoy File sorts first.
|
||||
`CREATE (:Function {id:'fn:orders.created', name:'orders.created', filePath:'src/consumer.ts', startLine:1, endLine:9, content:'', description:''})`,
|
||||
`CREATE (:File {id:'file:orders.created', name:'orders.created', filePath:'aaa/orders.created.ts'})`,
|
||||
];
|
||||
|
||||
/** Custom-branch `custom` query as emitted BEFORE the fix — kept to document
|
||||
* the real failure: its label list contains the reserved keywords `Macro` and
|
||||
* `Union`, which make LadybugDB's parser reject the whole disjunction. */
|
||||
const MULTI_LABEL_CUSTOM_QUERY = `MATCH (n:Function|Method|Class|Interface|Struct|Enum|Trait|Constructor|TypeAlias|Impl|Macro|Union|Typedef|Property|Record|Delegate|Annotation|Template|Const|Static|CodeElement)
|
||||
WHERE n.name = $symbolName
|
||||
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
|
||||
ORDER BY n.filePath ASC
|
||||
LIMIT 1`;
|
||||
|
||||
// Direct-query canary for the `labels(n)`-is-a-string assumption the whole fix
|
||||
// relies on. Uses the EXACT production query (imported, not hand-copied) so the
|
||||
// canary can never silently drift from the real allowlist.
|
||||
|
||||
withTestLbugDB(
|
||||
'issue-2325-manifest-resolveSymbol',
|
||||
(handle) => {
|
||||
afterEach(async () => {
|
||||
try {
|
||||
await closeLbug(handle.repoId);
|
||||
} catch {
|
||||
/* best-effort */
|
||||
}
|
||||
});
|
||||
|
||||
// role:'consumer' → providerRepo = link.to = 'repo-b' (the seeded DB); the
|
||||
// resolved symbol lands in crossLinks[0].to. consumerRepo ('repo-a') has no
|
||||
// executor, so to.symbolRef carries the provider-side resolution.
|
||||
const resolveVia = async (
|
||||
type: GroupManifestLink['type'],
|
||||
contract: string,
|
||||
): Promise<{ symbolUid: string; filePath: string; name: string }> => {
|
||||
await initLbug(handle.repoId, handle.dbPath);
|
||||
const executor: CypherExecutor = (query, params) =>
|
||||
executeParameterized(handle.repoId, query, params ?? {});
|
||||
const extractor = new ManifestExtractor();
|
||||
const result = await extractor.extractFromManifest(
|
||||
[{ from: 'repo-a', to: 'repo-b', type, contract, role: 'consumer' }],
|
||||
new Map([['repo-b', executor]]),
|
||||
);
|
||||
expect(result.crossLinks).toHaveLength(1);
|
||||
const to = result.crossLinks[0].to;
|
||||
return {
|
||||
symbolUid: to.symbolUid,
|
||||
filePath: to.symbolRef.filePath,
|
||||
name: to.symbolRef.name,
|
||||
};
|
||||
};
|
||||
|
||||
it('LadybugDB rejects the custom-branch disjunction (its list names reserved keywords Macro/Union)', async () => {
|
||||
await initLbug(handle.repoId, handle.dbPath);
|
||||
// The genuine #2325 trigger for the custom branch: `Macro` and `Union` are
|
||||
// reserved keywords, so the parser rejects the whole `(n:…|Macro|…|Union|…)`
|
||||
// disjunction — which the resolver's try/catch then swallowed.
|
||||
await expect(
|
||||
executeParameterized(handle.repoId, MULTI_LABEL_CUSTOM_QUERY, {
|
||||
symbolName: 'MyServiceFacade',
|
||||
}),
|
||||
).rejects.toThrow(/Parser exception|Invalid input|Prepare failed/i);
|
||||
});
|
||||
|
||||
it('direct labels(n) IN query resolves the symbol (canary for labels()-is-a-string)', async () => {
|
||||
await initLbug(handle.repoId, handle.dbPath);
|
||||
const rows = await executeParameterized(handle.repoId, CUSTOM_CONTRACT_RESOLVE_QUERY, {
|
||||
symbolName: 'MyServiceFacade',
|
||||
});
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].uid).toBe('cls:MyServiceFacade');
|
||||
expect(rows[0].filePath).toBe('src/main/java/com/example/MyServiceFacade.java');
|
||||
});
|
||||
|
||||
it('custom contract resolves the real Class symbol, excluding a same-named File (#2325)', async () => {
|
||||
const r = await resolveVia('custom', 'custom::MyServiceFacade');
|
||||
expect(r.symbolUid).toBe('cls:MyServiceFacade');
|
||||
expect(r.filePath).toBe('src/main/java/com/example/MyServiceFacade.java');
|
||||
expect(r.name).toBe('MyServiceFacade');
|
||||
// real resolution, not the synthetic fallback the bug produced
|
||||
expect(r.symbolUid.startsWith('manifest::')).toBe(false);
|
||||
});
|
||||
|
||||
it('grpc method contract resolves a Method, excluding a same-named Class', async () => {
|
||||
const r = await resolveVia('grpc', 'AuthService/Login');
|
||||
expect(r.symbolUid).toBe('mth:Login');
|
||||
expect(r.filePath).toBe('src/auth_grpc.ts');
|
||||
expect(r.symbolUid.startsWith('manifest::')).toBe(false);
|
||||
});
|
||||
|
||||
it('grpc service contract resolves a Class, excluding a same-named Function', async () => {
|
||||
const r = await resolveVia('grpc', 'AuthService');
|
||||
expect(r.symbolUid).toBe('cls:AuthService');
|
||||
expect(r.filePath).toBe('src/auth_service.ts');
|
||||
expect(r.symbolUid.startsWith('manifest::')).toBe(false);
|
||||
});
|
||||
|
||||
it('thrift service contract strips the package prefix before resolving the Class', async () => {
|
||||
// The thrift-only branch strips `package.` from the service name
|
||||
// (`com.example.AuthService` -> `AuthService`). Without the strip the
|
||||
// lookup would query for `com.example.AuthService`, match nothing, and
|
||||
// fall back to a synthetic `manifest::` uid — so this resolving to the
|
||||
// real Class is the load-bearing assertion for the package-strip path.
|
||||
const r = await resolveVia('thrift', 'com.example.AuthService');
|
||||
expect(r.symbolUid).toBe('cls:AuthService');
|
||||
expect(r.filePath).toBe('src/auth_service.ts');
|
||||
expect(r.symbolUid.startsWith('manifest::')).toBe(false);
|
||||
});
|
||||
|
||||
it('lib contract resolves a Module, excluding a same-named Function', async () => {
|
||||
const r = await resolveVia('lib', 'mylib');
|
||||
expect(r.symbolUid).toBe('mod:mylib');
|
||||
expect(r.filePath).toBe('src/index.ts');
|
||||
expect(r.symbolUid.startsWith('manifest::')).toBe(false);
|
||||
});
|
||||
|
||||
it('topic contract resolves a Function, excluding a same-named File', async () => {
|
||||
const r = await resolveVia('topic', 'orders.created');
|
||||
expect(r.symbolUid).toBe('fn:orders.created');
|
||||
expect(r.filePath).toBe('src/consumer.ts');
|
||||
expect(r.symbolUid.startsWith('manifest::')).toBe(false);
|
||||
});
|
||||
},
|
||||
{
|
||||
seed: SEED,
|
||||
poolAdapter: true,
|
||||
},
|
||||
);
|
||||
|
|
@ -839,7 +839,17 @@ describe('ManifestExtractor', () => {
|
|||
|
||||
await extractor.extractFromManifest(links, dbExecutors);
|
||||
|
||||
expect(capturedCypher).toContain('Function|Method|Class|Interface|Struct|Enum|Trait');
|
||||
// This `custom`-branch list contains the reserved keywords `Macro`/`Union`,
|
||||
// which make LadybugDB's parser reject the `MATCH (n:A|B|C)` disjunction
|
||||
// (#2325), so the allowlist is carried as `labels(n) IN [...]`, not `n:A|B`.
|
||||
expect(capturedCypher).toContain('labels(n) IN [');
|
||||
// Membership checks that tolerate label order/spacing changes in the
|
||||
// production allowlist (the negative guards below are the real regression
|
||||
// check — a re-introduced `:A|B` disjunction has no quoted labels at all).
|
||||
expect(capturedCypher).toContain("'Function'");
|
||||
expect(capturedCypher).toContain("'Method'");
|
||||
expect(capturedCypher).toContain("'CodeElement'");
|
||||
expect(capturedCypher).not.toContain('Function|Method');
|
||||
expect(capturedCypher).not.toContain('NOT n:File');
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -72,6 +72,8 @@ export default defineConfig({
|
|||
'test/integration/analyze-wal-checkpoint-failure.test.ts',
|
||||
'test/integration/lbug-non-ascii-path.test.ts',
|
||||
'test/integration/lbug-conn-serialization.test.ts',
|
||||
'test/integration/group/manifest-resolve-symbol-2325.test.ts',
|
||||
'test/integration/group/http-route-resolve-symbol.test.ts',
|
||||
],
|
||||
fileParallelism: false,
|
||||
sequence: { groupOrder: 1 },
|
||||
|
|
@ -107,6 +109,8 @@ export default defineConfig({
|
|||
'test/integration/analyze-wal-checkpoint-failure.test.ts',
|
||||
'test/integration/lbug-non-ascii-path.test.ts',
|
||||
'test/integration/lbug-conn-serialization.test.ts',
|
||||
'test/integration/group/manifest-resolve-symbol-2325.test.ts',
|
||||
'test/integration/group/http-route-resolve-symbol.test.ts',
|
||||
'test/integration/skills-e2e.test.ts',
|
||||
],
|
||||
},
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue