fix(subagents): fall through to gateway call when steer+fallback fail#75669
fix(subagents): fall through to gateway call when steer+fallback fail#75669yozakura-ava wants to merge 3 commits into
Conversation
|
Codex review: needs maintainer review before merge. Summary Reproducibility: yes. Against current main, call completion delivery with expectsCompletionMessage true, an active requester session id, queueEmbeddedPiMessage returning false, and direct fallback returning false or throwing; the code returns delivered:false before reaching callGateway. Next step before merge Security Review detailsBest possible solution: Land this focused fallthrough change after maintainer review and required validation, keeping the paired #75663 issue open until merge closes it. Do we have a high-confidence way to reproduce the issue? Yes. Against current main, call completion delivery with expectsCompletionMessage true, an active requester session id, queueEmbeddedPiMessage returning false, and direct fallback returning false or throwing; the code returns delivered:false before reaching callGateway. Is this the best way to solve the issue? Yes. Reusing the existing gateway agent delivery path after steer and direct fallback fail is the narrowest maintainable fix, while preserving early returns when steering or fallback succeeds. What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against a3564ae546f5. |
3c03af3 to
7f88a15
Compare
Review Feedback AddressedPushed an updated commit (7f88a15) that addresses both review findings: 1. Changelog entry addedAdded to CHANGELOG.md under Unreleased / Fixes: 2. Throw test now exercises the actual catch pathAdded internalEvents with a task_completion event to the fallback-throws test so completionFallbackText is populated and sendCompletionFallback actually invokes sendMessage (which throws), exercising the removed catch path. Added expect(sendMessage).toHaveBeenCalled() to confirm. SigningCommit is SSH-signed. Will add the signing key to GitHub verified keys shortly. |
fc5998f to
3be199c
Compare
When a subagent completes and the parent session has an active but
non-consuming Pi run, steer correctly returns false. The completion
fallback (direct channel send) often fails too (empty text, no target).
Previously both failure paths returned { delivered: false } as a dead-end,
preventing the working callGateway('agent', ...) path from being reached.
Fix: Remove early returns in the steer-fallback block. Let execution
fall through to the existing gateway call that starts a new run and
delivers the message.
Adds two tests:
- steer fails + fallback throws → falls through to gateway call
(with internalEvents so sendCompletionFallback actually exercises sendMessage)
- steer succeeds → returns steered, gateway not called (regression)
Includes CHANGELOG.md entry under Unreleased/Fixes.
Closes: openclaw#75663
Co-Authored-By: Paperclip <noreply@paperclip.ing>
The post-gateway fallback retry checks hasVisibleGatewayAgentPayload before retrying sendCompletionFallback. Without visible payloads in the mock response, the second fallback attempt also throws and the outer catch returns delivered:false. Fix: mock returns result.payloads with text so the retry is skipped and delivery succeeds. Co-Authored-By: Paperclip <noreply@paperclip.ing>
ClawSweeper P2: the catch block on the fallback failure path no longer
reads the error variable. Use catch {} to satisfy the
unicorn/prefer-optional-catch-binding lint rule.
3be199c to
c1d30f9
Compare
|
Closing — upstream has structurally changed the subagent completion delivery infrastructure since this PR was opened (removed agent-mediated completion path, added sendMessage-based fallback). Opening a new PR with a remediated fix against the current codebase. |
Summary
When a subagent completes and the parent session has an active but non-consuming Pi run (e.g., between turns, waiting for LLM response), the completion announcement is silently dropped instead of being delivered.
Fixes #75663
Root Cause
In
sendSubagentAnnounceDirectly, when steer returnsfalse(run not consuming) and the completion fallback also fails (often emptycompletionFallbackText), the function returns{ delivered: false }as a dead-end — even though a workingcallGateway("agent", ...)path exists 30 lines below.Changes
src/agents/subagent-announce-delivery.ts(~10 lines changed)return { delivered: false }in the catch block ofsendCompletionFallbackreturn { delivered: false }after the fallback try blockcallGateway("agent", { expectFinal: true })callsrc/agents/subagent-announce-delivery.test.ts(+2 tests)Impact
Testing
Risk
Low. The gateway call path already exists and works — we are just removing a dead-end that prevented it from being reached. No new dependencies, no architectural changes.