Commit graph

7 commits

Author SHA1 Message Date
Gergő Magyar
7e993ab897
fix(group): fail ambiguous sync names and honor analyze --name (#3094)
* fix(group): fail sync when a member name is ambiguous

Silent first-match bound the wrong clone when --allow-duplicate-name
registered two paths under one alias. Refs #3028.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(analyze): apply --name on the already-up-to-date path

A rename should not require --force when the index is already current.
Register before the same-commit branch restamp. Refs #3028.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(group): hint member path when impact --repo is an alias

$localRepo stays the yaml key; joining on the registry alias is a
non-join. List matching keys so operators can retry. Refs #3028.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(group): keep injected sync and alias hints consistent

Workspace-deps path maps reuse the resolved handle so duplicate names
cannot throw after an injected resolver. Alias hints match case-insensitively.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-29 21:48:46 +00:00
DuduPhudu
0f793558ad
fix(group)!: stop group sync claiming matching it never did (#3020)
* fix(group)!: remove the matching cascade that was advertised but never built

`gitnexus group create` wrote `matching.bm25_threshold` and
`matching.embedding_threshold` into every generated group.yaml, and no matcher
ever read either one. That was not the whole of it — an entire feature surface
described a BM25/embedding cascade that does not exist:

- `matching.bm25_threshold` / `matching.embedding_threshold` — parsed, persisted,
  unread
- `detect.embedding_fallback` — defaulted and templated, unread
- `MatchType` declared `'bm25' | 'embedding'`; both variants unreachable
- `SyncOptions.skipEmbeddings` — declared in sync.ts and never read
- `gitnexus group sync --skip-embeddings` — accepted, threaded through
  GroupService, ignored
- CLI help in en and zh-CN promised "Exact + BM25 only (no embedding fallback)"
- the MCP `group_sync` schema exposed `skipEmbeddings`, described as
  "Exact + BM25 only (Demo PR: same as default exact path)"

`sync.ts` imports exactly `buildProviderIndex`, `runExactMatch` and
`runWildcardMatch`, and the printed cascade has one stage. An operator whose
links do not match reaches for those thresholds first, and turning either knob
changes nothing — config that silently does nothing is how people conclude a
feature is broken.

Evidence that the cascade should be deleted rather than implemented, from a real
backend/frontend pair: of 165 consumer contracts, 149 link exactly and 16 do not.
Nine of the sixteen are third-party APIs (Google OAuth, Apple public keys,
PostHog, image annotation) with no in-group provider by construction — similarity
matching cannot recover them, it can only invent false links. Two are verb
mismatches: the frontend calls `POST /links` and `GET /links/check-exists` while
the backend declares `GET /links` and eleven other `/links/*` routes but neither
of those, so a fuzzy path match would link a POST consumer to a GET provider. The
rest are path-extraction artifacts. Roughly none of the sixteen would be
correctly recovered, and several would be actively mis-linked.

BREAKING CHANGE: `gitnexus group sync --skip-embeddings` and the MCP `group_sync`
`skipEmbeddings` parameter are removed. Both were accepted and ignored, so no
behavior changes — but a script passing the flag now fails with `unknown option`
instead of being silently misled. Existing group.yaml files keep loading: the
removed keys are simply no longer part of the schema, and a regression test pins
that a legacy config carrying all three still parses.

Closes #3006

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group)!: honour --exact-only, drop inert --allow-stale, report every matching stage

Addresses the review findings on #3020, all of which are the same defect the PR
itself is about: group-sync surface that describes behaviour the pipeline does
not have.

`exactOnly` was inert in exactly the way `skipEmbeddings` was — declared on
`SyncOptions`, threaded through the CLI and the MCP tool, and read by nothing —
and strictly worse, because the stage it promised to suppress DOES run and DOES
write `matchType:'wildcard'` links into contracts.json and the bridge, which
`group impact` and cross-repo `trace` then traverse. It is now honoured rather
than deleted: unlike the never-built BM25/embedding stages, the stage it names
exists, so the flag describes a real choice. The substituted result is
`{ matched: [], remaining: unmatched }`, not an empty result — `wildcard.remaining`
IS `SyncResult.unmatched`, so skipping the stage has to leave its input unmatched
rather than dropping it from the count an operator reads.

`allowStale` had no such stage to gate: `syncGroup` emits no stale warning at any
point (the `checkStaleness` call lives in `groupStatus`, a different path), so it
is removed under the same rationale as `skipEmbeddings`.

`group sync` now prints every matching stage instead of `exact` alone. The old
block printed a `Matching cascade:` header and counted only exact links while the
next line reported `result.crossLinks.length` — which also includes `manifest` and
`wildcard` — so for any group with those the two numbers disagreed with nothing on
screen explaining why. Counting is an exhaustive `Record<MatchType, number>`, so a
new MatchType fails the build here instead of going silently uncounted, and reads
through `?? 0` so a legacy registry carrying a removed matchType prints an honest
count rather than `NaN`.

Also: the MCP `group_sync` description no longer omits the wildcard stage that
always runs, `exactOnly`'s description no longer refers to a "cascade", and
bench/cross-repo-trace/verify.mjs no longer generates the removed threshold keys
into a fresh group.yaml.

Tests: `sync-exact-only.test.ts` pins both directions of the gate (mutation-verified:
removing the gate, or returning `remaining: []`, both go red). `group-tools.test.ts`
pins that the MCP schema dropped `skipEmbeddings` and kept `exactOnly`.
`group-cli.test.ts` pins that both removed flags are rejected, with `--exact-only`
as an accepted-flag control. `config-parser.test.ts` now pins that legacy keys are
PRESERVED (measured, not assumed) rather than only that parsing does not throw.

The type narrowing's fallout in test files is cleared: `tsc -p tsconfig.test.json`
is 987 errors at head against 987 measured on origin/main, with the two error sets
identical — zero net, zero new, zero masked.

Verification: `tsc --noEmit` exit 0; prettier clean; eslint 0 errors (2 warnings,
both pre-existing on base); 69 test files / 1169 tests green across
test/unit/group, test/integration/group, tools, cli-i18n and cli-index-help.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): reject malformed and retired group_sync parameters (U1)

`GroupService.groupSync` read `exactOnly` off an untyped MCP payload with
`Boolean(params.exactOnly)`. While the flag was inert that coercion was
harmless; now that it gates the wildcard matching stage, the string "false" --
a routine shape for an LLM caller emitting JSON -- is truthy, so a caller that
asked to KEEP wildcard matching got it suppressed and a registry with fewer
cross-links persisted to disk. The opposite of the request, written down.

Validate instead of coercing, at the service boundary: the MCP SDK does not
enforce a tool's advertised inputSchema and `callTool` is reachable directly,
so this method is the real gate. The validator mirrors `validateImpactMode`'s
`{ value } | { error }` shape -- the established idiom for this boundary, and
the one groupSync's other guards already return through.

Also refuse `skipEmbeddings` and `allowStale` by name. The CLI rejects them
outright because commander errors on an unknown option; the MCP path accepted
and silently dropped them, so an agent working from a cached tool schema was
never told. Removing them took away discoverability, not acceptance.

Both guards run before the group is read off disk, so a rejected call performs
no work. Every test asserts the sync did NOT run -- an error string alone
cannot distinguish "refused" from "refused but synced anyway".

The tool description gains the validation note AFTER the registryOutcome
paragraph: `tools.test.ts` slices that description by ordinal position of the
'preserved' / 'superseded' / 'no-prior-registry' literals, so appending past
all three leaves those slices intact (verified, 44/44).

tsc clean; 1039/1039 group unit tests pass.

* fix(group): record the matching stages a sync was told to skip (U2)

An `--exact-only` sync wrote a contracts.json and bridge with fewer cross-links
and nothing recorded that the wildcard stage had been suppressed by request.
`group_impact` and cross-repo `trace` read that registry as authoritative, so a
narrowed graph was indistinguishable from a complete one -- and because
`group_sync` is MCP-exposed, one agent call durably narrowed the shared answer
for every later reader with no signal at all.

Add `suppressedMatchStages` to ContractRegistry and SyncResult, following the
`unreadableRepos` tri-state end to end: absent means a registry written before
the field existed, `[]` is the measurement "this run suppressed nothing", and a
populated list names the stages. The writer always emits it, because omitting
the empty case is what made "measured, none" unreachable for `unreadableRepos`.

Two properties that are easy to get backwards, and are why the split matters:

- SyncResult carries the marker on EVERY outcome. The sync genuinely did skip
  the stage whatever happened to the file afterwards, and the CLI summary (U3)
  renders from this rather than re-deriving it from the caller's options.
- The PERSISTED registry stamps it only on the `written` outcome. The preserve
  path re-writes `{ ...prior }`, so a carried-forward registry keeps the marker
  of the sync that actually produced its contracts instead of being relabelled
  with this run's request. That holds by construction: the registry literal
  carrying the field is only reachable on the written path.

`loadContractRegistryResilient` gains an explicit line, because it rebuilds the
envelope field by field with no spread of the parsed root -- a new on-disk field
is silently dropped unless named there.

Its reader is `recordedMatchStages`, not the existing `recordedRepoList`: that
one 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, and dropping non-members keeps an unknown
stage name from reaching a caller typed as a live one.

Surfaced on `group_contracts` and on `group_sync`'s own return -- deliberately
kept separate from the truncated/truncationReason/riskEpistemic triple. That
triple reports limits a run hit by accident, whose remedy is to fix the repo; a
suppressed stage was asked for, and its remedy is to re-sync without the flag.
Conflating them would tell an agent to retry something that returns identically.

tsc clean; 1043/1043 group unit tests pass.

* fix(group): name a skipped matching stage as skipped, and pin it (U3, U4)

Two facts were printing as the same line. `wildcard: 0 cross-links` meant both
"the stage ran and matched nothing" and "the stage never ran because you passed
--exact-only" -- the same conflation this summary block was introduced to remove
one line up, reintroduced by the flag that made the block necessary.

Render a suppressed stage as `skipped (--exact-only)`, driven by the sync's own
`suppressedMatchStages` rather than by `opts.exactOnly`. The renderer reports
what the sync did, not what the caller asked for, so it stays correct on the
outcomes where the run ended without writing a registry -- which is where the
summary is least legible and a re-derivation from the options would have been
wrong.

Also drops the `?? 0` fallback and the comment justifying it. The comment
claimed a legacy registry could carry a retired matchType into this loop. It
cannot: `syncGroup` returns a freshly computed `crossLinks` array on every
outcome, and even on the preserve path the prior links go to disk while the
fresh array is returned. The code was harmless; the stated reason was false, and
a comment that explains an unreachable path is worse than no comment.

U4 pins both halves through the CLI. A manifest fixture is sufficient: the stage
counts must sum to the total on the `Wrote contracts.json (…)` line, and the
skipped rendering does not need a stage to have matched anything, because
--exact-only records the suppression whatever the fixture holds. That is why
this coverage did not need indexed gRPC/Thrift fixture repos.

Verified by mutation, not assertion: removing the skipped-rendering branch turns
`names a stage it was told to skip as skipped` red and leaves the other 22
green. A control case pins the opposite direction -- the same group without the
flag still reports the stage as zero -- so `skipped` cannot be printed
unconditionally and pass.

U3 and U4 land together: the test has no value without the renderer, so one
commit keeps a revert clean. Both depend on U2, which introduced the field they
read.

tsc clean; 1066/1066 across the group unit and CLI integration suites.

* fix(group): make every description of --exact-only match what it does (U5)

Two descriptions this branch wrote or touched still misstated behavior.

The MCP `exactOnly` description carries "Manifest links still apply." The CLI
help and both locale strings, rewritten in the same commit, omit it -- so the
surface most operators read understated what still runs. Manifest cross-links
are computed before the gate and are genuinely unaffected by the flag, so the
caveat is the accurate half and the CLI now says it too.

The `group_sync` tool description opened with "extract HTTP contracts". That
clause was carried forward byte-identical while only the trailing cross-linking
half was rewritten, and it is wrong: the detect config has six non-HTTP
extraction toggles, and this branch's own new test fixture is Thrift.

`help-i18n.ts` is deliberately untouched. It maps an option to its translation
key and that key already exists; only the commander string and the two locale
values carry text, so a text-only change does not reach it.

The tool-description edit sits ahead of the registryOutcome paragraph, leaving
the relative order of the 'preserved' / 'superseded' / 'no-prior-registry'
literals intact -- `tools.test.ts` slices that description by their positions.

tsc clean; 64/64 across the locale-parity, help-registration, tool-schema and
group-tool suites.

* fix(group)!: remove max_candidates_per_step and shared_libs (U6)

Both keys were declared, defaulted, written into every generated group.yaml,
and read by nothing -- the same three-station dead surface this PR removed for
bm25_threshold, embedding_threshold and detect.embedding_fallback. Every other
DetectConfig field gates a real extractor in sync.ts; shared_libs gates nothing,
because 'lib' contracts come only from the operator-declared manifest extractor.
MatchingConfig reaches matching.ts solely through buildNoisyContractFilter,
which reads exclude_links_paths and exclude_links_param_only_paths and nothing
else.

Existing group.yaml files keep loading and keep their keys. parseGroupConfig
spreads the raw block over its defaults, so a key the schema no longer knows
about survives into the returned config -- which matters because `group add` and
`group remove` round-trip the operator's file through loadGroupConfig ->
yaml.dump -> write, so anything the parser dropped would be deleted from their
checked-in file. The legacy-config test now pins both keys in the same cast form
as its three siblings, and the fixture carries shared_libs so that assertion is
not vacuous.

Two stations that are easy to miss and are swept here:

- gitnexus/bench/cross-repo-trace/verify.mjs GENERATES a fresh group.yaml. It is
  not a preserve-path fixture, so "leave YAML fixtures alone" does not cover it;
  the repo has two generators and both are updated. It is a .mjs file outside
  tsconfig's include, so no type gate would have caught it.
- config-parser.test.ts asserted the removed default at runtime, which vitest
  DOES run. That assertion is gone from the defaults case (the key no longer has
  a default) and re-formed as a preserve assertion in the legacy case.

Verification gate, corrected: "zero net new errors against origin/main" would
have measured the whole branch delta and been red through no fault of this
commit. Measured instead against the branch tip immediately before it --
tsc -p tsconfig.test.json --noEmit reports 989 before and 989 after. Twenty-four
typed-literal sites across ten test files, none of them CI-gated, plus the two
runtime sites above which are.

Note the deliberate side effect: removing a key from the defaults also stops the
group add round-trip from re-adding it to a file that never carried it. Nothing
in src reads either key, so no behavior changes.

BREAKING CHANGE: `matching.max_candidates_per_step` and `detect.shared_libs` are
no longer part of the group.yaml schema and are no longer written into generated
templates. Existing files carrying them continue to parse and retain them.

src tsc clean; 1189/1189 across the group unit, group integration, locale-parity,
help-registration and tool-schema suites.

* docs(group): map PR #3020 review findings to the commits that close them

Retitles the ledger to hold one section per reviewed PR and adds #3020's ten
findings. Two things are stated rather than claimed away: `abda0d041` closes
three findings because they are one code block plus the test that pins it, and
the suppressed-stage marker is a coupled set because the renderer consumes the
field the earlier commit introduces.

Also records what is NOT closed here -- the PR description's false claim about
`max_candidates_per_step` lives outside this branch.

* refactor(group): apply simplify-pass findings

Four cleanup agents (reuse, simplification, efficiency, altitude) over this
run's diff. Efficiency was clean. The rest found five things worth fixing, two
of which were real gaps rather than style.

`recordedMatchStages` filtered unknown values instead of rejecting the list.
That inverted the tri-state on the one field built to prevent exactly this
conflation: a stale `['bm25']` -- the scenario its own comment cites as the
motivation -- survived as `[]`, which on this field MEANS "measured, nothing was
suppressed". A confident clean answer manufactured from a value we could not
read. Now all-or-nothing, matching `recordedRepoList`.

`gitnexus group contracts` showed nothing after an exact-only sync. The human
renderer destructures a fixed field list and gates its incompleteness warning on
`truncated`, so the marker reached the MCP payload and the JSON output but not
the listing an operator actually reads. It now warns, separately from the
`truncated` warning, because the remedies differ: one says fix the repo, this
one says re-run without the flag.

`verbose` was still coerced with `Boolean()` in the same call whose tool
description this branch changed to promise "PARAMETERS ARE VALIDATED". Validated
now, and added to the tool schema -- it was read by the backend and advertised
nowhere.

Reuse: the thrift wildcard-matchable pair existed twice, near-verbatim, in
`sync-exact-only` and `registry-suppressed-stages`. Both now call a shared
`makeWildcardPair` fixture, so the shape `runWildcardMatch` fires on is defined
once.

Simplification: dropped a `Set` built per sync over a list that only ever holds
zero or one entries; iterating `Object.keys(STAGE_COUNTS) as MatchType[]` also
keeps the exhaustiveness the `Record` was built for, which `Object.entries` had
discarded.

Deliberately not done, with reasons: a schema-driven unknown-parameter layer at
the MCP chokepoint (five parameters are read by backends and declared in no
schema, so a strict layer rejects working calls today, and it cannot produce the
"was removed" message finding 3 is about); folding the marker into
`GROUP_IMPACT_TRUNCATION_REASONS` (reverses a recorded plan decision and the
bridge scope is an open question for the maintainer); a per-stage suppression
cause `Record` (no second suppressor exists -- speculative); collapsing the six
`detect` extractor branches into a table (a real generalization, but a refactor
outside this diff); and converging an untouched pre-existing CLI test onto the
new manifest helper (it captures a value the helper does not return, so the
change risks more than the duplication costs).

tsc clean; eslint 0 errors (1 pre-existing warning); 1085/1085.

* docs(group): remove REVIEW-FINDINGS-MAP.md

Removes the findings-to-commits ledger from the source tree.

Note for anyone reading this in history: the file was introduced on main by
#3012 and carried that PR's findings map; this branch had appended a #3020
section. Deleting it drops both. #3012's content is recoverable with
`git show 2c0fb7753:gitnexus/src/core/group/REVIEW-FINDINGS-MAP.md`.

* 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.

* fix(group): make the suppressed-stage signal actually reach its readers

Applies the mechanical findings from the code review of the previous commit.
That commit claimed cross-repo impact and trace stop reporting a narrowed graph
as complete. Trace did; impact did not, and two operator-facing messages said
something false. Four reviewers plus the cross-model pass converged on the same
two defects, and the untested seams were exactly where they were.

`runGroupImpact` recomputed the truncation reason and hardcoded its fallback, so
it could never emit 'suppressed-stage' -- the value the previous commit added to
the union and documented in the tool description. Every narrowed-but-readable
bridge was reported as 'incomplete-sync', telling the caller to repair a repo
that read fine. It now propagates the bridge's own reason, as cross-trace.ts
already did.

The preserve path stamped this run's request onto an older bridge. When no repo
can be read the database and registry are kept from an earlier sync, so
meta.json has to keep describing that sync; instead `{ ...existing,
...diagnostics }` overwrote its marker, leaving contracts.json, meta.json and
bridge.lbug describing three different runs. Currently masked by unreadable-repo
precedence, one loosened condition from a live wrong verdict.

`group contracts` printed "the last sync did not record which repos it could
read" after any exact-only sync: truncated was set with both repo lists empty,
so the message fell through to the wrong branch. It is now gated on the reason,
not the flag. `group impact` likewise blamed the local walk for a floor the flag
caused.

The tri-state reader is now defined once, in the leaf module whose own comment
says it exists so this exact duplication cannot recur -- it had been copied into
bridge-db.ts within one commit of that comment being true.

Both agent-facing descriptions now name the field. The previous commit added it
to three payloads and documented it on none.

Tests cover what shipped green: the preserve path for both artifacts (verified
by mutation -- reintroducing the stamp turns exactly one test red), and the
reason's REACHABILITY. The existing guard only asserted each reason is
described, which is why a documented-but-unemittable value passed it.

Also corrects a comment that said the marker is deliberately not folded into the
truncation triple. True when written; false one commit later.

tsc clean; eslint 0 errors; 1163/1163 across the group unit, group integration
and tool-schema suites.

* fix(group): drop verbose from the MCP surface, fail a superseded bridge closed

Two maintainer-directed findings from the review.

verbose is removed from the group_sync MCP schema and from GroupService, and
kept on the CLI. The parameter never did what either description claimed: the
gates emit workspace-dependency discovery stats and one aggregate manifest line,
not "each cross-link". Worse, they emit them through the server's logger, which
an MCP caller cannot read at all -- so advertising it introduced precisely the
kind of knob this PR exists to delete, in the PR that deletes them. SyncOptions
keeps the field and the CLI keeps --verbose, because a CLI user really can see
that output; its help now says "Show additional sync diagnostics", which is
what it shows. It was added to the MCP schema earlier in this same PR, so there
is no published compatibility burden in taking it back out. A caller that still
sends it is ignored rather than refused: it was never a documented parameter,
and the retired-name guard is reserved for ones this tool actually withdrew.

The second fixes a split-brain the completeness work made materially worse. When
contracts.json commits and the bridge write then fails, the previous database
stays in place describing an EARLIER sync. Until now it kept vouching for
itself, so group_impact could traverse the superseded graph and call its answer
complete while group_contracts reported the advanced registry -- two public
surfaces making contradictory epistemic claims out of one sync. That was
tolerable when the disagreement was about counts. It is not, now that
suppressed-stage makes completeness a correctness property.

markBridgeProvenanceUnknown withdraws the claim without touching the database:
bridgeMetaMatchesFile already gives provenanceUnknown highest precedence and
refuses to vouch for the pair, so cross-repo answers downgrade to a floor until
a sync succeeds. Deliberately not a re-stamp -- the metadata still describes the
database it was written for, and saying otherwise recreates the mis-pairing the
preserve path avoids. Deliberately not a delete -- the old graph is still worth
having as a floor, it just stops being called complete. Best-effort, because it
runs inside a failure handler and must not replace a reported bridge failure
with an unrelated one; the warning now states which of the two happened.

Shared registry+bridge generation identity is the architectural fix and is
deliberately NOT attempted here. This is the PR-sized containment.

Verified by mutation, both directions: neutering the withdrawal turns the new
test red, and a control pins that a healthy sync does not withdraw provenance --
otherwise every successful run would report its own answers as a floor.

tsc clean; eslint 0 errors; 1185/1185.

* refactor(group): apply simplify-pass findings

Four cleanup agents over the last five commits. Efficiency was clean and traced
why: the containment helper is failure-path only, the reason ternary sits after
the fan-out loop, and the tri-state readers run once per artifact read.

The strongest finding was one the diff itself proved. `refreshPreservedBridgeMeta`
enforced the never-persisted rule for `repoListsUnreadable` and
`pairedWithDatabase` with two deletes in its own body, under a comment noting it
was the only code that read metadata and wrote it back. That held exactly as
long as there was one such caller. `markBridgeProvenanceUnknown` made it two,
and inherited nothing. The strip now lives in `writeBridgeMeta`, so every writer
gets it and no future one can forget; `pairedWithDatabase` is the dangerous one,
because persisted it tells every later reader the pair was verified when nothing
verified it.

`group impact` still printed "fan-out stopped early" whenever `truncatedRepos`
was non-empty — but the bridge's incomplete repos are unioned into that list
even when zero crossings were attempted, so a structural gap was reported as a
runtime one, with the only working remedy omitted. That is the same false-cause
shape the contract listing was re-gated for one commit ago, left live one
command over because the new reason was bolted in front of the old branch rather
than replacing the thing it branched on. Now keyed on the reason.

The `?? 'incomplete-sync'` arm in cross-impact was unreachable: reaching it
needs `truncated` true with all three of its inputs false, which
`truncated = runtimeTruncated || bridge.truncated` forbids. Flattened.

Also: a `recordedMatchStages` insert had split `crossRepoCompleteness` from its
own JSDoc; one new test was a strict subset of another; and the bridge-failure
warning interleaved concatenation with a mid-chain ternary.

The new invariant assertion was caught being VACUOUS by mutation before it
shipped — seeded with a valid repo list, `readBridgeMeta` never sets the
reader-only field, so it passed with or without the strip. The fixture now seeds
an unreadable list, and both it and the pre-existing assertion go red when the
strip is removed.

Deliberately skipped, with reasons: a shared `firstTruncated` fold over
`TruncationFields` (the right altitude, but it changes cross-trace's return
assembly and that surface separately documents a 'timeout' rung it cannot emit —
a behavior change, not a cleanup); a reason-keyed `explainFloor` helper across
all four CLI renderers (real, but a four-site refactor); narrowing the persisted
stage vocabulary to a `SuppressibleStage` alias (would be undone by the very
extension the field was modelled as a list to allow); moving `verbose` to
`logger.debug` and deleting `SyncOptions.verbose` (the maintainer explicitly
directed keeping both); and merging the two tri-state readers behind a predicate
(they are adjacent in one file now, so a tightening applies to both by
inspection — the duplication the comment warned about was cross-FILE).

tsc clean; eslint 0 errors; 1164/1164.

* fix(group): address gitnexus-check findings

Seven bot comments across two review rounds; five distinct after dedup. Four
were valid and are fixed, two were already resolved by later commits the bot
had not seen.

The validator could throw from its own error path. `JSON.stringify` is the right
renderer there — it is what distinguishes the string "false" from the boolean,
which is the entire point of the message — but it throws on a BigInt and on a
cyclic object. So a validator promising a structured `{ error }` instead
rejected, and `callTool` is reachable directly, so neither input is
hypothetical. Guarded, keeping the distinction and falling back for the shapes
that cannot serialize.

An unreadable suppression record read as "nothing was suppressed".
`recordedMatchStages` is all-or-nothing by design, so garbage collapses to
`undefined` — and the consumer treated `undefined` as an empty measurement,
throwing that safety away and reporting a registry it could not parse as
complete. Present-but-unreadable now forces the floor, while absent stays
legitimate: a registry written before the field existed has no opinion and
should not be dragged to a floor for it.

Two test-side findings, both real and both invisible to CI because
`tsconfig.json` is src-only. Three `mock.calls[0][1]` accesses did not
type-check against a zero-arg mock, and four assertions read `truncationReason`
/ `riskEpistemic` straight off `CrossRepoCompleteness`, which is a discriminated
union carrying them on one arm. Also removed a `StoredContract` import that went
dead when those fixtures moved to `makeWildcardPair`.

Worth recording: U6 set a test-config gate at 989 errors and later commits
walked it to 994 without anyone re-measuring — the bot caught three of the five.
Now 987, below the original baseline.

Already fixed, not by this commit: the preserve-path stamp the bot flagged
against 6ceac8b1f (fixed in 1fbe0dc6b) and the displaced completeness JSDoc
(fixed in 2d2ef8c47).

Both behavior fixes are mutation-verified: restoring the unguarded stringify
turns the new unserializable-value test red, and a control pins that an absent
record still reads as complete so the fails-closed change cannot pass by forcing
every registry to a floor.

src tsc clean; eslint clean; 1167/1167.

* chore(autofix): apply prettier + eslint fixes via /autofix command

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-08-27 18:27:32 +01:00
DuduPhudu
2c0fb7753c
fix(group): stop reporting what could not be measured as a measurement of zero (#3012)
* fix: surface unreadable group indexes and escape raw NUL bytes in source

Two independent diagnostics failures, both of which turn a real error into a
confident, benign-looking answer.

**Unreadable member repos (#3011).** `syncGroup` wrapped `initLbug` plus all
contract extraction for each member in a bare `catch {}` that pushed the repo
onto `missingRepos` and discarded the error. A LadybugDB storage-version
mismatch therefore surfaced as "repo not found", `group sync` printed
`0 contracts, 0 cross-links` and exited 0, and the existing contracts.json was
overwritten with an empty registry. The two states need different answers from
the operator — a missing repo must be indexed, an unreadable one is usually
version skew or a lock — so they are now separate:

- the caught error is logged with the repo, group path and lbug path
- `unreadableRepos` is tracked alongside `missingRepos` on `SyncResult`,
  persisted (optionally, so older registries still parse) on `ContractRegistry`,
  and threaded through `GroupService` sync/status
- `group sync` reports both before the cascade counts, since an unread repo is
  the likely explanation for a small or empty count
- `group status` reports unreadable repos separately; calling them "missing"
  actively misdescribed them
- when EVERY configured repo fails to open, the write is skipped: an extraction
  that read nothing is not evidence the group has no contracts, and replacing a
  good registry with an empty one loses data while reporting success

**Raw NUL bytes (#3010).** `sync.ts` and `free-call-fallback.ts` each used a NUL
as a join delimiter, written as a literal 0x00 instead of `\0`. Identical at
runtime, but it makes the file test as binary: `file(1)` reports `data`, ugrep
returns empty with exit 1 — indistinguishable from "no match", with no message —
and BSD grep replaces matching lines with "Binary file ... matches". A search
that should hit comes back as a confident "not present". Both now use the escape,
and a unit test fails on any raw control byte in src/ so it cannot silently
return.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(hygiene): guard every tracked source file against a raw NUL, not just src/

The guard added with the NUL escapes only scanned gitnexus/src for .ts/.tsx.
Neither prior recurrence of this defect in this repo was in that scope:
b620773b1 was gitnexus/bench/cpp-qualified-ns/measure.mjs and 38d737bb5 was a
fixture under gitnexus/test. A guard that cannot see where the bug has actually
landed twice is not a guard.

Drive the file list from `git ls-files` at the repository root over
.ts/.tsx/.js/.jsx/.mjs/.cjs/.mts/.cts — 2483 files instead of 828 — and split
the byte class, which is the part that matters:

  - 0x00 is a hard failure repo-wide. It is the byte git's binary heuristic
    keys on, so it is the one that costs a file its diff (and, on the base side
    of a PR, its inline-comment anchors and its three-way merge).
  - The wider C0 class stays scoped to gitnexus/src. A repo-wide scan finds
    exactly one hit, test/unit/logger.test.ts:146, and that 0x1b is a
    legitimate ANSI-escape fixture that is the subject of the test. Widening
    this half would go red on day one.

Read Buffers and scan bytes instead of decoding each file to latin1, through a
bounded read pool: 1.5 s for 2483 files, against 8-21 s previously for 828.

Add a negative fixture — a planted 0x00 and 0x1b run through the same scanning
helper — so a future refactor of the collector cannot leave a permanently green
guard, plus an assertion that the collected set still reaches bench/, test/ and
.mjs, which goes red if the scope is ever narrowed back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(group): report a cross-repo impact built from an incomplete bridge as truncated

When a sync cannot read a member repo, that repo's contracts and every
cross-link touching them are simply absent from bridge.lbug. Nothing in the
impact walk could notice: the only incompleteness channel on a
GroupImpactResult is truncationFields(), which is driven by fan-out state
(truncatedRepos / localPartial / fanoutTimedOut), and a repo missing from the
bridge sets none of them.

So `group impact` on a symbol whose one downstream consumer lives in an
unreadable repo returned `{ cross: [], truncated: false }` — "complete: nothing
in another repo depends on this". That is a wrong answer, not an empty one, for
a tool an agent uses to license a delete or a rename.

BridgeMeta now records unreadableRepos alongside missingRepos, writeBridge
persists it when non-empty, and runGroupImpact folds a non-empty
unreadableRepos ∪ missingRepos into truncated / riskEpistemic: 'lower-bound',
naming the repos in truncatedRepos.

The reason is a new 'incomplete-sync' rather than the existing 'partial'
because the remedy differs: 'timeout' and 'partial' are runtime limits the same
query can clear on a retry, while this one clears only when `gitnexus group
sync` succeeds. Runtime limits still take precedence when both apply, since
those are what the caller can act on immediately.

The risk VALUE is never clamped down — mergeRisk is monotone in the traversed
crossing count, so an incomplete bridge can only under-report. Marking the floor
is what makes that legible.

Both shape changes are additive and optional, so a bridge written before this
still reads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(group): say truthfully what a sync did to contracts.json

Review follow-ups to the unreadable-repo diagnostics. Every item below is a
place where the code still answered a question it could not answer.

1. The CLI announced a write it did not perform. `group sync` printed "Wrote
   contracts.json (0 contracts, 0 cross-links)" unconditionally, including on
   the path that deliberately left the file alone. SyncResult now carries
   registryOutcome ('written' | 'preserved' | 'not-attempted'), the CLI prints
   from it, and group_sync returns it so an agent that calls group_sync then
   group_contracts can tell why the counts disagree.

2. Refusing to write anything on total failure threw away the diagnostic
   describing the run that just happened. `group status` reads contracts.json
   from disk, so the operator who saw the sync fail and ran status to find out
   why read the PREVIOUS sync's file: no unreadable list, an old lastSync, a
   healthy-looking group — or worse, the previous run's unreadable list
   presented as this one's. The skip is now targeted: contracts, crossLinks,
   repoSnapshots and generatedAt carry forward verbatim, only missingRepos and
   unreadableRepos are refreshed. generatedAt stays put because it dates the
   contracts, which are still the previous run's. With no prior file, or an
   unparseable one, nothing is written at all.

3. Per-repo extraction is now all-or-nothing. Extractors run in sequence and
   any one can throw; appending each one's results straight to autoContracts
   meant a repo whose HTTP extractor succeeded and whose gRPC extractor then
   failed contributed a partial set to the registry, while the same run told
   the operator that repo's "contracts are omitted from this sync".

4. readRegistry gains an opt-in strict mode, and syncGroup uses it. The lenient
   `catch { return []; }` converted "I could not read the registry" into "no
   repo is registered": every configured repo then resolved to MISSING, the
   total-failure guard stayed off (it needs a load error), and a good
   contracts.json was replaced by an empty one at exit 0. That is an unreadable
   condition reported as missing, one frame above the code this branch fixes.
   The default stays lenient for the other nine callers; ENOENT stays lenient
   in both modes.

5. Absence of unreadableRepos keeps meaning "not recorded". The loader spreads
   the key in only when present instead of defaulting to [], and getStatus
   passes undefined through, so a legacy registry no longer reads as "the last
   sync found none unreadable". getStatus also gates both list fields on
   Array.isArray: it reads through readContractRegistry, which is a bare
   JSON.parse cast, so a corrupt string in either slot used to reach
   cli/group.ts and die in .join(', ') — the command whose job is explaining an
   unreadable thing, crashing on one.

6. Smaller, same theme: the per-repo warning passes the Error itself rather
   than err.message, so pino keeps the stack; the total-failure warning no
   longer fires on a dry run, where it described a file the call was never
   going to touch and which need not exist; the status table's MISSING legend
   stops re-conflating the two states; the sync warning drops its
   GITNEXUS_LOG_LEVEL=warn hint, which would only have suppressed output (pino
   emits warn at the default info level, so the reason was already printed);
   and the group_sync tool description and its idempotency comment now describe
   what the tool actually does.

Testing. The original four cases could not see the change they were named
after. Mutation testing showed two survivors: dropping the ===
configuredRepoCount conjunct, which turns "every repo failed" into "any repo
failed" and would silently freeze contracts.json for a group where one of five
repos is skewed; and deleting both logger.warn calls, the stated purpose of the
change. Both survived because every case configured exactly one repo and
nothing read the log. There is now a two-repo case running the real per-repo
loop, an all-missing case, a _captureLogger assertion on the level 40 record,
partial-extraction cases, and strict-read cases. All five mutants are killed,
each by exactly one test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(group): tighten the registry list gates and stop naming a truncation reason on complete results

Three follow-ups from the check bot's pass over the previous commits.

1. `detect.includes` was missing from both group-sync test fixtures, so they did
   not satisfy the `GroupConfig` they claim to construct. It went unnoticed
   because `tsconfig.json` is src-only; `tsconfig.test.json` reports it. The
   older of the two fixtures carried the gap in from the original commit.

2. `runGroupImpact` named its truncation reason in a variable computed before
   the truncated check, so on a fully complete result the variable read
   'incomplete-sync'. `truncationFields` discards the reason when `truncated` is
   false, so nothing surfaced — but a value that is wrong whenever it is unused
   is a trap for the next reader. Computed inline at the one call site that can
   consult it, which is also how the neighbouring call sites are written.

3. `Array.isArray` alone let a corrupt registry through. `['app/backend']` and
   `[{repo:'x'}]` are both arrays, and only the second reaches `cli/group.ts`'s
   `.join(', ')` — as `[object Object]`, a measurement the operator can read but
   cannot act on. Both readers now go through one `recordedRepoList` helper that
   requires an array of strings; anything else is "not recorded", the same as
   absent. Two more rows in the corrupt-value table cover it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(group): keep readRegistry's signature, and stop describing unreadableRepos as index-only

Two items from the check bot's blocking pass.

1. `readRegistry` gained an optional `opts` parameter last commit. That is
   source-compatible — every zero-argument call still compiles and behaves
   identically — but the contract check treats any parameter-list change on a
   symbol with outside callers as a break, and it is right that the safest
   version of this change touches that signature not at all. The strict read is
   now its own export, `readRegistryStrict()`, over a shared private body.
   `readRegistry()` is byte-identical to what it was; `syncGroup` is the only
   caller of the strict one, and the mode is legible at the call site instead of
   hiding in an options bag.

2. `unreadableRepos` is described everywhere as "the index could not be opened".
   That was accurate before this branch and is not now: making per-repo
   extraction all-or-nothing means a repo also lands there when an extractor
   throws partway with the index open fine. The two belong in one bucket
   because the consequence is one thing — none of that repo's contracts are in
   this sync — but the docs have to say so, or an operator reads `unreadableRepos`
   as a storage diagnosis and goes looking at LadybugDB for an extractor bug.
   Corrected on `ContractRegistry`, `BridgeMeta`, `SyncResult`, the `group_sync`
   tool description, and the `group sync` console output, which now says
   "Could not extract contracts from" rather than "Could not read the index for".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(cli): stop calling an unreadable registry an old one in group status

`getStatus` reports `unreadableRepos` as `undefined` for two different reasons:
the field is genuinely absent, or it held something that was not a list of repo
paths and the shape gate declined to guess. The status line named only the
first — "registry predates this field" — so a corrupt value read as a merely
old registry.

That is the same shape of wrong answer this command exists to stop giving: a
condition we could not read, presented as a benign one we understand. The line
now names both, and asks for a sync either way, which is the fix in both cases.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(group): close the three fail-open paths left on the safety boundaries

Follow-ups from the re-review of 31c2b6e81. All three of its blocking findings
reproduce; each is a place where unknown state still resolved to a confident
benign answer, which is the one thing this branch exists to stop.

1. Strict registry reading accepted malformed rows. `[{}]` is a JSON array, so
   it passed the shape check: every configured repo then failed to resolve into
   `missingRepos`, none produced a load ERROR, the total-failure guard stayed
   off, and a good contracts.json was replaced with an empty one at exit 0 —
   the same fail-open the strict mode was added to close, one level down from
   the file to the rows inside it. Strict mode now requires `name`, `path` and
   `storagePath` on every row and rejects the WHOLE registry if any row fails.
   Rejecting rather than filtering is the point: dropping bad rows would report
   the repos they name as unregistered, which is the same wrong answer again.
   `indexedAt` / `lastCommit` are deliberately not required — callers already
   default them, so demanding them would trade a fail-open for a fail-shut on a
   legitimate legacy registry.

2. A failed bridge publication could make impact look complete. `writeBridge`
   swaps `bridge.lbug` and writes `meta.json` as two operations, and this branch
   made that meta load-bearing: `runGroupImpact` derives its truncation fields
   from it. A sync interrupted between the two steps therefore left a NEW bridge
   beside the PREVIOUS sync's metadata, and an impact query read that as
   "complete". Fixed from both ends. The write path removes the old meta before
   the swap, so the window leaves metadata ABSENT rather than stale. The read
   path treats absent-or-unparseable meta (`version: 0`) as unknown provenance
   and reports a floor, which also covers the caught `writeBridge` failure in
   `syncGroup`. Over-reporting truncation on a bridge that is actually fine is
   the safe direction, and the next successful sync clears it.

3. `preserved` was returned when there was nothing to preserve. On a group's
   first all-unreadable sync the outcome was set before the prior registry was
   read, so the CLI told an operator "the contracts from the previous sync are
   preserved" about a file that had never existed. Split out as
   `no-prior-registry`, with its own console message.

Also widened the NUL guard to the source languages it claimed to cover. The
commit that added it said "every tracked source file" while the collector
stopped at the JS/TS family, so a raw NUL in tracked Python, Java, Go, Rust,
C/C++, Ruby, PHP, Kotlin, Swift, C# or shell would still have turned those files
binary unnoticed. Measured before widening: 2315 non-JS tracked source files,
zero hits, so this was an unforced gap rather than a tradeoff. A planted `.py`
fixture and a collector-coverage assertion keep it honest.

Every fix is mutation-verified: reverting each one individually turns its own
tests red (3, 2, 2, 1 and 1 failures respectively), and all pass together.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(group): record the empty unreadable measurement instead of dropping it

Both writers omitted `unreadableRepos` when it was empty, which made the
tri-state this branch introduced unreachable in its most common case.

`ContractRegistry.unreadableRepos` is optional on the TYPE so a registry written
before the field existed still parses, and absence there means "not recorded".
But a sync that read every repo successfully HAS measured it, and `[]` is that
measurement. Dropping it collapsed "measured, none" into "never recorded", so
after every clean sync `gitnexus group status` printed

    Last sync unreadable repos: not recorded
    (the registry predates this field, or its value could not be read)
    Re-run `gitnexus group sync` to record it.

about the sync that had just succeeded. The distinction is only worth having if
the writer commits to it, so both `contracts.json` and the bridge's `meta.json`
now record the field whenever the sync supplied it, `[]` included.

The check bot found this on the bridge writer and attributed the consequence to
`group status`. The consequence is real but it is not the bridge's: `getStatus`
reads `contracts.json` and never touches `BridgeMeta`, whose only consumer is
`runGroupImpact` — where absent and empty are already equivalent. So the
user-visible half was in the registry writer, one file over from where it was
reported, and both are fixed.

Also fills in `DetectConfig.includes` (and `workspace_deps`) across the group
test fixtures that predate those fields. These are pre-existing on main and are
a no-op at runtime — `undefined` and `false` are both falsy at the gate — but
they are the same defect the bot flagged as an error in the new fixtures, and
`tsconfig.test.json` reported eleven of them. That file is not in CI, which is
why they survived; the group tree is now clean of them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* test(group): stop two bridge-metadata tests claiming coverage they do not have

Both were named for the swap window and neither injects a swap failure.

"drops the previous meta.json before swapping the database file" runs two
successful writeBridge calls. Its assertions hold with the removal in either
position, because writeBridge overwrites meta.json at the end regardless — so
it cannot pin the ordering it is named for. Renamed to what it does cover, the
successful-rebuild replacement, with the limit stated in the body rather than
left for the next reader to discover.

"leaves NO meta.json when the swap fails partway" removes the file by hand
after a successful write, so it exercises readBridgeMeta's missing-file
contract, not writeBridge. That contract is worth pinning on its own — version 0
is the signal runGroupImpact fails closed on — so the test stays, under a name
that says so.

The ordering itself is pinned in bridge-meta-swap-window.test.ts, which mocks
retryRename to throw on the bridge.lbug swap and asserts the previous sync's
metadata cannot survive it. Both renamed tests now point there, so the coverage
is findable from the place someone would look for it.

No production code changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* fix(group): pair bridge metadata to its database instead of deleting it

The previous commit closed the swap/metadata window by removing meta.json before
the database swap, so the window would fail to "absent" rather than "stale". That
was the wrong trade, and it destroyed recoverable state.

The old database's move to `.bak` sits inside a catch that swallows failures, not
just "no existing db". When that rename fails — a held read-only handle does this
on Windows, and a long-lived MCP server holds one — the failure is swallowed, the
following `tmp -> bridge.lbug` throws, and writeBridge exits with the OLD database
still in place and perfectly valid. Its metadata was already deleted. Cross-repo
impact then answers "we cannot say" for that group until some future sync
succeeds, and if the cause is a held handle or permissions there is no such sync.
A working feature, destroyed permanently to close a narrow window.

Deleting also only chose which way the window failed; it never closed it.

So destroy nothing, and make the pair self-describing instead: writeBridge stamps
the database's size and mtime into the metadata it writes, and
`bridgeMetaMatchesFile` lets a reader ask whether the two still belong together.
`runGroupImpact` treats a mismatch the same as absent metadata — provenance
unknown, report a floor. A metadata file left over from an earlier sync cannot
match a freshly renamed database, and a sync that fails before the swap leaves a
matching pair untouched. Metadata written before the stamp existed is
unverifiable rather than stale, and is accepted: failing those closed would mark
every pre-existing bridge incomplete, trading a narrow window for a repo-wide
regression.

The swap-window test now distinguishes the two failure shapes, because they want
different answers. When every rename fails the old database never moves, so the
surviving metadata still matches it and impact keeps answering from it. When only
the final rename fails the old database has already reached `.bak` and no
database is in place, so the metadata correctly matches nothing — and
`ensureBridgeReady` fails loudly on the absent file, which beats a silent floor.

Mutation-verified: reinstating the delete, neutering the pairing check, and
dropping the stamp each turn 2, 3 and 3 tests red respectively.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g48u4WcRZy543Wqp5NhpV

* chore: keep TypeScript diffs readable after a NUL leaves the tree

Git decides a pair is binary when EITHER blob carries a NUL, and it only
sniffs the first 8000 bytes. `gitnexus/src/core/group/sync.ts` carried one
at byte 5132 on main. This branch removes it, but the base side still has
it, so the file renders as "Binary files differ" in the pull request: no
hunks, no inline comments, and no three-way merge — however clean the head
side is. A head-side byte guard cannot detect that, by construction, since
it only ever sees the working tree.

Setting the `diff` attribute stops the heuristic from hiding the change.
It does not mark the files binary, does not imply `text`, and does not
change how blobs are stored, normalized, or checked out — the root
`* text=auto eol=lf` still governs all of that. It affects diff generation
and rendering only.

Locally this turns the branch's own sync.ts diff from `Bin 17612 -> 25346
bytes` into 154 insertions and 16 deletions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): answer "provenance unknown" for malformed bridge metadata

`readBridgeMeta` guarded the read and the parse but not the SHAPE of what it
parsed, then cast the result. `runGroupImpact` spread both repo lists straight
into a Set, so a `meta.json` whose `missingRepos` held an object threw a
TypeError out of the entire cross-repo query — and threw it from a point after
`ensureBridgeReady` had taken the bridge lease and before the `try` whose
`finally` releases it, so every such query also leaked a refcount the cached
handle could never get back. A malformed file is a reason to answer "we cannot
say", never a reason to crash the question.

The shape gate now lives where the metadata is read, mirroring the one
`service.ts` already applies to the registry's copies of these same two lists.
Each list is judged independently: a garbage `unreadableRepos` no longer
discards a `missingRepos` that was genuinely measured. A list that was present
but unusable is dropped rather than normalized to `[]`, because an unreadable
value is not a measurement of zero — the new reader-side `repoListsUnreadable`
carries that distinction, and `runGroupImpact` folds it into the same
provenance-unknown verdict it already reaches for `version: 0` and for
metadata that does not pair with the database beside it.

A root that is not an object is closed too. `JSON.parse` succeeds on `null`,
`7` and `[]`; the first threw on `.version`, and the other two read `undefined`
and sailed through the version gate as if the bridge had been vouched for.

Both provenance values moved inside the protected region and are initialized
fail-closed, so a future throw between the lease and the walk releases rather
than wedges.

`repoListsUnreadable` is reader-side only: the sole `writeBridgeMeta` call site
builds a fresh literal, so nothing persists it and no schema version moves.

Mutation-verified: reverting the shape gate alone turns 4 tests red — the three
malformed-list scenarios plus the handle-release regression.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(storage): reject registry rows that cannot identify a repo

The strict read's row gate gave `typeof v === 'string'`, and `typeof '' ===
'string'`. A row whose `name` was blank therefore passed as resolvable, then
matched nothing in `defaultResolveHandle` — putting every configured repo in
`missingRepos` and presenting an unusable registry as a clean answer about an
empty one. That is the same unreadable-as-missing fail-open the strict mode
exists to close, one level further in. A blank `storagePath` is worse than
useless: it joins to a relative `lbug` under the current directory, so the sync
opens an index that is not the repo's.

Both now have to be non-blank after trimming. `path` stays at the bare string
check, on the same reasoning that already exempts `indexedAt`/`lastCommit`:
require only what resolution depends on to IDENTIFY the repo. This gate rejects
the whole registry and the registry is machine-wide, so a field tightened past
what identification needs would let one blank value in one row break every
group sync on the machine — including groups whose repos all resolve. A blank
`path` still yields a working handle; `defaultResolveHandle` does read it, but
only for the pool id and `repoPath`, neither of which decides whether the row
names a repo.

The error now says what is actually wrong instead of naming three fields that
are all present.

Mutation-verified in both directions: dropping the trim turns the three
rejection tests red, and applying the wider fix that was considered and
declined — tightening `path` too — turns exactly the counter-case red, so that
test genuinely pins the narrow reading rather than passing either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): bound the per-repo contract staging append

`autoContracts.push(...repoContracts)` passes every staged contract as a
separate argument, and the engine caps how many arguments one call may take.
That cap is a function of the host's available stack, so it is a different
number on every machine — this one accepts a 125k-element spread and dies at
150k.

The spread itself is not new; what it carries is. Before staging, this line
appended a single extractor's output as it came back. Staging made it carry the
whole repo's, which is enough for a large repo to raise `RangeError: Maximum
call stack size exceeded` on the one line whose job is to commit work that just
succeeded. The throw lands in the catch below, so the sync reports a repo whose
extractors all ran cleanly as one whose index could not be read — a crash
wearing the costume of a diagnostic.

A bounded loop replaces it: the count a repo can stage is now bounded by memory
rather than by how much stack the process happened to get.

The guard is structural, not size-based, and deliberately so. A "make the
fixture big enough to crash" test passes against unfixed code on any host with
a larger stack, which is exactly the guarantee a regression gate cannot give
up. It walks the AST and locates the region by role — the `const` staging
buffer typed `StoredContract[]`, then the extractor `try` that is a direct
statement of the block declaring it — so renaming either identifier keeps it
pointed at the same code. `.apply()` is rejected alongside spread, being the
same hazard in different syntax.

Direct statements only, because `syncGroup` wraps this whole section in its own
try/finally for the lease sweep, and that ancestor reads the buffer too.
Matching any enclosing `try` pulls in the entire function body — including the
two windowed-manifest spreads, which are bounded by the window size and are not
what this fixes.

Mutation-verified in both directions: restoring the spread turns the gate red
naming that line alone; deleting a manifest-window spread, and separately
adding a third one, both leave it green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): keep unreadable repos out of manifest contracts too

Per-repo staging closed one door: a repo whose extractor threw contributes
nothing through the direct path. Deferred manifest resolution was a second
door, still open. It derives its known-repo set from the resolved-handle map,
which kept an entry for a repo the same run had already declared unreadable —
so the sync re-opened that index and resolved symbols against a database it had
just told the operator it could not read.

Deleting the handle in the catch stops the re-open, but it does not satisfy
R2 on its own: `ManifestExtractor` resolves both endpoints of a link and emits
a contract for each, and for an endpoint with no executor that contract is
still emitted with a synthetic UID. The registry ended up naming a repo the
same run reported unreadable.

So the emitted output is filtered by ENDPOINT, not by link. Dropping the whole
link would delete the healthy partner's contract as well — a repo losing its
own output because a neighbour's index would not open, which is wider than the
requirement and destroys good data to suppress bad. A cross-link is different:
it asserts something about a pair, so if either end is unreadable there is
nothing left to anchor it to, and a half-anchored link is exactly the
confident-about-what-it-could-not-read answer the registry must not give.

Deleting the handle also changed what the operator gets told, so the warning is
split. An unreadable repo IS configured; letting it fall into the "references
repos not in config.repos" branch states something false and sends the reader
to edit group.yaml for a problem only re-indexing fixes. It now gets its own
message naming what was actually omitted.

Mutation-verified four ways: reverting the endpoint filter turns three
scenarios red; the over-broad whole-link variant turns the healthy-partner
scenario red and nothing else; removing the handle delete turns the
no-re-open scenario red; and reverting the warning split turns the operator-
message scenario red. Every assertion reads the written contracts.json rather
than the in-memory result.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(group): keep readBridgeMeta's signature stable across the shape gate

The shape gate landed by widening the return type to a reader-only
`ReadBridgeMeta extends BridgeMeta`. That is source-compatible — a covariant
return, one added optional field, every existing caller unaffected, typecheck
and suite clean — but the contract check reads it as a changed signature with a
caller left behind, and blocks the merge on it. This branch already hit the
same wall on `readRegistry` and settled it the same way: leave the signature
alone and make the difference legible some other way.

So the flag moves onto `BridgeMeta` itself as an optional, documented,
never-persisted field, and `readBridgeMeta` goes back to the exact signature
its callers already compile against.

That is the better shape here anyway. The reader-only subtype would have split
the validation two ways: `openBridgeDbReadOnly` and `bridgeExists` both gate on
`meta.version`, and the normalization that comes with the gate is what stops a
`version: null` in a hand-edited meta.json from reading as `undefined` and
sailing through `version > 0` as though the bridge had been vouched for. One
type keeps all three callers behind the same guard.

Nothing persists the flag: `writeBridgeMeta`'s only caller builds a fresh
literal, so it cannot round-trip to disk.

No behavior change — pure type restructuring. 927 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): stop treating a half-written bridge stamp as a verified match

`bridgeMetaMatchesFile` joined its two `undefined` checks with `||`, so
metadata carrying a size and no mtime — or the reverse — returned `true`, the
same answer it gives a fully verified pair.

A stamp is a PAIR. Both halves absent is the legacy shape: metadata written
before stamping existed, which cannot be verified either way and is accepted
deliberately, because failing it closed would mark every pre-existing bridge
incomplete until re-synced. Exactly one half present is not that. Something
wrote a stamp and did not finish, which is precisely the condition stamping was
added to detect — so the check handed back "verified" for the one shape that
most deserves suspicion, and a cross-repo impact query built on it would report
a confident answer about a database its metadata cannot vouch for.

The two states are now separated: neither half present accepts, exactly one
rejects as provenance-unknown, both compare against the file as before.

Found by the repository's own contract check, not by the plan.

Mutation-verified: restoring the `||` form turns both half-stamp cases red
while the legacy and fully-stamped controls stay green, so the pair genuinely
pins the distinction rather than passing either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): pair unstamped bridge metadata by write order, before any open

Unstamped metadata was waved through: `bridgeMetaMatchesFile` returned "matches"
for any pair with no stamp to check, so the stale-meta-beside-a-new-database
window stayed open for every bridge written before stamping existed, and
`runGroupImpact` spent that metadata's completeness as fact.

`writeBridge` renames the database into place and writes the metadata after, so
`meta.mtime >= db.mtime` holds for any pair written together — including by
builds that predate the stamp. A database strictly newer than the metadata
beside it can only come from a swap whose metadata write did not land. That is
the fallback now. It is a heuristic on write order, not proof of provenance, and
it is wrong in two directions: a stale metadata file touched after the swap
still reads as paired, and a pair whose clock stepped backwards between the two
writes reads as unpaired. Both are recorded at the code; the second is the safe
direction. Equality counts as paired, or a coarse-granularity filesystem would
reject every legacy bridge for a reason that is about the filesystem.

The verdict is now taken in `ensureBridgeReady` BEFORE the database is opened,
and carried on the metadata rather than recomputed afterwards.

That ordering is load-bearing, not tidiness. Impact and trace both open the
bridge and only then ask about provenance, so on any platform or LadybugDB
build where a read-only open advances the file's mtime, every pre-stamp bridge
would report provenance-unknown from its first query onward — the exact
repo-wide regression this rule was chosen to avoid, arriving as a silent
downgrade rather than an error. It does not happen on Linux, which was measured.
It cannot be measured on Windows: pinning it by really opening the database
needs an in-process write→read reopen of the same bridge.lbug, which is a
documented limitation there. Rather than ship a Windows-skipped test and leave
the assumption unverified on the platform whose file semantics are most likely
to differ, the check moved ahead of the open so no platform has to be trusted.

The new guard forces the hostile case on every platform: the open is stubbed to
advance the database's mtime, and the verdict must still be "paired". It is
registered in the cross-platform list so the Windows and macOS shards run it,
and it has a control so it cannot pass vacuously.

Two existing fixtures mocked `readBridgeMeta` to return a stamped-era version
while never writing a meta.json — a state production cannot reach, since a
non-zero version can only come from a file that exists. They now write the
metadata their own mock claims to have read, rather than the helper being
loosened to accept metadata it cannot stat.

Mutation-verified twice: reverting the write-order branch turns both rejection
cases red while all four legacy-accept cases stay green, and moving the pairing
call back after the open turns the ordering guard red on its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(group): compute cross-repo completeness in one place

Three surfaces can return a partial cross-repo answer — impact, trace, and the
contract listing — and each decided for itself whether it was complete. Impact
carried the structured triple; trace said it in prose, if at all. An agent
reading a not-found trace had no machine-readable way to tell "there is no
path" from "there may be a path in a repo this sync could not read", which is
the difference between an answer and a floor.

`crossRepoCompleteness` is now the one computation, and its input deliberately
does not name where any of it came from. `BridgeMeta` is not in the signature
and must not be: `groupContracts` answers the same question from contracts.json
and never opens a bridge, so `version`, `repoListsUnreadable` and
`pairedWithDatabase` do not exist on that path. Each caller derives its own
`provenanceUnknown` — the bridge callers through `bridgeProvenanceUnknown`,
which stays separate for exactly that reason — and passes the boolean in.

Scope arrives as a predicate rather than a repo list or a subgroup, so
narrowing a query's scope stays a change to one argument at the call site.

The trace results now carry `truncated` / `truncationReason` / `riskEpistemic`
like impact does. `notes` is untouched; it remains an addition to the machine
channel, never the channel.

One correction to the approach as written: it said to pass the trace's two
endpoint repos as the predicate, but a destination trace declares no `to`. It
asks where a call lands, so any member may hold the answer — and an unreadable
provider repo is precisely how "no outgoing ContractLink leaves this repo"
becomes a wrong answer rather than an empty one. Filtering that path to the
`from` repo would have reintroduced the bug this unit exists to close, so it
passes every repo and a test pins it.

Two pre-existing paths become consistent with the vocabulary as a result: a
crossing-capped result now reports `truncationReason: 'partial'` alongside the
`truncated` flag it already set, and the destination path's `ambiguous` returns
now report the cap its `ok` and `not_found` siblings already reported. Both are
additive — no field is removed, and no `truncated` flips from true to false.

`truncationFields` returns a discriminated union now, so `truncationReason`
reads without a fallback on the branch where it cannot be absent.

Mutation-verified: reverting the provenance fold alone — one line in the shared
helper — turns 8 tests red across both surfaces, 2 new trace scenarios and 6
existing impact ones, which is the point of there being one helper.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): narrow the incomplete-repo set to the query's declared scope

A subgroup-scoped impact query was marked a lower bound by repos it had
explicitly excluded. The fan-out already drops every neighbour outside the
subgroup, so those repos could not have contributed a crossing to the answer —
and a completeness marker that fires on results it does not describe is how a
caller learns to ignore the marker.

The scope is the query's DECLARED one, not the one the walk reached. An
incomplete repo's contracts are absent from the bridge by definition, so it is
never in the traversed set; filtering on what was traversed would empty the
intersection on every query and silently restore the fail-open this channel
exists to close.

Declared scope here is the subgroup PLUS the query's own repo, which the
approach did not account for. The walk starts from that repo's contracts in the
bridge, so when it is the repo the sync could not read there are no crossings
to find under any scope — and a subgroup excluding it would have turned that
vacuum into a confident "nothing depends on this", for a tool an agent uses to
license a delete. That case reported a floor before this change, so narrowing to
the subgroup alone would have been a regression. The union only ever widens the
in-scope set, so it cannot re-mark a repo the query excluded.

Membership goes through the existing `repoInSubgroup` in both clauses, `exact`
for the origin equality, rather than growing a second notion of what it means
for a repo path to be in scope.

Sound only while `MAX_SUPPORTED_CROSS_DEPTH` is 1 — at depth 2 an out-of-scope
repo can sit between two in-scope ones — and that constraint is recorded at the
intersection.

Unscoped queries are byte-for-byte unchanged: `repoInSubgroup` answers true for
an absent subgroup, so the intersection is the whole set.

Mutation-verified: restoring the unfiltered predicate turns exactly the two
scoped cases red while the unscoped control and both in-scope guards stay green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): keep the preserved registry and the bridge from disagreeing

A total-failure sync refreshed contracts.json's diagnostic lists and left
meta.json alone. But meta.json, not contracts.json, is where runGroupImpact
reads completeness from — so the registry said "this sync could not read
app/backend" while a cross-repo query answered `{ cross: [], truncated: false }`.
Two surfaces describing the same run, one of them wrong, and the wrong one is
the machine-readable one an agent uses to license a delete.

The preserve path now refreshes the same two fields in the metadata. The
database stays untouched: it still holds the contracts being preserved, and
rebuilding it here would be the one write that could lose them.

Refreshing metadata is not free, though, and the obvious version of it is a
fail-open. The rewrite moves meta.json's mtime to now while bridge.lbug's stays
old, so an unstamped pair whose database is NEWER than its metadata — the shape
the write-order rule exists to reject — would come out of a preserve sync
passing the check. Writing "no stamp" does not help; the write-order comparison
is exactly what the moved mtime defeats. The verdict has to be recorded in the
metadata, because the refresh cannot avoid moving the mtime.

So `provenanceUnknown` is persisted whenever the existing pair does not already
check out, the existing stamp fields are carried through verbatim rather than
dropped, and `bridgeMetaMatchesFile` rejects the marker ahead of both the stamp
and the write-order heuristic. A pair that already matched is re-stamped
instead, which also upgrades a legacy unstamped-but-paired bridge to an exact
stamp. No preserve run can increase the number of pairs that pass the check.
The marker self-clears: `writeBridge` builds fresh metadata and never sets it.

`BridgeMeta` carries two reader-side fields documented as never persisted, and
this is the first code in the repo that reads metadata and writes it back. Both
are stripped explicitly before every write. `pairedWithDatabase` is the
dangerous one — persisted, it would tell every future reader the pair had been
verified — and a test seeds both on disk to pin that neither survives.

The write is not wrapped in a catch, unlike writeBridge on the success path.
There contracts.json is canonical and already written, so a stale bridge is a
recoverable degradation; here the write IS the guard against a confident wrong
answer, and swallowing its failure would reinstate the fail-open it closes.
`writeContractRegistry` above is unguarded into the same directory for the same
reason.

A group with neither file writes nothing: `readBridgeMeta` already answers
`version: 0` for an absent file, so a written one would say what the absence
already says while inventing state for a bridge that has never existed.

Mutation-verified three ways: dropping the marker write turns 6 red including
both laundering scenarios; moving the marker check below the stamp branches
turns the unstamped-laundering case red; removing the field stripping turns the
never-persisted test red. Each restored byte-exactly and re-verified.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): report group_contracts' completeness in the shared vocabulary

`group_contracts` returned contracts and cross-links and said nothing about
whether that listing was the whole story. An agent reading it after a sync that
could not open half the group got a confident-looking list with no way to tell
it was a floor — the same fail-open the impact path already closed, on a surface
that had no channel for the answer at all.

It now returns the registry's two diagnostic lists and the structured triple,
folded through the same helper the impact and trace surfaces use, so the three
cannot drift. The helper takes no `BridgeMeta` precisely so this path — which
reads contracts.json and never opens a bridge — can share it.

The three registry states stay distinguishable, which is the point:
  - key absent: the registry predates the field and has no opinion about which
    indexes opened, so the key is omitted rather than invented as `[]`, and the
    listing reports a floor. It cannot say which repos the sync failed to read,
    so it cannot claim to be complete.
  - key present and empty: measured, clean, not truncated.
  - key present and populated: the repos, and a floor.

`incompleteRepos` is dropped on this surface alone: both lists it derives from
are returned verbatim beside it, and a third name for the same repos is drift
waiting to happen.

The import is lazy, matching `groupImpact` and `groupTrace` in this same class.
`cross-impact.js` statically pulls the native LadybugDB binding through
`bridge-db.js`, and `service.ts` is loaded by every `gitnexus group` subcommand
including ones that touch no database.

One fix inside the same file that this unit forced: the registry loader gated
`missingRepos` with a bare `Array.isArray`, which admits `[{repo:'x'}]`. That
was inert while nothing read the list, but this change both returns it and folds
it into the completeness answer — so an unreadable value would have been printed
as a repo name and would have flipped `truncated` on garbage. It now uses the
same `recordedRepoList` gate `group status` already applies to the same field.
`missingRepos` has always been required, so unlike `unreadableRepos` it has no
"not recorded" state to preserve and an unreadable value degrades to empty.

Mutation-verified: reverting the fold alone turns 14 tests red and leaves the
control — the contract and cross-link payload this tool has always returned —
green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(cli): stop dropping group contracts' completeness fields on the way out

`group contracts --json` destructured `{ contracts, crossLinks }` from the
service payload and rebuilt an object from just those two. Everything else the
service returned was discarded on the way to stdout — so the completeness
fields the MCP tool now carries were invisible at the CLI, and the two surfaces
disagreed about the same registry.

It prints the payload whole now. A field added to the service reaches `--json`
without a matching edit here, which is the point: the re-serialized subset was
a second place that had to be remembered, and it was not.

The human-readable path gains the same signal in words. A listing built from a
sync that could not read part of the group shows counts that are a floor, not a
census, and it named neither fact. It now says so and names the repos when the
registry recorded them — and says the sync did not record which repos it could
read when it did not, because a listing that cannot say what it is missing is
still incomplete.

Mutation-verified: restoring the re-serialized subset turns the `--json` case
and the control red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): tell a missing registry entry apart from an unreadable one

`group status` printed MISSING for both "this repo has no row in the registry"
and "the registry itself could not be read", so an operator whose registry.json
was corrupt was told every repo was unregistered — and sent to re-register them
instead of to the one file that was actually broken.

The two are now separate. `missing` keeps its old meaning and still flags every
unusable repo, so an older consumer is unaffected; `unresolvable` is additive,
always present, and carries the reason that produced it.

This is the one caller that has to make that distinction, so it takes the
strict global-registry read. `readRegistry`'s `catch { return [] }` collapses a
malformed registry into an empty one, which is indistinguishable from a genuine
absence and is exactly what produced the wrong label. The cost is accepted
knowingly and recorded at the call site: the strict read rejects the whole
registry when any row fails to identify a repo, so one malformed row renders
every member unresolvable — including members whose own rows are fine. That is
the honest verdict, and it is reported as an unresolved state rather than a
clean one.

Choosing between the two labels needs to know whether a row exists at all,
which `registryIdentifies` answers by mirroring the two tiers the resolver
matches a bare group-config value on — registry name, case-insensitively, and
repo path. It deliberately stops short of the hashed-id and partial-name tiers:
those exist to be generous about what an operator typed, while this only picks
a label, and a looser match would relabel a genuine registry miss as an
unresolvable row — the same conflation this change removes, pointed the other
way.

The plan's third failure mode — a row that resolves but whose storage path
cannot be opened — turns out to be unreachable: `loadMeta` returns null on
every error and `checkStaleness` catches everything, so nothing after
`resolveRepo` inside the try can throw. The reachable per-repo case is
`resolveRepo` itself throwing, as it does for two registered clones sharing a
name, and that is what the tests drive end to end through the real CLI. The
code still handles the plan's case correctly if those helpers ever start
throwing.

Mutation-verified: reverting the split turns 6 unit and 2 CLI cases red while
both controls — a genuine miss, and a healthy group — stay green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(cli): say what the preserve path actually does to contracts.json

The sync summary announced "Did NOT write contracts.json" on the branch that
writes it. The preserve path rewrites the file — keeping the previous sync's
contracts and cross-links, replacing only the two diagnostic lists — so an
operator who checked the mtime and found it moved was told the opposite of what
had happened, on the command this PR exists to make legible.

It now says the previous contracts were kept and names what changed.

The no-prior-registry branch is narrowed for the same reason. It claimed
nothing at all was written, and that is no longer true either: this path still
records the run against an existing bridge's metadata. The claim is now scoped
to contracts.json, which is the file it can actually speak for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): stop the total-failure log promising a preservation that did not happen

The warning fired before the prior registry was read, so it could only ever
promise one of the two things that might be true — and it promised the wrong
one to every group that has never synced: "keeping the contracts from the
previous sync" about a file that does not exist. The console line for that same
run, driven by `registryOutcome`, said the opposite.

It now lives inside the branch, after the read, with one message per outcome
chosen at the point the outcome is decided. The log and the console cannot
disagree, because the same fact selects both.

Both messages keep the warn level and the two repo lists.

Mutation-verified: reverting the split turns the no-prior-registry case red
while the preserved case — whose claim was already true — stays green. The
dry-run test's log filter was also widened to the sentence both messages share,
or the new wording would have made that assertion match nothing and pass
regardless.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): make the bridge-failure warning describe what the code guarantees

The warning after a failed `writeBridge` promised that cross-repo impact would
report `truncated` until a sync succeeded. Nothing on that path produces that
signal.

The swap is the last step: `writeBridge` builds the new database in a staging
directory and only then moves the old one aside. A failure during the build
therefore leaves the previous sync's `bridge.lbug` exactly where it was, beside
the `meta.json` stamped for it — a pair that passes `bridgeMetaMatchesFile`
with the previous run's `unreadableRepos`. The next cross-repo query answers
`truncated: false` from superseded contracts, which is the opposite of what the
operator was told to expect, and worse than being told nothing.

The warning now says what is actually true: contracts.json is intact and
canonical, the bridge was not replaced, cross-repo queries may still answer from
the previous sync's contracts, and nothing marks them as superseded.

The metadata is deliberately NOT re-stamped to make the original promise true.
That would recreate exactly the metadata/database mis-pairing the stamping on
the preserve path exists to prevent, and the comment at the warning records it.

The claim is asserted against captured log output rather than left to the state
tests. Those check which pairs match and what the preserve path writes; every
one of them stays green while this sentence reverts to promising a truncation.
An unasserted user-facing branch is the defect class this change is closing, so
it does not get to close it while remaining one.

No filesystem shape makes the real `writeBridge` fail while
`writeContractRegistry` succeeds — they write into the same directory one line
apart — so the failure is armed through a pass-through wrapper on the file's
existing mock. It delegates byte-for-byte unless a test arms it, and is reset
around the new suite.

Mutation-verified: restoring the original wording turns its own assertion red
and nothing else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(mcp): name every registry outcome group_sync can actually return

The tool's description told agents `registryOutcome` is 'written' or
'preserved'. It has a third reachable value: 'no-prior-registry', returned when
nothing could be read AND there was no previous contracts.json to carry
forward. An agent calling this tool against a group that has never synced got a
value its own tool description said did not exist, and no way to tell it apart
from the case where the previous contracts survive.

The distinction is the whole point of the value. After 'preserved' there is a
registry to read — stale, but real. After 'no-prior-registry' there is nothing
on disk at all, so a following group_contracts or group_impact has no registry
rather than an old one. Those need different responses from the caller.

'not-attempted' stays undocumented because it is unreachable through this tool,
and a guard asserts it stays that way.

The code comment above the annotations claimed the preserve path does NOT write
contracts.json. It does — it rewrites the file, keeping the previous contracts
and cross-links and refreshing only the two diagnostic lists, which the CLI's
own summary was corrected to say a few commits ago. Left alone it would have
re-seeded the same wrong claim next to the text that now states it correctly.

Mutation-verified: deleting the 'no-prior-registry' sentence turns the guard red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(mcp): explain structural incompleteness on the impact tool and status resource

The impact tool's GROUP MODE paragraph described one cause of truncation — the
fan-out running out of room — and left an agent to assume that was the only one.
So a `truncated: true` carrying `truncationReason: 'incomplete-sync'` read as
"retry with a smaller scope", when retrying returns the identical floor forever:
the repos are absent from the bridge itself, and only a re-sync puts them back.
The old text also said the response carries the truncation fields "when it stops
early", which is wrong for that case — `truncatedRepos` names repos even when
ZERO crossings to them were attempted, because their contracts were never in the
bridge to cross to.

The paragraph now branches on the reason and gives each its remedy: 'timeout'
and 'partial' are runtime limits where a retry or a larger budget can help;
'incomplete-sync' is structural and the remedy is `group_sync`.

The reason union is now derived from an exported `as const` array rather than
written as a bare type. A type-only union gives a guard nothing to enumerate, so
the guard has to hand-list the members — and then it passes forever the moment a
fourth is added, which is the exact regression it exists to catch. The guard
iterates the runtime array instead. Verified by appending a probe member and
watching it go red, then removing it. The resolved type is unchanged; every
importer uses `import type` and none needed an edit.

The status resource said "Group index / contract staleness" and nothing about
the distinctions its payload now carries. It explains all of them: a repo absent
from the registry versus one whose entry could not be resolved, and the
`unreadableRepos` tri-state where an ABSENT key is not an empty one — absent
means the last sync never recorded what it could read, so cross-repo answers for
that group are a floor.

The description an MCP client actually receives lives in `getResourceTemplates`,
not in the context resource's inventory line the plan pointed at. Both now carry
the vocabulary, so the two surfaces cannot disagree about the same payload.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(group): serialize group syncs behind a fail-closed per-group lock

Two concurrent syncs of one group could lose one another's writes. Both read
the prior registry, both built contracts, both wrote — last writer won, and the
loser's work was gone with nothing reporting it. A group sync is long and
expensive and is exactly the operation whose lost update destroys contracts.

`syncGroup` now takes a lock for the whole persist section, acquired exactly
once. `acquireIndexLock` is not reentrant, so a second acquisition anywhere
below would deadlock the happy path rather than an edge case; `withGroupSyncLock`
has one call site and nothing inside it re-acquires.

The lock lives on a dedicated `sync-lock` directory inside the group directory,
mirroring the registry lock's dedicated directory rather than reusing the
resource's own — a lock directory that could collide with a per-repo index slot
repeats a bug the registry lock's comment already warns about.

It fails CLOSED, which is the opposite of `withRegistryLock` and deliberately
so. That one degrades to unlocked because it guards a sub-second JSON merge on
a latency-critical path; here running unprotected is the outcome the lock
exists to prevent. Three exits are covered: a timeout, an unwritable lock
directory, and the lock-free degradation the primitive performs silently.

That third exit needed a change in `index-lock.ts`, and it is the one declared
exception to keeping this work inside core/group/. `acquireIndexLock` answers a
read-only or permission-denied filesystem with a no-op handle that is
byte-identical in shape to a real one, so a caller for whom lock-free is not an
acceptable outcome could not tell the difference. It now carries an optional
`lockFree` marker. The change is additive by construction: no signature moves,
no control flow changes, nothing about when or how a lock is taken changes, and
every caller that ignores the field behaves exactly as before.

A filesystem probe inside the group module was considered and rejected on
evidence: `selectBackend` returns `socket` on Linux and Windows, where
`acquireViaSocket` never touches the filesystem and this branch cannot occur —
so a probe would refuse syncs on the two platforms that never degrade while
missing the one that does.

The timeout ceiling is a named 600s constant passed explicitly. The magnitude
matches the primitive's own analyze-sized default because a group sync is
analyze-shaped and a legitimately queued second sync must be able to wait out a
full first one. Passing it explicitly is about the override, not the magnitude:
`resolveTimeoutMs` resolves `GITNEXUS_INDEX_LOCK_TIMEOUT_MS <= 0` to Infinity,
which would turn fail-closed into a hang.

Cross-process exclusion is proved with a real spawned holder, not an in-process
mock, which cannot demonstrate the property this exists for. The lock-free
scenario pins `GITNEXUS_INDEX_LOCK_BACKEND=file` — unpinned it would pass on
two of three platforms while measuring nothing — and produces the failure by
injecting EACCES on one syscall rather than by chmod, so it runs identically on
Windows instead of being skipped there.

The CLI reports the failure through pino rather than a bare stderr write, which
this package lints as an error to keep that migration moving, and the test reads
the `msg` field rather than a raw substring — matching on the raw text would
have passed only by accident of quoting and would go green again if the line
were downgraded.

Nothing is skipped on any platform, and the test is registered for the
cross-platform shards.

Mutation-verified: removing the lock acquisition turns 6 scenarios red;
removing the lock-free rejection turns the degradation scenario red on its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): make the sync-lock timeout name a cause it can establish

The fail-closed lock surfaced the primitive's own timeout message to users for
the first time, and that message says the wait was on "another gitnexus
analyze" — a cause its detection path cannot establish. It is the same
confident-about-what-it-could-not-determine claim this PR exists to remove,
inherited rather than written.

The wrapper now throws its own. It names the group, the lock directory, the
operation, and the elapsed wait, and it says plainly that nothing was written.

The holder clause branches on `holderKnown`. The socket backend exposes no owner
metadata and reports a placeholder pid of -1, so on that backend — and on the
file backend's malformed or vanished-lock timeouts — the message says the lock
stayed held but the backend cannot identify who held it, rather than printing a
pid that means nothing.

The elapsed wait is measured by the wrapper. `IndexLockTimeoutError` carries
only `holder` and `holderKnown`; the figure exists solely inside the string
being replaced, so it had to be taken rather than read.

One pre-existing assertion changed with it: the timeout case asserted
`'Timed out after 600000ms'` from the inherited text, which is precisely the
message this replaces.

Mutation-verified: restoring the inherited message turns the three assertion
cases red and leaves the control — a real acquisition that succeeds — green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): stop a losing sync from downgrading the one that beat it to the lock

Serializing is not ordering. Both syncs run extraction outside the critical
section, so a total-failure sync that acquires second reads the winner's fresh
registry as `prior` and rewrites it with all-unreadable lists. The lock alone
does not prevent that — it only decides who goes second, and the loser then
overwrites a healthy registry with a description of its own failure.
Deterministically, not as a rare interleave.

The guard is a compare-and-swap on the registry file's own identity: stat
before acquiring, re-stat after, and write nothing when they differ. Identity is
presence plus size, mtime and inode — `writeContractRegistry` publishes through
write-then-rename, so a real replacement always changes the inode even if size
and mtime happen to collide.

Deliberately NOT keyed on `generatedAt`, for two independent reasons. It is
stamped when the registry object is built, before the lock is acquired, so a
winner that waited would write a value older than the loser's start. And the
preserve path carries it forward verbatim by design — it dates the contracts,
not the write — so after any preserve sync it does not date the write at all,
leaving the comparison blind on exactly the pairing this guards. A file-identity
compare also needs no cross-process clock agreement.

The skip reports the existing `preserved` outcome. Nothing was written and a
prior registry was kept, which is what that value already means; a new one would
falsify the guard asserting the sync tool's description names every reachable
outcome, and would fall through the CLI's outcome chain, which has no fallback.

The bridge metadata refresh is skipped too, which the plan did not specify.
`refreshPreservedBridgeMeta` stamps THIS run's repo lists into meta.json, and
meta.json is where cross-repo impact reads completeness — so writing it would
report as unaccounted-for exactly the repos the winning sync had just accounted
for. That is the same downgrade being refused, one file over. Skipping both is
what makes `preserved` an honest answer here.

Mutation-verified: removing the after-stat and the skip turns the three decisive
cases red while both non-misfire controls stay green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(group): run the bridge swap inside the caller's critical section

The bridge swap needed the group lock, and could not take it: `syncGroup`
already holds it when it calls `writeBridge`, and `acquireIndexLock` is not
reentrant. Acquiring inside the swap would deadlock every sync on the happy
path rather than on an edge case.

So the body splits the way this repo already splits this shape — a lock-free
`writeBridgeUnlocked` whose precondition is that the caller holds the lock, and
a thin `writeBridge` wrapper that acquires it for direct callers, mirroring
`registerRepoUnlocked` / `withRegistryLock`. `syncGroup` calls the inner one;
everything else keeps calling `writeBridge` and is now serialized by it.

`writeBridge`'s exported signature is byte-identical to before, so no caller
changed and nothing about the exported surface moved.

The precondition is enforced by a comment naming the single production call
site, which is what the existing precedent does. A type could carry it, but the
repo's own answer to this question is a comment, and diverging here would make
this the odd one out for no additional guarantee.

`refreshPreservedBridgeMeta` is deliberately left unsplit. Its one caller is
already inside the critical section and it has no test callers, so an acquiring
wrapper would be dead code standing in for a guarantee the caller already
provides — and moving the lock inside it would be the second acquisition this
change exists to avoid.

Scope: this delivers writer-writer exclusion only. The reader-side promotion of
a leftover `.bak` into place runs on ordinary reads, outside any lock, and is
not claimed here — the pairing check remains the reader's defense. Confirmed as
live behavior while writing the crash-recovery test, which asserts on file
state rather than through `bridgeExists` for exactly that reason.

One test file beyond the two the unit named had to change: a suite mocks
`bridge-db` to inject a `writeBridge` failure and exercise the bridge-write
warning. Once the sync calls `writeBridgeUnlocked`, that fault was being
injected into a function the path no longer calls, and the test went red. The
mock is repointed.

Mutation-verified three ways. Pointing the sync back at the acquiring wrapper
deadlocks a single UNCONTENDED sync — the evidence that the nesting defect is
real and that this split is what prevents it. Removing the wrapper's
acquisition turns the direct-write exclusion case red. Making the lock-free half
acquire for itself turns the held-lock case red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(hygiene): reach every tracked text file with the raw-byte guard

The guard claimed to protect tracked source from a raw NUL — the byte that
makes git classify a file binary and costs it its diff, its inline comments and
its three-way merge on GitHub. It matched on an end-anchored extension regex
covering the JavaScript family, so most of what this repo tracks was never
looked at: JSON, YAML, TOML, Markdown, snapshots, SQL, protobuf, the .NET
project files, the shell and batch scripts.

Worse, an extension regex cannot reach a file that has none. `Dockerfile`,
`CODEOWNERS`, `LICENSE`, the husky hook and every bare dotfile were unreachable
by construction — no amount of widening the pattern would have covered them —
so a second basename filter had to exist for the claim to be true.

It stays an allowlist rather than becoming "everything git tracks", because the
repo legitimately tracks binaries whose extensions must stay out.

The two filters together now collect every one of the 5000 tracked files except
31 — the 30 native prebuilds and one PNG — and those 31 are exactly the files
that carry a NUL. The allowlist no longer has a gap that is not a genuine
binary.

The planted-fixture cases route through the collector's own predicate rather
than straight into the scanner. The pre-existing fixture test bypassed the
filter entirely, so it could only ever prove the byte locator worked, never
that the collector would hand it the file — which is precisely how the gap
survived.

Mutation-verified both ways: removing the basename filter drops `.gitignore`
and `Dockerfile` from the planted results, and reverting the extension regex
drops `.json` and `.md`.

One added case is a preservation pin rather than proof — that tracked binary
formats stay out passes either way, and guards the allowlist from becoming a
denylist later.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(hygiene): stop the byte guard reading the vendored grammar tree

Widening the guard to every tracked text format also pulled in the vendored
tree-sitter grammars, and those are where the bytes are: four generated
`parser.c` files come to 62 MB between them, Kotlin's alone 33.7 MB. Excluding
that root drops 76 files but 66% of the bytes the scan reads — 97 MB down to
33 MB.

The exclusion is a single anchored prefix, matched case-sensitively with
`startsWith`, and both halves of that matter. A `vendor` path-SEGMENT match
would also drop first-party fixtures this repo tracks under directories named
`vendor` and `Vendor` — a Kotlin one, a PHP one, and three files under
gitnexus-web — silently narrowing coverage while the assertion pinned the loss
in place. Case-insensitivity would do the same to a `Vendor` directory at the
excluded root's own level.

The root is named in the guard itself, so the claim that it covers every
tracked text file stays honest about the one place it deliberately does not
look.

The cost comment was wrong and is now measured rather than estimated. It said
"the scan is ~10 ms" — ambiguous between locating the byte and reading the
files, and stale in its byte basis. Locating is ~14 ms; the reads dominate it
by two orders of magnitude, which is the actual reason for the concurrency pool
and the actual reason this exclusion is worth having. Every figure was
re-derived from the finished file rather than carried over from a draft.

The header's claim that `git ls-files` "never descends into vendor" was already
false — vendored code is tracked, so all 106 of its files were being reported
and read. Corrected here, where the distinction becomes load-bearing.

Registered in the cross-platform list first and given a shard weight second.
The weight table is only consulted for files already in that list, so a weight
entry alone is inert and the shard test filters unregistered keys without
complaining. The three-way split stays within 1.01x of ideal.

Mutation-verified three ways: a case-insensitive segment match, a
case-sensitive segment match, and a case-insensitive anchored prefix each turn
an assertion red.

The casing half was initially unfalsifiable — nothing tracked is named
`gitnexus/Vendor/`, so a tracked-set assertion could not distinguish it. Rather
than leave the claim unpinned or invent a fixture, it is pinned on the
predicate with a synthetic path; the tracked-set assertions pin the anchoring.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(group): make the strict-read test able to see which read ran

The file bound both registry exports to one mock:

    readRegistry:       (...args) => readRegistryMock(...args),
    readRegistryStrict: (...args) => readRegistryMock(...args),

so the case named for the strict read asserted a behavior it could not
attribute. Point the production call at the lenient export and every assertion
still holds, because the mock answers the same way whichever one is called.

That is not a hypothetical. With this file as it was, and `syncGroup` mutated to
call `readRegistry` instead of `readRegistryStrict`, all 32 tests passed — the
suite was blind to the exact substitution it exists to prevent, and the fix it
guards could have been reverted without a single red.

The exports now have separate mocks: the lenient one always resolves an empty
list, which is its real contract, and only the strict one is armed by the cases
that need a failure. The named case also asserts directly that the strict read
was called and the lenient one was not, so the attribution is explicit rather
than implied by an outcome.

No tests added — the unit is about what the existing ones can see.

Mutation-verified: the same substitution now turns 24 cases red, including the
named one, and everything stays green unmutated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(group): pin the CLI output branches this PR introduced

The three sync outcomes and the status table's new labels had no assertions.
Every one of them is a sentence about what happened on disk, and this PR
corrected several that were false — a preserve branch that announced it had not
written the file it rewrites, a status table that called an unreadable registry
a missing entry. Text that describes state, with nothing pinning it, is how
those got wrong in the first place.

Six cases drive the real CLI end to end, through the two shapes that need no
indexed repo: members absent from the registry, and members registered at a
storage path with no index file, which makes every repo unreadable. The file
header claimed no LadybugDB-backed command was driven end to end; that is no
longer true and it now says so.

Each branch was suppressed in turn and its assertion goes red — all five that
the plan named.

One of those mutations first reported PASS, and the cause is worth recording: the
string being suppressed also appears inside a neighbouring branch's comment, so
the harness silenced the wrong line. That is a bad mutation, not a weak test.
The harness now asserts the marker it suppresses is unique before trusting the
result, and the redone check goes red.

The plan's sixth scenario is already covered by an existing case that asserts
both labels in one table, so it is not duplicated. A seventh case was added
beyond the plan: without a populated-list case, "prints neither line" would pass
just as well against a CLI that never printed that line at all.

Adds about 15s of measured spawn time locally; CI runs these against the built
dist, which is materially faster per spawn.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(group): assert the MCP payloads by exact shape, not by partial match

Nothing asserted what the group tools actually return. The sync response's
unreadable list and registry outcome, and the contract listing's incompleteness
fields, are documented in the tool descriptions an agent reads — and could have
been dropped in a refactor without a single test noticing.

The assertions are exact-shape rather than partial. A `toMatchObject` would let
a dropped key pass, which is precisely the regression these exist to catch: the
failure mode is an absent field, and a partial match is defined not to see one.
Absences are additionally asserted explicitly.

The tri-state has to survive the response boundary, and it is the reason exact
shape matters here more than usual. An absent `unreadableRepos` means the sync
never recorded what it could read, so the listing is a floor; an empty list
means it measured none; a populated list names them. Collapsing absent into
empty turns "we do not know" into "we checked, it is fine" — so a mutation that
replaces the conditional spread with `?? []` is covered specifically, not just
the outright deletion.

Mutation-verified per field: removing either sync forwarding line, deleting the
conditional spread, replacing it with the invent-empty form, dropping the
truncation triple, or hardcoding the provenance flag each turns an assertion
red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(group): stop the bridge input narrowing what unreadableRepos means

The same field had three definitions. The registry and the bridge metadata both
say it covers a repo this sync could not extract from — an index that would not
open, or an extractor that threw partway through, one bucket because the
consequence is one thing. The bridge input said only "whose index could not be
opened", which describes one cause and silently excludes the other.

It now points at the registry's definition instead of restating it a third
time. A definition written once and referenced cannot drift; three copies of it
already had.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(group): record what the mtime pairing does and does not prove

The write-order fallback is a heuristic standing in for provenance, and a
future reader deciding whether to lean on it needs to know where it breaks
before they do. Both directions are now stated where the function is read
rather than only in the plan that introduced it.

The false-accept direction is a non-monotonic wall clock — mtime is realtime,
so an NTP step back, a snapshot restore, or container skew between the two
writes can leave a mis-paired set reading as ordered. Coarse filesystem
granularity is explicitly called out as NOT being that hazard, because it looks
like it: it collapses a pair written together to equal times, and equal is
accepted, which is the right answer for that pair.

The false-reject direction is any copy or restore that rewrites the database's
mtime after the metadata's. An intact legacy pair is demoted to a lower bound
and stays there until a sync re-stamps it, because nothing on the read path can
tell it apart from the swap window it imitates.

That second direction corrects a claim made while planning this work: that the
rule could only ever demote pairs already broken. It cannot. `cp -r` and
`rsync` without timestamp preservation both produce it on a healthy group, and
saying otherwise where the code is read would leave a future reader to discover
it the hard way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(storage): stop a corrupt registry quoting its own bytes into errors

`JSON.parse`'s SyntaxError embeds a window of the source around the failure —
V8 gives exactly ten characters either side — and the strict read rethrew it
untouched. The registry persists HTTPS remote URLs with their userinfo, so a
file that breaks next to one puts the credential into the error:

    Unexpected token 'L', ..."end.git"},LEAKCAN4RY"... is not valid JSON

The parse now has its own guarded region and reports the path and the failure
class, matching the two corrupt-registry errors already in this function.

The original error is discarded — not logged, not attached as `cause`. This
codebase's convention elsewhere is to hand the logger the Error so it captures
stack and cause, and following that convention here is precisely what would put
the byte window into the log. Under MCP stdio that log is written to the
client's log file on disk, so the thrown-error channel was never the only one
that mattered. The `catch` takes no binding, so the error cannot be reused by
accident later.

That was not theoretical: a sibling commit routes this message into
`unresolvableReason`, which `group status` returns to MCP clients and prints in
the CLI table. Every channel was traced — throw, cause, inspect with the full
chain, the logger, and both downstream consumers.

The leaking shape is narrower than it first appears, and worth recording. The
windowed message only fires when the parser fails at a value-start or trailing
position; a break inside a quoted string yields an unterminated-string error
carrying no window. So a plain mid-URL truncation does not leak — a short write
landing over a longer one does, leaving a URL fragment where a value was
expected. That is a reachable shape for the one machine-wide file every
gitnexus process writes.

The test asserts the message still names the path and the corruption class, not
only that the secret is absent. Asserting absence alone would stay green if the
message became empty.

Mutation-verified: restoring the raw rethrow brings the token back verbatim.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(storage): drop the stale lenient call-site count

The docstring said keeping `readRegistry`'s signature untouched leaves "its
nine other call sites" unaffected. There were thirteen when the discrepancy was
noticed and fourteen by the time it was fixed. The same figure appeared in the
test file's header.

Replaced rather than corrected. A count in prose next to code that moves is a
claim that goes stale without anything failing — which is the defect class this
change set exists to remove, so re-seeding a fresh number would be repeating it
with a longer fuse. The argument was never about the quantity: leaving the
signature alone keeps every lenient caller provably unaffected whether there is
one or fifty.

Also withdrawn while here: the claim that the bridge schema-version guards
diverge between call sites. They do not — the two forms are complements for
every value a writer can produce, there are three sites rather than the two
claimed, and all three agree. Recording a divergence that does not exist would
leave a future reader chasing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(group): add an auditable finding-to-commit map

The Definition of Done claims every review finding has exactly one commit and
that reverting it reintroduces that finding and no other. Without a map that
claim is only checkable by whoever holds the review report, which is one person
for a short time.

The map lists all 28 primary findings against their commits, the three findings
whose suggested fix was deliberately not implemented and what shipped instead,
and the four defects found while executing that no reviewer raised.

It also records the revert contract honestly. Revertability is
dependency-aware, not absolute: the shared completeness helper has three
consumers, so reverting it alone does not build. That coupled set is named
rather than left for someone to discover mid-revert.

Two sections exist because the work produced them, not because the plan asked.
Six claims in the plan turned out to be contradicted by the code — among them a
scope predicate that would have reintroduced the bug its unit was closing, and
an assertion about the mtime rule that was simply wrong. Recording only the
findings would leave the impression the plan was followed as written. Five
residual risks are listed for the same reason, including that R14 is not met on
this PR: the diff attribute works locally but GitHub reads it from the base
side, so this PR's own sync.ts stays binary in the web view and every PR after
it renders as text.

Not under docs/ — that path is gitignored, so a map written there would never
reach the PR and the audit it exists for could not be performed by anyone else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): read a version that is not a version as no provenance

Raised by the check bot on this PR, and real — the bot found one symptom of it;
the field splits four gates apart, not one.

`readBridgeMeta` accepted any numeric `version`, and `0` is this file's word for
"no provenance". A parseable but impossible value — negative, fractional — is
not a schema version, and each gate that reads the field disagreed about it:

  ensureBridgeReady      `> 0 && !== CURRENT`  → opens the bridge
  openBridgeDbReadOnly   `> 0 && !== CURRENT`  → opens the bridge
  bridgeExists           `=== 0 || === CURRENT` → says it is not there
  bridgeProvenanceUnknown `=== 0`               → reports the answer complete

Four verdicts about one file, and the last one is a fail-open of exactly the
class this PR exists to close: a bridge nothing can vouch for, reported as
fully accounted for.

The suggested fix was to widen the provenance check to `<= 0`. That closes the
reported symptom and leaves `bridgeExists` still disagreeing with both openers,
so it is fixed at the reader instead: a version that is not a positive integer
normalizes to the sentinel the gates were all written against. One change, four
gates agreeing by construction, rather than teaching each of them the same new
case and hoping the fifth reader remembers.

Infinity is covered too, though by the pre-existing type check rather than the
range one — JSON cannot carry it, so it arrives as `null`. Recorded at the test
so the case is not mistaken for proof of the range check.

Mutation-verified: restoring the loose numeric check turns the negative and
fractional cases red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): stop a malformed contracts.json reading as an unresolvable registry entry

Raised by the check bot on this PR. Its stated mechanism was wrong — `loadMeta`
returns null on every error and `checkStaleness` catches everything, so neither
can throw — but its conclusion was right, and there is a concrete path it did
not name.

`readContractRegistry` is a bare `JSON.parse(content) as ContractRegistry` with
no shape check, and the snapshot lookup guarded only the registry object:

    registry?.repoSnapshots[repoPath]

The `?.` covers `registry` being null, not `repoSnapshots` being absent. A
contracts.json without that field — a legacy file, a hand-edit, a truncated
write — throws `TypeError: Cannot read properties of undefined`, which lands in
the catch that labels failures as unresolvable GLOBAL-registry entries. So a
group whose own contracts file is malformed reported every repo as a broken
registry row, sending the operator to repair a file that was fine.

An error from one cause presented as another, which is the defect this PR has
been removing everywhere else.

The optional chain closes the crash. The try is also narrowed to the call that
earns the label: only `resolveRepo` sits inside it now, so "did not resolve"
describes something that actually failed to resolve rather than whatever else
happened to throw nearby. The comment records why the other two calls in that
block cannot throw, so the next reader does not have to re-derive it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(group): give the completeness fold a module no native binding reaches

The shared fold ended up in `cross-impact.ts`, which statically imports
`bridge-db.ts` and through it the native LadybugDB binding. `groupContracts`
therefore reached it through `await import('./cross-impact.js')` — loading that
whole module graph to run a Set union and a ternary. Measured: 44-51ms and
8.4MB of RSS on first call, paid once per MCP server and once per
`gitnexus group contracts` invocation.

`completeness.ts` holds the vocabulary and the fold and imports nothing but
types. `service.ts` imports it statically; the lazy import and the comment
justifying it both go. `cross-impact.ts` re-exports so the three surfaces still
have one import site for the vocabulary.

Three other duplications collapse into the same move.

`traceCompleteness` was hand-writing `{truncated, truncationReason,
riskEpistemic}` — a third writer of the pair `truncationFields` exists to keep
mechanically linked (#2787), in the file the consolidation had just touched. It
calls the helper now.

`recordedRepoList` existed twice, byte-identical, one copy's docblock saying it
mirrored the other. That gate is the predicate the whole
absent-vs-empty-vs-populated distinction rests on, applied to the same two
lists on both the registry and the bridge — tightening one copy would have
fixed one surface silently. One definition now.

The trace's scope predicate compared repo paths with `===` while its sibling in
`cross-impact.ts`, added in the same change, went through `repoInSubgroup` with
a comment about not growing a second notion of membership. It had grown one:
the helper normalizes separators and strips trailing slashes, so the same
group.yaml spelling could be in scope for impact and out of scope for trace.

Also here: `registryIdentifies` was a third, weaker copy of the registry's path
rule — it skipped `realpath`, so a symlinked row would not match where the real
resolver would. It uses `canonicalizePath`/`registryPathEquals` now.
`contracts.json` is no longer respelled as a literal in `sync.ts`; `storage.ts`
owns the name it reads and writes. And the runtime-truncation predicate is
bound once instead of written out at both the flag and the reason, where
forgetting the second would label a retry-able answer `incomplete-sync`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(group): give the lost-the-race sync its own outcome instead of overloading preserved

A sync that finds contracts.json replaced while it waited for the lock reported
`registryOutcome: 'preserved'`. That value already meant something else, and the
two differ in exactly the thing the value is for: `preserved` rewrites the file
with this run's diagnostics; this path does not touch it and deliberately does
not record them.

So both surfaces stated something false about disk. The tool description told
agents `preserved` means "contracts.json was rewritten ... refreshing only
missingRepos/unreadableRepos to describe THIS run (the file changed)". The CLI
said "only the unreadable/missing repo lists were refreshed to describe THIS
run". On the lost-race branch nothing was written and the log line beside it
says so outright.

That is the defect class this whole change set removes, reintroduced by the
change set itself — and the reasoning recorded at the time makes it worse, not
better: a new value was rejected because it "would fall through cli/group.ts's
outcome chain, which has no fallback branch". A renderer limitation decided a
domain value, and the description then had to cover two states with one
sentence that fits one of them.

`superseded` is its own outcome now, described in its own words to agents and
rendered in its own words at the CLI. The registry on disk is FRESHER than this
response's diagnostics, which is the opposite of every other non-written
outcome and is why an agent needs to tell them apart.

The CLI renders from a `Record` keyed on the union, so the next outcome fails
the build here rather than printing nothing — the gap that made folding the
state in look like the cheap option.

The description guard is scoped per clause rather than over the whole string.
It forbade "untouched" anywhere, which was right when one clause could only lie
in that direction and wrong now that another clause is accurately untouched. It
also asserts the superseded clause says so, or the two collapse back into one
word for two states.

Found by the quality pass over this branch, not by review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(group): read bytes and stat through one handle, not two path lookups

CodeQL flagged both sites as `js/file-system-race`, high severity, and it is
right about the shape. `stat(path)` followed by `readFile(path)` is two
independent path resolutions with a window between them — the classic
check-then-use race.

It also made the assertions weaker than they read. These two tests exist to
prove a specific file was left untouched, and two lookups can land on different
inodes, so "the bytes and the mtime are both unchanged" was not actually a
statement about one file. The distinction is the whole point here rather than a
technicality.

`snapshotFile` opens the path once and takes both answers from that handle. The
race is gone because there is no second lookup, and the assertion now genuinely
concerns one inode.

I had previously triaged these as below the ruleset's threshold and left them
for the repository owner. That was wrong: they carry
`security_severity_level: high`, and the branch ruleset gates on
`high_or_higher`, so they were blocking the merge rather than sitting under it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
2026-08-26 09:37:16 +01:00
Gergő Magyar
8402963198
fix(ci): shard platform-sensitive matrix + spawn built CLI to fix Windows cross-platform timeout (#2394)
* fix(ci): shard platform-sensitive matrix + spawn built CLI to fix Windows cross-platform timeout

The `windows-latest (platform-sensitive)` job was hitting its 15-min internal
vitest watchdog in run-cross-platform.ts. It's cumulative slowness, not a hang:
the fixed 72-file suite is dominated by ~50 CLI/worker process spawns, and
Windows is ~5x slower than macOS at process startup (macOS ran the same set in
~3min of tests). Two complementary changes bring it back under the watchdog with
headroom, without touching any test assertion:

- Shard the platform-sensitive matrix (windows/macos × shard [1,2]) and forward
  `--shard=i/2` through run-cross-platform.ts to vitest, which partitions the
  fixed file list deterministically (sha1, equal file-count) — halving each
  runner. macOS/Ubuntu were already under budget.
- New test/helpers/cli-entry.ts (`CLI_SPAWN_PREFIX`): spawn the built
  `dist/cli/index.js` when `GITNEXUS_E2E_CLI=dist` (set on the cross-platform job,
  which already builds) instead of `node --import tsx src/cli/index.ts`, which
  re-transpiles the whole CLI on every spawn. Defaults to tsx-on-source so local
  runs always reflect current source; `GITNEXUS_E2E_CLI=dist` on an unbuilt tree
  throws an actionable "run npm run build" error. dist is opt-in only — never
  inferred from a generic `CI` env — so an ambient `CI=1` can't silently run a
  stale build. Converted 8 spawn-based e2e suites; added test/unit/cli-entry.test.ts.

The Ubuntu coverage job leaves `GITNEXUS_E2E_CLI` unset, so the tsx-on-source path
stays exercised in CI too (both entry points covered).

Measured on Linux: cli-limit-e2e 121.5s→91s, cli-e2e 289s→217s (~25%); larger on
Windows where the transpile is a bigger share of each spawn.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(ci): derive platform-sensitive shard count from one source (#2394)

The shard total was hardcoded in three coupled, unenforced places (matrix
length, job-name suffix, --shard denominator); editing one without the others
silently dropped a shard's tests with green CI. Add a checkout-free shard-plan
job whose single TOTAL generates both the shard index list (consumed via
fromJSON) and the /N denominator (job name + --shard arg), so they cannot
drift. Asserts TOTAL>=1 to rule out an empty-matrix silent skip. No behavior
change — still 2 shards per OS.

Addresses PR #2394 tri-review finding F2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ci): 3 shards for real Windows headroom + honest sharding comments (#2394)

vitest shards by file COUNT, not runtime, so the heaviest spawn suites cluster
into one shard: live CI showed Windows shard 1/2 at 12m12s (~81% of the 15-min
watchdog) vs shard 2/2 at 3m0s. The old comments claimed "comfortable/generous
headroom", which the count-based split doesn't deliver at 2 shards. Bump TOTAL
to 3 (one line, single source) so even the busiest Windows shard clears the
watchdog, and reword the comments to describe count-based (not time-based)
sharding.

Addresses PR #2394 tri-review finding F1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(ci): extract testable parseShardArg from run-cross-platform (#2394)

The --shard parse/forward glue had no unit test. Extract it into a pure
scripts/shard-arg.ts (mirroring the computeSpawnPrefix extraction precedent) so
the branch logic is lockable without the script's top-level execFileSync, and
add test/unit/shard-arg.test.ts (absent -> undefined, valid token -> passed
through, found amid other args). Behavior unchanged; U4 adds the malformed
fail-loud on top.

Addresses PR #2394 tri-review finding F3.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ci): fail loud on a malformed --shard arg (#2394)

A shard-shaped-but-malformed arg (--shard=1, --shard, --shard=abc) was silently
ignored, dropping the shard flag so both legs ran the full unsharded ~50-spawn
suite — re-arming the Windows watchdog timeout with no signal. parseShardArg now
throws an actionable error on any --shard/--shard=… arg that fails the strict
regex (unrelated flags like --shardx= pass through), and the call site in
run-cross-platform.ts catches it into console.error + exit 1, kept outside the
execFileSync try so the message isn't swallowed by that catch's watchdog-only
branch.

Addresses PR #2394 tri-review finding F4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(test): fail loud on an unknown GITNEXUS_E2E_CLI value (#2394)

computeSpawnPrefix silently degraded any unknown GITNEXUS_E2E_CLI value to
tsx-on-source, so a typo (e.g. `dsit`) would make CI believe it tests the dist
entry point while actually running src. Throw on any value other than
'dist'/'src'/unset (the safe tsx default is preserved for unset/''/'src', so it
still never selects dist without an explicit opt-in). Flip the unknown-mode unit
test to assert the throw and add the missing {mode:undefined, distExists:true}
case. Only ci-tests.yml sets the var (=dist), so no existing suite is affected.

Addresses PR #2394 tri-review findings minor-a/b.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(ci): run cli-entry.test.ts on the cross-platform matrix (#2394)

cli-entry.test.ts resolves CLI_SPAWN_PREFIX from a real path, and its last
assertion (cli[/\\]index) has a Windows backslash branch that only Ubuntu
exercised. Register it in PLATFORM_LOGIC so it runs on the Windows/macOS matrix
too. (shard-arg.test.ts stays out — pure string logic, OS-independent.) List
grows 73 -> 74; the generated shard matrix keeps coverage complete.

Addresses PR #2394 tri-review finding minor-c.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(test): share tsxLoaderUrl(), dedup the last tsx-loader boilerplate (#2394)

bridge-cache-reopen.test.ts carried its own copy of the tsx-loader-resolution
boilerplate (createRequire -> resolve('tsx/package.json') -> pathToFileURL) —
the one site the PR's CLI_SPAWN_PREFIX migration didn't cover (it spawns a seed
script, not the CLI). Export the existing tsxLoaderUrl() from cli-entry.ts and
reuse it here; the resolved loader URL is byte-identical.

Addresses PR #2394 tri-review finding minor-d.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(test): make skipUnlessFtsAvailable install FTS on miss so shards are self-sufficient (#2394)

Sharding the platform-sensitive suite into 3 exposed a latent test-isolation
bug: load-only FTS primitives (test/integration/lbug-core-adapter.test.ts) only
passed because a sibling installer test happened to co-locate in the same shard
and install FTS into the shared ~/.lbdb first. At 3 shards, lbug-core-adapter
landed in a shard with no installer sibling, so its load-only loadFTSExtension()
failed deterministically on macOS+Windows shard 2/3 under GITNEXUS_REQUIRE_FTS=1.

Make the gate self-sufficient: on a load-only miss under REQUIRE_FTS, install
FTS with `auto` (LOAD-first, then one bounded network INSTALL) before treating
it as a hard failure — mirroring withTestIndexedDB. A pre-installed extension
still costs no network (auto is LOAD-first); offline/local runs (no env var)
still skip gracefully. Verified: with a fresh HOME (no pre-installed FTS) +
REQUIRE_FTS=1, lbug-core-adapter now passes 15/15 (previously threw).

Addresses the 3-shard CI failure surfaced while validating PR #2394's F1 fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* ci: warm-cache the LadybugDB FTS extension across platform shards (#2394)

Follow-up to the FTS self-install fix: cache ~/.lbdb/extension per OS + lockfile
so a warm run skips the network install entirely and the parallel shards share
one download across runs. Pure reliability/speed — on a cache miss the tests
still self-install FTS on demand (test/helpers/fts-availability.ts), so this is
never a correctness dependency, just a way to cut the network-install surface
that made the sharded FTS tests flaky. Keyed by lockfile hash (a LadybugDB
version bump re-installs); per-OS since the extension is a native binary.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ci): pass shard via env to clear zizmor template-injection (#2394)

Interpolating ${{ matrix.shard }} (now sourced from the shard-plan job output)
directly into the run: shell tripped zizmor's template-injection audit
(code-scanning alert #824, ci-tests.yml:147). Move the value into a SHARD env
var — assigned via ${{ }} but referenced as "$SHARD" in the shell, which is not
an injection sink — and set shell: bash so the expansion is uniform across the
windows + macOS matrix (the default run shell is pwsh on Windows, where $SHARD
would be empty and trip the new malformed-shard fail-loud). Verified locally
with zizmor: the :147 template-injection finding is gone.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* ci: shard the ubuntu coverage job and merge blobs before the threshold gate (#2394)

The coverage job ran the full suite unsharded (~16 min). Shard it like the
cross-platform matrix, then merge the per-shard coverage before enforcing the
threshold gate:

- shard-plan now also single-sources the coverage shard count (cov_total /
  cov_shards), so the coverage matrix + /N denominator can't drift.
- The `tests` job becomes a coverage shard matrix: each shard runs
  `vitest run --shard --coverage --reporter=blob` with thresholds forced to 0
  (a single shard's partial coverage can never meet the gate) and uploads its
  blob. FTS self-installs per shard, so sharding the full suite is safe.
- New `coverage-merge` job (needs: tests) reduces the blobs with
  `vitest --mergeReports`, enforcing the REAL config thresholds on the combined
  ('new') coverage — this is the gate. It also emits the merged test-results.json
  and runs the unsharded web + docker suites, so the `test-reports` artifact
  keeps the exact shape ci-report.yml consumes for its base-branch ('baseline')
  vs new coverage delta.

The shard arg goes through a SHARD env var + shell: bash (no template-injection).
Validated locally: shard blobs write and merge into a coverage-summary.json +
merged test-results.json; the merge enforces thresholds on the union. CI Gate
still aggregates the coverage-merge result via the reusable-workflow call.

Note: the coverage check names change (ubuntu / coverage 1/3 … + merge) — update
any pinned branch-protection required checks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ci): include hidden files when uploading the coverage blob (#2394)

The coverage shards write their blob to gitnexus/.vitest-reports/ (a dotdir).
actions/upload-artifact excludes hidden files by default, so the coverage-blob-*
artifacts uploaded empty — the merge job then downloaded 0 artifacts and
vitest --mergeReports failed with ENOENT scandir '.vitest-reports'. Set
include-hidden-files: true on the blob upload so the blobs actually ship.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ci): group shard-plan GITHUB_OUTPUT writes to satisfy shellcheck SC2129 (#2394)

Adding the coverage shard outputs (cov_shards/cov_total) made the shard-plan gen
step write four individual `>> "$GITHUB_OUTPUT"` redirects, which shellcheck
(run by the actionlint check) flags as SC2129. Group the echoes into a single
`{ …; } >> "$GITHUB_OUTPUT"` block.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(test): cost-balanced shard sequencer to cut CPU contention (#2394)

vitest's default --shard hashes file paths and splits by file COUNT, which
clustered the spawn-heavy suites onto one runner (Windows platform shard 1 ran
~4x the others). Add a custom sequence.sequencer that overrides only shard()
and balances by estimated WORK instead:

- specWeight() weights the fileParallelism:false spawn-heavy suites (cli-e2e,
  lbug-db — already isolated to run sequentially) far above the parallel default
  files, plus file size as a cheap finer signal. Deterministic per checkout.
- assignShards() does greedy longest-processing-time bin-packing (heaviest file
  into the currently-lightest shard). The partition stays complete and disjoint
  — verified: on the 74-file cross-platform set the three shards weigh
  7611/7610/8064 (the sequential-heavy files spread ~7/7/8) with zero overlap and
  no file dropped, vs the hash split's count-only balance.

sort() is left to the base sequencer so project groupOrder / duration-cache
ordering is untouched. Pure logic split into shard-balance.ts with a unit test
locking the disjoint+complete, balance, and determinism properties.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ci): install + cache FTS up front on the coverage (and cross-platform) shards (#2394)

coverage 3/3 failed on extension-binary-real.test.ts: it uses the file-path FTS
gate (requireFtsResourceOrSkip), which resolves ~/.lbdb/extension at MODULE LOAD
and cannot self-install the way the load-path gate (skipUnlessFtsAvailable, U8)
does. The coverage job had no FTS cache and relied on an installer test running
first in the shard — the balancing sequencer reshuffled the shards and dropped
extension-binary-real into a shard with no installer, so FTS was absent.

Remove the ordering dependency: add scripts/ensure-fts.ts (init a throwaway lbug
db, loadFTSExtension with policy:auto → LOAD-first, INSTALL on miss) and run it
up front on every coverage AND cross-platform shard, after restoring the per-OS
FTS cache. The coverage job now shares that same cache key (it previously had
none — this is the "share the cached FTS with coverage" the failure pointed at).
Cold cache installs once; warm cache is a no-network load. Verified locally:
ensure-fts installs FTS into a fresh HOME and is a no-op when already present.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-08 09:09:11 +01:00
ivkond
00966630c4
feat: cross-repo impact analysis (#794) — @repo MCP routing + group resources (#984) 2026-04-20 11:55:07 +01:00
ivkond
255e3e79eb fix(group): address 4 HIGH-priority issues from PR #626 review
1. Path traversal via group name — add validateGroupName() with regex
   [a-zA-Z0-9][a-zA-Z0-9_-]*, called in getGroupDir (defense in depth)

2. gRPC proto regex can't handle nested braces — replace serviceRe with
   extractServiceBlocks() brace-depth counter (init depth=1, skip
   malformed protos)

3. Service boundary detector directory exclusions — add EXCLUDED_DIRS
   set (vendor, target, build, dist, __pycache__, .venv, venv, .tox,
   .mypy_cache, .gradle, .mvn, out, bin) replacing inline node_modules

4. Double-close of LadybugDB pools — remove blanket closeLbug() from
   cli/group.ts; sync.ts per-id cleanup is sufficient

Tests: 22 new tests across 5 files. Full suite: 4706 passed, 0 failed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-02 12:55:33 +03:00
ivkond
4fed097abb feat(group): add sync pipeline, CLI, MCP tools, and monorepo fixture
Wire extractors into the sync pipeline with service boundary detection.
GroupService provides high-level API for all group operations.

- Sync pipeline: orchestrates extraction (HTTP, gRPC, topics) with
  service boundary assignment and exact matching
- GroupService: groupList, groupSync, groupContracts, groupQuery,
  groupStatus (groupImpact deferred to cross-repo follow-up PR)
- CLI: group create/add/remove/list/sync/contracts/query/status
- MCP tools: group_list, group_sync, group_contracts, group_query,
  group_status
- Monorepo fixture: 3 services (auth/orders/gateway) connected via
  gRPC + Kafka + HTTP — all intra-repo cross-links discovered
- Documentation: CLI commands and MCP tools added to both READMEs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-02 00:40:31 +03:00