Skip to content

fix: don't treat SubagentStop as the agent turn boundary#79

Merged
thejustinwalsh merged 1 commit into
mainfrom
fix/subagent-stop
May 23, 2026
Merged

fix: don't treat SubagentStop as the agent turn boundary#79
thejustinwalsh merged 1 commit into
mainfrom
fix/subagent-stop

Conversation

@thejustinwalsh

@thejustinwalsh thejustinwalsh commented May 23, 2026

Copy link
Copy Markdown
Owner

Summary

A dispatched agent that spawns a subagent (e.g. the implementing-github-issues skill dispatching an Explore agent for codebase research — which it explicitly recommends) was being torn down the moment that subagent finished.

SubagentStop was normalized to the same agent.stopped event as the main Stop, and awaitStop resolves on the first agent.stopped it sees. So when the Explore subagent returned, the dispatcher mistook it for the main agent's turn boundary → classifyStop found no done/blocked sentinel → bare-stopcompleted → killed the session and destroyed the worktree, mid-research, before any plan or PR.

Observed live dispatching Epic #21: the agent spawned an Explore subagent as its first move, and ~140s later its SubagentStop ended the whole dispatch.

Fix

  • New observational normalized event agent.subagent-stopped (packages/core/src/events.ts).
  • SubagentStopagent.subagent-stopped instead of agent.stopped (packages/adapters/claude/src/hooks.ts). It is still recorded for observability but never resolves the workflow's stop awaiter.
  • Build-spec "Normalized event taxonomy" corrected to match.

Test

  • New regression test: an agent.subagent-stopped POST does not resolve awaitStop; a subsequent real agent.stopped does.
  • Adapter hook-map test updated.
  • Full suite: 178 pass, 0 fail; typecheck clean.

Follow-up (not in this PR)

bare-stop → completed is still fragile even with this fix — a main agent that ends a turn mid-work would be declared done. The durable cure (a positive done-signal via the Phase 4 PR-ready gate, plus a continuation nudge) is tracked under #27.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed premature workflow shutdown when using subagents. Subagent completions are now correctly handled separately from main workflow stops, allowing subagents to complete without interrupting the overall workflow.

Review Change Stack

…d a dispatch

A spawned subagent (e.g. an Explore agent the implementer skill dispatches)
finishing is not the main agent's turn boundary. SubagentStop was normalized to
agent.stopped, so the first subagent's completion resolved the workflow's
awaitStop, classified a bare-stop, and tore the dispatch down mid-research.
Map SubagentStop to a new observational agent.subagent-stopped event that is
recorded but never resolves the stop awaiter.
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 89d973a2-d0d8-4ea9-b87a-72b3f1756042

📥 Commits

Reviewing files that changed from the base of the PR and between d73a086 and 0cdd56c.

📒 Files selected for processing (5)
  • packages/adapters/claude/src/hooks.ts
  • packages/adapters/claude/test/adapter.test.ts
  • packages/core/src/events.ts
  • packages/dispatcher/test/hook-server.test.ts
  • planning/middle-management-build-spec.md

📝 Walkthrough

Walkthrough

This PR introduces a new agent.subagent-stopped normalized event to distinguish subagent completion from main agent termination. The core event type is added to the allowlist, mapped through the Claude adapter, validated with a regression test confirming it does not resolve the main stop awaiter, and documented in the build specification.

Changes

Subagent-stopped event isolation

Layer / File(s) Summary
Event taxonomy definition
packages/core/src/events.ts
NormalizedEvent union and NORMALIZED_EVENTS allowlist both extended to include "agent.subagent-stopped" as a valid vocabulary member.
Claude adapter normalization
packages/adapters/claude/src/hooks.ts, packages/adapters/claude/test/adapter.test.ts
CLAUDE_EVENT_MAP maps SubagentStop to "agent.subagent-stopped" instead of "agent.stopped"; documentation comment clarifies this is a subagent turn boundary that does not resolve the stop awaiter; test assertion updated to match.
Stop awaiter behavior and spec
packages/dispatcher/test/hook-server.test.ts, planning/middle-management-build-spec.md
Regression test verifies awaitStop is not satisfied by agent.subagent-stopped but is satisfied by agent.stopped; build spec updated to explain the distinction between main-agent and subagent stop events to prevent premature workflow teardown.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • thejustinwalsh/middle#75: Extends the same hook-event normalization plumbing in Claude adapter and core event taxonomy that this PR builds upon.
  • thejustinwalsh/middle#73: Introduces the original Claude hook and HookServer.awaitStop semantics for main-agent stops that this PR now extends with subagent-specific event handling.

Suggested labels

ready-for-review

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: preventing SubagentStop events from being treated as the agent turn boundary, which is the core problem addressed across all file changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@thejustinwalsh thejustinwalsh merged commit df5363a into main May 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant