diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index ebd82dcf9..7bb7bc180 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -435,17 +435,27 @@ export function registerGroupCommands(program: Command): void { // 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( - 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)', - ); + // Keyed on the REASON, not on which incidental fact happens to be + // non-empty. `truncatedRepos` is populated for a structural gap too + // — the bridge's incomplete repos are unioned into it even when ZERO + // crossings were attempted — so branching on its length first + // reported "fan-out stopped early" for a run where nothing stopped + // early, and omitted the only remedy that works. Same false-cause + // shape the contract listing was just re-gated for, one command over. + const floorReason = (): string => { + if (reason === 'suppressed-stage') { + return 'the last sync skipped a matching stage (--exact-only); re-run `gitnexus group sync` without it for the complete graph'; + } + if (reason === 'incomplete-sync') { + return dropped.length > 0 + ? `the last sync could not account for ${dropped.join(', ')}; their contracts are absent from every query against this bridge — re-run \`gitnexus group sync\`` + : 'the last sync could not say which repos it read — re-run `gitnexus group sync`'; + } + return dropped.length > 0 + ? `fan-out stopped early; crossings to ${dropped.length} repo(s) not traversed: ${dropped.join(', ')}` + : 'the local impact walk did not complete (every bridge crossing was traversed)'; + }; + console.log(` risk is a LOWER BOUND — ${floorReason()}`); } } } finally { diff --git a/gitnexus/src/core/group/bridge-db.ts b/gitnexus/src/core/group/bridge-db.ts index f6d549ba4..3c8c07bf2 100644 --- a/gitnexus/src/core/group/bridge-db.ts +++ b/gitnexus/src/core/group/bridge-db.ts @@ -656,7 +656,15 @@ export async function closeBridgeDb(handle: BridgeHandle): Promise { /* ------------------------------------------------------------------ */ export async function writeBridgeMeta(groupDir: string, meta: BridgeMeta): Promise { - await writeFileAtomic(path.join(groupDir, 'meta.json'), JSON.stringify(meta, null, 2)); + // Strip the reader-only fields HERE rather than at each writer. `readBridgeMeta` + // sets both on what it returns, so any caller that reads-modifies-writes would + // round-trip them to disk — and `pairedWithDatabase` is the poisonous one: + // persisted, it tells every future reader the pair was verified when nothing + // verified it. That rule used to live in the body of the only such caller, + // which held exactly as long as there was one. There are now three writers and + // two of them read first. Enforced at the boundary, no writer can get it wrong. + const { repoListsUnreadable: _reader1, pairedWithDatabase: _reader2, ...persisted } = meta; + await writeFileAtomic(path.join(groupDir, 'meta.json'), JSON.stringify(persisted, null, 2)); } /** @@ -938,10 +946,8 @@ export async function refreshPreservedBridgeMeta( const refreshed: BridgeMeta = { ...existing, ...diagnostics }; // NEVER PERSISTED (see `BridgeMeta`): both are things a READER computes ABOUT // a file, and this is the first code in the repo that reads metadata and - // writes it back. `pairedWithDatabase` is the poisonous one — persisted, it - // would tell every future reader that the pair had been verified. - delete refreshed.repoListsUnreadable; - delete refreshed.pairedWithDatabase; + // writes it back. The strip itself now lives in `writeBridgeMeta`, so every + // writer inherits it rather than each remembering. if (paired) { const stat = await fsp.stat(dbPath).catch(() => null); diff --git a/gitnexus/src/core/group/completeness.ts b/gitnexus/src/core/group/completeness.ts index 430638d90..16aee0473 100644 --- a/gitnexus/src/core/group/completeness.ts +++ b/gitnexus/src/core/group/completeness.ts @@ -99,20 +99,6 @@ export type CrossRepoCompleteness = TruncationFields & { incompleteRepos: string[]; }; -/** - * The ONE computation of "is this cross-repo answer complete?" (KTD10). - * - * Three surfaces can return a partial cross-repo answer — impact, trace, and - * the contract listing — and each used to decide for itself, in its own - * vocabulary, which is how two of them ended up saying it in prose only. The - * answer is the same structured triple `GroupImpactResult` already carries, so - * an agent reading any of them learns "complete" vs "floor" the same way. - * - * `truncationFields` derives `riskEpistemic` from `truncated` mechanically, and - * is reused here rather than re-implemented for the same reason it exists: the - * 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. * @@ -130,6 +116,20 @@ export function recordedMatchStages(value: unknown): MatchType[] | undefined { return value.every((v): v is MatchType => known.includes(v as MatchType)) ? value : undefined; } +/** + * The ONE computation of "is this cross-repo answer complete?" (KTD10). + * + * Three surfaces can return a partial cross-repo answer — impact, trace, and + * the contract listing — and each used to decide for itself, in its own + * vocabulary, which is how two of them ended up saying it in prose only. The + * answer is the same structured triple `GroupImpactResult` already carries, so + * an agent reading any of them learns "complete" vs "floor" the same way. + * + * `truncationFields` derives `riskEpistemic` from `truncated` mechanically, and + * is reused here rather than re-implemented for the same reason it exists: the + * 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). + */ 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 60e265163..19ddd6fee 100644 --- a/gitnexus/src/core/group/cross-impact.ts +++ b/gitnexus/src/core/group/cross-impact.ts @@ -893,7 +893,9 @@ export async function runGroupImpact( ? 'timeout' : runtimeTruncated ? 'partial' - : ((bridge.truncated ? bridge.truncationReason : undefined) ?? 'incomplete-sync'), + : bridge.truncated + ? bridge.truncationReason + : 'incomplete-sync', ), truncatedRepos: [...new Set([...truncatedRepos, ...bridge.incompleteRepos])], summary: { diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index ecde38a3e..7efd65ef7 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -816,16 +816,16 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis // withdraws the claim without touching the database — the previous // graph stays queryable, it just stops being called complete. const withdrawn = await markBridgeProvenanceUnknown(groupDir); + const provenanceNote = withdrawn + ? 'Its metadata has been marked provenance-unknown, so those answers now report as ' + + 'a lower bound rather than as complete.' + : 'Its metadata could NOT be marked provenance-unknown, so those answers may still ' + + 'report as complete despite describing an older sync.'; logger.warn( { err: msg, groupDir, bridgeProvenanceWithdrawn: withdrawn }, '⚠️ writeBridge failed; contracts.json is intact and is the canonical copy, ' + 'but bridge.lbug was not replaced: cross-repo queries may still answer from ' + - "the previous sync's contracts. " + - (withdrawn - ? 'Its metadata has been marked provenance-unknown, so those answers now ' + - 'report as a lower bound rather than as complete. ' - : 'Its metadata could NOT be marked provenance-unknown, so those answers may ' + - 'still report as complete despite describing an older sync. ') + + `the previous sync's contracts. ${provenanceNote} ` + 'Re-run `gitnexus group sync` to retry.', ); } diff --git a/gitnexus/test/unit/group/registry-suppressed-stages.test.ts b/gitnexus/test/unit/group/registry-suppressed-stages.test.ts index 7a9b5430d..b2eb2bd20 100644 --- a/gitnexus/test/unit/group/registry-suppressed-stages.test.ts +++ b/gitnexus/test/unit/group/registry-suppressed-stages.test.ts @@ -162,27 +162,15 @@ describe('cross-repo completeness reflects a suppressed stage', () => { }); /** - * The reason has to be REACHABLE, not just documented. + * The reason must stay a DECLARED member of the union agents branch on. * - * 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. + * That it is reachable from real code is already pinned above, by a case that + * drives `crossRepoCompleteness` and gets the value back. What that cannot see + * is the union itself shrinking: a guard in `tools.test.ts` asserts every + * member is documented, so dropping a member keeps that guard green while every + * consumer silently loses the value. */ 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. diff --git a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts index af4718731..12a85439c 100644 --- a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts +++ b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts @@ -978,18 +978,39 @@ describe('the warning after a failed bridge write', () => { // reports the new registry — two public surfaces, contradictory claims, // out of one sync. Marking provenance unknown withdraws the completeness // claim without deleting a graph still useful as a floor. - await writeBridgeMeta(groupDir, { - version: 1, - generatedAt: '2026-01-01T00:00:00.000Z', - missingRepos: [], - unreadableRepos: [], - }); + // `unreadableRepos` is deliberately UNREADABLE here, not merely empty: that + // is what makes `readBridgeMeta` set the reader-only `repoListsUnreadable` + // on what it returns, so the assertion below can actually catch a + // read-modify-write writer round-tripping it back to disk. Seeded with a + // valid list the check passes whether or not the strip exists. + fs.writeFileSync( + path.join(groupDir, 'meta.json'), + JSON.stringify({ + version: 1, + generatedAt: '2026-01-01T00:00:00.000Z', + missingRepos: [], + unreadableRepos: 'not-a-list', + }), + ); + expect((await readBridgeMeta(groupDir)).repoListsUnreadable).toBe(true); expect((await readBridgeMeta(groupDir)).provenanceUnknown).toBeUndefined(); writeBridgeFailure = new Error('ENOSPC: no space left on device'); await runSync(); expect((await readBridgeMeta(groupDir)).provenanceUnknown).toBe(true); + + // ...and the withdrawal must not persist the reader-only fields. + // `readBridgeMeta` sets both on what it returns, so a read-modify-write + // writer round-trips them unless the write boundary strips them. + // `pairedWithDatabase` is the poisonous one: persisted, it would tell every + // later reader the pair was verified when nothing verified it. + const raw = JSON.parse(fs.readFileSync(path.join(groupDir, 'meta.json'), 'utf8')) as Record< + string, + unknown + >; + expect(raw).not.toHaveProperty('pairedWithDatabase'); + expect(raw).not.toHaveProperty('repoListsUnreadable'); }); // control: a sync whose bridge write SUCCEEDS must not withdraw provenance —