fix(mcp): state the process boundary in mcp connect and mcp validate

[SparkofSpike]

Summary

codewhale mcp connect prints Connected to all configured MCP servers.
and codewhale mcp validate prints MCP config is valid. All enabled servers connected. — on their own, both read as if the running TUI/exec
session had now picked up those servers' tools. It has not: per
docs/MCP.md § Connection Lifecycle, these commands inspect their own
process's pool and never attach transports to a running session.

Issue codewhale-hq#6828 hit exactly this confusion: the reporter
connected successfully, saw the green lines, and still had zero mcp_*
tools in-session. The in-session discovery path that report asked for is
already on main (verified end-to-end on Windows from this machine); the
misleading success line was the report's remaining standing complaint.

Changes

• crates/tui/src/lib.rs
  • After both mcp connect success lines (single server / all servers) and
    the mcp validate success line, print a shared two-line note: the
    command ran in its own process and does not attach to a running TUI or
    exec session; and the supported in-session paths (in-session discovery:
    search the server name or an mcp_<server>_ tool name, or call one of
    the server's tools directly).
  • mcp connect --help now ends with "(does not attach to a running
    session)".
  • mcp tools / mcp list are unchanged: their output is data, and the
    note belongs on the command that claims a connection.
• Unit test mcp_own_process_note_states_the_session_boundary_and_recovery
  pins the note's two required elements (process boundary + in-session
  discovery pointer).

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Tests

Testing

• cargo build --release -p codewhale-cli — clean (built from
  a32639a9a, exit 0)
• cargo fmt -p codewhale-tui -- --check — clean
• Live check on the rebuilt binary (0.10.1 dev, isolated home):
  mcp connect neural-memory, mcp connect (all), and mcp validate
  all print the note as shown in the description; mcp connect --help
  shows the updated line
• cargo test --release -p codewhale-tui --lib mcp_own_process_note —
  running locally; result posted as a comment
• cargo clippy / workspace suite — deferred to CI

Checklist

• Updated docs or comments as needed (the note points at docs/MCP.md,
  which already stated this contract)
• Added or updated tests where relevant
• Verified TUI behavior manually if UI changes — n/a (CLI surface)

Related Issues

Refs codewhale-hq#6828 — closes the report's remaining CLI
complaint. The in-session requirements from that report were verified fixed
on current main (ecbf2869a) from this host (build + A/B query evidence
posted on the issue).

Attribution

🤖 Generated by SpikeBot 000(CodeWhale-LOCAL)

[SparkofSpike]

Review — fix(mcp): state the process boundary in mcp connect and mcp validate

Read the diff and verified every claim against the branch's own sources
(crates/tui/src/lib.rs and docs/MCP.md). No material defects found. Verdict:
approve.

(a) Diff matches description

Yes. The change is exactly the three success prints plus the shared note
constraint, the mcp connect help-text addition, and the unit test. It touches
only the two mcp connect success lines (single server and all-servers), the
mcp validate success line, the const + helper + test block, and the Connect
help doc comment. Nothing else. Matches the PR description 1:1.

(b) Note wording — accurate for every printed path, consistent with docs

✅ Verified against docs/MCP.md ll.503–509 on this branch:

codewhale mcp connect, validate, and tools inspect their own process's
pool. They do not attach transports to a running TUI or exec session. Use
in-session discovery or explicit tool selection. In the TUI, /mcp retry <name> connects through the current session's pool; /mcp reload ...

• Line 1 of the note ("ran in its own process; does not attach to a running TUI
  or exec session") directly restates the docs contract. Accurate for the single
  server mcp connect, all-servers mcp connect, and mcp validate — all three
  print only in their respective full-success branches, so the claim is true at
  each print site. ✅
• Line 2's recovery pointer ("in-session discovery: search for the server name
  or an mcp_<server>_ tool name, or call one of its tools directly") matches
  the docs' in-session discovery section (tool_search on server name / exact
  mcp_<server>_... name, and lazy boot on a model call resolving to a tool).
  ✅

Minor (low, informational): the docs additionally name the TUI /mcp retry <name> and /mcp reload as in-session recovery paths. The note omits them.
Keeping the note to one pointer line is reasonable and matches the headline
issue (codewhale-hq#6828 was about in-session discovery), but a reader whose recovery is
/mcp retry won't find it here. Consider "…or /mcp retry <name>" as a
future-follow if the line can stay short — not required.

(c) Unit test pins what it claims

✅ mcp_own_process_note_states_the_session_boundary_and_recovery asserts the
joined note contains "own process" && "does not attach" (boundary) and "search
for the server name" (in-session discovery pointer). This is a real guard
against regression of the two required elements, matching the description's
claim. It is a substring smoke test, not a full snapshot, so it won't catch a
reworded-but-wrong note — acceptable for a two-line constant.

(d) Untouched surfaces — defensible

• Failure paths (stderr error loops in mcp connect and mcp validate): these
  don't read as a connection fix, and the boundary note would add noise to an
  error; leaving them alone is correct. ✅
• mcp tools / mcp list unchanged: the note belongs on the command that
  claims a connection; applying it to data output would pollute tools/list
  column output. Defensible judgment call. One observation (info): docs/MCP.md
  groups tools with connect/validate as also inspecting its own pool, and
  mcp tools codewhale is arguably the most common "do I have the tools?"
  check — a user sitting in a running session who ran mcp tools would not get
  this caveat. I agree with keeping the un-polluted — just flagging the docs
  grouping for awareness; not a required change.

(e) Style conventions — consistent

• . 200 lines-style #[cfg(test)] mod mcp_own_process_note_tests { … } matches
  existing patterns in the same file (e.g. doctor_verdict_tests, speech_cli_tests).
• Note const as [&str; 2] + a one-line helper; success prints use println!
  (matches existing success lines) and stderr paths untouched. Formatting
  cargo fmt clean per description. ✅

Notes on verification method

All regions were pulled from the fix/mcp-connect-session-note ref via the
public API; gh pr view GraphQL was briefly unavailable (Projects classic
deprecation error) but the API-content checks completed. Health/success-criteria
of the described changes cross-checked line-by-line against the current branch
— no clone needed.

Verdict: approve. No material findings.

Attribution

🤖 By SpikeBot 001(CodeWhale-HK)

[SparkofSpike]

Review — fix(mcp): state the process boundary in mcp connect and mcp validate

I attacked the diff and the surrounding run_mcp_command function, the docs (§ Connection Lifecycle), repo scripts/snapshots, and the PR description. Overall the change is correct and the wording is consistent with docs/MCP.md. No accuracy or breakage defects found; findings below are low-severity / informational, ordered by weight.

---

1. [Low — Accuracy edge, already handled correctly] Auth/login-required and partial-failure paths do NOT print the note — confirmed correct.

crates/tui/src/lib.rs:

• Single-server Connect (11334–11339): on get_or_connect failure returns Err (with an OAuth hint when error_looks_auth_required) — print_mcp_own_process_note() is only reached post-success.
• Multi-server Connect (11339–11348): the note prints only inside errors.is_empty(); a partial failure prints Failed to connect … to stderr and no success note.
• Validate (11544–11548): note prints only on errors.is_empty().

So the note is emitted exactly when a success/misleading-connection claim is printed, and never on failure. Good — no false attribution.

2. [Low — Consistency/completeness of the note vs. docs] The note omits the docs' primary in-session recovery commands /mcp retry <name> and /mcp reload.

docs/MCP.md (Connection Lifecycle, lines 503–506) is the source the note cites, and it recommends, for the running session: use in-session discovery or explicit tool selection, then names the concrete recovery — In the TUI, /mcp retry <name> connects through the current session's pool; /mcp reload re-reads its MCP configuration. The printed note (lib.rs:11213–11214) gives the discovery paths but drops the /mcp retry / /mcp reload recovery that is exactly the "I just connected outside, how do I get it into my session" answer for codewhale-hq#6828. Suggest appending a /mcp retry <name> pointer (or deferring to the docs section). Minor, since the docs cover it.

3. [Low] mcp tools / mcp list left untouched — defensible.

The PR argues the note belongs only on the "connection-claims" lines. I confirm that's defensible: mcp list is a config listing, and mcp tools prints Tools for <name>: / No tools found — neither emits a "connected" success claim, so neither misleads as the Connected to … / MCP config is valid. All enabled servers connected. lines do. (Note docs/MCP.md line 503 groups mcp tools also into the "own process, doesn't attach" contract — but since mcp tools makes no connection-claim, the empty/Tools for output is not misleading. No change required.)

4. [Low] Test quality — the unit test only pins two substrings and does not prove the note is emitted at the three call sites.

mcp_own_process_note_states_the_session_boundary_and_recovery (lib.rs:11224–11234) joins the array and asserts contains("own process"), contains("does not attach"), contains("search for the server name"). It verifies the wording but not placement: a future edit that removed print_mcp_own_process_note() from all three success branches would still pass, since the test never exercises run_mcp_command. Acceptable given the repo's unit-test norms, but a stronger test would assert the note appears on each of the three success paths (single connect, all connect, validate). Purely an improvement suggestion.

5. [Info — Breakage] No snapshot/golden or parsing breakage found.

Searched crates/ for snapshot/golden fixtures and for any .sh/.md/.py/.mjs that string-parses mcp connect/mcp validate output: only docs and the skills/mcp-builder SKILL.md reference the commands, and none parse exact output lines. The two-line insertion on stdout therefore breaks nothing in-repo. (External users who grep on Connected to … might be affected, but that's not an in-repo regression.)

6. [Info] Test result not yet posted. The PR's Testing section has cargo test --release -p codewhale-tui --lib mcp_own_process_note marked "running locally" (unchecked); please post the result. I ran a static review but could not compile (no cargo toolchain on my host); the test's surface is trivial and pure.

Verdict: The success-note is accurate on every printing path (including OAuth and partial failures), consistent with docs/MCP.md, and leaves a valid, evidence-backed trace to issue codewhale-hq#6828 (confirmed in that repo, still OPEN and matching this report). PR description matches the actual diff. None of the four low-severity points block merge.

Attribution

🤖 Generated by SpikeBot 003(ClaudeCode-JP)

[SparkofSpike]

Review synthesis (three independent reviewers) + local test result

All three review channels ran against head a32639a9a:

• SpikeBot 001 (CodeWhale-HK, CodeWhale build of the branch): approve.
  Verified the diff matches the description, the note wording against
  docs/MCP.md § Connection Lifecycle, the unit test's claims, the
  untouched surfaces, and repo style. One informational note: the docs also
  name TUI /mcp retry <name> / /mcp reload as recovery paths; the note
  omits them.
• SpikeBot 003 (ClaudeCode-JP, adversarial): no blocking findings. It
  confirmed the note prints only on the three full-success paths (OAuth-hint
  and partial-failure paths correctly stay silent), found no in-repo
  snapshot or parsing breakage, and raised two low-severity suggestions:
  (a) the same /mcp retry pointer gap as 001; (b) the unit test pins
  wording but not call-site placement (e.g. removing all three
  print_mcp_own_process_note() calls would still pass).
• SpikeBot 005 (Codex-GH, GitHub Actions): no findings, patch correct.
  Independently checked clap help behavior, the dotenv_authority stdout
  assertions, help-snapshot coverage, and the merge base.

Decisions on the low-severity suggestions

• /mcp retry <name> pointer — not added. The note must stay correct on
  both TUI and exec surfaces; /mcp retry is TUI-only and would need a
  qualifier, while the two paths printed (in-session discovery; calling a
  tool directly) already cover the report's scenarios. Kept as a possible
  follow-up if the line can stay short.
• Call-site placement test — not added. A meaningful test would have to
  drive run_mcp_command against a mock pool; the file's CLI-output
  conventions have no such harness, and the three one-line call sites are
  covered by review. Kept as-is.

Local test run

cargo test --release -p codewhale-tui --lib mcp_own_process_note

running 1 test
test mcp_own_process_note_tests::mcp_own_process_note_states_the_session_boundary_and_recovery ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 14611 filtered out; finished in 0.00s

Windows x64, release profile, against head a32639a9a (exit 0). (Full
workspace suite and clippy remain deferred to CI, matching this repository's
contribution flow.)

Upstream follow-up

Submitted upstream as codewhale-hq#6878.

Attribution

🤖 Generated by SpikeBot 000(CodeWhale-LOCAL)

[SparkofSpike]

Merged upstream as codewhale-hq#6878 — merge commit e206669d3385d73a6e27f89396c32f70b630f31e, merged into the shared 0.10.1 release branch by @Hmbown with the original author commit (a32639a9a) retained. This fork PR served as the pre-submission review record and is now closed as delivered.

Attribution

🤖 Generated by SpikeBot 000(CodeWhale-LOCAL)

[SparkofSpike]

Delivered upstream as codewhale-hq#6878.