fix(mcp): honor context on stdio spawn and stdin write

[SebTardif]

What Problem This Solves

Fixes an issue where users running slacrawl sync --source mcp with transport = stdio would hang forever when the MCP child stopped reading stdin. Ctrl-C canceled the Go context, but spawn used exec.Command and stdin.Write ignored that cancel, so the CLI never reached Close() and never killed the child.

Why This Change Was Made

NewStdio now starts the child with CommandContext and fails immediately if the spawn context is already done. Stdin writes run in a goroutine and, on cancel, close the pipe and kill the child so a full pipe cannot park the caller. JSON-RPC framing is unchanged. This does not restack the desktop Node decoder bound (#163), the HTTP client timeout (#126), or the repeated-cursor guard (#159).

User Impact

A stuck stdio MCP server no longer holds the CLI after cancel. sync --source mcp can return when the user stops the command, matching how provider.Sync already treats a hung provider child.

Evidence

Call chain: slacrawl sync --source mcp with transport = stdio -> slackmcp.Sync -> slackmcp.New -> mcpclient.NewStdio -> Initialize / CallToolText -> write. The spawn used exec.Command (context discarded) since MCP stdio landed in #46 on 2026-06-06 (84 days). write checked ctx.Err() once, then blocked on stdin.Write. provider.Sync already uses CommandContext and an async stdin write.

Before the patch, a 2MiB stdin write to a child that never reads (sleep) stayed blocked after a 400ms deadline:

$ /tmp/f004proof.bin old-write
old write start
old write still blocked elapsed=401ms

After the patch, the same public NewStdio + CallToolText path against /bin/sleep returns when the 400ms deadline fires, and an already-canceled spawn context does not start a child:

$ /tmp/f004proof.bin new-write
new write start
new write err=context deadline exceeded elapsed=403ms

$ /tmp/f004proof.bin new-spawn
new spawn start
new spawn err=context canceled elapsed=0s

Real behavior proof

• Behavior or issue addressed: MCP stdio spawn and stdin write ignored cancel, so a child that stopped reading stdin parked sync --source mcp through Ctrl-C.

• Real environment tested: macOS, Go 1.27.0. Patched slacrawl at /tmp/oc-pr-slacrawl-F004. Child: /bin/sleep (never reads stdin). Proof binary built from ./tmpproof against the patched module.

• Exact steps or command run after this patch: Built /tmp/f004proof.bin from the patched module and ran old-write (blocking stdin.Write), then new-write (CallToolText with a 2MiB payload and a 400ms deadline) and new-spawn (already-canceled NewStdio context).

• Evidence after fix: terminal output from the patched NewStdio / CallToolText binary:

  old write start
  old write still blocked elapsed=401ms
  new write start
  new write err=context deadline exceeded elapsed=403ms
  new spawn start
  new spawn err=context canceled elapsed=0s

• Observed result after fix: CallToolText returned context deadline exceeded in 403ms instead of staying blocked on a full stdin pipe. NewStdio with a canceled context returned context canceled immediately and did not leave a sleep child running.

• What was not tested: A live Slack MCP npm server (@modelcontextprotocol/server-slack) that hangs after a real tools/call, and a Ctrl-C against that server from an operator terminal.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 30, 2026, 7:51 PM ET / 23:51 UTC.

ClawSweeper review

What this changes

The PR makes MCP stdio child startup and blocked JSON-RPC stdin writes respond to context cancellation.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: the PR uniquely repairs the current MCP stdio cancellation path and the reviewed head has no actionable correctness or security finding.

Priority: P2
Reviewed head: bfefbd4e9ab620ab4b1bba8f920b91079ce38beb

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	The patch is narrow, covers each changed cancellation boundary, and includes convincing real process-and-pipe behavior proof.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The changed production owner is the stdio MCP client; the captured macOS terminal proof exercises its public NewStdio and CallToolText entrypoints with a real non-reading child and observes post-fix deadline cancellation in 403 ms plus immediate canceled-spawn rejection.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owner is the stdio MCP client; the captured macOS terminal proof exercises its public NewStdio and CallToolText entrypoints with a real non-reading child and observes post-fix deadline cancellation in 403 ms plus immediate canceled-spawn rejection.
Evidence reviewed	5 items	Current call path: The stdio transport passes the sync context into the MCP client, then initializes over that same client; this makes child and request cancellation part of the active sync path. Introduced cancellation repair: The reviewed delta rejects an already-canceled context, uses context-aware process launch, and closes stdin plus kills the process when a blocked write's context ends. Focused regression coverage: Three introduced tests cover canceled-before-spawn, cancellation killing a running child, and a full stdin pipe unblocking after cancellation.
Findings	None	None.
Security	None	None.

How this fits together

The MCP sync source launches a configured MCP server over stdio, sends JSON-RPC requests, and normalizes returned Slack data into the archive pipeline. Cancellation must stop the child and unblock request writes so the CLI can exit.

flowchart LR
A[CLI sync command] --> B[MCP sync source]
B --> C[Stdio MCP client]
C --> D[Child MCP process]
C --> E[JSON-RPC stdin request]
D --> F[Slack data response]
F --> B
E --> G[Context cancellation]

Loading

Before merge

• Complete next step (P2) - No mechanical repair is needed; this exact reviewed head is ready for ordinary maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test delta	production +29/-5, tests +54/-0	The small production change is paired with focused coverage for each cancellation boundary it adds.

Technical review

Best possible solution:

Land the narrow cancellation repair so a non-reading configured MCP subprocess cannot keep an operator's sync command alive after cancellation.

Do we have a high-confidence way to reproduce the issue?

Yes. The captured before/after terminal run exercises the public stdio client against a real non-reading child, and the introduced tests cover the same blocked-pipe and spawn-cancellation cases.

Is this the best way to solve the issue?

Yes. Propagating the existing sync context to process creation and making a blocked write cancellable directly fixes the owning stdio boundary without changing JSON-RPC framing or configuration.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against dcbc12d44837.

Labels

Label justifications:

• P2: This repairs a cancellation hang in the optional MCP sync workflow without evidence of broader service interruption.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is the stdio MCP client; the captured macOS terminal proof exercises its public NewStdio and CallToolText entrypoints with a real non-reading child and observes post-fix deadline cancellation in 403 ms plus immediate canceled-spawn rejection.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the stdio MCP client; the captured macOS terminal proof exercises its public NewStdio and CallToolText entrypoints with a real non-reading child and observes post-fix deadline cancellation in 403 ms plus immediate canceled-spawn rejection.

Evidence

What I checked:

• Current call path: The stdio transport passes the sync context into the MCP client, then initializes over that same client; this makes child and request cancellation part of the active sync path. (internal/slackmcp/client.go:88, bfefbd4e9ab6)
• Introduced cancellation repair: The reviewed delta rejects an already-canceled context, uses context-aware process launch, and closes stdin plus kills the process when a blocked write's context ends. (internal/mcpclient/stdio.go:54, bfefbd4e9ab6)
• Focused regression coverage: Three introduced tests cover canceled-before-spawn, cancellation killing a running child, and a full stdin pipe unblocking after cancellation. (internal/mcpclient/client_test.go:146, bfefbd4e9ab6)
• Real behavior proof: The captured PR body records a macOS run of the patched public NewStdio and CallToolText path against a non-reading sleep child: a 2 MiB write returned context deadline exceeded in 403 ms, while a canceled spawn returned immediately. (cb4e78aeec7a)
• Feature history: MCP stdio client history includes the merged MCP source introduction and a later subprocess-lifecycle hardening pass, making that feature lineage the appropriate review route. (internal/mcpclient/stdio.go:54, b4d5fecd6fd6)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

• reviewed 2026-08-29T18:51:03.821Z sha bfefbd4 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T07:56:07.431Z sha bfefbd4 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T15:08:26.766Z sha bfefbd4 :: needs maintainer review before merge. :: none

[steipete]

Maintainer triage: recommend LAND. No source repair was needed. The docs/changelog follow-up is on triage/slacrawl-164-20260831, with contributor credit preserved. No merge performed.

I reproduced the blocked stdin path through the real built CLI. A synthetic stdio MCP server completes initialization, returns a 2 MiB pagination cursor from tools/list, then stops reading stdin. The next pagination request fills the pipe. The harness waits for the server marker, sends SIGINT only to Slacrawl, and checks child cleanup. Config, HOME, database, and PATH are isolated; no real Slack credentials or data are used.

GOWORK=off go build -o /tmp/slacrawl-main ./cmd/slacrawl # main
GOWORK=off go build -o /tmp/slacrawl-164 ./cmd/slacrawl  # PR head
python3 proof-cancel.py mcp /tmp/slacrawl-main /tmp/slacrawl-164

Each invocation runs --config <isolated-config> sync --source mcp --workspace TTEST --with-media=false.

main: still hung after SIGINT; forced cleanup after 2.061s
PR: exit=130; after SIGINT=0.605s; child_alive=false

GOWORK=off go test -count=1 ./internal/mcpclient ./internal/slackmcp
ok github.com/openclaw/slacrawl/internal/mcpclient 0.709s
ok github.com/openclaw/slacrawl/internal/slackmcp 8.749s

Codex autoreview returned scoped-clean for the source patch and docs follow-up (the skill's default P0 scope). Existing GitHub CI is green. This is real process-and-pipe proof with an injected failure, not a claim that a production Slack MCP server naturally returns such a cursor.

[steipete]

Reviewed the integrated MCP stdio cancellation patch and docs/changelog follow-up. Focused mcpclient and slackmcp tests passed locally; the earlier synthetic built-CLI proof covers a full stdin pipe, SIGINT exit, and child cleanup. Codex autoreview found no actionable findings in its default P0 scope. Thanks @SebTardif.

[steipete]

Landed as 78545c2 with the docs/changelog follow-up. Thanks @SebTardif!

Final head 925ecd4 passed GOWORK=off go test -count=1 -timeout 3m ./internal/mcpclient ./internal/slackmcp, GOWORK=off go vet ./internal/mcpclient ./internal/slackmcp, and make smoke. All GitHub checks passed, including CI, Docker, CodeQL, and secret scanning. The earlier synthetic built-CLI proof above confirms blocked-pipe cancellation and child cleanup. Autoreview found no actionable findings in its default P0 scope.