fix(orchestration): hide raw sub-agent dumps, dedup device tools, scope delegation, pin master-chat scroll#4831
Merged
senamakel merged 5 commits intoJul 14, 2026
Conversation
…, release failed claims run_local_agent's raw '[local sub-agent … completed]' output was persisted as a visible master-chat row, so a brain that spawns many sub-agents floods the chat with raw dumps. Tag that row with an excluded event_kind so it is hidden from the transcript/previews/unread while still forwarding to the brain. Add a dedup guard to handle_tool_call so an at-least-once redelivery does not re-spawn the sub-agent, and release the claim when dispatch fails (A2A-gate denial / unknown cycle / invalid args) so a redelivery re-runs and returns the real error instead of a fabricated accept.
…ed inline A spawn_async_subagent result collected inline via wait_subagent was still queued for background delivery, so the idle-delivery turn re-answered it as a duplicate. Add a bounded collected-task tombstone set at the wait_subagent collection seam and checked in record_completion, closing both orderings of the wait/record race under the queue mutex. Genuinely-later fire-and-forget results are untouched.
…eb/date asks A connected integration (e.g. Gmail) was treated as a standing reason to delegate to integrations_agent, so a plain date/headline ask could fetch the inbox. Add a scope gate: delegate to a connected service only when the request operates on that service's own data/actions; route general-knowledge, web/news, date/time and math to a direct tool or the matching non-integration specialist. Additive; regression test added.
The /orchestration master chat had no scroll management: every send/receive flips the transcript to a full-height loading spinner and back, resetting scrollTop to 0 (the top) instead of the new message at the bottom. Adopt the app's existing useStickToBottom hook on the scroll container (same as /chat); it re-snaps to the bottom after the spinner swap and respects a user who scrolled up. Vitest added.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughChat scrolling is made deterministic, background completions avoid duplicate delivery, integration delegation receives explicit scope gates, and local-agent tool calls gain duplicate suppression with claim release on failed dispatch. ChangesChat scrolling
Background completion collection
Integration delegation scope
Local-agent delivery semantics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant handle_tool_call
participant dispatch_device_tool
participant release_call
participant run_local_agent_and_forward
handle_tool_call->>dispatch_device_tool: dispatch run_local_agent
dispatch_device_tool-->>handle_tool_call: return dispatch status
handle_tool_call->>release_call: release claim after failure
run_local_agent_and_forward->>run_local_agent_and_forward: persist lifecycle completion
run_local_agent_and_forward-->>run_local_agent_and_forward: restore visibility if forwarding fails
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f74b040b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
coderabbitai
Bot
added
agent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/src/components/orchestration/AgentChatPanel.tsx (1)
197-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd grep-friendly diagnostics for the new pin/disengage state transition.
These are new state-transition flows (stick-to-bottom pinning vs. user-scroll disengage), but neither call site logs entry into this behavior, unlike other flows in this file (
sendComposer,runSteeringReviewusedebug(...)). A one-linedebug()call at eachuseStickToBottomcall site (or inside the hook itself) noting thread/reset key would make scroll-pin regressions easier to diagnose in the field.As per coding guidelines: "New or changed flows must include verbose, grep-friendly diagnostics for entry/exit, branches, external calls, retries/timeouts, state transitions, and errors; never log secrets or full PII."
Also applies to: 362-369
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/orchestration/AgentChatPanel.tsx` around lines 197 - 201, Add a one-line debug diagnostic at each useStickToBottom call site, including the relevant thread/session and reset key, to mark entry into the pin/disengage scroll behavior. Use the existing debug pattern in AgentChatPanel, avoid secrets or full PII, and keep the hook behavior unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/agent_registry/agents/orchestrator/prompt.md`:
- Line 25: Update the “Scope gate (required)” guidance in the orchestrator
prompt so service delegation is blocked only when the request neither names nor
clearly implies a connected service. Preserve the existing exclusions for
general-knowledge, web/news, date/time, math, and other non-service requests,
while allowing implied operations such as sending email or checking an inbox to
reach the integrations path.
In `@src/openhuman/orchestration/effect_executor.rs`:
- Around line 403-412: The failed local-execution dispatch branch in the effect
executor releases the claim without emitting diagnostics. Update the !ok path
around is_local_execution_tool and release_call to add a namespaced debug or
warn log containing the call ID, tool name, failure outcome, and claim-release
state, while excluding raw arguments and sensitive error data.
- Around line 381-397: Update the deduplication logic used by is_duplicate_call
so reaching SEEN_CALL_IDS_CAP never removes claims for calls still in flight,
including run_local_agent executions. Keep active claims in a separate
unbounded/in-flight structure, or evict only completed tombstones, while
preserving bounded storage for completed call IDs and existing duplicate re-ack
behavior in the local execution path.
- Around line 890-904: Strengthen the redelivery assertions in the
handle_tool_call test by comparing the first and second error payloads, not just
their ok flags. Assert both attempts contain the expected non-Master denial
error and that the second response preserves the same real error as the first,
while retaining the duplicate-field check.
---
Nitpick comments:
In `@app/src/components/orchestration/AgentChatPanel.tsx`:
- Around line 197-201: Add a one-line debug diagnostic at each useStickToBottom
call site, including the relevant thread/session and reset key, to mark entry
into the pin/disengage scroll behavior. Use the existing debug pattern in
AgentChatPanel, avoid secrets or full PII, and keep the hook behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 466172b6-1583-4dcc-ba68-3a195c23cbec
📒 Files selected for processing (7)
app/src/components/orchestration/AgentChatPanel.tsxapp/src/components/orchestration/__tests__/AgentChatPanel.test.tsxsrc/openhuman/agent_orchestration/background_completions.rssrc/openhuman/agent_orchestration/tools/wait_subagent.rssrc/openhuman/agent_registry/agents/orchestrator/prompt.mdsrc/openhuman/agent_registry/agents/orchestrator/prompt.rssrc/openhuman/orchestration/effect_executor.rs
… scope-gate wording, log denied dispatch, cover fallbacks - run_local_agent_and_forward un-hides the completion row (clear event_kind) when push_event fails, so an undelivered result is not lost from the UI (+ store helper clear_message_event_kind + test). - Orchestrator scope gate now delegates when a request names OR clearly implies a service (check my inbox / send an email); only no-service asks stay off it. - Log the failed device-tool dispatch + claim release (no raw args or error body). - Assert the A2A redelivery returns the same real Master-only denial. - Cover the master-chat empty-messages fallback (diff-cover).
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
2 participants
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.
Summary
[local sub-agent … completed]dumps from the/orchestrationmaster chat and dedup device tool calls, so a hosted brain that spawns many on-device sub-agents no longer floods the transcript, and a redeliveredrun_local_agentno longer re-spawns.run_local_agent— which the gate denies — is consistently re-denied on redelivery instead of returning a fabricatedaccepted.wait_subagent(duplicate reply).Problem
Surfaced testing the hosted master chat (
/orchestration):run_local_agent; its raw completion output was persisted as a visible master-chat row, so a brain that spawned N sub-agents produced N raw dumps.orch:tool_callis at-least-once, so a redelivery re-spawned the sub-agent and forwarded a duplicatetool_completion.call_idbefore dispatch but never released it on failure, so a redelivery of an A2A-deniedrun_local_agentfabricated anaccepted/runningack for a call that never ran — masking the denial and stranding the brain.spawn_async_subagentresult collected inline was still queued for background delivery, so the idle turn re-answered it.integrations_agent, so a plain date/headline ask could fetch the inbox./orchestrationtranscript has no scroll management: a send/receive flips it to a full-height loading spinner and back, clampingscrollTopto 0 (the top).Solution
event_kind(hidden from transcript/preview/unread by the existingstorefilters) while still forwarding it to the brain viapush_event; add anis_local_execution_tooldedup guard tohandle_tool_call, andrelease_callon a failed dispatch so a redelivery re-runs (the success path stays latched so it never re-spawns).wait_subagentcollection seam and checked inrecord_completion, closing both orderings of the wait/record race under the queue mutex; never-waited fire-and-forget results are untouched.useStickToBottomhook (same as/chat) — re-snaps to the bottom after the spinner swap and respects a user who scrolled up.Tradeoff: hiding the raw dumps means the user reads the orchestrator's synthesized reply rather than intermediate sub-agent output (matches the distill-not-forward intent). The prompt scope-gate is model guidance (probabilistic).
Submission Checklist
N/A: behaviour-only bug fixes, no feature rows added/removed/renamed## Related—N/A: no matrix feature affectedN/A: fixes to existing surfaces, no new release-cut surfaceCloses #NNN—N/A: surfaced during hosted master-chat testing; no filed issueImpact
/orchestrationmaster chat: cleaner transcript (raw sub-agent dumps hidden), correct autoscroll, fewer duplicate replies. A2A safety preserved —run_local_agentstays Master-gated and is now consistently denied for A2A even under redelivery. No schema/RPC/wire changes.Related
vendor/tinyagents<|tool_call>content-sentinel recovery (needs a fork oftinyhumansai/tinyagents+ a submodule bump); backend counterparttinyhumansai/backend#1086(context-bleed window + over-spawn prompt overrides).AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/orchestration-device-side-qualityValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo fmt+ core builds (pre-pushpnpm rust:checkgreen)rust:checkcoversapp/src-tauriBehavior Changes
Parity Contract
run_local_agentMaster-gating unchanged; force-delegate for real service asks unchanged; genuinely-later fire-and-forget delivery unchanged; the completion is still forwarded to the brain.handle_send_dm/handle_evict; failed-dispatch release added so A2A denial is not masked.Duplicate / Superseded PR Handling
Summary by CodeRabbit