diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index c99c4e895..0ab72c06b 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -434,10 +434,17 @@ export function registerGroupCommands(program: Command): void { // repos — reporting it as crossings understates a fan-out cap the // same way #2787's totals did. const dropped = (raw as { truncatedRepos?: string[] })?.truncatedRepos ?? []; + const reason = (raw as { truncationReason?: string })?.truncationReason; + // A floor caused by the flag is not a floor caused by the walk. + // Reported together, an operator re-runs the query or repairs a + // repo and gets the identical answer, because neither is the fix. console.log( - dropped.length > 0 - ? ` risk is a LOWER BOUND — fan-out stopped early; crossings to ${dropped.length} repo(s) not traversed: ${dropped.join(', ')}` - : ' risk is a LOWER BOUND — the local impact walk did not complete (every bridge crossing was traversed)', + reason === 'suppressed-stage' + ? ' risk is a LOWER BOUND — the last sync skipped a matching stage (--exact-only);' + + ' re-run `gitnexus group sync` without it for the complete graph' + : dropped.length > 0 + ? ` risk is a LOWER BOUND — fan-out stopped early; crossings to ${dropped.length} repo(s) not traversed: ${dropped.join(', ')}` + : ' risk is a LOWER BOUND — the local impact walk did not complete (every bridge crossing was traversed)', ); } } @@ -530,6 +537,7 @@ export function registerGroupCommands(program: Command): void { unreadableRepos, missingRepos, suppressedMatchStages, + truncationReason, } = raw as { contracts: Array<{ role: string; @@ -546,6 +554,7 @@ export function registerGroupCommands(program: Command): void { }>; truncated?: boolean; suppressedMatchStages?: string[]; + truncationReason?: string; unreadableRepos?: string[]; missingRepos?: string[]; }; @@ -581,7 +590,15 @@ export function registerGroupCommands(program: Command): void { `\n Re-run \`gitnexus group sync\` without --exact-only for the complete set.`, ); } - if (truncated) { + // Gated on the REASON, not just the flag. A suppressed stage sets + // `truncated` with both repo lists empty, which sent this block down + // its else-branch and printed "the last sync did not record which + // repos it could read" — a false statement, with the wrong remedy, + // about a sync that recorded them fine. The suppressed-stage warning + // above already said the true thing. When a repo gap co-occurs the + // reason is 'incomplete-sync' (the repo side takes precedence in + // `crossRepoCompleteness`), so this block still runs for it. + if (truncated && truncationReason !== 'suppressed-stage') { // Counts above are a floor, not a census. Name the repos when the // registry recorded them, and say so plainly when it did not — a // listing that cannot say what it is missing is still incomplete. diff --git a/gitnexus/src/core/group/bridge-db.ts b/gitnexus/src/core/group/bridge-db.ts index 0216cc051..0d9adfab3 100644 --- a/gitnexus/src/core/group/bridge-db.ts +++ b/gitnexus/src/core/group/bridge-db.ts @@ -12,7 +12,7 @@ import type { MatchType, } from './types.js'; import { BRIDGE_SCHEMA_QUERIES, BRIDGE_SCHEMA_VERSION } from './bridge-schema.js'; -import { recordedRepoList } from './completeness.js'; +import { recordedMatchStages, recordedRepoList } from './completeness.js'; import { closeLbugConnection, openLbugConnection, @@ -833,14 +833,8 @@ export async function readBridgeMeta(groupDir: string): Promise { // was there and could not be read. if (unreadableRepos) meta.unreadableRepos = unreadableRepos; else delete meta.unreadableRepos; - // Same absent-vs-empty rule, and all-or-nothing: an unreadable list is "not - // recorded", never "measured, nothing suppressed". - const known = ['exact', 'manifest', 'wildcard']; - const suppressed = Array.isArray(raw.suppressedMatchStages) - ? raw.suppressedMatchStages.every((v) => typeof v === 'string' && known.includes(v)) - ? (raw.suppressedMatchStages as MatchType[]) - : undefined - : undefined; + // Same absent-vs-empty rule, through the one shared reader. + const suppressed = recordedMatchStages(raw.suppressedMatchStages); if (suppressed) meta.suppressedMatchStages = suppressed; else delete meta.suppressedMatchStages; if (repoListsUnreadable) meta.repoListsUnreadable = true; @@ -919,11 +913,13 @@ async function fileExists(filePath: string): Promise { */ export async function refreshPreservedBridgeMeta( groupDir: string, - diagnostics: { - missingRepos: string[]; - unreadableRepos: string[]; - suppressedMatchStages?: MatchType[]; - }, + // Deliberately NOT `suppressedMatchStages`. This path preserves an EARLIER + // sync's database, so stamping it with this run's request would claim the + // untouched bridge was built with a flag it never saw. The registry's own + // preserve write (`{ ...prior, missingRepos, unreadableRepos }`) omits it for + // exactly this reason, and the two artifacts have to agree about which run + // they describe. + diagnostics: { missingRepos: string[]; unreadableRepos: string[] }, ): Promise { const dbPath = path.join(groupDir, 'bridge.lbug'); const [metaOnDisk, dbOnDisk] = await Promise.all([ diff --git a/gitnexus/src/core/group/completeness.ts b/gitnexus/src/core/group/completeness.ts index f542a2def..430638d90 100644 --- a/gitnexus/src/core/group/completeness.ts +++ b/gitnexus/src/core/group/completeness.ts @@ -13,7 +13,7 @@ * Nothing here imports anything but types. Keep it that way: the moment this * file gains a runtime import, every consumer pays for it again. */ -import type { GroupImpactTruncationReason } from './types.js'; +import type { GroupImpactTruncationReason, MatchType } from './types.js'; /** * A union rather than `Pick` so the two states are @@ -113,6 +113,23 @@ export type CrossRepoCompleteness = TruncationFields & { * marker that says "this is a floor, not a verdict" may never drift away from * the flag that says the answer was cut short (#2787). */ +/** + * Read a persisted `suppressedMatchStages` list. + * + * Sibling of `recordedRepoList` and here for the same stated reason: it had + * lived in two files verbatim, so tightening one would silently leave the other. + * All-or-nothing like its sibling — a stale member (this repo has already + * retired `'bm25'` and `'embedding'`) makes the whole list unreadable rather + * than filtering down to `[]`, which on this field would mean "measured, + * nothing suppressed": a clean answer manufactured from a value we could not + * read. + */ +export function recordedMatchStages(value: unknown): MatchType[] | undefined { + if (!Array.isArray(value)) return undefined; + const known: MatchType[] = ['exact', 'manifest', 'wildcard']; + return value.every((v): v is MatchType => known.includes(v as MatchType)) ? value : undefined; +} + export function crossRepoCompleteness(input: CrossRepoCompletenessInput): CrossRepoCompleteness { const incompleteRepos = [ ...new Set([...(input.unreadableRepos ?? []), ...(input.missingRepos ?? [])]), diff --git a/gitnexus/src/core/group/cross-impact.ts b/gitnexus/src/core/group/cross-impact.ts index be3b6b921..60e265163 100644 --- a/gitnexus/src/core/group/cross-impact.ts +++ b/gitnexus/src/core/group/cross-impact.ts @@ -879,15 +879,21 @@ export async function runGroupImpact( // and under-reporting a blast radius is the unsafe direction (an agent told // LOW proceeds; told CRITICAL it stops). Marking the floor keeps the // warning intact while making the incompleteness legible. - // Runtime limits first — they are what the caller can retry. 'incomplete-sync' - // is the remaining cause once nothing was merely cut short, and its remedy is - // a different one: re-run `gitnexus group sync`, not the query. Computed - // inline because `truncationFields` reads the reason ONLY on the truncated - // branch — naming it in a variable invited reading it on the complete path, - // where it would say 'incomplete-sync' about a complete result. + // Runtime limits first — they are what the caller can retry. Past those, the + // BRIDGE's own reason wins: it already distinguished an unreadable repo + // ('incomplete-sync', remedy: re-sync) from a stage the sync was asked to + // skip ('suppressed-stage', remedy: re-sync WITHOUT the flag). Hardcoding + // the fallback here overrode that and told every caller to repair a repo + // that read fine — and made the second value unreachable from this surface + // while the tool description promised it. `cross-trace.ts` re-spreads the + // bridge's fields for the same reason. ...truncationFields( truncated, - fanoutTimedOut ? 'timeout' : runtimeTruncated ? 'partial' : 'incomplete-sync', + fanoutTimedOut + ? 'timeout' + : runtimeTruncated + ? 'partial' + : ((bridge.truncated ? bridge.truncationReason : undefined) ?? 'incomplete-sync'), ), truncatedRepos: [...new Set([...truncatedRepos, ...bridge.incompleteRepos])], summary: { diff --git a/gitnexus/src/core/group/service.ts b/gitnexus/src/core/group/service.ts index 24a767fbb..9cd2bb1e7 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -15,7 +15,7 @@ import { type RepoMeta, } from '../../storage/repo-manager.js'; import { crossRepoCompleteness } from './completeness.js'; -import { recordedRepoList } from './completeness.js'; +import { recordedMatchStages, recordedRepoList } from './completeness.js'; import { GroupNotFoundError, loadGroupConfig } from './config-parser.js'; import { fileMatchesServicePrefix, @@ -33,7 +33,6 @@ import type { CrossLink, GroupConfig, GroupContextResult, - MatchType, StoredContract, } from './types.js'; @@ -361,26 +360,6 @@ async function loadContractRegistryResilient( return { ok: true, registry, skippedCorrupt }; } -/** - * Read a persisted `suppressedMatchStages` list. - * - * Deliberately NOT `recordedRepoList`: that validates `string[]`, which is - * right for repo names and one notch too weak here. This repo has already - * retired MatchType members ('bm25', 'embedding'), so a stale value on disk is - * a real shape — dropping non-members keeps an unknown stage name from - * reaching a caller typed as a live one. Absence stays absence (tri-state). - */ -function recordedMatchStages(value: unknown): MatchType[] | undefined { - if (!Array.isArray(value)) return undefined; - const known: MatchType[] = ['exact', 'manifest', 'wildcard']; - // All-or-nothing, exactly like `recordedRepoList`. Filtering would be worse - // than useless here: a stale `['bm25']` would survive as `[]`, which on this - // field MEANS "measured, nothing was suppressed" — a confident clean answer - // manufactured from a value we could not read, on the one field that exists - // to stop that conflation. An unreadable list is "not recorded". - return value.every((v): v is MatchType => known.includes(v as MatchType)) ? value : undefined; -} - /** * Validate a boolean MCP parameter — reject, never coerce. * diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index cb11129de..02d4e2843 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -762,11 +762,7 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis // confident wrong answer, and swallowing its failure would reinstate the // very fail-open it closes. `writeContractRegistry` above is unguarded for // the same reason, into the same directory. - await refreshPreservedBridgeMeta(groupDir, { - missingRepos, - unreadableRepos, - suppressedMatchStages, - }); + await refreshPreservedBridgeMeta(groupDir, { missingRepos, unreadableRepos }); } if (!everyRepoFailed) { diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index d58a13a79..3411878bd 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -112,7 +112,11 @@ export function getResourceTemplates(): ResourceTemplate[] { 'three-state, and an ABSENT key is not an empty one: absent means the last sync never ' + 'recorded which repos it could read (provenance unknown — treat cross-repo answers for ' + 'this group as a floor), an empty list means the sync measured none, and a populated list ' + - 'names the repos whose contracts are missing from the registry.', + 'names the repos whose contracts are missing from the registry. suppressedMatchStages is ' + + 'three-state the same way: absent is a registry predating the field, an empty list means ' + + 'the sync skipped no matching stage, and a populated list names stages it was ASKED to ' + + 'skip — those cross-link counts are a lower bound by request, and the remedy is to re-sync ' + + 'without that flag rather than to repair a repo.', mimeType: 'text/yaml', }, ]; diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index db4c7a83d..a9aa36048 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -854,7 +854,7 @@ WHEN TO USE: Discover groups before group_sync. Optional "name" returns a single WHEN TO USE: After changing group.yaml or re-indexing member repos. -READ THE RESULT: \`missingRepos\` are configured repos with no entry in the registry (index them, or drop them from group.yaml); \`unreadableRepos\` ARE registered but this sync could not extract from them — the index would not open (version skew, lock, corruption), or an extractor failed partway — so NONE of their contracts are in this sync and a following group_impact / group_contracts is a lower bound, not a verdict. \`registryOutcome\` says what happened to the file, and the three values a call here can return each need a different response: 'written' — this run's contracts replaced contracts.json; 'preserved' — nothing could be read, so contracts.json was rewritten keeping the previous sync's contracts and cross-links verbatim and refreshing only \`missingRepos\`/\`unreadableRepos\` to describe THIS run (the file changed, the contracts in it did not, and they are as old as the last sync that succeeded); 'superseded' — nothing could be read, and another sync replaced contracts.json while this one waited for the group lock; that file was left untouched and this run's lists were NOT recorded, because they describe an older group state than what is on disk (so the registry is fresher than this response's diagnostics, not staler); 'no-prior-registry' — nothing could be read AND there was no previous contracts.json to carry forward, so none was written and this group has no contract registry on disk. Only 'no-prior-registry' means there is nothing to read: after it, group_contracts / group_impact have no registry at all rather than a stale one, so fix the repos above and re-run before trusting either.\n\nPARAMETERS ARE VALIDATED: \`exactOnly\` must be a real boolean — the string "false" is rejected, not coerced to true. The retired \`skipEmbeddings\` and \`allowStale\` parameters are refused by name; drop them from the call.`, +READ THE RESULT: \`missingRepos\` are configured repos with no entry in the registry (index them, or drop them from group.yaml); \`unreadableRepos\` ARE registered but this sync could not extract from them — the index would not open (version skew, lock, corruption), or an extractor failed partway — so NONE of their contracts are in this sync and a following group_impact / group_contracts is a lower bound, not a verdict. \`registryOutcome\` says what happened to the file, and the three values a call here can return each need a different response: 'written' — this run's contracts replaced contracts.json; 'preserved' — nothing could be read, so contracts.json was rewritten keeping the previous sync's contracts and cross-links verbatim and refreshing only \`missingRepos\`/\`unreadableRepos\` to describe THIS run (the file changed, the contracts in it did not, and they are as old as the last sync that succeeded); 'superseded' — nothing could be read, and another sync replaced contracts.json while this one waited for the group lock; that file was left untouched and this run's lists were NOT recorded, because they describe an older group state than what is on disk (so the registry is fresher than this response's diagnostics, not staler); 'no-prior-registry' — nothing could be read AND there was no previous contracts.json to carry forward, so none was written and this group has no contract registry on disk. Only 'no-prior-registry' means there is nothing to read: after it, group_contracts / group_impact have no registry at all rather than a stale one, so fix the repos above and re-run before trusting either. \`suppressedMatchStages\` names matching stages this sync was ASKED to skip, with the same three states as the repo lists: ABSENT means a registry written before the field existed, \`[]\` means this sync suppressed nothing, and a populated list means the cross-link set is a lower bound BY REQUEST — a later group_impact / group_contracts on it reports truncationReason 'suppressed-stage'.\n\nPARAMETERS ARE VALIDATED: \`exactOnly\` must be a real boolean — the string "false" is rejected, not coerced to true. The retired \`skipEmbeddings\` and \`allowStale\` parameters are refused by name; drop them from the call.`, // Usually writes contracts.json, so conservatively non-idempotent even // though output is deterministic for identical input. When no configured // repo could be read it still rewrites the file, keeping the previous diff --git a/gitnexus/test/unit/group/registry-suppressed-stages.test.ts b/gitnexus/test/unit/group/registry-suppressed-stages.test.ts index 6bdfd1856..7a9b5430d 100644 --- a/gitnexus/test/unit/group/registry-suppressed-stages.test.ts +++ b/gitnexus/test/unit/group/registry-suppressed-stages.test.ts @@ -24,6 +24,7 @@ import path from 'node:path'; import { syncGroup } from '../../../src/core/group/sync.js'; import { makeWildcardPair } from './fixtures.js'; import { crossRepoCompleteness } from '../../../src/core/group/completeness.js'; +import { GROUP_IMPACT_TRUNCATION_REASONS } from '../../../src/core/group/types.js'; import type { GroupConfig, StoredContract, @@ -159,3 +160,32 @@ describe('cross-repo completeness reflects a suppressed stage', () => { expect(out.truncationReason).toBe('incomplete-sync'); }); }); + +/** + * The reason has to be REACHABLE, not just documented. + * + * A guard in `tools.test.ts` asserts every truncation reason is explained in + * the impact tool description. That check passed while `runGroupImpact` + * hardcoded its fallback and could never emit `'suppressed-stage'` — a + * documented value no surface could produce. These pin the emitting side. + */ +describe('the suppressed-stage reason is reachable, not just documented', () => { + it('is the reason when a stage was suppressed and every repo read fine', () => { + const out = crossRepoCompleteness({ + unreadableRepos: [], + missingRepos: [], + suppressedMatchStages: ['wildcard'], + provenanceUnknown: false, + inScope: () => true, + }); + + expect(out.truncated).toBe(true); + expect(out.truncationReason).toBe('suppressed-stage'); + }); + + it('is a declared member of the reason union agents branch on', () => { + // If a future change drops it from the union, the tool description guard + // would still pass while every consumer lost the value. + expect(GROUP_IMPACT_TRUNCATION_REASONS).toContain('suppressed-stage'); + }); +}); diff --git a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts index 0aab17922..f9568d6e1 100644 --- a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts +++ b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts @@ -329,6 +329,55 @@ describe('syncGroup with an unreadable index', () => { expect(onDisk.unreadableRepos).toEqual(['app/backend']); }); + it('keeps the prior suppressedMatchStages instead of stamping this run request', async () => { + // The preserved registry describes an EARLIER sync. If this run's request + // were stamped onto it, a graph narrowed by `--exact-only` would be + // relabelled complete the moment a later plain sync failed to read + // anything — and `group_impact` reads exactly that field to decide whether + // its answer is a floor. + initLbugMock.mockRejectedValue(new Error(LBUG_VERSION_ERROR)); + + const contractsPath = path.join(groupDir, 'contracts.json'); + fs.writeFileSync( + contractsPath, + JSON.stringify({ ...PRIOR_REGISTRY, suppressedMatchStages: ['wildcard'] }), + ); + + // This run asks for NO suppression, and fails to read anything. + const result = await syncGroup(makeConfig({ 'app/backend': 'backend-repo' }), { groupDir }); + + expect(result.registryOutcome).toBe('preserved'); + // The result describes THIS run — it really did suppress nothing. + expect(result.suppressedMatchStages).toEqual([]); + + const onDisk = JSON.parse(fs.readFileSync(contractsPath, 'utf8')) as Record; + // The file still describes the sync that produced its contracts. + expect(onDisk.suppressedMatchStages).toEqual(['wildcard']); + }); + + it('does not stamp this run request onto the preserved bridge metadata', async () => { + // Same property one artifact over. `bridge.lbug` is untouched on this path, + // so its meta.json must keep describing the sync that built it; otherwise + // contracts.json, meta.json and the database describe three different runs. + initLbugMock.mockRejectedValue(new Error(LBUG_VERSION_ERROR)); + + fs.writeFileSync(path.join(groupDir, 'contracts.json'), JSON.stringify(PRIOR_REGISTRY)); + await writeBridgeMeta(groupDir, { + version: 1, + generatedAt: '2026-01-01T00:00:00.000Z', + missingRepos: [], + unreadableRepos: [], + suppressedMatchStages: ['wildcard'], + }); + + await syncGroup(makeConfig({ 'app/backend': 'backend-repo' }), { groupDir }); + + const meta = await readBridgeMeta(groupDir); + expect(meta.suppressedMatchStages).toEqual(['wildcard']); + // ...while the diagnostics describing THIS run are refreshed, as before. + expect(meta.unreadableRepos).toEqual(['app/backend']); + }); + it('writes nothing at all when there is no previous registry to preserve', async () => { initLbugMock.mockRejectedValue(new Error(LBUG_VERSION_ERROR));