From ae0dbad4f066c849593a107e11ff945ca0ba7a84 Mon Sep 17 00:00:00 2001 From: ZeroIce Date: Sat, 18 Jul 2026 22:20:48 +0800 Subject: [PATCH] Fix sub-issue attention escalation Co-authored-by: multica-agent --- server/internal/handler/issue.go | 15 +- server/internal/handler/issue_batch_test.go | 20 ++ server/internal/handler/issue_child_done.go | 196 ++++++++++++++++++ .../internal/handler/issue_child_done_test.go | 90 ++++++++ 4 files changed, 316 insertions(+), 5 deletions(-) diff --git a/server/internal/handler/issue.go b/server/internal/handler/issue.go index 82ca84338fc..4a39495301d 100644 --- a/server/internal/handler/issue.go +++ b/server/internal/handler/issue.go @@ -2958,6 +2958,7 @@ func (h *Handler) UpdateIssue(w http.ResponseWriter, r *http.Request) { // fails best-effort. if statusChanged { h.notifyParentOfChildDone(r.Context(), prevIssue, issue) + h.notifyParentOfChildAttention(r.Context(), prevIssue, issue, actorType, actorID) } writeJSON(w, http.StatusOK, resp) @@ -3256,6 +3257,8 @@ func (h *Handler) BatchUpdateIssues(w http.ResponseWriter, r *http.Request) { // the parent/stage notification is evaluated once against the final state // after the loop (MUL-4155) rather than per-child mid-batch. var childDoneCompleted []db.Issue + var childAttentionNeeded []db.Issue + batchActorType, batchActorID := h.resolveActor(r, userID, workspaceID) for _, issueID := range req.IssueIDs { issueUUID, err := util.ParseUUID(issueID) if err != nil { @@ -3415,15 +3418,13 @@ func (h *Handler) BatchUpdateIssues(w http.ResponseWriter, r *http.Request) { prefix := h.getIssuePrefix(r.Context(), issue.WorkspaceID) resp := issueToResponse(issue, prefix) - actorType, actorID := h.resolveActor(r, userID, workspaceID) - assigneeChanged := (req.Updates.AssigneeType != nil || req.Updates.AssigneeID != nil) && (prevIssue.AssigneeType.String != issue.AssigneeType.String || uuidToString(prevIssue.AssigneeID) != uuidToString(issue.AssigneeID)) statusChanged := req.Updates.Status != nil && prevIssue.Status != issue.Status priorityChanged := req.Updates.Priority != nil && prevIssue.Priority != issue.Priority projectChanged := req.Updates.ProjectID != nil && uuidToString(prevIssue.ProjectID) != uuidToString(issue.ProjectID) - h.publish(protocol.EventIssueUpdated, workspaceID, actorType, actorID, map[string]any{ + h.publish(protocol.EventIssueUpdated, workspaceID, batchActorType, batchActorID, map[string]any{ "issue": resp, "assignee_changed": assigneeChanged, "status_changed": statusChanged, @@ -3444,9 +3445,9 @@ func (h *Handler) BatchUpdateIssues(w http.ResponseWriter, r *http.Request) { AssigneeChanged: assigneeChanged, StatusChanged: statusChanged, }, - h.issueTriggerWriteProbe(r, actorType, issue), + h.issueTriggerWriteProbe(r, batchActorType, issue), ); ok && !req.Updates.SuppressRun { - h.dispatchIssueRun(r.Context(), issue, trigger, actorType, actorID, req.Updates.HandoffNote) + h.dispatchIssueRun(r.Context(), issue, trigger, batchActorType, batchActorID, req.Updates.HandoffNote) } // No status change — not even → cancelled — cancels active tasks here, @@ -3464,6 +3465,9 @@ func (h *Handler) BatchUpdateIssues(w http.ResponseWriter, r *http.Request) { !isTerminalChildStatus(prevIssue.Status) && isTerminalChildStatus(issue.Status) { childDoneCompleted = append(childDoneCompleted, issue) } + if statusChanged && issue.ParentIssueID.Valid && childNeedsParentAttention(prevIssue.Status, issue.Status) { + childAttentionNeeded = append(childAttentionNeeded, issue) + } updated++ } @@ -3473,6 +3477,7 @@ func (h *Handler) BatchUpdateIssues(w http.ResponseWriter, r *http.Request) { // of issue_ids order (MUL-4155). Best-effort; failure does not abort the // batch. Single-issue UpdateIssue is unchanged and still notifies inline. h.notifyParentsOfBatchChildDone(r.Context(), childDoneCompleted) + h.notifyParentsOfBatchChildAttention(r.Context(), childAttentionNeeded, batchActorType, batchActorID) slog.Info("batch update issues", append(logger.RequestAttrs(r), "count", updated)...) writeJSON(w, http.StatusOK, map[string]any{"updated": updated}) diff --git a/server/internal/handler/issue_batch_test.go b/server/internal/handler/issue_batch_test.go index 8725ab58f04..72146809bc6 100644 --- a/server/internal/handler/issue_batch_test.go +++ b/server/internal/handler/issue_batch_test.go @@ -383,6 +383,26 @@ func TestBatchChildDoneCrossStage_Cancelled(t *testing.T) { } } +func TestBatchChildAttention_OneCommentOneWake(t *testing.T) { + fx := newStagedBatchFixture(t) + + batchSetStatus(t, []string{fx.stage1[0].ID, fx.stage1[1].ID}, "blocked") + + if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 { + t.Fatalf("expected exactly 1 attention comment on parent, got %d", got) + } + content, _, _, _ := systemCommentOn(t, fx.parent.ID) + if !strings.Contains(content, "needs attention after a batch update") { + t.Errorf("expected batch attention handoff, got: %s", content) + } + if !strings.Contains(content, "is blocked") { + t.Errorf("expected blocked status in handoff, got: %s", content) + } + if got := countPendingTasksForAgent(t, fx.parent.ID, fx.agentID); got != 1 { + t.Fatalf("expected exactly 1 pending parent task, got %d", got) + } +} + // TestBatchChildDoneClosesLowerStageOnly — when a batch finishes only the lower // stage (a later stage still has open children), the parent must be told Stage 1 // is complete AND accurately pointed at Stage 2 as next. Guards against diff --git a/server/internal/handler/issue_child_done.go b/server/internal/handler/issue_child_done.go index b9f0be80c9a..fd2c4b4c5ce 100644 --- a/server/internal/handler/issue_child_done.go +++ b/server/internal/handler/issue_child_done.go @@ -2,6 +2,7 @@ package handler import ( "context" + "encoding/json" "fmt" "log/slog" "sort" @@ -135,6 +136,29 @@ func (h *Handler) notifyParentOfChildDone(ctx context.Context, prev, issue db.Is h.postChildDoneComment(ctx, parent, issue, children, staged, closedStage, false) } +// notifyParentOfChildAttention posts a parent-level handoff when a child moves +// into a state that needs intervention but is not terminal. Child completion is +// handled separately by notifyParentOfChildDone because it has stage-barrier +// semantics; review/block states must surface immediately or the parent can +// stall with no coordinator wake. +func (h *Handler) notifyParentOfChildAttention(ctx context.Context, prev, issue db.Issue, actorType, actorID string) { + if !issue.ParentIssueID.Valid || !childNeedsParentAttention(prev.Status, issue.Status) { + return + } + parent, err := h.Queries.GetIssue(ctx, issue.ParentIssueID) + if err != nil { + slog.Warn("child attention: failed to load parent", + "error", err, + "child_id", uuidToString(issue.ID), + "parent_id", uuidToString(issue.ParentIssueID)) + return + } + if parent.Status == "done" || parent.Status == "cancelled" || parent.Status == "backlog" { + return + } + h.postChildAttentionComment(ctx, parent, issue, false, actorType, actorID) +} + // notifyParentsOfBatchChildDone emits child-done parent notifications for a // whole batch AFTER every status write has committed. `completed` is the set of // children that transitioned non-terminal -> terminal during the batch. @@ -245,6 +269,51 @@ func (h *Handler) notifyParentsOfBatchChildDone(ctx context.Context, completed [ } } +// notifyParentsOfBatchChildAttention emits at most one review/block handoff per +// parent after a batch update has committed. If several children need attention, +// blocked wins over in_review because it is the stronger escalation signal. +func (h *Handler) notifyParentsOfBatchChildAttention(ctx context.Context, children []db.Issue, actorType, actorID string) { + if len(children) == 0 { + return + } + + type parentGroup struct { + parentID pgtype.UUID + child db.Issue + } + var groups []*parentGroup + index := map[string]*parentGroup{} + for _, c := range children { + if !c.ParentIssueID.Valid { + continue + } + key := uuidToString(c.ParentIssueID) + g, ok := index[key] + if !ok { + g = &parentGroup{parentID: c.ParentIssueID, child: c} + index[key] = g + groups = append(groups, g) + continue + } + if g.child.Status != "blocked" && c.Status == "blocked" { + g.child = c + } + } + + for _, g := range groups { + parent, err := h.Queries.GetIssue(ctx, g.parentID) + if err != nil { + slog.Warn("batch child attention: failed to load parent", + "error", err, "parent_id", uuidToString(g.parentID)) + continue + } + if parent.Status == "done" || parent.Status == "cancelled" || parent.Status == "backlog" { + continue + } + h.postChildAttentionComment(ctx, parent, g.child, true, actorType, actorID) + } +} + // postChildDoneComment builds and posts the parent's child-done system comment // for a closed stage barrier, then dispatches the parent-assignee trigger. It // assumes every guard in notifyParentOfChildDone / notifyParentsOfBatchChildDone @@ -334,6 +403,63 @@ func (h *Handler) postChildDoneComment(ctx context.Context, parent, completed db h.dispatchParentAssigneeTrigger(ctx, parent, comment) } +// postChildAttentionComment records a parent-level handoff for blocked or +// review-needed child work, then routes the handoff to the parent owner. +func (h *Handler) postChildAttentionComment(ctx context.Context, parent, child db.Issue, batch bool, actorType, actorID string) { + prefix := h.getIssuePrefix(ctx, child.WorkspaceID) + identifier := prefix + "-" + strconv.Itoa(int(child.Number)) + childID := uuidToString(child.ID) + title := sanitizeChildTitleForSystemComment(child.Title) + mentionPrefix := h.buildParentAssigneeMention(ctx, parent) + + stateLabel := statusLabelForChildAttention(child.Status) + action := "Review the child and decide whether to accept it, request follow-up, or continue the parent." + if child.Status == "blocked" { + action = "Review the child, unblock it, or decide who should continue the parent." + } + + var content string + if batch { + content = fmt.Sprintf( + "%sA sub-issue needs attention after a batch update — [%s](mention://issue/%s) — \"%s\" is %s. %s", + mentionPrefix, identifier, childID, title, stateLabel, action, + ) + } else { + content = fmt.Sprintf( + "%sA sub-issue needs attention — [%s](mention://issue/%s) — \"%s\" is %s. %s", + mentionPrefix, identifier, childID, title, stateLabel, action, + ) + } + + comment, err := h.Queries.CreateComment(ctx, db.CreateCommentParams{ + IssueID: parent.ID, + WorkspaceID: parent.WorkspaceID, + AuthorType: "system", + AuthorID: pgtype.UUID{Valid: true}, + Content: content, + Type: "system", + ParentID: pgtype.UUID{Valid: false}, + }) + if err != nil { + slog.Warn("child attention: create system comment failed", + "error", err, + "child_id", childID, + "parent_id", uuidToString(parent.ID)) + return + } + + h.publish(protocol.EventCommentCreated, uuidToString(parent.WorkspaceID), "system", "", map[string]any{ + "comment": commentToResponse(comment, nil, nil), + "issue_title": parent.Title, + "issue_assignee_type": textToPtr(parent.AssigneeType), + "issue_assignee_id": uuidToPtr(parent.AssigneeID), + "issue_status": parent.Status, + }) + + h.dispatchParentAssigneeTrigger(ctx, parent, comment) + h.notifyParentMemberOfChildAttention(ctx, parent, child, comment, actorType, actorID) +} + // isTerminalChildStatus reports whether a child issue status counts as // "finished" for stage-barrier purposes. Cancelled counts as terminal: a // cancelled sibling will never complete, so it must not hold a stage open. @@ -341,6 +467,25 @@ func isTerminalChildStatus(status string) bool { return status == "done" || status == "cancelled" } +func isChildAttentionStatus(status string) bool { + return status == "in_review" || status == "blocked" +} + +func childNeedsParentAttention(prevStatus, nextStatus string) bool { + return prevStatus != nextStatus && isChildAttentionStatus(nextStatus) +} + +func statusLabelForChildAttention(status string) string { + switch status { + case "in_review": + return "in review" + case "blocked": + return "blocked" + default: + return status + } +} + // siblingsAreStaged reports whether any child in the set carries an explicit // stage. A set with no stages is treated as a single implicit stage. func siblingsAreStaged(children []db.Issue) bool { @@ -596,6 +741,57 @@ func (h *Handler) dispatchParentAssigneeTrigger(ctx context.Context, parent db.I } } +func (h *Handler) notifyParentMemberOfChildAttention(ctx context.Context, parent, child db.Issue, systemComment db.Comment, actorType, actorID string) { + if !parent.AssigneeType.Valid || parent.AssigneeType.String != "member" || !parent.AssigneeID.Valid { + return + } + parentMemberID := uuidToString(parent.AssigneeID) + if actorType == "member" && actorID == parentMemberID { + return + } + + details, _ := json.Marshal(map[string]string{ + "parent_issue_id": uuidToString(parent.ID), + "child_issue_id": uuidToString(child.ID), + "child_status": child.Status, + "system_comment_id": uuidToString(systemComment.ID), + }) + item, err := h.Queries.CreateInboxItem(ctx, db.CreateInboxItemParams{ + WorkspaceID: parent.WorkspaceID, + RecipientType: "member", + RecipientID: parent.AssigneeID, + Type: "status_changed", + Severity: "action_required", + IssueID: child.ID, + Title: child.Title, + Body: strToText("A sub-issue under an issue you own needs attention."), + ActorType: strToText(actorType), + ActorID: optionalActorUUID(actorID), + Details: details, + }) + if err != nil { + slog.Warn("child attention: create parent member inbox failed", + "error", err, + "child_id", uuidToString(child.ID), + "parent_id", uuidToString(parent.ID), + "member_id", parentMemberID) + return + } + + resp := inboxToResponse(item) + resp.IssueStatus = &child.Status + h.publish(protocol.EventInboxNew, uuidToString(parent.WorkspaceID), actorType, actorID, map[string]any{ + "item": resp, + }) +} + +func optionalActorUUID(actorID string) pgtype.UUID { + if actorID == "" { + return pgtype.UUID{Valid: false} + } + return parseUUID(actorID) +} + // triggerChildDoneAgent enqueues a mention-style task for the parent's // agent assignee. // diff --git a/server/internal/handler/issue_child_done_test.go b/server/internal/handler/issue_child_done_test.go index d3949964a92..123f1bd3b3f 100644 --- a/server/internal/handler/issue_child_done_test.go +++ b/server/internal/handler/issue_child_done_test.go @@ -378,6 +378,96 @@ func TestChildDoneSkippedWhenParentMember(t *testing.T) { } } +func TestChildAttentionInReviewWakesParentAgent(t *testing.T) { + fx := newChildDoneFixture(t, "in_progress") + + var agentID string + if err := testPool.QueryRow(context.Background(), + `SELECT id FROM agent WHERE workspace_id = $1 AND name = $2`, + testWorkspaceID, "Handler Test Agent", + ).Scan(&agentID); err != nil { + t.Fatalf("locate test agent: %v", err) + } + setIssueAssigneeDirect(t, fx.parent.ID, "agent", agentID) + t.Cleanup(func() { + testPool.Exec(context.Background(), + `DELETE FROM agent_task_queue WHERE issue_id = $1`, fx.parent.ID) + }) + + updateChildStatus(t, fx.child.ID, "in_review") + + content := parentSystemCommentContent(t, fx.parent.ID) + if !strings.Contains(content, "A sub-issue needs attention") { + t.Errorf("expected child attention handoff, got: %s", content) + } + if !strings.Contains(content, "is in review") { + t.Errorf("expected in-review status in handoff, got: %s", content) + } + if !strings.Contains(content, "mention://issue/"+fx.child.ID) { + t.Errorf("expected child issue mention in handoff, got: %s", content) + } + if !strings.Contains(content, "mention://agent/"+agentID) { + t.Errorf("expected parent agent mention in handoff, got: %s", content) + } + if got := countPendingTasksForAgent(t, fx.parent.ID, agentID); got != 1 { + t.Fatalf("expected 1 pending parent task for in-review child, got %d", got) + } + + updateChildStatus(t, fx.child.ID, "in_review") + if got := countSystemCommentsOn(t, fx.parent.ID); got != 1 { + t.Fatalf("same-status save must not duplicate attention comments, got %d", got) + } + if got := countPendingTasksForAgent(t, fx.parent.ID, agentID); got != 1 { + t.Fatalf("same-status save must not duplicate parent tasks, got %d", got) + } +} + +func TestChildAttentionBlockedNotifiesParentMember(t *testing.T) { + fx := newChildDoneFixture(t, "in_progress") + memberID := createPermissionTestMember(t, "child-attention-owner@multica.test") + + setIssueAssigneeDirect(t, fx.parent.ID, "member", memberID) + t.Cleanup(func() { + testPool.Exec(context.Background(), + `DELETE FROM inbox_item WHERE issue_id IN ($1, $2)`, fx.parent.ID, fx.child.ID) + }) + + updateChildStatus(t, fx.child.ID, "blocked") + + content := parentSystemCommentContent(t, fx.parent.ID) + if !strings.Contains(content, "is blocked") { + t.Errorf("expected blocked status in handoff, got: %s", content) + } + if strings.Contains(content, "mention://member/") { + t.Errorf("system handoff should not inject a member mention, got: %s", content) + } + + var notifType, severity, issueID, body string + if err := testPool.QueryRow(context.Background(), ` + SELECT type, severity, issue_id::text, body + FROM inbox_item + WHERE recipient_type = 'member' + AND recipient_id = $1 + AND issue_id = $2 + ORDER BY created_at DESC + LIMIT 1 + `, memberID, fx.child.ID).Scan(¬ifType, &severity, &issueID, &body); err != nil { + t.Fatalf("read parent member inbox: %v", err) + } + if notifType != "status_changed" { + t.Errorf("inbox type = %q, want status_changed", notifType) + } + if severity != "action_required" { + t.Errorf("inbox severity = %q, want action_required", severity) + } + if issueID != fx.child.ID { + t.Errorf("inbox should point to child issue %s, got %s", fx.child.ID, issueID) + } + if !strings.Contains(body, "needs attention") { + t.Errorf("expected action body, got %q", body) + } +} + // TestChildDoneMentionsParentAssignee_Squad verifies the squad branch: the // system comment carries a `mention://squad/` link and the squad // leader receives a leader-role task. Reuses the squad fixture helper from