From bfefbd4e9ab620ab4b1bba8f920b91079ce38beb Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sat, 29 Aug 2026 11:47:43 -0700 Subject: [PATCH 1/2] fix(mcp): honor context on stdio spawn and stdin write NewStdio discarded its context and started exec.Command, and write checked ctx.Err() once then blocked on stdin.Write. A child that stopped reading stdin never reached Close(), so sync --source mcp hung through Ctrl-C. Spawn with CommandContext, fail canceled spawn immediately, and write stdin asynchronously so cancel can close the pipe and kill the child. Signed-off-by: Sebastien Tardif --- internal/mcpclient/client_test.go | 54 +++++++++++++++++++++++++++++++ internal/mcpclient/stdio.go | 34 ++++++++++++++++--- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/internal/mcpclient/client_test.go b/internal/mcpclient/client_test.go index 953af35..867d9e7 100644 --- a/internal/mcpclient/client_test.go +++ b/internal/mcpclient/client_test.go @@ -143,6 +143,60 @@ func TestStdioClientUsesMinimalEnvironment(t *testing.T) { require.JSONEq(t, `{"allowed":"visible","secret":""}`, text) } +func TestNewStdioRejectsCanceledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + client, err := NewStdio(ctx, StdioOptions{Command: "/bin/sleep", Args: []string{"30"}}) + if client != nil { + _ = client.Close() + } + require.ErrorIs(t, err, context.Canceled) + require.Nil(t, client) +} + +func TestNewStdioStopsChildWhenContextCanceled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + client, err := NewStdio(ctx, StdioOptions{Command: "/bin/sleep", Args: []string{"30"}}) + require.NoError(t, err) + cancel() + select { + case <-client.waitCh: + _ = client.stdin.Close() + case <-time.After(5 * time.Second): + _ = client.Close() + t.Fatal("MCP stdio child still running after spawn context cancel") + } +} + +func TestStdioWriteUnblocksWhenContextCanceled(t *testing.T) { + client, err := NewStdio(context.Background(), StdioOptions{Command: "/bin/sleep", Args: []string{"30"}}) + require.NoError(t, err) + defer func() { _ = client.Close() }() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan error, 1) + go func() { + done <- client.write(ctx, map[string]any{ + "jsonrpc": "2.0", + "method": "initialize", + "params": map[string]any{"blob": strings.Repeat("x", 2<<20)}, + }) + }() + select { + case err := <-done: + t.Fatalf("write returned before cancel: %v", err) + case <-time.After(100 * time.Millisecond): + } + cancel() + select { + case err := <-done: + require.ErrorIs(t, err, context.Canceled) + case <-time.After(5 * time.Second): + t.Fatal("write stayed blocked after context cancel") + } +} + func TestMCPStdioHelperProcess(t *testing.T) { if os.Getenv("GO_WANT_MCP_STDIO_HELPER") != "1" { return diff --git a/internal/mcpclient/stdio.go b/internal/mcpclient/stdio.go index 114930f..2b8ae6f 100644 --- a/internal/mcpclient/stdio.go +++ b/internal/mcpclient/stdio.go @@ -51,10 +51,13 @@ type StdioClient struct { waitErr error } -func NewStdio(_ context.Context, opts StdioOptions) (*StdioClient, error) { +func NewStdio(ctx context.Context, opts StdioOptions) (*StdioClient, error) { if strings.TrimSpace(opts.Command) == "" { return nil, errors.New("MCP stdio command is required") } + if err := ctx.Err(); err != nil { + return nil, err + } if opts.ProtocolVersion == "" { opts.ProtocolVersion = DefaultProtocolVersion } @@ -64,7 +67,7 @@ func NewStdio(_ context.Context, opts StdioOptions) (*StdioClient, error) { if opts.ClientVersion == "" { opts.ClientVersion = "dev" } - cmd := exec.Command(opts.Command, opts.Args...) + cmd := exec.CommandContext(ctx, opts.Command, opts.Args...) cmd.Env = stdioEnvironment(opts) stdin, err := cmd.StdinPipe() if err != nil { @@ -277,10 +280,31 @@ func (c *StdioClient) write(ctx context.Context, value any) error { } c.writeMu.Lock() defer c.writeMu.Unlock() - if _, err := c.stdin.Write(append(raw, '\n')); err != nil { - return fmt.Errorf("write MCP stdio request: %w", err) + if err := ctx.Err(); err != nil { + return err + } + // Write from a goroutine so a child that stops reading stdin cannot + // park the caller after context cancel. Closing stdin and killing the + // process unblocks a full pipe, matching provider.Sync. + errCh := make(chan error, 1) + go func() { + _, writeErr := c.stdin.Write(append(raw, '\n')) + errCh <- writeErr + }() + select { + case <-ctx.Done(): + _ = c.stdin.Close() + if c.cmd != nil && c.cmd.Process != nil { + _ = c.cmd.Process.Kill() + } + <-errCh + return ctx.Err() + case err := <-errCh: + if err != nil { + return fmt.Errorf("write MCP stdio request: %w", err) + } + return nil } - return nil } func (c *StdioClient) readLoop(stdout io.Reader) { From b9509480d5ab9b8c9715603f9f6b1d4fc5661c78 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 31 Aug 2026 01:01:21 -0700 Subject: [PATCH 2/2] docs: record cancellable MCP stdio requests Co-authored-by: Sebastien Tardif --- CHANGELOG.md | 4 ++++ docs/configuration.md | 2 ++ 2 files changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 293df4a..236ad8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Stop MCP stdio servers and unblock full stdin pipes when sync is canceled. Thanks @SebTardif! (#164) + ### Maintenance - Updated Go to 1.27.0, SQLite to 1.57.0, Go runtime and test dependencies, Alpine to 3.24, pre-commit hooks to 6.0.0, and CodeQL and TruffleHog action pins. diff --git a/docs/configuration.md b/docs/configuration.md index a8a723b..df13f79 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -188,6 +188,8 @@ The subprocess receives a minimal environment plus known Slack/Codex token varia Tool discovery selects either the Codex Slack connector contract or the reference `slack_list_channels`, `slack_get_channel_history`, `slack_get_thread_replies`, and `slack_get_users` contract. +Ctrl-C cancels MCP sync and stops its stdio server, including when the server stops reading requests and fills the stdin pipe. + ## External Archive Providers Use `[[providers]]` to adapt another local archive to the canonical SQLite