PR #84383: fix(agents): credit delivered subagent completions

• Repository: openclaw/openclaw
• Author: neeravmakwana
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/84383

Description (problem / solution / changelog)

Summary

• Problem: expectsCompletionMessage subagent cleanup treated the later parent announce as the only delivery signal, so a parent NO_REPLY could retry even after the requester thread already had a committed delivery mirror.
• Solution: when a completion announce reports no visible delivery, lifecycle cleanup now checks the requester session for a matching openclaw/delivery-mirror assistant message before retrying.
• What changed: matching prior delivery mirrors clear the delivery error path, preserve the mirror timestamp as completionDeliveredAt, finalize cleanup as delivered, and skip pending-final retry bookkeeping.
• What did NOT change (scope boundary): no channel delivery APIs, prompt instructions, message-tool requirements, config defaults, or provider behavior changed.

Motivation

• Fixes the bookkeeping failure reported in #84270, where Slack already had the subagent completion reply but the registry retried the completion announce until retry-limit.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes #84270
• This PR fixes a bug or regression

Real behavior proof (required for external PRs)

• Behavior addressed: completion cleanup now credits an already-committed requester delivery mirror instead of retrying a parent NO_REPLY announce.
• Real environment tested: local macOS source checkout, Node/Vitest test harness for the subagent lifecycle controller.
• Exact steps or command run after this patch: node scripts/run-vitest.mjs src/agents/subagent-registry-lifecycle.test.ts; pnpm check:changed; git diff --check.
• Evidence after fix: copied terminal output from the full lifecycle file run: Test Files 2 passed (2) and Tests 46 passed (46).
• Observed result after fix: the regression asserts chat.history is queried for the requester session, completionDeliveredAt/completionAnnouncedAt are set from the mirror timestamp, lastAnnounceDeliveryError is cleared, pendingFinalDelivery and announceRetryCount remain unset, detached task delivery is marked delivered, and logAnnounceGiveUp is not called.
• What was not tested: live Slack workspace delivery with a real gateway session.
• Before evidence: the issue log showed Subagent announce give up (retry-limit) ... retries=3; the pre-existing focused lifecycle proof covered the old failure bookkeeping path with completion agent did not produce a visible reply / UNAVAILABLE: requester wake failed style delivery errors.

Root Cause (if applicable)

• Root cause: cleanup finalized based on didAnnounce = delivery.delivered from the later parent-mediated announce only, and did not consult requester-session delivery mirror evidence from the child’s already-committed visible reply.
• Missing detection / guardrail: lifecycle tests covered invisible completion announce failure and retry state, but not the case where a matching requester delivery mirror already proved user-visible delivery.
• Contributing context (if known): N/A.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: src/agents/subagent-registry-lifecycle.test.ts.
• Scenario the test should lock in: parent announce returns no visible delivery while the requester session already contains a matching delivery-mirror completion reply.
• Why this is the smallest reliable guardrail: it exercises the lifecycle retry/finalize decision directly without requiring a live Slack workspace.
• Existing test that already covers this (if any): N/A.
• If no new test is added, why not: N/A.

User-visible / Behavior Changes

Subagent completion runs that already delivered a visible reply to the requester thread should stop retrying/giving up on the later completion announce when the parent correctly replies NO_REPLY.

Diagram (if applicable)

Before:
child delivered reply -> requester delivery mirror exists -> parent announce NO_REPLY -> retry/give-up

After:
child delivered reply -> requester delivery mirror exists -> parent announce NO_REPLY -> cleanup delivered

Security Impact (required)

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No
• Command/tool execution surface changed? No
• Data access scope changed? No
• If any Yes, explain risk + mitigation: N/A

Security/runtime controls unchanged: message-tool-only delivery policy and channel send enforcement remain in the existing delivery path. This patch only credits a persisted openclaw delivery-mirror in the same requester session after runtime delivery has already committed it.

Repro + Verification

Environment

• OS: macOS local checkout
• Runtime/container: Node/Vitest via repo wrapper
• Model/provider: N/A
• Integration/channel (if any): mocked requester session history for Slack-like completion lifecycle
• Relevant config (redacted): N/A

Steps

1. Simulate a completed expectsCompletionMessage subagent run.
2. Make the parent announce report delivered: false with completion agent did not produce a visible reply.
3. Return a matching requester delivery-mirror message from chat.history.

Expected

• Cleanup finalizes as delivered and does not schedule pending final delivery retries.

Actual

• The new regression passes with delivered status, cleared error state, no pending final delivery, no retry count, and no give-up log.

Evidence

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Verification run output:

git diff --check
# passed with no output

node scripts/run-vitest.mjs src/agents/subagent-registry-lifecycle.test.ts
Test Files  2 passed (2)
Tests  46 passed (46)

pnpm check:changed
Found 0 warnings and 0 errors.
runtime-sidecar-loaders: local runtime sidecar loaders look OK.
Import cycle check: 0 runtime value cycle(s).

Human Verification (required)

• Verified scenarios: matching requester delivery mirror credits a failed parent announce; existing failure bookkeeping still remains covered.
• Edge cases checked: matching uses normalized visible text and requires role: assistant, provider: openclaw, and model: delivery-mirror in the same requester session history.
• What you did not verify: live Slack end-to-end delivery.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

AI-assisted: yes.

Compatibility / Migration

• Backward compatible? Yes
• Config/env changes? No
• Migration needed? No
• If yes, exact upgrade steps: N/A

Risks and Mitigations

• Risk: accidentally crediting the wrong transcript entry.
  • Mitigation: only requester-session openclaw/delivery-mirror assistant messages with matching normalized completion text are credited.
• Out-of-scope follow-up: live Slack proof for the original redacted production run remains outside this small code/test patch.

Made with Cursor

Changed files

• src/agents/subagent-registry-lifecycle.test.ts (modified, +112/-0)
• src/agents/subagent-registry-lifecycle.ts (modified, +76/-4)