Skip to content

[codex] Queue subagent completion announces#76927

Closed
steipete wants to merge 1 commit into
mainfrom
fix/subagent-completion-announce-queue
Closed

[codex] Queue subagent completion announces#76927
steipete wants to merge 1 commit into
mainfrom
fix/subagent-completion-announce-queue

Conversation

@steipete

@steipete steipete commented May 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Route subagent completion announces through the requester session steer/announce queue before direct fallback.
  • Force completion handoffs onto an internal queue independent of channel queue mode, while preserving bound route delivery.
  • Keep direct expectFinal announce delivery only as fallback when queue routing is unavailable, and avoid retrying gateway timeout failures before direct message fallback.

Root Cause

Completion-mode announces were direct-first and used synchronous gateway agent calls with expectFinal, so a busy or stuck requester session could leave direct announce turns waiting behind active work and trigger repeated 120s timeout retries.

Validation

  • pnpm test src/agents/subagent-announce-dispatch.test.ts src/agents/subagent-announce-delivery.test.ts src/agents/subagent-announce.format.e2e.test.ts src/agents/subagent-announce.timeout.test.ts src/agents/subagent-registry-lifecycle.test.ts
  • pnpm exec oxfmt --check --threads=1 src/agents/subagent-announce-dispatch.ts src/agents/subagent-announce-dispatch.test.ts src/agents/subagent-announce-delivery.ts src/agents/subagent-announce-delivery.test.ts src/agents/subagent-announce.format.e2e.test.ts src/agents/subagent-announce.timeout.test.ts src/agents/subagent-registry-lifecycle.test.ts
  • git diff --check
  • Testbox pnpm check:changed: https://github.com/openclaw/openclaw/actions/runs/25289933276

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels May 3, 2026
@clawsweeper

clawsweeper Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep this PR open for maintainer review: the central queue-first completion announce change is not on current main or the latest release, but the PR is draft/conflicting, has a protected maintainer label, lacks real behavior proof, and overlaps the broader durable delivery tracker.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #67777
Summary: This PR is a narrow candidate fix for the direct-first/timeout portion of the canonical durable subagent completion delivery problem, while the broader restart/orphan/durable-spool design remains open.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

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

Yes at source level: current main still dispatches completion handoffs through direct-primary before steer fallback, and the PR changes that ordering. I did not run a fresh live busy-requester transport repro in this read-only pass.

Is this the best way to solve the issue?

Unclear as the complete best fix: queue-first routing is a plausible scoped mitigation, but the durable spool, restart/orphan preservation, and task-ledger contract remains canonical in #67777.

Security review:

Security review cleared: The diff changes agent delivery logic, tests, docs, and changelog text; no dependency, workflow, package, credential, or supply-chain change was found.

AGENTS.md: found and applied where relevant.

What I checked:

  • stale F-rated PR: PR was opened 2026-05-03T20:30:19Z, is older than 60 days, and the latest review rated it F.
  • proof blocker: real behavior proof is mock_only and proof tier is F, so this branch is not merge-ready without contributor follow-up.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • steipete: Introduced the unified subagent announce delivery pipeline, later split announce delivery helpers, and authored the queue-first PR head under review. (role: feature-history contributor; confidence: high; commits: 4258a3307f5a, b75be0914491, f9eb7d993c26; files: src/agents/subagent-announce-dispatch.ts, src/agents/subagent-announce-delivery.ts, docs/tools/subagents.md)
  • vincentkoc: Authored related completion-timeout and release stabilization changes, and issue discussion identifies his partial delivery hardening commits in the same subagent completion area. (role: recent area contributor; confidence: high; commits: 8ad0ca309e84, e085fa1a3ffd; files: src/agents/subagent-announce-delivery.ts, src/agents/subagent-registry-lifecycle.ts)
  • Takhoffman: History shows focused work preserving direct subagent dispatch failures on abort in the dispatch file that this PR changes. (role: adjacent dispatch contributor; confidence: medium; commits: 0fc27409c0e4; files: src/agents/subagent-announce-dispatch.ts)
  • tyler6204: History shows adjacent work including extension channels in the subagent announce delivery path. (role: adjacent delivery contributor; confidence: medium; commits: 41cf93efff4d; files: src/agents/subagent-announce-delivery.ts)

Codex review notes: model internal, reasoning high; reviewed against 010b61746379.

@steipete
steipete force-pushed the fix/subagent-completion-announce-queue branch from d3fc201 to f9eb7d9 Compare May 3, 2026 21:12
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 14, 2026
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. labels Jun 15, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Closing this stale draft per the latest review. The queued-completion direction was useful, but the canonical subagent completion path has since evolved and this branch no longer shares current history. Any remaining work should be recreated against current main under #67777 with queue ownership, wake precedence, and live delivery proof established together.

@steipete steipete closed this Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation maintainer Maintainer-authored PR mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant