fix(subagents): fixed subagents returning empty responses#3125
Merged
Conversation
Contributor
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 makes runSubAgent more resilient by ensuring sub-agent output is preserved even when parent cost updates fail, and improves output extraction by aggregating text from multiple sources (final response blocks, step outputs, or persisted assistant messages).
Changes:
- Make parent session cost update best-effort (log warning instead of failing the tool response).
- Add
subAgentOutputlogic to derive text output from final response content, step outputs, or stored assistant messages. - Expand coordinator tests to cover new output/fallback behaviors and update recover tests to use context-aware requests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/server/recover_test.go | Uses context-aware request creation in recover handler tests. |
| internal/agent/coordinator_test.go | Adds helper constructors and multiple new test cases validating sub-agent output/fallback scenarios. |
| internal/agent/coordinator.go | Makes cost update best-effort and introduces output aggregation / fallback logic for sub-agent responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…test Return the sub-agent diagnostic fallback as an error response (IsError=true, nil Go error) so the parent LLM can distinguish "no usable output" from a real answer; subAgentOutput now returns (text, ok) with ok=false only for the last-resort diagnostic. Also replace the brittle 100ms wall-clock assertion in TestJQ_CtxCancel_PreCancel with an invariant-based reader that fails if read, fixing flaky Windows CI under -race.
Member
|
i believe we can simplify this a bit by making a change upstream in fantasy charmbracelet/fantasy#283 |
Contributor
Author
|
Nice, I like simplifications and when things are made more elegant 👍 |
Collapse agentResultWithTextBlocks into agentResultWithText and fix subAgentOutput doc to match actual fantasy behavior.
cd01c22 to
aed4c19
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3118
This PR appears to fix a bug where the
agenttool (and maybeagentic_fetchtoo) would sometimes fail to return any response.I've tested it and it appears to work.
AI Disclosure
The investigation is described in #3118 and was done as 3 separate independent investigations by GLM-5.2, Opus 4.8, and GPT-5.5. GPT-5.5 and Opus 4.8 both reached the same conclusion. GLM-5.2 found a separate possibility that Opus 4.8 said was valid. Both bugs were fixed by Opus 4.8 (planning, max thinking) and GPT-5.5 (implementation, x-high thinking).
1 round of review was done with Opus 4.8.
CONTRIBUTING.md.