Skip to content

fix: prevent session hang when tool approval required in headless mode#7915

Merged
jh-block merged 3 commits into
aaif-goose:mainfrom
vincenzopalazzo:claude/interesting-zhukovsky
Apr 15, 2026
Merged

fix: prevent session hang when tool approval required in headless mode#7915
jh-block merged 3 commits into
aaif-goose:mainfrom
vincenzopalazzo:claude/interesting-zhukovsky

Conversation

@vincenzopalazzo
Copy link
Copy Markdown
Contributor

Summary

  • Fix infinite hang in non-interactive/headless CLI sessions when a tool requires approval (e.g., activating an MCP server in SmartApprove/Approve mode)
  • Guard cliclack::select() interactive prompts behind the existing interactive flag in process_agent_response
  • Auto-allow tool calls with Permission::AllowOnce in headless mode (with a tracing warning for observability)
  • Gracefully cancel elicitation requests in headless mode instead of blocking on terminal input

Root Cause

process_agent_response in crates/goose-cli/src/session/mod.rs unconditionally called cliclack::select() — an interactive terminal prompt — when a tool required approval, regardless of whether the session was interactive or headless. Since headless/remote sessions have no terminal attached, cliclack::select() would block forever waiting for input that never comes. This prevented MCP server activation (via manage_extensions) and any other tool requiring approval from completing in headless sessions.

The interactive flag was already passed into the function and used for display decisions, but was never checked before the approval prompt.

Test plan

  • cargo check -p goose-cli compiles cleanly
  • cargo test -p goose-cli — all 112 tests pass
  • Manual: run a headless session that triggers tool approval and verify it no longer hangs
  • Manual: verify interactive sessions still show the approval prompt as before

🤖 Generated with Claude Code

@vincenzopalazzo vincenzopalazzo force-pushed the claude/interesting-zhukovsky branch from 35211f1 to 831f5f8 Compare March 15, 2026 20:32
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 35211f16ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose-cli/src/session/mod.rs
@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

Addressed review feedback from @chatgpt-codex-connector: process_message() now takes an explicit interactive parameter instead of hardcoding false. This ensures the initial prompt in interactive() correctly shows the approval dialog (passes true), while headless() and scenario tests pass false. Fixed in 839ee26.

@chatgpt-codex-connector
Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 839ee268bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose-cli/src/session/mod.rs
Comment thread crates/goose-cli/src/session/mod.rs
In non-interactive/headless CLI sessions, tool confirmation prompts
(cliclack::select) block forever since there is no terminal attached.
This caused MCP server activation and any tool requiring approval to
hang indefinitely in remote/headless sessions.

Changes:
- Add interactive parameter to process_message() so callers pass the
  correct value (true for interactive sessions, false for headless)
- Guard prompt_tool_confirmation() behind the interactive flag: in
  headless mode, auto-allow with Permission::AllowOnce + log warning
- Guard elicitation collection behind the interactive flag: in headless
  mode, cancel gracefully instead of blocking on terminal input

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the claude/interesting-zhukovsky branch from 839ee26 to 687db41 Compare March 16, 2026 05:27
@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

@jh-block Re: your comment — good point, I agree. Silently auto-allowing defeats the purpose of Approve/SmartApprove mode. Fail-closed is safer here.

I checked the callers of the interactive=false path:

  • headless() — user explicitly chose headless, should know their config
  • Debug session helper (builder.rs) — creates its own agent
  • Scenario tests — typically use Auto mode, so unaffected

None of these have a legitimate reason to silently bypass approval. I'll update this to error out early in process_agent_response when !interactive and the GooseMode is Approve/SmartApprove, making it clear the configuration is invalid rather than hiding it behind a warning log.

…lled in headless mode

When elicitation is requested in non-interactive mode, return an error
so automation pipelines correctly report failure instead of exit 0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the claude/interesting-zhukovsky branch from e35c71b to b8e42cb Compare March 17, 2026 20:42
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8e42cb3aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose-cli/src/session/mod.rs
@DOsinga DOsinga added the needs_human label to set when a robot looks at a PR and can't handle it label Mar 20, 2026
@vincenzopalazzo
Copy link
Copy Markdown
Contributor Author

Hi @jh-block & @DOsinga, is there anything left in this PR to unlock it

…ve mode

Instead of silently auto-allowing tool calls when running headless in
Approve/SmartApprove mode (which bypasses the safety contract), return
an error indicating this is an invalid configuration. Auto mode still
auto-allows in headless sessions as before.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the claude/interesting-zhukovsky branch from c6d9166 to 6306be2 Compare March 31, 2026 16:32
@jh-block jh-block added this pull request to the merge queue Apr 15, 2026
Merged via the queue into aaif-goose:main with commit b7f1316 Apr 15, 2026
22 checks passed
@vincenzopalazzo vincenzopalazzo deleted the claude/interesting-zhukovsky branch April 15, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs_human label to set when a robot looks at a PR and can't handle it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants