From 4f9407ed54696d8f29677f620fda5330711c3019 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 22 Jun 2026 15:50:14 +0100 Subject: [PATCH] fix: show queued CI auto-fix prompts in order (#1463) --- .../event_handlers_github_ci_automation.go | 52 +++++++++----- ...vent_handlers_github_ci_automation_test.go | 49 ++++++------- .../orchestrator/event_handlers_queue_test.go | 69 +++++++++++++++++++ docs/specs/ui/ci-pr-automation.md | 6 +- 4 files changed, 131 insertions(+), 45 deletions(-) diff --git a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go index 3c4a049436..8a6634d8af 100644 --- a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go +++ b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go @@ -143,13 +143,7 @@ func (s *Service) dispatchCIAutomationPrompt(ctx context.Context, session *model if s.messageQueue == nil { return fmt.Errorf("message queue is not configured") } - if !s.recordCIAutomationUserMessage(ctx, session.TaskID, session.ID, chatPrompt) { - return fmt.Errorf("failed to record CI automation user message") - } - metadata := map[string]interface{}{ - "origin": ciAutomationOrigin, - metaKeyUserMessageRecorded: true, - } + metadata := ciAutomationMessageMetadata() _, err := s.messageQueue.QueueMessageWithMetadata(ctx, session.ID, session.TaskID, chatPrompt, "", messagequeue.QueuedByWorkflow, false, nil, metadata) if err != nil { return err @@ -157,12 +151,12 @@ func (s *Service) dispatchCIAutomationPrompt(ctx context.Context, session *model s.publishQueueStatusEvent(ctx, session.ID) return nil case models.TaskSessionStateWaitingForInput, models.TaskSessionStateIdle: - if _, err := s.PromptTask(ctx, session.TaskID, session.ID, chatPrompt, "", false, nil, false); err != nil { - return err - } if !s.recordCIAutomationUserMessage(ctx, session.TaskID, session.ID, chatPrompt) { return fmt.Errorf("failed to record CI automation user message") } + if _, err := s.PromptTask(ctx, session.TaskID, session.ID, chatPrompt, "", false, nil, false); err != nil { + return err + } return nil default: return fmt.Errorf("session is not promptable: %s", session.State) @@ -178,11 +172,7 @@ func (s *Service) recordCIAutomationUserMessage(ctx context.Context, taskID, ses s.startTurnForSession(ctx, sessionID) turnID = s.getActiveTurnID(sessionID) } - meta := NewUserMessageMeta().WithAutoStart(true).ToMap() - if meta == nil { - meta = map[string]interface{}{} - } - meta["origin"] = ciAutomationOrigin + meta := ciAutomationMessageMetadata() if err := s.messageCreator.CreateUserMessage(ctx, taskID, prompt, sessionID, turnID, meta); err != nil { s.logger.Error("failed to create CI automation user message", zap.String("task_id", taskID), @@ -193,8 +183,21 @@ func (s *Service) recordCIAutomationUserMessage(ctx context.Context, taskID, ses return true } +func ciAutomationMessageMetadata() map[string]interface{} { + meta := NewUserMessageMeta().WithAutoStart(true).ToMap() + if meta == nil { + meta = map[string]interface{}{} + } + meta["origin"] = ciAutomationOrigin + return meta +} + func ciAutomationChatPrompt(prompt string) string { - return "@ci-auto-fix\n\n" + sysprompt.Wrap(prompt) + prompt = strings.TrimSpace(prompt) + if prompt == "" { + return "@ci-auto-fix" + } + return "@ci-auto-fix\n\n" + prompt } func (s *Service) recordCIAutomationError(ctx context.Context, pr *github.TaskPR, message string) { @@ -269,9 +272,22 @@ func ciAutomationCurrentCheckpoint(feedback *github.PRFeedback) ciAutomationChec } func ciAutomationRenderPrompt(base string, pr *github.TaskPR, delta ciAutomationCheckpoint) string { + var parts []string + if base = strings.TrimSpace(base); base != "" { + parts = append(parts, sysprompt.Wrap(base)) + } + if snapshot := ciAutomationRenderSnapshot(pr, delta); snapshot != "" { + parts = append(parts, snapshot) + } + return strings.Join(parts, "\n\n") +} + +func ciAutomationRenderSnapshot(pr *github.TaskPR, delta ciAutomationCheckpoint) string { + if pr == nil { + return "" + } var b strings.Builder - b.WriteString(strings.TrimSpace(base)) - b.WriteString("\n\nPR: ") + b.WriteString("PR: ") b.WriteString(fmt.Sprintf("%s/%s#%d", pr.Owner, pr.Repo, pr.PRNumber)) if len(delta.FailedChecks) > 0 { b.WriteString("\n\nNew or changed failing checks:") diff --git a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go index cb2b011727..9a5c871a21 100644 --- a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go @@ -101,6 +101,13 @@ func TestCIAutomationFeedbackDelta(t *testing.T) { if !strings.Contains(prompt, "Base instructions") || !strings.Contains(prompt, "acme/widget#42") || !strings.Contains(prompt, "also this") { t.Fatalf("rendered prompt missing expected content:\n%s", prompt) } + visible := sysprompt.StripSystemContent(prompt) + if strings.Contains(visible, "Base instructions") { + t.Fatalf("shared CI prompt should be hidden from visible chat content, got:\n%s", visible) + } + if !strings.Contains(visible, "acme/widget#42") || !strings.Contains(visible, "also this") { + t.Fatalf("PR snapshot should remain visible, got:\n%s", visible) + } } func TestCIAutomationCheckpointPrunesResolvedFailures(t *testing.T) { @@ -189,21 +196,11 @@ func TestHandleTaskPRCIAutomationQueuesFixDedupesAndMerges(t *testing.T) { if status.Count != 1 || !strings.Contains(status.Entries[0].Content, "@ci-auto-fix") || !strings.Contains(status.Entries[0].Content, "acme/widget#42") || !strings.Contains(status.Entries[0].Content, "unit") { t.Fatalf("expected queued CI fix prompt, got %+v", status) } - if status.Entries[0].Metadata[metaKeyUserMessageRecorded] != true { - t.Fatalf("expected queued prompt to be tagged as already recorded, got %+v", status.Entries[0].Metadata) + if status.Entries[0].Metadata[metaKeyUserMessageRecorded] == true { + t.Fatalf("expected queued CI prompt to be recorded when drained, got %+v", status.Entries[0].Metadata) } - if len(messageCreator.userMessages) != 1 { - t.Fatalf("expected one visible CI automation user message, got %d", len(messageCreator.userMessages)) - } - chatMessage := messageCreator.userMessages[0] - if got := sysprompt.StripSystemContent(chatMessage.content); got != "@ci-auto-fix" { - t.Fatalf("expected visible chat prompt to be @ci-auto-fix, got %q", got) - } - if !strings.Contains(chatMessage.content, "") || !strings.Contains(chatMessage.content, "Fix the PR") || !strings.Contains(chatMessage.content, "unit") { - t.Fatalf("expected raw chat message to preserve hidden CI prompt, got %q", chatMessage.content) - } - if chatMessage.metadata["origin"] != ciAutomationOrigin || chatMessage.metadata["auto_start"] != true { - t.Fatalf("expected CI automation metadata, got %+v", chatMessage.metadata) + if len(messageCreator.userMessages) != 0 { + t.Fatalf("expected queued CI automation to record chat message on drain, got %d", len(messageCreator.userMessages)) } if len(ghSvc.fixAttempts) != 1 { t.Fatalf("expected one fix attempt, got %d", len(ghSvc.fixAttempts)) @@ -217,7 +214,7 @@ func TestHandleTaskPRCIAutomationQueuesFixDedupesAndMerges(t *testing.T) { if got := svc.messageQueue.GetStatus(ctx, "session-1").Count; got != 1 { t.Fatalf("expected dedupe to avoid second queued prompt, got %d", got) } - if len(messageCreator.userMessages) != 1 { + if len(messageCreator.userMessages) != 0 { t.Fatalf("expected dedupe to avoid second chat message, got %d", len(messageCreator.userMessages)) } @@ -256,7 +253,7 @@ func TestDispatchCIAutomationPromptDoesNotRecordUserMessageWhenQueueFails(t *tes } } -func TestDispatchCIAutomationPromptFailsWhenRunningUserMessageCannotBeRecorded(t *testing.T) { +func TestDispatchCIAutomationPromptQueuesWhenRunningUserMessageCannotBeRecorded(t *testing.T) { ctx := context.Background() repo := setupTestRepo(t) seedTaskAndSession(t, repo, "task-1", "session-1", models.TaskSessionStateRunning) @@ -270,15 +267,19 @@ func TestDispatchCIAutomationPromptFailsWhenRunningUserMessageCannotBeRecorded(t } err = svc.dispatchCIAutomationPrompt(ctx, session, "Fix the PR") - if err == nil || !strings.Contains(err.Error(), "failed to record CI automation user message") { - t.Fatalf("expected user-message failure, got %v", err) + if err != nil { + t.Fatalf("expected queued CI automation prompt, got %v", err) } - if got := svc.messageQueue.GetStatus(ctx, "session-1").Count; got != 0 { - t.Fatalf("expected no queued prompt when user message could not be recorded, got %d", got) + status := svc.messageQueue.GetStatus(ctx, "session-1") + if status.Count != 1 { + t.Fatalf("expected queued prompt when user message cannot be recorded yet, got %d", status.Count) + } + if status.Entries[0].Metadata[metaKeyUserMessageRecorded] == true { + t.Fatalf("expected queued prompt to retry user-message recording on drain, got %+v", status.Entries[0].Metadata) } } -func TestDispatchCIAutomationPromptRecordsUserMessageAfterDirectPrompt(t *testing.T) { +func TestDispatchCIAutomationPromptRecordsUserMessageBeforeDirectPrompt(t *testing.T) { ctx := context.Background() repo := setupTestRepo(t) seedTaskAndSession(t, repo, "task-1", "session-1", models.TaskSessionStateWaitingForInput) @@ -312,7 +313,7 @@ func TestDispatchCIAutomationPromptRecordsUserMessageAfterDirectPrompt(t *testin } } -func TestDispatchCIAutomationPromptDoesNotRecordUserMessageWhenDirectPromptFails(t *testing.T) { +func TestDispatchCIAutomationPromptRecordsUserMessageBeforeDirectPromptFailure(t *testing.T) { ctx := context.Background() repo := setupTestRepo(t) seedTaskAndSession(t, repo, "task-1", "session-1", models.TaskSessionStateWaitingForInput) @@ -340,8 +341,8 @@ func TestDispatchCIAutomationPromptDoesNotRecordUserMessageWhenDirectPromptFails if err == nil || !strings.Contains(err.Error(), "agent rejected prompt") { t.Fatalf("expected prompt failure, got %v", err) } - if len(messageCreator.userMessages) != 0 { - t.Fatalf("expected no visible CI automation user message on prompt failure, got %d", len(messageCreator.userMessages)) + if len(messageCreator.userMessages) != 1 { + t.Fatalf("expected visible CI automation user message before prompt failure, got %d", len(messageCreator.userMessages)) } } diff --git a/apps/backend/internal/orchestrator/event_handlers_queue_test.go b/apps/backend/internal/orchestrator/event_handlers_queue_test.go index eae074341b..af962c7fae 100644 --- a/apps/backend/internal/orchestrator/event_handlers_queue_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_queue_test.go @@ -3,10 +3,13 @@ package orchestrator import ( "context" "reflect" + "strings" "testing" + "github.com/kandev/kandev/internal/github" "github.com/kandev/kandev/internal/orchestrator/executor" "github.com/kandev/kandev/internal/orchestrator/messagequeue" + "github.com/kandev/kandev/internal/sysprompt" "github.com/kandev/kandev/internal/task/models" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -102,6 +105,72 @@ func TestExecuteQueuedMessage_SkipsUserMessageWhenAlreadyRecorded(t *testing.T) } } +func TestExecuteQueuedMessage_RecordsCIAutomationPromptOnDrain(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "t1", "s1", "step1") + + session, err := repo.GetTaskSession(ctx, "s1") + if err != nil { + t.Fatalf("failed to get session: %v", err) + } + session.State = models.TaskSessionStateWaitingForInput + session.AgentExecutionID = "exec-1" + if err := repo.UpdateTaskSession(ctx, session); err != nil { + t.Fatalf("failed to update session: %v", err) + } + + taskRepo := newMockTaskRepo() + agentMgr := &mockAgentManager{isAgentRunning: true, repoForExecutionLookup: repo} + svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr) + svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{}) + seedExecutorRunning(t, repo, "s1", "t1", "exec-1") + + mc := &mockMessageCreator{} + svc.messageCreator = mc + + queuedMsg := &messagequeue.QueuedMessage{ + ID: "q1", + SessionID: "s1", + TaskID: "t1", + Content: ciAutomationChatPrompt(ciAutomationRenderPrompt( + "Fix the PR", + &github.TaskPR{Owner: "acme", Repo: "widget", PRNumber: 42}, + ciAutomationCheckpoint{ + FailedChecks: []ciAutomationCheckSnapshot{{Name: "unit", Conclusion: "failure"}}, + }, + )), + QueuedBy: messagequeue.QueuedByWorkflow, + Metadata: map[string]interface{}{ + "origin": ciAutomationOrigin, + "auto_start": true, + }, + } + + svc.executeQueuedMessage("s1", queuedMsg) + + if len(mc.userMessages) != 1 { + t.Fatalf("expected CI automation user message to be recorded on drain, got %d", len(mc.userMessages)) + } + chatMessage := mc.userMessages[0] + visible := sysprompt.StripSystemContent(chatMessage.content) + if !strings.Contains(visible, "@ci-auto-fix") || !strings.Contains(visible, "PR: acme/widget#42") || !strings.Contains(visible, "unit: failure") { + t.Fatalf("expected visible chat prompt to include @ci-auto-fix and PR snapshot, got %q", visible) + } + if strings.Contains(visible, "Fix the PR") { + t.Fatalf("expected shared CI prompt to stay hidden, got %q", visible) + } + if !strings.Contains(chatMessage.content, "") || !strings.Contains(chatMessage.content, "Fix the PR") || !strings.Contains(chatMessage.content, "unit") { + t.Fatalf("expected raw chat message to preserve hidden CI prompt, got %q", chatMessage.content) + } + if chatMessage.metadata["origin"] != ciAutomationOrigin || chatMessage.metadata["auto_start"] != true { + t.Fatalf("expected CI automation metadata, got %+v", chatMessage.metadata) + } + if len(agentMgr.capturedPrompts) != 1 { + t.Fatalf("expected the prompt to reach PromptAgent, captured=%d", len(agentMgr.capturedPrompts)) + } +} + func TestExecuteQueuedMessage_StoresAttachmentsInUserMessageMetadata(t *testing.T) { ctx := context.Background() repo := setupTestRepo(t) diff --git a/docs/specs/ui/ci-pr-automation.md b/docs/specs/ui/ci-pr-automation.md index fd1b17e797..b39858e503 100644 --- a/docs/specs/ui/ci-pr-automation.md +++ b/docs/specs/ui/ci-pr-automation.md @@ -136,7 +136,7 @@ Auto-fix cycle for one task/PR: 3. Kandev fetches full PR feedback. 4. Kandev compares the current feedback snapshot to `last_fix_checkpoint_json` and `last_fix_signature`. 5. If there is no material change, the cycle ends without prompting. -6. If there is new or materially changed feedback, Kandev renders the task override or default `ci-auto-fix` prompt and sends or queues it for the task session. +6. If there is new or materially changed feedback, Kandev renders the task override or default `ci-auto-fix` prompt and sends or queues it for the task session. The saved/shared `ci-auto-fix` instructions are hidden system context, but the PR snapshot details are visible in the chat message after `@ci-auto-fix`, before the agent output for that automation turn. 7. The default prompt instructs the agent to classify the new feedback before editing. If the new feedback is only summaries, status updates, no-finding reports, duplicated or already addressed comments, rate-limit notices, or other non-actionable review diagnostics, the agent @@ -168,7 +168,7 @@ Auto-merge cycle for one task/PR: | PR is closed or merged | Controls are disabled for that PR; no automation runs. | | Full PR feedback fetch fails | Auto-fix does not prompt; per-PR automation state records the error and the next materially changed lightweight status may retry. | | Task has no promptable session | Auto-fix records an error instead of creating a surprising new session. | -| Task session is busy | Auto-fix queues the rendered prompt with workflow/automation metadata for later delivery. | +| Task session is busy | Auto-fix queues the rendered prompt with workflow/automation metadata for later delivery; the visible `@ci-auto-fix` chat message, including PR snapshot details, is created when the queued prompt is delivered and before the agent's response for that turn. | | Same feedback snapshot repeats | Auto-fix does not send another prompt. | | GitHub merge fails | Auto-merge records the error and does not retry until the readiness signature changes. | | Default prompt row is missing | Backend falls back to the embedded `ci-auto-fix.md` content. | @@ -196,7 +196,7 @@ Auto-merge cycle for one task/PR: - **GIVEN** auto-fix already prompted for a failure snapshot, **WHEN** the same failure is observed again on a later poll, **THEN** no duplicate prompt is sent. - **GIVEN** auto-fix already prompted for a failure snapshot, **WHEN** a new failed check or new unresolved review comment appears, **THEN** Kandev sends or queues a new prompt containing the new or materially changed feedback. - **GIVEN** auto-fix sends a prompt for a snapshot that contains only non-actionable automation summaries or status updates, **WHEN** the agent reviews that prompt, **THEN** the agent does not modify files, commit, or push and only reports that there is nothing actionable to address. -- **GIVEN** auto-fix is enabled and the task session is running, **WHEN** new actionable PR feedback appears, **THEN** the prompt is queued and delivered after the current turn rather than interrupting the running session. +- **GIVEN** auto-fix is enabled and the task session is running, **WHEN** new actionable PR feedback appears, **THEN** the prompt is queued and delivered after the current turn rather than interrupting the running session, and the chat history shows the `@ci-auto-fix` user message with visible PR snapshot details before the agent output for the queued turn. - **GIVEN** auto-merge is enabled and the PR has passing checks, required reviews, no unresolved threads, and clean mergeability, **WHEN** the PR watch poll observes the ready state, **THEN** Kandev merges the PR with the existing backend merge-method selection. - **GIVEN** auto-merge is enabled but the PR has requested changes, pending required review, failing checks, unresolved threads, or dirty mergeability, **WHEN** the PR watch poll observes the state, **THEN** Kandev does not merge. - **GIVEN** auto-merge attempted a ready-state merge and GitHub rejected it, **WHEN** the same ready state is observed again, **THEN** Kandev does not retry until the readiness signature changes.