Greptile Summary

This PR fixes two related reliability gaps in streamTo:"parent" ACP runs: (1) the run was never registered in subagentRuns, so the normal announce/cleanup path had no record to resolve final completions against, and (2) the parent relay could emit a false 60-second stall warning even when the child had already finished (because projected lifecycle events were silent).

What changed

• acp-spawn.ts: calls registerSubagentRun for streamTo:"parent" runs, inside the same effectiveStreamToParent guard that starts the relay. The relay is correctly re-created when the gateway returns a runId that differs from the idempotency key, ensuring both the relay and the registry use the same runId.
• acp-spawn-parent-stream.ts: adds probeForCompletedRelay (a short agent.wait call to the gateway) and a readSubagentOutput transcript fallback that are checked before the stall/timeout message is emitted. If either confirms the child finished, finalizeCompletedRelay is called to emit the completion message and invoke completeSubagentRun instead.
• subagent-registry.ts: completeSubagentRun is exported so the relay can call it from the probing path.
• subagent-announce.ts: readSubagentOutput is exported and gains a synchronous JSONL transcript fallback (readSubagentOutputFromTranscript) as a last resort after the gateway history and latest-assistant-reply lookups.
• Tests across all three files migrate from vi.mock factory patterns to vi.spyOn + per-test mockRestore(), and two new tests cover the probe and transcript-fallback paths.

Two minor points to consider

• A narrow double-completion window exists if a late lifecycle phase:"end" event arrives after finalizeCompletedRelay has already called completeSubagentRun via the probe path (see inline comment at the stall-watcher block).
• The bare catch {} inside probeForCompletedRelay discards diagnostic context; a debug log would make transient probe failures easier to trace.

Confidence Score: 5/5

• Safe to merge — the fix correctly addresses the root cause and the two P2 points are edge-case concerns, not blockers.
• The core bug (missing registerSubagentRun for streamTo:"parent" runs) is fixed with a minimal, well-scoped change. The probing additions are well-guarded (disposed, stallNotified, and completed checks prevent redundant work). Test coverage is improved via the vi.spyOn migration and two new scenario tests. The double-completion concern only manifests in a narrow race where lifecycle events are in-flight during the probe window, and the existing mutated guard in completeSubagentRun provides partial protection.
• No files require special attention, though the double-completion edge case in acp-spawn-parent-stream.ts around finalizeCompletedRelay is worth a follow-up hardening pass.

This is a comment left during a code review.
Path: src/agents/acp-spawn-parent-stream.ts
Line: 336-355

Comment:
**Potential double-completion via late lifecycle events**

`finalizeCompletedRelay` calls `completeSubagentRun` with `sendFarewell: true` and then disposes the relay (removing the `onAgentEvent` listener). If a late lifecycle `phase === "end"` event subsequently arrives through the gateway's own event system (not through this relay), the normal registry path will call `completeSubagentRun` a second time with a fresh `Date.now()` timestamp. Because `endedAt` will differ from the first call, `mutated` will be `true` and a second farewell message may be emitted.

In practice this only triggers if the child completes very close to the stall threshold *and* the lifecycle event was in-flight during the probe window — narrow but possible. Adding an early-exit guard in `completeSubagentRun` when `entry.endedAt` is already set (or checking `entry.endedReason`) would fully close this window.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/acp-spawn-parent-stream.ts
Line: 304-324

Comment:
**Silent catch discards diagnostic context**

The bare `catch {}` on the `callGateway("agent.wait")` call means any transient error (timeout, deserialization failure, network blip) is silently swallowed. Since `probeForCompletedRelay` is the gate that determines whether the stall/timeout warning fires, a hidden error here could cause false stall messages to re-appear in ways that are difficult to diagnose.

Consider adding a debug-level log:
```ts
} catch (err) {
  log.debug("probeForCompletedRelay: agent.wait failed for run %s: %s", runId, err);
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: export completeSubagentRun, use spi..." | Re-trigger Greptile}

A bit more user-facing context on why this fix matters:

This was needed to make Claude Code usable reliably on my remote OpenClaw machine through the Telegram chat flow.

Before this change, streamTo:"parent" ACP runs could appear to stall in the parent session and, in the successful path, the final completion often never made it back to the chat even though the child run had actually finished.

In practice that meant I could kick off Claude Code remotely from Telegram, see partial progress, and then either get a misleading no-output warning or never receive the final result in the parent conversation.

This PR fixes that relay/completion path so remote Claude Code runs report back to the Telegram parent session properly.

Re-checked against current origin/main on 2026-03-29. This branch still contains useful diagnosis, but as a merge candidate it is now too bundled.

The remaining live seams appear to be split across: ACP lifecycle semantics (#40885), persisted/cross-process parent relay fallback (#54701), and possibly a smaller ACP run-registration gap. I would not merge this as one broad package against current main without first carving it into current-state-focused pieces.

Re-checked against current origin/main on 2026-03-29. This branch still contains useful diagnosis, but as a merge candidate it is now too bundled.

The remaining live seams appear to be split across ACP lifecycle semantics (#40885), persisted/cross-process parent relay fallback (#54701), and possibly a smaller ACP run-registration gap. I would not merge this as one broad package against current main without first carving it into current-state-focused pieces.

Closing as superseded. This branch was useful diagnosis, but it is now too bundled against current main. The remaining live pieces have been split into narrower current-state fixes: ACP lifecycle/registration in #40885 and persisted/cross-process parent relay fallback in #54701. Keeping this open now adds review noise and overlap rather than a clean merge path. Thanks for the investigation and for narrowing the failure mode.