-
Notifications
You must be signed in to change notification settings - Fork 55
refactor(#2429): instruct orchestrator to trust subagent results #2431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -419,6 +419,27 @@ orchestrator's core value-add — no sub-agent sees findings from other | |
| dimensions, so only the orchestrator can detect overlaps and | ||
| cross-references. | ||
|
|
||
| **Trust subagent investigation results.** Sub-agents perform thorough | ||
| investigation during their dispatch — reading source files, querying | ||
| external APIs (npm, GitHub, etc.), and tracing code paths. Their tool | ||
| call outputs and conclusions are authoritative evidence. During | ||
| synthesis, the orchestrator MUST: | ||
|
|
||
| 1. **Consume subagent evidence as-is.** Do not re-execute commands | ||
| that a subagent already ran (e.g., `npm view`, `gh api` for tags, | ||
| releases, or commits, `curl` to registries). The subagent's output | ||
| is the evidence — re-running the same command wastes tool calls and | ||
| adds latency without producing new information. | ||
| 2. **Re-investigate only on conflict.** The only justification for | ||
| re-executing a subagent's command is when two subagents return | ||
| contradictory findings about the same artifact and the orchestrator | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case Rule 2 ('Re-investigate only on conflict') does not explicitly account for subagent commands that returned errors or empty results. If a subagent's external command failed transiently, the orchestrator would be instructed to trust the failed output as authoritative. Step 5 handles complete subagent failures, but the narrower scenario of a subagent completing successfully while one of its internal commands errored is not covered. Suggested fix: Consider adding a clause: 'or when a subagent command returned an error or empty result indicating a transient failure.' |
||
| needs to resolve the conflict. In that case, note why the | ||
| re-investigation is necessary. | ||
| 3. **Do not re-read files that subagents already read.** If a | ||
| subagent's findings reference specific file contents or code | ||
| patterns, trust those references. Use `Read` or `Grep` only for | ||
| files or lines that no subagent examined. | ||
|
|
||
| #### 6a. Group findings by file and line range | ||
|
|
||
| Group all findings by file path and overlapping line ranges. Findings | ||
|
|
@@ -828,3 +849,9 @@ wins. | |
| - **In pipeline mode, `gh pr review` is reserved for the post-script.** | ||
| The sandbox token is read-only. Write JSON to | ||
| `$FULLSEND_OUTPUT_DIR/agent-result.json` and exit. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [info] code-organization The new constraint bullet at the end of the Constraints section restates the 'Trust subagent investigation results' section from step 6. This duplication is consistent with the document's existing pattern — several constraint bullets restate rules from earlier sections to serve as a quick-reference checklist. |
||
| - **Do not re-execute subagent investigation commands during | ||
| synthesis.** Subagent tool call outputs are authoritative evidence. | ||
| The orchestrator must not re-run the same external commands (npm | ||
| view, gh api, curl, etc.) that a subagent already executed unless | ||
| resolving a specific conflict between subagent findings. See step 6 | ||
| for details. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] architectural-conflict
The added guidance improves LLM-orchestrator efficiency through prompting. ADR-0018 rejected LLM-based orchestration for deterministic workflows, but SKILL.md lines 13-19 already explicitly acknowledge this departure and flag that a superseding ADR is needed. This PR does not widen the architectural conflict — it optimizes behavior within the already-acknowledged non-conforming implementation.