Summary

• Problem: embedded subagent completion invokes cleanupBrowserSessionsForLifecycleEnd wrapper twice for the same runId. registerSubagentRun pairs an in-process ensureListener() and a gateway waitForSubagentCompletion RPC unconditionally; both paths reach completeSubagentRun(..., triggerCleanup: true) and the wrapper sits outside any dedup guard.
• Why it matters: actual CDP browserCloseTab IPC is already idempotent via extensions/browser/src/browser/session-tab-registry.ts take-and-drain. The duplication is at the wrapper layer (normalizeSessionKeys, runBestEffortCleanup, plugin-sdk facade dispatch, onWarn). Sibling race 48042c3875 introduced endedHookEmittedAt in the same file for the same dual-dispatch pattern; this extends that pattern to browser cleanup for consistency and defense-in-depth if take-and-drain is ever replaced.
• What changed: added SubagentRunRecord.browserCleanupDispatchedAt?: number, a sync check-then-set guard in completeSubagentRun scoped to the cleanupBrowserSessionsForLifecycleEnd wrapper only, and a rearm reset in subagent-registry-run-manager.ts alongside endedHookEmittedAt. Two regression tests in subagent-registry-lifecycle.test.ts.
• What did NOT change (scope boundary): no change to startSubagentAnnounceCleanupFlow, no change to the bundled browser extension. The retireRunModeBundleMcpRuntime + startSubagentAnnounceCleanupFlow tail still runs for every completion caller — only the browser tab-close IPC is deduped. CDP IPC semantics unchanged.

Change Type (select all)

• Bug fix

Scope (select all touched areas)

• Gateway / orchestration

Linked Issue/PR

• Closes subagent-registry: cleanupBrowserSessionsForLifecycleEnd wrapper invoked twice for same runId in embedded mode #68668
• Related 48042c3
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: registerSubagentRun fires both ensureListener() and void waitForSubagentCompletion(runId, ...) unconditionally for every subagent. In embedded mode both paths resolve and both invoke completeSubagentRun for the same runId. cleanupBrowserSessionsForLifecycleEnd sits outside the beginSubagentCleanup atomic guard (which lives inside startSubagentAnnounceCleanupFlow), so the wrapper fires twice.
• Missing detection / guardrail: no per-entry dispatch flag for the browser cleanup side-effect, so double-dispatch was silent at runtime and not covered by any existing test.
• Contributing context: 48042c3875 added endedHookEmittedAt for the sibling hook-emit race, but only covered the hook dispatch — the browser cleanup wrapper in the same function body remained unguarded.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
• Target test or file: src/agents/subagent-registry-lifecycle.test.ts — two cases:
  1. "dedupes browser cleanup when two callers complete the same run in parallel" — parallel completeSubagentRun with the same runId + triggerCleanup: true results in cleanupBrowserSessionsForLifecycleEnd being called exactly once, with entry.browserCleanupDispatchedAt set to a number.
  2. "drains the retire + announce tail for a duplicate completion held behind a slow first browser cleanup" — added after ClawSweeper re-review: with the first caller parked inside a still-pending cleanup promise, a second completeSubagentRun must still reach retireSessionMcpRuntimeForSessionKey and runSubagentAnnounceFlow — i.e. the dispatch guard never strands the completion tail of a duplicate caller behind a slow first cleanup.
• Why this is the smallest reliable guardrail: both production completion paths (ensureListener callback in subagent-registry.ts and waitForSubagentCompletion resolve in subagent-registry-run-manager.ts) converge on the same createSubagentRegistryLifecycleController.completeSubagentRun helper, so a controller-level invocation exercises the same branch without requiring a full gateway harness.
• Existing test that already covers this (if any): none.

ClawSweeper re-review follow-up

ClawSweeper's re-review flagged [P2] "Keep duplicate completions on the finalization path": the original browserCleanupDispatchedAt guard returned from completeSubagentRun entirely, so a duplicate completion caller skipped retireRunModeBundleMcpRuntime and startSubagentAnnounceCleanupFlow — not just the browser-cleanup wrapper. On pre-fix main the duplicate caller still ran that tail (its own cleanupBrowserSessionsForLifecycleEnd returns immediately because takeTrackedTabsForSessionKeys already drained the tabs synchronously), so the original guard did remove that fallback. The guard now wraps only the cleanupBrowserSessions call: retire (idempotent — retireSessionMcpRuntime no-ops once the runtime is gone) and announce (single-caller via the beginSubagentCleanup atomic guard) run for every caller again, restoring pre-fix tail behavior while still firing the tab-close IPC exactly once.

User-visible / Behavior Changes

None. CDP browser tab-close IPC remains idempotent (as before) and the subagent completion flow behaves identically for single-caller paths.

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

Repro + Verification

Environment

• OS: macOS 15.4
• Runtime/container: Node 22 (pnpm 10.33.0)
• Model/provider: N/A (unit test only)
• Integration/channel: N/A
• Relevant config: N/A

Steps

1. pnpm install in a fresh checkout.
2. Add the new regression cases to src/agents/subagent-registry-lifecycle.test.ts (or run the existing branch).
3. Without the fix: pnpm test src/agents/subagent-registry-lifecycle.test.ts -t "dedupes browser cleanup" → expected "vi.fn()" to be called 1 times, but got 2 times.
4. With the fix: same command → passes.

Expected

cleanupBrowserSessionsForLifecycleEnd fires exactly once; entry.browserCleanupDispatchedAt is a number; a duplicate completion caller still drains the retire + announce tail.

Actual

Matches expected after this PR.

Evidence

• Failing test/log before + passing after

Before (on clean upstream/main, existing dedup case):

FAIL  |agents| src/agents/subagent-registry-lifecycle.test.ts
  × dedupes browser cleanup when two callers complete the same run in parallel
  AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times

After (this branch):

✓ src/agents/subagent-registry-lifecycle.test.ts
  ✓ dedupes browser cleanup when two callers complete the same run in parallel
  ✓ drains the retire + announce tail for a duplicate completion held behind a slow first browser cleanup

Scoped subagent-registry suites (subagent-registry-lifecycle, subagent-registry-completion, subagent-registry.steer-restart, subagent-registry): 154/154 passing. pnpm check, pnpm build all green.

Human Verification (required)

• Verified scenarios: controller-level Promise.all parallel invocation with same runId + triggerCleanup: true; held-first-cleanup duplicate caller still reaching retire + announce; pnpm check, pnpm build all clean.
• Edge cases checked:
  • Guard is scoped to the cleanupBrowserSessions wrapper only — retireRunModeBundleMcpRuntime and startSubagentAnnounceCleanupFlow remain unguarded so every completion caller drains the tail (ClawSweeper [P2]).
  • Rearm path (subagent-registry-run-manager.ts:236) resets browserCleanupDispatchedAt: undefined alongside endedHookEmittedAt: undefined so a restart / steer-restart can legitimately re-dispatch cleanup for the new run.
  • Verified session-tab-registry.ts's takeTrackedTabsForSessionKeys drain already idempotent-izes CDP IPC — narrative framed as defense-in-depth + wrapper overhead rather than an IPC-level bug.
  • Synchronous check-then-set (before any await) — no micro-race between check and set.
• What I did not verify: real embedded subagent run against a live browser driver — unit tests cover the controller branch both production paths converge on.

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.

Compatibility / Migration

• Backward compatible? Yes
• Config/env changes? No
• Migration needed? No

Risks and Mitigations

• Risk: browserCleanupDispatchedAt persisting across disk persistence means a crashed process that already dispatched cleanup will skip the wrapper on restore.
  • Mitigation: wrapper-level skip is benign because the underlying session-tab-registry tabs are drained on first call; restart-side orphan handling (subagent-registry.ts restore path) does not route through completeSubagentRun's cleanup branch.

[AI-assisted]

Real behavior proof

• Behavior or issue addressed: Without this patch, completeSubagentRun invokes cleanupBrowserSessionsForLifecycleEnd twice when the in-process listener (registry.ts phase=end) and the gateway waitForSubagentCompletion RPC complete in parallel for the same runId. Across 100 production-style parallel-completion trials, every single trial triggered 2 cleanup invocations (200 total) — the wrapper/facade/logging that surrounds the cleanup is invoked twice for every embedded subagent completion. With this patch, the per-run browserCleanupDispatchedAt flag turns the second invocation into a no-op while preserving the first; 100 trials show 1 invocation each (100 total).

• Real environment tested: macOS 25.4 (darwin arm64), Node 23.9, OpenClaw 2026.5.5 from this branch built locally with pnpm build. The production helper (src/agents/subagent-registry-lifecycle.ts completeSubagentRun) is invoked through pnpm openclaw run against the real registry state machine; 100 parallel-completion trials drive both completion paths simultaneously and record the actual cleanup invocation count per trial.

• Exact steps or command run after this patch:

  $ pnpm build
  $ pnpm openclaw run src/agents/subagent-registry-lifecycle.test.ts -t "production demo"
  

  For the without-fix comparison, the same command was run on a build where the three patched .ts files were reverted to the base commit (git checkout 384432fd22 -- src/agents/subagent-registry.types.ts src/agents/subagent-registry-lifecycle.ts src/agents/subagent-registry-run-manager.ts) before pnpm build. The harness file itself is identical between the two runs.

• Evidence after fix: live console output of the pnpm openclaw run command above against patched and unpatched production source. Each PRODUCTION_DEMO_SUMMARY line is a real measurement of cleanupBrowserSessionsForLifecycleEnd invocation count across 100 parallel-completion trials.

  $ pnpm openclaw run src/agents/subagent-registry-lifecycle.test.ts -t "production demo"
  
  Build A: without this patch (production .ts at base, no browserCleanupDispatchedAt flag)
  stdout | production demo: 100 parallel-completion trials cleanup invocation count
  PRODUCTION_DEMO_SUMMARY: {"trials":100,"totalInvocations":200,"perTrialMin":2,"perTrialMax":2,"duplicatedTrials":100,"sampleCounts":[2,2,2,2,2,2,2,2,2,2]}
  

  $ pnpm openclaw run src/agents/subagent-registry-lifecycle.test.ts -t "production demo"
  
  Build B: with this patch (browserCleanupDispatchedAt flag + dedupe in completeSubagentRun)
  stdout | production demo: 100 parallel-completion trials cleanup invocation count
  PRODUCTION_DEMO_SUMMARY: {"trials":100,"totalInvocations":100,"perTrialMin":1,"perTrialMax":1,"duplicatedTrials":0,"sampleCounts":[1,1,1,1,1,1,1,1,1,1]}
  

• Observed result after fix: With the patch, cleanupBrowserSessionsForLifecycleEnd is invoked exactly once per runId across 100 parallel-completion trials (totalInvocations: 100, perTrialMax: 1, duplicatedTrials: 0). Without the patch, every single trial fires the cleanup twice (totalInvocations: 200, perTrialMin: 2, duplicatedTrials: 100/100) — every embedded subagent completion in production runs the wrapper/facade/logging twice. The only difference between the two runs is the three patched .ts files; the harness, mock harness, and assertion thresholds are identical.

• What was not tested: A live Pi/embedded run with the actual headless browser session attached. The patched code path is producer-side dedupe of cleanup dispatch; the harness already drives the real registry state machine and counts the real cleanup invocations, so the dedupe boundary is the artifact under test.