Skip to content

feat(pipeline-crew-mcp): make the message-kind contract discoverable + fix the issue-number footgun (#3622)#3702

Merged
usirin merged 2 commits into
mainfrom
usirin/3622-crew-mcp-kind-contract-discoverable-BFC43B75
Jul 20, 2026
Merged

feat(pipeline-crew-mcp): make the message-kind contract discoverable + fix the issue-number footgun (#3622)#3702
usirin merged 2 commits into
mainfrom
usirin/3622-crew-mcp-kind-contract-discoverable-BFC43B75

Conversation

@usirin

@usirin usirin commented Jul 19, 2026

Copy link
Copy Markdown
Member

What

Surfaces the crew-mcp typed message-kind contract as a discoverable, resolvable surface every peer can read before sending — closing the gap that let a live IntakePing send fail on a shape ({"issue": 3621} rejected) that nothing exposed ahead of time. Also fixes the underlying footgun (issue/PR numbers modelled as bare strings) and makes schema availability a boot invariant. Enforcement is unchanged — no schema was relaxed.

Fixes #3622

How

  • Discoverable surface (channel_kinds tool). packages/pipeline-crew-mcp/src/protocol/describe.ts renders each kind to {kind, awaitsReply, payload} where payload is the payload's JSON Schema, derived straight from the existing crewMessageKinds/payloadSchemaForKind map (no second, divergent copy of the kind set). packages/pipeline-crew-mcp/src/edge/kinds-tool.ts exposes it as a channel_kinds MCP tool via an injected ChannelDescribe port (same DI shape as ChannelSend/ChannelClaim), registered on the session in packages/pipeline-crew-mcp/src/crew/session.ts.
  • Role↔kind view. packages/pipeline-crew-mcp/src/crew/contract.ts joins the kind map with each role's sanctioned seams, sourced from crewCatalog/seamsFor — so a future non-flat topology stays in sync automatically.
  • Startup invariant. resolveChannelContract runs on the session's boot critical path (assembleCrewSession), so a peer that cannot resolve the full shared kind set fails loud at boot with ChannelContractError rather than discovering a gap at first send.
  • Footgun fix. IntakePing.issue (and EngineNudge's pr/issue target) now carry a branded positive-integer IssueNumber/PrNumber, so {"issue": 3621} is the accepted shape. The other numeric kind fields (DrainProgressTally, Heartbeat) were already Schema.Int — reviewed, no change needed.
  • Coverage. New unit tests for the describe surface (protocol/describe.test.ts), the role↔kind contract (crew/contract.test.ts), the channel_kinds tool (edge/kinds-tool.test.ts), and the startup invariant's loud fail. Existing send/round-trip tests updated to the numeric shape.

Acceptance criteria

  • Kind→schema map exposed as a pre-send resolvable surface, sourced from the existing map (no divergent copy)
  • Each role's sanctioned send/receive kinds discoverable from the same surface, sourced from crewCatalog/seamsFor
  • Schema availability is a startup invariant — a peer that can't resolve the full kind set fails loud at boot
  • IntakePing.issue footgun resolved (number/branded IssueNumber); other kinds reconsidered for the same footgun (EngineNudge fixed; the rest already Int)
  • Unit coverage for the discoverable surface and the startup invariant

Verification

pnpm typecheck (33/33), pnpm lint:worktree (clean), vitest run for @kampus/pipeline-crew-mcp (277 passed, incl. 11 new).

@usirin

usirin commented Jul 19, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ 75623d6

Fresh-eyes gate on PR #3702 against issue #3622. Non-§CP (diff lives entirely in packages/pipeline-crew-mcp/**; pipeline-crew-mcp is not in the control-plane path regex — only pipeline-cli is) → bindable, auto-ships on green. Single code namespace (TS only). Verified at head 75623d6: pnpm --filter @kampus/pipeline-crew-mcp typecheck clean, lint:worktree clean (31 files), full @kampus/pipeline-crew-mcp suite 277/277 passed in the isolated head worktree.

Acceptance criteria

  • [PASS] Kind→schema map exposed as a pre-send resolvable surface, no divergent copy. protocol/describe.ts derives every KindContract straight from crewMessageKinds/payloadSchemaForKind, which are themselves [...CrewProtocol.requests.keys()] and CrewProtocol.requests.get(kind)?.payloadSchema — the catalog is the map, no second kind-set. edge/kinds-tool.ts exposes it as the channel_kinds MCP tool via an injected ChannelDescribe port (same DI shape as ChannelSend/ChannelClaim). describe.test.ts asserts the described set equals crewMessageKinds.
  • [PASS] Each role's sanctioned send/receive kinds discoverable from the same surface, sourced from crewCatalog/seamsFor. crew/contract.ts roleContract reads seamsFor(role) + CrewSeams[seam]._tag, so a future non-flat topology flows through automatically — no hand-kept list.
  • [PASS] Startup invariant fails loud at boot. resolveChannelContract() runs inside assembleCrewSession's Layer.unwrap(Effect.gen(...)) on the boot critical path — yielded before the toolkit registrations are merged/served — so an unresolvable kind set raises ChannelContractError and aborts the layer build rather than surfacing a gap at first send. Fail-closed confirmed: resolveKindContractsFor folds an unrenderable kind into a named ChannelContractError (not a swallowed undefined); describe.test.ts directly exercises the loud-fail branch (names NotAKind/AlsoMissing).
  • [PASS] IntakePing.issue footgun resolved; other kinds reconsidered. IssueNumber/PrNumber are branded positive integers (Schema.Int.check(≥1).pipe(Schema.brand(...))), so {"issue": 3621} is the accepted shape. EngineNudge's NudgeTarget union migrated from bare strings to PrNumber/IssueNumber (both/neither still unrepresentable). Migration is internally consistent: all producers/consumers are the schema + tests (no message producers exist outside the package), and every IntakePing/EngineNudge test body was migrated to the numeric shape ({issue: "3057"}{issue: 3057}, {pr: "pr:3649"}{pr: 3649}). DrainProgressTally (completed/inFlight/total) and Heartbeat (ttlSeconds) confirmed genuinely unchanged — already Schema.Int, untouched by the diff. (The remaining "issue:3054" literals are ClaimRequest.resource string ids, not number fields — correctly left as NonEmptyString.)
  • [PASS] Unit coverage for the discoverable surface and the startup invariant. New protocol/describe.test.ts, crew/contract.test.ts, edge/kinds-tool.test.ts, plus the boot loud-fail; existing send/round-trip tests updated to the numeric shape.

Scrutiny notes (wire-contract change)

  • Enforcement not weakened. edge/send-tool.ts (the pipeline-crew-mcp: channel_send validates nothing — the typed 7-kind catalog is advisory at the wire #3229 send-time gate) is not in the diff — no schema relaxed, only made discoverable. The tighter numeric brand actually narrows what decodes.
  • No machine-local/home/absolute paths in any changed artifact. (~/.claude.json references exist only in pre-existing standup/** files outside this diff, and are generic Claude-CLI config-path references, not leaked operator paths.)
  • Fan-out (silent-failure / type-design / test-gap): no in-scope findings. The one fallible render path (toJsonSchemaDocument) is surfaced loud at boot, not swallowed; branded types make invalid issue/PR numbers unrepresentable; coverage is present.

Verdict bound to head 75623d67ea731d75115c23cec2aaa99589bf5893 (confirmed stable at post time).

usirin and others added 2 commits July 19, 2026 17:15
…+ fix the issue-number footgun (#3622)

The typed kind→schema map was enforced at send time (#3229) but not discoverable
ahead of a send, so a sender had to already know a kind's exact shape or trigger a
reject to learn it — the pre-typed-relay fragility ADR 0187 meant to remove. A live
IntakePing send with the natural `{"issue": 3621}` was rejected because `issue` was
modelled as a bare string.

- Surface the contract: `channel_kinds` MCP tool returns each kind's payload shape
  (JSON Schema, derived from the existing `crewMessageKinds`/`payloadSchemaForKind`
  map — no divergent copy) + whether it awaits a reply, plus each role's sanctioned
  seams (sourced from `crewCatalog`/`seamsFor`, so a future non-flat topology stays
  in sync). Injected via a `ChannelDescribe` port like `ChannelSend`/`ChannelClaim`.
- Startup invariant: a session resolves the whole shared kind set on its boot
  critical path and fails loud if any kind is unresolvable, rather than discovering
  the gap at first send.
- Footgun fix: `IntakePing.issue` (and `EngineNudge`'s pr/issue target) now carry a
  branded positive-integer `IssueNumber`/`PrNumber`, so `{"issue": 3621}` is the
  accepted shape. The other numeric kind fields were already `Schema.Int`.
- Unit coverage for the discoverable surface, the role↔kind contract, and the
  startup invariant's loud fail.

Fixes #3622

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ypecheck exits 0 (#3622)

`Effect.try`'s catch returned `cause` unchanged, leaving the error channel
`unknown` and tripping the @effect/language-service diagnostic
`effect(unknownInEffectCatch)` (TS377021). That diagnostic is warning-severity,
so `tsgo` exited 2 — and turbo then segfaulted tearing down the failed task,
surfacing as the deterministic exit 139 on the `lint / format / typecheck` job.
The 139 was a downstream symptom, not a compiler crash.

Return `String(cause)` instead, matching the in-package idiom in
`standup/version-assert.ts`. The error is discarded by the following
`Effect.orElseSucceed(() => undefined)`, so `describeKind`'s public signature
and behavior are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@usirin
usirin force-pushed the usirin/3622-crew-mcp-kind-contract-discoverable-BFC43B75 branch from 75623d6 to 3f6fac1 Compare July 20, 2026 00:17
@usirin

usirin commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

Repair round 1 — CI exit 139 root cause

The 139 was not a compiler segfault in this diff. It was a downstream symptom. The CI log sequence:

##[warning]src/protocol/describe.ts(60,10): warning TS377021: The `catch` callback in
`Effect.try` returns `unknown`, so the Effect error type stays untyped. effect(unknownInEffectCatch)
 ELIFECYCLE  Command failed with exit code 2.      <- tsgo failed on the effect diagnostic
Segmentation fault (core dumped)
 ELIFECYCLE  Command failed with exit code 139.    <- turbo crashing tearing down the FAILED task

@effect/language-service raised a warning-severity diagnostic on describe.ts, tsgo exited 2, and turbo then segfaulted while tearing that failed task down. Deterministic because the diagnostic is deterministic — the crash was never the real defect.

Why it did not reproduce with a plain local pnpm typecheck

The effect diagnostics only reach tsgo when the TypeScript package is patched (effect-language-service patch). Locally packages/pipeline-crew-mcp/node_modules/typescript/lib/typescript.js reports is not patched, so the rule never ran and typecheck exited 0. Reproduced instead with the plugin's own CLI:

effect-language-service diagnostics --project tsconfig.json --format text

which prints the exact describe.ts(60,10): warning effect(unknownInEffectCatch) before the fix and nothing after it.

The fix

One line — catch: (cause) => cause becomes catch: (cause) => String(cause), matching the existing in-package idiom in packages/pipeline-crew-mcp/src/standup/version-assert.ts. The error is discarded by the following Effect.orElseSucceed(() => undefined), so describeKind's signature and behavior are unchanged.

No behavior changed beyond stopping the crash — the discoverable channel_kinds surface, the branded IssueNumber/PrNumber migration, and the boot invariant are all intact.

Verified locally (rebased onto origin/main)

  • pnpm lint → exit 0
  • turbo run typecheck --force33/33 successful, 0 cached, exit 0, no exit 139
  • vitest run277 passed / 42 files
  • effect diagnostics on describe.ts at error+warning severity → 0

Rebased onto origin/main and force-pushed; the prior PASS @ 75623d67 is staleness-invalidated by the moved head (ADR 0058) and needs a fresh review against 3f6fac18.

@usirin

usirin commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ 3f6fac1 — merge-ready

Re-review after the CI-fix force-push. The prior review-code: PASS @ 75623d67 is staleness-invalidated (ADR 0058); this verdict supersedes it and is bound to the new head.

Class routing: class-probe classify --namespaces over the 22 changed files → has-code only ⇒ review-code is the sole required namespace. No docs, no skills, no apps/web/src/ path ⇒ no review-doc/review-skill/review-design dispatch.

§CP: NON-control-plane, verified against CONTROL_PLANE_RE re-resolved from origin/main (not the snapshot). The diff is entirely under packages/pipeline-crew-mcp/**; the regex's ^packages/pipeline-cli/ branch does not match pipeline-crew-mcp. Zero §CP paths matched ⇒ bindable verdict, auto-ships on green.

What changed since the prior PASS — scoped, not assumed

The raw 75623d67..3f6fac18 range spans main's advance (the force-push rebased). Scoped to the PR's own package, the delta is exactly one line, as claimed:

packages/pipeline-crew-mcp/src/protocol/describe.ts:62
-  catch: (cause) => cause,
+  catch: (cause) => String(cause),

Every other file differing between the two heads is main's advance, not PR work. The previously-verified surface is byte-identical.

The four verification points

  1. Behavior-preserving — confirmed. Effect.try's catch only maps the error channel; it cannot alter the success path. The mapped value is consumed two lines later by Effect.orElseSucceed(() => undefined), which discards it unread, so narrowing unknown → string is unobservable. describeKind's signature is unchanged (Effect.Effect<KindContract | undefined> — error channel already never via the orElseSucceed fold), and the undefined that resolveKindContractsFor turns into the loud boot failure is produced identically. The fix addresses the unknownInEffectCatch diagnostic only.
  2. Matches the in-package idiom — confirmed. src/standup/version-assert.ts:54 uses the identical catch: (cause) => String(cause) form on its Effect.try. This is convergence on the existing local convention, not a new one.
  3. Rest of the diff unchanged from the previously-passing review — confirmed (by the scoped delta above, then spot-re-verified):
    • No divergent catalog copy. describe.ts derives from CrewProtocol / crewMessageKinds / payloadSchemaForKind; contract.ts derives the role half from seamsFor / CrewSeams. The channel_kinds surface renders what those produce — no second kind list.
    • Send-time enforcement NOT weakened. src/edge/send-tool.ts (the pipeline-crew-mcp: channel_send validates nothing — the typed 7-kind catalog is advisory at the wire #3229 gate) is untouched by this PR — it does not appear in the changed-file set at all. send-tool.test.ts's edits are payload-literal migrations ("3057"3057) tracking the branded-number change; every assert.isTrue(result.isError) / isFalse assertion is preserved, including the unknown-kind and missing-required-field rejects.
    • Branded migration internally consistent. IssueNumber / PrNumber are both Schema.Int + >= 1 + distinctly branded; IntakePing.issue and both arms of the NudgeTarget union consume them coherently.
    • Boot invariant still fails closed. session.ts:210 yields resolveChannelContract() on the boot critical path before serving; an unresolvable kind set raises ChannelContractError naming the offending kinds.
  4. No machine-local/absolute/home paths in any added line (scanned the full package diff for /Users/, /home/, ~/, /private/tmp, /Volumes/) — clean.

Acceptance criteria (issue #3622)

# Criterion Verdict
1 Pre-send kind→schema surface, no divergent copy PASS — channel_kinds tool, derived from crewMessageKinds/payloadSchemaForKind
2 Per-role sanctioned kinds from the same surface PASS — roleContracts() off seamsFor/CrewSeams
3 Schema availability is a startup invariant PASS — resolved on the boot path; loud ChannelContractError
4 IntakePing.issue footgun resolved; other kinds reconsidered PASS — branded IssueNumber; NudgeTarget swept to PrNumber/IssueNumber
5 Unit coverage for the surface + the invariant PASS — describe.test.ts, kinds-tool.test.ts, contract.test.ts, schema.test.ts

Evidence run at this head

Verified in an isolated throwaway worktree materialized from a per-run ref at 3f6fac18 (never a checkout; instruction denylist removed and asserted absent per ADR 0052/0067).

  • Typecheck (authoritative, uncached): exit 0. The repo-wide pnpm typecheck returned FULL TURBO replaying another PR's cached logs, so I did not accept it — I re-ran tsgo directly on packages/pipeline-crew-mcp. Clean, and specifically no unknownInEffectCatch diagnostic, which is the exact regression this force-push fixes.
  • Tests: 42 files, 277/277 passed, 0 failed.

CI at this head — independently confirmed

  • lint / format / typechecksuccess (the reported tsgo exit-2 → turbo exit-139 teardown is resolved)
  • ci-requiredsuccess
  • packages unit tests, unit + client tests — success

Three non-green checks, all infrastructure, not code, and none in the blocking ci-required aggregate: produce run-evidence bundle (failed at Install crabbox (pinned)), detect deploy relevance (failed at dorny/paths-filter@v3.0.2), and one Analyze (javascript-typescript) attempt that also has a passing run at this same SHA. These are consistent with the ongoing GitHub REST 503 window and are not findings against this PR.

Run-evidence bundle: absent (its producing job failed on the infra step above). Per the graceful-degrade rule this is not a gate failure — and the degrade did not narrow the surface: I ran the full package suite, not a feature-scoped subset. The in-worktree typecheck + full suite are the authoritative signal here, so this is a full-trust PASS, not a fenced one.

Unresolved inline review threads: none. The inline-comments endpoint 503'd persistently, so I resolved it via the reviews endpoint, which read cleanly at count=0 — an inline review comment necessarily creates a review, so zero reviews entails zero unresolved threads. Nothing unaccounted reaches merge-ready.

Reviewed-head: @ 3f6fac1

@usirin
usirin added this pull request to the merge queue Jul 20, 2026
Merged via the queue into main with commit 2a00ac5 Jul 20, 2026
46 of 49 checks passed
@usirin
usirin deleted the usirin/3622-crew-mcp-kind-contract-discoverable-BFC43B75 branch July 20, 2026 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crew-mcp message kinds/schemas aren't uniformly available to every crew peer

1 participant