fix(mcp): phase agents receive the user's entire Claude Desktop config — capability over-grant + literal secrets in argv

[admarble]

Symptom

With settings.run.mcp enabled (the default), every headless phase agent
receives the user's ENTIRE Claude Desktop config:
getMcpServersConfig() (src/lib/system.ts:179) reads
claude_desktop_config.json and claude-code.ts:119 passes the whole
object to the SDK query().

Two consequences, both observed in the 2026-08-10 overnight run on
ad-motion:

1. Capability over-grant. Autonomous phase agents inherited an
   email-sending MCP server (SMTP send-as), a Stripe server, a Google
   Sheets server with a service-account file, a GitHub server with a PAT,
   and a filesystem server rooted at ~/Desktop + ~/Documents. A phase
   agent implementing a GitHub issue needs none of these — and fix(containment): a phase agent killed its own orchestrator — no Bash signal guard, no process-group isolation, machine re-prompts read as human consent #934's
   incident shows phase agents will use surprising capabilities when they
   misread a situation.

2. Secrets in argv. The SDK serializes the merged config into
   --mcp-config <json> on the claude child's command line. Claude
   Desktop configs cannot use ${VAR} references (the desktop app does
   not expand them), so they contain literal secrets — which are then
   visible to ANY local process via ps, and get captured into agent
   transcripts whenever an agent runs ps aux (this is exactly how live
   credentials ended up in the fix(containment): a phase agent killed its own orchestrator — no Bash signal guard, no process-group isolation, machine re-prompts read as human consent #934 incident transcript).

Proposed fix

Phases should get an allowlist, not a passthrough:

• Default: inject ONLY the sequant MCP server (and anything the project's
  own .mcp.json declares — that file is designed to be secret-free).
• Never read claude_desktop_config.json for phase execution. It is a
  different trust domain: servers a human uses interactively in the
  desktop app, not servers an autonomous agent should hold.
• If passthrough is ever wanted, make it opt-in per server:
  settings.run.mcpAllowlist: ["name", ...].

AC

• Phase agent argv contains no MCP servers beyond sequant + project
  .mcp.json entries (test: spawn a phase with a populated fake
  desktop config; assert its --mcp-config excludes those servers)
• No secret-bearing env values from the desktop config appear in the
  phase process argv
• Docs: run.mcp documented as sequant-server injection, not
  desktop passthrough

Related: #161 (introduced the passthrough), #934 (containment incident
whose transcript captured the leaked credentials), #395.

[admarble]

Plan: replace desktop-config passthrough with an explicit phase MCP allowlist

Complexity: Standard (3 ACs, 2 directories)

Acceptance Criteria (canonicalized — original checkboxes were unlabeled + line-wrapped, so the AC parser extracted 0; these are the machine-parseable equivalents, stored in state)

• AC-1: Phase agent MCP config (--mcp-config argv / SDK mcpServers option) contains no servers beyond sequant + project .mcp.json entries — with a populated fake desktop config present, its servers are excluded
• AC-2: No secret-bearing env values from the desktop config appear in the phase process argv (--mcp-config serialization)
• AC-3: Docs describe run.mcp as sequant-server injection, not desktop passthrough

Verification: AC-1 Unit + Integration Test · AC-2 Unit Test · AC-3 Manual (docs review)

Design

The single production call site is src/lib/workflow/drivers/claude-code.ts:119 (config.mcp ? getMcpServersConfig() : undefined), downstream of both ExecutionConfig producers (config-resolver and ready-gate), so one driver-level change covers every phase path, including the cold-start MCP fallback in phase-executor.ts:1621.

1. New builder in src/lib/mcp-config.ts (it already owns .mcp.json + getSequantMcpConfig()): getPhaseMcpServersConfig(cwd) reads <cwd>/.mcp.json mcpServers (graceful on missing/corrupt, same try/catch pattern as createProjectMcpJson), and guarantees a sequant entry via getSequantMcpConfig() (no clientType → no cwd, no env, no API key). It never touches claude_desktop_config.json.
2. Driver wiring: claude-code.ts:119 → config.mcp ? getPhaseMcpServersConfig(config.cwd) : undefined. cwd is the worktree, and .mcp.json is committed, so worktrees resolve their own copy.
3. getMcpServersConfig() stays in system.ts for doctor.ts only (interactive diagnostics, different trust domain) — docstring gains a "never pass to phase execution" warning.
4. settings.run.mcpAllowlist opt-in passthrough: deferred (issue phrases it as conditional; YAGNI until someone asks). Migration path for optional MCPs is declaring them in project .mcp.json.

Implementation Plan

Phase 1 — allowlist builder + wiring

1. Add getPhaseMcpServersConfig(cwd) to src/lib/mcp-config.ts (project entries ∪ guaranteed sequant entry).
2. Rewire claude-code.ts:119; drop the getMcpServersConfig import from the driver.
3. Empirically verify no double-registration: the SDK runs with settingSources: ["project"], which may auto-load .mcp.json in addition to the explicit mcpServers option (today's desktop passthrough already duplicates the sequant name and works, so expected fine — confirm).

Phase 2 — tests (driver tests already mock query() — clean assertion seam)
4. Unit tests for the builder: fake HOME containing a populated claude_desktop_config.json (servers with literal secret env values) + temp project with an extra .mcp.json server → result excludes all desktop servers (AC-1), JSON.stringify(result) contains none of the fake secret values (AC-2), includes sequant + project entries; no .mcp.json → sequant only.
5. Driver test in claude-code.test.ts: config.mcp: true → query() receives mcpServers without desktop servers; config.mcp: false → no mcpServers key. (The SDK serializes options.mcpServers verbatim into --mcp-config argv, so the options boundary is the argv assertion — see Open Question 1.)
6. AC-1/AC-2 tests are gate tests → mutation-verify (commit first; revert the driver line to getMcpServersConfig(); confirm exactly those tests fail; restore; record SEQUANT_MUTATION markers in the PR body per /qa §6i).

Phase 3 — docs (all run.mcp surfaces)
7. src/lib/settings.ts JSDoc (~line 131: "reads MCP config from Claude Desktop and passes to SDK" — now wrong) + the generated settings-docs table (~line 1125 "Enable MCP servers in headless mode").
8. docs/reference/run-command.md (lines ~66, 617–620, 720–745), verify cheat-sheet.md:149 / ready-command.md:69 (already say "injection" — likely fine), docs/troubleshooting.md:400 context.
9. docs/guides/mcp-integrations.md — required, not optional: it currently instructs adding Chrome DevTools / Sequential Thinking to the Claude Desktop config, which only reached phase agents via the passthrough being removed. Rewrite to: declare optional MCPs in project .mcp.json (secret-free entries; use ${VAR} env expansion via Claude Code if needed) for phase-agent availability.

Derived ACs

Source	Derived AC	Priority
Docs coverage	AC-4: docs/guides/mcp-integrations.md instructs declaring optional MCPs in project .mcp.json (not desktop config) for phase availability	Medium

Known gaps / sibling findings (surfaced, not silently scoped in)

• doctor drift: checkOptionalMcpServers() reads only the desktop config, so after this change doctor reports MCPs as "configured" that phases can no longer see. Recommend a follow-up issue (extend doctor to check .mcp.json) rather than widening this PR.
• Behavior regression by design: users who configured chrome-devtools/context7/sequential-thinking in the desktop config lose them in phases until they add .mcp.json entries. Docs step 9 is the mitigation; worth a CHANGELOG "breaking-ish" note.

Open Questions

1. AC-1 prescribes an argv-level spawn test ("spawn a phase with a populated fake desktop config"). Recommendation: assert at the mocked query() options boundary + builder unit tests with a real fake desktop config file — the SDK owns the options→argv serialization, and spawning a real claude child is heavy/flaky. Impact if wrong: QA flags literal-AC divergence; an e2e spawn test can be added then.
2. Defer mcpAllowlist? Recommendation: defer. Impact if wrong: desktop-passthrough users need the .mcp.json migration with no escape hatch this release.
3. Doctor drift fix in-PR or follow-up? Recommendation: follow-up issue. Impact if wrong: misleading doctor output until it lands.

Scope Assessment

Non-Goals: ⚠️ not found in issue body — treat mcpAllowlist and the doctor fix as explicit non-goals per above. Metrics: features 1 ✅ · ACs 3 ✅ · directory spread 2 ✅ — Verdict: ⚠️ SCOPE_WARNING (missing Non-Goals only; auto-enables quality loop).

Recommended Workflow

Phases: exec → qa · Quality Loop: enabled (SCOPE_WARNING) · Testgen: skipped (fix-shaped, tests land with implementation in the existing suites) · security-review: considered, not added — the change strictly removes capabilities and parses only the project-trust-domain .mcp.json; QA should still adversarially trace the secret path (desktop config → argv) per AC-2.

[admarble]

QA Review for Issue #936

AC Coverage

AC	Source	Description	Status	Notes
AC-1	Original	Phase agent MCP config excludes all servers beyond sequant + project .mcp.json entries	MET	getPhaseMcpServersConfig (mcp-config.ts:99) never reads claude_desktop_config.json; only reads .mcp.json + guarantees sequant. Mutation-verified: reintroducing a desktop-passthrough spread broke exactly the AC-1 test and no other.
AC-2	Original	No secret-bearing desktop env values appear in phase argv	MET	Same builder — since desktop config is never read, its secrets structurally cannot reach the SDK options object serialized into argv. Mutation-verified independently.
AC-3	Original	Docs describe run.mcp as sequant-server injection, not desktop passthrough	MET	run-command.md "MCP Server Support" section and mcp-integrations.md "Installation" section rewritten; settings.ts/system.ts docstrings updated. Verified by direct re-read against the AC text (see Manual Test AC Override below).
Derived ACs
AC-4	Derived (Docs coverage)	mcp-integrations.md instructs declaring optional MCPs in project .mcp.json, not desktop config	MET	Installation section rewritten with .mcp.json examples; troubleshooting note added pointing out doctor's desktop-based detection is now a different source than what phases use.

Coverage: 4/4 AC items fully met
Original ACs: 3/3 met
Derived ACs: 1/1 met

---

Quality Plan Verification

N/A - No "Feature Quality Planning" section found in the /spec comment (spec used a lighter Standard-tier template). The one Derived AC it did surface (AC-4, docs coverage) is tracked and verified above.

---

CI Status

Check	State	Bucket	Impact
typecheck	SUCCESS	pass	✅ MET
build (22.x)	SUCCESS	pass	✅ MET
node-floor-guard	SUCCESS	pass	✅ MET
validate-skills	SUCCESS	pass	✅ MET
validate-plugin	SUCCESS	pass	✅ MET

CI Summary: 5 passed, 0 pending, 0 failed
CI-related AC items: None of the ACs explicitly name CI; noted for completeness — all green.

---

Quality Metrics

Metric	Value	Status
Type issues (any)	0	OK
Deleted tests	0	OK
Files changed	9	OK
Lines added	+263	-
Lines deleted	-47	-

---

Build Verification

N/A - Build passed (both locally in the worktree during exec, and in CI).

---

Test Coverage Analysis

Changed File	Tier	Has Tests?	Test File
src/lib/mcp-config.ts	Standard	✅ Yes	src/lib/mcp-config.test.ts (6 new cases)
src/lib/workflow/drivers/claude-code.ts	Standard	✅ Yes	src/lib/workflow/drivers/claude-code.test.ts (2 new cases)
src/lib/settings.ts	Optional	- (docstring only)	-
src/lib/system.ts	Optional	- (docstring only)	-
CHANGELOG.md, docs/*.md	Optional	- (docs)	-

Coverage: 2/2 behavior-changing source files have corresponding tests. settings.ts/system.ts diffs are comment-only (confirmed via git diff), so no test gap there.
Critical paths without tests: None — no auth/payment/security/server-action/middleware/admin path in this diff.

---

Code Review

Strengths:

• Single driver call site (claude-code.ts:119) means both ExecutionConfig producers (config-resolver and ready-gate.ts) inherit the fix automatically — no risk of the two-producer drift class seen in fix(cli): bare parseInt on run --timeout lets a malformed value abort every phase instantly (sibling of #818) #833.
• The new builder guarantees the sequant entry even if a project .mcp.json declares its own (stale-fork protection), verified by a dedicated test.
• getMcpServersConfig() is deliberately kept, not deleted, and its docstring now explicitly warns against phase-execution reuse — reduces risk of a future regression re-wiring it back in.
• Both gate-test ACs (AC-1, AC-2) were mutation-verified with markers in the PR body, isolating each mutation to exactly the test(s) meant to catch it.

Issues:

• None found.

Suggestions:

• None beyond the two already-disclosed known gaps (doctor drift, deferred mcpAllowlist) — see Risk Assessment.

---

Test Quality Review

Category	Status	Notes
Behavior vs Implementation	✅ OK	New tests assert on getPhaseMcpServersConfig's return value and on query()'s received options — outputs, not internals.
Coverage Depth	✅ OK	Covers happy path, missing .mcp.json, corrupt .mcp.json, stale sequant override, and the two secret-leak scenarios (AC-1/AC-2).
Mock Hygiene	✅ OK	Driver test mocks only the SDK query() and the builder module (2 mocks) — not over-mocked. Builder test uses real filesystem with a temp dir + HOME override, no mocking needed.
Test Reliability	✅ OK	No timing dependencies; temp dirs cleaned in afterEach; HOME restored in afterEach.

Issues Found: None.

---

Anti-Pattern Detection

Dependency Audit

N/A - No package.json changes.

Code Patterns

No critical or warning patterns found. Security sub-agent scan (see Risk Assessment) confirmed no secret-leak regression and no hardcoded credentials in test fixtures (fake key strings are self-evidently synthetic, don't match real Stripe/Anthropic formats).

Critical Issues: 0
Warnings: 0

---

Call-Site Review

New exported functions detected: 1 (getPhaseMcpServersConfig)

Function	Call Sites	Loop?	Conditions	AC Match
getPhaseMcpServersConfig()	src/lib/workflow/drivers/claude-code.ts:121	No	config.mcp ? getPhaseMcpServersConfig(config.cwd) : undefined	✅ Matches AC-1 — the only production call site, gated identically to the pre-existing config.mcp flag

Findings: None — single call site, condition unchanged from the pre-fix code (config.mcp ? … : undefined), so the --no-mcp opt-out behavior is preserved exactly.

Recommendations: None.

---

Execution Evidence

N/A - No scripts/ or CLI (bin/) files modified.

---

Product Review

User-facing changes: src/lib/settings.ts (run.mcp JSDoc/behavior) — no new CLI flags or commands.

Question	Finding
First-time setup	No new prerequisite — behavior change is transparent for users who don't rely on desktop-config MCPs in headless mode.
Per-environment	Reduces variance, if anything: the old path had 3 platform-specific desktop-config paths (macOS/Windows/Linux); the new path is a single .mcp.json lookup relative to cwd.
User sees	No visible UI. Observable only via sequant doctor (now stale for headless — documented) or by testing which MCPs a phase actually gets.
After completion	Users who relied on Claude Desktop config for headless MCPs must migrate entries to project .mcp.json — documented in mcp-integrations.md with worked examples.
Likely failure modes	A user upgrades, doesn't read the CHANGELOG, and is confused when Context7/Chrome DevTools/Sequential Thinking silently stop being available in /exec//test//fullsolve phases. Mitigated by the CHANGELOG entry and rewritten docs, but this is a real behavior change with no runtime warning.

Gaps found: doctor's "MCP Servers (headless)" check still reports on Claude Desktop config, so it will actively mislead a user checking availability post-fix. This is disclosed in the PR body, CHANGELOG, and both doc files as a known gap with a recommended follow-up issue — not silently left inconsistent. Acceptable to ship with the disclosure; recommend filing the follow-up issue after merge.

---

Manual Test ACs

AC	Verification Method	Runtime Test Status	Evidence
AC-3	Manual (docs review, per /spec)	🔄 Overridden	See override below

Manual Test Enforcement: 1/1 manual-test AC verified (via override).

Manual Test Override

AC: AC-3
Requirement: Runtime verification for manual-test AC
Override: Yes
Justification: No runtime surface — AC-3 ("docs describe X, not Y") is a static documentation content claim with no executable code path to run. Verified instead by direct content re-read of run-command.md §"MCP Server Support" and mcp-integrations.md §"Installation" against the literal AC text (see AC Coverage above) — confirmed neither section instructs editing Claude Desktop config for headless/phase use anymore.
Risk Assessment: None.

---

Risk Assessment

• Likely failure mode: A user who has optional MCPs (Context7, Chrome DevTools, Sequential Thinking) configured only in their Claude Desktop config will find them silently unavailable to phase agents after upgrading, with no runtime error — just reduced capability (e.g. /exec falls back to codebase search instead of fetching library docs). This is an intentional, disclosed behavior change, not a defect, and the fallback behavior for a missing MCP was already graceful before this change.
• Not tested: No end-to-end test spawns a real claude child process and inspects its actual --mcp-config argv — the driver test asserts at the mocked SDK options boundary instead (an explicit, disclosed tradeoff from /spec's Open Question 1: spawning a real child is heavy/flaky, and the SDK owns options→argv serialization, not sequant code).
• Sibling sites considered: Grepped the full codebase for other getMcpServersConfig call sites — only doctor.ts remains, and that's the intended, disclosed exception (interactive diagnostics). No other production code path passed the desktop config anywhere.
• Sibling-line audit: mcp-config.ts's other exported functions (getSequantMcpConfig, createProjectMcpJson, addSequantToMcpConfig, syncSequantMcpPin) were read during review — none share the desktop-passthrough root cause; they already operate on .mcp.json or client-specific configs, not the interactive Claude Desktop file used for phase execution.

---

Trust-Boundary Check

Finding: Clean — diff implements only the product requirements from the issue body (allowlist MCP config for phase agents, update docs). The issue body itself was reviewed in /spec and contains no hidden agent-directed instructions (no HTML comments, no embedded execute/fetch/exfiltrate imperatives).

Status: Clean

---

Behavior-Rule Survival Check

N/A — none of the 3 original or 1 derived AC trigger the behavior-rule heuristic (checked: no AC text contains ≥2 of default|always|never|rule|behavior|skip, nor an explicit always X unless Y / never X unless Y pattern).

Status: N/A

---

CHANGELOG Verification

Check	Status
CHANGELOG.md exists	✅ Found
User-facing changes	✅ Yes (fix(#936): … commit, behavior change to run.mcp)
[Unreleased] entry	✅ Present
Entry format	✅ Valid (bold headline, issue number, behavior-change callout)

Result: CHANGELOG requirements met

---

Adversarial Re-Read

Findings: No gaps found because: (1) the issue body names no verbatim literal repro snippet to replay — the incident description (SMTP/Stripe/Sheets/GitHub-PAT/filesystem servers leaking via desktop passthrough) was reconstructed as a faithful test fixture in mcp-config.test.ts and mutation-verified against it; (2) the PR body's two SEQUANT_MUTATION markers are genuine post-fix validations — each reintroduces the removed passthrough and confirms the specific AC test (not some other test) fails, then restores — not pre-fix bug repros mislabeled as validation; (3) process state is clean — correct feature branch, no uncommitted work, PR mergeable, all commits pushed; (4) sibling sites are explicitly cited above (cross-file: only doctor.ts retains the old function, by design; intra-file: mcp-config.ts's other exports don't share the root cause); (5) the issue body has no Non-Goals section (flagged as SCOPE_WARNING at /spec time), and scope stayed contained — the two deferred items (mcpAllowlist, doctor fix) are explicitly disclosed as known gaps in the PR body rather than silently expanded or silently dropped; (6) confirmed via the Trust-Boundary Check above.

Status: Clean

---

Verdict: READY_FOR_MERGE

All 3 original ACs and 1 derived AC are MET, both gate-test ACs are mutation-verified, all 5 CI checks pass, and the adversarial re-read surfaced no gaps beyond the two explicitly-disclosed known gaps (doctor drift, deferred mcpAllowlist opt-in) that were deliberately scoped out at /spec time and documented in the PR body, CHANGELOG, and both doc files. Code review, test quality, call-site review, product review, and trust-boundary check are all clean.

Documentation

• README/docs updated — run-command.md and mcp-integrations.md rewritten
• CHANGELOG entry added
• N/A - No documentation needed

Next Steps

1. Merge PR fix(#936): phase agents get an .mcp.json allowlist instead of the full Claude Desktop config #952.
2. File a follow-up issue for the disclosed doctor drift (checkOptionalMcpServers/MCP Servers (headless) check still reads Claude Desktop config, not the project .mcp.json phases actually use).

[admarble]

/fullsolve Complete

Issue: #936
Status: READY_FOR_MERGE

Summary

• AC: 3/3 original + 1/1 derived met
• Build/lint/tests: all pass (267 test files, 5499 passed, 1 expected fail, 2 skipped)
• QA: READY_FOR_MERGE — all quality checks (type safety, scope, security) passed clean; all 5 CI checks green

Progress

Phase	Iterations	Status
Spec	1/1	Complete
Exec	1/1	Complete
Test	Skipped	No ui/frontend/admin label — backend/CLI fix
QA	1/1	Complete — READY_FOR_MERGE

PR: #952

---

Ready for human review and merge. Awaiting explicit go-ahead to merge, since merging to main is a shared-state action.

[admarble]

QA Review for Issue #936 (re-run — gap fixes added)

Re-running the full QA pipeline after two follow-on gap fixes were added to the same PR: settings.run.mcpAllowlist (the opt-in escape hatch the original issue proposed but deferred) and the doctor headless-MCP drift fix. Both were flagged as known gaps in the first QA pass and are now implemented.

AC Coverage

AC	Source	Description	Status	Notes
AC-1	Original	Phase agent MCP config excludes all servers beyond sequant + project .mcp.json entries	MET	Unchanged from first pass — still holds with the new desktopAllowlist param defaulting to no passthrough.
AC-2	Original	No secret-bearing desktop env values appear in phase argv	MET	Unchanged; the default (unset allowlist) path is untouched by the new feature.
AC-3	Original	Docs describe run.mcp as sequant-server injection, not desktop passthrough	MET	Docs re-verified after this round's edits — still accurate, no drift introduced.
Derived / follow-up ACs
AC-4	Derived (docs coverage)	mcp-integrations.md instructs declaring optional MCPs in project .mcp.json, not desktop config	MET	Unchanged, plus a new pointer to the allowlist section for the desktop-only case.
AC-5	Follow-up (gap fix)	settings.run.mcpAllowlist — a named server passes through only when explicitly allowlisted, never overrides project .mcp.json/sequant, and is opt-in by default	MET	5 dedicated tests + 5 mutation-verified claims (filter correctness, precedence vs .mcp.json, precedence vs sequant, driver forwarding, opt-in default via 3 more threading tests). Security sub-agent independently confirmed precedence, filter, opt-in-default, and config.mcp-gating all correct.
AC-6	Follow-up (gap fix)	doctor's "MCP Servers (headless)" check reports the same source phases actually use	MET	Mutation-verified; security sub-agent confirmed it now calls getPhaseMcpServersConfig, not the old desktop reader.

Coverage: 6/6 AC items fully met (3 original + 3 follow-up/derived)

---

CI Status

Check	State	Bucket	Impact
typecheck	SUCCESS	pass	✅ MET
build (22.x)	SUCCESS	pass	✅ MET
node-floor-guard	SUCCESS	pass	✅ MET
validate-skills	SUCCESS	pass	✅ MET
validate-plugin	SUCCESS	pass	✅ MET

CI Summary: 5 passed, 0 pending, 0 failed
PR mergeable: MERGEABLE / CLEAN

---

Quality Metrics

Metric	Value	Status
Type issues (any)	0	OK
Deleted tests	0	OK
Files changed	20	OK
Lines added	+615	-
Lines deleted	-68	-

---

Test Coverage Analysis

Changed File	Tier	Has Tests?
src/lib/mcp-config.ts	Standard	✅ Yes — 11 cases total (6 base + 5 desktopAllowlist)
src/commands/doctor.ts	Standard	✅ Yes — 2 new cases
src/lib/workflow/drivers/claude-code.ts	Standard	✅ Yes — 1 new case (forwarding)
src/lib/workflow/ready-gate.ts	Standard	✅ Yes — 2 new cases
src/commands/ready.ts	Standard	✅ Yes — 1 new case
src/lib/workflow/config-resolver.ts	Standard	✅ Yes — new dedicated file, 2 cases
src/lib/workflow/phase-executor.ts	Standard	Covered indirectly via AgentExecutionConfig shape; single-line pass-through, low branching risk
src/lib/workflow/drivers/agent-driver.ts, src/lib/workflow/types.ts	Optional	Type-only additions
src/lib/settings.ts	Optional	Schema field; covered indirectly by the config-resolver/ready threading tests reading real settings shapes

Coverage: All behavior-changing files have direct tests. Every hop in the settings→driver plumbing chain (config-resolver.ts, ready-gate.ts, ready.ts, claude-code.ts) has its own dedicated test — closing exactly the kind of silent-drop gap a producer-parity feature like this is prone to.

---

Code Review

Strengths:

• The mcpAllowlist threading exactly mirrors the existing feat(run): per-phase model and effort configuration for the claude-code driver #914 model/effort producer-parity pattern (both ExecutionConfig producers resolve from settings the same way), so it doesn't introduce a new drift class.
• doctor.ts's fix reuses the exact builder phases use (getPhaseMcpServersConfig) rather than reimplementing count logic — the check can no longer drift from reality by construction.
• The allowlist filter is a strict per-name copy loop, not a bulk merge — confirmed independently by both my mutation testing and the security sub-agent.
• Precedence is layered correctly and deliberately: allowlisted desktop < project .mcp.json < guaranteed sequant, each verified by its own test and mutation.

Issues:

• None found.

Suggestions:

• None.

---

Call-Site Review

Exported function with a modified signature: getPhaseMcpServersConfig (new optional opts.desktopAllowlist param)

Call Site	Loop?	Conditions	AC Match
claude-code.ts:122 (production, phase execution)	No	config.mcp ? getPhaseMcpServersConfig(config.cwd, { desktopAllowlist: config.mcpAllowlist }) : undefined	✅ Matches AC-1/AC-5 — allowlist only reaches the builder when MCP is enabled at all
doctor.ts:518 (diagnostics)	No	Unconditional (diagnostic report, not gated on config.mcp since there's no live phase to gate)	✅ Matches AC-6 — reports what a phase would get if run now

Findings: Both call sites pass settings.run.mcpAllowlist through consistently. No call site bypasses the per-name filter or the config.mcp gate.

---

Product Review

User-facing changes: New settings.run.mcpAllowlist config key; doctor output for the "MCP Servers (headless)" line changes (always passes now, different message).

Question	Finding
First-time setup	No new prerequisite — the field is entirely opt-in and undocumented-by-default users never touch it.
Per-environment	None — same JSON key everywhere.
User sees	doctor output changes: the headless line always shows "pass" now (previously could warn). Users checking that line for the first time post-upgrade see a different but more accurate message.
After completion	A user who wants a desktop-only MCP available headlessly now has a documented path (mcpAllowlist) instead of no path at all — net positive vs. the state right after the original #936 merge.
Likely failure modes	A user copies an example mcpAllowlist: ["some-server"] from docs without reading the secret-exposure warning immediately above it. Mitigated: the warning is bolded with an explicit ⚠️ and is the very next paragraph after the JSON example, not buried elsewhere.

Gaps found: None — this closes the doctor-drift gap from the prior QA pass rather than opening a new one.

---

Risk Assessment

• Likely failure mode: A user sets mcpAllowlist for a server whose desktop config entry has a literal secret, without reading the warning — the secret then reaches phase argv exactly as documented. This is the deliberate, disclosed tradeoff of the feature itself (an opt-in escape hatch), not an implementation defect; the docs make the risk explicit at the point of use.
• Not tested: Same as the first QA pass — no end-to-end test spawns a real claude child process to inspect actual --mcp-config argv; all threading is verified at the mocked SDK options boundary and via unit tests on the builder. This was an accepted tradeoff from /spec's Open Question 1 in the original round and applies equally to the new allowlist path.
• Sibling sites considered: Grepped for all getPhaseMcpServersConfig call sites — exactly 2 in production code (claude-code.ts, doctor.ts), both reviewed above. getMcpServersConfig (desktop reader) now has exactly 1 production call site remaining, inside the gated allowlist loop in mcp-config.ts itself — confirmed via repo-wide grep, no stray desktop-reading call sites elsewhere.
• Sibling-line audit: Reviewed ready-gate.ts's other buildPhaseConfig-adjacent fields (phasePolicies, effortEscalation) for the same producer-parity pattern mcpAllowlist follows — consistent, no divergence introduced.

---

Trust-Boundary Check

Finding: Clean — this round's changes originated entirely from the user's own follow-up request ("fix all gaps") after an explicit design-choice question was asked and answered (fix doctor drift + build mcpAllowlist, not the narrower doctor-only option). No issue-body or comment text was a source of any imperative here; the work traces to direct user instruction in this conversation.

Status: Clean

---

Behavior-Rule Survival Check

N/A — keyword scan against AC-5/AC-6 text found at most 1 of the 6 trigger keywords (never, in AC-5's "never overrides project .mcp.json/sequant") and no always X unless Y / never X unless Y pattern. Below the ≥2-keyword threshold.

Status: N/A

---

CHANGELOG Verification

Check	Status
CHANGELOG.md exists	✅ Found
User-facing changes	✅ Yes (new settings key, doctor output change)
[Unreleased] entry	✅ Present — new ### Added entry for mcpAllowlist, existing ### Fixed entry for #936 updated to mention the doctor fix
Entry format	✅ Valid

Result: CHANGELOG requirements met

---

Adversarial Re-Read

Findings: No gaps found because: (1) both new gaps addressed this round were explicitly named by the user, not discovered fresh, so there's no missed-fixture risk — the mutation tests are the fixtures, and all 6 new mutations (filter bypass, two precedence swaps, driver-forwarding drop, and 3 settings-plumbing drops) were run individually with clean isolation, confirmed, and restored; (2) the PR body's mutation markers for this round are genuine — each names the exact mutation and the exact test(s) it broke, verified live in this session, not asserted from memory; (3) process state is clean — correct branch, no uncommitted work, PR mergeable/CLEAN, all commits pushed; (4) sibling sites are cited above with an actual repo-wide grep, not hand-waved; (5) no Non-Goals silently expanded — the two items added were explicitly authorized by the user via a direct design-choice question, not scope creep discovered mid-implementation; (6) confirmed via the Trust-Boundary Check above.

Status: Clean

---

Verdict: READY_FOR_MERGE

All 6 ACs (3 original + 3 follow-up) are MET. Both new gate-worthy claims for this round (mcpAllowlist correctness, doctor fix correctness) are mutation-verified with clean isolation. All 5 CI checks pass, PR is MERGEABLE/CLEAN. Security sub-agent independently confirmed the re-introduced desktop passthrough is correctly scoped (per-server, opt-in, subordinate to .mcp.json/sequant, gated behind config.mcp, documented with an explicit secret-exposure warning). No gaps remain from either QA pass.

Documentation

• README/docs updated — new "Allowlisting a Desktop-Only Server" section, doctor-related notes corrected across both doc files
• CHANGELOG entry added/updated
• N/A - No documentation needed

Next Steps

1. Merge PR fix(#936): phase agents get an .mcp.json allowlist instead of the full Claude Desktop config #952.