Include delivery errors in subagent announce give-up logs by TurboTheTurtle · Pull Request #84281 · openclaw/openclaw

TurboTheTurtle · 2026-05-19T19:37:55Z

Summary

Include the saved lastAnnounceDeliveryError in subagent announce retry-limit / expiry give-up warnings.
Normalize multiline delivery errors onto one gateway log line so gateway.log remains searchable.
Log direct completion announce delivery failures through the normal gateway log path instead of stderr-only output.

Root cause

Subagent announce delivery already formatted and persisted the concrete delivery failure on the run entry, but logAnnounceGiveUp() only emitted run, child, requester, retry count, and ended age. On macOS managed gateways where stderr can be discarded, the final gateway.log warning lost the useful delivery cause.

Real behavior proof

Behavior or issue addressed:
Subagent announce retry-limit give-up logs now include the underlying delivery error in the normal gateway log output.

Real environment tested:
Local OpenClaw source checkout on macOS using the production logAnnounceGiveUp() implementation and defaultRuntime.log capture.

Exact steps or command run after this patch:

PATH=/Users/andy/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH node --import tsx --input-type=module --eval 'import { defaultRuntime } from "./src/runtime.ts"; import { logAnnounceGiveUp } from "./src/agents/subagent-registry-helpers.ts"; const originalLog = defaultRuntime.log; const lines = []; defaultRuntime.log = (line) => lines.push(String(line)); const originalNow = Date.now; Date.now = () => 9000; logAnnounceGiveUp({ runId: "run-proof", childSessionKey: "agent:main:subagent:child", requesterSessionKey: "agent:main:main", requesterDisplayKey: "main", task: "finish", cleanup: "keep", createdAt: 1000, startedAt: 2000, endedAt: 4000, announceRetryCount: 3, lastAnnounceDeliveryError: "direct-primary: routed-dispatch-did-not-queue-final\nsteer-fallback: queue_message_failed" }, "retry-limit"); Date.now = originalNow; defaultRuntime.log = originalLog; console.log(JSON.stringify({ loggedLine: lines[0], hasDeliveryError: lines[0]?.includes("deliveryError="), multilineCollapsed: !lines[0]?.includes("\\n") }, null, 2));'

Evidence after fix:

{
  "loggedLine": "[warn] Subagent announce give up (retry-limit) run=run-proof child=agent:main:subagent:child requester=agent:main:main retries=3 endedAgo=5s deliveryError=\"direct-primary: routed-dispatch-did-not-queue-final steer-fallback: queue_message_failed\"",
  "hasDeliveryError": true,
  "multilineCollapsed": true
}

Observed result after fix:
The normal gateway log warning includes deliveryError=..., preserves the concrete delivery failure, and collapses multiline errors onto one log line.

What was not tested:
No live macOS LaunchAgent was started. The proof exercises the production logging helper directly, and focused tests cover the direct completion announce failure path.

Validation

node scripts/run-vitest.mjs src/agents/subagent-registry-lifecycle.test.ts
node scripts/run-vitest.mjs src/agents/subagent-announce.test.ts src/agents/subagent-announce-dispatch.test.ts
git diff --check

Attribution

If maintainers squash or rework this PR, please preserve author attribution or include:

Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>

clawsweeper · 2026-05-19T19:40:30Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The branch adds delivery error text to subagent announce give-up logging, moves direct completion failure logging to the gateway logger, and adds focused tests for both paths.

Reproducibility: yes. at source level: current main persists lastAnnounceDeliveryError during failed announce cleanup but logAnnounceGiveUp omits it from the final warning. I did not execute tests because this review is read-only.

PR rating
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Summary: Good focused PR with sufficient terminal proof, narrow implementation, and no blocking findings.

Rank-up moves:

none

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

PR egg
✨ Hatched: 🥚 common Clockwork Patch Peep

       /\  .---.  /\         
      /  \/     \/  \        
     /   ( -   - )   \       
    |       ._.       |      
    |   /|  ===  |\   |      
     \  \|______/|/  /       
      '._  `--'  _.'         
         '-.__.-'            
       _/|_|  |_|\_          
      /__|      |__\         
       .-----------.         
      '-------------'

Rarity: 🥚 common.
Trait: guards the happy path.
Image traits: location release reef; accessory tiny test log scroll; palette plum, gold, and soft gray; mood proud; pose leaning over a miniature review desk; shell starlit enamel shell; lighting tiny status-light glow; background miniature CI buoys.
How to hatch it: once this PR reaches status: 👀 ready for maintainer look or status: 🚀 automerge armed, the PR author or a maintainer can comment @clawsweeper hatch to turn this ASCII egg into its generated creature image.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Clockwork Patch Peep in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchable usually means sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

Real behavior proof
Sufficient (terminal): The PR body supplies after-fix terminal output from a local source checkout showing the production helper logs deliveryError and collapses multiline text.

Risk before merge
Why this matters: - No live macOS LaunchAgent run was attached; the supplied proof exercises the production helper and focused tests cover the affected logging paths.

Maintainer options:

Decide the mitigation before merge
Land the focused observability fix after normal CI and maintainer review, preserving the existing retry policy, delivery ordering, and cleanup semantics.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No ClawSweeper repair job is needed; the remaining action is ordinary maintainer review and CI gating, with the linked issue left open until this PR lands.

Security
Cleared: The diff only changes local diagnostic logging and focused tests; it does not touch dependencies, workflows, permissions, credentials, or external code execution paths.

Review details

Best possible solution:

Land the focused observability fix after normal CI and maintainer review, preserving the existing retry policy, delivery ordering, and cleanup semantics.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main persists lastAnnounceDeliveryError during failed announce cleanup but logAnnounceGiveUp omits it from the final warning. I did not execute tests because this review is read-only.

Is this the best way to solve the issue?

Yes: the PR uses the existing persisted error field and normal gateway log path, which is the narrow maintainable fix for this observability-only bug.

Label justifications:

P2: The PR addresses a concrete agents observability bug with limited blast radius and focused tests/proof.

What I checked:

Current main persists the delivery error: The lifecycle code formats failed announce delivery details from the delivery error and phase errors, then stores the formatted value in entry.lastAnnounceDeliveryError during cleanup retry handling. (src/agents/subagent-registry-lifecycle.ts:136, 5c9a8f33b312)
Current main drops that value from the final warning: logAnnounceGiveUp currently logs run, child, requester, retry count, and ended age, but does not include lastAnnounceDeliveryError. (src/agents/subagent-registry-helpers.ts:68, 5c9a8f33b312)
Current main still uses stderr for direct completion failures: The direct completion failure path logs through defaultRuntime.error, matching the reported stderr-only diagnostic gap on managed gateways. (src/agents/subagent-announce.ts:575, 5c9a8f33b312)
PR diff addresses the implicated paths: The PR head appends deliveryError to give-up warnings with whitespace normalization, switches direct completion failure logging to defaultRuntime.log with a [warn] prefix, and adds focused tests for direct failure logging and multiline error collapse. (src/agents/subagent-registry-helpers.ts:65, 0da8934e1f07)
Current tests already prove the saved-error half: An existing lifecycle test on current main verifies failed cleanup persists the concrete announce delivery error, so the remaining bug is final log visibility rather than capture. (src/agents/subagent-registry-lifecycle.test.ts:841, 5c9a8f33b312)
Feature-history provenance: git blame and git log both tie the current helper/direct-log implementation and lastAnnounceDeliveryError surface to commit 1f794d2. (src/agents/subagent-registry-helpers.ts:68, 1f794d2816d1)

Likely related people:

Ayaan Zaidi: git blame and git log -S point the current subagent announce lifecycle files, logAnnounceGiveUp helper, direct completion failure logging, and lastAnnounceDeliveryError field to commit 1f794d2. (role: introduced current behavior; confidence: medium; commits: 1f794d2816d1; files: src/agents/subagent-announce.ts, src/agents/subagent-registry-helpers.ts, src/agents/subagent-registry-lifecycle.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 5c9a8f33b312.

TurboTheTurtle · 2026-05-19T22:16:44Z

Pushed a fix for the failed check-test-types job. The failing test intentionally mocked an announce delivery error response, but the test spy's inferred return type only allowed { runId, status }; I widened that local test response type to include error.

Local validation:

node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
node scripts/run-vitest.mjs src/agents/subagent-announce.test.ts src/agents/subagent-registry-helpers.test.ts (26 tests passed)
./node_modules/.bin/oxfmt --check --threads=1 src/agents/subagent-announce.test.ts src/agents/subagent-announce.ts src/agents/subagent-registry-helpers.ts src/agents/subagent-registry-helpers.test.ts
git diff --check

@clawsweeper re-review

clawsweeper · 2026-05-19T22:18:34Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26128733446
Updated: 2026-05-19T22:25:25.702Z

bek91 · 2026-05-20T03:22:26Z

Thanks @TurboTheTurtle! I inspected the PR diff, changed files, issue/PR discussion, surrounding delivery/lifecycle code, and current PR checks. Implementation is narrow and does not change delivery behavior, just where the warning is written. Resolves #84272

…4281) * Include delivery error in subagent announce give-up logs * test(agents): type announce delivery error response

Include delivery error in subagent announce give-up logs

3e64322

openclaw-barnacle Bot added agents Agent runtime and tooling size: S proof: supplied External PR includes structured after-fix real behavior proof. labels May 19, 2026

TurboTheTurtle marked this pull request as ready for review May 19, 2026 19:38

test(agents): type announce delivery error response

0da8934

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026

bek91 self-requested a review May 20, 2026 03:17

bek91 merged commit 9108ae0 into openclaw:main May 20, 2026
110 checks passed

TurboTheTurtle deleted the fix/subagent-announce-delivery-error-log-84272 branch May 20, 2026 03:39

SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026

Include delivery errors in subagent announce give-up logs (openclaw#8…

5fd507e

…4281) * Include delivery error in subagent announce give-up logs * test(agents): type announce delivery error response

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Include delivery errors in subagent announce give-up logs#84281

Include delivery errors in subagent announce give-up logs#84281
bek91 merged 2 commits into
openclaw:mainfrom
TurboTheTurtle:fix/subagent-announce-delivery-error-log-84272

TurboTheTurtle commented May 19, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 •

edited

Loading

Uh oh!

TurboTheTurtle commented May 19, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 •

edited

Loading

Uh oh!

bek91 commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TurboTheTurtle commented May 19, 2026

Summary

Root cause

Real behavior proof

Validation

Attribution

Uh oh!

clawsweeper Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TurboTheTurtle commented May 19, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bek91 commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented May 19, 2026 •

edited

Loading

clawsweeper Bot commented May 19, 2026 •

edited

Loading