fix: scope PR auto-fix outcome reporting (#3602)
This commit is contained in:
@@ -0,0 +1,213 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
type ciOutcomeScopeSnapshot struct {
|
||||
legacyOptionRows int
|
||||
legacyOptions []TaskCIOptions
|
||||
prOptions []*TaskPRAutomationOptions
|
||||
states []*TaskCIPRAutomationState
|
||||
}
|
||||
|
||||
func snapshotCIAutoFixScope(t *testing.T, store *Store, taskID string) ciOutcomeScopeSnapshot {
|
||||
t.Helper()
|
||||
var legacyOptionRows int
|
||||
if err := store.db.GetContext(context.Background(), &legacyOptionRows,
|
||||
"SELECT COUNT(*) FROM github_task_ci_options WHERE task_id = ?", taskID); err != nil {
|
||||
t.Fatalf("count legacy options: %v", err)
|
||||
}
|
||||
var legacyOptions []TaskCIOptions
|
||||
if err := store.ro.SelectContext(context.Background(), &legacyOptions,
|
||||
store.ro.Rebind("SELECT * FROM github_task_ci_options WHERE task_id = ?"), taskID); err != nil {
|
||||
t.Fatalf("list legacy options: %v", err)
|
||||
}
|
||||
prOptions, err := store.ListTaskPRAutomationOptions(context.Background(), taskID)
|
||||
if err != nil {
|
||||
t.Fatalf("list PR options: %v", err)
|
||||
}
|
||||
states, err := store.ListTaskCIPRStates(context.Background(), taskID)
|
||||
if err != nil {
|
||||
t.Fatalf("list PR states: %v", err)
|
||||
}
|
||||
return ciOutcomeScopeSnapshot{
|
||||
legacyOptionRows: legacyOptionRows,
|
||||
legacyOptions: legacyOptions,
|
||||
prOptions: prOptions,
|
||||
states: states,
|
||||
}
|
||||
}
|
||||
|
||||
func requireCIAutoFixScopeUnchanged(t *testing.T, before, after ciOutcomeScopeSnapshot) {
|
||||
t.Helper()
|
||||
if !reflect.DeepEqual(before, after) {
|
||||
t.Fatalf("auto-fix state changed after rejected report:\nbefore: %+v\nafter: %+v", before, after)
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-UI-CI-PR-AUTOMATION-001.10
|
||||
// @covers AC-UI-CI-PR-AUTOMATION-001.17
|
||||
func TestStorePRAutoFixOutcomeOrdinaryTurnHasNoSideEffects(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
report := func(store *Store, taskID, sessionID, turnID string) error {
|
||||
return store.ReportTaskCIAutoFixOutcome(ctx, TaskCIAutoFixOutcomeReport{
|
||||
TaskID: taskID, SessionID: sessionID, TurnID: turnID,
|
||||
Outcome: TaskCIAutoFixOutcomeBlocked, Summary: "ordinary PR work",
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("absent settings", func(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
before := snapshotCIAutoFixScope(t, store, "task-absent")
|
||||
if err := report(store, "task-absent", "session-current", "turn-current"); !errors.Is(err, ErrTaskCIAutoFixAttemptNotFound) {
|
||||
t.Fatalf("report error = %v, want %v", err, ErrTaskCIAutoFixAttemptNotFound)
|
||||
}
|
||||
after := snapshotCIAutoFixScope(t, store, "task-absent")
|
||||
requireCIAutoFixScopeUnchanged(t, before, after)
|
||||
})
|
||||
|
||||
t.Run("explicitly disabled settings", func(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
disabled := false
|
||||
if _, err := store.UpdateTaskPRAutomationOptions(ctx, "task-disabled", "repo-a", 101,
|
||||
TaskPRAutomationOptionsPatch{AutoFixEnabled: &disabled}, false); err != nil {
|
||||
t.Fatalf("seed disabled options: %v", err)
|
||||
}
|
||||
before := snapshotCIAutoFixScope(t, store, "task-disabled")
|
||||
if err := report(store, "task-disabled", "session-current", "turn-current"); !errors.Is(err, ErrTaskCIAutoFixAttemptNotFound) {
|
||||
t.Fatalf("report error = %v, want %v", err, ErrTaskCIAutoFixAttemptNotFound)
|
||||
}
|
||||
after := snapshotCIAutoFixScope(t, store, "task-disabled")
|
||||
requireCIAutoFixScopeUnchanged(t, before, after)
|
||||
})
|
||||
|
||||
t.Run("enabled without attempt", func(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
enabled := true
|
||||
if _, err := store.UpdateTaskPRAutomationOptions(ctx, "task-enabled", "repo-a", 101,
|
||||
TaskPRAutomationOptionsPatch{AutoFixEnabled: &enabled}, false); err != nil {
|
||||
t.Fatalf("seed enabled options: %v", err)
|
||||
}
|
||||
before := snapshotCIAutoFixScope(t, store, "task-enabled")
|
||||
if err := report(store, "task-enabled", "session-current", "turn-current"); !errors.Is(err, ErrTaskCIAutoFixAttemptNotFound) {
|
||||
t.Fatalf("report error = %v, want %v", err, ErrTaskCIAutoFixAttemptNotFound)
|
||||
}
|
||||
after := snapshotCIAutoFixScope(t, store, "task-enabled")
|
||||
requireCIAutoFixScopeUnchanged(t, before, after)
|
||||
})
|
||||
|
||||
t.Run("foreign bound attempt across differently configured PRs", func(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
enabled := true
|
||||
disabled := false
|
||||
for _, target := range []struct {
|
||||
repositoryID string
|
||||
prNumber int
|
||||
value *bool
|
||||
}{
|
||||
{repositoryID: "repo-a", prNumber: 101, value: &enabled},
|
||||
{repositoryID: "repo-b", prNumber: 202, value: &disabled},
|
||||
} {
|
||||
if _, err := store.UpdateTaskPRAutomationOptions(ctx, "task-foreign", target.repositoryID, target.prNumber,
|
||||
TaskPRAutomationOptionsPatch{AutoFixEnabled: target.value}, false); err != nil {
|
||||
t.Fatalf("seed %s#%d options: %v", target.repositoryID, target.prNumber, err)
|
||||
}
|
||||
}
|
||||
if err := store.RecordTaskCIFixAttempt(ctx, TaskCIFixAttempt{
|
||||
TaskID: "task-foreign", RepositoryID: "repo-a", PRNumber: 101,
|
||||
Signature: "foreign-feedback", CheckpointJSON: `{}`,
|
||||
SessionID: "session-foreign", TurnID: "turn-foreign",
|
||||
ProviderGeneration: "head-foreign", State: TaskCIAutoFixAttemptRunning,
|
||||
IncrementRound: true, EnqueuedAt: time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC),
|
||||
}); err != nil {
|
||||
t.Fatalf("seed foreign attempt: %v", err)
|
||||
}
|
||||
before := snapshotCIAutoFixScope(t, store, "task-foreign")
|
||||
if err := report(store, "task-foreign", "session-current", "turn-current"); !errors.Is(err, ErrTaskCIAutoFixAttemptNotFound) {
|
||||
t.Fatalf("report error = %v, want %v", err, ErrTaskCIAutoFixAttemptNotFound)
|
||||
}
|
||||
after := snapshotCIAutoFixScope(t, store, "task-foreign")
|
||||
requireCIAutoFixScopeUnchanged(t, before, after)
|
||||
})
|
||||
|
||||
t.Run("already reported attempt", func(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
attempt := TaskCIFixAttempt{
|
||||
TaskID: "task-reported", RepositoryID: "repo-a", PRNumber: 101,
|
||||
Signature: "reported-feedback", CheckpointJSON: `{}`,
|
||||
SessionID: "session-current", TurnID: "turn-current",
|
||||
ProviderGeneration: "head-reported", State: TaskCIAutoFixAttemptRunning,
|
||||
IncrementRound: true,
|
||||
}
|
||||
if err := store.RecordTaskCIFixAttempt(ctx, attempt); err != nil {
|
||||
t.Fatalf("seed reported attempt: %v", err)
|
||||
}
|
||||
if err := store.ReportTaskCIAutoFixOutcome(ctx, TaskCIAutoFixOutcomeReport{
|
||||
TaskID: attempt.TaskID, SessionID: attempt.SessionID, TurnID: attempt.TurnID,
|
||||
Outcome: TaskCIAutoFixOutcomeNonActionable, Summary: "first report",
|
||||
}); err != nil {
|
||||
t.Fatalf("record first outcome: %v", err)
|
||||
}
|
||||
before := snapshotCIAutoFixScope(t, store, attempt.TaskID)
|
||||
if err := store.ReportTaskCIAutoFixOutcome(ctx, TaskCIAutoFixOutcomeReport{
|
||||
TaskID: attempt.TaskID, SessionID: attempt.SessionID, TurnID: attempt.TurnID,
|
||||
Outcome: TaskCIAutoFixOutcomeActionTaken, Summary: "duplicate report",
|
||||
}); !errors.Is(err, ErrTaskCIAutoFixAttemptNotFound) {
|
||||
t.Fatalf("duplicate report error = %v, want %v", err, ErrTaskCIAutoFixAttemptNotFound)
|
||||
}
|
||||
after := snapshotCIAutoFixScope(t, store, attempt.TaskID)
|
||||
requireCIAutoFixScopeUnchanged(t, before, after)
|
||||
})
|
||||
}
|
||||
|
||||
// @covers AC-UI-CI-PR-AUTOMATION-001.10
|
||||
func TestStorePRAutoFixOutcomeAcceptsAllValidOutcomes(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
outcome TaskCIAutoFixOutcome
|
||||
wantState TaskCIAutoFixAttemptState
|
||||
wantLastError bool
|
||||
}{
|
||||
{name: "action taken", outcome: TaskCIAutoFixOutcomeActionTaken, wantState: TaskCIAutoFixAttemptAwaitingProviderProgress},
|
||||
{name: "non actionable", outcome: TaskCIAutoFixOutcomeNonActionable, wantState: TaskCIAutoFixAttemptAcknowledged},
|
||||
{name: "blocked", outcome: TaskCIAutoFixOutcomeBlocked, wantState: TaskCIAutoFixAttemptAcknowledged, wantLastError: true},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
attempt := TaskCIFixAttempt{
|
||||
TaskID: "task-valid-" + test.name, RepositoryID: "repo-valid", PRNumber: 303,
|
||||
Signature: "valid-feedback", CheckpointJSON: `{}`,
|
||||
SessionID: "session-valid", TurnID: "turn-valid",
|
||||
ProviderGeneration: "head-valid", State: TaskCIAutoFixAttemptRunning,
|
||||
}
|
||||
if err := store.RecordTaskCIFixAttempt(ctx, attempt); err != nil {
|
||||
t.Fatalf("seed attempt: %v", err)
|
||||
}
|
||||
if err := store.ReportTaskCIAutoFixOutcome(ctx, TaskCIAutoFixOutcomeReport{
|
||||
TaskID: attempt.TaskID, SessionID: attempt.SessionID, TurnID: attempt.TurnID,
|
||||
Outcome: test.outcome, Summary: "valid outcome",
|
||||
}); err != nil {
|
||||
t.Fatalf("record %s outcome: %v", test.outcome, err)
|
||||
}
|
||||
state, err := store.GetTaskCIPRState(ctx, attempt.TaskID, attempt.RepositoryID, attempt.PRNumber)
|
||||
if err != nil {
|
||||
t.Fatalf("get state: %v", err)
|
||||
}
|
||||
if state == nil {
|
||||
t.Fatal("attempt state is missing")
|
||||
}
|
||||
if state.AutoFixAttemptState != test.wantState || state.AutoFixAttemptOutcome != test.outcome {
|
||||
t.Fatalf("state = %+v, want state %q and outcome %q", state, test.wantState, test.outcome)
|
||||
}
|
||||
if (state.LastError != nil) != test.wantLastError {
|
||||
t.Fatalf("last error presence = %t, want %t", state.LastError != nil, test.wantLastError)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -29,6 +29,8 @@ type TaskPRAutoFixOutcomeService interface {
|
||||
ReportTaskPRAutoFixOutcome(ctx context.Context, taskID, sessionID, outcome, summary string) error
|
||||
}
|
||||
|
||||
const taskPRAutoFixOutcomeUnmatchedMessage = "No matching unresolved GitHub PR auto-fix attempt exists for this turn. Finish ordinary work without retrying this report or enabling auto-fix."
|
||||
|
||||
func (h *Handlers) SetTaskPRAutomationService(automation TaskPRAutomationService) {
|
||||
h.taskPRAutomation = automation
|
||||
}
|
||||
@@ -153,8 +155,9 @@ func (h *Handlers) handleReportTaskPRAutoFixOutcome(ctx context.Context, msg *ws
|
||||
ctx, req.TaskID, req.SessionID, req.Outcome, req.Summary,
|
||||
); err != nil {
|
||||
switch {
|
||||
case errors.Is(err, github.ErrTaskCIAutoFixAttemptNotFound),
|
||||
errors.Is(err, github.ErrTaskCIAutoFixOutcomeInvalid):
|
||||
case errors.Is(err, github.ErrTaskCIAutoFixAttemptNotFound):
|
||||
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, taskPRAutoFixOutcomeUnmatchedMessage, nil)
|
||||
case errors.Is(err, github.ErrTaskCIAutoFixOutcomeInvalid):
|
||||
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil)
|
||||
default:
|
||||
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to record PR auto-fix outcome: "+err.Error(), nil)
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/github"
|
||||
ws "github.com/kandev/kandev/pkg/websocket"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestReportPRAutoFixOutcomeExplainsUnmatchedTurn(t *testing.T) {
|
||||
automation := &recordingTaskPRAutomationService{outcomeErr: github.ErrTaskCIAutoFixAttemptNotFound}
|
||||
h := &Handlers{taskPRAutoFixOutcome: automation, logger: testLogger(t).WithFields()}
|
||||
|
||||
response, err := h.handleReportTaskPRAutoFixOutcome(
|
||||
taskPRAutoFixOutcomeContext(),
|
||||
makeWSMessage(t, ws.ActionMCPReportPRAutoFixOutcome, map[string]any{
|
||||
"outcome": "blocked",
|
||||
"summary": "provider is unavailable",
|
||||
}),
|
||||
)
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, ws.MessageTypeError, response.Type)
|
||||
var payload ws.ErrorPayload
|
||||
require.NoError(t, response.ParsePayload(&payload))
|
||||
assert.Equal(t, ws.ErrorCodeValidation, payload.Code)
|
||||
assert.Contains(t, payload.Message, "No matching unresolved GitHub PR auto-fix attempt exists for this turn")
|
||||
assert.Contains(t, payload.Message, "Finish ordinary work without retrying this report or enabling auto-fix")
|
||||
assert.NotContains(t, strings.ToLower(payload.Message), "disabled")
|
||||
}
|
||||
@@ -203,7 +203,7 @@ func TestHandleReportTaskPRAutoFixOutcomeRejectsInvalidAndStaleReports(t *testin
|
||||
}))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, ws.MessageTypeError, response.Type)
|
||||
assert.Contains(t, string(response.Payload), "not found")
|
||||
assert.Contains(t, string(response.Payload), "Finish ordinary work without retrying this report or enabling auto-fix")
|
||||
}
|
||||
|
||||
func TestHandleReportTaskPRAutoFixOutcomeRejectsSpoofedIdentity(t *testing.T) {
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
package mcp
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/mark3labs/mcp-go/mcp"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestReportPRAutoFixOutcomeToolDescriptionScopesCurrentTurn(t *testing.T) {
|
||||
s := newTaskModeServer(t, &testBackend{}, "task-current")
|
||||
|
||||
tool, ok := s.mcpServer.ListTools()["report_pr_auto_fix_outcome_kandev"]
|
||||
require.True(t, ok, "report outcome tool must be registered")
|
||||
description := tool.Tool.Description
|
||||
|
||||
assert.Contains(t, description, "current Kandev-dispatched auto-fix turn")
|
||||
assert.Contains(t, description, "server-owned outcome protocol")
|
||||
assert.Contains(t, description, "manual PR fixup")
|
||||
assert.Contains(t, description, "sibling review")
|
||||
assert.Contains(t, description, "earlier auto-fix turn")
|
||||
assert.Contains(t, description, "Tool availability or enabled automation settings alone do not establish an obligation to report")
|
||||
assert.Contains(t, description, "exactly once")
|
||||
}
|
||||
|
||||
func TestReportPRAutoFixOutcomeServerSurfacesBackendError(t *testing.T) {
|
||||
// Keep this transport fixture synchronized with handlers.taskPRAutoFixOutcomeUnmatchedMessage.
|
||||
const explanation = "No matching unresolved GitHub PR auto-fix attempt exists for this turn. Finish ordinary work without retrying this report or enabling auto-fix."
|
||||
backend := &testBackend{err: errors.New(explanation)}
|
||||
s := newTaskModeServer(t, backend, "task-current")
|
||||
|
||||
result := callTool(t, s, "report_pr_auto_fix_outcome_kandev", map[string]interface{}{
|
||||
"outcome": "blocked",
|
||||
"summary": "provider is unavailable",
|
||||
})
|
||||
|
||||
require.True(t, result.IsError)
|
||||
require.Len(t, result.Content, 1)
|
||||
content, ok := result.Content[0].(mcp.TextContent)
|
||||
require.True(t, ok)
|
||||
assert.Contains(t, content.Text, explanation)
|
||||
}
|
||||
@@ -1371,10 +1371,14 @@ func (s *Server) registerPRAutomationTools() {
|
||||
s.mcpServer.AddTool(
|
||||
mcp.NewTool("report_pr_auto_fix_outcome_kandev",
|
||||
mcp.WithDescription(
|
||||
"Report the one explicit outcome for the current GitHub PR auto-fix turn. "+
|
||||
"Report the one explicit outcome for the current Kandev-dispatched auto-fix turn. "+
|
||||
"Use this tool only when this turn received the server-owned outcome protocol. "+
|
||||
"Do not use it for manual PR fixup, sibling review messages, or instructions carried over from an earlier auto-fix turn. "+
|
||||
"Tool availability or enabled automation settings alone do not establish an obligation to report. "+
|
||||
"Use action_taken when a concrete provider-visible change was made, "+
|
||||
"non_actionable when the feedback identifies no change this task can make, "+
|
||||
"or blocked when an external condition prevents the needed change. "+
|
||||
"Report exactly once for the dispatched turn. "+
|
||||
"The task, session, turn, and PR are bound by Kandev and are not tool arguments.",
|
||||
),
|
||||
mcp.WithString("outcome", mcp.Required(), mcp.Enum("action_taken", "non_actionable", "blocked"), mcp.Description("The disposition of this auto-fix turn.")),
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
package orchestrator
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/sysprompt"
|
||||
)
|
||||
|
||||
func TestCIAutomationOutcomeProtocolScopesCurrentTurn(t *testing.T) {
|
||||
for _, passthrough := range []bool{false, true} {
|
||||
t.Run(map[bool]string{false: "structured", true: "passthrough"}[passthrough], func(t *testing.T) {
|
||||
prompt := ciAutomationAppendOutcomeProtocol("repair the PR", passthrough)
|
||||
visible := prompt
|
||||
if !passthrough {
|
||||
visible = sysprompt.StripSystemContent(prompt)
|
||||
}
|
||||
|
||||
assertions := []string{
|
||||
"current Kandev-dispatched auto-fix turn",
|
||||
"expires when this turn ends",
|
||||
"manual PR fixup",
|
||||
"sibling review",
|
||||
"historical auto-fix instructions",
|
||||
"Tool availability or enabled automation settings alone do not establish this obligation",
|
||||
"report_pr_auto_fix_outcome_kandev exactly once",
|
||||
}
|
||||
for _, want := range assertions {
|
||||
if !strings.Contains(prompt, want) {
|
||||
t.Errorf("protocol does not contain %q: %s", want, visible)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -1203,10 +1203,12 @@ func ciAutomationRenderPromptTemplate(base, snapshot string) string {
|
||||
}
|
||||
|
||||
const ciAutomationOutcomeProtocol = `Kandev PR auto-fix outcome protocol:
|
||||
This instruction applies only to the current Kandev-dispatched auto-fix turn that received this protocol. It expires when this turn ends.
|
||||
Before this turn ends, call report_pr_auto_fix_outcome_kandev exactly once with one of these outcomes:
|
||||
- action_taken: you made or requested a concrete provider-visible change and want Kandev to wait for CI or PR progress.
|
||||
- non_actionable: the current feedback does not identify a change this task can make.
|
||||
- blocked: a concrete change is needed, but an external condition prevents it. Include a short reason.
|
||||
Do not report an outcome for manual PR fixup, sibling review messages, or historical auto-fix instructions. Tool availability or enabled automation settings alone do not establish this obligation.
|
||||
Do not claim action_taken from a plan or an attempted command alone. If the tool is unavailable, continue the repair work and explain that the outcome could not be recorded.`
|
||||
|
||||
func ciAutomationAppendOutcomeProtocol(prompt string, passthrough bool) string {
|
||||
|
||||
@@ -0,0 +1,168 @@
|
||||
---
|
||||
created: 2026-09-11
|
||||
status: complete
|
||||
requirements:
|
||||
- REQ-UI-CI-PR-AUTOMATION-001
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/ci-pr-automation-02.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Fix plan: PR auto-fix outcome scope
|
||||
|
||||
## Overview
|
||||
|
||||
Clarify when an agent must report a PR auto-fix outcome.
|
||||
First correct the tool and dispatch guidance. Then explain rejected calls and
|
||||
prove that ordinary turns cannot change auto-fix state.
|
||||
The implementation applies the approved guidance and rejection-scope package
|
||||
without changing automation eligibility or retry behavior.
|
||||
|
||||
## Diagnosis and evidence
|
||||
|
||||
Affected task: `93a74cfc-6135-452e-85d1-ccd6d7af78cd`, linked PR #3590.
|
||||
Session: `2edf3b96-66b5-4409-9238-36c97f4d3a63`.
|
||||
Turn: `71c201a0-4115-4ca3-bb61-bdc60dd48d6c`.
|
||||
|
||||
- The original user requested implementation, PR creation, a 15-minute wait,
|
||||
and manual PR fixup. This request did not enable automatic PR repair.
|
||||
- The affected turn started at `2026-09-11T09:28:21Z` from a sibling review
|
||||
message, `9f209c9c-d7d3-54b6-bee4-f3b6a7501a2d`.
|
||||
- At `2026-09-11T10:12:12Z`, message
|
||||
`5865c632-ea68-4140-a507-6a4b6391180d` called
|
||||
`report_pr_auto_fix_outcome_kandev` with `action_taken`.
|
||||
Its summary described manual review fixes and head `46ce139`.
|
||||
- The tool returned `VALIDATION_ERROR` with
|
||||
`CI auto-fix attempt not found or no longer matches`.
|
||||
- Read-only inspection of `/root/.kandev/data/kandev.db` found zero automation
|
||||
option rows and zero attempt-state rows for this task. PR #3590 remained linked.
|
||||
The legacy task settings row was absent too. `GetTaskPRAutomationOptions`
|
||||
returns disabled defaults for a missing row.
|
||||
- The recorded turn metadata had no auto-fix binding. The complete primary
|
||||
conversation contained the original user prompt and sibling review prompt,
|
||||
with no Kandev auto-fix dispatch message.
|
||||
|
||||
Current database state alone cannot prove every historical switch value.
|
||||
The transcript, turn record, rejected call, and disabled defaults agree with the
|
||||
user's report. No evidence indicates a scheduler dispatch with auto-fix disabled.
|
||||
No settings, sessions, GitHub objects, or live records changed during diagnosis.
|
||||
|
||||
## Root cause
|
||||
|
||||
The agent used an automatic-attempt reporting tool to summarize manual fixup.
|
||||
`registerPRAutomationTools` exposes the tool to GitHub-capable task sessions,
|
||||
independent of automation settings or an active attempt. Its description names
|
||||
an auto-fix turn but does not explicitly exclude ordinary PR work.
|
||||
The existing dispatch protocol also lacks an explicit expiry across later turns.
|
||||
That latter omission is a preventive concern, not a proven trigger in this incident.
|
||||
|
||||
The conditional store write correctly rejected the call. This repair addresses
|
||||
agent guidance and error recovery, not an observed authorization bypass.
|
||||
The exact internal reason for the model's choice is not independently provable.
|
||||
|
||||
## Requirement reconciliation
|
||||
|
||||
`AC-UI-CI-PR-AUTOMATION-001.10` already requires exact turn binding.
|
||||
The implementation preserved that boundary in this incident.
|
||||
Criteria `.16` and `.17` clarify guidance and rejection behavior that were missing.
|
||||
The existing UI-owned automation pair remains authoritative during migration.
|
||||
The integration index owns adjacent provider signals, not a second copy of this
|
||||
attempt contract. No new ADR is needed because the existing outcome ADR already
|
||||
separates ordinary turns from automatic attempts.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Explicit current-turn eligibility in tool and immutable dispatch guidance.
|
||||
- A specific explanation for an unmatched outcome report.
|
||||
- Regression coverage for valid and invalid report contexts.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Dynamic catalog removal, a new eligibility endpoint, or session restarts.
|
||||
- Automatic settings changes or conversion of manual work into an attempt.
|
||||
- Changes to retry counts, provider progress, queue delivery, or GitLab behavior.
|
||||
- UI layout, translated browser copy, and modification of repository skills.
|
||||
- Repairing or changing the affected task or PR.
|
||||
|
||||
## Technical approach
|
||||
|
||||
Keep the existing provider-scoped catalog and backend identity checks.
|
||||
Update the reporting tool description in `internal/mcp/server/server.go`.
|
||||
Update `ciAutomationOutcomeProtocol` in
|
||||
`internal/orchestrator/event_handlers_github_ci_automation.go`.
|
||||
Map the missing-attempt sentinel separately in
|
||||
`internal/mcp/handlers/task_pr_automation.go`.
|
||||
Retain the sentinel, error code, schema, and all failure semantics.
|
||||
|
||||
Proposed rejection meaning: no matching unresolved auto-fix attempt exists for
|
||||
this turn. Use this tool only for a current Kandev auto-fix dispatch.
|
||||
For ordinary PR work, finish normally without retrying this report or enabling
|
||||
auto-fix. Do not claim that the switch is off from this error alone.
|
||||
|
||||
## Tests
|
||||
|
||||
| Criteria | Planned evidence |
|
||||
| --- | --- |
|
||||
| `.16` | `TestReportPRAutoFixOutcomeToolDescriptionScopesCurrentTurn` in new `internal/mcp/server/pr_auto_fix_outcome_scope_test.go` |
|
||||
| `.16` | `TestCIAutomationOutcomeProtocolScopesCurrentTurn` in new `internal/orchestrator/ci_automation_outcome_scope_test.go` |
|
||||
| `.17` | `TestReportPRAutoFixOutcomeExplainsUnmatchedTurn` in new `internal/mcp/handlers/task_pr_automation_outcome_scope_test.go` |
|
||||
| `.10`, `.17` | `TestStorePRAutoFixOutcomeOrdinaryTurnHasNoSideEffects` in new `internal/github/store_ci_outcome_scope_test.go` |
|
||||
| `.10` | `TestStorePRAutoFixOutcomeAcceptsAllValidOutcomes` in new `internal/github/store_ci_outcome_scope_test.go` |
|
||||
|
||||
The first three tests must fail before the planned correction.
|
||||
The store matrix protects the already-working rejection boundary.
|
||||
Exercise a real temporary SQLite store for state assertions.
|
||||
Cover absent and disabled settings, enabled-without-attempt, another turn's
|
||||
attempt, an already-reported attempt, and all three valid outcomes.
|
||||
Include two PRs with different settings and a foreign bound attempt.
|
||||
|
||||
## End-to-end evidence
|
||||
|
||||
The observable surface is the agent tool protocol. Use the existing in-process
|
||||
MCP client pattern (`callTool`) to verify the returned error and caller binding.
|
||||
Store tests prove persistent effects. No browser rendering changes require
|
||||
Playwright, and deterministic tests do not claim to prove every model choice.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Scope agent reporting guidance](task-01-scope-reporting-guidance.md)
|
||||
- [x] [Task 02: Explain unmatched outcome reports](task-02-explain-unmatched-reports.md)
|
||||
|
||||
Execute sequentially. The existing
|
||||
[outcome retry package](../pr-auto-fix-outcome-retries/plan.md) is implemented.
|
||||
This package adds clarification without reopening or rewriting its results.
|
||||
|
||||
## Verification results
|
||||
|
||||
Implementation: complete. Both work orders are done.
|
||||
|
||||
- `rtk python3 scripts/lint-spec-files.test.py`: passed, 36 tests.
|
||||
- `rtk python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `rtk git diff --check`: passed.
|
||||
- `rtk go test ./internal/mcp/handlers -run 'Test.*Report.*PRAutoFix' -count=1`: passed, 7 tests.
|
||||
- `rtk go test ./internal/mcp/server -run 'TestReportPRAutoFixOutcome|TestTaskPRAutomationTools' -count=1`: passed, 5 tests.
|
||||
- `rtk go test ./internal/github -run 'TestStorePRAutoFixOutcomeOrdinaryTurnHasNoSideEffects|TestStoreTaskCIAutoFixAttempt' -count=1`: passed, 9 tests.
|
||||
- `rtk go test ./internal/orchestrator -run 'TestCIAutomationOutcomeProtocol' -count=1`: passed, 4 tests.
|
||||
- `rtk make -C apps/backend build`: passed; the environment reported only the existing missing codesign/rcodesign warning for Darwin artifacts.
|
||||
- `rtk make -C apps/backend test`: the changed packages passed, but the full suite exited 2 in unrelated existing areas: process-tree probing, config/home discovery, launcher service configuration, and an Office SQLite migration.
|
||||
- `rtk gofmt -l` on all changed Go files: passed.
|
||||
- Work-order references and existing code symbols checked against the workspace.
|
||||
|
||||
## Risks
|
||||
|
||||
- Clear guidance reduces misuse but cannot guarantee a model never calls the tool.
|
||||
The backend guard remains mandatory.
|
||||
- A static catalog must remain usable when a later auto-fix turn reuses a session.
|
||||
- Historical instructions can remain in model context. The protocol must state
|
||||
its current-turn scope without weakening reporting for valid dispatches.
|
||||
- A missing attempt can mean stale or completed work, not only disabled settings.
|
||||
|
||||
## Documentation impact
|
||||
|
||||
Implementation changes are limited to agent-facing backend guidance, rejection
|
||||
classification, regression coverage, and the delivery record. Public guidance
|
||||
remains accurate because automatic retries and controls do not change. The
|
||||
implementation retains the distinction between manual fixup and
|
||||
Kandev-dispatched auto-fix in agent-facing text.
|
||||
@@ -0,0 +1,93 @@
|
||||
---
|
||||
id: "01-scope-reporting-guidance"
|
||||
title: "Scope agent reporting guidance"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-CI-PR-AUTOMATION-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-CI-PR-AUTOMATION-001.16
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/ci-pr-automation-02.md
|
||||
---
|
||||
|
||||
# Task 01: Scope agent reporting guidance
|
||||
|
||||
## Summary
|
||||
|
||||
Explain the current-turn reporting requirement at discovery and dispatch.
|
||||
Keep the GitHub task catalog stable across manual and automatic turns.
|
||||
|
||||
## In scope
|
||||
|
||||
- Tool description exclusions for manual fixup, sibling review, and historical instructions.
|
||||
- Explicit expiry of the immutable protocol at the end of its dispatched turn.
|
||||
- Structured, passthrough, and customized-prompt coverage.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Catalog filtering, new tool arguments, setting changes, and retry logic.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Discovery explains that availability or enabled settings do not establish a reporting obligation.
|
||||
- A valid current auto-fix dispatch still requires exactly one outcome.
|
||||
- Prior-turn protocol text does not instruct later manual turns to report.
|
||||
|
||||
## TDD sequence
|
||||
|
||||
Add the two guidance tests named in the plan. Run them and record the expected failures.
|
||||
Then change the two instruction surfaces and rerun the tests.
|
||||
Retain existing provider membership and protocol visibility coverage.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from the repository root:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && rtk go test ./internal/mcp/server -run 'TestReportPRAutoFixOutcome|TestTaskPRAutomationTools|TestServerModeTask_ProviderMembership' -count=1)
|
||||
(cd apps/backend && rtk go test ./internal/orchestrator -run 'TestCIAutomationOutcomeProtocol' -count=1)
|
||||
rtk python3 scripts/lint-spec-files.py --all
|
||||
rtk git diff --check
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/mcp/server/server.go`
|
||||
- `apps/backend/internal/mcp/server/pr_auto_fix_outcome_scope_test.go` (new)
|
||||
- `apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go`
|
||||
- `apps/backend/internal/orchestrator/ci_automation_outcome_scope_test.go` (new)
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
Overly broad exclusions can suppress legitimate automatic reports.
|
||||
Assert the positive reporting instruction as well as its scope.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- Requirement `.16` and design section `Outcome reporting scope`.
|
||||
- `TestCIAutomationOutcomeProtocolVisibility` and `newTaskModeServer`.
|
||||
- [Incident evidence and test matrix](plan.md).
|
||||
|
||||
## Results
|
||||
|
||||
Done. The GitHub outcome tool description now scopes reporting to the current
|
||||
Kandev-dispatched auto-fix turn and excludes manual fixup, sibling review, and
|
||||
historical instructions. The immutable structured and passthrough dispatch
|
||||
protocols state the same scope and expiry. Existing provider membership and
|
||||
positive outcome visibility remain covered.
|
||||
|
||||
- `rtk go test ./internal/mcp/server -run 'TestReportPRAutoFixOutcome|TestTaskPRAutomationTools|TestServerModeTask_ProviderMembership' -count=1`: passed, 10 tests.
|
||||
- `rtk go test ./internal/orchestrator -run 'TestCIAutomationOutcomeProtocol' -count=1`: passed, 4 tests.
|
||||
- `rtk python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `rtk git diff --check`: passed.
|
||||
@@ -0,0 +1,104 @@
|
||||
---
|
||||
id: "02-explain-unmatched-reports"
|
||||
title: "Explain unmatched outcome reports"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on:
|
||||
- "01-scope-reporting-guidance"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-CI-PR-AUTOMATION-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-CI-PR-AUTOMATION-001.10
|
||||
- AC-UI-CI-PR-AUTOMATION-001.17
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/ci-pr-automation-02.md
|
||||
---
|
||||
|
||||
# Task 02: Explain unmatched outcome reports
|
||||
|
||||
## Summary
|
||||
|
||||
Return a scope explanation when the backend cannot match an unresolved attempt.
|
||||
Prove that rejected reports leave automation settings and attempt state unchanged.
|
||||
|
||||
## In scope
|
||||
|
||||
- A separate error mapping for `ErrTaskCIAutoFixAttemptNotFound`.
|
||||
- Handler and MCP-client coverage for rejection guidance and `VALIDATION_ERROR`.
|
||||
- Real-store regression coverage for the context matrix in the plan.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Successful no-op responses, synthetic attempts, schema changes, and automatic retries.
|
||||
- Changing the conditional store write or caller identity contract.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The error explains scope without asserting that auto-fix is disabled.
|
||||
- Invalid reports create no attempt, consume no round, and change no settings.
|
||||
- Valid bound reports retain all three outcomes and reject duplicate or foreign-turn reports.
|
||||
|
||||
## TDD sequence
|
||||
|
||||
First add `TestReportPRAutoFixOutcomeExplainsUnmatchedTurn` and record its expected failure.
|
||||
Add the state matrix through the real temporary store.
|
||||
Change only the missing-attempt error mapping, then run all commands below.
|
||||
Extend the MCP client test to prove the explanation survives the tool transport.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from the repository root:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && rtk go test ./internal/mcp/handlers -run 'Test.*Report.*PRAutoFix' -count=1)
|
||||
(cd apps/backend && rtk go test ./internal/mcp/server -run 'TestReportPRAutoFixOutcome|TestTaskPRAutomationTools' -count=1)
|
||||
(cd apps/backend && rtk go test ./internal/github -run 'TestStorePRAutoFixOutcome|TestStoreTaskCIAutoFixAttempt' -count=1)
|
||||
rtk python3 scripts/lint-spec-files.test.py
|
||||
rtk python3 scripts/lint-spec-files.py --all
|
||||
rtk git diff --check
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/mcp/handlers/task_pr_automation.go`
|
||||
- `apps/backend/internal/mcp/handlers/task_pr_automation_outcome_scope_test.go` (new)
|
||||
- `apps/backend/internal/mcp/server/pr_auto_fix_outcome_scope_test.go`
|
||||
- `apps/backend/internal/github/store_ci_outcome_scope_test.go` (new)
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01 owns the shared new MCP test file and the guidance terminology.
|
||||
|
||||
## Risks
|
||||
|
||||
The sentinel covers missing, stale, and completed attempts.
|
||||
The explanation must not recommend changing settings to repair a reporting error.
|
||||
State assertions must compare rows before and after rejected calls, including absent rows.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- Criteria `.10` and `.17`, and design section `Outcome reporting scope`.
|
||||
- `handleReportTaskPRAutoFixOutcome`, `ReportTaskCIAutoFixOutcome`, and existing handler tests.
|
||||
- `TestStoreTaskCIAutoFixAttemptLifecycleUsesExactIdentity`.
|
||||
- [Incident evidence and test matrix](plan.md).
|
||||
|
||||
## Results
|
||||
|
||||
Done. Unmatched auto-fix outcome reports now keep `VALIDATION_ERROR` while
|
||||
returning a scope explanation that covers missing, stale, and completed turns.
|
||||
The explanation directs ordinary work to continue without retrying the report
|
||||
or changing automation settings. The MCP transport and temporary-SQLite tests
|
||||
prove that the existing task/session/turn guard and all three valid outcomes
|
||||
remain intact.
|
||||
|
||||
- `rtk go test ./internal/mcp/handlers -run 'Test.*Report.*PRAutoFix' -count=1`: passed, 7 tests.
|
||||
- `rtk go test ./internal/mcp/server -run 'TestReportPRAutoFixOutcome|TestTaskPRAutomationTools' -count=1`: passed, 5 tests.
|
||||
- `rtk go test ./internal/github -run 'TestStorePRAutoFixOutcomeOrdinaryTurnHasNoSideEffects|TestStoreTaskCIAutoFixAttempt' -count=1`: passed, 9 tests.
|
||||
- `rtk python3 scripts/lint-spec-files.test.py`: passed, 36 tests.
|
||||
- `rtk python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `rtk git diff --check`: passed.
|
||||
@@ -2,7 +2,7 @@
|
||||
status: active
|
||||
system: ui
|
||||
created: 2026-06-18
|
||||
updated: 2026-09-06
|
||||
updated: 2026-09-11
|
||||
owners:
|
||||
- tbd
|
||||
---
|
||||
@@ -64,6 +64,19 @@ Consequences section).
|
||||
- **AC-UI-CI-PR-AUTOMATION-001.14:** Every retry shall consume the existing per-PR 10-round budget. Kandev shall never create an 11th accepted round, and disabling then re-enabling auto-fix shall reset the retry state with the existing checkpoint and round state.
|
||||
- **AC-UI-CI-PR-AUTOMATION-001.15:** Existing installations whose built-in `ci-auto-fix` prompt still matches a shipped, untouched legacy revision shall receive the current default content on startup. Kandev shall preserve edited prompt rows, and the server-owned outcome instructions shall apply even when a task or global prompt is customized.
|
||||
|
||||
## System design
|
||||
### Outcome scope clarification
|
||||
|
||||
The following criteria clarify the existing turn-bound outcome contract.
|
||||
The [scope repair plan](../../../plans/pr-auto-fix-outcome-scope/plan.md) records the implementation and regression coverage.
|
||||
|
||||
- **AC-UI-CI-PR-AUTOMATION-001.16:** Agent guidance shall restrict outcome reporting to the current Kandev-dispatched auto-fix turn. Manual PR fixup, sibling review messages, and historical auto-fix instructions shall not establish that obligation. Tool availability and enabled automation settings alone shall not establish that obligation.
|
||||
- **AC-UI-CI-PR-AUTOMATION-001.17:** When no matching unresolved attempt exists, Kandev shall reject the report with a scope explanation. The explanation shall direct the agent to finish ordinary work without retrying the report or enabling automation. The rejected report shall not create an attempt, consume a round, or change automation settings.
|
||||
|
||||
## Implementation plans
|
||||
|
||||
- [Outcome retries](../../../plans/pr-auto-fix-outcome-retries/plan.md): existing implementation.
|
||||
- [Outcome scope repair](../../../plans/pr-auto-fix-outcome-scope/plan.md): implementation record.
|
||||
|
||||
## System-design references
|
||||
|
||||
The migrated technical source is split into [part 1](../system-design/ci-pr-automation-01.md), [part 2](../system-design/ci-pr-automation-02.md), [part 3](../system-design/ci-pr-automation-03.md).
|
||||
|
||||
@@ -4,7 +4,7 @@ system: ui
|
||||
requirements:
|
||||
- REQ-UI-CI-PR-AUTOMATION-001
|
||||
created: 2026-06-18
|
||||
updated: 2026-09-06
|
||||
updated: 2026-09-11
|
||||
owners:
|
||||
- tbd
|
||||
---
|
||||
@@ -144,7 +144,44 @@ Auto-merge cycle for one task/PR:
|
||||
4. If the PR is ready and the readiness signature is new, Kandev calls the existing PR merge operation using the backend default merge-method selection.
|
||||
5. Kandev records the merge attempt and refreshes PR state after a successful merge when practical.
|
||||
|
||||
## Permissions
|
||||
## Outcome reporting scope
|
||||
|
||||
This clarification maps to `AC-UI-CI-PR-AUTOMATION-001.10`,
|
||||
`AC-UI-CI-PR-AUTOMATION-001.16`, and `AC-UI-CI-PR-AUTOMATION-001.17`.
|
||||
Its delivery record is the [scope repair plan](../../../plans/pr-auto-fix-outcome-scope/plan.md).
|
||||
|
||||
`registerPRAutomationTools` exposes the reporting tool through the GitHub task
|
||||
catalog. Catalog membership describes capability, not an active auto-fix attempt.
|
||||
The catalog remains stable across ordinary and automatic turns in one session.
|
||||
|
||||
The tool description must require a current Kandev auto-fix prompt with the
|
||||
server-owned outcome protocol. It must explicitly exclude manual PR fixup and
|
||||
sibling review work. A setting, a quoted marker, or an earlier turn's protocol
|
||||
does not prove that the current turn owns an attempt.
|
||||
|
||||
`ciAutomationOutcomeProtocol` must state that its reporting instruction applies
|
||||
only to the turn receiving that dispatch. Structured and passthrough sessions
|
||||
retain their existing transport behavior. Custom prompts cannot remove this scope.
|
||||
|
||||
`Service.ReportTaskPRAutoFixOutcome` continues to resolve the active turn.
|
||||
`Store.ReportTaskCIAutoFixOutcome` remains the authoritative conditional write.
|
||||
The exact task, session, turn, running state, and empty outcome must match.
|
||||
No caller-supplied identity, new attempt, or success fallback is introduced.
|
||||
|
||||
`handleReportTaskPRAutoFixOutcome` must map
|
||||
`ErrTaskCIAutoFixAttemptNotFound` to a specific scope explanation while retaining
|
||||
`VALIDATION_ERROR`. The explanation covers missing, stale, and completed attempts
|
||||
without claiming that auto-fix is disabled. It directs the agent to finish ordinary
|
||||
work without another report or a settings change. Other validation and internal
|
||||
errors retain their existing classification.
|
||||
|
||||
Regression coverage must exercise missing settings, explicit disabled settings,
|
||||
an enabled PR without an attempt, and a valid bound attempt. A mixed-PR case must
|
||||
prove that an attempt on another session or turn cannot authorize this call.
|
||||
No UI composition, persistence schema, retry policy, or provider catalog changes
|
||||
are required.
|
||||
|
||||
## Access permissions
|
||||
|
||||
- Any user who can view and interact with the task chat can read and update the task CI automation options for that task.
|
||||
- Any user who can edit prompts in Settings > Prompts can edit the default `ci-auto-fix` prompt.
|
||||
|
||||
Reference in New Issue
Block a user