Skip to content

fix: use command.process_group(0) for CLI providers, not just MCP#7083

Merged
codefromthecrypt merged 2 commits intomainfrom
adrian/process_group
Feb 9, 2026
Merged

fix: use command.process_group(0) for CLI providers, not just MCP#7083
codefromthecrypt merged 2 commits intomainfrom
adrian/process_group

Conversation

@codefromthecrypt
Copy link
Collaborator

Summary

Ctrl+C cannot cancel CLI provider responses — the user must wait for the full response. The subprocess shares the parent's process group and receives SIGINT directly, racing with goose's cancel_token and preventing the cancel branch from firing.

MCP servers already had process_group(0) in extension_manager.rs. This PR centralizes it into configure_command_no_window() in subprocess.rs, isolating all subprocesses. With the fix, the subprocess never receives SIGINT and the cancel token wins cleanly.

I could not reproduce the "Broken pipe" from #7070 — the claude CLI (2.1.37) survives SIGINT. Other CLI providers may not. The fix eliminates this variable.

Type of Change

  • Bug fix

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

Before (goose v1.23.2, no process_group(0) on CLI providers): Ctrl+C has no effect. The response completes in full and cannot be interrupted.

$ goose --version                                                                                                                                                                 
 1.23.2
$ claude --version
2.1.37 (Claude Code)
$ goose
starting session | provider: claude-code model: claude-sonnet-4-20250514
    session id: 20260209_22
    working directory: /Users/codefromthecrypt/oss/goose

goose is running! Enter your instructions, or try asking what goose can do.

Context: ○○○○○○○○○○ 0% (0/200000 tokens)
( O)> write a poem
Here's a poem for you:

---

**The Goose That Codes**

A goose once wandered through the wire,
Through circuits humming, climbing higher,
Not feathered wings but Rust and steel,
With tools and agents at its heel.

It parsed the prompts, it read the files,
It traversed directories for miles,
Through crates and modules, line by line,
It made the tangled code align.

"Honk!" it said, or so we'd dream,
As tokens flowed in endless stream,
No pond, no bread, no grassy lawn—
Just terminals from dusk to dawn.

So raise a cup to goose, our friend,
Who'll debug with you until the end,
An agent born of open source,
A most magnificent digital goose. 🪿

---

⏱️  Elapsed time: 9.87s
Context: ○○○○○○○○○○ 0% (4/200000 tokens)
( O)> write a poem again
Here's another one for you:
--snip--

After (dev build with process_group(0) in subprocess.rs): Ctrl+C cancels immediately. Retry works.

$ cargo build -p goose-cli
--snip--
$ target/debug/goose
starting session | provider: claude-code model: claude-sonnet-4-20250514
    session id: 20260209_19
    working directory: /Users/codefromthecrypt/oss/goose

goose is running! Enter your instructions, or try asking what goose can do.

Context: ○○○○○○○○○○ 0% (0/200000 tokens)
( O)> write a poem
◒  Unifying understanding units...                                                                                                                                                                               ^CInterrupted before the model replied and removed the last message.

⏱️  Elapsed time: 0.59s
Context: ○○○○○○○○○○ 0% (0/200000 tokens)
( O)> write a poem again
◓  Exploring solution space...                                                                                                                                                                                   ◓  Exploring solution space...                                                                                                                                                                                   ◒  Exploring solution space...                                                                                                                                                                                   Interrupted before the model replied and removed the last message.

⏱️  Elapsed time: 1.48s
Context: ○○○○○○○○○○ 0% (0/200000 tokens)

Related Issues

Relates to #7070 once this change is in, the original "ctrl-c" test should be retried with the latest version of claude-code.

Related claude-code issues: #16135, #17466

Follow-up: The surviving subprocess may continue writing stale output after Ctrl+C. A follow-up PR will drain or cancel this before the next turn.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Ctrl+C cancellation for CLI-based providers by ensuring spawned subprocesses are isolated into their own process group on Unix, preventing them from receiving SIGINT intended for goose.

Changes:

  • Centralize Unix process_group(0) configuration into configure_command_no_window() so it applies consistently to CLI provider subprocesses.
  • Remove the now-redundant process_group(0) call from the MCP extension manager path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/goose/src/subprocess.rs Extends the shared subprocess configuration helper to also isolate Unix subprocesses into their own process group.
crates/goose/src/agents/extension_manager.rs Removes the local process_group(0) call in favor of the centralized helper.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
Signed-off-by: Adrian Cole <adrian@tetrate.io>
Copilot AI review requested due to automatic review settings February 9, 2026 05:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks nice.

@codefromthecrypt codefromthecrypt added this pull request to the merge queue Feb 9, 2026
Merged via the queue into main with commit d4865ae Feb 9, 2026
25 checks passed
@codefromthecrypt codefromthecrypt deleted the adrian/process_group branch February 9, 2026 05:43
rabi added a commit to rabi/goose that referenced this pull request Feb 9, 2026
Add streaming support for the Claude Code CLI provider using
--include-partial-messages for incremental text deltas.

When a streaming response is cancelled mid-read (e.g. Ctrl+C), the
child process stays alive (protected by process_group(0) from block#7083)
but has unread data in its stdout pipe. On the next turn, this stale
data would corrupt the NDJSON protocol.

Fix this by tracking a needs_drain flag on CliProcess. If a stream is
dropped before reaching a terminal event (result/error), the flag
remains set. Before the next turn, drain_pending_response reads and
discards leftover events until the protocol is re-synced.

Signed-off-by: rabi <ramishra@redhat.com>
rabi added a commit to rabi/goose that referenced this pull request Feb 9, 2026
Add streaming support for the Claude Code CLI provider using
--include-partial-messages for incremental text deltas.

When a streaming response is cancelled mid-read (e.g. Ctrl+C), the
child process stays alive (protected by process_group(0) from block#7083)
but has unread data in its stdout pipe. On the next turn, this stale
data would corrupt the NDJSON protocol.

Fix this by tracking a needs_drain flag on CliProcess. If a stream is
dropped before reaching a terminal event (result/error), the flag
remains set. Before the next turn, drain_pending_response reads and
discards leftover events until the protocol is re-synced.

Signed-off-by: rabi <ramishra@redhat.com>
rabi added a commit to rabi/goose that referenced this pull request Feb 9, 2026
Add streaming support for the Claude Code CLI provider using
--include-partial-messages for incremental text deltas.

When a streaming response is cancelled mid-read (e.g. Ctrl+C), the
child process stays alive (protected by process_group(0) from block#7083)
but has unread data in its stdout pipe. On the next turn, this stale
data would corrupt the NDJSON protocol.

Fix this by tracking a needs_drain flag on CliProcess. If a stream is
dropped before reaching a terminal event (result/error), the flag
remains set. Before the next turn, drain_pending_response reads and
discards leftover events until the protocol is re-synced.

Signed-off-by: rabi <ramishra@redhat.com>
jh-block added a commit that referenced this pull request Feb 9, 2026
* origin/main: (54 commits)
  chore: strip posthog for sessions/models/daily only (#7079)
  tidy: clean up old benchmark and add gym (#7081)
  fix: use command.process_group(0) for CLI providers, not just MCP (#7083)
  added build notify (#6891)
  test(mcp): add image tool test and consolidate MCP test fixtures (#7019)
  fix: remove Option from model listing return types, propagate errors (#7074)
  fix: lazy provider creation for goose acp (#7026) (#7066)
  Smoke tests: split compaction test and use debug build (#6984)
  fix(deps): trim bat to resolve RUSTSEC-2024-0320 (#7061)
  feat: expose AGENT_SESSION_ID env var to extension child processes (#7072)
  fix: add XML tool call parsing fallback for Qwen3-coder via Ollama (#6882)
  Remove clippy too_many_lines lint and decompose long functions (#7064)
  refactor: move disable_session_naming into AgentConfig (#7062)
  Add global config switch to disable automatic session naming (#7052)
  docs: add blog post - 8 Things You Didn't Know About Code Mode (#7059)
  fix: ensure animated elements are visible when prefers-reduced-motion is enabled (#7047)
  Show recommended model on failture (#7040)
  feat(ui): add session content search via API (#7050)
  docs: fix img url (#7053)
  Desktop UI for deleting custom providers (#7042)
  ...
tlongwell-block added a commit that referenced this pull request Feb 9, 2026
* origin/main:
  Docs: require auth optional for custom providers (#7098)
  fix: improve text-muted contrast for better readability (#7095)
  Always sync bundled extensions (#7057)
  feat: Add tom (Top Of Mind) platform extension (#7073)
  chore(docs): update GOOSE_SESSION_ID -> AGENT_SESSION_ID (#6669)
  fix(ci): switch from cargo-audit to cargo-deny for advisory scanning (#7032)
  chore(deps): bump @isaacs/brace-expansion from 5.0.0 to 5.0.1 in /evals/open-model-gym/suite (#7085)
  chore(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 in /evals/open-model-gym/mcp-harness (#7086)
  fix: switch to windows msvc (#7080)
  fix: allow unlisted models for CLI providers (#7090)
  Use goose port (#7089)
  chore: strip posthog for sessions/models/daily only (#7079)
  tidy: clean up old benchmark and add gym (#7081)
  fix: use command.process_group(0) for CLI providers, not just MCP (#7083)
  added build notify (#6891)
michaelneale added a commit that referenced this pull request Feb 10, 2026
* main: (125 commits)
  chore: add a new scenario (#7107)
  fix: Goose Desktop missing Calendar and Reminders entitlements (#7100)
  Fix 'Edit In Place' and 'Fork Session' features (#6970)
  Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082)
  Docs: require auth optional for custom providers (#7098)
  fix: improve text-muted contrast for better readability (#7095)
  Always sync bundled extensions (#7057)
  feat: Add tom (Top Of Mind) platform extension (#7073)
  chore(docs): update GOOSE_SESSION_ID -> AGENT_SESSION_ID (#6669)
  fix(ci): switch from cargo-audit to cargo-deny for advisory scanning (#7032)
  chore(deps): bump @isaacs/brace-expansion from 5.0.0 to 5.0.1 in /evals/open-model-gym/suite (#7085)
  chore(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 in /evals/open-model-gym/mcp-harness (#7086)
  fix: switch to windows msvc (#7080)
  fix: allow unlisted models for CLI providers (#7090)
  Use goose port (#7089)
  chore: strip posthog for sessions/models/daily only (#7079)
  tidy: clean up old benchmark and add gym (#7081)
  fix: use command.process_group(0) for CLI providers, not just MCP (#7083)
  added build notify (#6891)
  test(mcp): add image tool test and consolidate MCP test fixtures (#7019)
  ...
lifeizhou-ap added a commit that referenced this pull request Feb 11, 2026
* main: (85 commits)
  Fix 'Edit In Place' and 'Fork Session' features (#6970)
  Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082)
  Docs: require auth optional for custom providers (#7098)
  fix: improve text-muted contrast for better readability (#7095)
  Always sync bundled extensions (#7057)
  feat: Add tom (Top Of Mind) platform extension (#7073)
  chore(docs): update GOOSE_SESSION_ID -> AGENT_SESSION_ID (#6669)
  fix(ci): switch from cargo-audit to cargo-deny for advisory scanning (#7032)
  chore(deps): bump @isaacs/brace-expansion from 5.0.0 to 5.0.1 in /evals/open-model-gym/suite (#7085)
  chore(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 in /evals/open-model-gym/mcp-harness (#7086)
  fix: switch to windows msvc (#7080)
  fix: allow unlisted models for CLI providers (#7090)
  Use goose port (#7089)
  chore: strip posthog for sessions/models/daily only (#7079)
  tidy: clean up old benchmark and add gym (#7081)
  fix: use command.process_group(0) for CLI providers, not just MCP (#7083)
  added build notify (#6891)
  test(mcp): add image tool test and consolidate MCP test fixtures (#7019)
  fix: remove Option from model listing return types, propagate errors (#7074)
  fix: lazy provider creation for goose acp (#7026) (#7066)
  ...
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
rabi added a commit to rabi/goose that referenced this pull request Feb 12, 2026
Add streaming support for the Claude Code CLI provider using
--include-partial-messages for incremental text deltas.

When a streaming response is cancelled mid-read (e.g. Ctrl+C), the
child process stays alive (protected by process_group(0) from block#7083)
but has unread data in its stdout pipe. On the next turn, this stale
data would corrupt the NDJSON protocol.

Fix this by tracking a needs_drain flag on CliProcess. If a stream is
dropped before reaching a terminal event (result/error), the flag
remains set. Before the next turn, drain_pending_response reads and
discards leftover events until the protocol is re-synced.

Change-Id: I7eadaa829f2445b71f01fa028ffdafc43d2bc7e7
Signed-off-by: rabi <ramishra@redhat.com>
rabi added a commit to rabi/goose that referenced this pull request Feb 12, 2026
Add streaming support for the Claude Code CLI provider using
--include-partial-messages for incremental text deltas.

When a streaming response is cancelled mid-read (e.g. Ctrl+C), the
child process stays alive (protected by process_group(0) from block#7083)
but has unread data in its stdout pipe. On the next turn, this stale
data would corrupt the NDJSON protocol.

Fix this by tracking a needs_drain flag on CliProcess. If a stream is
dropped before reaching a terminal event (result/error), the flag
remains set. Before the next turn, drain_pending_response reads and
discards leftover events until the protocol is re-synced.

Change-Id: I7eadaa829f2445b71f01fa028ffdafc43d2bc7e7
Signed-off-by: rabi <ramishra@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants