MUL-2766: fix(squad): wake parent on child in_review, not just done

The runtime brief tells child agents to move their issue to `in_review`
when finished (the leader/human promotes to `done` after review). The
child-done notification was hardcoded to fire only on the `done`
transition, so squads were getting stuck: leader assigned work out,
child agents completed and moved to in_review, parent leader was never
woken.

Widen the trigger to fire on the transition into either `in_review` or
`done` (autopilot already treats them as a single completion signal).
Reuse the existing idempotency rule: once the parent has been told the
child is complete, the in_review ↔ done step is a no-op; a real reopen
(in_progress) followed by a fresh completion fires again.

Use different wording per status — in_review asks the leader to inspect
and accept/send back, done carries the existing promote-backlog-sibling
guidance.

Co-authored-by: multica-agent <github@multica.ai>

server/internal/daemon/execenv/runtime_config.go

@@ -388,7 +388,8 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
 	// Sub-issue Protocol (PR #2918) that still belongs in the brief. The
 	// parent-notification guidance was dropped in MUL-2538: the platform
 	// now posts a system comment on the parent itself when a child enters
-	// `done`, and the agent has nothing to do or avoid on that path.
+	// a completed state (`in_review` after MUL-2766, or `done`), and the
+	// agent has nothing to do or avoid on that path.
 	// Section is skipped for chat, quick-create, and run-only autopilot
 	// runs (no parent/child semantics there).
 	if ctx.IssueID != "" && ctx.ChatSessionID == "" && ctx.QuickCreatePrompt == "" && ctx.AutopilotRunID == "" {

server/internal/handler/issue_child_done.go

@@ -13,16 +13,25 @@ import (
 )
 
 // notifyParentOfChildDone posts a top-level system comment on the parent
-// issue when a child issue transitions from non-done into done. This replaces
-// the agent-prompt rule that previously made child agents post the
-// notification themselves (PR #2918 user feedback — the agent rule caused
-// self-mention loops, planner ping-pong, and accidental `MUL-` prefix
-// hardcoding because the agent did not always know the workspace prefix).
+// issue when a child issue transitions from a non-completed state into a
+// completed one. A child is "completed" once it reaches `in_review` (the
+// child agent has finished its work and is waiting for the parent's review)
+// or `done` (the work is accepted). Both transitions fire the notification;
+// the in_review case was MUL-2766 — squads were getting stuck because the
+// child agent followed the runtime workflow (`in_review` when done, leader
+// promotes to `done` after review) and the parent never woke up. The
+// `in_review` ↔ `done` transition itself is a no-op (the parent has already
+// been informed). This replaces the agent-prompt rule that previously made
+// child agents post the notification themselves (PR #2918 user feedback —
+// the agent rule caused self-mention loops, planner ping-pong, and
+// accidental `MUL-` prefix hardcoding because the agent did not always know
+// the workspace prefix).
 //
 // Guards on whether the comment fires at all:
-//   - prev.Status must not already be "done" (idempotent — repeat saves of
-//     done do not re-fire; only the transition fires)
-//   - issue.Status must be "done"
+//   - prev.Status must not already be a completed state (idempotent — once
+//     the parent has been told the child is in_review, moving it to done
+//     does not re-fire, and vice versa)
+//   - issue.Status must be a completed state (`in_review` or `done`)
 //   - issue.ParentIssueID must be set
 //   - parent must not be "done" or "cancelled" — the parent is already
 //     closed and a notification has no follow-up to drive
@@ -52,7 +61,7 @@ func (h *Handler) notifyParentOfChildDone(ctx context.Context, prev, issue db.Is
 	if !issue.ParentIssueID.Valid {
 		return
 	}
-	if prev.Status == "done" || issue.Status != "done" {
+	if isChildCompletedStatus(prev.Status) || !isChildCompletedStatus(issue.Status) {
 		return
 	}
 	parent, err := h.Queries.GetIssue(ctx, issue.ParentIssueID)
@@ -83,10 +92,11 @@ func (h *Handler) notifyParentOfChildDone(ctx context.Context, prev, issue db.Is
 	// agent the workspace lost track of, etc.).
 	mentionPrefix := h.buildParentAssigneeMention(ctx, parent)
 
-	content := fmt.Sprintf(
-		"%sSub-issue [%s](mention://issue/%s) — \"%s\" — is done. Before promoting any waiting `backlog` sub-issue, read each sibling's description and only promote items whose stated dependencies are already satisfied — do not rely on this parent's higher-level breakdown alone. If a sibling's description conflicts with that breakdown (e.g. it lists a prerequisite the parent treats as parallel), do NOT change its status — leave it `backlog` and post a comment to confirm first.",
-		mentionPrefix, identifier, childID, title,
-	)
+	content := childCompletedSystemCommentContent(issue.Status, mentionPrefix, identifier, childID, title)
+	if content == "" {
+		// Defensive: isChildCompletedStatus already filtered the status above.
+		return
+	}
 
 	// author_type='system', author_id=zero UUID. The zero UUID is a valid 16
 	// byte value and the column is NOT NULL; frontend code should branch on
@@ -125,6 +135,48 @@ func (h *Handler) notifyParentOfChildDone(ctx context.Context, prev, issue db.Is
 	h.dispatchParentAssigneeTrigger(ctx, parent, issue, comment)
 }
 
+// isChildCompletedStatus reports whether a child issue status represents
+// "work handed back to the parent". Both `in_review` (waiting for leader
+// review) and `done` (accepted) count: the parent's assignee should be
+// woken on the transition INTO either, and transitions between them
+// (in_review → done, done → in_review) are no-ops. See MUL-2766 for the
+// in_review case; autopilot already treats these two statuses as a single
+// completion signal (see service/autopilot.go), and this helper aligns the
+// child-done notification with that convention.
+func isChildCompletedStatus(status string) bool {
+	return status == "in_review" || status == "done"
+}
+
+// childCompletedSystemCommentContent renders the system-comment body for a
+// child-completed notification. The wording differs by status because the
+// follow-up the parent assignee should take is different:
+//
+//   - `in_review` is a coordination signal — the leader must inspect the
+//     work and either accept it (move child to `done`) or send it back
+//     (move child to `in_progress` with comment-described changes).
+//     Promoting backlog siblings is premature until the review lands.
+//   - `done` is a completion signal — the work is accepted, so the
+//     follow-up shifts to advancing the plan: promoting the next ready
+//     backlog sibling. The MUL-2538 promotion guardrails apply.
+//
+// Returns "" when the status is not a recognised completed state; callers
+// should treat that as "do not post".
+func childCompletedSystemCommentContent(status, mentionPrefix, identifier, childID, title string) string {
+	switch status {
+	case "in_review":
+		return fmt.Sprintf(
+			"%sSub-issue [%s](mention://issue/%s) — \"%s\" — is ready for review. Inspect the work the child reported, then either accept it by moving the child to `done`, or send it back by moving the child to `in_progress` with a comment describing the requested changes. Do NOT promote any waiting `backlog` sub-issue until the review lands.",
+			mentionPrefix, identifier, childID, title,
+		)
+	case "done":
+		return fmt.Sprintf(
+			"%sSub-issue [%s](mention://issue/%s) — \"%s\" — is done. Before promoting any waiting `backlog` sub-issue, read each sibling's description and only promote items whose stated dependencies are already satisfied — do not rely on this parent's higher-level breakdown alone. If a sibling's description conflicts with that breakdown (e.g. it lists a prerequisite the parent treats as parallel), do NOT change its status — leave it `backlog` and post a comment to confirm first.",
+			mentionPrefix, identifier, childID, title,
+		)
+	}
+	return ""
+}
+
 // sanitizeChildTitleForSystemComment removes mention-style markdown from a
 // child issue's title before it is embedded into the parent's system
 // comment. Smuggled mentions are already harmless on the listener path

server/internal/handler/issue_child_done_test.go

@@ -154,6 +154,80 @@ func TestChildDoneNotifiesParent(t *testing.T) {
 	}
 }
 
+// TestChildInReviewNotifiesParent — MUL-2766. The runtime workflow tells
+// child agents to move their issue to `in_review` when they finish (the
+// human/leader then promotes to `done`). Before the fix, only the
+// `done` transition fired the parent notification, so squads were getting
+// stuck: leader assigned work out, child agents moved to in_review, parent
+// leader was never woken. This test asserts that the in_review transition
+// produces exactly one top-level system comment on the parent and that the
+// body uses the in_review-flavoured wording (review-action verb, not the
+// done-flavoured "Before promoting any waiting backlog sub-issue" clause).
+func TestChildInReviewNotifiesParent(t *testing.T) {
+	fx := newChildDoneFixture(t, "in_progress")
+
+	updateChildStatus(t, fx.child.ID, "in_review")
+
+	if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 {
+		t.Fatalf("expected exactly 1 system comment on parent after in_review, got %d", got)
+	}
+	content, _, parentNull, typeStr := systemCommentOn(t, fx.parent.ID)
+	if !parentNull {
+		t.Errorf("system comment must be top-level (parent_id IS NULL)")
+	}
+	if typeStr != "system" {
+		t.Errorf("system comment type should be 'system', got %q", typeStr)
+	}
+	if !strings.Contains(content, "is ready for review") {
+		t.Errorf("expected in_review-flavoured wording, got: %s", content)
+	}
+	if strings.Contains(content, "Before promoting any waiting `backlog`") {
+		t.Errorf("in_review comment should NOT carry the done-flavoured promote-backlog clause, got: %s", content)
+	}
+	if !strings.Contains(content, fx.child.Identifier) {
+		t.Errorf("expected comment to contain child identifier %q, got: %s", fx.child.Identifier, content)
+	}
+	if !strings.Contains(content, "mention://issue/"+fx.child.ID) {
+		t.Errorf("expected mention://issue/<child-id> link in comment, got: %s", content)
+	}
+}
+
+// TestChildInReviewThenDoneDoesNotRefire — the parent has already been
+// woken on in_review; promoting the child from in_review to done is the
+// leader accepting the work, not a new completion event, and must not
+// produce a second system comment.
+func TestChildInReviewThenDoneDoesNotRefire(t *testing.T) {
+	fx := newChildDoneFixture(t, "in_progress")
+
+	updateChildStatus(t, fx.child.ID, "in_review")
+	if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 {
+		t.Fatalf("after in_review: expected 1 comment, got %d", got)
+	}
+
+	updateChildStatus(t, fx.child.ID, "done")
+	if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 {
+		t.Fatalf("after in_review→done: expected still 1 comment (no re-fire), got %d", got)
+	}
+}
+
+// TestChildDoneAfterInReviewSendBackFiresAgain — the leader-driven
+// "send back for changes" path: child reaches in_review (fires once),
+// leader sends it back by moving to in_progress, child completes again
+// and lands in_review (or done). The second completion is a real new
+// event and must fire a second notification — without this, the leader
+// would never be woken for the re-review.
+func TestChildDoneAfterInReviewSendBackFiresAgain(t *testing.T) {
+	fx := newChildDoneFixture(t, "in_progress")
+
+	updateChildStatus(t, fx.child.ID, "in_review")
+	updateChildStatus(t, fx.child.ID, "in_progress")
+	updateChildStatus(t, fx.child.ID, "in_review")
+
+	if got := countSystemCommentsOn(t, fx.parent.ID); got != 2 {
+		t.Fatalf("expected 2 system comments after send-back + re-review cycle, got %d", got)
+	}
+}
+
 // TestChildDoneNotificationIsIdempotent — re-saving an already-done child
 // must NOT fire a second notification. UpdateIssue is called with the same
 // status='done' twice; only the first call is a transition and should