fix: tool confirmation handling for multiple requests by lifeizhou-ap · Pull Request #7856 · block/goose

lifeizhou-ap · 2026-03-13T10:53:09Z

Summary

Fix Issue #5558

Change

Replace the shared Mutexmpsc::Receiver confirmation channel with a ToolConfirmationRouter that uses per-request oneshot channels keyed by request ID, so concurrent tool confirmations are routed directly to the correct task instead of being silently discarded when they arrive out of order.

Type of Change

AI Assistance

This PR was created or reviewed with AI assistance

Testing

Unit tests
Manual test (regression)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b895697720

ℹ️ 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".

chatgpt-codex-connector · 2026-03-13T11:56:08Z

crates/goose/src/agents/tool_confirmation_router.rs

+
+    pub async fn register(&self, request_id: String) -> oneshot::Receiver<PermissionConfirmation> {
+        let (tx, rx) = oneshot::channel();
+        self.pending.lock().await.insert(request_id, tx);


Remove pending confirmation entry when waiter is cancelled

register stores a oneshot::Sender in pending, but there is no corresponding cleanup path if the waiting task drops its receiver before any confirmation is delivered (for example, client disconnect/cancel while waiting on action-required input). In that case the map entry persists indefinitely unless a later deliver happens for the same request ID, so repeated cancelled confirmations can cause unbounded growth of stale entries in the long-lived Agent.

Useful? React with 👍 / 👎.

removed when the next register is called

wpfleger96

this is great thank you!

* origin/main: fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843)

* main: (191 commits) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) Add github actions workflow for unused deps (#7681) fix: prevent SSE connection drops from silently truncating responses (#7831) doc: Added notes in contribution guide for pnpm (#7833) add prefer-offline to pnpm config to skip unnecessary registry lookups (#7828) fix: remove dead read handler from DeveloperClient (#7821) ...

* main: (65 commits) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) ...

…oken-retry * origin/main: (21 commits) Remove java/.ai-usage-marker directory (#7925) test(acp): add terminal delegation fixtures and fix shell singleton (#7923) fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892) feat: persist GooseMode per-session via session DB (#7854) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) ...

* origin/main: (72 commits) No Check do Check (#7942) Log 500 errors and also show error for direct download (#7936) fix: retry on authentication failure with credential refresh (#7812) Remove java/.ai-usage-marker directory (#7925) test(acp): add terminal delegation fixtures and fix shell singleton (#7923) fix: bump pctx_code_mode to 0.3.0 for iterator type checking fix (#7892) feat: persist GooseMode per-session via session DB (#7854) feat(otel): propagate session.id to spans and log records (#7490) fix(test): add env_lock to is_openai_reasoning_model tests (#7917) fix(acp): pass session_id when loading extensions so skills are discovered (#7868) updated canonical models (#7920) feat(autovisualiser): Migrate the autovisualiser extension to MCP Apps (#7852) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) ...

lifeizhou-ap added 2 commits March 13, 2026 21:52

tool confirmation handling for multiple requests

243b61b

refactor

b895697

lifeizhou-ap changed the title ~~tool confirmation handling for multiple requests~~ fix: tool confirmation handling for multiple requests Mar 13, 2026

lifeizhou-ap requested review from DOsinga and wpfleger96 March 13, 2026 11:49

lifeizhou-ap marked this pull request as ready for review March 13, 2026 11:49

chatgpt-codex-connector bot reviewed Mar 13, 2026

View reviewed changes

Prune stale pending entries when reciever was dropped

c4345ba

wpfleger96 approved these changes Mar 13, 2026

View reviewed changes

lifeizhou-ap added this pull request to the merge queue Mar 15, 2026

Merged via the queue into main with commit 291bbd8 Mar 15, 2026
22 of 23 checks passed

lifeizhou-ap deleted the lifei/tool-confirmation-handling branch March 15, 2026 22:59

github-actions bot mentioned this pull request Mar 16, 2026

chore(release): release version 1.28.0 (minor) #7780

Open

wpfleger96 mentioned this pull request Mar 16, 2026

Tool calls hang indefinitely when confirmations arrive out of order #5558

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: tool confirmation handling for multiple requests#7856

fix: tool confirmation handling for multiple requests#7856
lifeizhou-ap merged 3 commits intomainfrom
lifei/tool-confirmation-handling

lifeizhou-ap commented Mar 13, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 13, 2026

Uh oh!

lifeizhou-ap Mar 13, 2026

Uh oh!

wpfleger96 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lifeizhou-ap commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change

Type of Change

AI Assistance

Testing

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

lifeizhou-ap Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

wpfleger96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lifeizhou-ap commented Mar 13, 2026 •

edited

Loading