fix(inputs): a codex_config source is one loaded source per MCP server

[pengfei-threemoonslab]

Summary

• A codex_config tool source no longer aborts the scan on any MCP server it finds. A tool_sources[] row of type: codex_config failed outright with InputParseError whenever the workspace contained a .mcp.json or .codex/config.toml declaring a server. Since a server with no enumerable tools still mints a <server>.* wildcard, any server at all reached it — the source type worked only on workspaces that had no MCP declarations to read, which is the opposite of what it is for.
• The loader-contract failure now offers a way forward. "That is a defect in the loader, not in this repository's configuration" was accurate and terminal. When the dispatcher can prove which tool_sources entry produced the read, the message now names it, after the diagnosis and labelled as the workaround it is.

The defect

inputs.mcp_manifest minted a per-server id for every tool (mcp_json:<server>, codex_config_mcp:<server>) and then wrapped a whole file's servers in one LoadedToolSource named for the file (codex_config_mcp:<path>). core.tool_identity requires a loaded source to name the id its tools carry — a tool arriving under another source's name is a loader defect — so the two spellings could never agree.

Reported for .mcp.json; .codex/config.toml was equally broken (codex_config_mcp:<path> source holding codex_config_mcp:<server> tools). Both are fixed and both are parametrized in the regression test.

Which side is wrong, and why

The loader's shape, not its ids. It now emits one LoadedToolSource per MCP server, built from the server, so there is no second place that spells the id.

The other candidate repair — stamp the tools with the file-level id — is wrong. _native_locator is the bare tool name for MCP-like sources, so collapsing a file's servers into one source makes a legal .mcp.json whose two servers both expose query fail with "defines the tool 'query' more than once": a false error with no repair. Verified directly, and pinned by test_two_servers_in_one_file_may_expose_the_same_tool_name.

The minted ids themselves are unchanged and stay deliberately path-free. I first also qualified them per file, to keep two packages that each declare github apart — that fails test_mcp_audit_same_adapter_pure_rename_is_not_a_capability_addition, because an MCP capability is its server and tool, so moving a .mcp.json must not read as a capability addition. Reverted, and the rejected option is now pinned by test_the_minted_server_id_stays_free_of_the_path_it_was_read_from so it isn't rediscovered.

Why no test caught it

The three existing type: codex_config fixtures (test_verify_orchestrator.py:351, test_preflight.py:470, test_codex_boundary_check.py:757) all point at workspaces where the loader mints no tools, so nothing reached the check.

Type

• Input adapter change
• Check or risk-model change
• CLI or GitHub Action behavior
• Report, schema, or SARIF output
• Documentation only

Verification

CI is authoritative for python -m ruff check ., python -m compileall -q src tests, and python -m pytest.

Additional local checks run:

• Full suite the way CI runs it: pytest -n auto -m "not perf" --ignore=tests/test_adapter_static_only.py, then tests/test_latency_budget.py -m perf and tests/test_adapter_static_only.py separately — all exit 0. ruff check src tests clean.
• 7 of the 8 new tests fail on unpatched src; the 8th is the rename-invariant pin that must pass both ways by design.
• Two perturbations, to check the guards are load-bearing rather than incidental: dropping the one-source-per-server shape fails 6 tests, and file-qualifying the minted id fails the mcp audit rename test.
• The reported repro scans to exit 0 through ./shipgate scan.

Release-readiness notes

• No user-code import added to default scan paths
• No network access added to default scan paths
• New or changed check IDs are documented in docs/checks.md — n/a, no check IDs change
• Report/schema changes are additive or documented in STABILITY.md — no version moves (report_schema_version, contract_version, every published schema document unchanged). Nothing that previously produced a report changes shape, because no workspace with an MCP server could produce one. details gains configured_source_id (additive).

Reviewer notes

Known gap, deliberately out of scope. Two files of the same kind declaring the same server name — pkg_a/.mcp.json and pkg_b/.mcp.json both declaring github, likely in a monorepo since server names are conventional — still fail:

• scan with a duplicate-observation message naming a file the manifest never listed twice;
• mcp audit with an unhandled ConfigError: Duplicate capability ids are not allowed and no JSON envelope — this one is independent of this PR and reproduces on origin/main.

Both come from the same tension: path-free MCP capability identity and one-observation-per-identity are each deliberate. Reconciling them means deciding whether one repo declaring a server twice is one capability or two, and since the declarations may disagree, any merge rule is fail-open unless it records what it dropped. That is a design call for its own issue, not a drive-by inside this fix. Tracked separately.

No issue reference. No issue is filed for this bug. Happy to open one and add the citation to the comments if you'd like the usual provenance.

🤖 Generated with Claude Code

[pengfei-threemoonslab]

Expert review found four actionable issues: two catalog-completeness/identity blockers and two normalization/recovery defects. The per-server split fixes the reported happy path, but some accepted inputs can still disappear from the analyzed surface or be conflated under one provider.

Validation performed on this exact head merged with current main (fc96bad91):

• 367 relevant adapter/identity/binding/verification tests passed.
• Ruff and compileall passed on the touched files/tree.
• All current GitHub checks are green.
• Agents Shipgate verification receipt validated; release decision passed, merge verdict mergeable, control complete. Those static checks do not exercise the adversarial input shapes called out inline.

I recommend fixing the P1 findings before merge and covering all four repros with end-to-end tests.

[pengfei-threemoonslab]

[P1] Preserve rejected MCP servers as typed omissions

_servers_from_mapping() silently skips every non-object server value, and this new return publishes only the normalized rows. A mixed input such as {"mcpServers":{"good":{"command":"node","tools":{"read_docs":{}}},"dropped":"not-an-object"}} now completes run_scan with good in tool_catalog, source_warnings=[], and no SourceSurfaceOmission for dropped. Before this PR the valid row hit the loader mismatch and aborted; after this change the affected config can reach a report with part of its declared surface missing. Please reject malformed server rows or attach a warning plus typed omission/evidence gap, following inputs/mcp.py, for JSON, top-level TOML, and plugin-nested maps. Add mixed-valid/invalid end-to-end tests.

[pengfei-threemoonslab]

[P1] Fail closed when a path-free provider ID is not unique

The path-free key is not unique among simultaneous declarations. Two nested .mcp.json files that both declare server github with disjoint tool names produce two sources named mcp_json:github; _observations accepts both because their native locators differ, silently assigns both tools one provider, and emits no warning. Only overlapping tool names hit the documented failure, so the changelog's claim that duplicate server declarations always fail is incomplete. The raw : composition also collides for plugin a:b / server c versus plugin a / server b:c. Track the origin tuple and fail closed when one minted ID comes from multiple origins, or use an injective component encoding while keeping relocation equivalence separate. Please add disjoint cross-file and delimiter-collision tests.

[pengfei-threemoonslab]

[P2] Canonicalize server IDs before enforcing source equality

A server key ending in whitespace still triggers the exact loader defect this PR is meant to remove. With .mcp.json containing {"mcpServers":{"srv ":{"command":"node","tools":{"query":{}}}}}, this row preserves mcp_json:srv while _observations strips the loaded ID to mcp_json:srv before comparing it with the unstripped tool ID, so run_scan aborts. Normalize or reject outer whitespace once when building the server identity (while preserving the raw key for source pointers), apply the rule consistently to JSON/TOML/plugin forms, and add end-to-end regressions.

[pengfei-threemoonslab]

[P2] Route loader defects without recommending source deletion

This workaround removes the only configured source, so the next scan omits that capability entirely. Even though the sentence discloses the cost, deletion is coverage-bypass guidance from a merge gate. It also does not satisfy the agent-mode recovery contract: the details have no typed failure, so input_parse_recovery() returns the generic review action with no path, while this prose hardcodes shipgate.yaml even when the actual manifest is nested or named differently. Keep the hard failure, add a stable mismatch discriminator and a recovery branch that uses details.manifest_path, and route to fixing/upgrading the adapter or explicit human review. If exclusion is intentional, require a reviewed, verifier-gated manifest change rather than presenting deletion as scanner recovery.

[pengfei-threemoonslab]

Second-pass review of current head d0de953 after merging current main (b867f31, #445).

Disposition: changes still needed before merge. This review is posted by the PR author account, so GitHub permits only COMMENTED, but the remaining P2 is actionable.

• P2 — route loader defects without recommending source deletion remains PR-specific. A mismatching adapter now tells the user to remove its tool_sources row, so the next scan omits that capability surface. The exception still has no typed failure; agent-mode recovery falls through to an unrelated generic action with no path, while the prose hardcodes shipgate.yaml and points at report.json even though this abort can occur before that report is created. Keep the hard failure, add a stable mismatch discriminator plus manifest-aware recovery, and route to adapter repair or upgrade, or explicit human review. Do not present deleting the gated source as scanner recovery.

Status of the earlier threads after the merge:

• #445 now supplies the primary per-server loader fix and conservative same-server reconciliation across files read by one codex_config row.
• Malformed server omission, outer-whitespace IDs, and the remaining cross-config or delimiter identity gaps still reproduce on current main. They are follow-up base defects, not behaviors introduced by the current net diff, so I am no longer treating them as blockers unique to #442.

Validation on exact head d0de953: the focused MCP, identity, diagnostics, permissions, authority, and exclusion suite passed (235 tests); Ruff and bytecode compilation passed; all current GitHub checks are green. Agents Shipgate verification against b867f31 passed and its receipt reproduced as valid. Shipgate is static evidence only and does not supersede the code-review finding above.