mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-04 17:18:35 +02:00
This reverts commit 738217c275.
Re-firing an assigned agent when an Issue leaves `blocked` is not the
product behaviour we want: `blocked` is a human-attention state, and
clearing it is not by itself a signal to start work. Reverted on product
grounds ahead of the v0.4.18 release, not because of a defect.
Restores the pre-#6155 behaviour exactly: only `backlog` -> active
promotes a parked Issue into a run, and the write/preview trigger probes
go back to being separate.
Requested by Bohan on MUL-5716.
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -2974,7 +2974,7 @@ func (h *Handler) UpdateIssue(w http.ResponseWriter, r *http.Request) {
|
||||
AssigneeChanged: assigneeChanged,
|
||||
StatusChanged: statusChanged,
|
||||
},
|
||||
h.issueTriggerProbe(r, actorType, actorID, workspaceID, issue),
|
||||
h.issueTriggerWriteProbe(r, actorType, issue),
|
||||
); ok && !req.SuppressRun {
|
||||
h.dispatchIssueRun(r.Context(), issue, trigger, actorType, actorID, req.HandoffNote)
|
||||
}
|
||||
@@ -3067,8 +3067,8 @@ func (h *Handler) validateAssigneePair(ctx context.Context, r *http.Request, wor
|
||||
// shouldEnqueueAgentTask returns true when an issue creation or assignment
|
||||
// should trigger the assigned agent. Backlog issues are skipped — backlog
|
||||
// acts as a parking lot where issues can be pre-assigned without immediately
|
||||
// triggering execution. Moving out of backlog or blocked is handled separately
|
||||
// in UpdateIssue.
|
||||
// triggering execution. Moving out of backlog is handled separately in
|
||||
// UpdateIssue.
|
||||
func (h *Handler) shouldEnqueueAgentTask(ctx context.Context, issue db.Issue) bool {
|
||||
if issue.Status == "backlog" {
|
||||
return false
|
||||
@@ -3472,7 +3472,7 @@ func (h *Handler) BatchUpdateIssues(w http.ResponseWriter, r *http.Request) {
|
||||
AssigneeChanged: assigneeChanged,
|
||||
StatusChanged: statusChanged,
|
||||
},
|
||||
h.issueTriggerProbe(r, actorType, actorID, workspaceID, issue),
|
||||
h.issueTriggerWriteProbe(r, actorType, issue),
|
||||
); ok && !req.Updates.SuppressRun {
|
||||
h.dispatchIssueRun(r.Context(), issue, trigger, actorType, actorID, req.Updates.HandoffNote)
|
||||
}
|
||||
|
||||
@@ -16,10 +16,25 @@ import (
|
||||
// selection cannot fan out into thousands of readiness probes.
|
||||
const maxPreviewTriggerIssues = 500
|
||||
|
||||
// issueTriggerProbe builds the request-scoped gates shared by preview and the
|
||||
// real write paths. Status-only updates do not pass through validateAssigneePair,
|
||||
// so the invocation gate must remain part of the enqueue decision itself.
|
||||
func (h *Handler) issueTriggerProbe(r *http.Request, actorType, actorID, workspaceID string, issue db.Issue) service.IssueTriggerProbe {
|
||||
// issueTriggerWriteProbe builds the probe the write paths feed to
|
||||
// WillEnqueueRun. The private-agent gate is already enforced at the HTTP
|
||||
// boundary (validateAssigneePair on assign) and inside enqueueSquadLeaderTask
|
||||
// (canEnqueueSquadLeader), so a write must NOT re-run or sink it — it passes
|
||||
// allow-all. The self-loop check needs the request's X-Task-ID header.
|
||||
func (h *Handler) issueTriggerWriteProbe(r *http.Request, actorType string, issue db.Issue) service.IssueTriggerProbe {
|
||||
return service.IssueTriggerProbe{
|
||||
CanAccessAgent: nil, // allow-all; gate lives at the write boundary
|
||||
IsSelfLoop: func() bool {
|
||||
return h.isAgentRunningOnIssue(r, actorType, issue)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// issueTriggerPreviewProbe mirrors the real write-time gates for the read-only
|
||||
// preview: the private-agent gate (so preview never leaks a private agent's
|
||||
// readiness to a member who cannot see it — matching validateAssigneePair /
|
||||
// canEnqueueSquadLeader) and the same self-loop guard.
|
||||
func (h *Handler) issueTriggerPreviewProbe(r *http.Request, actorType, actorID, workspaceID string, issue db.Issue) service.IssueTriggerProbe {
|
||||
originatorUserID := h.invokeOriginatorFromRequest(r, actorType, actorID)
|
||||
return service.IssueTriggerProbe{
|
||||
CanAccessAgent: func(agent db.Agent) bool {
|
||||
@@ -143,7 +158,7 @@ func (h *Handler) PreviewIssueTrigger(w http.ResponseWriter, r *http.Request) {
|
||||
resp := IssueTriggerPreviewResponse{Triggers: make([]IssueTriggerPreviewItem, 0)}
|
||||
|
||||
appendTrigger := func(issue db.Issue, in service.IssueTriggerInput) {
|
||||
probe := h.issueTriggerProbe(r, actorType, actorID, workspaceID, issue)
|
||||
probe := h.issueTriggerPreviewProbe(r, actorType, actorID, workspaceID, issue)
|
||||
if trigger, ok := h.IssueService.WillEnqueueRun(r.Context(), in, probe); ok {
|
||||
resp.Triggers = append(resp.Triggers, IssueTriggerPreviewItem{
|
||||
IssueID: uuidToString(trigger.IssueID),
|
||||
|
||||
@@ -24,13 +24,9 @@ func seededReadyAgentID(t *testing.T) string {
|
||||
}
|
||||
|
||||
func previewIssueTrigger(t *testing.T, body map[string]any) IssueTriggerPreviewResponse {
|
||||
return previewIssueTriggerAs(t, testUserID, body)
|
||||
}
|
||||
|
||||
func previewIssueTriggerAs(t *testing.T, userID string, body map[string]any) IssueTriggerPreviewResponse {
|
||||
t.Helper()
|
||||
w := httptest.NewRecorder()
|
||||
req := newRequestAs(userID, "POST", "/api/issues/preview-trigger?workspace_id="+testWorkspaceID, body)
|
||||
req := newRequest("POST", "/api/issues/preview-trigger?workspace_id="+testWorkspaceID, body)
|
||||
testHandler.PreviewIssueTrigger(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("PreviewIssueTrigger: expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
@@ -72,15 +68,6 @@ func taskCountFor(t *testing.T, issueID, agentID string) int {
|
||||
return n
|
||||
}
|
||||
|
||||
func assignIssueToAgentForTest(t *testing.T, issueID, agentID string) {
|
||||
t.Helper()
|
||||
if _, err := testPool.Exec(context.Background(), `
|
||||
UPDATE issue SET assignee_type = 'agent', assignee_id = $2 WHERE id = $1
|
||||
`, issueID, agentID); err != nil {
|
||||
t.Fatalf("assign issue to agent: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPreviewIssueTrigger_CreateAgentVsBacklog covers the create entry point:
|
||||
// an active status with an agent assignee previews one run; the same assignee
|
||||
// parked in backlog previews none.
|
||||
@@ -151,201 +138,6 @@ func TestPreviewIssueTrigger_BatchAggregates(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestBlockedToTodoRestartsSquad verifies that once a squad-assigned issue is
|
||||
// parked as blocked and its previous run has ended, moving it back to todo must
|
||||
// preview and enqueue a fresh leader task.
|
||||
func TestBlockedToTodoRestartsSquad(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
leaderID := seededReadyAgentID(t)
|
||||
|
||||
var squadID string
|
||||
if err := testPool.QueryRow(ctx, `
|
||||
INSERT INTO squad (workspace_id, name, description, leader_id, creator_id)
|
||||
VALUES ($1, $2, '', $3, $4)
|
||||
RETURNING id
|
||||
`, testWorkspaceID, "Blocked Recovery Squad", leaderID, testUserID).Scan(&squadID); err != nil {
|
||||
t.Fatalf("create squad: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _, _ = testPool.Exec(ctx, `DELETE FROM squad WHERE id = $1`, squadID) })
|
||||
|
||||
issue := createIssueForTest(t, map[string]any{
|
||||
"title": "blocked squad recovery",
|
||||
"status": "blocked",
|
||||
})
|
||||
if _, err := testPool.Exec(ctx, `
|
||||
UPDATE issue SET assignee_type = 'squad', assignee_id = $2 WHERE id = $1
|
||||
`, issue.ID, squadID); err != nil {
|
||||
t.Fatalf("assign blocked issue to squad: %v", err)
|
||||
}
|
||||
|
||||
preview := previewIssueTrigger(t, map[string]any{
|
||||
"issue_ids": []string{issue.ID},
|
||||
"status": "todo",
|
||||
})
|
||||
if preview.TotalCount != 1 || len(preview.Triggers) != 1 {
|
||||
t.Fatalf("blocked->todo preview: expected 1 trigger, got %+v", preview)
|
||||
}
|
||||
if trigger := preview.Triggers[0]; trigger.AgentID != leaderID || trigger.Source != "status" {
|
||||
t.Fatalf("blocked->todo preview: wrong trigger %+v", trigger)
|
||||
}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequest("PUT", "/api/issues/"+issue.ID, map[string]any{
|
||||
"status": "todo",
|
||||
}), "id", issue.ID)
|
||||
testHandler.UpdateIssue(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("UpdateIssue blocked->todo: %d %s", w.Code, w.Body.String())
|
||||
}
|
||||
if got := taskCountFor(t, issue.ID, leaderID); got != 1 {
|
||||
t.Fatalf("blocked->todo: expected 1 squad-leader task, got %d", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBlockedToNonRunnableStatusDoesNotRestart(t *testing.T) {
|
||||
agentID := seededReadyAgentID(t)
|
||||
for _, status := range []string{"backlog", "done", "cancelled"} {
|
||||
t.Run(status, func(t *testing.T) {
|
||||
issue := createIssueForTest(t, map[string]any{
|
||||
"title": "blocked non-runnable " + status,
|
||||
"status": "blocked",
|
||||
})
|
||||
if _, err := testPool.Exec(context.Background(), `
|
||||
UPDATE issue SET assignee_type = 'agent', assignee_id = $2 WHERE id = $1
|
||||
`, issue.ID, agentID); err != nil {
|
||||
t.Fatalf("assign blocked issue: %v", err)
|
||||
}
|
||||
|
||||
preview := previewIssueTrigger(t, map[string]any{
|
||||
"issue_ids": []string{issue.ID},
|
||||
"status": status,
|
||||
})
|
||||
if preview.TotalCount != 0 {
|
||||
t.Fatalf("blocked->%s: expected no trigger, got %+v", status, preview)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestBlockedPrivateAgentResumeRequiresInvokePermission covers status-only
|
||||
// writes, which keep the existing assignee and therefore do not pass through
|
||||
// validateAssigneePair. Preview and write must both deny an unrelated member,
|
||||
// while still applying the requested issue status.
|
||||
func TestBlockedPrivateAgentResumeRequiresInvokePermission(t *testing.T) {
|
||||
agentID, _, memberID := privateAgentTestFixture(t)
|
||||
|
||||
for _, status := range []string{"todo", "in_progress", "in_review"} {
|
||||
t.Run(status, func(t *testing.T) {
|
||||
issue := createIssueForTest(t, map[string]any{
|
||||
"title": "private agent resume denied " + status,
|
||||
"status": "blocked",
|
||||
})
|
||||
assignIssueToAgentForTest(t, issue.ID, agentID)
|
||||
|
||||
preview := previewIssueTriggerAs(t, memberID, map[string]any{
|
||||
"issue_ids": []string{issue.ID},
|
||||
"status": status,
|
||||
})
|
||||
if preview.TotalCount != 0 {
|
||||
t.Fatalf("blocked->%s preview: expected no trigger, got %+v", status, preview)
|
||||
}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequestAs(memberID, "PUT", "/api/issues/"+issue.ID, map[string]any{
|
||||
"status": status,
|
||||
}), "id", issue.ID)
|
||||
testHandler.UpdateIssue(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("UpdateIssue blocked->%s: %d %s", status, w.Code, w.Body.String())
|
||||
}
|
||||
var updated IssueResponse
|
||||
if err := json.NewDecoder(w.Body).Decode(&updated); err != nil {
|
||||
t.Fatalf("decode blocked->%s response: %v", status, err)
|
||||
}
|
||||
if updated.Status != status {
|
||||
t.Fatalf("blocked->%s status was not applied: got %q", status, updated.Status)
|
||||
}
|
||||
if got := taskCountFor(t, issue.ID, agentID); got != 0 {
|
||||
t.Fatalf("blocked->%s write disagreed with preview: enqueued %d tasks", status, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBatchBlockedPrivateAgentResumeRequiresInvokePermission(t *testing.T) {
|
||||
agentID, _, memberID := privateAgentTestFixture(t)
|
||||
issues := []IssueResponse{
|
||||
createIssueForTest(t, map[string]any{"title": "private batch resume 1", "status": "blocked"}),
|
||||
createIssueForTest(t, map[string]any{"title": "private batch resume 2", "status": "blocked"}),
|
||||
}
|
||||
issueIDs := make([]string, 0, len(issues))
|
||||
for _, issue := range issues {
|
||||
assignIssueToAgentForTest(t, issue.ID, agentID)
|
||||
issueIDs = append(issueIDs, issue.ID)
|
||||
}
|
||||
|
||||
preview := previewIssueTriggerAs(t, memberID, map[string]any{
|
||||
"issue_ids": issueIDs,
|
||||
"status": "todo",
|
||||
})
|
||||
if preview.TotalCount != 0 {
|
||||
t.Fatalf("batch preview: expected no triggers, got %+v", preview)
|
||||
}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := newRequestAs(memberID, "POST", "/api/issues/batch-update", map[string]any{
|
||||
"issue_ids": issueIDs,
|
||||
"updates": map[string]any{"status": "todo"},
|
||||
})
|
||||
testHandler.BatchUpdateIssues(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("BatchUpdateIssues: %d %s", w.Code, w.Body.String())
|
||||
}
|
||||
var batchResp struct {
|
||||
Updated int `json:"updated"`
|
||||
}
|
||||
if err := json.NewDecoder(w.Body).Decode(&batchResp); err != nil {
|
||||
t.Fatalf("decode batch response: %v", err)
|
||||
}
|
||||
if batchResp.Updated != len(issues) {
|
||||
t.Fatalf("batch status update: expected %d updated, got %d", len(issues), batchResp.Updated)
|
||||
}
|
||||
for _, issue := range issues {
|
||||
if got := taskCountFor(t, issue.ID, agentID); got != 0 {
|
||||
t.Fatalf("batch write disagreed with preview for issue %s: enqueued %d tasks", issue.ID, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBlockedPrivateAgentResumeAllowsOwner(t *testing.T) {
|
||||
agentID, ownerID, _ := privateAgentTestFixture(t)
|
||||
issue := createIssueForTest(t, map[string]any{
|
||||
"title": "private agent owner resume",
|
||||
"status": "blocked",
|
||||
})
|
||||
assignIssueToAgentForTest(t, issue.ID, agentID)
|
||||
|
||||
preview := previewIssueTriggerAs(t, ownerID, map[string]any{
|
||||
"issue_ids": []string{issue.ID},
|
||||
"status": "todo",
|
||||
})
|
||||
if preview.TotalCount != 1 || len(preview.Triggers) != 1 {
|
||||
t.Fatalf("owner preview: expected one trigger, got %+v", preview)
|
||||
}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParam(newRequestAs(ownerID, "PUT", "/api/issues/"+issue.ID, map[string]any{
|
||||
"status": "todo",
|
||||
}), "id", issue.ID)
|
||||
testHandler.UpdateIssue(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("UpdateIssue as owner: %d %s", w.Code, w.Body.String())
|
||||
}
|
||||
if got := taskCountFor(t, issue.ID, agentID); got != 1 {
|
||||
t.Fatalf("owner write disagreed with preview: expected 1 task, got %d", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPreviewIssueTrigger_MatchesWritePath is the core invariant: when preview
|
||||
// says a run will start, the real write path enqueues it; when preview says it
|
||||
// won't, the write path enqueues nothing.
|
||||
|
||||
@@ -15,22 +15,24 @@ const (
|
||||
// RunSourceAssign covers issue creation and assignee changes — the issue
|
||||
// is being handed to an agent/squad. Parks silently on backlog.
|
||||
RunSourceAssign RunEnqueueSource = "assign"
|
||||
// RunSourceStatus covers activating an already-assigned parked issue by
|
||||
// moving it out of backlog or blocked into a runnable status.
|
||||
// RunSourceStatus covers promoting an already-assigned issue out of
|
||||
// backlog into an active status.
|
||||
RunSourceStatus RunEnqueueSource = "status"
|
||||
)
|
||||
|
||||
// IssueTriggerProbe carries the request-scoped checks WillEnqueueRun cannot
|
||||
// resolve from issue state alone.
|
||||
//
|
||||
// CanAccessAgent is the invocation-permission gate shared by preview and write
|
||||
// paths. Keeping it in this decision is required for status-only writes, which
|
||||
// do not revalidate an unchanged assignee. A nil func is treated as allow-all
|
||||
// for callers that have no request-scoped permission model.
|
||||
// CanAccessAgent is the private-agent gate. The write paths enforce it at the
|
||||
// HTTP boundary (validateAssigneePair on assign, canEnqueueSquadLeader inside
|
||||
// the squad enqueue helper) and therefore pass an allow-all probe so the gate
|
||||
// is never duplicated or sunk into the service layer. Preview passes the real
|
||||
// gate so it never leaks a private agent's readiness to a member who cannot
|
||||
// see it. A nil func is treated as allow-all.
|
||||
//
|
||||
// IsSelfLoop reports whether activating this parked issue would be the calling
|
||||
// agent re-triggering its own running task. Only the status source consults it;
|
||||
// create and assign never do. A nil func means "not a self-loop".
|
||||
// IsSelfLoop reports whether promoting this issue out of backlog would be the
|
||||
// calling agent re-triggering its own running task. Only the status source
|
||||
// consults it; create and assign never do. A nil func means "not a self-loop".
|
||||
type IssueTriggerProbe struct {
|
||||
CanAccessAgent func(agent db.Agent) bool
|
||||
IsSelfLoop func() bool
|
||||
@@ -75,7 +77,7 @@ func allowAllAgents(db.Agent) bool { return true }
|
||||
// CreateAgentTask, guarded by the (issue_id, agent_id) partial unique index
|
||||
// over pending (queued/dispatched) tasks; the pending check below mirrors that
|
||||
// guard, and only the status source needs it:
|
||||
// - status source (parked → runnable) can re-fire against an assignee that
|
||||
// - status source (backlog → active) can re-fire against an assignee that
|
||||
// already holds a pending task (e.g. one a @mention raised while the issue
|
||||
// sat in backlog); the check keeps preview from promising a run the unique
|
||||
// index would coalesce away.
|
||||
@@ -102,8 +104,8 @@ func (s *IssueService) WillEnqueueRun(ctx context.Context, in IssueTriggerInput,
|
||||
return IssueRunTrigger{}, false
|
||||
}
|
||||
source = RunSourceAssign
|
||||
case in.StatusChanged && isParkedIssueStatus(in.PrevStatus) &&
|
||||
isRunnableIssueStatus(issue.Status):
|
||||
case in.StatusChanged && in.PrevStatus == "backlog" &&
|
||||
issue.Status != "done" && issue.Status != "cancelled":
|
||||
if probe.IsSelfLoop != nil && probe.IsSelfLoop() {
|
||||
return IssueRunTrigger{}, false
|
||||
}
|
||||
@@ -163,19 +165,6 @@ func (s *IssueService) WillEnqueueRun(ctx context.Context, in IssueTriggerInput,
|
||||
return IssueRunTrigger{}, false
|
||||
}
|
||||
|
||||
func isParkedIssueStatus(status string) bool {
|
||||
return status == "backlog" || status == "blocked"
|
||||
}
|
||||
|
||||
func isRunnableIssueStatus(status string) bool {
|
||||
switch status {
|
||||
case "todo", "in_progress", "in_review":
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// hasPendingRun reports whether the agent already holds a queued or dispatched
|
||||
// task for the issue (the (issue_id, agent_id) unique-index slot). Errors fail
|
||||
// closed to "pending" so preview never over-promises a run.
|
||||
|
||||
Reference in New Issue
Block a user