Follow-up from PR #1145: test coverage gap, indefinite-block risk, dead callback parameter

[edelauna]

Context

PR #1145 ([Fix] Zoo starts another step before terminal command finishes) removes the command_output ask mechanism and replaces nine lifecycle tests with three contract tests. This issue tracks three follow-up items surfaced during review.

---

#1 — Missing integration-level coverage for the foreground-completion contract (Medium)

File: `src/core/tools/tests/executeCommandTool.spec.ts`

The three replacement tests use hand-driven mock terminals with fake timers. They prove `task.ask` and `process.continue()` are not called before the test manually fires `onCompleted`, but they do not exercise real Execa or VS Code shell-integration event chains through `presentAssistantMessage` to the next reasoning request.

All four existing e2e cases in `apps/vscode-e2e/src/suite/tools/execute-command.test.ts` set `terminalShellIntegrationDisabled: true`. No VS Code shell-integration scenario exists, and none asserts next-command ordering after early output.

Suggested action: Add a case to the e2e suite (entry point: `globalThis.api.startNewTask()`) that:

• Enables shell integration
• Emits an early marker, waits more than five seconds, then emits final output
• Asserts the second tool call did not start before the first finished

Add a separate explicit-timeout case asserting `still running` text precedes eventual completion.

---

#2 — Removal of user-controlled escape path for long-running foreground commands (Medium)

File: `src/core/tools/ExecuteCommandTool.ts` (pre-image lines ~337–398)

PR #1145 removes `askForCommandOutput`, `scheduleCommandOutputAsk`, and the five-second timer. These were the only production mechanism letting a user provide feedback or background a foreground command that was still running after producing output.

The agent timeout (`agentTimeout > 0`) remains as the opt-in background path, but only fires when the model specifies it at tool-call time. A foreground command with neither `agentTimeout` nor `commandExecutionTimeout` configured now blocks indefinitely with no user-controlled interrupt short of aborting the entire task.

The removed mechanism was itself added as a fix (commit `e209c7c81`: "fix(terminal): delay command_output ask for short foreground commands"). PR #1145 trades the overlapping-commands bug (ask triggered `process.continue()` early, causing the next reasoning step to start before the command finished) for an indefinite-block problem on unbounded commands.

Suggested action: One of:

• Add a task-level background/abort UI action that calls `process.continue()` or `task.terminalProcess?.abort()` independently of streamed output.
• Add a configurable hard-deadline background transition that does not depend on whether output has been emitted.
• At minimum, document that commands with no configured `agentTimeout` may block the agent indefinitely, and surface a recommendation to set one for long-running commands.

---

#3 — RooTerminalCallbacks process parameter dead across all five callbacks (Low)

File: `src/integrations/terminal/types.ts`, lines 25–29

All five callbacks in the `RooTerminalCallbacks` interface declare `process: RooTerminalProcess` as a second argument. After PR #1145, no callback implementation reads this parameter. Both adapters still forward the reference, and test call sites still pass `proc` as the second arg — all are silent no-ops.

// types.ts — all five still carry the unused second param
onLine: (line: string, process: RooTerminalProcess) => void
onCompleted: (output: string | undefined, process: RooTerminalProcess) => void | Promise<void>
onShellExecutionStarted: (pid: number | undefined, process: RooTerminalProcess) => void
onShellExecutionComplete: (details: ExitCodeDetails, process: RooTerminalProcess) => void
onNoShellIntegration?: (details: ShellIntegrationErrorDetails, process: RooTerminalProcess) => void

Suggested action: Remove `process: RooTerminalProcess` from all five signatures; stop forwarding it in both adapter `runCommand` methods; update test call sites to match the reduced signatures. This is a mechanical cleanup with no functional impact.

---

Surfaced by automated code review of PR #1145.