diff --git a/gitnexus/src/core/group/bridge-db.ts b/gitnexus/src/core/group/bridge-db.ts index 32e045463..27446a0ab 100644 --- a/gitnexus/src/core/group/bridge-db.ts +++ b/gitnexus/src/core/group/bridge-db.ts @@ -76,13 +76,14 @@ export async function closeBridgeDb(handle: BridgeHandle): Promise { const RETRY_CODES = new Set(['EBUSY', 'EPERM', 'EACCES']); -async function retryRename(src: string, dst: string, attempts = 3): Promise { +export async function retryRename(src: string, dst: string, attempts = 3): Promise { 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 { @@ -119,7 +124,42 @@ export interface WriteBridgeInput { missingRepos: string[]; } -export async function writeBridge(groupDir: string, input: WriteBridgeInput): Promise { +/** + * 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 { 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 => { - 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 => { + 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 { 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) : ({ @@ -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); 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 { 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) : ({ @@ -376,6 +397,9 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise); const uids = collectPhase1Uids(local); @@ -383,6 +407,9 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise wallDeadline) { truncated = true; + truncationReason ??= 'wall_deadline'; break; } if (crossDepth < 1) break; @@ -456,6 +484,7 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise wallDeadline) { truncated = true; + truncationReason ??= 'wall_deadline'; truncatedRepos.push(row.fanOutRepo); break; } @@ -477,6 +506,7 @@ export async function runGroupImpact(opts: GroupImpactOptions): Promise(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++; diff --git a/gitnexus/src/core/group/extractors/http-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index e1ab77e8b..2a9f662c3 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -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; diff --git a/gitnexus/src/core/group/extractors/manifest-extractor.ts b/gitnexus/src/core/group/extractors/manifest-extractor.ts index 3609b8aa6..09817b356 100644 --- a/gitnexus/src/core/group/extractors/manifest-extractor.ts +++ b/gitnexus/src/core/group/extractors/manifest-extractor.ts @@ -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::::`. 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[]; 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; } diff --git a/gitnexus/src/core/group/extractors/topic-extractor.ts b/gitnexus/src/core/group/extractors/topic-extractor.ts index d09129e2b..3d5280d86 100644 --- a/gitnexus/src/core/group/extractors/topic-extractor.ts +++ b/gitnexus/src/core/group/extractors/topic-extractor.ts @@ -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', diff --git a/gitnexus/src/core/group/matching.ts b/gitnexus/src/core/group/matching.ts index 2b648abe1..ec793968b 100644 --- a/gitnexus/src/core/group/matching.ts +++ b/gitnexus/src/core/group/matching.ts @@ -33,6 +33,22 @@ export function normalizeContractId(id: string): string { return id; } case 'grpc': { + // Canonical form: `grpc::[/]`. + // + // 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': diff --git a/gitnexus/src/core/group/service.ts b/gitnexus/src/core/group/service.ts index 18ba1518c..8d532f680 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -163,7 +163,24 @@ export class GroupService { meta: string; }>(handle, cypher, queryParams as Record); - // 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 => { + if (raw == null) return {}; + if (typeof raw !== 'string') return raw as Record; + try { + return JSON.parse(raw) as Record; + } 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 => { + 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).crossDepthWarning = crossDepthWarning; + result.crossDepthWarning = crossDepthWarning; } return result; } @@ -319,14 +430,7 @@ export class GroupService { direction, bridgeQuery: (cypher, p) => queryBridge(handle, cypher, p as Record), - 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).crossDepthWarning = crossDepthWarning; + result.crossDepthWarning = crossDepthWarning; } return result; } finally { diff --git a/gitnexus/src/core/group/storage.ts b/gitnexus/src/core/group/storage.ts index aefab11b9..7d48cfdb1 100644 --- a/gitnexus/src/core/group/storage.ts +++ b/gitnexus/src/core/group/storage.ts @@ -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); diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index 62581e363..f53a14671 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -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; + /** 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 { const missingRepos: string[] = []; const repoSnapshots: Record = {}; + const extractorFailures: ExtractorFailure[] = []; let autoContracts: StoredContract[] = []; let dbExecutors: Map | undefined; let manifestResult: Awaited>; @@ -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> = []; + 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 } : {}), }; } diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index 5a1c38cdc..8795a3995 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -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; diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 52ff08329..8f5fa1410 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -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'], }, diff --git a/gitnexus/test/unit/group/bridge-db.test.ts b/gitnexus/test/unit/group/bridge-db.test.ts index 56f78c49e..31add36bd 100644 --- a/gitnexus/test/unit/group/bridge-db.test.ts +++ b/gitnexus/test/unit/group/bridge-db.test.ts @@ -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); + }); +}); diff --git a/gitnexus/test/unit/group/grpc-extractor.test.ts b/gitnexus/test/unit/group/grpc-extractor.test.ts index 51af6a62b..82d79cbd6 100644 --- a/gitnexus/test/unit/group/grpc-extractor.test.ts +++ b/gitnexus/test/unit/group/grpc-extractor.test.ts @@ -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', () => { diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index c9db110f2..653b4952c 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -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(); diff --git a/gitnexus/test/unit/group/manifest-extractor.test.ts b/gitnexus/test/unit/group/manifest-extractor.test.ts index 19b0654dc..c2c67a33a 100644 --- a/gitnexus/test/unit/group/manifest-extractor.test.ts +++ b/gitnexus/test/unit/group/manifest-extractor.test.ts @@ -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) => Promise[]> + >([ + [ + '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) => Promise[]> + >([ + [ + '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) => Promise[]> + >([ + [ + '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 () => { diff --git a/gitnexus/test/unit/group/service.test.ts b/gitnexus/test/unit/group/service.test.ts index eeb33ea45..940c79d1c 100644 --- a/gitnexus/test/unit/group/service.test.ts +++ b/gitnexus/test/unit/group/service.test.ts @@ -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', () => { diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index 7219ea1e7..0ecb7f982 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -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 }); diff --git a/gitnexus/test/unit/group/topic-extractor.test.ts b/gitnexus/test/unit/group/topic-extractor.test.ts index 38d8a11f9..bf821de63 100644 --- a/gitnexus/test/unit/group/topic-extractor.test.ts +++ b/gitnexus/test/unit/group/topic-extractor.test.ts @@ -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',