scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-09-11 22:53:04 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

fix(group): self-review before merge — correctness and robustness fixes

Browse source 
Before this PR lands, I ran a systematic self-review of the cross-impact
module and fixed 13 correctness / robustness issues caught during it.
All changes target files added by this PR; nothing outside of group/*
or the new MCP group_impact tool schema is touched.

Everything below is in this single commit. The rest of the PR history
is unchanged.

Full group suite: all unit and integration test files green when run
individually (bridge-db.test.ts hits a pre-existing native LadybugDB
worker cleanup segfault that sometimes flakes the reported count —
documented in vitest.config.ts, unrelated to these fixes). CI on this
commit: 11/11 green across ubuntu/macos/windows + quality + typecheck
+ e2e.

═══════════════════════════════════════════════════════════════════════
HIGH severity — correctness bugs with regression tests
═══════════════════════════════════════════════════════════════════════

HIGH #1 — manifest-extractor.resolveSymbol was too fuzzy
───────────────────────────────────────────────────────────
resolveSymbol() used CONTAINS on route/name fields plus an unconditional
`filePath ENDS WITH '.proto'` fallback for gRPC. Consequences in
production:
  * HTTP: manifest "/orders" matched "/suborders", "/myorders/{id}"
  * gRPC: ANY repo with any .proto file returned a random proto symbol
    regardless of whether the service/method matched
  * topic/lib: similar CONTAINS false positives

Now all four types use EXACT equality on the relevant name field plus
deterministic ORDER BY before LIMIT 1. HTTP paths are canonicalized
(ensureSlash + trailing-slash strip) before matching Route.name via a
new normalizeRoutePath() helper. gRPC matches by structured
service/method name with NO .proto fallback.

To avoid cross-impact breakage when resolveSymbol now legitimately
returns null (the indexer may not expose a node with the exact method
name), unresolved manifest contracts now get a deterministic synthetic
symbolUid `manifest::<repo>::<contractId>` instead of an empty string.
This gives the bridge Cypher query a stable anchor on both provider
and consumer sides.

HIGH #2 — sync.ts per-extractor isolation
───────────────────────────────────────────────────────────
syncGroup() wrapped all three extractors (http, grpc, topic) plus
service-boundary detection plus meta.json read in ONE try/catch around
the whole repo loop. A single extractor throwing would mark the whole
repo as `missingRepos` (looking like "repo not found") and prevent the
other extractors from running.

Now each step is isolated with per-failure reporting:
  * initLbug failure → repo marked missing, labeled `init`
  * detectServiceBoundaries failure → empty boundaries, contracts still
    extracted without service attribution
  * each extractor failure → recorded with {repo, extractor, message};
    other extractors in the repo keep running
  * manifest extractor failure → recorded but doesn't fail the sync
  * bridge write partial failures → summarized as `bridge_write`

HIGH #3 — bridge-db.writeBridge per-item tolerance
───────────────────────────────────────────────────────────
writeBridge() had no per-item try/catch on CREATE loops. A single
malformed contract, snapshot, or link would abort the whole write. The
atomic swap protected the OLD bridge but threw away all valid inserts
from the current run.

Each CREATE is now wrapped individually. writeBridge() returns a new
WriteBridgeReport with insert/fail counts plus a capped sampleErrors
array. sync.ts surfaces non-zero failure counts via extractorFailures.
Dropped cross-links (findContractNode returning null) are counted
separately in linksDroppedMissingNode — previously silently continued.

HIGH #4 — writeBridge handle lifecycle
───────────────────────────────────────────────────────────
writeBridge() opened the tmp LadybugDB handle before the insert loops
but only closed it at the end of the happy path. If ensureBridgeSchema,
any queryBridge CREATE, findContractNode, or the contract/snapshot/link
loops threw, the handle leaked — which holds the native file lock on
`bridge.lbug.tmp`, leaking an FD and preventing the next sync from
reusing the tmp slot.

Wrapped everything from `ensureBridgeSchema(handle)` to the explicit
`closeBridgeDb(handle)` in try/finally with a `handleClosed` sentinel
so we never double-close on the happy path and always close on the
error path.

HIGH #5 — openBridgeDbReadOnly + openBridgeOrFallback leaks
───────────────────────────────────────────────────────────
Two leak paths:
  * openBridgeDbReadOnly: `new lbug.Database(...)` followed by
    `new lbug.Connection(db)` was wrapped in a single try/catch, but
    if Connection construction threw AFTER Database allocation, the
    Database was leaked (no teardown path for a partial handle).
  * openBridgeOrFallback (storage.ts): called `readBridgeMeta` AFTER
    the handle was opened; any error from meta.json parse would bubble
    up leaving the handle open on the caller.

Fixed:
  * openBridgeDbReadOnly now tracks `db` and `conn` separately and
    closes each of them on the error path.
  * openBridgeOrFallback now wraps `readBridgeMeta` in try/catch and
    explicitly calls closeBridgeDb (with .catch() swallow) before
    rethrowing.

HIGH #6 — groupContracts JSON.parse of contract meta
───────────────────────────────────────────────────────────
GroupService.groupContracts() (bridge path) reconstructed StoredContract
from flat rows via `JSON.parse(r.meta)`. A single corrupted meta row
(malformed JSON, non-string leftover from a schema migration) would
abort the whole listing with an unhandled exception.

Replaced with a `safeParseMeta` helper that returns `{}` on parse
failure and increments a `metaParseFailures` counter, which is
surfaced in the return value when non-zero so callers can log and
debug.

═══════════════════════════════════════════════════════════════════════
MEDIUM severity — robustness gaps, input validation, logging
═══════════════════════════════════════════════════════════════════════

MEDIUM #1 — service.groupImpact input range validation + safeLocalImpact
───────────────────────────────────────────────────────────
groupImpact() accepted any finite number for maxDepth, minConfidence,
timeout, crossDepth (including negative, zero, or 1e9). MCP is a
public surface; unbounded values were a DoS vector. Additionally,
`direction: 'upstreem'` (typo) silently became 'upstream'.

All five params are now strictly validated with explicit error messages:
  * direction ∈ {'upstream', 'downstream'}
  * maxDepth    ∈ [1, 10] integer
  * minConfidence ∈ [0, 1]
  * timeout     ∈ [100, 300000] ms
  * crossDepth  ∈ [0, 10] integer

Additionally: the localImpactFn callback called resolveGroupRepo()
which throws if the repo isn't in the group config. runPhase1WithTimeout
only catches timeouts, not rejections, so a missing-repo error would
crash the whole MCP handler. Wrapped in a safeLocalImpact helper that
catches and returns an error-shaped stub instead of bubbling past the
timeout guard.

MEDIUM #2 — Phase-1 timeout reason field
───────────────────────────────────────────────────────────
runGroupImpact()/runGroupImpactLegacy() stub for Phase-1 timeout used
impactedCount: 0, risk: 'LOW', byDepth: {} — indistinguishable from a
verified empty walk. Consumers treating these as real could miss
actual impact.

Added:
  * GroupImpactResult.truncationReason ('phase1_timeout' | 'wall_deadline')
  * phase1TimedOut: true marker inside the `local` stub
  * source repoPath pushed into truncatedRepos immediately on Phase-1
    timeout (was only done post-Phase-2)

MEDIUM #3 — grpc-extractor proto parser
───────────────────────────────────────────────────────────
extractServiceBlocks() used brace-depth counting on the raw .proto
content. Braces inside string literals or `// line` / `/* block */`
comments were counted as real body braces — a valid proto with
`option deprecated_reason = "use NewService { instead"` would have the
service body closed early, dropping methods after the offending string.
Previously marked as a known TODO but never fixed.

Added stripProtoCommentsAndStrings() helper that returns a sanitized
copy with offsets and line breaks preserved (comments/strings replaced
by spaces). extractServiceBlocks now scans the sanitized copy for
service headers and brace depth, but slices the returned body from
the original content to keep source text intact.

MEDIUM #4 — topic-extractor Kafka Go regex
───────────────────────────────────────────────────────────
Sarama patterns anchored on `sarama.NewSyncProducer[\s\S]{0,300}?Topic:`
had two failure modes:
  * In a loop with multiple ProducerMessage literals, only the FIRST
    Topic was captured — the engine's lastIndex advanced past the
    Topic match, and there was no second NewSyncProducer to re-anchor.
  * Multiple unrelated messages within 300 chars of the constructor
    would attribute the wrong topic to the wrong producer context.

Changed the sarama patterns to match the struct literal directly:
`sarama\.ProducerMessage\s*\{[\s\S]{0,200}?Topic:\s*"([^"]+)"`.
This captures every ProducerMessage regardless of producer scope.

MEDIUM #5 — cross-impact crossDepth clamp deduplication
───────────────────────────────────────────────────────────
`Math.min(1, …)` was duplicated at cross-impact.ts:149 (legacy) and
:354 (bridge path) PLUS at service.ts:252. Extracted the clamp into
MAX_SUPPORTED_CROSS_DEPTH with a comment pointing at service.ts so the
two places don't drift when multi-hop traversal lands.

MEDIUM #6 — matching.ts gRPC normalize comment
───────────────────────────────────────────────────────────
No behavior change. The original comment ("MVP: lowercase the whole
token; differs from pkg/method split") falsely implied inconsistency
between the no-slash and pkg/method branches. Rewritten to document
the wire-protocol rationale (pkg/service case-insensitive, RPC method
case-sensitive) and to explicitly note that package-only and
pkg/method ids are intentionally distinct canonical forms.

MEDIUM #7 — manifest-extractor silent catch
───────────────────────────────────────────────────────────
The Cypher executor try/catch dropped errors with `/* fall through */`.
A broken graph query in one repo would silently degrade resolveSymbol
to the synthetic-uid path with no diagnostic output. Added a
`console.warn` with the link type, contract name, repo key, and error
message.

MEDIUM #8 — writeBridgeMeta uses retryRename
───────────────────────────────────────────────────────────
writeBridgeMeta was using `fsp.rename` directly while writeBridge used
retryRename with exponential backoff for EBUSY/EPERM/EACCES. A
concurrent Windows reader could cause the meta.json rename to fail
transiently while the bridge.lbug swap survived. Pulled the rename
through retryRename so the meta write is at least as robust as the
bridge write it accompanies.

MEDIUM #9 — sync.ts extractor failure labels
───────────────────────────────────────────────────────────
The `ExtractorFailure.extractor` enum labeled initLbug failures as
'boundaries' and bridge-write failures as 'manifest' — both confusing
in production logs. Extended the type union with explicit 'init' and
'bridge_write' variants, moved initLbug failures to 'init', moved
writeBridge failure summary to 'bridge_write', and documented each
label kind on the new ExtractorKind type.

MEDIUM #10 — MCP schema bounds for group_impact
───────────────────────────────────────────────────────────
group_impact tool schema declared crossDepth, maxDepth, minConfidence,
timeout as bare `type: 'number'` — clients had no way to discover
valid ranges without trial-and-error. Added JSON Schema draft-7
`minimum`/`maximum` bounds mirroring the server-side validation in
GroupService.groupImpact(). Extended ToolDefinition interface to
allow the new keys.

MEDIUM #11 — typed GroupImpactResult.crossDepthWarning
───────────────────────────────────────────────────────────
service.ts's groupImpact was mutating the result via
`(result as unknown as Record<string, unknown>).crossDepthWarning = …`.
This cast defeated the type system. Added `crossDepthWarning?: string`
to GroupImpactResult in types.ts and removed the double-unknown cast
in both runGroupImpactLegacy and runGroupImpact code paths.

MEDIUM #12 — retryRename unit tests
───────────────────────────────────────────────────────────
retryRename had zero test coverage — critical code for Windows CI,
tested only implicitly via writeBridge on macOS/Linux (where EBUSY
never fires). Exported the function and added four vi.spyOn-based
tests: retries on EBUSY, rethrows non-retryable codes (ENOENT), gives
up after attempt count, retries on EACCES.

═══════════════════════════════════════════════════════════════════════
Intentionally deferred to follow-up PRs
═══════════════════════════════════════════════════════════════════════

  * AbortSignal plumbing for Phase-1 timeout — requires changing the
    port.impact / impactByUid interface across lbug-adapter and
    GroupToolPort. Too broad for a correctness fix batch; tracked
    as follow-up.
  * Duplication between runGroupImpact and runGroupImpactLegacy
    (~500 LOC that share ~70% of logic) — genuine maintenance burden
    but a dedicated refactor PR is the right tool, not this batch.
  * Low-impact items: CLI double-open of bridge, proto parser
    performance optimization, loadPackageDefinition scanner false
    positives, `catch(err: any)` → `unknown` cleanup. All safe to
    land separately.

═══════════════════════════════════════════════════════════════════════
Test deltas (all via --pool=forks, files run individually)
═══════════════════════════════════════════════════════════════════════

  bridge-db.test.ts          15 → 21   (+6: report shape, dropped
                                        links, 4× retryRename)
  sync.test.ts               10 → 11   (+1: initLbug failure path)
  manifest-extractor.test.ts  5 →  8   (+3: exact match, synthetic
                                        uid, regression cases)
  service.test.ts            25 → 31   (+6: range validation,
                                        safeLocalImpact wrap)
  grpc-extractor.test.ts     41 → 43   (+2: proto strings, comments)
  topic-extractor.test.ts    29 → 30   (+1: sarama loop regression)
  matching.test.ts           28 → 28   (comment change only)
  ──────────────────────────────────────
  NEW TESTS:                 +19
  GROUP INTEGRATION:         10/10 green (cli, impact, sync, monorepo)

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
ivkond 2026-04-11 13:32:26 +00:00 • committed by Claude
parent ca81e8748d
commit d15b8cb92e
No known key found for this signature in database
19 changed files with 1396 additions and 272 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

338
gitnexus/src/core/group/bridge-db.ts
View file

@ -76,13 +76,14 @@ export async function closeBridgeDb(handle: BridgeHandle): Promise<void> {
const RETRY_CODES = new Set(['EBUSY', 'EPERM', 'EACCES']);
async function retryRename(src: string, dst: string, attempts = 3): Promise<void> {
export async function retryRename(src: string, dst: string, attempts = 3): Promise<void> {
for (let i = 1; i <= attempts; i++) {
try {
await fsp.rename(src, dst);
return;
} catch (err: any) {
if (!RETRY_CODES.has(err.code) || i === attempts) throw err;
} catch (err: unknown) {
const code = (err as NodeJS.ErrnoException).code;
if (!code || !RETRY_CODES.has(code) || i === attempts) throw err;
await new Promise((r) => setTimeout(r, 100 * Math.pow(2, i - 1)));
}
}
@ -96,7 +97,11 @@ export async function writeBridgeMeta(groupDir: string, meta: BridgeMeta): Promi
const target = path.join(groupDir, 'meta.json');
const tmp = `${target}.tmp.${Date.now()}`;
await fsp.writeFile(tmp, JSON.stringify(meta, null, 2), 'utf-8');
await fsp.rename(tmp, target);
// Use retryRename for consistency with writeBridge's atomic swap — on
// Windows a concurrent reader can cause EBUSY/EPERM even on a tiny
// meta.json, and we don't want meta write to be less robust than the
// bridge.lbug swap it accompanies.
await retryRename(tmp, target);
}
export async function readBridgeMeta(groupDir: string): Promise<BridgeMeta> {
@ -119,7 +124,42 @@ export interface WriteBridgeInput {
missingRepos: string[];
}
export async function writeBridge(groupDir: string, input: WriteBridgeInput): Promise<void> {
/**
* Non-fatal issues encountered during writeBridge. Callers can log these to
* surface partial-success state without aborting the whole sync.
* `sampleErrors` is capped at MAX_SAMPLE_ERRORS per category to bound memory.
*/
export interface WriteBridgeReport {
contractsInserted: number;
contractsFailed: number;
snapshotsInserted: number;
snapshotsFailed: number;
linksInserted: number;
linksFailed: number;
/** Cross-links skipped because their from/to contract nodes weren't found. */
linksDroppedMissingNode: number;
sampleErrors: Array<{
kind: 'contract' | 'snapshot' | 'link';
id: string;
message: string;
}>;
}
const MAX_SAMPLE_ERRORS = 10;
function errMessage(err: unknown): string {
if (err instanceof Error) return err.message;
try {
return String(err);
} catch {
return 'unknown error';
}
}
export async function writeBridge(
groupDir: string,
input: WriteBridgeInput,
): Promise<WriteBridgeReport> {
await fsp.mkdir(groupDir, { recursive: true });
const contracts = dedupeContracts(input.contracts);
const crossLinks = dedupeCrossLinks(input.crossLinks);
@ -128,6 +168,23 @@ export async function writeBridge(groupDir: string, input: WriteBridgeInput): Pr
const tmpPath = path.join(groupDir, 'bridge.lbug.tmp');
const bakPath = path.join(groupDir, 'bridge.lbug.bak');
const report: WriteBridgeReport = {
contractsInserted: 0,
contractsFailed: 0,
snapshotsInserted: 0,
snapshotsFailed: 0,
linksInserted: 0,
linksFailed: 0,
linksDroppedMissingNode: 0,
sampleErrors: [],
};
const recordError = (kind: 'contract' | 'snapshot' | 'link', id: string, err: unknown) => {
if (report.sampleErrors.length < MAX_SAMPLE_ERRORS) {
report.sampleErrors.push({ kind, id, message: errMessage(err) });
}
};
// Clean up any leftover tmp
try {
await fsp.rm(tmpPath, { recursive: true, force: true });
@ -135,16 +192,29 @@ export async function writeBridge(groupDir: string, input: WriteBridgeInput): Pr
/* ignore */
}
// 1. Create temp DB, insert all data
// 1. Create temp DB, insert all data.
//
// Everything after `openBridgeDb` must run inside a try/finally so that
// if ANY step before the explicit `closeBridgeDb` throws — schema
// creation, a contract insert loop that rethrows, a snapshot write, the
// cross-link loop, or anything else — the handle is still released. A
// leaked handle holds the native LadybugDB file lock on tmpPath, which
// (a) leaks a FD and (b) prevents the next writeBridge call from
// reusing the same tmp slot.
const handle = await openBridgeDb(tmpPath);
await ensureBridgeSchema(handle);
let handleClosed = false;
try {
await ensureBridgeSchema(handle);
// Insert contracts
for (const c of contracts) {
const id = contractNodeId(c.repo, c.contractId, c.role, c.symbolRef.filePath);
await queryBridge(
handle,
`CREATE (n:Contract {
// Insert contracts — tolerate individual failures (e.g., a corrupt meta
// that can't be serialized). The whole sync must not fail because one
// contract is broken.
for (const c of contracts) {
const id = contractNodeId(c.repo, c.contractId, c.role, c.symbolRef.filePath);
try {
await queryBridge(
handle,
`CREATE (n:Contract {
id: $id,
contractId: $contractId,
type: $type,
@ -157,99 +227,115 @@ export async function writeBridge(groupDir: string, input: WriteBridgeInput): Pr
confidence: $confidence,
meta: $meta
})`,
{
id,
contractId: c.contractId,
type: c.type,
role: c.role,
repo: c.repo,
service: c.service ?? '',
symbolUid: c.symbolUid,
filePath: c.symbolRef.filePath,
symbolName: c.symbolName,
confidence: c.confidence,
meta: JSON.stringify(c.meta),
},
);
}
{
id,
contractId: c.contractId,
type: c.type,
role: c.role,
repo: c.repo,
service: c.service ?? '',
symbolUid: c.symbolUid,
filePath: c.symbolRef.filePath,
symbolName: c.symbolName,
confidence: c.confidence,
meta: JSON.stringify(c.meta),
},
);
report.contractsInserted++;
} catch (err) {
report.contractsFailed++;
recordError('contract', id, err);
}
}
// Insert repo snapshots
for (const [repoId, snap] of Object.entries(input.repoSnapshots)) {
await queryBridge(
handle,
`CREATE (s:RepoSnapshot {
// Insert repo snapshots
for (const [repoId, snap] of Object.entries(input.repoSnapshots)) {
try {
await queryBridge(
handle,
`CREATE (s:RepoSnapshot {
id: $id,
indexedAt: $indexedAt,
lastCommit: $lastCommit
})`,
{
id: repoId,
indexedAt: snap.indexedAt,
lastCommit: snap.lastCommit,
},
);
}
// Insert cross-links (tolerating missing nodes).
// Use repo-scoped matching: find FROM node by (repo, role=consumer) and TO by (repo, role=provider)
// with symbolRef matching, because link.contractId is the consumer's ID which may differ
// from the provider's contractId (e.g. wildcard consumer vs method-level provider).
const findContractNode = async (
repo: string,
role: 'consumer' | 'provider',
symbolUid: string,
filePath: string,
symbolName: string,
): Promise<string | null> => {
if (symbolUid) {
const uidRows = await queryBridge<{ id: string }>(
handle,
`MATCH (c:Contract) WHERE c.repo = $repo AND c.role = $role
AND c.symbolUid = $symbolUid RETURN c.id AS id LIMIT 1`,
{ repo, role, symbolUid },
);
if (uidRows.length > 0) return uidRows[0].id;
{
id: repoId,
indexedAt: snap.indexedAt,
lastCommit: snap.lastCommit,
},
);
report.snapshotsInserted++;
} catch (err) {
report.snapshotsFailed++;
recordError('snapshot', repoId, err);
}
}
const refRows = await queryBridge<{ id: string }>(
handle,
`MATCH (c:Contract) WHERE c.repo = $repo AND c.role = $role
// Insert cross-links (tolerating missing nodes).
// Use repo-scoped matching: find FROM node by (repo, role=consumer) and TO by (repo, role=provider)
// with symbolRef matching, because link.contractId is the consumer's ID which may differ
// from the provider's contractId (e.g. wildcard consumer vs method-level provider).
const findContractNode = async (
repo: string,
role: 'consumer' | 'provider',
symbolUid: string,
filePath: string,
symbolName: string,
): Promise<string | null> => {
if (symbolUid) {
const uidRows = await queryBridge<{ id: string }>(
handle,
`MATCH (c:Contract) WHERE c.repo = $repo AND c.role = $role
AND c.symbolUid = $symbolUid RETURN c.id AS id LIMIT 1`,
{ repo, role, symbolUid },
);
if (uidRows.length > 0) return uidRows[0].id;
}
const refRows = await queryBridge<{ id: string }>(
handle,
`MATCH (c:Contract) WHERE c.repo = $repo AND c.role = $role
AND c.filePath = $filePath AND c.symbolName = $symbolName
RETURN c.id AS id LIMIT 1`,
{ repo, role, filePath, symbolName },
);
if (refRows.length > 0) return refRows[0].id;
{ repo, role, filePath, symbolName },
);
if (refRows.length > 0) return refRows[0].id;
const fileRows = await queryBridge<{ id: string }>(
handle,
`MATCH (c:Contract) WHERE c.repo = $repo AND c.role = $role
const fileRows = await queryBridge<{ id: string }>(
handle,
`MATCH (c:Contract) WHERE c.repo = $repo AND c.role = $role
AND c.filePath = $filePath
RETURN c.id AS id LIMIT 2`,
{ repo, role, filePath },
);
if (fileRows.length === 1) return fileRows[0].id;
return null;
};
{ repo, role, filePath },
);
if (fileRows.length === 1) return fileRows[0].id;
return null;
};
for (const link of crossLinks) {
const fromId = await findContractNode(
link.from.repo,
'consumer',
link.from.symbolUid,
link.from.symbolRef.filePath,
link.from.symbolRef.name,
);
const toId = await findContractNode(
link.to.repo,
'provider',
link.to.symbolUid,
link.to.symbolRef.filePath,
link.to.symbolRef.name,
);
if (!fromId || !toId) continue;
await queryBridge(
handle,
`
for (const link of crossLinks) {
const linkId = `${link.from.repo}::${link.contractId}->${link.to.repo}::${link.contractId}`;
try {
const fromId = await findContractNode(
link.from.repo,
'consumer',
link.from.symbolUid,
link.from.symbolRef.filePath,
link.from.symbolRef.name,
);
const toId = await findContractNode(
link.to.repo,
'provider',
link.to.symbolUid,
link.to.symbolRef.filePath,
link.to.symbolRef.name,
);
if (!fromId || !toId) {
report.linksDroppedMissingNode++;
continue;
}
await queryBridge(
handle,
`
MATCH (a:Contract), (b:Contract)
WHERE a.id = $fromId AND b.id = $toId
CREATE (a)-[:ContractLink {
@ -260,20 +346,35 @@ export async function writeBridge(groupDir: string, input: WriteBridgeInput): Pr
toRepo: $toRepo
}]->(b)
`,
{
fromId,
toId,
matchType: link.matchType,
confidence: link.confidence,
contractId: link.contractId,
fromRepo: link.from.repo,
toRepo: link.to.repo,
},
);
}
{
fromId,
toId,
matchType: link.matchType,
confidence: link.confidence,
contractId: link.contractId,
fromRepo: link.from.repo,
toRepo: link.to.repo,
},
);
report.linksInserted++;
} catch (err) {
report.linksFailed++;
recordError('link', linkId, err);
}
}
// 2. Close temp DB
await closeBridgeDb(handle);
// 2. Close temp DB (happy path). The finally block also calls
// closeBridgeDb if we threw above; `handleClosed` prevents a
// double-close on the native handle.
await closeBridgeDb(handle);
handleClosed = true;
} finally {
if (!handleClosed) {
await closeBridgeDb(handle).catch(() => {
/* ignore: cleanup path, best effort */
});
}
}
// 3. Atomic swap: old→.bak, tmp→final, rm .bak
try {
@ -295,6 +396,8 @@ export async function writeBridge(groupDir: string, input: WriteBridgeInput): Pr
generatedAt: new Date().toISOString(),
missingRepos: input.missingRepos,
});
return report;
}
/* ------------------------------------------------------------------ */
@ -321,11 +424,30 @@ export async function openBridgeDbReadOnly(groupDir: string): Promise<BridgeHand
return null; // incompatible schema version — fallback to JSON or re-sync
}
// Open the native handle. If Connection construction throws AFTER
// Database was successfully allocated, we'd leak the native Database
// object. Wrap each step separately and tear down the partial handle.
let db: lbug.Database | undefined;
let conn: lbug.Connection | undefined;
try {
const db = new lbug.Database(dbPath, 0, false, true); // readOnly
const conn = new lbug.Connection(db);
db = new lbug.Database(dbPath, 0, false, true); // readOnly
conn = new lbug.Connection(db);
return { _db: db, _conn: conn, groupDir } as BridgeHandle;
} catch {
if (conn) {
try {
await conn.close();
} catch {
/* ignore */
}
}
if (db) {
try {
await db.close();
} catch {
/* ignore */
}
}
return null;
}
}

34
gitnexus/src/core/group/cross-impact.ts
View file

@ -4,6 +4,7 @@ import type {
GroupImpactResult,
CrossRepoImpact,
OutOfScopeLink,
TruncationReason,
} from './types.js';
export interface LegacyGroupImpactOptions {
@ -141,12 +142,17 @@ async function runPhase1WithTimeout(
}
}
// Multi-hop cross-boundary traversal is not implemented yet; both entry points
// enforce the 1-hop limit. When multi-hop lands, remove this clamp here AND in
// GroupService.groupImpact (which surfaces crossDepthWarning to the caller).
const MAX_SUPPORTED_CROSS_DEPTH = 1;
export async function runGroupImpactLegacy(
opts: LegacyGroupImpactOptions,
): Promise<GroupImpactResult> {
const timeout = opts.timeout ?? 30000;
const minConfidence = opts.minConfidence ?? 0.5;
const crossDepth = Math.min(1, opts.crossDepth ?? 1);
const crossDepth = Math.min(MAX_SUPPORTED_CROSS_DEPTH, opts.crossDepth ?? 1);
const tStart = Date.now();
const wallDeadline = tStart + timeout;
@ -160,6 +166,9 @@ export async function runGroupImpactLegacy(
);
let truncated = !localResult.ok;
let truncationReason: TruncationReason | undefined = localResult.ok
? undefined
: 'phase1_timeout';
const local = localResult.ok
? (localResult.v as Record<string, unknown>)
: ({
@ -171,6 +180,9 @@ export async function runGroupImpactLegacy(
affected_processes: [],
affected_modules: [],
byDepth: {},
// Marks the local block as a placeholder produced by the Phase-1 timeout path.
// Consumers should treat zero counts as "unknown" rather than "verified empty".
phase1TimedOut: true,
} as Record<string, unknown>);
const uids = collectPhase1Uids(local);
@ -178,6 +190,9 @@ export async function runGroupImpactLegacy(
const cross: CrossRepoImpact[] = [];
const outOfScope: OutOfScopeLink[] = [];
const truncatedRepos: string[] = [];
if (!localResult.ok) {
truncatedRepos.push(opts.repoPath);
}
const links = [...opts.registry.crossLinks]
.filter((l) => l.confidence >= minConfidence)
@ -200,6 +215,7 @@ export async function runGroupImpactLegacy(
for (const link of applicable) {
if (Date.now() > wallDeadline) {
truncated = true;
truncationReason ??= 'wall_deadline';
break;
}
@ -240,6 +256,7 @@ export async function runGroupImpactLegacy(
if (Date.now() > wallDeadline) {
truncated = true;
truncationReason ??= 'wall_deadline';
truncatedRepos.push(fanOutRepo);
break;
}
@ -261,6 +278,7 @@ export async function runGroupImpactLegacy(
outOfScope,
truncated,
truncatedRepos,
...(truncationReason ? { truncationReason } : {}),
summary: {
direct: summaryLocal.direct ?? 0,
processes_affected: summaryLocal.processes_affected ?? 0,
@ -351,7 +369,7 @@ interface CrossImpactRow {
export async function runGroupImpact(opts: GroupImpactOptions): Promise<GroupImpactResult> {
const timeout = opts.timeout ?? 30000;
const minConfidence = opts.minConfidence ?? 0.5;
const crossDepth = Math.min(1, opts.crossDepth ?? 1);
const crossDepth = Math.min(MAX_SUPPORTED_CROSS_DEPTH, opts.crossDepth ?? 1);
const tStart = Date.now();
const wallDeadline = tStart + timeout;
@ -365,6 +383,9 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise<GroupImp
);
let truncated = !localResult.ok;
let truncationReason: TruncationReason | undefined = localResult.ok
? undefined
: 'phase1_timeout';
const local = localResult.ok
? (localResult.v as Record<string, unknown>)
: ({
@ -376,6 +397,9 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise<GroupImp
affected_processes: [],
affected_modules: [],
byDepth: {},
// Marks the local block as a placeholder produced by the Phase-1 timeout path.
// Consumers should treat zero counts as "unknown" rather than "verified empty".
phase1TimedOut: true,
} as Record<string, unknown>);
const uids = collectPhase1Uids(local);
@ -383,6 +407,9 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise<GroupImp
const cross: CrossRepoImpact[] = [];
const outOfScope: OutOfScopeLink[] = [];
const truncatedRepos: string[] = [];
if (!localResult.ok) {
truncatedRepos.push(opts.repoPath);
}
/* Phase 2 — Cypher bridge query */
const normalizedSubgroup = opts.subgroup?.trim().replace(/\/+$/, '') || null;
@ -427,6 +454,7 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise<GroupImp
if (Date.now() > wallDeadline) {
truncated = true;
truncationReason ??= 'wall_deadline';
break;
}
if (crossDepth < 1) break;
@ -456,6 +484,7 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise<GroupImp
if (Date.now() > wallDeadline) {
truncated = true;
truncationReason ??= 'wall_deadline';
truncatedRepos.push(row.fanOutRepo);
break;
}
@ -477,6 +506,7 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise<GroupImp
outOfScope,
truncated,
truncatedRepos,
...(truncationReason ? { truncationReason } : {}),
summary: {
direct: summaryLocal.direct ?? 0,
processes_affected: summaryLocal.processes_affected ?? 0,

98
gitnexus/src/core/group/extractors/grpc-extractor.ts
View file

@ -25,20 +25,110 @@ function serviceOnlyContractId(serviceName: string): string {
return `grpc::${serviceName}/*`;
}
/**
* Replace all .proto comments and string literals with spaces, preserving the
* original length and character offsets of the input. This lets downstream
* regex / brace-depth parsers run on a "sanitized" copy without having to
* understand proto syntax, while any RegExp.exec/index-based lookups that
* were already positional against `content` continue to work against the
* original string.
*
* Supported comment forms: `// line comment`, `/* block comment * /`.
* Supported strings: double-quoted ("…") and single-quoted ('…') with `\`
* escape handling. Raw/unterminated strings are not supported — we stop
* on a line break for line-style comments and on EOF for unterminated
* strings/blocks, which matches how most real proto files parse.
*/
function stripProtoCommentsAndStrings(content: string): string {
const out = new Array<string>(content.length);
let i = 0;
while (i < content.length) {
const ch = content[i];
const next = content[i + 1];
// Line comment: // ... \n
if (ch === '/' && next === '/') {
out[i] = ' ';
out[i + 1] = ' ';
i += 2;
while (i < content.length && content[i] !== '\n') {
out[i] = content[i] === '\r' ? '\r' : ' ';
i++;
}
continue;
}
// Block comment: /* ... */
if (ch === '/' && next === '*') {
out[i] = ' ';
out[i + 1] = ' ';
i += 2;
while (i < content.length) {
if (content[i] === '*' && content[i + 1] === '/') {
out[i] = ' ';
out[i + 1] = ' ';
i += 2;
break;
}
// Preserve newlines so line numbers stay stable for downstream code.
out[i] = content[i] === '\n' || content[i] === '\r' ? content[i] : ' ';
i++;
}
continue;
}
// String literal: "..." or '...'
if (ch === '"' || ch === "'") {
const quote = ch;
out[i] = ' '; // replace opening quote
i++;
while (i < content.length) {
const c = content[i];
if (c === '\\' && i + 1 < content.length) {
// Skip escaped pair (e.g. \" \n \\)
out[i] = ' ';
out[i + 1] = ' ';
i += 2;
continue;
}
if (c === quote) {
out[i] = ' ';
i++;
break;
}
// Preserve newlines; proto technically disallows unescaped newlines
// inside strings, but real files occasionally have them.
out[i] = c === '\n' || c === '\r' ? c : ' ';
i++;
}
continue;
}
out[i] = ch;
i++;
}
return out.join('');
}
function extractServiceBlocks(content: string): Array<{ name: string; body: string }> {
const results: Array<{ name: string; body: string }> = [];
// v1: brace-depth only — braces inside comments or string literals are not filtered (see spec Fix 2)
// Sanitize comments and string literals so braces inside them don't
// throw off the depth counter. The sanitized copy has the same length
// and offsets as the original, so we use it ONLY to scan for service
// headers and braces; the service body we return is sliced from the
// ORIGINAL content to preserve exact source text for downstream use.
const sanitized = stripProtoCommentsAndStrings(content);
const headerRe = /service\s+(\w+)\s*\{/g;
let headerMatch: RegExpExecArray | null;
while ((headerMatch = headerRe.exec(content)) !== null) {
while ((headerMatch = headerRe.exec(sanitized)) !== null) {
const serviceName = headerMatch[1];
const bodyStart = headerMatch.index + headerMatch[0].length;
let depth = 1;
let pos = bodyStart;
while (pos < content.length && depth > 0) {
const ch = content[pos];
while (pos < sanitized.length && depth > 0) {
const ch = sanitized[pos];
if (ch === '{') depth++;
else if (ch === '}') depth--;
pos++;

44
gitnexus/src/core/group/extractors/http-route-extractor.ts
View file

@ -498,10 +498,13 @@ export class HttpRouteExtractor implements ContractExtractor {
}
const normalized = normalizeHttpPath(pathOnly || '/');
const segments = normalized.split('/').filter(Boolean).map((segment) => {
if (/^\d+$/.test(segment)) return '{param}';
return segment;
});
const segments = normalized
.split('/')
.filter(Boolean)
.map((segment) => {
if (/^\d+$/.test(segment)) return '{param}';
return segment;
});
return `/${segments.join('/')}`.replace(/\/+$/, '') || '/';
}
@ -522,13 +525,16 @@ export class HttpRouteExtractor implements ContractExtractor {
const methodRe = /requests\.(get|post|put|delete|patch)\s*\(\s*['"]([^'"]+)['"]/gi;
let m: RegExpExecArray | null;
while ((m = methodRe.exec(content)) !== null) {
out.push(this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7));
out.push(
this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7),
);
}
const genericRe =
/requests\.request\s*\(\s*['"](\w+)['"]\s*,\s*['"]([^'"]+)['"]/gi;
const genericRe = /requests\.request\s*\(\s*['"](\w+)['"]\s*,\s*['"]([^'"]+)['"]/gi;
while ((m = genericRe.exec(content)) !== null) {
out.push(this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7));
out.push(
this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7),
);
}
return out;
@ -554,14 +560,21 @@ export class HttpRouteExtractor implements ContractExtractor {
/webClient\.method\s*\(\s*HttpMethod\.(GET|POST|PUT|DELETE|PATCH)\s*,\s*['"]([^'"]+)['"]/gi;
let m: RegExpExecArray | null;
while ((m = webClientMethodRe.exec(content)) !== null) {
out.push(this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7));
out.push(
this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7),
);
}
const okHttpRe =
/new\s+Request\.Builder\s*\(\)\s*\.url\s*\(\s*['"]([^'"]+)['"]\s*\)(?:\s*\.\s*method\s*\(\s*['"](\w+)['"])?/gim;
while ((m = okHttpRe.exec(content)) !== null) {
out.push(
this.makeConsumer(filePath, (m[2] || 'GET').toUpperCase(), this.normalizeConsumerPath(m[1]), 0.7),
this.makeConsumer(
filePath,
(m[2] || 'GET').toUpperCase(),
this.normalizeConsumerPath(m[1]),
0.7,
),
);
}
@ -577,15 +590,18 @@ export class HttpRouteExtractor implements ContractExtractor {
out.push(this.makeConsumer(filePath, method, this.normalizeConsumerPath(m[2]), 0.7));
}
const newRequestRe =
/\bhttp\.NewRequest\s*\(\s*['"](\w+)['"]\s*,\s*['"]([^'"]+)['"]/gi;
const newRequestRe = /\bhttp\.NewRequest\s*\(\s*['"](\w+)['"]\s*,\s*['"]([^'"]+)['"]/gi;
while ((m = newRequestRe.exec(content)) !== null) {
out.push(this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7));
out.push(
this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7),
);
}
const restyRe = /\b\w+\.R\(\)\.(Get|Post|Put|Delete|Patch)\s*\(\s*['"]([^'"]+)['"]/gi;
while ((m = restyRe.exec(content)) !== null) {
out.push(this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7));
out.push(
this.makeConsumer(filePath, m[1].toUpperCase(), this.normalizeConsumerPath(m[2]), 0.7),
);
}
return out;

128
gitnexus/src/core/group/extractors/manifest-extractor.ts
View file

@ -6,6 +6,46 @@ export interface ManifestExtractResult {
crossLinks: CrossLink[];
}
/**
* Canonicalize an HTTP path for matching against Route.name in the graph.
* Mirrors core/ingestion/pipeline.ts ensureSlash semantics:
* - Ensures a leading slash.
* - Strips trailing slashes (except the root "/").
* - Normalizes consecutive slashes.
* - Does NOT lowercase (route matching is case-sensitive).
*/
function normalizeRoutePath(raw: string): string {
const trimmed = raw.trim();
if (!trimmed) return '/';
const withLeading = trimmed.startsWith('/') ? trimmed : `/${trimmed}`;
const collapsed = withLeading.replace(/\/+/g, '/');
if (collapsed === '/') return '/';
return collapsed.replace(/\/+$/, '');
}
/**
* Stable synthetic symbolUid for a manifest-declared contract whose target
* symbol could not be resolved against the per-repo graph (resolveSymbol
* returned null). Two reasons we don't leave the uid empty:
*
* 1. The bridge stores Contract nodes keyed in part by symbolUid; an empty
* uid means downstream Cypher queries that anchor on `provider.symbolUid`
* can't tell two different unresolved manifest contracts apart.
* 2. The cross-impact bridge query in cross-impact.ts joins local impact
* results to bridge contracts via `WHERE provider.symbolUid IN $localUids`.
* If the local impact engine produces a deterministic identifier for the
* unresolved target, it must agree with the value the bridge stored. A
* synthetic uid keyed off (repo, contractId) is the only thing both sides
* can derive without knowing about each other.
*
* Format: `manifest::<repo>::<contractId>`. Stable across syncs, scoped to a
* single repo within a group, and never collides with real indexer uids
* (which never start with `manifest::`).
*/
export function manifestSymbolUid(repo: string, contractId: string): string {
return `manifest::${repo}::${contractId}`;
}
export class ManifestExtractor {
async extractFromManifest(
links: GroupManifestLink[],
@ -24,8 +64,10 @@ export class ManifestExtractor {
const consumerSymbol = await this.resolveSymbol(consumerRepo, link, dbExecutors);
const providerRef = providerSymbol || { filePath: '', name: link.contract };
const consumerRef = consumerSymbol || { filePath: '', name: link.contract };
const providerUid = providerSymbol?.uid ?? '';
const consumerUid = consumerSymbol?.uid ?? '';
// When the resolver finds a real graph symbol we keep its uid, otherwise
// fall back to the deterministic synthetic uid (see manifestSymbolUid).
const providerUid = providerSymbol?.uid || manifestSymbolUid(providerRepo, contractId);
const consumerUid = consumerSymbol?.uid || manifestSymbolUid(consumerRepo, contractId);
contracts.push({
contractId,
@ -72,44 +114,77 @@ export class ManifestExtractor {
const executor = dbExecutors?.get(repoPathKey);
if (!executor) return null;
// NOTE: All lookups use EXACT equality on the relevant name field and
// deterministic ORDER BY before LIMIT 1. Previous versions used CONTAINS
// for fuzzy matching (plus an unconditional ".proto" fallback for gRPC)
// which produced silent false positives: e.g. manifest "/orders" would
// match "/suborders", and a gRPC manifest entry in a repo with any
// .proto file would attach to a random proto symbol.
//
// If resolveSymbol returns null, the extractor creates a contract with
// an empty symbolUid/ref — cross-impact still works via name-based
// matching through the `hint` path in runGroupImpact.
try {
let rows: Record<string, unknown>[];
if (link.type === 'http') {
// Route.name is the canonicalized URL path (see
// core/ingestion/pipeline.ts ensureSlash + generateId('Route', ...)).
// Normalize the manifest contract the same way so a user-written
// "/api/orders" matches "api/orders" in the graph.
const normalized = normalizeRoutePath(link.contract);
rows = await executor(
`MATCH (handler)-[r:CodeRelation {type: 'HANDLES_ROUTE'}]->(route:Route)
WHERE route.name CONTAINS $contract
WHERE route.name = $normalized
RETURN handler.id AS uid, handler.name AS name, handler.filePath AS filePath
ORDER BY handler.filePath ASC
LIMIT 1`,
{ contract: link.contract },
{ normalized },
);
} else if (link.type === 'topic') {
rows = await executor(
`MATCH (n) WHERE n.name CONTAINS $contract
`MATCH (n) WHERE n.name = $contract
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
ORDER BY n.filePath ASC
LIMIT 1`,
{ contract: link.contract },
);
} else if (link.type === 'grpc') {
const [serviceName, methodName = ''] = link.contract.split('/');
rows = await executor(
`MATCH (n)
WHERE n.name CONTAINS $serviceName
OR n.name CONTAINS $methodName
OR n.filePath ENDS WITH '.proto'
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
LIMIT 1`,
{ contract: link.contract, serviceName, methodName },
);
// Contract is "Service/Method" or just "Service" (or package.Service
// variants). Prefer matching by method name when present, otherwise
// by service name. NO .proto path fallback — that's guaranteed to
// return a wrong symbol in any repo with more than one proto file.
const parts = link.contract.split('/');
const serviceName = parts[0]?.trim() ?? '';
const methodName = parts[1]?.trim() ?? '';
if (methodName) {
rows = await executor(
`MATCH (n) WHERE n.name = $methodName
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
ORDER BY n.filePath ASC
LIMIT 1`,
{ methodName },
);
} else if (serviceName) {
rows = await executor(
`MATCH (n) WHERE n.name = $serviceName
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
ORDER BY n.filePath ASC
LIMIT 1`,
{ serviceName },
);
} else {
rows = [];
}
} else if (link.type === 'lib') {
const packageName = link.contract.split('/').pop() ?? link.contract;
// 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.
rows = await executor(
`MATCH (n)
WHERE n.name = $contract
OR n.name CONTAINS $packageName
OR n.filePath CONTAINS $packageName
`MATCH (n) WHERE n.name = $contract
RETURN n.id AS uid, n.name AS name, n.filePath AS filePath
ORDER BY n.filePath ASC
LIMIT 1`,
{ contract: link.contract, packageName },
{ contract: link.contract },
);
} else {
return null;
@ -121,8 +196,15 @@ export class ManifestExtractor {
uid: String(rows[0].uid ?? ''),
};
}
} catch {
/* fall through */
} catch (err) {
// Log but don't throw: a broken graph query in one repo shouldn't
// fail the whole manifest extraction. Unresolved contracts still
// get a synthetic symbolUid below, so cross-impact can proceed.
const message = err instanceof Error ? err.message : String(err);
console.warn(
`[manifest-extractor] resolveSymbol failed for ${link.type}:${link.contract} ` +
`in ${repoPathKey}: ${message}`,
);
}
return null;
}

38
gitnexus/src/core/group/extractors/topic-extractor.ts
View file

@ -119,25 +119,37 @@ const KAFKA_PATTERNS: PatternDef[] = [
topicGroup: 1,
symbolName: 'producer.send',
},
// Go: sarama.NewSyncProducer(...); producer.SendMessage(&sarama.ProducerMessage{Topic: "xxx"})
// Go: sarama.ProducerMessage{Topic: "xxx"} struct literal (emitted by
// both NewSyncProducer and NewAsyncProducer client code paths).
//
// Previous pattern was `sarama.NewSyncProducer[\s\S]{0,300}?Topic:...`
// which anchored to the producer constructor and used a 300-char
// lookahead. In a loop like
// producer := sarama.NewSyncProducer(...)
// for _, item := range items {
// msg1 := &sarama.ProducerMessage{Topic: "order.created"}
// msg2 := &sarama.ProducerMessage{Topic: "order.shipped"}
// }
// the regex captured only "order.created" (first Topic after the
// constructor) and silently missed "order.shipped". Matching on the
// struct literal directly fixes both the false negative in loops and
// the spurious cross-message capture when multiple unrelated messages
// sit within 300 chars of the constructor.
{
regex: /sarama\.NewSyncProducer[\s\S]{0,300}?Topic:\s*"([^"]+)"/g,
regex: /sarama\.ProducerMessage\s*\{[\s\S]{0,200}?Topic:\s*"([^"]+)"/g,
role: 'provider',
broker: 'kafka',
confidence: 0.75,
topicGroup: 1,
symbolName: 'sarama.ProducerMessage',
},
// Go: sarama.NewAsyncProducer(...); producer.Input() <- &sarama.ProducerMessage{Topic: "xxx"}
{
regex: /sarama\.NewAsyncProducer[\s\S]{0,300}?Topic:\s*"([^"]+)"/g,
role: 'provider',
broker: 'kafka',
confidence: 0.75,
topicGroup: 1,
symbolName: 'sarama.ProducerMessage',
},
// Go: kafka.Writer{Topic: "xxx"} or kafka.NewWriter(...Topic: "xxx")
// Go: kafka-go writer construction. kafka-go does NOT wrap messages in
// a struct with a Topic field (the writer owns the topic), so we match
// the Writer itself. A 200-char window bridges the gap between
// `kafka.NewWriter(...)` / `kafka.Writer{` and the Topic field inside
// the config literal — kafka-go writer configs are small and rarely
// contain more than one Topic field, so the risk of cross-message
// capture is low here.
{
regex: /kafka\.(?:NewWriter|Writer)\b[\s\S]{0,200}?Topic:\s*"([^"]+)"/g,
role: 'provider',
@ -146,7 +158,7 @@ const KAFKA_PATTERNS: PatternDef[] = [
topicGroup: 1,
symbolName: 'kafka.Writer',
},
// Go: kafka.NewReader(...Topic: "xxx") or kafka.Reader{Topic: "xxx"}
// Go: kafka-go reader construction, mirrors Writer above.
{
regex: /kafka\.(?:NewReader|Reader)\b[\s\S]{0,200}?Topic:\s*"([^"]+)"/g,
role: 'consumer',

24
gitnexus/src/core/group/matching.ts
View file

@ -33,6 +33,22 @@ export function normalizeContractId(id: string): string {
return id;
}
case 'grpc': {
// Canonical form: `grpc::<lowercased-package-or-service>[/<method>]`.
//
// The package/service segment is lowercased because gRPC package
// names are effectively case-insensitive across language bindings
// (`auth.AuthService`, `auth.authservice`, `AUTH.AUTHSERVICE` all
// describe the same wire protocol service). The RPC method segment
// is preserved as-is because the HTTP/2 path used on the wire is
// case-sensitive per the gRPC spec (`/Service/MethodName`), and
// method names in generated clients match the proto source exactly.
//
// A package-only id (no slash) and a package/method id are treated
// as DISTINCT canonical forms: `grpc::userservice` does not match
// `grpc::userservice/Login`. That's by design — callers that want
// service-level manifest matching against method-level providers
// should use the gRPC wildcard form `grpc::UserService/*` which is
// handled by runWildcardMatch below.
const slashIdx = rest.indexOf('/');
if (slashIdx > 0) {
const pkg = rest.substring(0, slashIdx).toLowerCase();
@ -40,12 +56,12 @@ export function normalizeContractId(id: string): string {
return `grpc::${pkg}${method}`;
}
if (slashIdx === 0) {
// Malformed "package/method" with leading slash — do not lowercase the whole string
// (method segment is case-sensitive per spec).
// Malformed "/method" with leading slash — keep as-is so two
// equally malformed ids can still match each other.
return `grpc::${rest}`;
}
// No slash: spec is ambiguous (package-only vs full service.method). MVP: lowercase
// the whole token; differs from pkg/method split above where RPC method keeps case.
// No slash: package/service only. Lowercase to match the package
// segment produced by the pkg/method branch above.
return `grpc::${rest.toLowerCase()}`;
}
case 'topic':

180
gitnexus/src/core/group/service.ts
View file

@ -163,7 +163,24 @@ export class GroupService {
meta: string;
}>(handle, cypher, queryParams as Record<string, import('@ladybugdb/core').LbugValue>);
// Reconstruct StoredContract shape for CLI compatibility
// Reconstruct StoredContract shape for CLI compatibility.
// meta is stored in the bridge as a JSON-stringified blob (see
// writeBridge()). A single corrupted or non-JSON meta row must not
// take down the whole groupContracts() call — degrade to an empty
// object for that row and keep going. The error is swallowed
// intentionally here because there is no per-row logger yet; the
// metaParseFailures counter is returned so callers can surface it.
let metaParseFailures = 0;
const safeParseMeta = (raw: unknown): Record<string, unknown> => {
if (raw == null) return {};
if (typeof raw !== 'string') return raw as Record<string, unknown>;
try {
return JSON.parse(raw) as Record<string, unknown>;
} catch {
metaParseFailures++;
return {};
}
};
let contracts = rawContracts.map((r) => ({
contractId: r.contractId,
type: r.type,
@ -174,7 +191,7 @@ export class GroupService {
symbolRef: { filePath: r.filePath, name: r.symbolName },
symbolName: r.symbolName,
confidence: r.confidence,
meta: typeof r.meta === 'string' ? JSON.parse(r.meta) : r.meta,
meta: safeParseMeta(r.meta),
}));
// Query cross-links
@ -210,7 +227,11 @@ export class GroupService {
contracts = contracts.filter((c) => !matchedIds.has(`${c.repo}::${c.contractId}`));
}
return { contracts, crossLinks };
return {
contracts,
crossLinks,
...(metaParseFailures > 0 ? { metaParseFailures } : {}),
};
} finally {
await closeBridgeDb(handle);
}
@ -224,17 +245,84 @@ export class GroupService {
return { error: 'name, target, and repo are required' };
}
const direction = (params.direction as string) === 'downstream' ? 'downstream' : 'upstream';
const maxDepth =
typeof params.maxDepth === 'number' && Number.isFinite(params.maxDepth) ? params.maxDepth : 3;
const minConfidence =
typeof params.minConfidence === 'number' && Number.isFinite(params.minConfidence)
? params.minConfidence
: 0.5;
const timeout =
typeof params.timeout === 'number' && Number.isFinite(params.timeout)
? params.timeout
: 30000;
// Strict validation for numeric/enum params — MCP is a public interface
// and the worker may be invoked by untrusted LLMs. Bounds are chosen
// conservatively to prevent DoS (unbounded impact walks, long timeouts)
// while still allowing reasonable traversal.
if (
params.direction !== undefined &&
params.direction !== 'upstream' &&
params.direction !== 'downstream'
) {
return {
error: `direction must be 'upstream' or 'downstream', got ${JSON.stringify(params.direction)}`,
};
}
const direction: 'upstream' | 'downstream' =
params.direction === 'downstream' ? 'downstream' : 'upstream';
const maxDepthRaw = params.maxDepth;
if (maxDepthRaw !== undefined) {
if (
typeof maxDepthRaw !== 'number' ||
!Number.isFinite(maxDepthRaw) ||
!Number.isInteger(maxDepthRaw) ||
maxDepthRaw < 1 ||
maxDepthRaw > 10
) {
return {
error: `maxDepth must be an integer in [1, 10], got ${JSON.stringify(maxDepthRaw)}`,
};
}
}
const maxDepth = typeof maxDepthRaw === 'number' ? maxDepthRaw : 3;
const minConfidenceRaw = params.minConfidence;
if (minConfidenceRaw !== undefined) {
if (
typeof minConfidenceRaw !== 'number' ||
!Number.isFinite(minConfidenceRaw) ||
minConfidenceRaw < 0 ||
minConfidenceRaw > 1
) {
return {
error: `minConfidence must be a number in [0, 1], got ${JSON.stringify(minConfidenceRaw)}`,
};
}
}
const minConfidence = typeof minConfidenceRaw === 'number' ? minConfidenceRaw : 0.5;
const timeoutRaw = params.timeout;
if (timeoutRaw !== undefined) {
if (
typeof timeoutRaw !== 'number' ||
!Number.isFinite(timeoutRaw) ||
timeoutRaw < 100 ||
timeoutRaw > 300000
) {
return {
error: `timeout must be a number in [100, 300000] ms, got ${JSON.stringify(timeoutRaw)}`,
};
}
}
const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : 30000;
const crossDepthRaw = params.crossDepth;
if (crossDepthRaw !== undefined) {
if (
typeof crossDepthRaw !== 'number' ||
!Number.isFinite(crossDepthRaw) ||
!Number.isInteger(crossDepthRaw) ||
crossDepthRaw < 0 ||
crossDepthRaw > 10
) {
return {
error: `crossDepth must be an integer in [0, 10], got ${JSON.stringify(crossDepthRaw)}`,
};
}
}
const requestedCrossDepth = typeof crossDepthRaw === 'number' ? crossDepthRaw : 1;
const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined;
const groupDir = getGroupDir(getDefaultGitnexusDir(), name);
@ -244,12 +332,9 @@ export class GroupService {
if (fallback.type === 'none') {
return { error: `No contract data for group "${name}". Run group_sync first.` };
}
const requestedCrossDepth =
typeof params.crossDepth === 'number' && Number.isFinite(params.crossDepth)
? params.crossDepth
: 1;
const crossDepth = Math.min(requestedCrossDepth, 1);
// NOTE: crossDepth is clamped to 1 by runGroupImpact itself
// (MAX_SUPPORTED_CROSS_DEPTH); we only surface a warning here.
const crossDepth = Math.max(0, Math.min(requestedCrossDepth, 1));
const crossDepthWarning =
requestedCrossDepth > 1
? `Multi-hop cross-boundary traversal is not yet implemented. Using --cross-depth 1 (requested: ${requestedCrossDepth}).`
@ -257,6 +342,11 @@ export class GroupService {
const defaultRelTypes = ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS'];
// impactOpts.minConfidence is intentionally 0: it's applied to the
// intra-repo impact walk where edges (CALLS/IMPORTS/EXTENDS/IMPLEMENTS)
// don't carry a meaningful confidence score. The user-facing
// `minConfidence` param filters CROSS-REPO contract links in
// runGroupImpact/runGroupImpactLegacy (see minConfidence passed below).
const impactOpts = {
maxDepth,
relationTypes: defaultRelTypes,
@ -270,6 +360,34 @@ export class GroupService {
return this.port.resolveRepo(registryName);
};
// Wrap localImpactFn callbacks so any exception (e.g. resolveGroupRepo
// throwing on a missing repo) becomes a null/error result instead of
// bubbling past runPhase1WithTimeout (which only catches timeouts, not
// rejections). Without this wrap, an unhandled rejection from the
// callback would crash the MCP tool handler.
const safeLocalImpact = async (t: string, d: string): Promise<unknown> => {
try {
const repoObj = await resolveGroupRepo(repoGroupPath);
return await this.port.impact(repoObj, {
target: t,
direction: d as 'upstream' | 'downstream',
...impactOpts,
});
} catch (err) {
return {
error: `local impact failed: ${err instanceof Error ? err.message : String(err)}`,
target: { id: '', name: t, filePath: '' },
direction: d,
impactedCount: 0,
risk: 'LOW',
summary: { direct: 0, processes_affected: 0, modules_affected: 0 },
affected_processes: [],
affected_modules: [],
byDepth: {},
};
}
};
if (fallback.type === 'json') {
// Legacy JSON path
const result = await runGroupImpactLegacy({
@ -278,14 +396,7 @@ export class GroupService {
repoPath: repoGroupPath,
direction,
registry: fallback.registry,
localImpactFn: async (t: string, d: string) => {
const repoObj = await resolveGroupRepo(repoGroupPath);
return this.port.impact(repoObj, {
target: t,
direction: d as 'upstream' | 'downstream',
...impactOpts,
});
},
localImpactFn: safeLocalImpact,
crossImpactFn: async (targetGroupPath: string, uid: string, d: string) => {
const registryName = config.repos[targetGroupPath];
if (!registryName) return null;
@ -304,7 +415,7 @@ export class GroupService {
});
if (crossDepthWarning) {
(result as unknown as Record<string, unknown>).crossDepthWarning = crossDepthWarning;
result.crossDepthWarning = crossDepthWarning;
}
return result;
}
@ -319,14 +430,7 @@ export class GroupService {
direction,
bridgeQuery: (cypher, p) =>
queryBridge(handle, cypher, p as Record<string, import('@ladybugdb/core').LbugValue>),
localImpactFn: async (t: string, d: string) => {
const repoObj = await resolveGroupRepo(repoGroupPath);
return this.port.impact(repoObj, {
target: t,
direction: d as 'upstream' | 'downstream',
...impactOpts,
});
},
localImpactFn: safeLocalImpact,
crossImpactFn: async (
targetGroupPath: string,
uid: string,
@ -368,7 +472,7 @@ export class GroupService {
});
if (crossDepthWarning) {
(result as unknown as Record<string, unknown>).crossDepthWarning = crossDepthWarning;
result.crossDepthWarning = crossDepthWarning;
}
return result;
} finally {

16
gitnexus/src/core/group/storage.ts
View file

@ -8,7 +8,7 @@ import type {
BridgeMeta,
LegacyContractRegistry,
} from './types.js';
import { openBridgeDbReadOnly, readBridgeMeta } from './bridge-db.js';
import { closeBridgeDb, openBridgeDbReadOnly, readBridgeMeta } from './bridge-db.js';
export function getDefaultGitnexusDir(): string {
return process.env.GITNEXUS_HOME || path.join(os.homedir(), '.gitnexus');
@ -114,8 +114,18 @@ export async function openBridgeOrFallback(
> {
const handle = await openBridgeDbReadOnly(groupDir);
if (handle) {
const meta = await readBridgeMeta(groupDir);
return { type: 'bridge', handle, meta };
// readBridgeMeta has its own try/catch and returns a default when
// meta.json is missing, but defensively guard against any other
// failure so we never leak an opened bridge handle.
try {
const meta = await readBridgeMeta(groupDir);
return { type: 'bridge', handle, meta };
} catch (err) {
await closeBridgeDb(handle).catch(() => {
/* ignore: cleanup path, best effort */
});
throw err;
}
}
// JSON fallback
const registry = await readContractRegistryJson(groupDir);

175
gitnexus/src/core/group/sync.ts
View file

@ -27,12 +27,49 @@ export interface SyncOptions {
skipEmbeddings?: boolean;
}
/**
* Per-repo failure kind captured during syncGroup. A non-empty array on
* the result means at least one repo had something fail mid-pipeline; the
* repo was NOT marked missing (we kept whatever the other steps produced),
* but the user should see these to debug incomplete coverage.
*
* Label meanings:
* - `init` — opening the per-repo LadybugDB pool failed; repo
* gets added to missingRepos and the other steps are
* skipped for that repo.
* - `boundaries` — detectServiceBoundaries() threw; contracts are
* still extracted but without service attribution.
* - `http|grpc|topic` — the named extractor threw; the other extractors
* in the same repo still run.
* - `manifest` — ManifestExtractor.extractFromManifest() threw.
* - `bridge_write` — a non-fatal error inside writeBridge (individual
* contracts/links/snapshots that failed to insert).
* The bridge is still written; `message` includes a
* summary of the partial-failure counts.
*/
export type ExtractorKind =
| 'init'
| 'boundaries'
| 'http'
| 'grpc'
| 'topic'
| 'manifest'
| 'bridge_write';
export interface ExtractorFailure {
repo: string;
extractor: ExtractorKind;
message: string;
}
export interface SyncResult {
contracts: StoredContract[];
crossLinks: CrossLink[];
unmatched: StoredContract[];
missingRepos: string[];
repoSnapshots: Record<string, RepoSnapshot>;
/** Populated when individual extractors threw. See ExtractorFailure. */
extractorFailures?: ExtractorFailure[];
}
export function stableRepoPoolId(entry: RegistryEntry, allEntries: RegistryEntry[]): string {
@ -61,9 +98,19 @@ function defaultResolveHandle(allEntries: RegistryEntry[]) {
};
}
function errMessage(err: unknown): string {
if (err instanceof Error) return err.message;
try {
return String(err);
} catch {
return 'unknown error';
}
}
export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promise<SyncResult> {
const missingRepos: string[] = [];
const repoSnapshots: Record<string, RepoSnapshot> = {};
const extractorFailures: ExtractorFailure[] = [];
let autoContracts: StoredContract[] = [];
let dbExecutors: Map<string, CypherExecutor> | undefined;
let manifestResult: Awaited<ReturnType<ManifestExtractor['extractFromManifest']>>;
@ -91,18 +138,45 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
const poolId = handle.id;
const lbugPath = path.join(handle.storagePath, 'lbug');
// Step 1: open the per-repo LadybugDB pool. Failure here means the
// repo itself is broken/unindexed — mark missing and skip entirely.
try {
await initLbug(poolId, lbugPath);
openPoolIds.push(poolId);
} catch (err) {
missingRepos.push(groupPath);
extractorFailures.push({
repo: groupPath,
extractor: 'init',
message: errMessage(err),
});
continue;
}
const executor: CypherExecutor = (query, params) =>
executeParameterized(poolId, query, params ?? {});
const executor: CypherExecutor = (query, params) =>
executeParameterized(poolId, query, params ?? {});
dbExecutors.set(groupPath, executor);
dbExecutors.set(groupPath, executor);
const boundaries = await detectServiceBoundaries(handle.repoPath);
// Step 2: service boundary detection. Degrade gracefully to empty
// boundaries on failure — contracts will still be extracted, just
// without service attribution.
let boundaries: Awaited<ReturnType<typeof detectServiceBoundaries>> = [];
try {
boundaries = await detectServiceBoundaries(handle.repoPath);
} catch (err) {
extractorFailures.push({
repo: groupPath,
extractor: 'boundaries',
message: errMessage(err),
});
}
if (config.detect.http) {
// Step 3: run each extractor in isolation. One failure must not
// cascade to the others in the same repo.
if (config.detect.http) {
try {
const extracted = await httpEx.extract(executor, handle.repoPath, handle);
for (const c of extracted) {
autoContracts.push({
@ -111,9 +185,17 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
service: assignService(c.symbolRef.filePath, boundaries),
});
}
} catch (err) {
extractorFailures.push({
repo: groupPath,
extractor: 'http',
message: errMessage(err),
});
}
}
if (config.detect.grpc) {
if (config.detect.grpc) {
try {
const extracted = await grpcEx.extract(executor, handle.repoPath, handle);
for (const c of extracted) {
autoContracts.push({
@ -122,9 +204,17 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
service: assignService(c.symbolRef.filePath, boundaries),
});
}
} catch (err) {
extractorFailures.push({
repo: groupPath,
extractor: 'grpc',
message: errMessage(err),
});
}
}
if (config.detect.topics) {
if (config.detect.topics) {
try {
const extracted = await topicEx.extract(executor, handle.repoPath, handle);
for (const c of extracted) {
autoContracts.push({
@ -133,29 +223,46 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
service: assignService(c.symbolRef.filePath, boundaries),
});
}
} catch (err) {
extractorFailures.push({
repo: groupPath,
extractor: 'topic',
message: errMessage(err),
});
}
}
const metaPath = path.join(handle.storagePath, 'meta.json');
try {
const raw = await fs.readFile(metaPath, 'utf-8');
const m = JSON.parse(raw) as { indexedAt?: string; lastCommit?: string };
repoSnapshots[groupPath] = {
indexedAt: m.indexedAt || '',
lastCommit: m.lastCommit || '',
};
} catch {
const e = entries.find((en) => en.name === regName);
repoSnapshots[groupPath] = {
indexedAt: e?.indexedAt || '',
lastCommit: e?.lastCommit || '',
};
}
// Step 4: read repo snapshot meta. Pre-existing fallback is fine.
const metaPath = path.join(handle.storagePath, 'meta.json');
try {
const raw = await fs.readFile(metaPath, 'utf-8');
const m = JSON.parse(raw) as { indexedAt?: string; lastCommit?: string };
repoSnapshots[groupPath] = {
indexedAt: m.indexedAt || '',
lastCommit: m.lastCommit || '',
};
} catch {
missingRepos.push(groupPath);
const e = entries.find((en) => en.name === regName);
repoSnapshots[groupPath] = {
indexedAt: e?.indexedAt || '',
lastCommit: e?.lastCommit || '',
};
}
}
manifestResult = await new ManifestExtractor().extractFromManifest(config.links, dbExecutors);
try {
manifestResult = await new ManifestExtractor().extractFromManifest(
config.links,
dbExecutors,
);
} catch (err) {
extractorFailures.push({
repo: '*',
extractor: 'manifest',
message: errMessage(err),
});
manifestResult = { contracts: [], crossLinks: [] };
}
} finally {
for (const id of [...new Set(openPoolIds)]) {
await closeLbug(id).catch(() => {});
@ -183,12 +290,29 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
]);
if (opts?.groupDir && !opts.skipWrite) {
await writeBridge(opts.groupDir, {
const writeReport = await writeBridge(opts.groupDir, {
contracts: allContracts,
crossLinks,
repoSnapshots,
missingRepos,
});
// Surface per-item write failures as sync-level extractorFailures so the
// user sees them alongside extractor errors. Repo='*' because the error
// is at the bridge layer, not tied to a single source repo.
if (
writeReport.contractsFailed > 0 ||
writeReport.linksFailed > 0 ||
writeReport.snapshotsFailed > 0
) {
const summary =
`bridge write: ${writeReport.contractsFailed} contracts, ` +
`${writeReport.snapshotsFailed} snapshots, ` +
`${writeReport.linksFailed} links failed to insert` +
(writeReport.sampleErrors.length > 0
? `; first error: ${writeReport.sampleErrors[0].kind}[${writeReport.sampleErrors[0].id}]: ${writeReport.sampleErrors[0].message}`
: '');
extractorFailures.push({ repo: '*', extractor: 'bridge_write', message: summary });
}
}
return {
@ -197,5 +321,6 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
unmatched: remaining,
missingRepos,
repoSnapshots,
...(extractorFailures.length > 0 ? { extractorFailures } : {}),
};
}

19
gitnexus/src/core/group/types.ts
View file

@ -96,6 +96,8 @@ export interface RepoHandle {
storagePath: string;
}
export type TruncationReason = 'phase1_timeout' | 'wall_deadline';
export interface GroupImpactResult {
local: unknown;
group: string;
@ -103,6 +105,23 @@ export interface GroupImpactResult {
outOfScope: OutOfScopeLink[];
truncated: boolean;
truncatedRepos: string[];
/**
* Why the result is partial. Absent when `truncated` is false.
* - `phase1_timeout` — local impact walk hit the Phase-1 timeout; Phase-2
* continued with empty local seed, so cross-repo fanout is almost certainly
* empty. Local stub fields (`impactedCount: 0`, `risk: 'LOW'`, `byDepth: {}`)
* are placeholders, NOT real zero-impact results.
* - `wall_deadline` — Phase-2 ran out of wall-clock time while iterating
* cross-link candidates; some results may be present but more could exist.
*/
truncationReason?: TruncationReason;
/**
* Populated when the caller requested a crossDepth greater than the
* MVP-supported max (currently 1). The traversal still runs at the
* supported depth, but the warning is echoed back so the caller (CLI,
* MCP, test) can surface it to the user.
*/
crossDepthWarning?: string;
summary: {
direct: number;
processes_affected: number;

30
gitnexus/src/mcp/tools.ts
View file

@ -18,6 +18,11 @@ export interface ToolDefinition {
default?: any;
items?: { type: string };
enum?: string[];
// Numeric bounds (JSON Schema draft 7). Clients that honor these
// reject out-of-range values before the request hits the server.
// The server still validates server-side — these are belt-and-suspenders.
minimum?: number;
maximum?: number;
}
>;
required: string[];
@ -443,21 +448,40 @@ WHEN TO USE: When a symbol may affect other repos in the same group. Multi-hop c
description: 'upstream or downstream',
enum: ['upstream', 'downstream'],
},
// Numeric bounds mirror the server-side validation in
// GroupService.groupImpact(). Keep them in sync: clients that
// honor the JSON Schema get a client-side rejection before the
// request round-trips, and clients that don't still hit the
// server-side guard.
crossDepth: {
type: 'number',
type: 'integer',
minimum: 0,
maximum: 10,
description:
'Cross-boundary hops (MVP: capped at 1; values above 1 are ignored with a warning)',
},
maxDepth: { type: 'number', description: 'Max graph depth within each repo (default 3)' },
maxDepth: {
type: 'integer',
minimum: 1,
maximum: 10,
description: 'Max graph depth within each repo (default 3)',
},
minConfidence: {
type: 'number',
minimum: 0,
maximum: 1,
description: 'Minimum cross-link confidence (default 0.5)',
},
subgroup: {
type: 'string',
description: 'Only fan out into repos under this group path prefix',
},
timeout: { type: 'number', description: 'Wall-clock budget in ms (default 30000)' },
timeout: {
type: 'integer',
minimum: 100,
maximum: 300000,
description: 'Wall-clock budget in ms (default 30000)',
},
},
required: ['name', 'target', 'repo'],
},

127
gitnexus/test/unit/group/bridge-db.test.ts
View file

@ -1,4 +1,4 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import fsp from 'node:fs/promises';
import path from 'node:path';
import os from 'node:os';
@ -8,6 +8,7 @@ import {
queryBridge,
closeBridgeDb,
contractNodeId,
retryRename,
writeBridge,
openBridgeDbReadOnly,
readBridgeMeta,
@ -140,6 +141,56 @@ describe('writeBridge + read', () => {
expect(exists).toBe(true);
});
it('test_writeBridge_returns_report_with_insert_counts', async () => {
const report = await writeBridge(tmpDir, {
contracts: [makeContract(), makeContract({ repo: 'frontend', role: 'consumer' })],
crossLinks: [],
repoSnapshots: { backend: { indexedAt: '2026-01-01', lastCommit: 'abc' } },
missingRepos: [],
});
expect(report.contractsInserted).toBe(2);
expect(report.contractsFailed).toBe(0);
expect(report.snapshotsInserted).toBe(1);
expect(report.snapshotsFailed).toBe(0);
expect(report.linksInserted).toBe(0);
expect(report.linksFailed).toBe(0);
expect(report.linksDroppedMissingNode).toBe(0);
expect(report.sampleErrors).toHaveLength(0);
});
it('test_writeBridge_counts_dropped_links_with_missing_nodes', async () => {
// Provider + cross-link that references a non-existent consumer node →
// findContractNode returns null for `from`, link gets dropped.
const provider = makeContract({ role: 'provider' });
const report = await writeBridge(tmpDir, {
contracts: [provider],
crossLinks: [
{
from: {
repo: 'ghost',
symbolUid: '',
symbolRef: { filePath: 'nowhere.ts', name: 'ghostFn' },
},
to: {
repo: provider.repo,
symbolUid: provider.symbolUid,
symbolRef: provider.symbolRef,
},
type: 'http',
contractId: provider.contractId,
matchType: 'exact',
confidence: 1.0,
},
],
repoSnapshots: {},
missingRepos: [],
});
expect(report.linksInserted).toBe(0);
expect(report.linksDroppedMissingNode).toBe(1);
expect(report.linksFailed).toBe(0);
expect(report.contractsInserted).toBe(1);
});
it('test_writeBridge_contracts_queryable', async () => {
await writeBridge(tmpDir, {
contracts: [makeContract(), makeContract({ repo: 'frontend', role: 'consumer' })],
@ -341,3 +392,77 @@ describe('writeBridge + read', () => {
expect(meta.missingRepos).toEqual([]);
});
});
describe('retryRename', () => {
afterEach(() => {
vi.restoreAllMocks();
});
it('retries on EBUSY and eventually succeeds', async () => {
// Spy on fs.promises.rename and make the first two attempts fail with
// EBUSY, then succeed on the third. Verifies that Windows-style
// transient rename failures don't immediately bubble up.
const attempts: Array<[string, string]> = [];
let calls = 0;
const spy = vi.spyOn(fsp, 'rename').mockImplementation(async (src, dst) => {
attempts.push([String(src), String(dst)]);
calls++;
if (calls < 3) {
const err = new Error('resource busy or locked') as NodeJS.ErrnoException;
err.code = 'EBUSY';
throw err;
}
// Third attempt: pretend the rename worked.
return undefined;
});
await retryRename('/src/a', '/dst/b', 3);
expect(spy).toHaveBeenCalledTimes(3);
expect(attempts.every(([s, d]) => s === '/src/a' && d === '/dst/b')).toBe(true);
});
it('rethrows non-retryable errors immediately', async () => {
// A non-retryable code (e.g. ENOENT) should NOT be swallowed into a
// retry loop — that would mask real bugs and waste time.
let calls = 0;
vi.spyOn(fsp, 'rename').mockImplementation(async () => {
calls++;
const err = new Error('no such file') as NodeJS.ErrnoException;
err.code = 'ENOENT';
throw err;
});
await expect(retryRename('/src/a', '/dst/b', 5)).rejects.toMatchObject({ code: 'ENOENT' });
expect(calls).toBe(1);
});
it('gives up after the configured number of attempts', async () => {
let calls = 0;
vi.spyOn(fsp, 'rename').mockImplementation(async () => {
calls++;
const err = new Error('locked') as NodeJS.ErrnoException;
err.code = 'EPERM';
throw err;
});
await expect(retryRename('/src/a', '/dst/b', 3)).rejects.toMatchObject({ code: 'EPERM' });
expect(calls).toBe(3);
});
it('retries on EACCES as well', async () => {
let calls = 0;
vi.spyOn(fsp, 'rename').mockImplementation(async () => {
calls++;
if (calls < 2) {
const err = new Error('permission denied') as NodeJS.ErrnoException;
err.code = 'EACCES';
throw err;
}
return undefined;
});
await retryRename('/src/a', '/dst/b', 3);
expect(calls).toBe(2);
});
});

60
gitnexus/test/unit/group/grpc-extractor.test.ts
View file

@ -211,6 +211,66 @@ service IncompleteService {
// The old regex would find partial match; the new parser should skip it
expect(providers).toHaveLength(0);
});
it('test_extract_proto_ignores_braces_inside_string_literals', async () => {
// Regression for a known parser limitation: braces inside string
// literals used to be counted as real service-body braces, which
// would terminate the service early and drop methods after the
// offending string.
writeFile(
'api/strings.proto',
`syntax = "proto3";
package strings;
service TrickyService {
rpc First (Req) returns (Res) {
option (google.api.http).additional_bindings = {
post: "/v1/first";
};
}
// Previously the "{" inside this literal would close the service body.
option deprecated_reason = "use NewService { instead";
rpc Second (Req) returns (Res);
rpc Third (Req) returns (Res);
}
`,
);
const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir));
const protoProviders = contracts.filter(
(c) => c.role === 'provider' && c.symbolRef.filePath === 'api/strings.proto',
);
// All three methods must be extracted even though a string literal
// contains an unbalanced "{".
expect(protoProviders.map((c) => c.symbolName).sort()).toEqual([
'TrickyService.First',
'TrickyService.Second',
'TrickyService.Third',
]);
});
it('test_extract_proto_ignores_braces_inside_comments', async () => {
writeFile(
'api/commented.proto',
`syntax = "proto3";
package commented;
service Svc {
// TODO: move { or } from this comment — parser used to count them
/* A block comment with { unbalanced braces } */
rpc Alpha (Req) returns (Res);
// }} end of the method block (in comment)
rpc Beta (Req) returns (Res);
}
`,
);
const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir));
const protoProviders = contracts.filter(
(c) => c.role === 'provider' && c.symbolRef.filePath === 'api/commented.proto',
);
expect(protoProviders.map((c) => c.symbolName).sort()).toEqual(['Svc.Alpha', 'Svc.Beta']);
});
});
describe('Go server detection', () => {

4
gitnexus/test/unit/group/http-route-extractor.test.ts
View file

@ -335,9 +335,7 @@ class ApiClient {
const contracts = await extractor.extract(null, dir, makeRepo(dir));
const consumers = contracts.filter((c) => c.role === 'consumer');
expect(
consumers.find((c) => c.contractId === 'http::GET::/api/users/{param}'),
).toBeDefined();
expect(consumers.find((c) => c.contractId === 'http::GET::/api/users/{param}')).toBeDefined();
expect(
consumers.find((c) => c.contractId === 'http::PATCH::/api/users/{param}'),
).toBeDefined();

192
gitnexus/test/unit/group/manifest-extractor.test.ts
View file

@ -60,7 +60,7 @@ describe('ManifestExtractor', () => {
expect(result.crossLinks[0].to.repo).toBe('sales/crm/backend');
});
it('resolves grpc manifest links to concrete provider and consumer symbols', async () => {
it('resolves grpc manifest provider by exact method name (no .proto fallback)', async () => {
const links: GroupManifestLink[] = [
{
from: 'platform/orders',
@ -78,27 +78,27 @@ describe('ManifestExtractor', () => {
[
'platform/auth',
async (_cypher, params) => {
if (params?.contract !== 'auth.AuthService/Login') return [];
return [
{
uid: 'uid-auth-login',
name: 'Login',
filePath: 'src/auth.proto',
},
];
// Exact match on method name.
if (params?.methodName === 'Login') {
return [
{
uid: 'uid-auth-login',
name: 'Login',
filePath: 'src/auth.proto',
},
];
}
return [];
},
],
[
'platform/orders',
async (_cypher, params) => {
if (params?.contract !== 'auth.AuthService/Login') return [];
return [
{
uid: 'uid-orders-client',
name: 'AuthServiceClient',
filePath: 'src/client.ts',
},
];
// No symbol with the exact method name — resolve returns null and
// the consumer contract gets an empty symbolUid, falling back to
// name-based hint at cross-impact time.
if (params?.methodName === 'Login') return [];
return [];
},
],
]);
@ -108,15 +108,68 @@ describe('ManifestExtractor', () => {
const provider = result.contracts.find((c) => c.role === 'provider');
const consumer = result.contracts.find((c) => c.role === 'consumer');
// Provider resolved to the concrete proto symbol.
expect(provider?.symbolUid).toBe('uid-auth-login');
expect(provider?.symbolRef.filePath).toBe('src/auth.proto');
expect(consumer?.symbolUid).toBe('uid-orders-client');
expect(consumer?.symbolRef.filePath).toBe('src/client.ts');
// Consumer falls back to a deterministic synthetic uid + name-based ref.
// The synthetic uid lets the bridge cross-impact query anchor on it
// even when the indexer doesn't expose a matching symbol.
expect(consumer?.symbolUid).toBe('manifest::platform/orders::grpc::auth.AuthService/Login');
expect(consumer?.symbolRef.name).toBe('auth.AuthService/Login');
expect(result.crossLinks[0].to.symbolRef.filePath).toBe('src/auth.proto');
expect(result.crossLinks[0].from.symbolRef.filePath).toBe('src/client.ts');
expect(result.crossLinks[0].from.symbolUid).toBe(
'manifest::platform/orders::grpc::auth.AuthService/Login',
);
});
it('resolves lib manifest links to concrete provider and consumer symbols', async () => {
it('does NOT resolve grpc manifest to an arbitrary .proto file', async () => {
// Regression test for a previous bug: the extractor had an unconditional
// `OR n.filePath ENDS WITH '.proto'` fallback that returned the first
// proto symbol in the repo, regardless of whether it matched the contract.
const links: GroupManifestLink[] = [
{
from: 'platform/orders',
to: 'platform/auth',
type: 'grpc',
contract: 'auth.AuthService/Login',
role: 'consumer',
},
];
const dbExecutors = new Map<
string,
(cypher: string, params?: Record<string, unknown>) => Promise<Record<string, unknown>[]>
>([
[
'platform/auth',
// Executor returns matches for ANY query (simulates the old buggy
// fallback that returned a random .proto file). The new code must
// only accept a hit when the method/service name matches exactly.
async (_cypher, params) => {
if (params?.methodName === 'Login' || params?.serviceName === 'auth.AuthService') {
return [
{
uid: 'uid-correct-login',
name: 'Login',
filePath: 'src/auth.proto',
},
];
}
return [];
},
],
['platform/orders', async () => []],
]);
const result = await extractor.extractFromManifest(links, dbExecutors);
const provider = result.contracts.find((c) => c.role === 'provider');
// Must resolve to the correct symbol (not a random proto one).
expect(provider?.symbolUid).toBe('uid-correct-login');
});
it('resolves lib manifest links by exact name only', async () => {
const links: GroupManifestLink[] = [
{
from: 'platform/web',
@ -148,13 +201,7 @@ describe('ManifestExtractor', () => {
'platform/web',
async (_cypher, params) => {
if (params?.contract !== '@platform/contracts') return [];
return [
{
uid: 'uid-importer',
name: 'contractsClient',
filePath: 'src/app.ts',
},
];
return [];
},
],
]);
@ -165,9 +212,92 @@ describe('ManifestExtractor', () => {
const consumer = result.contracts.find((c) => c.role === 'consumer');
expect(provider?.symbolUid).toBe('uid-lib');
expect(consumer?.symbolUid).toBe('uid-importer');
expect(result.crossLinks[0].to.symbolUid).toBe('uid-lib');
expect(result.crossLinks[0].from.symbolUid).toBe('uid-importer');
// Consumer doesn't have a symbol named exactly '@platform/contracts' —
// exact matching returns null, falling back to the synthetic manifest uid.
expect(consumer?.symbolUid).toBe('manifest::platform/web::lib::@platform/contracts');
});
it('does NOT resolve lib manifest via CONTAINS on name', async () => {
// Regression test: previous CONTAINS fallback would match "react" to
// "react-native" or "@types/react". Exact matching must reject both.
const links: GroupManifestLink[] = [
{
from: 'web',
to: 'packages/ui',
type: 'lib',
contract: 'react',
role: 'consumer',
},
];
const dbExecutors = new Map<
string,
(cypher: string, params?: Record<string, unknown>) => Promise<Record<string, unknown>[]>
>([
[
'packages/ui',
async (_cypher, params) => {
// Executor is called with contract='react'. Only exact matches
// should come back; return only wrong candidates to verify the
// Cypher uses `=` not `CONTAINS`.
if (params?.contract === 'react') {
// Simulated DB returns nothing because it has only "react-native"
// and "@types/react" — neither is an exact match for "react".
return [];
}
return [];
},
],
['web', async () => []],
]);
const result = await extractor.extractFromManifest(links, dbExecutors);
const provider = result.contracts.find((c) => c.role === 'provider');
// No exact match → synthetic manifest uid, not a wrong real one.
expect(provider?.symbolUid).toBe('manifest::packages/ui::lib::react');
});
it('normalizes http contract path for exact Route.name match', async () => {
// Manifest may be written as "/api/orders/" or "api/orders"; both should
// match the canonical "/api/orders" stored in the graph.
const variants = ['/api/orders', '/api/orders/', 'api/orders', '//api//orders'];
for (const raw of variants) {
const links: GroupManifestLink[] = [
{
from: 'gateway',
to: 'orders-svc',
type: 'http',
contract: raw,
role: 'consumer',
},
];
let seenParam: string | undefined;
const dbExecutors = new Map<
string,
(cypher: string, params?: Record<string, unknown>) => Promise<Record<string, unknown>[]>
>([
[
'orders-svc',
async (_cypher, params) => {
seenParam = params?.normalized as string;
return [
{
uid: 'uid-orders-list',
name: 'listOrders',
filePath: 'src/orders.ts',
},
];
},
],
['gateway', async () => []],
]);
const result = await extractor.extractFromManifest(links, dbExecutors);
expect(seenParam).toBe('/api/orders');
const provider = result.contracts.find((c) => c.role === 'provider');
expect(provider?.symbolUid).toBe('uid-orders-list');
}
});
it('returns empty for no links', async () => {

90
gitnexus/test/unit/group/service.test.ts
View file

@ -546,6 +546,96 @@ describe('GroupService', () => {
const result = (await svc.groupImpact({})) as { error: string };
expect(result.error).toContain('name, target, and repo are required');
});
it('test_groupImpact_rejects_unknown_direction', async () => {
const svc = new GroupService(makePort());
const result = (await svc.groupImpact({
name: 'x',
target: 'x',
repo: 'x',
direction: 'upstreem', // typo
})) as { error: string };
expect(result.error).toMatch(/direction must be/);
});
it('test_groupImpact_rejects_out_of_range_maxDepth', async () => {
const svc = new GroupService(makePort());
for (const bad of [-1, 0, 11, 1000, 1.5, Number.NaN]) {
const result = (await svc.groupImpact({
name: 'x',
target: 'x',
repo: 'x',
maxDepth: bad,
})) as { error?: string };
expect(result.error).toMatch(/maxDepth must be/);
}
});
it('test_groupImpact_rejects_out_of_range_minConfidence', async () => {
const svc = new GroupService(makePort());
for (const bad of [-0.1, 1.1, -5, 10]) {
const result = (await svc.groupImpact({
name: 'x',
target: 'x',
repo: 'x',
minConfidence: bad,
})) as { error?: string };
expect(result.error).toMatch(/minConfidence must be/);
}
});
it('test_groupImpact_rejects_out_of_range_timeout', async () => {
const svc = new GroupService(makePort());
for (const bad of [0, 99, 300001, 1e9]) {
const result = (await svc.groupImpact({
name: 'x',
target: 'x',
repo: 'x',
timeout: bad,
})) as { error?: string };
expect(result.error).toMatch(/timeout must be/);
}
});
it('test_groupImpact_rejects_out_of_range_crossDepth', async () => {
const svc = new GroupService(makePort());
for (const bad of [-1, 11, 1.5, Number.NaN]) {
const result = (await svc.groupImpact({
name: 'x',
target: 'x',
repo: 'x',
crossDepth: bad,
})) as { error?: string };
expect(result.error).toMatch(/crossDepth must be/);
}
});
it('test_groupImpact_wraps_localImpactFn_exception_from_missing_repo', async () => {
// If the configured repoGroupPath is not in the group's config, the
// resolveGroupRepo helper throws. That exception must NOT bubble past
// runPhase1WithTimeout — it should be caught inside safeLocalImpact
// and surfaced as a local.error field on the result.
const { groupDir, cleanup, tmpDir } = makeTmpGroup();
try {
vi.stubEnv('GITNEXUS_HOME', tmpDir);
await writeContractRegistryJson(
groupDir,
makeRegistry([makeContract('http::GET::/api/x', 'provider', 'app/backend')]),
);
const svc = new GroupService(makePort());
const result = (await svc.groupImpact({
name: 'test-group',
target: 'whatever',
repo: 'not/in/config',
})) as { local?: { error?: string } };
// Should not throw; instead local.error is populated.
expect(result).toBeDefined();
expect(result.local?.error).toMatch(/local impact failed/);
} finally {
vi.unstubAllEnvs();
cleanup();
}
});
});
describe('groupStatus', () => {

43
gitnexus/test/unit/group/sync.test.ts
View file

@ -225,6 +225,49 @@ describe('syncGroup', () => {
}
});
it('reports initLbug failures via extractorFailures and marks repo missing', async () => {
const config = makeConfig({
'app/backend': 'backend-repo',
'app/frontend': 'frontend-repo',
});
const { vi } = await import('vitest');
const poolAdapter = await import('../../../src/core/lbug/pool-adapter.js');
const initSpy = vi.spyOn(poolAdapter, 'initLbug').mockImplementation(async (id: string) => {
if (id === 'app-backend') throw new Error('lbug corruption: CRC mismatch');
});
const closeSpy = vi.spyOn(poolAdapter, 'closeLbug').mockResolvedValue(undefined);
try {
const result = await syncGroup(config, {
resolveRepoHandle: async (_name, groupPath) => ({
id: groupPath.replace(/\//g, '-'),
path: groupPath,
repoPath: '/tmp/' + groupPath,
storagePath: '/tmp/' + groupPath + '/.gitnexus',
}),
skipWrite: true,
}).catch(() => undefined);
expect(result).toBeDefined();
// app/backend should be missing (initLbug threw) AND reported in
// extractorFailures so the user can see the real reason.
expect(result!.missingRepos).toContain('app/backend');
expect(result!.extractorFailures).toBeDefined();
const failure = result!.extractorFailures!.find((f) => f.repo === 'app/backend');
expect(failure).toBeDefined();
expect(failure!.message).toMatch(/CRC mismatch/);
// initLbug failure should be labeled 'init', not 'boundaries'
// (which is reserved for detectServiceBoundaries failures).
expect(failure!.extractor).toBe('init');
// app/frontend init succeeded — it must NOT be in missingRepos.
expect(result!.missingRepos).not.toContain('app/frontend');
} finally {
initSpy.mockRestore();
closeSpy.mockRestore();
}
});
it('writes bridge.lbug to groupDir when skipWrite is false', async () => {
const tmpDir = path.join(os.tmpdir(), `gitnexus-sync-write-${Date.now()}`);
fs.mkdirSync(tmpDir, { recursive: true });

28
gitnexus/test/unit/group/topic-extractor.test.ts
View file

@ -353,6 +353,34 @@ producer.Input() <- &sarama.ProducerMessage{Topic: "inventory.update"}`,
expect(producers[0].meta.broker).toBe('kafka');
});
it('test_extract_sarama_producer_in_loop_captures_all_topics', async () => {
// Regression: a for loop that constructs multiple ProducerMessage
// literals inside a single NewSyncProducer scope. The previous
// regex anchored on NewSyncProducer and captured only the first
// Topic within 300 chars, silently dropping the rest.
writeFile(
'internal/multi-publisher.go',
`package publisher
func publishAll(producer sarama.SyncProducer, items []Item) error {
_, _ = sarama.NewSyncProducer(brokers, cfg)
for _, item := range items {
msg1 := &sarama.ProducerMessage{Topic: "order.created"}
msg2 := &sarama.ProducerMessage{Topic: "order.shipped"}
_ = msg1
_ = msg2
}
return nil
}`,
);
const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir));
const producers = contracts.filter((c) => c.role === 'provider');
const topics = producers.map((c) => c.contractId).sort();
// Both topics must appear (exact set match to catch any duplicates).
expect(topics).toEqual(['topic::order.created', 'topic::order.shipped']);
});
it('test_extract_kafka_go_writer_returns_provider', async () => {
writeFile(
'internal/writer.go',