Greptile Summary

This PR adds a registerSubagentRun() call in acp-spawn.ts after a successful ACP dispatch, mirroring the existing pattern in subagent-spawn.ts. This correctly makes ACP sessions visible to the subagent lifecycle/registry system so that subagent_ended hooks fire, completion notifications reach the requester, and zombie session accumulation is prevented.

• The approach is correct and well-motivated — the mirrored pattern from subagent-spawn.ts is the right model.
• The registerSubagentRun call is missing a try/catch guard. In subagent-spawn.ts, a throw from this function is caught and results in a graceful error return. Here, a throw would propagate out of spawnAcpDirect after the ACP session is already live, causing the caller to see a failure while the session is running — recreating orphaned sessions via a different path.
• resolveMainSessionAlias(cfg) is called a second time to obtain mainKey/alias for the display key. These values are already derived inside resolveRequesterInternalSessionKey at the top of the function; surfacing them to the outer scope would eliminate the duplicate call.

Confidence Score: 3/5

• Functionally safe to merge with low risk, but the missing try/catch creates a narrow path to post-dispatch exception propagation that should be addressed.
• The change is well-scoped and the registration parameters are correct. The main concern is the absence of a try/catch around registerSubagentRun: since persistSubagentRunsToDisk already swallows its errors, the practical risk is low, but a thrown exception from loadConfig() or ensureListener() inside the registry call would still propagate out of spawnAcpDirect after a successful dispatch — the exact failure mode the PR is trying to prevent. The existing pattern in subagent-spawn.ts guards against this explicitly.
• Pay attention to src/agents/acp-spawn.ts around the new registerSubagentRun block (lines 746–757) — specifically the missing error guard.

This is a comment left during a code review.
Path: src/agents/acp-spawn.ts
Line: 740-745

Comment:
**Redundant `resolveMainSessionAlias` call**

`resolveMainSessionAlias(cfg)` is called here to obtain `mainKey` and `alias`, but these values are already derived at the very top of `spawnAcpDirect` inside `resolveRequesterInternalSessionKey` (line 413–416), which calls `resolveMainSessionAlias` internally. They are just not surfaced to the outer scope.

Consider either returning `mainKey`/`alias` from `resolveRequesterInternalSessionKey`, or calling `resolveMainSessionAlias(cfg)` once at the top of `spawnAcpDirect` (before calling `resolveRequesterInternalSessionKey`) so the values can be shared — matching the pattern in `subagent-spawn.ts` at line 319.

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.ts
Line: 746-757

Comment:
**Missing try/catch around `registerSubagentRun`**

The registration call is not wrapped in error handling. In `subagent-spawn.ts` (the pattern being mirrored at line 692–735), `registerSubagentRun` is wrapped in a `try/catch` that catches any thrown error and returns `{ status: "error", error: "Failed to register subagent run: ..." }` — preventing an unhandled exception from escaping the spawner function.

Without the same protection here, if `registerSubagentRun` throws (e.g. if `loadConfig()` fails inside it, or `ensureListener()` misbehaves), the exception will propagate out of `spawnAcpDirect` **even though the ACP session was already successfully dispatched** at lines 703–720. The caller would see an error response while the ACP session is actually live — creating exactly the class of orphaned/zombie sessions this PR aims to eliminate, just via a different code path.

Since the dispatch already succeeded, the right behavior on registration failure is to log a warning and continue returning the accepted response — not to throw:

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

_{Last reviewed commit: 5c35c02}

Thanks for the thorough review! Both issues addressed in f89aee6:

1. try/catch added: registerSubagentRun is now wrapped in a try/catch. On failure, it logs a warning and continues returning the accepted response -- the ACP session is already live, so surfacing an error would create exactly the orphan scenario we are fixing.

2. Deduplicated resolveMainSessionAlias: Refactored resolveRequesterInternalSessionKey to return { key, mainKey, alias } instead of just the key string, so the caller can reuse mainKey/alias without a second call.

Closing as duplicate of the ACP run-registration path now consolidated under #53125.

This PR identified the same missing registerSubagentRun() seam in acp-spawn.ts. I am keeping the broader canonical open and folding duplicate discussion there so contributor credit stays attached to the surviving thread.

Closing as duplicate of the ACP run-registration path now consolidated under #53125.

This PR identified the same missing registerSubagentRun() seam in acp-spawn.ts. I am keeping the broader canonical open and folding duplicate discussion there so contributor credit stays attached to the surviving thread.