Wire tool_result feedback for multi-turn command execution#314
Wire tool_result feedback for multi-turn command execution#314ashishpatel26 wants to merge 2 commits into
Conversation
When the planner agent sends a Send action and the coordinator executes it in a terminal pane, the agent previously had no way to observe the outcome. This meant every recommendation was a one-shot fire-and-forget with no follow-up reasoning. This commit closes the feedback loop: - `ChoiceExecution` gains a `session_id` field so the recommendation executor knows which ACP session dispatched the choice. - `execute_choice` passes `session_id` down and, after a non-insert Send action completes, calls the new `capture_and_feed_output` helper. - `capture_and_feed_output` waits 1.5 s for the command to finish, reads the pane output (OSC 133 shell-integration marks when available, falling back to the last 30 buffer lines), and emits `AppEvent::CommandOutputReady`. - `App::handle_command_output_ready` synthesizes a follow-up `PromptSubmission` carrying the command text and captured output, calls `turn_submit_prompt`, and forwards the submission over `prompt_tx` so the ACP client delivers it as the next user turn. The agent now receives the terminal output after each recommended command and can decide next steps: confirm success, diagnose errors, suggest corrections, or continue a multi-step plan. The autofix fallback path (`session_id = None`) is explicitly excluded from the loop; the heavy send-and-capture path is skipped for insert- only actions where no command was executed. All 904 existing tests pass; one new test (`command_output_ready_submits _follow_up_prompt`) documents and protects the behaviour. Closes microsoft#203
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds a multi-turn command execution feedback loop: after a Send action runs in a terminal pane, the coordinator captures recent terminal output and the app submits it back to the agent as a follow-up prompt.
Changes:
- Extend
ChoiceExecutionto carry an optional ACPsession_idand thread it into choice execution. - Capture terminal output after
Sendand emit a newAppEvent::CommandOutputReady. - Handle
CommandOutputReadyby synthesizing and submitting a follow-up prompt, with a new unit test validating the behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/wta/src/coordinator.rs | Adds session-aware post-Send output capture and emits CommandOutputReady. |
| tools/wta/src/app/autofix.rs | Sets session_id: None for the autofix fallback execution path. |
| tools/wta/src/app.rs | Introduces CommandOutputReady, handles it by submitting a follow-up prompt, and adds a unit test. |
| if let Some(sid) = session_id { | ||
| capture_and_feed_output( | ||
| sid, | ||
| parent, | ||
| input, | ||
| shell_mgr, | ||
| event_tx, | ||
| ) | ||
| .await; | ||
| } |
| let follow_up_text = format!( | ||
| "## Command Output\n\nThe command `{}` was executed in pane `{}`.\n\nHere is the terminal output:\n\n```\n{}\n```\n\nPlease review the output and continue helping the user. If the command succeeded, let me know. If there were any errors or unexpected results, please help diagnose and resolve them.", |
| const MAX_CHARS: usize = 4000; | ||
|
|
||
| // Try shell-integration first (OSC 133 marks give us the exact last command+output). | ||
| if let Ok(value) = shell_mgr.wt_read_last_prompt(pane_id).await { | ||
| let has_marks = value | ||
| .get("has_marks") | ||
| .and_then(|v| v.as_bool()) | ||
| .unwrap_or(false); | ||
| if has_marks { | ||
| if let Some(content) = value.get("content").and_then(|c| c.as_str()) { | ||
| if !content.is_empty() { | ||
| return Some(truncate_for_log(content, MAX_CHARS)); | ||
| } | ||
| } | ||
| } | ||
| } |
@check-spelling-bot Report
|
| Dictionary | Entries | Covers | Uniquely |
|---|---|---|---|
| cspell:csharp/csharp.txt | 32 | 2 | 2 |
| cspell:aws/aws.txt | 232 | 2 | 2 |
| cspell:fonts/fonts.txt | 536 | 1 | 1 |
Consider adding to the extra_dictionaries array (in the .github/actions/spelling/config.json file):
"cspell:csharp/csharp.txt",
"cspell:aws/aws.txt",
"cspell:fonts/fonts.txt",
To stop checking additional dictionaries, put (in the .github/actions/spelling/config.json file):
"check_extra_dictionaries": []Forbidden patterns 🙅 (1)
In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.
These forbidden patterns matched content:
Should be reentrancy
[Rr]e[- ]entrancy
Errors and Warnings ❌ (2)
See the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.
| ❌ Errors and Warnings | Count |
|---|---|
| 54 | |
| ❌ forbidden-pattern | 1 |
See ❌ Event descriptions for more information.
✏️ Contributor please read this
By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
- ... misspelled, then please correct them instead of using the command.
- ... names, please add them to
.github/actions/spelling/allow/names.txt. - ... APIs, you can add them to a file in
.github/actions/spelling/allow/. - ... just things you're using, please add them to an appropriate file in
.github/actions/spelling/expect/. - ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in
.github/actions/spelling/patterns/.
See the README.md in each directory for more information.
🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
|
@microsoft-github-policy-service agree |
1 similar comment
|
@microsoft-github-policy-service agree |
Closes #203
Summary
ChoiceExecutiongainssession_id— the recommendation executor now knows which ACP session dispatched the Send action.capture_and_feed_outputhelper (coordinator.rs) — after a non-insert Send completes, waits 1.5 s for the command to finish, reads pane output (OSC 133 shell-integration marks first, falls back to last 30 buffer lines), and emitsAppEvent::CommandOutputReady.AppEvent::CommandOutputReadyvariant (app.rs) — carriessession_id,pane_id,command, andoutput.App::handle_command_output_readyhandler — synthesizes a follow-upPromptSubmissionwith the command text and captured output, submits it viaturn_submit_prompt, and forwards it overprompt_txto the ACP client as the next user turn.The agent now receives terminal output after each recommended command and can decide next steps: confirm success, diagnose errors, suggest corrections, or continue a multi-step plan.
Exclusions:
session_id = None) is excluded from the loop.Test plan
command_output_ready_submits_follow_up_promptverifies thatCommandOutputReadyforwards aPromptSubmissiontoprompt_txcontaining the command + output text