Skip to content

[Fix] Zoo prompts about command output when short foreground commands emit output - #1043

Merged
edelauna merged 5 commits into
mainfrom
fix/command-output-ask-delay-0o6ia7mkzlnpy
Jul 29, 2026
Merged

[Fix] Zoo prompts about command output when short foreground commands emit output#1043
edelauna merged 5 commits into
mainfrom
fix/command-output-ask-delay-0o6ia7mkzlnpy

Conversation

@zoomote

@zoomote zoomote Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Created by Roomote. Follow up by mentioning @zoomote, in the web UI, or in Discord.

Related GitHub Issue

Closes: #1042

Description

After the terminal streaming fixes in #834 made output capture reliable, a pre-existing prompt path became much more visible: the command_output ask in ExecuteCommandTool.executeCommandInTerminal fired on the first output chunk of any foreground command, so Zoo prompted users about command output even for routine commands that were about to complete.

This PR changes the ask policy so the first output line no longer triggers the ask by itself:

  • The ask is now scheduled via a timer anchored to actual execution start (COMMAND_OUTPUT_ASK_DELAY_MS = 5_000 from onShellExecutionStarted, falling back to the command dispatch time for providers that never report that event). It only fires if the command is still running once the delay elapses, so short foreground commands stream their output and complete without ever prompting — including on cold terminals, where the shell-integration startup wait would otherwise consume the grace period.
  • The timer is cancelled when the command completes, when the agent timeout moves the command to the background, and on all exit paths, so completion never races a stale prompt. If output somehow arrives before the execution-started event, the pending ask is re-anchored rather than firing early.
  • When the command completes while an ask is still on screen, the pending ask is superseded immediately (instead of lingering until the next interactive message), and the webview clears the Proceed/Kill controls when the final command_output update arrives.
  • Any ask answer — not just typed feedback — now continues the process, so an answered ask actually backgrounds the command and the tool resolves with the "still running" result instead of blocking until completion.
  • The long-running behavior is otherwise unchanged: once the delay elapses with the command still running, the ask fires and users can interrupt or provide feedback exactly as before. Output streaming to the UI, persisted output artifacts, and the final tool result are untouched.

Reviewers may want to focus on the timer lifecycle in src/core/tools/ExecuteCommandTool.ts (scheduling in scheduleCommandOutputAsk, anchoring in onShellExecutionStarted, supersession and cancellation in onCompleted, the agent-timeout path, and the finally block) and the controls cleanup in webview-ui/src/components/chat/ChatView.tsx.

Test Procedure

  • The "command_output ask policy" describe block in src/core/tools/__tests__/executeCommandTool.spec.ts uses a controllable terminal mock (whose continue() mirrors real terminals by resolving the wait early) and fake timers to cover:
    • A short command that emits output and exits normally never triggers task.ask("command_output", ""), even after advancing past the full ask delay.
    • A command still running after COMMAND_OUTPUT_ASK_DELAY_MS does trigger the ask, a messageResponse continues the process, and later output does not re-ask.
    • Cold-terminal anchoring: shell-integration startup time does not consume the grace period, and a pending ask is re-anchored if output precedes the execution-started event — while still firing at the re-anchored deadline.
    • Fallback anchoring when the execution-started event never fires.
    • Agent-timeout background transition cancels a pending ask and suppresses later asks.
    • Ask errors are swallowed without failing the command; a non-message answer backgrounds the command and the tool resolves before the process completes.
    • A pending ask is superseded when the command completes.
  • A ChatView spec verifies the Proceed/Kill controls clear when the final non-partial command_output say arrives.
  • Reproduce/verify with: pnpm --dir src exec vitest run core/tools/__tests__/executeCommandTool.spec.ts (25 tests) and pnpm --dir webview-ui exec vitest run src/components/chat/__tests__/ChatView.spec.tsx (24 tests).
  • Full pnpm test, pnpm lint, and pnpm check-types pass.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Visual Snapshot (UI changes only): Not applicable per webview-ui/AGENTS.md — the webview change is behavior-only (control state transition), covered by a Vitest JSDOM test, not a static rendered-state change.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Visual Snapshots

Not applicable — behavior-only change per webview-ui/AGENTS.md snapshot guidance.

Videos (interaction / animation only)

Not applicable — behavior change is covered by tool-layer and webview unit tests; no browser surface exists for the extension host flow.

Documentation Updates

Related PRs

Additional Notes

The separate /tmp/roo-cmd-* multiline wrapper UX mentioned in the issue is intentionally out of scope, per the issue's "Related" note. A pre-existing cross-command edge (a backgrounded command's final output say clearing a newer command's controls) was noted during review and left for a follow-up since says carry no executionId.

Get in Touch

Available via the linked Roomote task or Discord thread above.

The command_output ask fired on the first output chunk of any foreground
command, prompting users even for commands about to complete. Schedule the
ask instead so it only fires when the command is still running after a
5s delay, preserving the interrupt/feedback path for long-running commands
while letting short commands finish without prompting.

Closes #1042
@zoomote

zoomote Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

No new code issues found. See task

  • src/core/tools/ExecuteCommandTool.ts:484 — The ask-delay anchor (commandStartedAt = Date.now()) is set before runCommand, so on the VS Code terminal provider the shell-integration startup wait (waitForShellIntegration, default timeout 5s in BaseTerminal.defaultShellIntegrationTimeout) counts against the 5s grace period. On a cold terminal that takes several seconds to activate shell integration, remainingDelay is ~0 by the time the command actually executes, so a short command still prompts immediately on its first output — the exact cold-terminal scenario Reduce noisy command_output prompts for normal foreground commands #1042 calls out. Consider anchoring the delay to onShellExecutionStarted (fired by both terminal providers when execution actually begins), falling back to the runCommand timestamp if that event never fires.

Reviewed 6027c41

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/core/tools/ExecuteCommandTool.ts 93.93% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@zoomote

zoomote Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

All three review threads are addressed in 6027c41 (pushed; substantive changes in ab70d8f):

  • Pending ask at completion: onCompleted now supersedes a still-pending command_output ask immediately, and ChatView clears the Proceed/Kill controls when the final non-partial command_output say arrives (partial streaming says can't trigger it). Covered by new tool-layer and ChatView tests.
  • Re-anchor test: now also asserts the ask still fires at the re-anchored deadline, so a clear-without-reschedule regression fails.
  • Non-message answers: exposed and fixed a real inconsistency — runInBackground = true was set but the process was only continued for typed messages, so non-message answers blocked until actual completion. Any ask answer now continues the process; the test asserts the tool resolves with the "still running" result without the process ever completing.

All three threads were replied to and resolved. Validation: 25/25 tool-layer tests, 24/24 ChatView tests, full pnpm test, pnpm lint, and pnpm check-types all pass. PR description refreshed to match the shipped diff.

roomote added 2 commits July 29, 2026 00:01
Anchor the ask-delay timer to onShellExecutionStarted (falling back to the
pre-runCommand timestamp) so shell-integration startup on cold terminals
does not consume the grace period, and expand ask-policy tests to cover
the agent-timeout cancel, re-anchor reschedule, ask error handling, and
non-message responses to restore patch coverage.
Comment thread src/core/tools/ExecuteCommandTool.ts
Comment thread src/core/tools/__tests__/executeCommandTool.spec.ts
Comment thread src/core/tools/__tests__/executeCommandTool.spec.ts
roomote added 2 commits July 29, 2026 00:49
- Supersede a still-pending command_output ask in onCompleted so it
  resolves immediately instead of lingering until the next interactive
  message, and clear the Proceed/Kill controls in the webview when the
  final non-partial command_output say arrives.
- Continue the process for any ask answer, not just typed messages, so a
  non-message response actually backgrounds the command and the tool
  resolves before the process completes.
- Strengthen ask-policy tests: assert the rescheduled ask still fires at
  the re-anchored deadline, assert early resolution for non-message
  answers, and cover ask supersession on completion.
@edelauna
edelauna marked this pull request as ready for review July 29, 2026 01:19
@github-actions github-actions Bot added the awaiting-review PR changes are ready and waiting for maintainer re-review label Jul 29, 2026
@edelauna
edelauna added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 2c987fc Jul 29, 2026
29 checks passed
@edelauna
edelauna deleted the fix/command-output-ask-delay-0o6ia7mkzlnpy branch July 29, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR changes are ready and waiting for maintainer re-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce noisy command_output prompts for normal foreground commands

2 participants