@clawsweeper review

🦞🧹
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: Failed
• Detail: The targeted re-review did not finish cleanly. Check the workflow run for details.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27689225267
• Updated: 2026-06-17T13:12:31.745Z

Codex review: needs real behavior proof before merge. Reviewed July 1, 2026, 3:31 PM ET / 19:31 UTC.

Summary
The PR adds an AbortSignal-keyed tool-activity heartbeat and notifies it from the embedded runner Tool Search catalog executor so subagent LLM idle timers can reset after tool completion.

PR surface: Source +61. Total +61 across 3 files.

Reproducibility: yes. for the PR defects at source level: the signal-key mismatch, dropped between-wait activity, and missed direct-tool path are visible from the current code and PR diff. I did not run a live subagent timeout reproduction in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94124
Summary: This PR is a flawed candidate fix for the canonical subagent LLM idle-timeout issue; a newer open PR is the stronger current candidate, while broader timeout and completion-delivery reports remain distinct or partial overlaps.

Members:

• canonical: fix: Reset subagent session idle timeout on successful tool calls to prevent premature termination #94124 - The issue describes the same subagent LLM idle-timeout symptom during long-running tool calls.
• fixed_by_candidate: fix(agents): add tool-activity heartbeat to keep subagent alive during tool calls #95536 - The open PR targets the same canonical issue with a runId-scoped heartbeat and positive real behavior proof.
• partial_overlap: Feature: Activity-aware run timeout (wall-clock timeout kills active multi-tool runs) #41588 - That issue tracks broader activity-aware wall-clock run-timeout semantics, not only the subagent LLM idle watchdog.
• adjacent_distinct: feat: Persist subagent completion announcements to message queue instead of direct session dispatch #94153 - That issue concerns durable delivery of completed subagent announcements after child runs finish, not active tool-execution idle timeout.
• adjacent_distinct: fix: per-model cooldown scope, stepped backoff, and user-facing rate-limit message #49834 - That merged PR changed model fallback cooldown behavior and does not implement tool-activity idle-timeout refresh.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Rework the heartbeat around run-scoped state or a single shared lifecycle key.
• Cover shared direct tools and cataloged/deferred tools with focused idle-timeout tests.
• [P1] Add redacted terminal output, logs, recording, or linked artifact from a real after-fix subagent run.

Proof guidance:

• [P1] Needs real behavior proof before merge: No after-fix real subagent run evidence is present for this branch; tests or CI alone would not satisfy the external PR proof gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] The listener and notifier use different AbortSignal objects, so the heartbeat can be a no-op on the reported path.
• [P1] The PR emits a one-shot completion event without persistent run-scoped activity state, so activity between provider waits can be dropped.
• [P1] The hook is limited to the Tool Search catalog executor and misses normal direct tools such as read and exec through agent-core.
• [P1] The PR lacks after-fix real behavior proof for this exact branch.

Maintainer options:

1. Rework and prove this branch before merge (recommended)
   Use one run-scoped heartbeat state, emit from shared visible and deferred tool execution paths, add focused idle-timeout tests, and require redacted real subagent proof for this exact branch.
2. Coordinate with the newer candidate
   If maintainers prefer the proof-positive runId-scoped approach, close or pause this branch only after preserving useful attribution and landing the newer candidate.

Next step before merge

• [P1] Contributor and maintainer follow-up is needed because this branch needs code rework plus contributor-supplied real behavior proof, and a stronger open candidate already exists.

Security
Cleared: The diff adds only in-process timeout signaling and does not change dependencies, workflows, package scripts, credentials, permissions, or external code execution.

Review findings

• [P1] Use one lifecycle key for tool heartbeats — src/agents/embedded-agent-runner/run/llm-idle-timeout.ts:300
• [P1] Persist activity across provider waits — src/agents/embedded-agent-runner/run/attempt.ts:3681
• [P2] Emit heartbeats from shared tool execution — src/agents/embedded-agent-runner/run/attempt.ts:3681

Hey @kumaxs — I dug into the ClawSweeper review and wanted to flag the three P1 findings with concrete pointers, plus how #95536 already addresses each one. Not opening a competing PR; just trying to help the canonical issue get unblocked whichever PR lands.

The 3 P1 findings, with the exact mismatch

1. Signal-key mismatch (confidence 0.95) — current code is a no-op

// src/agents/embedded-agent-runner/run/llm-idle-timeout.ts:299
const unsubscribeToolActivity = onToolActivity(streamAbortController.signal, armTimer);

// src/agents/embedded-agent-runner/run/attempt.ts:3678
notifyToolActivity(runAbortController.signal);

streamAbortController.signal and runAbortController.signal are two different AbortSignal objects. The new module keys the listener map by AbortSignal:

const toolActivityListeners = new WeakMap<AbortSignal, Set<() => void>>();

So notifyToolActivity(runAbortController.signal) looks up listeners registered with runAbortController.signal, but the only listener is registered against streamAbortController.signal. The notification never reaches armTimer(). The heartbeat is effectively dead code on the bug path.

2. Activity dropped between stream waits (confidence 0.88)

onToolActivity is registered inside streamWithIdleTimeout's cleanupIterator. The lifecycle is:

• register listener → wait for stream → cleanupIterator unregisters listener
• Tool completes after stream cleanup → notify fires with zero listeners

So even if the signal keys matched, the notification would still miss because the listener has already been torn down by the time the tool completes.

3. Cataloged-tools only (confidence 0.84)

The two notifyToolActivity(...) calls at attempt.ts:3678 and :3694 are inside the Tool Search catalog executor wrapper. Direct tools (read, exec, web_fetch) execute through packages/agent-core/src/agent-loop.ts executePreparedToolCall — that path bypasses this notification entirely.

How #95536 addresses all three

For awareness (so it's clear there's already a working shape on the table):

So #95536 already has the architectural pieces; it just needs proof: sufficient (real subagent run evidence) to clear the same needs-real-behavior-proof gate.

Paths forward (your call)

A few options I see, roughly ordered by blast radius:

1. Coordinate with fix(agents): add tool-activity heartbeat to keep subagent alive during tool calls #95536 — ask @xydt-tanshanshan whether they'd accept a Co-authored-by: or a co-PR with real proof on top of their runId-scoped design. Most likely to clear both gates fastest.
2. Adopt the runId-keyed pattern in this PR — re-key the WeakMap by runId, persist last-activity-ms across stream waits, wrap the shared direct-tool path. Bigger diff but matches the reviewer guidance and could replace fix(agents): add tool-activity heartbeat to keep subagent alive during tool calls #95536.
3. Close in favor of fix(agents): add tool-activity heartbeat to keep subagent alive during tool calls #95536 — if you'd rather not grind a re-key, marking this as superseded by fix(agents): add tool-activity heartbeat to keep subagent alive during tool calls #95536 unblocks the canonical issue. Your diagnosis and PoC still count.

Whatever you pick, I'd suggest pinging ClawSweeper once the patch is updated so the re-review queue picks it up — the sweep.yml dispatch has been slow on this shard.

If it's useful, I can draft a small repro harness script that exercises a subagent with a 3-minute mock tool call and shows model_call:started → tool → tool → done progression under the new heartbeat. Just let me know.

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.