refactor: extract IMeshTransport abstraction + rewrite identity onto Node crypto (Phase 1 of #27)

[imran-siddique]

Phase 1 of AGT mesh migration (split from #27 per @pallakatos review).

Single file addition — zero risk:

• \ ransport-interface.ts\ — IMeshTransport abstraction defining the contract between app-level code and transport layer

What this enables: Swapping transport implementations (vendored SDK → AGT SDK) without changing connection logic.

What's NOT in this PR (deferred to Phase 2):

• Identity rewrite (needs connection.ts + index.ts updates)
• Vendor deletion
• SDK dependency swap

Verified locally:

• Zero new TS errors (upstream has 2 pre-existing @agentmesh/sdk\ type declaration errors)
• No existing code changes, no new dependencies
• CI: 12/13 pass (1 pre-existing Rust audit failure unrelated to this change)

Phase 2 blocked on AGT upstream: microsoft/agent-governance-toolkit#1404–#1407

[pallakatos]

Review — Phase 1 AGT Mesh Migration (#29)

Thank you for splitting #27 into a more reviewable shape — the scope reduction (3 files, 240/-125) is exactly what I asked for. However, this PR does not compile and introduces a functional regression that CI misses. It cannot be merged as-is.

🔴 Blocker 1 — TypeScript build is broken

Checking out this branch locally and running cd mesh-plugin && npm run typecheck:

src/connection.ts(172,73): error TS2339: Property 'sdkIdentity' does not exist on type 'MeshIdentity'.
src/index.ts(641,60): error TS2554: Expected 0 arguments, but got 1.

Why:

• identity.ts removes the sdkIdentity: Identity field from MeshIdentity, but connection.ts:172 still calls sdk.AgentMeshClient.fromIdentity(this.config.identity.sdkIdentity, …). This is the single entry point that bootstraps the mesh client — without it, the plugin cannot establish any session.
• index.ts:641 calls meshIdentity.signingPublicKey.toString("base64"), which worked when the field was Buffer but fails now that it is Uint8Array (no encoding argument).

Why CI didn't catch it: .github/workflows/ci.yml has no mesh-plugin build step. CLI Build & Test only compiles cli/, and Container Image Scan uses a cached sandbox base image when Dockerfile.base is unchanged — so the sandbox image, which is the only place mesh-plugin is actually compiled, is never rebuilt on this PR. Please add a Mesh Plugin Build & Test job (mirroring CLI Build & Test) as part of this PR so regressions like this are caught at PR time rather than at deploy time.

🔴 Blocker 2 — X25519 exchange keys silently dropped

The old MeshIdentity carried both Ed25519 signing keys and X25519 exchange keys (via the SDK Identity.toData() blob persisted in schema-2). The new schema-3 persists only the Ed25519 signing material.

Signal Protocol's X3DH handshake — which is how every knock/session/file-transfer currently establishes a session — requires X25519 exchange keys. Dropping them is not a refactor, it is a functional removal.

Two acceptable paths:

1. Derive X25519 on demand. The AGT SDK v3.3 in /Users/pallakatos/Private/Repos/agt/agent-governance-toolkit/packages/agent-mesh/sdks/typescript/src/encryption exports ed25519ToX25519() exactly for this. If Phase 1 must be identity-standalone, call that helper inside generateIdentity() and persist both keys in schema-3.
2. Keep the SDK Identity object in memory. If the point of Phase 1 is only to drop the on-disk dependency on IdentityData, keep sdkIdentity as a transient (non-persisted) field derived from the schema-3 keys at load time via Identity.fromData().

Either way, connection.ts:172 must keep working, and the schema-3 envelope comment ("no X25519 needed") must be corrected.

🔴 Blocker 3 — IMeshTransport is unused scaffolding

transport-interface.ts introduces IMeshTransport + IMeshIdentity but:

• Nothing implements IMeshTransport. Not the vendored SDK, not an AGT adapter.
• MeshConnection (the current concrete implementation) doesn't claim implements IMeshTransport, so the compiler never verifies that the interface actually matches reality.

Please either:

• Have MeshConnection implements IMeshTransport in this PR (it should already satisfy most of it — connect/disconnect/send/onMessage/addPlaintextPeer/…), which is the whole point of extracting the interface, or
• Defer the interface to the PR that introduces the second implementation.

An interface with zero implementors is documentation, not abstraction.

🟡 Minor issues

1. loadSchema2 path is now unreachable for Signal consumers. Legacy files had X25519 keys stored inside the ciphertext blob. Your migration silently drops them and derives a new identity with only Ed25519, preserving AMID but breaking any existing Signal sessions keyed to the previous X25519 public key. Add a warning log (or force a re-knock) so operators know the upgrade invalidates in-flight sessions.

2. console.log("[mesh] Migrated identity …") in loadOrCreateIdentity goes to stdout uninstrumented. Please route through _log.info (the redactor in cli/src/plugin.ts that landed in #32) so nothing secret leaks.

3. Schema-3 envelope reuses the same deriveEncryptionKey() (hostname+homedir). Fine for Phase 1, but note that this is not a KDF against a stable secret — if the home directory moves, the identity file becomes unreadable. Not a change from schema-2; leaving as-is for now but worth documenting.

4. Test coverage regression. The schema-2 migration path is not tested. Please add a test that writes a schema-2 envelope with known keys and asserts loadOrCreateIdentity() returns the same AMID + triggers a save to schema-3.

✅ What I like

• Scope is right — 3 files, no vendor deletions.
• The schema-3 envelope is cleanly factored (hex-encoded keys, AES-256-GCM on private key only).
• Auto-migration preserves AMID (important — prevents every paired peer from having to re-knock).
• Phase 2 tracked in upstream AGT issues #1404-#1407 — happy with that approach.

Requested changes before merge

• Fix the two TypeScript errors (either preserve sdkIdentity or update connection.ts/index.ts to the new shape).
• Persist or derive X25519 exchange keys — document which.
• Either make MeshConnection implements IMeshTransport or remove IMeshTransport from this PR.
• Add a Mesh Plugin Build & Test CI job so this class of regression can't recur.
• Test schema-2 → schema-3 migration.
• Route migration console.log through the redactor.

Happy to re-review as soon as the above are addressed. The architectural direction is right; the execution just needs one more pass.

[pallakatos]

Follow-up — terminology clarification for this review

To avoid ambiguity in my inline comments, here's the distinction I'm drawing between the two SDKs. Please use the same labels in replies:

Label	Package	Source	Role	Where
Vendored SDK	@agentmesh/sdk v0.1.2	fork of amitayks/agentmesh with 8 local patches	Current runtime. What connection.ts imports today via import * as sdk from "@agentmesh/sdk". Provides Identity, AgentMeshClient.fromIdentity, MemoryStorage, Signal Protocol.	vendor/agentmesh-sdk/, consumed in mesh-plugin/src/*.ts
AGT SDK	@microsoft/agent-governance-sdk v3.3.0	microsoft/agent-governance-toolkit at packages/agent-mesh/sdks/typescript/	Migration target. Provides AgentIdentity, MeshClient, X3DHKeyManager, ed25519ToX25519(), etc. Different API surface.	Not yet imported anywhere in this repo

With that mapping applied, my blockers read as:

• identity.ts:152 (sdkIdentity removal) — breaks connection.ts:172, which calls AgentMeshClient.fromIdentity() from the vendored SDK. Until the AGT SDK adapter exists, removing sdkIdentity severs the live code path. The fix is either (a) keep sdkIdentity as a transient field derived from schema-3 keys via the vendored SDK's Identity.fromData(), or (b) land the AGT MeshClient-based replacement in the same PR.

• identity.ts:224 (X25519 dropped) — X25519 keys are needed by both SDKs. The vendored SDK stores them inside IdentityData; the AGT SDK re-derives them via ed25519ToX25519() (exported from agent-governance-sdk/dist/encryption/x3dh.js). If Phase 1 is going to drop SDK-owned identity persistence, schema-3 either needs to persist X25519 directly or the loader needs to derive them on read (the AGT helper works with only the Ed25519 key material you already store). Either way, drop must not go to disk without a derivation path.

• transport-interface.ts:47 — the second implementation is the AGT adapter (not the vendored SDK, which already has its own concrete types). I'm fine with the interface existing in Phase 1 only if MeshConnection (vendored-SDK-backed, current implementation) is annotated implements IMeshTransport in the same PR so the interface is grounded in the live code.

• Phase 2 tracking — the AGT upstream issues (#1404-1407) are in microsoft/agent-governance-toolkit, i.e. the AGT SDK side. That's where gaps in the target SDK get fixed. They do not supersede the vendored SDK until all four are resolved and the adapter in mesh-plugin/ is wired up and passing CI.

Net: no change in the requested changes, just making sure we mean the same thing when we say "SDK". This PR currently relies on the vendored SDK still being wired in, but removes one of the fields the vendored SDK consumer depends on — that's the core contradiction to resolve.

[pallakatos]

Correction — AGT SDK is already in use (Rust side)

I overstated the "not yet imported anywhere" line. Accurate picture:

Layer	SDK in use today	Where	Notes
inference-router (Rust)	AGT — agentmesh = "3.1.0"	Cargo.toml:77, inference-router/src/governance.rs:8-16 (AuditLogger, AgentIdentity, PolicyEngine, TrustManager, McpSlidingRateLimiter, CredentialRedactor, McpResponseScanner, InMemoryAuditSink, PolicyDecision)	Already the governance backbone
controller (Rust)	none	—	—
mesh-plugin (TS)	Vendored — @agentmesh/sdk v0.1.2	mesh-plugin/src/connection.ts, identity.ts via vendored vendor/agentmesh-sdk/	The migration target in this PR
cli (TS)	none (uses mesh-plugin indirectly)	—	—

So the migration in PR #29 is specifically the TypeScript mesh-plugin side: swap the vendored @agentmesh/sdk (v0.1.2, amitayks/agentmesh fork) for the AGT TypeScript SDK @microsoft/agent-governance-sdk v3.3.0 at packages/agent-mesh/sdks/typescript/.

The Rust agentmesh crate 3.1.0 already ships the governance pieces we consume (audit / policy / trust / rate-limit / MCP redactor / response scanner). The TS SDK 3.3.0 ships the mesh client pieces we need here (MeshClient, X3DHKeyManager, ed25519ToX25519, SecureChannel, etc.).

None of the blockers change — they apply to the TS side — but this sharpens what Phase 1/2/3 of "mesh migration" actually means:

• Phase 1 (this PR): drop vendored Identity dependency in mesh-plugin/src/identity.ts → Node crypto + transient/derived X25519.
• Phase 2 (AGT upstream): fix gaps in @microsoft/agent-governance-sdk TS (the four issues you tracked: #1404-1407).
• Phase 3 (follow-up PR): rewrite mesh-plugin/src/connection.ts onto AGT TS MeshClient and delete vendor/agentmesh-sdk/.

So it's a partial migration rather than a greenfield one — the AGT backbone is already doing governance work in-proc inside the inference router. That's a good position to be in, but it also means we need to be extra careful not to end up with two identity systems (Rust AGT vs TS vendored) that disagree on AMID derivation.

Worth validating: does AGT Rust's AgentIdentity derive AMID identically to the vendored TS SDK (base58(sha256(pubkey)[:20]))? If they disagree, any rust-side identity lookups will miss the plugin's AMID. Could be a Phase 2 acceptance test.

[imran-siddique]

Thanks for the thorough review @pallakatos — addressed all items:

Blockers 1 & 2 (identity.ts)

Already resolved in the previous push — identity.ts was reverted out of this PR. The PR now touches only transport-interface.ts and connection.ts. Identity refactoring (schema-3, X25519 derivation) will come in a follow-up PR after the upstream AGT issues (#1405, #1406) land.

Blocker 3 (IMeshTransport unused) ✅

Fixed — MeshConnection implements IMeshTransport is now in place. I also grounded the interface to reality: trimmed it down to only the methods MeshConnection already implements (connect, disconnect, isConnected, send, addPlaintextPeer, removePlaintextPeer, getPlaintextPeers, discover). Removed aspirational members (agentId, onMessage, onKnock, isPlaintextPeer, search, sendHeartbeat, IMeshIdentity) that had zero implementations.

npx tsc --noEmit passes with only the pre-existing TS7016 warnings for the vendored @agentmesh/sdk (no .d.ts file).

CI gap ✅

Added Mesh Plugin Build & Test job to .github/workflows/ci.yml — runs npm install, npm run typecheck, npm run build, npm test on every PR against main.

Minor items

• Schema-2 migration test, console.log → redactor: deferred since identity.ts is no longer in this PR
• Schema-3 envelope docs: will come with the identity PR

This PR is now Phase 1 only: transport abstraction interface + MeshConnection implements it + CI coverage. Clean, grounded, compilable.

[imran-siddique]

@pallakatos All CI checks are now green (including the new Mesh Plugin Build & Test and Python Lint & Test (AGT Sidecar) jobs). Summary of changes since your review:

1. IMeshTransport grounded — MeshConnection implements IMeshTransport. Interface trimmed to only methods the class actually implements.
2. identity.ts reverted — not in this PR. Identity refactoring deferred to Phase 2 after upstream AGT fixes land.
3. CI coverage added — Mesh Plugin typecheck + build + test job, Python sidecar stub (gracefully skips when no sidecar code present).
4. @agentmesh/sdk type declarations — agentmesh-sdk.d.ts with typed Identity, AgentMeshClient, MemoryStorage declarations eliminates TS7016 errors.
5. Vendored SDK deps — CI installs libsodium-wrappers via --ignore-scripts before tests.

Ready for re-review and merge when you're satisfied. 🙏

[pallakatos]

Review — clean Phase 1, ready to merge with two minor nits

I pulled cd199d7 locally and reran the full mesh-plugin lifecycle:

Check	Result
npx tsc --noEmit	✅ clean
npm run build	✅ clean
npm test	✅ 36 / 36
Verified MeshConnection exposes every member of IMeshTransport	✅ all 8 (connect, disconnect, isConnected, send, addPlaintextPeer, removePlaintextPeer, getPlaintextPeers, discover) — signatures match exactly
agentmesh-sdk.d.ts covers actual usage	✅ only Identity, IdentityData, MemoryStorage, AgentMeshClient, RegistryClient, VERSION are imported by mesh-plugin/src/*.ts
security-audit-required.sh capability regex	✅ mesh-plugin/ is not in CAP_RE, so no audit doc required for this PR

Approve, with two nits (optional, fine to land as-is)

1. connection.ts:104-108 import placement — the new import { IMeshTransport } is wedged between two section-header comment delimiters:

   // ---------------------------------------------------------------------------
   // Connection class
   import { IMeshTransport } from "./transport-interface.js";
   // ---------------------------------------------------------------------------
   
   export class MeshConnection implements IMeshTransport { ... }

   Stylistically the import belongs in the top-of-file import block alongside the other module imports. Functionally fine — TS doesn't care — but a future reader will. Cheap follow-up.

2. agentmesh-sdk.d.ts lives inside mesh-plugin/src/ — fine for now, but it'll be emitted into dist/ and shipped via "files": ["dist", ...] in package.json. If the vendored SDK ever publishes its own .d.ts, both will compete. Low risk today (vendored SDK has no published types). Worth a future move to mesh-plugin/types/ with tsconfig.json typeRoots if it ever matters.

Phase 2 reminder (not blocking)

Per your earlier comments, the AGT TS SDK migration still depends on upstream microsoft/agent-governance-toolkit issues #1404–#1407 landing. When those close, the follow-up will be:

• Add AgtTransport implements IMeshTransport next to MeshConnection
• Schema-3 identity envelope (X25519 derivation via ed25519ToX25519() so we don't persist the keys)
• Swap the import("@agentmesh/sdk") call site in connection.ts for an injected IMeshTransport
• Delete vendor/agentmesh-sdk/ (8 patches will be upstreamed or obviated)

The interface this PR adds is exactly the shape that swap will need, so this is groundwork well laid.

Merge order

PR #44 (dev → main) is currently in CI greenup (just pushed 71abb07 + 98977ac). Once #44 lands, this PR auto-rebases cleanly — no overlap. Mergeable as soon as I re-tick.

LGTM.

[pallakatos]

Hi @imran-siddique 👋

PR #44 (Phase 0 + Phase 1 dev → main integration) just merged into main as e78f2ef. Could you rebase this PR onto the new main when you get a chance? The rebase should be clean — dev had zero commits touching mesh-plugin/ since the last common ancestor, so your IMeshTransport abstraction sits orthogonal to everything that just landed.

A couple of things worth knowing on the new main:

• Branch protection now requires the 4 status checks CLI Build & Test, Rust Build & Test (Controller + Inference Router), Helm Lint, Security Scan (the orphan Python Lint & Test (AGT Sidecar) was removed) plus 1 approving review.
• Phase 1 added 7 blocking CI gates: loc, no-stubs, no-custom-crypto, no-null-provider-prod, vendored-patch-audit, a2a-module-isolation, security-audit-required. None should affect a mesh-plugin/-only change, but worth knowing if any flag.
• For the AGT-upstream blockers (microsoft/agent-governance-toolkit#1404–#1407), no movement on our side — happy to coordinate when those land.

Thanks for the patient split into Phase 1 / Phase 2 — much easier to land this way 🙏