Skip to content

fix(mcp): preserve native provider identity custody - #111

Merged
kjgbot merged 2 commits into
mainfrom
fix/provider-binding-participant-isolation
Nov 11, 2026
Merged

kjgbot merged 2 commits into
mainfrom
fix/provider-binding-participant-isolation

Conversation

@kjgbot

@kjgbot kjgbot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

A native provider binding must keep custody of its Relay identity after disconnect while remote cleanup is pending. Previously, generic join_relay could treat a same-owner Factory remote as an interrupted generic join, recover its token and insert a participant row even though the Factory binding was retired and its own resolver refused it.

Extend the Capy isolation from #110 to Factory and Amp at both generic participant ownership and OAuth name availability. The shared metadata check reserves each provider's binding marker and creator marker, including malformed own-property binding IDs. Ordinary capy-/factory-/amp- names remain available; explicit provider paths retain their identities. No permission, migration, runtime flag or deployment change.

Validation:

  • Real Factory bind/disconnect with loopback fake provider and Relaycast services reproduces the pending-cleanup adoption before the fix. No production or real provider action.
  • Four desired regressions fail against the previous source; reverting only Factory/Amp classification reproduces all four failures while retaining Capy protection.
  • aw-heavy -- node --import tsx --test --test-concurrency=1 test/capy-mcp-identity.test.ts test/factory-mcp.test.ts test/mcp-oauth-admission.test.ts test/amp.test.ts: 74 pass.
  • aw-heavy -- npm run typecheck and aw-heavy -- npm run build: clean.
  • aw-heavy -- node --import tsx --test --test-concurrency=1 test/mcp.test.ts test/mcp-desktop-link.test.ts: 47 pass (121 combined focused checks). Current-head required CI and independent review remain the merge gates.

This is an adjacent custody fix in the native-provider parity work tracked by #99. It does not complete provider delivery, background history or real-account E2E.


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Pre-merge checks: Inconclusive 2

Failed Failed checks: 2 inconclusive

Check name Status Explanation Resolution
Title check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
Description check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@agent-relay-code agent-relay-code Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE on exact head 7961ba3 (base main e598c30, #110).

Custody contract checked against the diff and the provider services:

  • isBindingOwnedIdentity (src/messaging/binding-identities.ts) covers every marker each provider actually registers: Capy created_by: capy-binding / capy_binding_id (src/capy/bindings.ts), Factory created_by: factory-binding / factory_binding_id (src/factory/service.ts), Amp created_by: amp-binding / amp_binding_id (src/amp/service.ts). Key presence or created_by value alone is enough, so a partial marker set still fences.
  • ownsRemote in src/mcp/relay-participants.ts is the only generic adoption gate (participant() and registerParticipant()), so retired or disconnected Factory/Amp identities are no longer recoverable through generic join_relay; the provider resolvers keep their own binding checks and are untouched.
  • Consent availability in src/app.ts reports the same identities as taken, so OAuth consent cannot adopt them either. OAuth assertActor and the pin are untouched; no flag, no pin change, no migration (0046 stays free).
  • Tests: the marker matrix now runs for all three providers; test/factory-mcp.test.ts reproduces the failed-cleanup recovery for both retired and live custody and asserts no token rotation, no participant row, live binding still resolvable, and ordinary factory-support join unaffected; the admission test asserts 9 provider markers.

Non-blocking observation: a generic participant row adopted from a provider remote before this fix will now fail participant() with identity changed and must re-join under a non-provider name. That is the correct outcome, noting it for anyone reading support logs.

Local checks on 7961ba3 (layered-heavy, one worker, serial, 4 GiB guard): typecheck exit 0; capy-mcp-identity 6/6; factory-mcp 8/8; amp 14/14; mcp-oauth-admission run by name pattern for the renamed admission test 1/1 (the whole file is guard-killed at test 18 on main too). test/mcp.test.ts was killed by the memory guard after test 18, so that local run is incomplete, not a failure; the file is not part of this PR. Exact-head CI 'Typecheck and test' run 42995768793 on 7961ba3: success.

Review-only: I do not merge.

🤖 Generated with Claude Code

@kjgbot

kjgbot commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Review5856661239 is addressed. A generic participant previously adopted from a provider identity now fails closed with identity changed; matching owner metadata does not restore authority to that alias. Recovery must use an ordinary hosted identity through the normal join/reconnect path (a fresh native MCP session when the old session is bound; a new ordinary connection when an OAuth grant is pinned). Do not recover or release the provider remote through generic participant cleanup. Its provider binding retains custody. No existing production alias is asserted or automatically rewritten by this PR.

CodeRabbit's visible title/description checks are inconclusive with no code finding/location; the check is skipped. Required exact-head CI42995768793 is SUCCESS, approval is on7961ba3 and there are0 unresolved threads. Author validation includes121 focused checks, clean typecheck/build and4 named failures when only Factory/Amp classification is reverted. The reviewer's memory-guard-terminated runs are explicitly incomplete; they are not represented as full-suite proof. Runtime/E2E remain tracked in#99.

@kjgbot
kjgbot merged commit 379f811 into main Nov 11, 2026
3 checks passed
@kjgbot
kjgbot deleted the fix/provider-binding-participant-isolation branch November 11, 2026 09:37
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.

1 participant