fix(group): stop cross-repo impact and trace claiming a narrowed graph is complete

Closes the half of the suppressed-stage finding that was deferred. The reviewers
were right that deferring it was the weak point: the motivating harm was named
as `group_impact` and cross-repo `trace` traversing a graph missing real edges,
and those were exactly the surfaces left uncovered.

The deferral rested on an assumption that does not hold. "It is already blind to
this, so we do not make it worse" is false: `--exact-only` was inert before this
PR, so the number of narrowed registries in the world goes from zero to nonzero
exactly when this lands. The blindness was harmless only while narrowing was
impossible. And silence there is not neutral -- `cross-impact.ts` documents
`truncated: false` as an affirmative completeness claim, so those tools were
about to start asserting a complete answer over a knowingly short graph.

`suppressedMatchStages` now rides the bridge the same way `unreadableRepos`
does: persisted in meta.json (no BRIDGE_SCHEMA_VERSION bump -- meta fields have
this precedent), read back all-or-nothing, and carried across the preserve path
through `refreshPreservedBridgeMeta`'s diagnostics so a preserved bridge keeps
the marker of the sync that actually built it.

`crossRepoCompleteness` folds it in, which is what makes this one change reach
all three surfaces -- that function is by design the ONE computation behind the
truncation triple. Precedence is explicit: an unreadable or unaccounted repo
outranks a suppressed stage, because it is the more serious structural gap and
its remedy has to be the one reported.

`'suppressed-stage'` is a new member of the truncation-reason union rather than
a reuse of `'incomplete-sync'`. The earlier decision not to touch that union was
about not conflating remedies -- telling an agent to repair a repo that read
fine, for a narrowing it requested. A distinct member preserves that reasoning
while letting the answer stop claiming completeness, which is what reusing the
existing member would have destroyed.

The union's guard test did its job: adding a member failed the check that every
reason is explained on the agent-facing surface, so the impact tool description
now names this one and its distinct remedy (re-run WITHOUT the flag; nothing
failed to read).

`group status` and its CLI renderer surface it too, on the populated case only
-- absent is a registry predating the field and empty is the ordinary clean
sync; neither earns a line.

Deliberately still not done, and why: a repo-wide unknown-parameter layer for
every MCP tool. Five parameters are read by backends and declared in no schema
(`subgroupExact`, `unmatchedOnly`, `showClusters`, `showProcesses`, and
`verbose` until this branch declared it), and three tools dispatch with no
schema entry at all, so a strict layer rejects working calls until each is
reconciled. That reconciliation is the work; the layer is the cheap part. It
also cannot produce the "was removed and is no longer accepted" message the
retired-parameter guard exists to give.

tsc clean; eslint 0 errors (2 pre-existing warnings); 1159/1159 across the group
unit, group integration and tool-schema suites.
This commit is contained in:
Gergo Magyar 2026-08-27 15:20:43 +00:00
parent 8f7c25dd3e
commit 6ceac8b1f6
10 changed files with 150 additions and 6 deletions

View file

@ -135,6 +135,7 @@ export function registerGroupCommands(program: Command): void {
>;
missingRepos?: string[];
unreadableRepos?: string[];
suppressedMatchStages?: string[];
};
console.log(' Repo index / contracts staleness:');
@ -189,6 +190,18 @@ export function registerGroupCommands(program: Command): void {
if ((st.missingRepos || []).length > 0) {
console.log(`\n Last sync missing repos: ${st.missingRepos!.join(', ')}`);
}
// Only the populated case prints. Absent means a registry that predates
// the field, and empty is the ordinary clean sync — neither is worth a
// line, whereas a narrowed registry changes how every later answer
// should be read.
const skippedStages = st.suppressedMatchStages ?? [];
if (skippedStages.length > 0) {
console.log(
`\n Last sync skipped matching stages: ${skippedStages.join(', ')}` +
`\n Cross-links those stages would have found are absent by request.` +
`\n Re-run \`gitnexus group sync\` without --exact-only for the complete set.`,
);
}
} finally {
await backend.dispose().catch(() => {});
}

View file

@ -3,7 +3,14 @@ import path from 'node:path';
import { createHash } from 'node:crypto';
import lbug from '@ladybugdb/core';
import type { LbugValue } from '@ladybugdb/core';
import type { BridgeHandle, BridgeMeta, StoredContract, CrossLink, RepoSnapshot } from './types.js';
import type {
BridgeHandle,
BridgeMeta,
StoredContract,
CrossLink,
RepoSnapshot,
MatchType,
} from './types.js';
import { BRIDGE_SCHEMA_QUERIES, BRIDGE_SCHEMA_VERSION } from './bridge-schema.js';
import { recordedRepoList } from './completeness.js';
import {
@ -826,6 +833,16 @@ export async function readBridgeMeta(groupDir: string): Promise<BridgeMeta> {
// 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;
if (suppressed) meta.suppressedMatchStages = suppressed;
else delete meta.suppressedMatchStages;
if (repoListsUnreadable) meta.repoListsUnreadable = true;
return meta;
}
@ -902,7 +919,11 @@ async function fileExists(filePath: string): Promise<boolean> {
*/
export async function refreshPreservedBridgeMeta(
groupDir: string,
diagnostics: { missingRepos: string[]; unreadableRepos: string[] },
diagnostics: {
missingRepos: string[];
unreadableRepos: string[];
suppressedMatchStages?: MatchType[];
},
): Promise<PreservedBridgeMetaOutcome> {
const dbPath = path.join(groupDir, 'bridge.lbug');
const [metaOnDisk, dbOnDisk] = await Promise.all([
@ -969,6 +990,13 @@ export interface WriteBridgeInput {
* contract those repos own.
*/
unreadableRepos?: string[];
/**
* Matching stages the sync was asked to skip. Recorded here for the same
* reason `unreadableRepos` is: a later cross-repo query reads this bridge
* with no access to the run that built it, and a graph narrowed by request
* looks exactly like a complete one.
*/
suppressedMatchStages?: MatchType[];
}
/**
@ -1322,6 +1350,9 @@ export async function writeBridgeUnlocked(
// different claim from a bridge that never recorded the field. Omitted
// only when the caller passed nothing to record.
...(input.unreadableRepos ? { unreadableRepos: input.unreadableRepos } : {}),
...(input.suppressedMatchStages
? { suppressedMatchStages: input.suppressedMatchStages }
: {}),
});
return report;

View file

@ -69,6 +69,12 @@ export interface CrossRepoCompletenessInput {
*/
unreadableRepos?: readonly string[];
missingRepos?: readonly string[];
/**
* Matching stages the sync was asked to skip. Absent or empty means it
* suppressed none; a populated list makes the answer a floor for a reason
* that is neither a runtime limit nor an unreadable repo.
*/
suppressedMatchStages?: readonly string[];
/** Computed by the caller; see `bridgeProvenanceUnknown` for the bridge one. */
provenanceUnknown: boolean;
/**
@ -111,8 +117,17 @@ export function crossRepoCompleteness(input: CrossRepoCompletenessInput): CrossR
const incompleteRepos = [
...new Set([...(input.unreadableRepos ?? []), ...(input.missingRepos ?? [])]),
].filter((repoPath) => input.inScope(repoPath));
// An unreadable or unaccounted repo outranks a suppressed stage: it is the
// more serious structural gap and its remedy (repair the repo, re-sync) has
// to be the one reported. A suppressed stage only decides the reason when
// the repo side is otherwise clean.
const suppressed = (input.suppressedMatchStages ?? []).length > 0;
const repoSideIncomplete = input.provenanceUnknown || incompleteRepos.length > 0;
return {
...truncationFields(input.provenanceUnknown || incompleteRepos.length > 0, 'incomplete-sync'),
...truncationFields(
repoSideIncomplete || suppressed,
repoSideIncomplete ? 'incomplete-sync' : 'suppressed-stage',
),
incompleteRepos,
};
}

View file

@ -856,6 +856,7 @@ export async function runGroupImpact(
const bridge = crossRepoCompleteness({
unreadableRepos: bridgePrep.meta.unreadableRepos,
missingRepos: bridgePrep.meta.missingRepos,
suppressedMatchStages: bridgePrep.meta.suppressedMatchStages,
provenanceUnknown,
inScope: (candidate) =>
repoInSubgroup(candidate, subgroup) || repoInSubgroup(candidate, repoPath, true),

View file

@ -264,6 +264,7 @@ function bridgeCompletenessFor(
return crossRepoCompleteness({
unreadableRepos: meta.unreadableRepos,
missingRepos: meta.missingRepos,
suppressedMatchStages: meta.suppressedMatchStages,
provenanceUnknown: bridgeProvenanceUnknown(meta),
inScope,
});

View file

@ -544,6 +544,7 @@ export class GroupService {
const { incompleteRepos: _incompleteRepos, ...truncation } = crossRepoCompleteness({
unreadableRepos,
missingRepos,
suppressedMatchStages: registry.suppressedMatchStages,
// An unrecorded `unreadableRepos` means this listing cannot say which
// repos the sync failed to read — so it cannot claim to be complete.
provenanceUnknown: unreadableRepos === undefined,
@ -888,6 +889,10 @@ export class GroupService {
// "none" (see ContractRegistry), and a value we could not read is equally
// unrecorded. Reporting either as an empty list is the same conflation.
unreadableRepos: recordedRepoList(registry?.unreadableRepos),
// Same tri-state, same reason: `group status` is where an operator goes
// to ask "is this group's answer trustworthy right now", and a registry
// narrowed on purpose is a different answer from a complete one.
suppressedMatchStages: recordedMatchStages(registry?.suppressedMatchStages),
repos: repoStatuses,
};
}

View file

@ -762,7 +762,11 @@ 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 });
await refreshPreservedBridgeMeta(groupDir, {
missingRepos,
unreadableRepos,
suppressedMatchStages,
});
}
if (!everyRepoFailed) {
@ -792,6 +796,7 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
repoSnapshots,
missingRepos,
unreadableRepos,
suppressedMatchStages,
});
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);

View file

@ -146,6 +146,13 @@ export interface RepoHandle {
* a retry. `'incomplete-sync'` is structural: the bridge itself was built from a
* sync that could not read every configured repo, so those repos' contracts are
* absent from every query against it until `gitnexus group sync` succeeds.
* `'suppressed-stage'` is structural too but has its own remedy: the sync was
* ASKED to skip a matching stage (`--exact-only`), so cross-links that stage
* would have found are absent by request. Retrying returns the same floor, and
* so does re-running the sync the fix is to re-run it WITHOUT the flag. Kept a
* separate member rather than folded into `'incomplete-sync'` precisely because
* that remedy differs; telling an agent to repair a repo it read fine is the
* failure this distinction exists to prevent.
*
* A runtime array rather than a bare type union: every value here has to be
* explained on the agent-facing surface that returns it, and only an enumerable
@ -154,7 +161,12 @@ export interface RepoHandle {
* catch, so the list an agent is promised and the list the code can emit have
* to come from the same place.
*/
export const GROUP_IMPACT_TRUNCATION_REASONS = ['timeout', 'partial', 'incomplete-sync'] as const;
export const GROUP_IMPACT_TRUNCATION_REASONS = [
'timeout',
'partial',
'incomplete-sync',
'suppressed-stage',
] as const;
export type GroupImpactTruncationReason = (typeof GROUP_IMPACT_TRUNCATION_REASONS)[number];
@ -366,4 +378,12 @@ export interface BridgeMeta {
* Optional: a bridge written before this field existed does not record it.
*/
unreadableRepos?: string[];
/**
* Matching stages the sync that built this bridge was asked to skip.
* PERSISTED, like `unreadableRepos` and unlike `repoListsUnreadable` a
* later `group_impact` or `trace` reads this bridge with no access to the run
* that produced it, and a narrowed graph is otherwise indistinguishable from
* a complete one. Same tri-state: absent is "not recorded".
*/
suppressedMatchStages?: MatchType[];
}

View file

@ -505,7 +505,7 @@ Handles disambiguation: when multiple symbols share the target name, returns ran
EdgeType: CALLS, IMPORTS, EXTENDS, IMPLEMENTS, HAS_METHOD, HAS_PROPERTY, METHOD_OVERRIDES, METHOD_IMPLEMENTS, ACCESSES
Confidence: 1.0 = certain, <0.8 = fuzzy match
GROUP MODE: set "repo" to "@<groupName>" for cross-repo impact anchored at the default member (lexicographically first key in group.yaml "repos"), or "@<groupName>/<groupRepoPath>" to choose the member (same path keys as in group.yaml). Phase-1 walk runs in that member; cross-boundary fan-out uses the group bridge. A cross entry with fanout_status:"not_attempted" proves the declared repository boundary, but its far endpoint has no graph symbol; do not interpret empty by_depth or affected_processes on that entry as a completed zero-impact walk. The fan-out attempts at most 50 neighbour crossings, strongest-confidence first. Any short answer carries truncated:true, truncatedRepos, riskEpistemic:"lower-bound" AND a truncationReason dropping a crossing can only move risk DOWN, so treat that risk as a floor, never as a verdict. truncated:true does NOT always mean the fan-out ran out of room, so branch on truncationReason: the remedy differs. 'timeout' (the fan-out's wall-clock budget expired) and 'partial' (a neighbour crossing, or the local walk, was cut short) are runtime limits — the same query can return more on a retry or with a larger timeoutMs. 'incomplete-sync' is structural: the group bridge was built by a sync that could not say which repos it read, or that could not read an in-scope repo, so those repos' contracts are absent from EVERY query against this bridge, and truncatedRepos names them even when ZERO crossings to them were attempted. Retrying returns the same floor run group_sync (\`gitnexus group sync\`) and query again.
GROUP MODE: set "repo" to "@<groupName>" for cross-repo impact anchored at the default member (lexicographically first key in group.yaml "repos"), or "@<groupName>/<groupRepoPath>" to choose the member (same path keys as in group.yaml). Phase-1 walk runs in that member; cross-boundary fan-out uses the group bridge. A cross entry with fanout_status:"not_attempted" proves the declared repository boundary, but its far endpoint has no graph symbol; do not interpret empty by_depth or affected_processes on that entry as a completed zero-impact walk. The fan-out attempts at most 50 neighbour crossings, strongest-confidence first. Any short answer carries truncated:true, truncatedRepos, riskEpistemic:"lower-bound" AND a truncationReason dropping a crossing can only move risk DOWN, so treat that risk as a floor, never as a verdict. truncated:true does NOT always mean the fan-out ran out of room, so branch on truncationReason: the remedy differs. 'timeout' (the fan-out's wall-clock budget expired) and 'partial' (a neighbour crossing, or the local walk, was cut short) are runtime limits — the same query can return more on a retry or with a larger timeoutMs. 'incomplete-sync' is structural: the group bridge was built by a sync that could not say which repos it read, or that could not read an in-scope repo, so those repos' contracts are absent from EVERY query against this bridge, and truncatedRepos names them even when ZERO crossings to them were attempted. Retrying returns the same floor run group_sync (\`gitnexus group sync\`) and query again. 'suppressed-stage' is also structural but has a DIFFERENT remedy: the sync was asked to skip a matching stage (\`--exact-only\` / exactOnly), so cross-links that stage would have found are absent BY REQUEST. Re-running the sync unchanged returns the same floor — re-run it WITHOUT that flag. Do not report a repo as broken for this reason; nothing failed to read.
SERVICE: optional monorepo path prefix (case-sensitive path segments). When "repo" starts with "@", scopes the local impact walk and cross-repo symbol paths to files under that prefix; ignored for a normal indexed repo name.`,
annotations: READ_ONLY_TOOL_ANNOTATIONS,

View file

@ -23,6 +23,7 @@ import os from 'node:os';
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 type {
GroupConfig,
StoredContract,
@ -106,3 +107,55 @@ describe('a sync records the matching stages it was told to skip', () => {
expect(registry).toHaveProperty('suppressedMatchStages');
});
});
/**
* The half that matters to a later reader: does a narrowed graph still claim
* to be complete? `crossRepoCompleteness` is the ONE computation behind the
* truncation triple that `group_impact`, cross-repo `trace` and the contract
* listing all return, so pinning it here covers all three.
*/
describe('cross-repo completeness reflects a suppressed stage', () => {
it('reports a floor, with its own reason, when a stage was suppressed', () => {
const out = crossRepoCompleteness({
unreadableRepos: [],
missingRepos: [],
suppressedMatchStages: ['wildcard'],
provenanceUnknown: false,
inScope: () => true,
});
expect(out.truncated).toBe(true);
expect(out.truncationReason).toBe('suppressed-stage');
expect(out.riskEpistemic).toBe('lower-bound');
});
// control: without a suppressed stage the same clean input is complete.
// Without this, hardcoding `truncated: true` would pass the case above.
it('control: a clean sync with nothing suppressed is not truncated', () => {
const out = crossRepoCompleteness({
unreadableRepos: [],
missingRepos: [],
suppressedMatchStages: [],
provenanceUnknown: false,
inScope: () => true,
});
expect(out.truncated).toBe(false);
expect(out.truncationReason).toBeUndefined();
});
// An unreadable repo is the more serious gap and its remedy differs, so it
// has to win the reason slot rather than being masked by the flag.
it('lets an unreadable repo outrank a suppressed stage in the reason', () => {
const out = crossRepoCompleteness({
unreadableRepos: ['app/backend'],
missingRepos: [],
suppressedMatchStages: ['wildcard'],
provenanceUnknown: false,
inScope: () => true,
});
expect(out.truncated).toBe(true);
expect(out.truncationReason).toBe('incomplete-sync');
});
});