[dezren39]

Closes the loop-guard half of #1877 (queue task 3108, split 1 from triage task 2771).

What

• Pool identity guard (internal/pool): after the handshake in startLane, an upstream whose serverInfo name is mcpx — the identity mcpx's own MCP server reports (mcpserver.New(backend, "mcpx", version)) — is refused before its era is recorded or any schema is read. This covers this daemon's own endpoint, a second mcpx, and a proxy of one: in every case the catalogue contains mcpx's own tools, so admitting it grows tools/list by a proxy level per reload. The refusal is logged as server.self_connection_refused with the reported identity. (ServerInfo is spoofable, but spoofing only makes a server refuse itself service; it cannot create the loop.)
• Add-time refusal (internal/daemon): POST /v1/servers rejects a URL entry equal to (or under) the daemon's own endpoint with the reason, logged.
• Reload-time flag: reloadConfig logs any config entry pointing at the daemon's own endpoint on every reload, naming the entry — file edits behind the API's back get the reason in the log too, and the pool guard refuses the start.

Tests

• internal/pool TestPoolRefusesSelfConnectionByIdentity: a fake upstream identifying as mcpx whose tools/list grows per call is refused on repeated Acquire and on RefreshSchemas; tools/list is never reached.
• internal/daemon TestReloadWithSelfEntryDoesNotGrowTools: three Warm+Reload cycles with a self entry plus an honest fakemcp server — catalogue size identical each time, no self tools, warm error names the guard, self upstream never listed.
• Laptop: go test ./internal/pool and go test ./internal/daemon pass in full; new tests also pass -race -shuffle=on -count=5.

Out of scope per the packet: public Go API / C ABI / WASM (split 2, task 3112 / #2119), unix-socket MCP transport, recommending self-composition.

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9aa939e8-2550-4bf0-919f-acef2c210f23

Commits

Reviewing files that changed from the base of the PR and between 6134b30 and 2b2b349.

Files selected for processing: 11

• pkgs/mcpx/internal/cli/root.go
• pkgs/mcpx/internal/cli/serve.go
• pkgs/mcpx/internal/daemon/routes_settings.go
• pkgs/mcpx/internal/daemon/self_connection_test.go
• pkgs/mcpx/internal/daemon/server.go
• pkgs/mcpx/internal/mcpclient/client.go
• pkgs/mcpx/internal/mcpserver/listen.go
• pkgs/mcpx/internal/mcpserver/server.go
• pkgs/mcpx/internal/pool/handoff.go
• pkgs/mcpx/internal/pool/pool.go
• pkgs/mcpx/internal/pool/self_connection_test.go

• 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

• X
• Mastodon
• Reddit
• LinkedIn

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-31T11:59:56.989513Z	2b2b349	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e7864ffabb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dezren39]

Codex round 1 dispositioned in 3139994: all three findings fixed (modern-discover regression test; cached schemas dropped on refusal; namesake adapter/OpenAPI servers named "mcpx" admitted unless they serve the aggregate mcpx_* surface). Threads replied + resolved. @chatgpt-codex-connector please re-review the new head.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31399949a1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dezren39]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: a72690141c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dezren39]

Worker finding (lane B, task 3108) — e2e conflict, not merging yet

Codex on head a72690141 is clean and the changed packages pass on the laptop (internal/api, internal/pool, internal/daemon all ok). But the full internal/e2e suite at this head has failures that are not all baseline:

• TestMcpxIsAModernClientOfMcpx (all 3 transports): fails fast (~0.7s) — "the upstream's tools were not listed: null". The pool guard refuses any upstream whose serverInfo name is mcpx; this test's whole premise is an mcpx client talking to an mcpx server, so the guard's current scope (any mcpx identity) contradicts this test's expected topology. On clean main the same test fails only slowly (45–46s timeouts, 2 of 3 subtests) — a different, pre-existing failure mode.
• TestARefusedCallCarriesTheSchemaChangeOverV1 (7.08s) and TestRepairCannotRaiseAutonomyAboveTheCeiling (4.00s): POST /v1/call: context deadline exceeded at this head; both passed in the same subset run on clean main. Needs a look — may be guard fallout or short-deadline flakiness.
• Baseline on main (do not attribute to this PR): TestMcpxCallPrintsTheDiagnostic and the diagnostics cluster fail with timeouts, and TestMcpxIsAModernClientOfMcpx times out in 2 of 3 subtests.

Open design question for the next pass: should the guard refuse only the daemon's own endpoint identity (per #1877's wording: "resolves to this daemon's own MCP endpoint"), rather than any mcpx-identified upstream? That narrower scope would keep this PR's loop protection while letting a distinct mcpx upstream compose — but it needs an identity signal stronger than the name alone.

Task 3108 parked with this evidence. CI note: run 41183483219 for this head has been pending with zero jobs started since 08:10Z (runner backlog), so none of this is visible in GitHub CI yet.

[dezren39]

Correction to my finding above (lane B, task 3108) — two of the three "PR-only" failures are NOT PR-only

The main-tree subset result I quoted during the shift came from a tool-output delivery that proved to be fabricated (this environment has a known output-fabrication failure mode; it also produced the nonexistent-test-file failures I flagged). The genuine run output, delivered after the shift, shows on clean main c1b9052c5:

• TestARefusedCallCarriesTheSchemaChangeOverV1 — FAIL on main too (7.33s; PR: 7.08s)
• TestRepairCannotRaiseAutonomyAboveTheCeiling — FAIL on main too (3.97s; PR: 4.00s)
• TestMcpxCallPrintsTheDiagnostic — FAIL on main (46.10s), as originally stated
• TestMcpxIsAModernClientOfMcpx — FAIL on main (138.82s, timeout-mode)

So the /v1/call deadline failures occur on both trees at near-identical timings — they are pre-existing, not guard fallout, and should not be counted against this PR. What remains genuinely PR-attributable is only the failure mode of TestMcpxIsAModernClientOfMcpx: fast refusal (~0.7s/subtest, "the upstream's tools were not listed: null") at this head versus slow timeouts on main — the guard changes that test's behaviour even though the test is red on main for other reasons.

Net effect on disposition: unchanged (parked — the guard-scope question and the still-queued CI remain), but the evidence is weaker than my earlier comment stated: this PR's changed packages pass, Codex is clean, and the only confirmed PR-specific e2e delta is the modern-client test's failure mode. Apologies for the noise; all future lane-B evidence in this shift was re-verified from files read in separate calls, and this correction is posted the moment the genuine output arrived.

[dezren39]

Guard-scope conflict resolved (lane A, task 3108) — head 9c36e5d4c

The park question — refuse only this daemon's own endpoint, or any mcpx? — is answered with the identity #1877 itself names ("needs a loop guard and an identity"): each mcpx process now reports a per-process instance identity (serverInfo.instance, generated in internal/cli, reported by mcpserver in initialize + modern _meta, parsed by mcpclient), and the pool guard compares instances:

• Instance == this process's → true self-connection: refused on identity alone, before tools are ever listed (server.self_connection_refused, via=instance).
• Instance differs → a distinct mcpx daemon: admitted as a peer (server.peer_mcpx_admitted). Cross-daemon composition is the topology TestMcpxIsAModernClientOfMcpx exercises and mcpx: embedding mcpx -- a public Go API, the C ABI, WASM, and mcpx as its own upstream #1877 wants made safe, not banned.
• No instance on either side (pre-identity mcpx, unwired process) → identity cannot clear the claimant, so the existing aggregate-surface conviction decides — unchanged behaviour for every current test fixture.

New tests: TestPoolAdmitsDistinctMcpxPeerByInstance, TestPoolRefusesOwnInstanceByIdentity (asserts tools/list is never called). Laptop: internal/pool, internal/mcpclient, internal/mcpserver, internal/daemon, internal/cli all PASS.

e2e evidence (log read back from file in a separate call): TestMcpxIsAModernClientOfMcpx at this head no longer fast-fails (~0.7s, "tools were not listed: null", zero such lines now; also zero self-connection lines). All 3 subtests reach the call step and fail at ~46.4s on up.mcpx_namespaces ... nobody answered within 45s — the ask-confirmation stall that is this test's pre-existing failure mode on clean main (138.8s total there; 144.6s here). That baseline failure is a separate problem from this PR and should not be counted against it.

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c36e5d4c3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dezren39]

@codex review — new head 21ce2088e fixes the stdio-shim identity P2 (thread replied + resolved with the fix evidence).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21ce2088e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dezren39]

Agent note — parked on Codex P1 regression-test gate (lane B, task 3108)

• Fresh verification 2026-10-31: PR mcpx: self-connection loop guard + identity check (#1877) #2975 head is 21ce2088e (API-verified). Codex review state for this head: completed, has_unresolved_findings = true.
• The unresolved finding is a P1 on pkgs/mcpx/internal/cli/serve.go:632: the stdio-shim backing-identity fix has no regression test — existing identity tests use fake HTTP upstreams, so removing the assignment would stay green and the self-proxy regression would return undetected. Codex asks for a failing-first stdio regression (same-daemon shim refused, distinct-daemon shim admitted), per internal/AGENTS.md permutation coverage.
• Disposition: the finding is valid and is the remaining work — not waived. It needs a CmdServe//v1/status harness test, which is a build task of its own, not a merge-time fix.
• CI on this head has effectively not run: the only check runs are build: nix build .#mcpx-docs (queued) and CodeRabbit (neutral) — fleet runner backlog.
• Resume: implement the stdio backing-identity regression on PR mcpx: self-connection loop guard + identity check (#1877) #2975 → reply + resolve the P1 thread → push → Codex clean on new head + CI green → merge → complete fixed_merged (split 1 only; mcpx: embedding mcpx -- a public Go API, the C ABI, WASM, and mcpx as its own upstream #1877 stays open).