Files
msd-core/gsd-core
Tom Boucher c547e73a71 fix(#2927): merge installed overlay reviewer lanes into review-lane invocation (#3062)
* test(#2927): prove overlay reviewer lanes are invisible to review-lane

Failing-first regression for #2927. routeReviewLane builds its lane map from
the static REVIEWER_LANES array only, so an installed overlay reviewer lane
(role:"reviewer" capability) is roster-visible and disclosed at install but
never selectable, plannable, or invocable. The test exercises a pure
mergeReviewerLanes(firstParty, registry) helper that does not exist yet, so
every row fails at the require().

* fix(#2927): merge installed overlay reviewer lanes into review-lane invocation

routeReviewLane built its lane map exclusively from the frozen first-party
REVIEWER_LANES array, so an installed, consented third-party reviewer lane
(role:"reviewer" capability) was roster-visible and disclosed at install but
never selectable, plannable, or invocable — sections/flags/plan/invoke all
shared the one static map.

Add a pure, total mergeReviewerLanes(firstParty, registry) helper
(src/review-lane-descriptor.cts) implementing ADR-2782 D8: first-party ∪
installed overlay reviewer bodies, first-party winning on slug collision. The
overlay body is field-identical to ReviewerLane per ADR-2782 D1 ("no
translation layer"), so the helper MERGES rather than PROJECTS. Malformed
overlays (missing/non-object body, empty or grammar-invalid slug) are skipped,
never thrown — one bad third-party manifest cannot take the first-party lanes
down. routeReviewLane consults loadRegistry({includeInstalled:true}) and
degrades to the static set on any load failure.

* test(#2927): add CLI-seam coverage for the wiring defect + normalize slug

Two findings from the isolated adversarial review:

1. The test matrix's rows 9-10 (acceptance criteria #1-#3: overlay appears
   in sections/flags and plan resolves ok) were documented as covered but
   had no backing tests. The eight pure-helper tests would stay green if the
   one-line routeReviewLane wiring were reverted — the actual defect this PR
   closes had no regression guard. Add real end-to-end CLI tests that install
   a global-scope role:"reviewer" overlay and assert review-lane
   sections/flags/plan see it through loadRegistry -> mergeReviewerLanes.

2. mergeReviewerLanes trimmed the slug for the map key but stored the body
   with its untrimmed slug, diverging from deriveReviewerSlugs (which trims
   before adding to the roster). Normalize the stored lane's slug to the
   trimmed value so the two surfaces agree on the canonical key.

* test(#2927): correct CLI-seam fixtures for reviewer manifest shape

Two corrections from local CLI smoke-testing before the verification run:

1. role:"reviewer" manifests must omit feature-only fields (skills/agents/
   steps/contributions/gates/hooks/runtimeCompat) — the validator rejects them.
   Match the shipped capabilities/lm-studio shape.

2. The plan subcommand renders an ARRAY of {slug,ok,section,transport,...}
   (it strips the nested invocation plan object), so assert on the array
   element, not a top-level object. Also drop the malformed-flag-filter
   assertion: the capability validator enforces flag grammar at install time,
   so a lane with a malformed flag cannot be installed and never reaches the
   flags shape filter (which is defense-in-depth, not independently reachable).

* fix(#2927): drop unnecessary type assertion flagged by lint:ci

The `body as object` cast inside the spread is redundant — body is already
narrowed to object by the preceding typeof check. eslint no-unnecessary-type-
assertion flagged it; lint:ci is a merge gate.

* chore(#2927): add changeset fragment

pr:0 placeholder backfilled with the real PR number once the PR exists.

* fix(#2927): access runGsdTools result via .output in CLI-seam tests

runGsdTools returns {success, output, exitCode, error}, not a string. The CLI
tests (rows 9-10) passed the result object directly to JSON.parse/.split, which
string-coerced to "[object Object]" and threw under gsd-test (3 failures). My
local smoke test ran the CLI directly (string stdout), so it missed this — the
helper wraps execFileSync and returns a result object. Access .output and assert
.success explicitly, matching the established capability-cli.test.cjs convention.

* chore(#2927): backfill changeset PR number 3062

---------

Co-authored-by: sim <sim@local>
2026-08-04 18:59:19 -04:00
..