fix(gitlab): scope MR automation switches per linked MR (#2676)

Co-authored-by: ayattara <alassane.yattara@savoirfairelinux.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Alassane
2026-08-22 14:44:56 +00:00
committed by GitHub
parent 6d7e4e8698
commit ecda06748e
58 changed files with 3821 additions and 711 deletions
+3
View File
@@ -155,3 +155,6 @@ apps/web/e2e/.auth-screenshots/
# Restart-action screenshot artifacts (generated by feature-toggles-restart.spec.ts)
apps/web/e2e/.restart-screenshots/
# Manual dev-environment scratch repos (apps/web/e2e/manual-seed-*.ts)
apps/backend/.manual-env/
@@ -39,6 +39,8 @@ var errUnknownMRAutomationField = errors.New("unknown MR automation field")
// as a bad request.
var errNullMRAutomationSwitch = errors.New("MR automation switch must be a boolean, not null")
var errNullMRAutomationIdentity = errors.New("MR automation identity fields must not be null")
// RegisterMRAutomationHTTPRoutes registers the GET/PATCH MR automation
// endpoints on an existing /api/v1/gitlab router group.
func (c *Controller) RegisterMRAutomationHTTPRoutes(api *gin.RouterGroup) {
@@ -74,6 +76,12 @@ func (c *Controller) httpPatchTaskMRAutomation(ctx *gin.Context) {
if writeMRAutomationTaskNotFound(ctx, err) {
return
}
// A patch naming an MR that isn't linked (or naming one only
// partially) is a caller mistake, not a server fault.
if errors.Is(err, ErrTaskMRNotLinked) {
ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: err.Error()})
return
}
c.logger.Error("update task MR automation failed", zap.String("task_id", ctx.Param("taskID")), zap.Error(err))
ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: "failed to update MR automation options"})
return
@@ -150,6 +158,12 @@ func applyMRAutomationPatchField(patch *TaskMRAutomationPatch, key string, value
return decodeMRAutomationSwitch(value, &patch.PromptOnMerged)
case "prompt_on_closed":
return decodeMRAutomationSwitch(value, &patch.PromptOnClosed)
case "repository_id":
return decodeMRAutomationIdentityString(value, &patch.RepositoryID)
case "project_path":
return decodeMRAutomationIdentityString(value, &patch.ProjectPath)
case "mr_iid":
return decodeMRAutomationIdentityInteger(value, &patch.MRIID)
case "review_prompt_override", "merged_prompt_override", "closed_prompt_override":
return errLifecyclePromptOverridesUnsupported
default:
@@ -187,6 +201,20 @@ func decodeMRAutomationSwitch(value json.RawMessage, dst **bool) error {
return json.Unmarshal(value, dst)
}
func decodeMRAutomationIdentityString(value json.RawMessage, dst **string) error {
if string(value) == "null" {
return errNullMRAutomationIdentity
}
return json.Unmarshal(value, dst)
}
func decodeMRAutomationIdentityInteger(value json.RawMessage, dst **int) error {
if string(value) == "null" {
return errNullMRAutomationIdentity
}
return json.Unmarshal(value, dst)
}
func (c *Controller) publishTaskMRAutomationUpdated(ctx context.Context, resp *TaskMRAutomationResponse) {
if c.service == nil || resp == nil {
return
@@ -20,6 +20,11 @@ func newMRAutomationControllerFixture(t *testing.T) (*gin.Engine, *Service) {
store := newTestStore(t)
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
// The switches are per-MR, so a PATCH only has somewhere to land once the
// task has at least one linked MR.
if err := store.UpsertTaskMR(context.Background(), newTestMR("task-1", "", "group/a", 1)); err != nil {
t.Fatalf("seed linked MR: %v", err)
}
if err := store.SaveConfigForWorkspace(context.Background(), "ws-1", &GitLabConfig{
Host: "https://gitlab.example.com", AuthMethod: AuthMethodPAT,
}); err != nil {
@@ -379,3 +384,80 @@ func TestControllerPatchTaskMRAutomation_PublishesEvent(t *testing.T) {
t.Fatal("expected GitLabTaskMROptionsUpdated event to be published")
}
}
// TestControllerPatchTaskMRAutomation_ScopesToTheNamedMR covers the per-MR
// PATCH contract: a body carrying repository_id/project_path/mr_iid applies
// the switch to that MR alone.
func TestControllerPatchTaskMRAutomation_ScopesToTheNamedMR(t *testing.T) {
router, svc := newMRAutomationControllerFixture(t)
if err := svc.store.UpsertTaskMR(context.Background(), newTestMR("task-1", "", "group/b", 2)); err != nil {
t.Fatalf("seed second MR: %v", err)
}
body := `{"repository_id":"","project_path":"group/a","mr_iid":1,"auto_merge_enabled":true}`
req := httptest.NewRequest(http.MethodPatch, "/api/v1/gitlab/tasks/task-1/mr-automation", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
router.ServeHTTP(resp, req)
if resp.Code != http.StatusOK {
t.Fatalf("PATCH status = %d, body = %s", resp.Code, resp.Body.String())
}
var got TaskMRAutomationResponse
if err := json.Unmarshal(resp.Body.Bytes(), &got); err != nil {
t.Fatalf("decode: %v", err)
}
if len(got.MROptions) != 2 {
t.Fatalf("expected one mr_options entry per linked MR, got %+v", got.MROptions)
}
for _, opt := range got.MROptions {
wantEnabled := opt.ProjectPath == "group/a"
if opt.AutoMergeEnabled != wantEnabled {
t.Errorf("MR %s auto_merge_enabled = %v, want %v", opt.ProjectPath, opt.AutoMergeEnabled, wantEnabled)
}
}
}
// TestControllerPatchTaskMRAutomation_RejectsBadMRIdentity keeps a caller
// mistake a 400 rather than a 500 — and, for a partial identity, keeps it
// from being silently reinterpreted as "apply to every linked MR".
func TestControllerPatchTaskMRAutomation_RejectsBadMRIdentity(t *testing.T) {
router, _ := newMRAutomationControllerFixture(t)
for name, body := range map[string]string{
"unlinked MR": `{"repository_id":"","project_path":"group/nope","mr_iid":9,"auto_fix_enabled":true}`,
"partial identity": `{"project_path":"group/a","auto_fix_enabled":true}`,
"null identity": `{"repository_id":null,"project_path":null,"mr_iid":null,"auto_fix_enabled":true}`,
"identity only": `{"repository_id":"","project_path":"group/a","mr_iid":1}`,
} {
req := httptest.NewRequest(http.MethodPatch, "/api/v1/gitlab/tasks/task-1/mr-automation", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
router.ServeHTTP(resp, req)
if resp.Code != http.StatusBadRequest {
t.Errorf("%s: status = %d, want 400 (body = %s)", name, resp.Code, resp.Body.String())
}
}
}
// TestControllerPatchTaskMRAutomation_RejectsSwitchesWithNoLinkedMRs is the
// HTTP half of the zero-target rule: because the switches only exist per MR,
// a task with none has nowhere to store them, and answering 200 would report
// a write that never happened. The task-level prompt override stays accepted.
func TestControllerPatchTaskMRAutomation_RejectsSwitchesWithNoLinkedMRs(t *testing.T) {
router, svc := newMRAutomationControllerFixture(t)
seedTask(t, svc.store, "task-2", "ws-1")
patch := func(body string) *httptest.ResponseRecorder {
req := httptest.NewRequest(http.MethodPatch, "/api/v1/gitlab/tasks/task-2/mr-automation", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
router.ServeHTTP(resp, req)
return resp
}
if resp := patch(`{"auto_merge_enabled":true}`); resp.Code != http.StatusBadRequest {
t.Errorf("switch patch status = %d, want 400 (body = %s)", resp.Code, resp.Body.String())
}
if resp := patch(`{"auto_fix_prompt_override":"custom"}`); resp.Code != http.StatusOK {
t.Errorf("prompt override status = %d, want 200 (body = %s)", resp.Code, resp.Body.String())
}
}
@@ -18,26 +18,87 @@ const (
// Mirrors github.TaskCIAutoFixMaxRounds.
const TaskMRAutoFixMaxRounds = 10
// TaskMRAutomationOptions stores task-level MR automation preferences:
// the three lifecycle notification switches from #2125, plus auto-fix CI
// and auto-merge (this task). Parallel to github.TaskCIOptions.
// MRIdentity names one merge request linked to a task. RepositoryID may be
// empty for single-repo tasks, matching gitlab_task_mrs' own key.
type MRIdentity struct {
RepositoryID string
ProjectPath string
MRIID int
}
// TaskMRAutomationOptions stores the genuinely task-level MR automation
// preferences: the auto-fix prompt override and the server-resolved reviewer
// username. Parallel to github.TaskCIOptions.
//
// The five switch fields below are legacy: they are no longer written by
// UpdateTaskMRAutomationOptions and are read only by the one-time
// mr_scope_migrated_at fan-out migration (migrateTaskMROptionsToMRScope).
// The per-MR source of truth is TaskMRAutomationOptionsForMR /
// gitlab_task_mr_automation_options.
type TaskMRAutomationOptions struct {
TaskID string `json:"task_id" db:"task_id"`
AutoFixEnabled bool `json:"auto_fix_enabled" db:"auto_fix_enabled"`
AutoMergeEnabled bool `json:"auto_merge_enabled" db:"auto_merge_enabled"`
AutomationRevision int64 `json:"-" db:"automation_revision"`
AutoFixEnabled bool `json:"-" db:"auto_fix_enabled"`
AutoMergeEnabled bool `json:"-" db:"auto_merge_enabled"`
AutoFixPromptOverride *string `json:"auto_fix_prompt_override,omitempty" db:"auto_fix_prompt_override"`
PromptOnReviewRequested bool `json:"prompt_on_review_requested" db:"prompt_on_review_requested"`
PromptOnMerged bool `json:"prompt_on_merged" db:"prompt_on_merged"`
PromptOnClosed bool `json:"prompt_on_closed" db:"prompt_on_closed"`
PromptOnReviewRequested bool `json:"-" db:"prompt_on_review_requested"`
PromptOnMerged bool `json:"-" db:"prompt_on_merged"`
PromptOnClosed bool `json:"-" db:"prompt_on_closed"`
ReviewReviewerUsername string `json:"review_reviewer_username" db:"review_reviewer_username"`
CreatedAt time.Time `json:"created_at" db:"created_at"`
UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
}
// TaskMRAutomationOptionsForMR stores the five automation switches for one
// linked merge request. This is the per-MR source of truth; the aggregated
// booleans on TaskMRAutomationResponse only report "every linked MR has this
// switch on". Parallel to github.TaskPRAutomationOptions.
type TaskMRAutomationOptionsForMR struct {
TaskID string `json:"task_id" db:"task_id"`
RepositoryID string `json:"repository_id" db:"repository_id"`
ProjectPath string `json:"project_path" db:"project_path"`
MRIID int `json:"mr_iid" db:"mr_iid"`
AutoFixEnabled bool `json:"auto_fix_enabled" db:"auto_fix_enabled"`
AutoMergeEnabled bool `json:"auto_merge_enabled" db:"auto_merge_enabled"`
PromptOnReviewRequested bool `json:"prompt_on_review_requested" db:"prompt_on_review_requested"`
PromptOnMerged bool `json:"prompt_on_merged" db:"prompt_on_merged"`
PromptOnClosed bool `json:"prompt_on_closed" db:"prompt_on_closed"`
CreatedAt time.Time `json:"created_at" db:"created_at"`
UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
}
// Identity returns the MR this options row belongs to.
func (o *TaskMRAutomationOptionsForMR) Identity() MRIdentity {
return MRIdentity{RepositoryID: o.RepositoryID, ProjectPath: o.ProjectPath, MRIID: o.MRIID}
}
// TaskMRAutomationSwitchPatch is a partial update for one MR's automation
// switches.
type TaskMRAutomationSwitchPatch struct {
AutoFixEnabled *bool
AutoMergeEnabled *bool
PromptOnReviewRequested *bool
PromptOnMerged *bool
PromptOnClosed *bool
}
// HasAny reports whether the patch contains at least one requested field change.
func (p TaskMRAutomationSwitchPatch) HasAny() bool {
return p.AutoFixEnabled != nil || p.AutoMergeEnabled != nil ||
p.PromptOnReviewRequested != nil || p.PromptOnMerged != nil || p.PromptOnClosed != nil
}
// TaskMRAutomationPatch is a partial update for task MR automation options.
// ReviewReviewerUsername is intentionally absent — it is server-resolved from
// the workspace's authenticated GitLab user, never client-supplied.
// RepositoryID/ProjectPath/MRIID optionally target one linked MR for the five
// automation switches; when all three are nil the switches fan out to every
// MR currently linked to the task. AutoFixPromptOverride is always
// task-level. ReviewReviewerUsername is intentionally absent — it is
// server-resolved from the workspace's authenticated GitLab user, never
// client-supplied.
type TaskMRAutomationPatch struct {
RepositoryID *string
ProjectPath *string
MRIID *int
AutoFixEnabled *bool
AutoMergeEnabled *bool
AutoFixPromptOverride *string
@@ -46,16 +107,71 @@ type TaskMRAutomationPatch struct {
PromptOnClosed *bool
}
// HasAny reports whether the patch contains at least one requested field change.
// HasAny reports whether the patch contains at least one requested field
// change. MR identity alone is not a change — it only says which MR the
// (absent) switch changes would have applied to.
func (p TaskMRAutomationPatch) HasAny() bool {
return p.AutoFixEnabled != nil || p.AutoMergeEnabled != nil || p.AutoFixPromptOverride != nil ||
p.PromptOnReviewRequested != nil || p.PromptOnMerged != nil || p.PromptOnClosed != nil
return p.AutoFixPromptOverride != nil || p.SwitchPatch().HasAny()
}
// SwitchPatch extracts the per-MR automation switch fields.
func (p TaskMRAutomationPatch) SwitchPatch() TaskMRAutomationSwitchPatch {
return TaskMRAutomationSwitchPatch{
AutoFixEnabled: p.AutoFixEnabled,
AutoMergeEnabled: p.AutoMergeEnabled,
PromptOnReviewRequested: p.PromptOnReviewRequested,
PromptOnMerged: p.PromptOnMerged,
PromptOnClosed: p.PromptOnClosed,
}
}
// HasMRIdentity reports whether the patch names a specific MR.
func (p TaskMRAutomationPatch) HasMRIdentity() bool {
return p.RepositoryID != nil && p.ProjectPath != nil && p.MRIID != nil
}
// HasPartialMRIdentity reports whether the patch names some but not all of
// the three MR identity fields — a caller mistake that must be rejected
// rather than silently fanned out to every linked MR.
func (p TaskMRAutomationPatch) HasPartialMRIdentity() bool {
set := 0
if p.RepositoryID != nil {
set++
}
if p.ProjectPath != nil {
set++
}
if p.MRIID != nil {
set++
}
return set != 0 && set != 3
}
// MRIdentity returns the MR named by the patch. Only meaningful when
// HasMRIdentity reports true.
func (p TaskMRAutomationPatch) MRIdentity() MRIdentity {
id := MRIdentity{}
if p.RepositoryID != nil {
id.RepositoryID = *p.RepositoryID
}
if p.ProjectPath != nil {
id.ProjectPath = *p.ProjectPath
}
if p.MRIID != nil {
id.MRIID = *p.MRIID
}
return id
}
// TaskMRAutomationResponse is the HTTP/MCP shape for task MR automation
// options, including the per-MR lifecycle checkpoints for observability.
// options, including the per-MR switches, and the per-MR lifecycle
// checkpoints for observability. The five top-level switch booleans are an
// aggregate over MROptions ("every linked MR has this switch on, and at
// least one MR is linked") kept for MCP/API read compatibility; MROptions is
// the per-MR source of truth.
type TaskMRAutomationResponse struct {
TaskID string `json:"task_id"`
AutomationRevision int64 `json:"automation_revision"`
AutoFixEnabled bool `json:"auto_fix_enabled"`
AutoMergeEnabled bool `json:"auto_merge_enabled"`
AutoFixPromptOverride *string `json:"auto_fix_prompt_override"`
@@ -68,6 +184,9 @@ type TaskMRAutomationResponse struct {
ReviewReviewerUsername string `json:"review_reviewer_username"`
UpdatedAt time.Time `json:"updated_at"`
MRStates []*TaskMRLifecycleState `json:"mr_states"`
// MROptions carries one entry per MR currently linked to the task, so
// the UI can render each MR's own switches instead of the aggregate.
MROptions []*TaskMRAutomationOptionsForMR `json:"mr_options"`
// WorkspaceID is internal routing metadata (best-effort resolved, may be
// empty) that lets the websocket broadcaster scope the
// gitlab.task_mr_options.updated event to the owning workspace instead of
@@ -51,11 +51,9 @@ func TestPoller_RunMRLifecycleSync_SyncsSubscribedRowsAndPublishes(t *testing.T)
if err := store.UpsertTaskMR(ctx, unsubscribed); err != nil {
t.Fatalf("seed unsubscribed MR: %v", err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
setMRSwitches(t, store, "task-1", mrIdentity("group/subscribed", 1), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
})
memBus := bus.NewMemoryEventBus(newTestLogger(t))
svc.SetEventBus(memBus)
@@ -128,13 +126,12 @@ func TestPoller_RunMRLifecycleSync_ErrorOnOneRowDoesNotAbortOthers(t *testing.T)
if err := store.UpsertTaskMR(ctx, ok); err != nil {
t.Fatalf("seed ok MR: %v", err)
}
for _, taskID := range []string{"task-1", "task-2"} {
if _, err := store.UpdateTaskMRAutomationOptions(ctx, taskID, TaskMRAutomationPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch for %s: %v", taskID, err)
}
}
setMRSwitches(t, store, "task-1", mrIdentity("group/broken", 1), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
})
setMRSwitches(t, store, "task-2", mrIdentity("group/ok", 2), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
})
memBus := bus.NewMemoryEventBus(newTestLogger(t))
svc.SetEventBus(memBus)
@@ -194,11 +191,9 @@ func TestPoller_RunMRLifecycleSync_UsesStrictClient(t *testing.T) {
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/subscribed", 1)); err != nil {
t.Fatalf("seed subscribed MR: %v", err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
setMRSwitches(t, store, "task-1", mrIdentity("group/subscribed", 1), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
})
memBus := bus.NewMemoryEventBus(newTestLogger(t))
svc.SetEventBus(memBus)
@@ -260,11 +255,9 @@ func TestPoller_RunMRLifecycleSync_RejectsHostChangeSinceLink(t *testing.T) {
if err := store.UpsertTaskMR(ctx, linked); err != nil {
t.Fatalf("seed linked MR: %v", err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
setMRSwitches(t, store, "task-1", mrIdentity("group/subscribed", 1), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
})
memBus := bus.NewMemoryEventBus(newTestLogger(t))
svc.SetEventBus(memBus)
@@ -323,11 +316,9 @@ func TestPoller_RunMRLifecycleSync_ClearsRecoveredError(t *testing.T) {
if err := store.UpsertTaskMR(ctx, subscribed); err != nil {
t.Fatalf("seed MR: %v", err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
setMRSwitches(t, store, "task-1", mrIdentity("group/subscribed", 1), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
})
if err := store.RecordTaskMRSyncError(ctx, "task-1", "", "group/subscribed", 1, "prior failure"); err != nil {
t.Fatalf("seed prior error: %v", err)
}
@@ -18,11 +18,9 @@ func TestDeleteReviewMRTaskIfTerminal_RetainsWhenLifecyclePromptsEnabled(t *test
mock.SeedMR(project, &MR{IID: 7, State: gitlabStateMerged})
seedTask(t, svc.store, "task-subscribed", "")
if _, err := svc.store.UpdateTaskMRAutomationOptions(ctx, "task-subscribed", TaskMRAutomationPatch{
setMRSwitches(t, svc.store, "task-subscribed", mrIdentity(project, 7), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
})
rec := &recordingReasonDeleter{}
task := &ReviewMRTask{ID: "rmt-1", ProjectPath: project, MRIID: 7, TaskID: "task-subscribed"}
@@ -47,11 +45,9 @@ func TestDeleteReviewMRTaskIfTerminal_AlwaysPolicyIgnoresLifecyclePrompts(t *tes
mock.SeedMR(project, &MR{IID: 7, State: gitlabStateMerged})
seedTask(t, svc.store, "task-subscribed", "")
if _, err := svc.store.UpdateTaskMRAutomationOptions(ctx, "task-subscribed", TaskMRAutomationPatch{
setMRSwitches(t, svc.store, "task-subscribed", mrIdentity(project, 7), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
})
rec := &recordingReasonDeleter{}
task := &ReviewMRTask{ID: "rmt-1", ProjectPath: project, MRIID: 7, TaskID: "task-subscribed"}
@@ -3,6 +3,7 @@ package gitlab
import (
"context"
"errors"
"fmt"
"strings"
promptcfg "github.com/kandev/kandev/config/prompts"
@@ -13,6 +14,19 @@ import (
// defaultCIAutoFixPromptName.
const defaultMRAutoFixPromptName = "mr-auto-fix"
// ErrTaskMRNotLinked reports a patch naming an MR that is not linked to the
// task. Exported so the HTTP controller and MCP handler can map it to a
// client error instead of a 500.
var ErrTaskMRNotLinked = errors.New("gitlab: merge request is not linked to this task")
// ErrTaskMRIdentityIncomplete reports a patch that set some but not all of
// repository_id/project_path/mr_iid. Sending a partial identity is a caller
// mistake, and treating it as "no identity" would silently fan the switch
// change out to every linked MR.
var ErrTaskMRIdentityIncomplete = fmt.Errorf(
"%w: repository_id, project_path and mr_iid must all be set", ErrTaskMRNotLinked,
)
// GetTaskMRAutomationResponse returns a task's MR automation options plus
// its per-MR lifecycle checkpoints (AC1).
func (s *Service) GetTaskMRAutomationResponse(ctx context.Context, taskID string) (*TaskMRAutomationResponse, error) {
@@ -35,11 +49,52 @@ func (s *Service) GetTaskMRAutomationResponse(ctx context.Context, taskID string
if err != nil {
return nil, err
}
mrOptions, err := s.taskMRAutomationOptionsList(ctx, taskID)
if err != nil {
return nil, err
}
states, err := store.ListTaskMRLifecycleStates(ctx, taskID)
if err != nil {
return nil, err
}
return s.taskMRAutomationResponseFromOptions(ctx, opts, states, workspaceID), nil
return s.taskMRAutomationResponseFromOptions(ctx, opts, mrOptions, states, workspaceID), nil
}
// taskMRAutomationOptionsList returns one entry per MR currently linked to
// the task, synthesizing all-off defaults for a linked MR that has no stored
// row yet. A stored row whose MR is no longer linked is dropped: it can no
// longer be configured or evaluated, and surfacing it would let a detached
// MR's leftover switch drag the aggregate down.
func (s *Service) taskMRAutomationOptionsList(ctx context.Context, taskID string) ([]*TaskMRAutomationOptionsForMR, error) {
store := s.requireStore()
if store == nil {
return nil, errStoreUnavailable
}
stored, err := store.ListTaskMRAutomationOptions(ctx, taskID)
if err != nil {
return nil, err
}
mrs, err := store.ListTaskMRsByTask(ctx, taskID)
if err != nil {
return nil, err
}
byIdentity := make(map[MRIdentity]*TaskMRAutomationOptionsForMR, len(stored))
for _, row := range stored {
byIdentity[row.Identity()] = row
}
out := make([]*TaskMRAutomationOptionsForMR, 0, len(mrs))
for _, mr := range mrs {
id := MRIdentity{RepositoryID: mr.RepositoryID, ProjectPath: mr.ProjectPath, MRIID: mr.MRIID}
if row, ok := byIdentity[id]; ok {
out = append(out, row)
continue
}
out = append(out, &TaskMRAutomationOptionsForMR{
TaskID: taskID, RepositoryID: id.RepositoryID,
ProjectPath: id.ProjectPath, MRIID: id.MRIID,
})
}
return out, nil
}
// GetTaskMRAutomationEvaluation returns the narrow snapshot needed for one
@@ -79,6 +134,13 @@ func (s *Service) GetTaskMRAutomationEvaluation(
if err != nil {
return nil, err
}
id := MRIdentity{RepositoryID: repositoryID, ProjectPath: projectPath, MRIID: mrIID}
// Only this MR's own switches drive this evaluation — a sibling MR's
// configuration must never enable automation here.
mrOpts, err := store.GetTaskMRAutomationOptionsForMR(ctx, taskID, id)
if err != nil {
return nil, err
}
checkpoint, err := store.GetTaskMRLifecycleState(ctx, taskID, repositoryID, projectPath, mrIID)
if err != nil {
return nil, err
@@ -89,7 +151,9 @@ func (s *Service) GetTaskMRAutomationEvaluation(
evaluationOpts := *opts
evaluationOpts.ReviewReviewerUsername = reviewerUsername
return &TaskMRAutomationEvaluation{
Options: s.taskMRAutomationResponseFromOptions(ctx, &evaluationOpts, nil, workspaceID),
Options: s.taskMRAutomationResponseFromOptions(
ctx, &evaluationOpts, []*TaskMRAutomationOptionsForMR{mrOpts}, nil, workspaceID,
),
Checkpoint: checkpoint,
}, nil
}
@@ -103,27 +167,54 @@ func (s *Service) rebindTaskMRReviewerFromConfig(ctx context.Context, taskID, us
}
func (s *Service) taskMRAutomationResponseFromOptions(
ctx context.Context, opts *TaskMRAutomationOptions, states []*TaskMRLifecycleState, workspaceID string,
ctx context.Context, opts *TaskMRAutomationOptions, mrOptions []*TaskMRAutomationOptionsForMR,
states []*TaskMRLifecycleState, workspaceID string,
) *TaskMRAutomationResponse {
effectivePrompt, usingDefault := s.effectiveMRAutoFixPrompt(ctx, opts)
aggregate := aggregateMRAutomationOptions(mrOptions)
return &TaskMRAutomationResponse{
TaskID: opts.TaskID,
AutoFixEnabled: opts.AutoFixEnabled,
AutoMergeEnabled: opts.AutoMergeEnabled,
AutomationRevision: opts.AutomationRevision,
AutoFixEnabled: aggregate.AutoFixEnabled,
AutoMergeEnabled: aggregate.AutoMergeEnabled,
AutoFixPromptOverride: opts.AutoFixPromptOverride,
AutoFixMaxRounds: TaskMRAutoFixMaxRounds,
EffectiveAutoFixPrompt: effectivePrompt,
UsingDefaultPrompt: usingDefault,
PromptOnReviewRequested: opts.PromptOnReviewRequested,
PromptOnMerged: opts.PromptOnMerged,
PromptOnClosed: opts.PromptOnClosed,
PromptOnReviewRequested: aggregate.PromptOnReviewRequested,
PromptOnMerged: aggregate.PromptOnMerged,
PromptOnClosed: aggregate.PromptOnClosed,
ReviewReviewerUsername: opts.ReviewReviewerUsername,
UpdatedAt: opts.UpdatedAt,
MRStates: states,
MROptions: mrOptions,
WorkspaceID: workspaceID,
}
}
// aggregateMRAutomationOptions collapses per-MR switches into the response's
// top-level booleans: a switch reads as on only when every linked MR has it
// on and at least one MR is linked. "Every" rather than "any" keeps the
// aggregate a safe answer to "is this on for the MRs I'd act on" for an MCP
// caller that cannot name one MR.
func aggregateMRAutomationOptions(mrOptions []*TaskMRAutomationOptionsForMR) TaskMRAutomationOptionsForMR {
if len(mrOptions) == 0 {
return TaskMRAutomationOptionsForMR{}
}
aggregate := TaskMRAutomationOptionsForMR{
AutoFixEnabled: true, AutoMergeEnabled: true, PromptOnReviewRequested: true,
PromptOnMerged: true, PromptOnClosed: true,
}
for _, opt := range mrOptions {
aggregate.AutoFixEnabled = aggregate.AutoFixEnabled && opt.AutoFixEnabled
aggregate.AutoMergeEnabled = aggregate.AutoMergeEnabled && opt.AutoMergeEnabled
aggregate.PromptOnReviewRequested = aggregate.PromptOnReviewRequested && opt.PromptOnReviewRequested
aggregate.PromptOnMerged = aggregate.PromptOnMerged && opt.PromptOnMerged
aggregate.PromptOnClosed = aggregate.PromptOnClosed && opt.PromptOnClosed
}
return aggregate
}
// effectiveMRAutoFixPrompt resolves the prompt text that will actually be
// sent on the next auto-fix dispatch: a non-empty per-task override wins;
// otherwise the default template, itself resolved through the editable
@@ -144,11 +235,18 @@ func (s *Service) effectiveMRAutoFixPrompt(ctx context.Context, opts *TaskMRAuto
return resolver.ResolvePromptContent(ctx, defaultMRAutoFixPromptName, fallback), true
}
// UpdateTaskMRAutomationOptions applies a partial update. When the patch
// turns prompt_on_review_requested on, the workspace's authenticated GitLab
// username is resolved and persisted (AC5); turning it off clears the
// stored username. Resolution always goes through the strict, non-ambient
// workspace client (AC32).
// UpdateTaskMRAutomationOptions applies a partial update. The five automation
// switches are applied to the single MR named by
// patch.RepositoryID/ProjectPath/MRIID, or — when no MR is named — fanned out
// to every MR currently linked to the task, which preserves the behavior of
// MCP callers that have no MR identity to send. The auto-fix prompt override
// stays task-level either way.
//
// When the patch turns prompt_on_review_requested on, the workspace's
// authenticated GitLab username is resolved and persisted (AC5); turning it
// off clears the stored username unless another linked MR still needs it.
// Resolution always goes through the strict, non-ambient workspace client
// (AC32).
func (s *Service) UpdateTaskMRAutomationOptions(ctx context.Context, taskID string, patch TaskMRAutomationPatch) (*TaskMRAutomationResponse, error) {
if err := s.authorizeTaskMRAccess(ctx, taskID); err != nil {
return nil, err
@@ -157,17 +255,42 @@ func (s *Service) UpdateTaskMRAutomationOptions(ctx context.Context, taskID stri
if store == nil {
return nil, errStoreUnavailable
}
if patch.HasPartialMRIdentity() {
return nil, ErrTaskMRIdentityIncomplete
}
// See the identical check in GetTaskMRAutomationResponse: rejects an
// unknown task ID before it can create an orphan options row.
workspaceID, err := store.WorkspaceIDForTask(ctx, taskID)
if err != nil {
return nil, err
}
reviewerUsername, err := s.resolveReviewerUsernameForPatch(ctx, taskID, patch)
targets, err := s.resolveTaskMRAutomationTargets(ctx, taskID, patch)
if err != nil {
return nil, err
}
opts, err := store.UpdateTaskMRAutomationOptions(ctx, taskID, patch, reviewerUsername)
// The switches are per-MR, so with nothing linked there is no row to write
// them to. Returning 200 here would report success for a write that stored
// nothing, and the caller would only discover it much later when the
// automation it thought it had enabled never fired. (Before the switches
// became per-MR this persisted on the task row and a later-linked MR
// inherited it.) The prompt override is task-level and still allowed.
if patch.SwitchPatch().HasAny() && len(targets) == 0 {
return nil, fmt.Errorf("%w: the task has no linked merge requests", ErrTaskMRNotLinked)
}
reviewerUsername, err := s.resolveReviewerUsernameForPatch(ctx, taskID, patch, targets)
if err != nil {
return nil, err
}
// One transaction covering the task-level fields and every target: a
// failure must not leave the reviewer or prompt override committed while
// the per-MR switches roll back.
opts, err := store.UpdateTaskMRAutomationOptionsWithMRs(
ctx, taskID, patch, reviewerUsername, targets, patch.SwitchPatch(),
)
if err != nil {
return nil, err
}
mrOptions, err := s.taskMRAutomationOptionsList(ctx, taskID)
if err != nil {
return nil, err
}
@@ -175,22 +298,97 @@ func (s *Service) UpdateTaskMRAutomationOptions(ctx context.Context, taskID stri
if err != nil {
return nil, err
}
return s.taskMRAutomationResponseFromOptions(ctx, opts, states, workspaceID), nil
return s.taskMRAutomationResponseFromOptions(ctx, opts, mrOptions, states, workspaceID), nil
}
func (s *Service) resolveReviewerUsernameForPatch(ctx context.Context, taskID string, patch TaskMRAutomationPatch) (*string, error) {
if patch.PromptOnReviewRequested == nil {
return nil, nil
// resolveTaskMRAutomationTargets resolves which linked MR(s) a patch applies
// to. No identity fans out to every linked MR; a named identity that is not
// currently linked is rejected rather than silently creating an orphan
// automation row.
func (s *Service) resolveTaskMRAutomationTargets(
ctx context.Context, taskID string, patch TaskMRAutomationPatch,
) ([]MRIdentity, error) {
store := s.requireStore()
if store == nil {
return nil, errStoreUnavailable
}
if !*patch.PromptOnReviewRequested {
empty := ""
return &empty, nil
}
username, err := s.resolveAuthenticatedUsernameStrict(ctx, taskID)
mrs, err := store.ListTaskMRsByTask(ctx, taskID)
if err != nil {
return nil, err
}
return &username, nil
linked := make([]MRIdentity, 0, len(mrs))
for _, mr := range mrs {
linked = append(linked, MRIdentity{
RepositoryID: mr.RepositoryID, ProjectPath: mr.ProjectPath, MRIID: mr.MRIID,
})
}
if !patch.HasMRIdentity() {
return linked, nil
}
wanted := patch.MRIdentity()
for _, id := range linked {
if id == wanted {
return []MRIdentity{id}, nil
}
}
return nil, fmt.Errorf("%w: project_path=%s mr_iid=%d", ErrTaskMRNotLinked, wanted.ProjectPath, wanted.MRIID)
}
// resolveReviewerUsernameForPatch resolves the task-level reviewer username
// implied by a review-request switch change. Disabling only blanks the shared
// username when no *other* linked MR still has the switch on — otherwise this
// would silently break that MR's automation.
func (s *Service) resolveReviewerUsernameForPatch(
ctx context.Context, taskID string, patch TaskMRAutomationPatch, targets []MRIdentity,
) (*string, error) {
if patch.PromptOnReviewRequested == nil {
return nil, nil
}
if *patch.PromptOnReviewRequested {
username, err := s.resolveAuthenticatedUsernameStrict(ctx, taskID)
if err != nil {
return nil, err
}
return &username, nil
}
stillNeeded, err := s.anyOtherMRHasReviewRequestEnabled(ctx, taskID, targets)
if err != nil {
return nil, err
}
if stillNeeded {
return nil, nil
}
empty := ""
return &empty, nil
}
// anyOtherMRHasReviewRequestEnabled reports whether a linked MR outside
// `targets` still has prompt_on_review_requested on.
func (s *Service) anyOtherMRHasReviewRequestEnabled(
ctx context.Context, taskID string, targets []MRIdentity,
) (bool, error) {
store := s.requireStore()
if store == nil {
return false, errStoreUnavailable
}
stored, err := store.ListTaskMRAutomationOptions(ctx, taskID)
if err != nil {
return false, err
}
targeted := make(map[MRIdentity]struct{}, len(targets))
for _, id := range targets {
targeted[id] = struct{}{}
}
for _, opt := range stored {
if !opt.PromptOnReviewRequested {
continue
}
if _, ok := targeted[opt.Identity()]; ok {
continue
}
return true, nil
}
return false, nil
}
func (s *Service) resolveAuthenticatedUsernameStrict(ctx context.Context, taskID string) (string, error) {
@@ -208,18 +406,25 @@ func (s *Service) resolveAuthenticatedUsernameStrict(ctx context.Context, taskID
return username, nil
}
// HasEnabledTaskMRAgentPrompts reports whether any lifecycle switch is on,
// used by cleanup retention (AC24) and by the poll/evaluation gate (AC16).
// HasEnabledTaskMRAgentPrompts reports whether any MR linked to the task has
// a lifecycle switch on, used by cleanup retention (AC24) and by the
// poll/evaluation gate (AC16). Reads the per-MR table, so a task whose MRs
// are configured differently is still detected.
func (s *Service) HasEnabledTaskMRAgentPrompts(ctx context.Context, taskID string) (bool, error) {
store := s.requireStore()
if store == nil {
return false, errStoreUnavailable
}
opts, err := store.GetTaskMRAutomationOptions(ctx, taskID)
stored, err := store.ListTaskMRAutomationOptions(ctx, taskID)
if err != nil {
return false, err
}
return opts.PromptOnReviewRequested || opts.PromptOnMerged || opts.PromptOnClosed, nil
for _, opt := range stored {
if opt.PromptOnReviewRequested || opt.PromptOnMerged || opt.PromptOnClosed {
return true, nil
}
}
return false, nil
}
// RebindTaskMRReviewer re-resolves the workspace's authenticated GitLab
@@ -143,11 +143,12 @@ func TestHasEnabledTaskMRAgentPrompts(t *testing.T) {
t.Fatalf("expected disabled by default: enabled=%v err=%v", enabled, err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
// One linked MR with a lifecycle switch on is enough — the gate reads the
// per-MR table, so a task whose MRs are configured differently still
// counts as subscribed.
setMRSwitches(t, store, "task-1", mrIdentity("group/a", 1), TaskMRAutomationSwitchPatch{
PromptOnClosed: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
})
enabled, err = svc.HasEnabledTaskMRAgentPrompts(ctx, "task-1")
if err != nil || !enabled {
t.Fatalf("expected enabled after patch: enabled=%v err=%v", enabled, err)
@@ -171,6 +172,9 @@ func TestUpdateTaskMRAutomationOptions_ResolvesAndClearsReviewer(t *testing.T) {
return mock, nil
}
ctx := context.Background()
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/a", 1)); err != nil {
t.Fatalf("upsert MR: %v", err)
}
resp, err := svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
PromptOnReviewRequested: boolPtr(true),
@@ -193,6 +197,209 @@ func TestUpdateTaskMRAutomationOptions_ResolvesAndClearsReviewer(t *testing.T) {
}
}
// newMRAutomationServiceFixture builds a workspace-configured service whose
// strict client resolves to `username`, the shape every targeting test below
// needs.
func newMRAutomationServiceFixture(t *testing.T, username string) (*Service, *Store) {
t.Helper()
store := newTestStore(t)
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
if err := store.SaveConfigForWorkspace(context.Background(), "ws-1", &GitLabConfig{
Host: "https://gitlab.example.com", AuthMethod: AuthMethodPAT,
}); err != nil {
t.Fatalf("save config: %v", err)
}
secrets := &configTestSecrets{values: map[string]string{SecretKeyForWorkspace("ws-1"): "token"}}
svc := newWorkspaceConfigService(t, store, secrets)
mock := NewMockClient("https://gitlab.example.com")
mock.SetUser(username)
svc.workspaceClientFn = func(_ context.Context, _ *GitLabConfig, _ string) (Client, error) {
return mock, nil
}
return svc, store
}
// TestUpdateTaskMRAutomationOptions_TargetsOnlyTheNamedMR is the regression
// this change exists for at the service layer: a patch carrying one MR's
// identity must not touch the task's other linked MRs.
func TestUpdateTaskMRAutomationOptions_TargetsOnlyTheNamedMR(t *testing.T) {
svc, store := newMRAutomationServiceFixture(t, "alice")
ctx := context.Background()
for _, mr := range []*TaskMR{
newTestMR("task-1", "", "group/a", 1),
newTestMR("task-1", "", "group/b", 2),
} {
if err := store.UpsertTaskMR(ctx, mr); err != nil {
t.Fatalf("upsert MR %s: %v", mr.ProjectPath, err)
}
}
resp, err := svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
RepositoryID: stringPtr(""), ProjectPath: stringPtr("group/a"), MRIID: intPtr(1),
AutoMergeEnabled: boolPtr(true),
})
if err != nil {
t.Fatalf("UpdateTaskMRAutomationOptions: %v", err)
}
if len(resp.MROptions) != 2 {
t.Fatalf("expected one entry per linked MR, got %+v", resp.MROptions)
}
byProject := map[string]*TaskMRAutomationOptionsForMR{}
for _, opt := range resp.MROptions {
byProject[opt.ProjectPath] = opt
}
if !byProject["group/a"].AutoMergeEnabled {
t.Errorf("targeted MR did not get the switch: %+v", byProject["group/a"])
}
if byProject["group/b"].AutoMergeEnabled {
t.Errorf("untargeted MR inherited the switch: %+v", byProject["group/b"])
}
// The aggregate is "every linked MR", so a partially configured task
// must not report the switch as on.
if resp.AutoMergeEnabled {
t.Errorf("aggregate reported auto-merge on while only one of two MRs has it")
}
}
// TestUpdateTaskMRAutomationOptions_FansOutWhenNoMRIsNamed preserves the
// behavior of MCP callers that have no MR identity to send.
func TestUpdateTaskMRAutomationOptions_FansOutWhenNoMRIsNamed(t *testing.T) {
svc, store := newMRAutomationServiceFixture(t, "alice")
ctx := context.Background()
for _, mr := range []*TaskMR{
newTestMR("task-1", "", "group/a", 1),
newTestMR("task-1", "", "group/b", 2),
} {
if err := store.UpsertTaskMR(ctx, mr); err != nil {
t.Fatalf("upsert MR %s: %v", mr.ProjectPath, err)
}
}
resp, err := svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
PromptOnMerged: boolPtr(true),
})
if err != nil {
t.Fatalf("UpdateTaskMRAutomationOptions: %v", err)
}
for _, opt := range resp.MROptions {
if !opt.PromptOnMerged {
t.Errorf("MR %s missed the fan-out: %+v", opt.ProjectPath, opt)
}
}
if !resp.PromptOnMerged {
t.Errorf("aggregate should report on once every linked MR has it: %+v", resp)
}
}
// TestUpdateTaskMRAutomationOptions_RollsBackMixedPatch keeps the task-level
// prompt override and per-MR switch write in one transaction. A failed switch
// write must not leave the prompt override committed while returning an error.
func TestUpdateTaskMRAutomationOptions_RollsBackMixedPatch(t *testing.T) {
svc, store := newMRAutomationServiceFixture(t, "alice")
ctx := context.Background()
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/a", 1)); err != nil {
t.Fatalf("upsert MR: %v", err)
}
if _, err := store.db.Exec(`
CREATE TRIGGER fail_mr_switch BEFORE UPDATE ON gitlab_task_mr_automation_options
BEGIN SELECT RAISE(ABORT, 'injected failure'); END`); err != nil {
t.Fatalf("create failure trigger: %v", err)
}
_, err := svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
AutoFixPromptOverride: stringPtr("must rollback"),
AutoMergeEnabled: boolPtr(true),
})
if err == nil {
t.Fatal("expected mixed patch to fail")
}
options, err := store.GetTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("get task options: %v", err)
}
if options.AutoFixPromptOverride != nil {
t.Fatalf("failed mixed patch persisted prompt override: %+v", options)
}
}
// TestUpdateTaskMRAutomationOptions_RejectsUnlinkedOrPartialIdentity keeps a
// caller mistake from silently creating an orphan automation row, or from
// being reinterpreted as a fan-out over every MR.
func TestUpdateTaskMRAutomationOptions_RejectsUnlinkedOrPartialIdentity(t *testing.T) {
svc, store := newMRAutomationServiceFixture(t, "alice")
ctx := context.Background()
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/a", 1)); err != nil {
t.Fatalf("upsert MR: %v", err)
}
_, err := svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
RepositoryID: stringPtr(""), ProjectPath: stringPtr("group/nope"), MRIID: intPtr(42),
AutoFixEnabled: boolPtr(true),
})
if !errors.Is(err, ErrTaskMRNotLinked) {
t.Fatalf("expected ErrTaskMRNotLinked for an unlinked MR, got %v", err)
}
_, err = svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
ProjectPath: stringPtr("group/a"), AutoFixEnabled: boolPtr(true),
})
if !errors.Is(err, ErrTaskMRIdentityIncomplete) {
t.Fatalf("expected ErrTaskMRIdentityIncomplete for a partial identity, got %v", err)
}
stored, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("ListTaskMRAutomationOptions: %v", err)
}
if len(stored) != 0 {
t.Fatalf("a rejected patch still wrote rows: %+v", stored)
}
}
// TestUpdateTaskMRAutomationOptions_KeepsReviewerWhileAnotherMRNeedsIt covers
// the one field the switches share: disabling review-request on one MR must
// not blank the task-level reviewer another MR's automation still depends on.
func TestUpdateTaskMRAutomationOptions_KeepsReviewerWhileAnotherMRNeedsIt(t *testing.T) {
svc, store := newMRAutomationServiceFixture(t, "alice")
ctx := context.Background()
for _, mr := range []*TaskMR{
newTestMR("task-1", "", "group/a", 1),
newTestMR("task-1", "", "group/b", 2),
} {
if err := store.UpsertTaskMR(ctx, mr); err != nil {
t.Fatalf("upsert MR %s: %v", mr.ProjectPath, err)
}
}
if _, err := svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
PromptOnReviewRequested: boolPtr(true),
}); err != nil {
t.Fatalf("enable on both MRs: %v", err)
}
resp, err := svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
RepositoryID: stringPtr(""), ProjectPath: stringPtr("group/a"), MRIID: intPtr(1),
PromptOnReviewRequested: boolPtr(false),
})
if err != nil {
t.Fatalf("disable on one MR: %v", err)
}
if resp.ReviewReviewerUsername != "alice" {
t.Fatalf("reviewer cleared while another MR still needs it: %+v", resp)
}
resp, err = svc.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
RepositoryID: stringPtr(""), ProjectPath: stringPtr("group/b"), MRIID: intPtr(2),
PromptOnReviewRequested: boolPtr(false),
})
if err != nil {
t.Fatalf("disable on the last MR: %v", err)
}
if resp.ReviewReviewerUsername != "" {
t.Fatalf("expected reviewer cleared once no MR needs it, got %+v", resp)
}
}
func TestGetTaskMRAutomationEvaluation_UsesConfigUsernameAndExactCheckpoint(t *testing.T) {
store := newTestStore(t)
seedWorkspace(t, store, "ws-1")
@@ -255,3 +462,73 @@ func TestGetTaskMRAutomationEvaluation_UsesConfigUsernameAndExactCheckpoint(t *t
t.Fatalf("evaluation loaded task-wide MR states: %+v", evaluation.Options.MRStates)
}
}
// TestGetTaskMRAutomationEvaluation_UsesOnlyTheTargetMRsSwitches is the
// orchestrator-facing half of the per-MR scoping: an evaluation for one MR
// must not see a sibling MR's enabled automation.
func TestGetTaskMRAutomationEvaluation_UsesOnlyTheTargetMRsSwitches(t *testing.T) {
svc, store := newMRAutomationServiceFixture(t, "alice")
ctx := context.Background()
for _, mr := range []*TaskMR{
newTestMR("task-1", "", "group/a", 1),
newTestMR("task-1", "", "group/b", 2),
} {
if err := store.UpsertTaskMR(ctx, mr); err != nil {
t.Fatalf("upsert MR %s: %v", mr.ProjectPath, err)
}
}
setMRSwitches(t, store, "task-1", mrIdentity("group/a", 1), TaskMRAutomationSwitchPatch{
AutoFixEnabled: boolPtr(true), PromptOnMerged: boolPtr(true),
})
configured, err := svc.GetTaskMRAutomationEvaluation(ctx, "task-1", "", "group/a", 1)
if err != nil {
t.Fatalf("evaluate configured MR: %v", err)
}
if !configured.Options.AutoFixEnabled || !configured.Options.PromptOnMerged {
t.Fatalf("configured MR lost its own switches: %+v", configured.Options)
}
sibling, err := svc.GetTaskMRAutomationEvaluation(ctx, "task-1", "", "group/b", 2)
if err != nil {
t.Fatalf("evaluate sibling MR: %v", err)
}
if sibling.Options.AutoFixEnabled || sibling.Options.PromptOnMerged ||
sibling.Options.AutoMergeEnabled || sibling.Options.PromptOnReviewRequested ||
sibling.Options.PromptOnClosed {
t.Fatalf("sibling MR inherited another MR's automation: %+v", sibling.Options)
}
}
// TestUpdateTaskMRAutomationOptions_RejectsSwitchesWithNoLinkedMRs covers the
// zero-target case. The switches live per-MR, so with nothing linked there is
// no row to write them to; returning success would report a write that stored
// nothing, and the caller would only find out when the automation it believed
// it had enabled never fired.
func TestUpdateTaskMRAutomationOptions_RejectsSwitchesWithNoLinkedMRs(t *testing.T) {
svc, _ := newMRAutomationServiceFixture(t, "alice")
_, err := svc.UpdateTaskMRAutomationOptions(context.Background(), "task-1", TaskMRAutomationPatch{
PromptOnMerged: boolPtr(true),
})
if !errors.Is(err, ErrTaskMRNotLinked) {
t.Fatalf("expected ErrTaskMRNotLinked for a task with no linked MRs, got %v", err)
}
}
// TestUpdateTaskMRAutomationOptions_AllowsPromptOverrideWithNoLinkedMRs is the
// other half of the same rule: the auto-fix prompt override is task-level, so
// it stays settable before any MR is linked.
func TestUpdateTaskMRAutomationOptions_AllowsPromptOverrideWithNoLinkedMRs(t *testing.T) {
svc, _ := newMRAutomationServiceFixture(t, "alice")
resp, err := svc.UpdateTaskMRAutomationOptions(context.Background(), "task-1", TaskMRAutomationPatch{
AutoFixPromptOverride: stringPtr("fix it please"),
})
if err != nil {
t.Fatalf("prompt override with no linked MRs should succeed, got %v", err)
}
if resp.AutoFixPromptOverride == nil || *resp.AutoFixPromptOverride != "fix it please" {
t.Errorf("override not persisted: %+v", resp.AutoFixPromptOverride)
}
}
+19 -5
View File
@@ -223,6 +223,11 @@ func (s *Store) createTables() error {
if err := s.migrateMRAutomationAutomationColumns(); err != nil {
return err
}
// Must follow migrateMRAutomationAutomationColumns, which adds the
// mr_scope_migrated_at column this migration is guarded by.
if err := s.migrateTaskMROptionsToMRScope(); err != nil {
return err
}
if err := s.migrateConfigRevision(); err != nil {
return err
}
@@ -639,12 +644,15 @@ func (s *Store) ListTaskMRsByWorkspaceID(ctx context.Context, workspaceID string
}
// DeleteTaskMR removes a single task↔MR row, cascading to that MR's
// lifecycle checkpoint (gitlab_task_mr_state). Without this, re-linking the
// lifecycle checkpoint (gitlab_task_mr_state) and its per-MR automation
// switches (gitlab_task_mr_automation_options). Without this, re-linking the
// same MR later would inherit the old checkpoint and could suppress its next
// lifecycle prompt — gitlab_task_mrs has no FK relationship to
// gitlab_task_mr_state (it's keyed by (task_id, repository_id, project_path,
// mr_iid), not by gitlab_task_mrs.id) for the database to cascade this
// automatically.
// lifecycle prompt, or silently re-arm a switch — including auto-merge — that
// no surface displays but the evaluator still reads. gitlab_task_mrs has no FK
// relationship to either table (both are keyed by (task_id, repository_id,
// project_path, mr_iid), not by gitlab_task_mrs.id) for the database to
// cascade this automatically. Keep in step with
// DeleteTaskMRForWorkspace, which performs the same three-table cleanup.
func (s *Store) DeleteTaskMR(ctx context.Context, id string) error {
tx, err := s.db.BeginTxx(ctx, nil)
if err != nil {
@@ -671,6 +679,12 @@ func (s *Store) DeleteTaskMR(ctx context.Context, id string) error {
mr.TaskID, mr.RepositoryID, mr.ProjectPath, mr.MRIID); err != nil {
return err
}
if _, err := tx.ExecContext(ctx,
`DELETE FROM gitlab_task_mr_automation_options
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
mr.TaskID, mr.RepositoryID, mr.ProjectPath, mr.MRIID); err != nil {
return err
}
if _, err := tx.ExecContext(ctx, `DELETE FROM gitlab_task_mrs WHERE id = ?`, id); err != nil {
return err
}
@@ -3,19 +3,21 @@ package gitlab
import "testing"
// TestStoreResetWorkspaceE2E_ClearsMRAutomationState covers AC38-adjacent
// discipline for the E2E reset invariant (apps/backend/AGENTS.md): the two
// new MR automation tables must be wiped for the reset workspace and left
// untouched for other workspaces, ahead of the task rows they reference.
// discipline for the E2E reset invariant (apps/backend/AGENTS.md): every MR
// automation table must be wiped for the reset workspace and left untouched
// for other workspaces, ahead of the task rows they reference.
func TestStoreResetWorkspaceE2E_ClearsMRAutomationState(t *testing.T) {
store := newTestStore(t)
for _, workspaceID := range []string{"ws-a", "ws-b"} {
seedWorkspace(t, store, workspaceID)
seedTask(t, store, "task-"+workspaceID, workspaceID)
if _, err := store.UpdateTaskMRAutomationOptions(t.Context(), "task-"+workspaceID, TaskMRAutomationPatch{
PromptOnMerged: boolPtr(true),
AutoFixPromptOverride: stringPtr("custom"),
}, nil); err != nil {
t.Fatalf("seed options %s: %v", workspaceID, err)
}
setMRSwitches(t, store, "task-"+workspaceID, mrIdentity("group/project", 7),
TaskMRAutomationSwitchPatch{PromptOnMerged: boolPtr(true)})
if err := store.SetTaskMRObservedState(t.Context(), "task-"+workspaceID, "", "group/project", 7, "open"); err != nil {
t.Fatalf("seed state %s: %v", workspaceID, err)
}
@@ -33,6 +35,7 @@ func assertMRAutomationRowCount(t *testing.T, store *Store, taskID string, want
t.Helper()
for _, query := range []string{
`SELECT COUNT(*) FROM gitlab_task_mr_options WHERE task_id = ?`,
`SELECT COUNT(*) FROM gitlab_task_mr_automation_options WHERE task_id = ?`,
`SELECT COUNT(*) FROM gitlab_task_mr_state WHERE task_id = ?`,
} {
var got int
@@ -7,6 +7,8 @@ import (
"fmt"
"strings"
"time"
"github.com/jmoiron/sqlx"
)
const createMRAutomationTablesSQL = `
@@ -19,11 +21,35 @@ const createMRAutomationTablesSQL = `
prompt_on_merged BOOLEAN NOT NULL DEFAULT 0,
prompt_on_closed BOOLEAN NOT NULL DEFAULT 0,
review_reviewer_username TEXT NOT NULL DEFAULT '',
automation_revision INTEGER NOT NULL DEFAULT 0,
mr_scope_migrated_at DATETIME,
created_at DATETIME NOT NULL,
updated_at DATETIME NOT NULL,
FOREIGN KEY (task_id) REFERENCES tasks(id) ON DELETE CASCADE
);
-- Per-MR automation switches. Source of truth for the five switches
-- (auto-fix, auto-merge, and the three lifecycle notifications) that
-- gitlab_task_mr_options used to hold task-wide. Keyed by the same
-- four-part MR identity as gitlab_task_mr_state, so a task with two
-- linked MRs configures each independently. See
-- migrateTaskMROptionsToMRScope.
CREATE TABLE IF NOT EXISTS gitlab_task_mr_automation_options (
task_id TEXT NOT NULL,
repository_id TEXT NOT NULL DEFAULT '',
project_path TEXT NOT NULL,
mr_iid INTEGER NOT NULL,
auto_fix_enabled BOOLEAN NOT NULL DEFAULT 0,
auto_merge_enabled BOOLEAN NOT NULL DEFAULT 0,
prompt_on_review_requested BOOLEAN NOT NULL DEFAULT 0,
prompt_on_merged BOOLEAN NOT NULL DEFAULT 0,
prompt_on_closed BOOLEAN NOT NULL DEFAULT 0,
created_at DATETIME NOT NULL,
updated_at DATETIME NOT NULL,
PRIMARY KEY (task_id, repository_id, project_path, mr_iid),
FOREIGN KEY (task_id) REFERENCES tasks(id) ON DELETE CASCADE
);
CREATE TABLE IF NOT EXISTS gitlab_task_mr_state (
task_id TEXT NOT NULL,
repository_id TEXT NOT NULL DEFAULT '',
@@ -72,6 +98,10 @@ func (s *Store) migrateMRAutomationAutomationColumns() error {
{"auto_fix_enabled", sqlBooleanDefaultFalse},
{"auto_merge_enabled", sqlBooleanDefaultFalse},
{"auto_fix_prompt_override", "TEXT"},
{"automation_revision", sqlIntegerDefaultZero},
// Guards the one-time fan-out of the legacy task-wide switches onto
// per-MR rows — see migrateTaskMROptionsToMRScope.
{"mr_scope_migrated_at", "DATETIME"},
}
if err := addMissingColumns(s, "gitlab_task_mr_options", optionsColumns); err != nil {
return err
@@ -89,6 +119,89 @@ func (s *Store) migrateMRAutomationAutomationColumns() error {
return addMissingColumns(s, "gitlab_task_mr_state", stateColumns)
}
// legacyMROptionsRow is one pre-scope-migration gitlab_task_mr_options row,
// read only so its task-wide switch values can be fanned out onto the task's
// linked MRs exactly once.
type legacyMROptionsRow struct {
taskID string
autoFix, autoMerge, promptReview, promptMerged, promptClosed bool
}
// migrateTaskMROptionsToMRScope seeds gitlab_task_mr_automation_options rows
// from each pre-upgrade gitlab_task_mr_options row's legacy switches, fanning
// each task's values out onto every gitlab_task_mrs row currently linked to
// it. Guarded by mr_scope_migrated_at, stamped in the same transaction as the
// fan-out insert: without the marker, replaying this on every boot would
// re-enable a switch the user has since turned off for one MR, and an MR
// linked to the task after migration would inherit the legacy value instead
// of starting all-off (which ON CONFLICT DO NOTHING alone cannot prevent).
// Mirrors github.Store.migrateTaskCIOptionsToPRScope.
func (s *Store) migrateTaskMROptionsToMRScope() error {
legacy, err := s.unmigratedMROptionRows()
if err != nil {
return err
}
for _, row := range legacy {
if err := s.fanOutMROptionsToMRScope(row); err != nil {
return err
}
}
return nil
}
func (s *Store) unmigratedMROptionRows() ([]legacyMROptionsRow, error) {
rows, err := s.ro.Query(`
SELECT task_id, auto_fix_enabled, auto_merge_enabled, prompt_on_review_requested,
prompt_on_merged, prompt_on_closed
FROM gitlab_task_mr_options
WHERE mr_scope_migrated_at IS NULL`)
if err != nil {
return nil, fmt.Errorf("list unmigrated task MR options: %w", err)
}
defer func() { _ = rows.Close() }()
var legacy []legacyMROptionsRow
for rows.Next() {
var row legacyMROptionsRow
if err := rows.Scan(&row.taskID, &row.autoFix, &row.autoMerge,
&row.promptReview, &row.promptMerged, &row.promptClosed); err != nil {
return nil, fmt.Errorf("scan unmigrated task MR options: %w", err)
}
legacy = append(legacy, row)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("iterate unmigrated task MR options: %w", err)
}
return legacy, nil
}
func (s *Store) fanOutMROptionsToMRScope(row legacyMROptionsRow) error {
tx, err := s.db.Beginx()
if err != nil {
return err
}
defer func() { _ = tx.Rollback() }()
now := time.Now().UTC()
if _, err := tx.Exec(`
INSERT INTO gitlab_task_mr_automation_options (
task_id, repository_id, project_path, mr_iid, auto_fix_enabled, auto_merge_enabled,
prompt_on_review_requested, prompt_on_merged, prompt_on_closed, created_at, updated_at
)
SELECT task_id, repository_id, project_path, mr_iid, ?, ?, ?, ?, ?, ?, ?
FROM gitlab_task_mrs
WHERE task_id = ?
ON CONFLICT(task_id, repository_id, project_path, mr_iid) DO NOTHING`,
row.autoFix, row.autoMerge, row.promptReview, row.promptMerged, row.promptClosed,
now, now, row.taskID); err != nil {
return fmt.Errorf("fan out task MR options for %s: %w", row.taskID, err)
}
if _, err := tx.Exec(
`UPDATE gitlab_task_mr_options SET mr_scope_migrated_at = ? WHERE task_id = ?`, now, row.taskID,
); err != nil {
return fmt.Errorf("stamp mr_scope_migrated_at for %s: %w", row.taskID, err)
}
return tx.Commit()
}
func addMissingColumns(s *Store, table string, columns []struct{ name, ddl string }) error {
existing, err := s.tableColumns(table)
if err != nil {
@@ -112,7 +225,7 @@ type execContext interface {
ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error)
}
const mrAutomationOptionsSelectCols = `task_id, auto_fix_enabled, auto_merge_enabled, auto_fix_prompt_override,
const mrAutomationOptionsSelectCols = `task_id, automation_revision, auto_fix_enabled, auto_merge_enabled, auto_fix_prompt_override,
prompt_on_review_requested, prompt_on_merged, prompt_on_closed,
review_reviewer_username, created_at, updated_at`
@@ -142,20 +255,23 @@ func boolPatchValue(value *bool) (bool, bool) {
return true, *value
}
// UpdateTaskMRAutomationOptions applies a partial update. Every column write
// is a single atomic UPDATE ... CASE WHEN <field was patched> THEN <new
// value> ELSE <column> END statement — there is no read-modify-write step for
// the values themselves, so two concurrent PATCHes touching different fields
// cannot lose one side's change to the other's stale snapshot (this does NOT
// hold under a bare transaction + full-row read-then-upsert on PostgreSQL,
// which only guarantees isolation between statements a transaction actually
// executes, not implicit serialization of concurrent read-then-write pairs).
// UpdateTaskMRAutomationOptions applies a partial update to the genuinely
// task-level MR automation fields: the auto-fix prompt override and the
// server-resolved reviewer username. The five automation switches are
// per-MR — see UpdateTaskMRAutomationOptionsForMR.
//
// Every column write is a single atomic UPDATE ... CASE WHEN <field was
// patched> THEN <new value> ELSE <column> END statement — there is no
// read-modify-write step for the values themselves, so two concurrent PATCHes
// touching different fields cannot lose one side's change to the other's
// stale snapshot (this does NOT hold under a bare transaction + full-row
// read-then-upsert on PostgreSQL, which only guarantees isolation between
// statements a transaction actually executes, not implicit serialization of
// concurrent read-then-write pairs).
//
// The one read this still performs (of the pre-patch row) exists solely to
// decide whether the review-request baseline and per-event terminal
// checkpoints need resetting — a stale read there only risks a redundant or
// skipped reset on a genuinely concurrent flip of the exact same field,
// never a lost field write.
// detect a reviewer-identity change, which invalidates every linked MR's
// review-request baseline at once.
func (s *Store) UpdateTaskMRAutomationOptions(
ctx context.Context, taskID string, patch TaskMRAutomationPatch, reviewerUsername *string,
) (*TaskMRAutomationOptions, error) {
@@ -164,41 +280,7 @@ func (s *Store) UpdateTaskMRAutomationOptions(
return nil, err
}
defer func() { _ = tx.Rollback() }()
now := time.Now().UTC()
if _, err := tx.ExecContext(ctx, `
INSERT INTO gitlab_task_mr_options (task_id, created_at, updated_at)
VALUES (?, ?, ?)
ON CONFLICT(task_id) DO NOTHING`, taskID, now, now); err != nil {
return nil, err
}
var previous TaskMRAutomationOptions
if err := tx.GetContext(ctx, &previous,
`SELECT `+mrAutomationOptionsSelectCols+`
FROM gitlab_task_mr_options WHERE task_id = ?`, taskID); err != nil {
return nil, err
}
fields := mrAutomationPatchFields(patch, reviewerUsername)
if _, err := tx.ExecContext(ctx, `
UPDATE gitlab_task_mr_options SET
auto_fix_enabled = CASE WHEN ? THEN ? ELSE auto_fix_enabled END,
auto_merge_enabled = CASE WHEN ? THEN ? ELSE auto_merge_enabled END,
auto_fix_prompt_override = CASE WHEN ? THEN ? ELSE auto_fix_prompt_override END,
prompt_on_review_requested = CASE WHEN ? THEN ? ELSE prompt_on_review_requested END,
prompt_on_merged = CASE WHEN ? THEN ? ELSE prompt_on_merged END,
prompt_on_closed = CASE WHEN ? THEN ? ELSE prompt_on_closed END,
review_reviewer_username = CASE WHEN ? THEN ? ELSE review_reviewer_username END,
updated_at = ?
WHERE task_id = ?`,
fields.autoFixSet, fields.autoFixValue, fields.autoMergeSet, fields.autoMergeValue,
fields.promptSet, fields.promptValue,
fields.reviewSet, fields.reviewValue, fields.mergedSet, fields.mergedValue,
fields.closedSet, fields.closedValue, fields.reviewerSet, fields.reviewerValue,
now, taskID); err != nil {
return nil, err
}
if err := applyMRAutomationOptionResets(ctx, tx, taskID, now, previous, fields); err != nil {
if err := updateTaskMRAutomationOptionsTx(ctx, tx, taskID, patch, reviewerUsername); err != nil {
return nil, err
}
if err := tx.Commit(); err != nil {
@@ -207,39 +289,270 @@ func (s *Store) UpdateTaskMRAutomationOptions(
return s.GetTaskMRAutomationOptions(ctx, taskID)
}
// mrAutomationOptionsPatchFields flattens an options patch (plus the
// server-resolved reviewer username) into the "was this field present, what
// value" pairs both the atomic UPDATE and the reset-decision logic need.
type mrAutomationOptionsPatchFields struct {
// UpdateTaskMRAutomationOptionsWithMRs applies task-level fields and one
// per-MR switch patch in the same transaction. A mixed PATCH must either
// commit both kinds of settings or leave neither behind.
func (s *Store) UpdateTaskMRAutomationOptionsWithMRs(
ctx context.Context, taskID string, patch TaskMRAutomationPatch, reviewerUsername *string,
ids []MRIdentity, switches TaskMRAutomationSwitchPatch,
) (*TaskMRAutomationOptions, error) {
tx, err := s.db.BeginTxx(ctx, nil)
if err != nil {
return nil, err
}
defer func() { _ = tx.Rollback() }()
if err := updateTaskMRAutomationOptionsTx(ctx, tx, taskID, patch, reviewerUsername); err != nil {
return nil, err
}
if err := applyMRSwitchPatchBatchTx(ctx, tx, taskID, ids, switches); err != nil {
return nil, err
}
if err := tx.Commit(); err != nil {
return nil, err
}
return s.GetTaskMRAutomationOptions(ctx, taskID)
}
// updateTaskMRAutomationOptionsTx applies the task-level half of an
// automation patch in its caller's transaction.
func updateTaskMRAutomationOptionsTx(
ctx context.Context, tx *sqlx.Tx, taskID string, patch TaskMRAutomationPatch, reviewerUsername *string,
) error {
now := time.Now().UTC()
if _, err := tx.ExecContext(ctx, `
INSERT INTO gitlab_task_mr_options (task_id, created_at, updated_at)
VALUES (?, ?, ?)
ON CONFLICT(task_id) DO NOTHING`, taskID, now, now); err != nil {
return err
}
var previous TaskMRAutomationOptions
if err := tx.GetContext(ctx, &previous,
`SELECT `+mrAutomationOptionsSelectCols+`
FROM gitlab_task_mr_options WHERE task_id = ?`, taskID); err != nil {
return err
}
promptSet := patch.AutoFixPromptOverride != nil
promptValue := normalizedMRPromptOverride(patch.AutoFixPromptOverride)
reviewerSet := reviewerUsername != nil
reviewerValue := ""
if reviewerSet {
reviewerValue = *reviewerUsername
}
if _, err := tx.ExecContext(ctx, `
UPDATE gitlab_task_mr_options SET
auto_fix_prompt_override = CASE WHEN ? THEN ? ELSE auto_fix_prompt_override END,
review_reviewer_username = CASE WHEN ? THEN ? ELSE review_reviewer_username END,
automation_revision = automation_revision + 1,
updated_at = ?
WHERE task_id = ?`,
promptSet, promptValue, reviewerSet, reviewerValue, now, taskID); err != nil {
return err
}
// A changed connected GitLab account invalidates every linked MR's
// review-request baseline, not just one MR's: a baseline recorded against
// the old identity would otherwise survive and could suppress or misfire
// the next prompt evaluated against the new one.
if reviewerSet && previous.ReviewReviewerUsername != reviewerValue {
if err := resetReviewBaselinesForTask(ctx, tx, taskID); err != nil {
return err
}
}
return nil
}
const mrAutomationSwitchSelectCols = `task_id, repository_id, project_path, mr_iid,
auto_fix_enabled, auto_merge_enabled, prompt_on_review_requested,
prompt_on_merged, prompt_on_closed, created_at, updated_at`
// GetTaskMRAutomationOptionsForMR returns one linked MR's automation
// switches, or all-off defaults when nothing has been persisted for it yet.
func (s *Store) GetTaskMRAutomationOptionsForMR(
ctx context.Context, taskID string, id MRIdentity,
) (*TaskMRAutomationOptionsForMR, error) {
var row TaskMRAutomationOptionsForMR
err := s.ro.GetContext(ctx, &row, `
SELECT `+mrAutomationSwitchSelectCols+`
FROM gitlab_task_mr_automation_options
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
taskID, id.RepositoryID, id.ProjectPath, id.MRIID)
if errors.Is(err, sql.ErrNoRows) {
return &TaskMRAutomationOptionsForMR{
TaskID: taskID, RepositoryID: id.RepositoryID,
ProjectPath: id.ProjectPath, MRIID: id.MRIID,
}, nil
}
if err != nil {
return nil, err
}
return &row, nil
}
// ListTaskMRAutomationOptions returns every stored per-MR switch row for a task.
func (s *Store) ListTaskMRAutomationOptions(ctx context.Context, taskID string) ([]*TaskMRAutomationOptionsForMR, error) {
var rows []TaskMRAutomationOptionsForMR
if err := s.ro.SelectContext(ctx, &rows, `
SELECT `+mrAutomationSwitchSelectCols+`
FROM gitlab_task_mr_automation_options
WHERE task_id = ? ORDER BY project_path ASC, mr_iid ASC`, taskID); err != nil {
return nil, err
}
out := make([]*TaskMRAutomationOptionsForMR, 0, len(rows))
for i := range rows {
out = append(out, &rows[i])
}
return out, nil
}
// UpdateTaskMRAutomationOptionsForMR applies a partial update to one linked
// MR's five automation switches, upserting the row when absent. It carries
// the same atomic CASE-WHEN write and single pre-patch read as
// UpdateTaskMRAutomationOptions; here the read decides which of that MR's
// checkpoints need resetting.
func (s *Store) UpdateTaskMRAutomationOptionsForMR(
ctx context.Context, taskID string, id MRIdentity, patch TaskMRAutomationSwitchPatch,
) (*TaskMRAutomationOptionsForMR, error) {
if err := s.UpdateTaskMRAutomationOptionsForMRs(ctx, taskID, []MRIdentity{id}, patch); err != nil {
return nil, err
}
return s.GetTaskMRAutomationOptionsForMR(ctx, taskID, id)
}
// UpdateTaskMRAutomationOptionsForMRs applies one switch patch to every
// identity in ids inside a SINGLE transaction. The fan-out case (a PATCH or
// MCP call that names no MR, so the change applies to every linked MR) must
// not be a loop of independent transactions: a failure partway through would
// leave the switch armed on the MRs already committed while returning an
// error to the caller, and auto-merge would then act on those MRs even though
// the operation reported failure. All-or-nothing instead.
func (s *Store) UpdateTaskMRAutomationOptionsForMRs(
ctx context.Context, taskID string, ids []MRIdentity, patch TaskMRAutomationSwitchPatch,
) error {
if len(ids) == 0 {
return nil
}
tx, err := s.db.BeginTxx(ctx, nil)
if err != nil {
return err
}
defer func() { _ = tx.Rollback() }()
if err := applyMRSwitchPatchBatchTx(ctx, tx, taskID, ids, patch); err != nil {
return err
}
return tx.Commit()
}
// applyMRSwitchPatchBatchTx applies a switch patch to every identity inside
// an existing transaction so task-level and per-MR changes can commit together.
func applyMRSwitchPatchBatchTx(
ctx context.Context, tx *sqlx.Tx, taskID string, ids []MRIdentity, patch TaskMRAutomationSwitchPatch,
) error {
if !patch.HasAny() {
return nil
}
now := time.Now().UTC()
fields := mrAutomationSwitchFields(patch)
for _, id := range ids {
if err := applyMRSwitchPatchTx(ctx, tx, taskID, id, now, fields); err != nil {
return err
}
}
return nil
}
// applyMRSwitchPatchTx upserts one MR's switch row, applies the patch, and
// runs that MR's checkpoint resets — all against the caller's transaction, so
// several MRs can be updated atomically. Split out of
// UpdateTaskMRAutomationOptionsForMRs to keep it under the function-length
// limit.
func applyMRSwitchPatchTx(
ctx context.Context, tx *sqlx.Tx, taskID string, id MRIdentity,
now time.Time, fields mrAutomationSwitchPatchFields,
) error {
if err := ensureTaskMRLinkTx(ctx, tx, taskID, id); err != nil {
return err
}
if _, err := tx.ExecContext(ctx, `
INSERT INTO gitlab_task_mr_automation_options (
task_id, repository_id, project_path, mr_iid, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?)
ON CONFLICT(task_id, repository_id, project_path, mr_iid) DO NOTHING`,
taskID, id.RepositoryID, id.ProjectPath, id.MRIID, now, now); err != nil {
return err
}
var previous TaskMRAutomationOptionsForMR
if err := tx.GetContext(ctx, &previous, `
SELECT `+mrAutomationSwitchSelectCols+`
FROM gitlab_task_mr_automation_options
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
taskID, id.RepositoryID, id.ProjectPath, id.MRIID); err != nil {
return err
}
if _, err := tx.ExecContext(ctx, `
UPDATE gitlab_task_mr_automation_options SET
auto_fix_enabled = CASE WHEN ? THEN ? ELSE auto_fix_enabled END,
auto_merge_enabled = CASE WHEN ? THEN ? ELSE auto_merge_enabled END,
prompt_on_review_requested = CASE WHEN ? THEN ? ELSE prompt_on_review_requested END,
prompt_on_merged = CASE WHEN ? THEN ? ELSE prompt_on_merged END,
prompt_on_closed = CASE WHEN ? THEN ? ELSE prompt_on_closed END,
updated_at = ?
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
fields.autoFixSet, fields.autoFixValue, fields.autoMergeSet, fields.autoMergeValue,
fields.reviewSet, fields.reviewValue, fields.mergedSet, fields.mergedValue,
fields.closedSet, fields.closedValue,
now, taskID, id.RepositoryID, id.ProjectPath, id.MRIID); err != nil {
return err
}
return applyMRAutomationOptionResets(ctx, tx, taskID, id, now, previous, fields)
}
func ensureTaskMRLinkTx(ctx context.Context, tx *sqlx.Tx, taskID string, id MRIdentity) error {
// A plain SELECT is not enough here. PostgreSQL's READ COMMITTED isolation
// allows an unlink to delete the association after the read and before the
// options INSERT, which leaves an orphan row that can re-arm on relink.
// The no-op UPDATE takes a write lock on the association row and is portable
// across PostgreSQL and SQLite. The unlink transaction must wait until this
// transaction commits, then removes the association and its options.
result, err := tx.ExecContext(ctx, `
UPDATE gitlab_task_mrs SET updated_at = updated_at
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
taskID, id.RepositoryID, id.ProjectPath, id.MRIID)
if err != nil {
return err
}
linked, err := result.RowsAffected()
if err != nil {
return err
}
if linked == 0 {
return fmt.Errorf("%w: project_path=%s mr_iid=%d", ErrTaskMRNotLinked, id.ProjectPath, id.MRIID)
}
return nil
}
// mrAutomationSwitchPatchFields flattens a switch patch into the "was this
// field present, what value" pairs both the atomic UPDATE and the
// reset-decision logic need.
type mrAutomationSwitchPatchFields struct {
autoFixSet, autoFixValue bool
autoMergeSet, autoMergeValue bool
promptSet bool
promptValue *string
reviewSet, reviewValue bool
mergedSet, mergedValue bool
closedSet, closedValue bool
reviewerSet bool
reviewerValue string
}
func mrAutomationPatchFields(patch TaskMRAutomationPatch, reviewerUsername *string) mrAutomationOptionsPatchFields {
func mrAutomationSwitchFields(patch TaskMRAutomationSwitchPatch) mrAutomationSwitchPatchFields {
autoFixSet, autoFixValue := boolPatchValue(patch.AutoFixEnabled)
autoMergeSet, autoMergeValue := boolPatchValue(patch.AutoMergeEnabled)
reviewSet, reviewValue := boolPatchValue(patch.PromptOnReviewRequested)
mergedSet, mergedValue := boolPatchValue(patch.PromptOnMerged)
closedSet, closedValue := boolPatchValue(patch.PromptOnClosed)
reviewerValue := ""
if reviewerUsername != nil {
reviewerValue = *reviewerUsername
}
return mrAutomationOptionsPatchFields{
return mrAutomationSwitchPatchFields{
autoFixSet: autoFixSet, autoFixValue: autoFixValue,
autoMergeSet: autoMergeSet, autoMergeValue: autoMergeValue,
promptSet: patch.AutoFixPromptOverride != nil, promptValue: normalizedMRPromptOverride(patch.AutoFixPromptOverride),
reviewSet: reviewSet, reviewValue: reviewValue,
mergedSet: mergedSet, mergedValue: mergedValue,
closedSet: closedSet, closedValue: closedValue,
reviewerSet: reviewerUsername != nil, reviewerValue: reviewerValue,
}
}
@@ -253,36 +566,28 @@ func normalizedMRPromptOverride(override *string) *string {
return override
}
// applyMRAutomationOptionResets resets the review-request baseline, a
// terminal event's checkpoint, or the auto-fix round-cap state when the
// corresponding switch actually changed value against the pre-patch row.
// Mirrors GitHub's applyTaskCIOptionResets.
// applyMRAutomationOptionResets resets the targeted MR's review-request
// baseline, a terminal event's checkpoint, or the auto-fix round-cap state
// when the corresponding switch actually changed value against the pre-patch
// row. Every reset is scoped to the one MR being patched, so reconfiguring
// one linked MR never re-arms another's checkpoints. Mirrors GitHub's
// applyTaskPRAutomationOptionResets.
func applyMRAutomationOptionResets(
ctx context.Context, tx execContext, taskID string, now time.Time,
previous TaskMRAutomationOptions, fields mrAutomationOptionsPatchFields,
ctx context.Context, tx execContext, taskID string, id MRIdentity, now time.Time,
previous TaskMRAutomationOptionsForMR, fields mrAutomationSwitchPatchFields,
) error {
// Reset on either a boolean flip or a reviewer-identity change: a patch
// that resends prompt_on_review_requested=true while it was already true
// still re-resolves the authenticated username
// (resolveReviewerUsernameForPatch), which can differ from the stored one
// after the workspace's connected GitLab account changes. Without the
// second condition, a baseline recorded against the old identity would
// survive and could suppress or misfire the next prompt evaluated
// against the new one.
reviewChanged := (fields.reviewSet && previous.PromptOnReviewRequested != fields.reviewValue) ||
(fields.reviewerSet && previous.ReviewReviewerUsername != fields.reviewerValue)
if reviewChanged {
if err := resetReviewBaselinesForTask(ctx, tx, taskID); err != nil {
if fields.reviewSet && previous.PromptOnReviewRequested != fields.reviewValue {
if err := resetReviewBaselineForMR(ctx, tx, taskID, id); err != nil {
return err
}
}
if err := resetMRTerminalCheckpointsOnReenable(ctx, tx, taskID, now, previous, fields); err != nil {
if err := resetMRTerminalCheckpointsOnReenable(ctx, tx, taskID, id, now, previous, fields); err != nil {
return err
}
// Re-enabling auto-fix after it was off must clear the round cap and
// exhaustion so a fresh evaluation pass can dispatch again (AC11).
if fields.autoFixSet && fields.autoFixValue && !previous.AutoFixEnabled {
if err := resetMRAutoFixState(ctx, tx, taskID, now); err != nil {
if err := resetMRAutoFixState(ctx, tx, taskID, id, now); err != nil {
return err
}
}
@@ -296,27 +601,27 @@ func applyMRAutomationOptionResets(
// applyMRAutomationOptionResets to keep that function under the cyclomatic
// complexity limit.
func resetMRTerminalCheckpointsOnReenable(
ctx context.Context, tx execContext, taskID string, now time.Time,
previous TaskMRAutomationOptions, fields mrAutomationOptionsPatchFields,
ctx context.Context, tx execContext, taskID string, id MRIdentity, now time.Time,
previous TaskMRAutomationOptionsForMR, fields mrAutomationSwitchPatchFields,
) error {
if fields.mergedSet && fields.mergedValue && !previous.PromptOnMerged {
if err := resetMRTerminalCheckpoint(ctx, tx, taskID, gitlabStateMerged, now); err != nil {
if err := resetMRTerminalCheckpoint(ctx, tx, taskID, id, gitlabStateMerged, now); err != nil {
return err
}
}
if fields.closedSet && fields.closedValue && !previous.PromptOnClosed {
if err := resetMRTerminalCheckpoint(ctx, tx, taskID, gitlabStateClosed, now); err != nil {
if err := resetMRTerminalCheckpoint(ctx, tx, taskID, id, gitlabStateClosed, now); err != nil {
return err
}
}
return nil
}
// resetMRAutoFixState clears every MR auto-fix round-cap/checkpoint column
// for a task, mirroring GitHub's resetTaskCIAutoFixState. A previously
// recorded exhaustion error is cleared along with it; any other kind of
// error (sync, merge) is left untouched.
func resetMRAutoFixState(ctx context.Context, exec execContext, taskID string, now time.Time) error {
// resetMRAutoFixState clears one MR's auto-fix round-cap/checkpoint columns,
// mirroring GitHub's resetTaskCIAutoFixState. A previously recorded
// exhaustion error is cleared along with it; any other kind of error (sync,
// merge) is left untouched.
func resetMRAutoFixState(ctx context.Context, exec execContext, taskID string, id MRIdentity, now time.Time) error {
_, err := exec.ExecContext(ctx, `
UPDATE gitlab_task_mr_state
SET auto_fix_round_count = 0,
@@ -327,7 +632,8 @@ func resetMRAutoFixState(ctx context.Context, exec execContext, taskID string, n
last_error = CASE WHEN auto_fix_exhausted_at IS NOT NULL THEN NULL ELSE last_error END,
auto_fix_exhausted_at = NULL,
updated_at = ?
WHERE task_id = ?`, now, taskID)
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
now, taskID, id.RepositoryID, id.ProjectPath, id.MRIID)
return err
}
@@ -553,6 +859,10 @@ func (s *Store) RebindTaskMRReviewer(ctx context.Context, taskID, username strin
return true, nil
}
// resetReviewBaselinesForTask clears every linked MR's review-request
// baseline. Reserved for a change of the task-level reviewer identity, which
// invalidates all of them at once; a single MR's switch flip goes through
// resetReviewBaselineForMR instead.
func resetReviewBaselinesForTask(ctx context.Context, exec execContext, taskID string) error {
_, err := exec.ExecContext(ctx, `
UPDATE gitlab_task_mr_state
@@ -561,12 +871,24 @@ func resetReviewBaselinesForTask(ctx context.Context, exec execContext, taskID s
return err
}
// resetMRTerminalCheckpoint clears the terminal checkpoint for a task's MR
// rows currently observed in (or last recorded as) the given terminal state,
// so a switch re-enabled after being off can re-evaluate and re-fire for an
// MR that reached that state while the switch was disabled. Matches GitHub's
// resetTaskCITerminalCheckpoint.
func resetMRTerminalCheckpoint(ctx context.Context, exec execContext, taskID, state string, now time.Time) error {
// resetReviewBaselineForMR clears one linked MR's review-request baseline.
func resetReviewBaselineForMR(ctx context.Context, exec execContext, taskID string, id MRIdentity) error {
_, err := exec.ExecContext(ctx, `
UPDATE gitlab_task_mr_state
SET review_request_initialized = 0, last_review_requested = 0, updated_at = ?
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
time.Now().UTC(), taskID, id.RepositoryID, id.ProjectPath, id.MRIID)
return err
}
// resetMRTerminalCheckpoint clears the terminal checkpoint for one MR when it
// is currently observed in (or was last recorded as) the given terminal
// state, so a switch re-enabled after being off can re-evaluate and re-fire
// for an MR that reached that state while the switch was disabled. Matches
// GitHub's resetTaskCITerminalCheckpoint.
func resetMRTerminalCheckpoint(
ctx context.Context, exec execContext, taskID string, id MRIdentity, state string, now time.Time,
) error {
_, err := exec.ExecContext(ctx, `
UPDATE gitlab_task_mr_state
SET last_observed_state = '',
@@ -574,21 +896,28 @@ func resetMRTerminalCheckpoint(ctx context.Context, exec execContext, taskID, st
last_lifecycle_prompt_at = NULL,
last_lifecycle_session_id = NULL,
updated_at = ?
WHERE task_id = ? AND (last_observed_state = ? OR last_lifecycle_event = ?)`,
now, taskID, state, state)
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?
AND (last_observed_state = ? OR last_lifecycle_event = ?)`,
now, taskID, id.RepositoryID, id.ProjectPath, id.MRIID, state, state)
return err
}
// ListAutomationSubscribedTaskMRs returns every linked MR (gitlab_task_mrs
// row) whose task has at least one lifecycle switch OR auto-fix OR
// auto-merge enabled. Drives the poller's sync pass (AC22); widened from
// the #2125 lifecycle-only ListLifecycleSubscribedTaskMRs so auto-fix and
// auto-merge get evaluated on the same poll without a second query.
// row) that has at least one lifecycle switch OR auto-fix OR auto-merge
// enabled. Drives the poller's sync pass (AC22); widened from the #2125
// lifecycle-only ListLifecycleSubscribedTaskMRs so auto-fix and auto-merge
// get evaluated on the same poll without a second query. The join is on the
// full MR identity, not just task_id, so a task's unconfigured MRs are not
// polled just because a sibling MR has a switch on.
func (s *Store) ListAutomationSubscribedTaskMRs(ctx context.Context) ([]*TaskMR, error) {
var mrs []TaskMR
if err := s.ro.SelectContext(ctx, &mrs, `
SELECT `+taskMRSelectColsQualified+` FROM gitlab_task_mrs gtm
INNER JOIN gitlab_task_mr_options o ON o.task_id = gtm.task_id
INNER JOIN gitlab_task_mr_automation_options o
ON o.task_id = gtm.task_id
AND o.repository_id = gtm.repository_id
AND o.project_path = gtm.project_path
AND o.mr_iid = gtm.mr_iid
WHERE o.prompt_on_review_requested = 1 OR o.prompt_on_merged = 1 OR o.prompt_on_closed = 1
OR o.auto_fix_enabled = 1 OR o.auto_merge_enabled = 1
ORDER BY gtm.created_at ASC`); err != nil {
@@ -636,7 +965,9 @@ func (s *Store) RecordTaskMRFixAttempt(ctx context.Context, attempt TaskMRFixAtt
}
// RefreshTaskMRFixCheckpoint updates the current feedback checkpoint
// without recording a new prompt dispatch (the empty-delta path — AC7).
// without recording a new prompt dispatch (the empty-delta path — AC7). It
// is called only at the start of a fresh auto-fix cycle, so replacing a
// conflicting row intentionally clears its stale enqueue/session markers.
// Mirrors GitHub's RefreshTaskCIFixCheckpoint.
func (s *Store) RefreshTaskMRFixCheckpoint(ctx context.Context, taskID, repositoryID, projectPath string, mrIID int, signature, checkpointJSON string) error {
ctx = context.WithoutCancel(ctx)
@@ -717,6 +1048,11 @@ func (s *Store) deleteMRAutomationForWorkspace(ctx context.Context, tx execConte
(SELECT id FROM tasks WHERE workspace_id = ?)`, workspaceID); err != nil {
return fmt.Errorf("delete gitlab_task_mr_state: %w", err)
}
if _, err := tx.ExecContext(ctx, `
DELETE FROM gitlab_task_mr_automation_options WHERE task_id IN
(SELECT id FROM tasks WHERE workspace_id = ?)`, workspaceID); err != nil {
return fmt.Errorf("delete gitlab_task_mr_automation_options: %w", err)
}
if _, err := tx.ExecContext(ctx, `
DELETE FROM gitlab_task_mr_options WHERE task_id IN
(SELECT id FROM tasks WHERE workspace_id = ?)`, workspaceID); err != nil {
@@ -0,0 +1,42 @@
package gitlab
import (
"context"
"strings"
"testing"
)
func TestStore_UpdateTaskMRAutomationOptionsForMR_LocksLinkBeforeWritingOptions(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
id := mrIdentity("group/linked", 7)
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", id.ProjectPath, id.MRIID)); err != nil {
t.Fatalf("link MR: %v", err)
}
// A concurrent unlink must be serialized behind the link validation write.
// This trigger stands in for a database-side conflict: the old read-only
// validation never touches the trigger and would incorrectly create an
// options row, while the locking validation surfaces the conflict before
// it can write one.
if _, err := store.db.Exec(`
CREATE TRIGGER reject_link_update BEFORE UPDATE ON gitlab_task_mrs
BEGIN SELECT RAISE(ABORT, 'simulated concurrent unlink'); END`); err != nil {
t.Fatalf("create link conflict trigger: %v", err)
}
_, err := store.UpdateTaskMRAutomationOptionsForMR(
ctx, "task-1", id, TaskMRAutomationSwitchPatch{AutoMergeEnabled: boolPtr(true)},
)
if err == nil || !strings.Contains(err.Error(), "simulated concurrent unlink") {
t.Fatalf("expected link conflict before options write, got %v", err)
}
options, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("list options: %v", err)
}
if len(options) != 0 {
t.Fatalf("link conflict created automation options: %+v", options)
}
}
@@ -2,6 +2,7 @@ package gitlab
import (
"context"
"errors"
"path/filepath"
"testing"
"time"
@@ -13,36 +14,140 @@ import (
func boolPtr(b bool) *bool { return &b }
func stringPtr(s string) *string { return &s }
func intPtr(i int) *int { return &i }
// TestStore_UpdateTaskMRAutomationOptions_AutoMergeAndPromptOverrideRoundTrip
// closes a coverage gap: auto_fix_enabled is exercised indirectly by
// TestStore_UpdateTaskMRAutomationOptions_ReenablingAutoFixResetsRoundCap,
// but nothing wrote auto_merge_enabled or auto_fix_prompt_override and read
// them back, including the empty-string-normalizes-to-NULL path
// (normalizedMRPromptOverride).
func TestStore_UpdateTaskMRAutomationOptions_AutoMergeAndPromptOverrideRoundTrip(t *testing.T) {
// mrIdentity builds the single-repo MR identity most of these tests use.
func mrIdentity(projectPath string, iid int) MRIdentity {
return MRIdentity{ProjectPath: projectPath, MRIID: iid}
}
// setMRSwitches is the per-MR switch write these tests exercise, kept short
// so the assertions stay the focus.
func setMRSwitches(
t *testing.T, store *Store, taskID string, id MRIdentity, patch TaskMRAutomationSwitchPatch,
) *TaskMRAutomationOptionsForMR {
t.Helper()
mrs, err := store.ListTaskMRsByTask(context.Background(), taskID)
if err != nil {
t.Fatalf("list linked MRs: %v", err)
}
linked := false
for _, mr := range mrs {
if (MRIdentity{RepositoryID: mr.RepositoryID, ProjectPath: mr.ProjectPath, MRIID: mr.MRIID}) == id {
linked = true
break
}
}
if !linked {
if err := store.UpsertTaskMR(context.Background(), newTestMR(taskID, id.RepositoryID, id.ProjectPath, id.MRIID)); err != nil {
t.Fatalf("link MR for switch write: %v", err)
}
}
got, err := store.UpdateTaskMRAutomationOptionsForMR(context.Background(), taskID, id, patch)
if err != nil {
t.Fatalf("UpdateTaskMRAutomationOptionsForMR(%s, %+v): %v", taskID, id, err)
}
return got
}
func TestStore_UpdateTaskMRAutomationOptionsForMR_RejectsUnlinkedMR(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
id := mrIdentity("group/not-linked", 99)
updated, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
AutoMergeEnabled: boolPtr(true),
}, nil)
if err != nil {
t.Fatalf("enable auto-merge: %v", err)
_, err := store.UpdateTaskMRAutomationOptionsForMR(
ctx, "task-1", id, TaskMRAutomationSwitchPatch{AutoMergeEnabled: boolPtr(true)},
)
if !errors.Is(err, ErrTaskMRNotLinked) {
t.Fatalf("expected ErrTaskMRNotLinked, got %v", err)
}
options, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("list options: %v", err)
}
if len(options) != 0 {
t.Fatalf("unlinked write created automation options: %+v", options)
}
}
// TestStore_UpdateTaskMRAutomationOptionsForMR_AutoMergeRoundTrip closes a
// coverage gap: auto_fix_enabled is exercised indirectly by
// TestStore_UpdateTaskMRAutomationOptionsForMR_ReenablingAutoFixResetsRoundCap,
// but nothing wrote auto_merge_enabled and read it back.
func TestStore_UpdateTaskMRAutomationOptionsForMR_AutoMergeRoundTrip(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
id := mrIdentity("group/a", 1)
updated := setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{
AutoMergeEnabled: boolPtr(true),
})
if !updated.AutoMergeEnabled {
t.Fatalf("AutoMergeEnabled = false immediately after patch, want true")
}
got, err := store.GetTaskMRAutomationOptions(ctx, "task-1")
got, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", id)
if err != nil {
t.Fatalf("GetTaskMRAutomationOptions: %v", err)
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if !got.AutoMergeEnabled {
t.Fatalf("AutoMergeEnabled = false after persisted read-back, want true")
}
updated, err = store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
// An unrelated per-MR patch must not disturb it.
setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{PromptOnClosed: boolPtr(true)})
got, err = store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", id)
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if !got.AutoMergeEnabled || !got.PromptOnClosed {
t.Fatalf("expected both switches on after independent patches, got %+v", got)
}
}
// TestStore_UpdateTaskMRAutomationOptionsForMR_ScopesSwitchesPerMR is the
// regression this change exists for: configuring one linked MR must leave
// every other MR on the same task untouched.
func TestStore_UpdateTaskMRAutomationOptionsForMR_ScopesSwitchesPerMR(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
first := mrIdentity("group/a", 1)
second := mrIdentity("group/b", 2)
setMRSwitches(t, store, "task-1", first, TaskMRAutomationSwitchPatch{
AutoFixEnabled: boolPtr(true), AutoMergeEnabled: boolPtr(true),
PromptOnMerged: boolPtr(true),
})
other, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", second)
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR(second): %v", err)
}
if other.AutoFixEnabled || other.AutoMergeEnabled || other.PromptOnMerged ||
other.PromptOnReviewRequested || other.PromptOnClosed {
t.Fatalf("second MR inherited the first MR's switches: %+v", other)
}
stored, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("ListTaskMRAutomationOptions: %v", err)
}
if len(stored) != 1 || stored[0].Identity() != first {
t.Fatalf("expected exactly the configured MR to be stored, got %+v", stored)
}
}
// TestStore_UpdateTaskMRAutomationOptions_PromptOverrideRoundTrip covers the
// one field that is still genuinely task-level, including the
// empty-string-normalizes-to-NULL path (normalizedMRPromptOverride).
func TestStore_UpdateTaskMRAutomationOptions_PromptOverrideRoundTrip(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
updated, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
AutoFixPromptOverride: stringPtr("custom prompt text"),
}, nil)
if err != nil {
@@ -51,16 +156,14 @@ func TestStore_UpdateTaskMRAutomationOptions_AutoMergeAndPromptOverrideRoundTrip
if updated.AutoFixPromptOverride == nil || *updated.AutoFixPromptOverride != "custom prompt text" {
t.Fatalf("AutoFixPromptOverride = %v immediately after patch, want \"custom prompt text\"", updated.AutoFixPromptOverride)
}
got, err = store.GetTaskMRAutomationOptions(ctx, "task-1")
firstRevision := updated.AutomationRevision
got, err := store.GetTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("GetTaskMRAutomationOptions: %v", err)
}
if got.AutoFixPromptOverride == nil || *got.AutoFixPromptOverride != "custom prompt text" {
t.Fatalf("AutoFixPromptOverride = %v after persisted read-back, want \"custom prompt text\"", got.AutoFixPromptOverride)
}
if !got.AutoMergeEnabled {
t.Fatalf("AutoMergeEnabled reverted to false after an unrelated patch, want still true")
}
// Clearing via an empty string must normalize to NULL, not persist "".
updated, err = store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
@@ -72,6 +175,9 @@ func TestStore_UpdateTaskMRAutomationOptions_AutoMergeAndPromptOverrideRoundTrip
if updated.AutoFixPromptOverride != nil {
t.Fatalf("AutoFixPromptOverride = %v immediately after clearing, want nil", updated.AutoFixPromptOverride)
}
if updated.AutomationRevision <= firstRevision {
t.Fatalf("automation revision = %d after update, want > %d", updated.AutomationRevision, firstRevision)
}
got, err = store.GetTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("GetTaskMRAutomationOptions: %v", err)
@@ -94,37 +200,42 @@ func TestStore_GetTaskMRAutomationOptions_ImplicitDefault(t *testing.T) {
}
}
func TestStore_UpdateTaskMRAutomationOptions_RoundTrip(t *testing.T) {
func TestStore_UpdateTaskMRAutomationOptionsForMR_RoundTrip(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
updated, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
id := mrIdentity("group/a", 1)
updated := setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil)
if err != nil {
t.Fatalf("UpdateTaskMRAutomationOptions: %v", err)
}
})
if !updated.PromptOnMerged || updated.PromptOnReviewRequested || updated.PromptOnClosed {
t.Fatalf("unexpected options after first patch: %+v", updated)
}
username := "alice"
updated, err = store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
updated = setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{
PromptOnReviewRequested: boolPtr(true),
}, &username)
if err != nil {
t.Fatalf("UpdateTaskMRAutomationOptions second patch: %v", err)
}
if !updated.PromptOnMerged || !updated.PromptOnReviewRequested || updated.ReviewReviewerUsername != "alice" {
t.Fatalf("expected merged switch preserved and reviewer set, got %+v", updated)
})
if !updated.PromptOnMerged || !updated.PromptOnReviewRequested {
t.Fatalf("expected merged switch preserved alongside the new one, got %+v", updated)
}
got, err := store.GetTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("GetTaskMRAutomationOptions: %v", err)
// The reviewer username stays task-level.
username := "alice"
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{}, &username); err != nil {
t.Fatalf("persist reviewer: %v", err)
}
if !got.PromptOnMerged || !got.PromptOnReviewRequested || got.ReviewReviewerUsername != "alice" {
t.Fatalf("persisted options mismatch: %+v", got)
got, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", id)
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if !got.PromptOnMerged || !got.PromptOnReviewRequested {
t.Fatalf("persisted switches mismatch: %+v", got)
}
taskOpts, err := store.GetTaskMRAutomationOptions(ctx, "task-1")
if err != nil || taskOpts.ReviewReviewerUsername != "alice" {
t.Fatalf("persisted reviewer mismatch: %+v err=%v", taskOpts, err)
}
}
@@ -394,38 +505,33 @@ func TestStore_UpdateTaskMRAutomationOptions_ResendingSameSwitchResetsBaselineOn
}
}
// TestStore_UpdateTaskMRAutomationOptions_ReenablingMergedSwitchResetsCheckpoint
// TestStore_UpdateTaskMRAutomationOptionsForMR_ReenablingMergedSwitchResetsCheckpoint
// is the P1 finding: an MR that reached the merged state while the switch was
// off must not stay permanently suppressed once the switch is re-enabled.
func TestStore_UpdateTaskMRAutomationOptions_ReenablingMergedSwitchResetsCheckpoint(t *testing.T) {
// The sibling MR proves the reset is scoped to the MR being reconfigured.
func TestStore_UpdateTaskMRAutomationOptionsForMR_ReenablingMergedSwitchResetsCheckpoint(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
id := mrIdentity("group/a", 1)
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch: %v", err)
}
if err := store.RecordTaskMRLifecyclePrompt(ctx, TaskMRLifecyclePrompt{
TaskID: "task-1", ProjectPath: "group/a", MRIID: 1,
Event: gitlabStateMerged, PromptedAt: time.Now().UTC(), ObservedState: gitlabStateMerged,
}); err != nil {
t.Fatalf("record merged prompt: %v", err)
setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{PromptOnMerged: boolPtr(true)})
for _, mr := range []struct {
project string
iid int
}{{"group/a", 1}, {"group/b", 2}} {
if err := store.RecordTaskMRLifecyclePrompt(ctx, TaskMRLifecyclePrompt{
TaskID: "task-1", ProjectPath: mr.project, MRIID: mr.iid,
Event: gitlabStateMerged, PromptedAt: time.Now().UTC(), ObservedState: gitlabStateMerged,
}); err != nil {
t.Fatalf("record merged prompt for %s: %v", mr.project, err)
}
}
// Disable, then re-enable — the checkpoint from the still-merged MR must
// not survive, or the re-enabled switch would never fire for it again.
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
PromptOnMerged: boolPtr(false),
}, nil); err != nil {
t.Fatalf("disable switch: %v", err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("re-enable switch: %v", err)
}
setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{PromptOnMerged: boolPtr(false)})
setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{PromptOnMerged: boolPtr(true)})
got, err := store.GetTaskMRLifecycleState(ctx, "task-1", "", "group/a", 1)
if err != nil || got == nil {
@@ -434,22 +540,26 @@ func TestStore_UpdateTaskMRAutomationOptions_ReenablingMergedSwitchResetsCheckpo
if got.LastObservedState != "" || got.LastLifecycleEvent != "" {
t.Fatalf("expected terminal checkpoint reset after re-enabling the switch, got %+v", got)
}
sibling, err := store.GetTaskMRLifecycleState(ctx, "task-1", "", "group/b", 2)
if err != nil || sibling == nil {
t.Fatalf("GetTaskMRLifecycleState(sibling): %+v err=%v", sibling, err)
}
if sibling.LastLifecycleEvent != gitlabStateMerged {
t.Fatalf("sibling MR's checkpoint was cleared by another MR's switch flip: %+v", sibling)
}
}
// TestStore_UpdateTaskMRAutomationOptions_ReenablingAutoFixResetsRoundCap
// TestStore_UpdateTaskMRAutomationOptionsForMR_ReenablingAutoFixResetsRoundCap
// covers AC11: toggling auto-fix off and on again must clear
// auto_fix_exhausted_at and auto_fix_round_count so a subsequent failing
// pipeline can dispatch again.
func TestStore_UpdateTaskMRAutomationOptions_ReenablingAutoFixResetsRoundCap(t *testing.T) {
func TestStore_UpdateTaskMRAutomationOptionsForMR_ReenablingAutoFixResetsRoundCap(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedTask(t, store, "task-1", "")
id := mrIdentity("group/a", 1)
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
AutoFixEnabled: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable auto-fix: %v", err)
}
setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{AutoFixEnabled: boolPtr(true)})
if err := store.RecordTaskMRFixAttempt(ctx, TaskMRFixAttempt{
TaskID: "task-1", ProjectPath: "group/a", MRIID: 1,
Signature: "sig-1", CheckpointJSON: "{}", SessionID: "sess-1",
@@ -470,16 +580,8 @@ func TestStore_UpdateTaskMRAutomationOptions_ReenablingAutoFixResetsRoundCap(t *
}
// Disable, then re-enable — the round cap and exhaustion must clear.
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
AutoFixEnabled: boolPtr(false),
}, nil); err != nil {
t.Fatalf("disable auto-fix: %v", err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
AutoFixEnabled: boolPtr(true),
}, nil); err != nil {
t.Fatalf("re-enable auto-fix: %v", err)
}
setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{AutoFixEnabled: boolPtr(false)})
setMRSwitches(t, store, "task-1", id, TaskMRAutomationSwitchPatch{AutoFixEnabled: boolPtr(true)})
got, err = store.GetTaskMRLifecycleState(ctx, "task-1", "", "group/a", 1)
if err != nil || got == nil {
@@ -564,10 +666,13 @@ func TestStore_TaskDeleteCascadesMRAutomationRows(t *testing.T) {
seedTask(t, store, "task-1", "")
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
PromptOnMerged: boolPtr(true),
AutoFixPromptOverride: stringPtr("custom"),
}, nil); err != nil {
t.Fatalf("seed options: %v", err)
}
setMRSwitches(t, store, "task-1", mrIdentity("group/a", 1), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
})
if err := store.SetTaskMRObservedState(ctx, "task-1", "", "group/a", 1, "opened"); err != nil {
t.Fatalf("seed checkpoint: %v", err)
}
@@ -580,9 +685,16 @@ func TestStore_TaskDeleteCascadesMRAutomationRows(t *testing.T) {
if err != nil {
t.Fatalf("GetTaskMRAutomationOptions: %v", err)
}
if opts.PromptOnMerged {
if opts.AutoFixPromptOverride != nil {
t.Fatalf("expected options row cascaded away, got %+v", opts)
}
switches, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("ListTaskMRAutomationOptions: %v", err)
}
if len(switches) != 0 {
t.Fatalf("expected per-MR switch rows cascaded away, got %+v", switches)
}
state, err := store.GetTaskMRLifecycleState(ctx, "task-1", "", "group/a", 1)
if err != nil {
t.Fatalf("GetTaskMRLifecycleState: %v", err)
@@ -612,19 +724,15 @@ func TestStore_ListAutomationSubscribedTaskMRs(t *testing.T) {
if err := store.UpsertTaskMR(ctx, autoFixOnly); err != nil {
t.Fatalf("upsert auto-fix-only MR: %v", err)
}
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-1", TaskMRAutomationPatch{
setMRSwitches(t, store, "task-1", mrIdentity("group/subscribed", 1), TaskMRAutomationSwitchPatch{
PromptOnMerged: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable switch for task-1: %v", err)
}
})
// task-3 has no lifecycle switch on, only auto-fix — must still be
// returned since the query was widened to OR in auto_fix_enabled /
// auto_merge_enabled (this task's change).
if _, err := store.UpdateTaskMRAutomationOptions(ctx, "task-3", TaskMRAutomationPatch{
setMRSwitches(t, store, "task-3", mrIdentity("group/autofix", 3), TaskMRAutomationSwitchPatch{
AutoFixEnabled: boolPtr(true),
}, nil); err != nil {
t.Fatalf("enable auto-fix for task-3: %v", err)
}
})
rows, err := store.ListAutomationSubscribedTaskMRs(ctx)
if err != nil {
@@ -706,3 +814,155 @@ func assertMRAutomationTablesExist(t *testing.T, sqlxDB *sqlx.DB) {
}
}
}
// TestStore_DeleteTaskMRForWorkspace_DropsPerMRAutomationOptions covers the
// unlink half of the per-MR switch lifecycle. Leaving the switch row behind
// meant re-linking the same MR — by hand, or through push-detection auto-link
// — silently re-armed whatever was configured before the unlink, including
// auto-merge, with no surface showing it (taskMRAutomationOptionsList hides
// rows whose MR is not linked) but the evaluator still reading it.
func TestStore_DeleteTaskMRForWorkspace_DropsPerMRAutomationOptions(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
mr := newTestMR("task-1", "", "group/a", 1)
if err := store.UpsertTaskMR(ctx, mr); err != nil {
t.Fatalf("upsert MR: %v", err)
}
id := MRIdentity{RepositoryID: "", ProjectPath: "group/a", MRIID: 1}
if _, err := store.UpdateTaskMRAutomationOptionsForMR(
ctx, "task-1", id, TaskMRAutomationSwitchPatch{AutoMergeEnabled: boolPtr(true)},
); err != nil {
t.Fatalf("enable auto-merge: %v", err)
}
if err := store.SetTaskMRObservedState(ctx, "task-1", "", "group/a", 1, "merged"); err != nil {
t.Fatalf("seed lifecycle state: %v", err)
}
if err := store.DeleteTaskMRForWorkspace(ctx, "ws-1", mr.ID); err != nil {
t.Fatalf("DeleteTaskMRForWorkspace: %v", err)
}
stored, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("ListTaskMRAutomationOptions: %v", err)
}
if len(stored) != 0 {
t.Fatalf("unlink left automation rows behind: %+v", stored)
}
state, err := store.GetTaskMRLifecycleState(ctx, "task-1", "", "group/a", 1)
if err != nil {
t.Fatalf("get lifecycle state: %v", err)
}
if state != nil {
t.Fatalf("unlink left lifecycle state behind: %+v", state)
}
// Re-linking the same MR must start from all-off, not resurrect the
// pre-unlink configuration.
relinked := newTestMR("task-1", "", "group/a", 1)
if err := store.UpsertTaskMR(ctx, relinked); err != nil {
t.Fatalf("re-link MR: %v", err)
}
opts, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", id)
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if opts.AutoMergeEnabled {
t.Errorf("re-linked MR silently re-armed auto-merge: %+v", opts)
}
}
// TestStore_DeleteTaskMR_DropsPerMRAutomationOptions pins the same cleanup on
// the association-ID delete path, which cascades to gitlab_task_mr_state
// independently of DeleteTaskMRForWorkspace. The two must stay in step: a
// caller routed through this one would otherwise leave an enabled auto-merge
// switch behind for the next link of the same MR to inherit.
func TestStore_DeleteTaskMR_DropsPerMRAutomationOptions(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
mr := newTestMR("task-1", "", "group/a", 1)
if err := store.UpsertTaskMR(ctx, mr); err != nil {
t.Fatalf("upsert MR: %v", err)
}
id := MRIdentity{RepositoryID: "", ProjectPath: "group/a", MRIID: 1}
if _, err := store.UpdateTaskMRAutomationOptionsForMR(
ctx, "task-1", id, TaskMRAutomationSwitchPatch{AutoMergeEnabled: boolPtr(true)},
); err != nil {
t.Fatalf("enable auto-merge: %v", err)
}
if err := store.DeleteTaskMR(ctx, mr.ID); err != nil {
t.Fatalf("DeleteTaskMR: %v", err)
}
stored, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("ListTaskMRAutomationOptions: %v", err)
}
if len(stored) != 0 {
t.Fatalf("delete left automation rows behind: %+v", stored)
}
}
// TestStore_UpdateTaskMRAutomationOptionsForMRs_IsAllOrNothing pins the
// fan-out atomicity: a failure partway through must not leave the switch
// armed on the MRs processed before it while the caller is told the operation
// failed. A trigger supplies the deterministic mid-batch failure — the second
// identity's UPDATE aborts, and the first identity's already-applied write
// must roll back with it.
func TestStore_UpdateTaskMRAutomationOptionsForMRs_IsAllOrNothing(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
first := MRIdentity{RepositoryID: "", ProjectPath: "group/a", MRIID: 1}
second := MRIdentity{RepositoryID: "", ProjectPath: "group/b", MRIID: 2}
for _, id := range []MRIdentity{first, second} {
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", id.RepositoryID, id.ProjectPath, id.MRIID)); err != nil {
t.Fatalf("link MR %s: %v", id.ProjectPath, err)
}
}
if _, err := store.db.Exec(`
CREATE TRIGGER fail_second_mr BEFORE UPDATE ON gitlab_task_mr_automation_options
WHEN NEW.project_path = 'group/b'
BEGIN SELECT RAISE(ABORT, 'injected failure'); END`); err != nil {
t.Fatalf("create failure trigger: %v", err)
}
err := store.UpdateTaskMRAutomationOptionsForMRs(
ctx, "task-1", []MRIdentity{first, second},
TaskMRAutomationSwitchPatch{AutoMergeEnabled: boolPtr(true)},
)
if err == nil {
t.Fatal("expected the batch to fail on the second identity")
}
got, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", first)
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if got.AutoMergeEnabled {
t.Error("first MR kept auto-merge after the batch failed — fan-out was not atomic")
}
}
// TestStore_UpdateTaskMRAutomationOptionsForMRs_EmptyIsNoop guards the
// zero-target call so it cannot open an empty transaction per request.
func TestStore_UpdateTaskMRAutomationOptionsForMRs_EmptyIsNoop(t *testing.T) {
store := newTestStore(t)
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
if err := store.UpdateTaskMRAutomationOptionsForMRs(
context.Background(), "task-1", nil,
TaskMRAutomationSwitchPatch{AutoMergeEnabled: boolPtr(true)},
); err != nil {
t.Fatalf("empty batch should be a no-op, got %v", err)
}
}
@@ -0,0 +1,246 @@
package gitlab
import (
"context"
"path/filepath"
"testing"
"github.com/jmoiron/sqlx"
"github.com/kandev/kandev/internal/db"
)
// legacySwitches names the five task-wide booleans as they were stored
// before this change, so each test reads as the configuration it is
// simulating rather than as five positional bools.
type legacySwitches struct {
autoFix, autoMerge, review, merged, closed bool
}
// writeLegacyTaskMROptions puts a task back into the pre-scope-migration
// shape: task-wide switch values on gitlab_task_mr_options with the
// mr_scope_migrated_at marker cleared, exactly what an installation upgrading
// across this change starts from.
func writeLegacyTaskMROptions(t *testing.T, store *Store, taskID string, s legacySwitches) {
t.Helper()
if _, err := store.db.Exec(`
INSERT INTO gitlab_task_mr_options (
task_id, auto_fix_enabled, auto_merge_enabled, prompt_on_review_requested,
prompt_on_merged, prompt_on_closed, mr_scope_migrated_at, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
ON CONFLICT(task_id) DO UPDATE SET
auto_fix_enabled = excluded.auto_fix_enabled,
auto_merge_enabled = excluded.auto_merge_enabled,
prompt_on_review_requested = excluded.prompt_on_review_requested,
prompt_on_merged = excluded.prompt_on_merged,
prompt_on_closed = excluded.prompt_on_closed,
mr_scope_migrated_at = NULL`,
taskID, s.autoFix, s.autoMerge, s.review, s.merged, s.closed); err != nil {
t.Fatalf("write legacy options for %s: %v", taskID, err)
}
}
// TestStore_MigrateTaskMROptionsToMRScope_FansOutToEveryLinkedMR covers the
// upgrade path: a task that had the switches on task-wide keeps them on for
// every MR it had linked at the time, so nobody's configured automation
// silently stops working across the upgrade.
func TestStore_MigrateTaskMROptionsToMRScope_FansOutToEveryLinkedMR(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
for _, mr := range []*TaskMR{
newTestMR("task-1", "", "group/a", 1),
newTestMR("task-1", "", "group/b", 2),
} {
if err := store.UpsertTaskMR(ctx, mr); err != nil {
t.Fatalf("upsert MR %s: %v", mr.ProjectPath, err)
}
}
writeLegacyTaskMROptions(t, store, "task-1", legacySwitches{autoFix: true, merged: true})
if err := store.migrateTaskMROptionsToMRScope(); err != nil {
t.Fatalf("migrateTaskMROptionsToMRScope: %v", err)
}
stored, err := store.ListTaskMRAutomationOptions(ctx, "task-1")
if err != nil {
t.Fatalf("ListTaskMRAutomationOptions: %v", err)
}
if len(stored) != 2 {
t.Fatalf("expected one row per linked MR, got %+v", stored)
}
for _, row := range stored {
if !row.AutoFixEnabled || !row.PromptOnMerged {
t.Errorf("MR %s did not inherit the legacy switches: %+v", row.ProjectPath, row)
}
if row.AutoMergeEnabled || row.PromptOnReviewRequested || row.PromptOnClosed {
t.Errorf("MR %s gained a switch the task never had on: %+v", row.ProjectPath, row)
}
}
}
// TestStore_MigrateTaskMROptionsToMRScope_DoesNotReplayOverADeliberateDisable
// is what the mr_scope_migrated_at marker exists for: once the fan-out has
// run, a later boot must not re-enable a switch the user has since turned off
// for one specific MR.
func TestStore_MigrateTaskMROptionsToMRScope_DoesNotReplayOverADeliberateDisable(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
first := mrIdentity("group/a", 1)
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/a", 1)); err != nil {
t.Fatalf("upsert MR: %v", err)
}
writeLegacyTaskMROptions(t, store, "task-1", legacySwitches{merged: true})
if err := store.migrateTaskMROptionsToMRScope(); err != nil {
t.Fatalf("first migration: %v", err)
}
setMRSwitches(t, store, "task-1", first, TaskMRAutomationSwitchPatch{PromptOnMerged: boolPtr(false)})
// Simulates the next process start.
if err := store.migrateTaskMROptionsToMRScope(); err != nil {
t.Fatalf("replayed migration: %v", err)
}
got, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", first)
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if got.PromptOnMerged {
t.Fatalf("replayed migration re-enabled a deliberately disabled switch: %+v", got)
}
}
// TestStore_MigrateTaskMROptionsToMRScope_LaterLinkedMRStartsAllOff pins the
// other half of the marker's job: an MR linked *after* the fan-out must start
// with every switch off rather than inheriting the task's legacy values.
func TestStore_MigrateTaskMROptionsToMRScope_LaterLinkedMRStartsAllOff(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/a", 1)); err != nil {
t.Fatalf("upsert first MR: %v", err)
}
writeLegacyTaskMROptions(t, store, "task-1", legacySwitches{autoFix: true, autoMerge: true, merged: true, closed: true})
if err := store.migrateTaskMROptionsToMRScope(); err != nil {
t.Fatalf("first migration: %v", err)
}
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/late", 9)); err != nil {
t.Fatalf("upsert later MR: %v", err)
}
if err := store.migrateTaskMROptionsToMRScope(); err != nil {
t.Fatalf("replayed migration: %v", err)
}
got, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", mrIdentity("group/late", 9))
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if got.AutoFixEnabled || got.AutoMergeEnabled || got.PromptOnMerged || got.PromptOnClosed {
t.Fatalf("an MR linked after the migration inherited legacy switches: %+v", got)
}
}
// TestStore_MigrateTaskMROptionsToMRScope_UpgradesADBWithoutTheMarkerColumn
// runs the whole boot path against a database whose gitlab_task_mr_options
// predates mr_scope_migrated_at: the ALTER must add the column and the
// fan-out must then run against the rows already there.
func TestStore_MigrateTaskMROptionsToMRScope_UpgradesADBWithoutTheMarkerColumn(t *testing.T) {
tmp := t.TempDir()
dbConn, err := db.OpenSQLite(filepath.Join(tmp, "gitlab-legacy-scope.db"))
if err != nil {
t.Fatalf("open db: %v", err)
}
t.Cleanup(func() { _ = dbConn.Close() })
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
t.Cleanup(func() { _ = sqlxDB.Close() })
if _, err := sqlxDB.Exec(`
CREATE TABLE workspaces (id TEXT PRIMARY KEY);
CREATE TABLE tasks (id TEXT PRIMARY KEY, workspace_id TEXT NOT NULL DEFAULT '', archived_at DATETIME);
INSERT INTO tasks (id, workspace_id) VALUES ('task-1', 'ws-1');
CREATE TABLE gitlab_task_mr_options (
task_id TEXT PRIMARY KEY,
auto_fix_enabled BOOLEAN NOT NULL DEFAULT 0,
auto_merge_enabled BOOLEAN NOT NULL DEFAULT 0,
auto_fix_prompt_override TEXT,
prompt_on_review_requested BOOLEAN NOT NULL DEFAULT 0,
prompt_on_merged BOOLEAN NOT NULL DEFAULT 0,
prompt_on_closed BOOLEAN NOT NULL DEFAULT 0,
review_reviewer_username TEXT NOT NULL DEFAULT '',
created_at DATETIME NOT NULL,
updated_at DATETIME NOT NULL
);
INSERT INTO gitlab_task_mr_options (
task_id, auto_merge_enabled, prompt_on_closed, created_at, updated_at
) VALUES ('task-1', 1, 1, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP);`); err != nil {
t.Fatalf("create legacy schema: %v", err)
}
store, err := NewStore(sqlxDB, sqlxDB)
if err != nil {
t.Fatalf("NewStore against legacy DB: %v", err)
}
ctx := context.Background()
// The MR is linked before the *second* boot, so the first boot's fan-out
// found nothing to seed — the marker is already stamped, which is exactly
// the "starts all-off" contract.
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "", "group/a", 1)); err != nil {
t.Fatalf("upsert MR: %v", err)
}
if _, err := NewStore(sqlxDB, sqlxDB); err != nil {
t.Fatalf("second boot: %v", err)
}
columns, err := store.tableColumns("gitlab_task_mr_options")
if err != nil {
t.Fatalf("tableColumns: %v", err)
}
if _, ok := columns["mr_scope_migrated_at"]; !ok {
t.Fatalf("mr_scope_migrated_at missing after upgrading a legacy DB")
}
got, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1", mrIdentity("group/a", 1))
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if got.AutoMergeEnabled || got.PromptOnClosed {
t.Fatalf("an MR linked after the marker was stamped inherited legacy switches: %+v", got)
}
}
// TestStore_MigrateTaskMROptionsToMRScope_FansOutOnFirstBootOfALegacyDB is the
// companion to the test above: when the MR was already linked, the very first
// boot after the upgrade must carry the task's switches onto it.
func TestStore_MigrateTaskMROptionsToMRScope_FansOutOnFirstBootOfALegacyDB(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
seedWorkspace(t, store, "ws-1")
seedTask(t, store, "task-1", "ws-1")
if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "repo-1", "group/a", 1)); err != nil {
t.Fatalf("upsert MR: %v", err)
}
writeLegacyTaskMROptions(t, store, "task-1", legacySwitches{autoMerge: true, review: true})
if err := store.migrateTaskMROptionsToMRScope(); err != nil {
t.Fatalf("migrateTaskMROptionsToMRScope: %v", err)
}
got, err := store.GetTaskMRAutomationOptionsForMR(ctx, "task-1",
MRIdentity{RepositoryID: "repo-1", ProjectPath: "group/a", MRIID: 1})
if err != nil {
t.Fatalf("GetTaskMRAutomationOptionsForMR: %v", err)
}
if !got.AutoMergeEnabled || !got.PromptOnReviewRequested {
t.Fatalf("legacy switches did not reach the linked MR: %+v", got)
}
if got.RepositoryID != "repo-1" {
t.Fatalf("fan-out lost the MR's repository identity: %+v", got)
}
}
@@ -394,6 +394,27 @@ func (s *Store) DeleteTaskMRForWorkspace(ctx context.Context, workspaceID, assoc
); err != nil {
return fmt.Errorf("delete task MR refresh watch: %w", err)
}
if _, err = tx.ExecContext(ctx, `
DELETE FROM gitlab_task_mr_state
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
association.TaskID, association.RepositoryID, association.ProjectPath, association.MRIID,
); err != nil {
return fmt.Errorf("delete task MR lifecycle state: %w", err)
}
// Drop this MR's automation switches with the association. Leaving the row
// behind means re-linking the same MR later — by hand, or through
// push-detection auto-link — silently re-arms whatever was configured
// before it was unlinked, including auto-merge. Unlinking is the natural
// "stop automating this" action, so it must not leave a latent enabled
// switch that no surface displays (taskMRAutomationOptionsList hides rows
// whose MR is not linked) but the evaluator still reads.
if _, err = tx.ExecContext(ctx, `
DELETE FROM gitlab_task_mr_automation_options
WHERE task_id = ? AND repository_id = ? AND project_path = ? AND mr_iid = ?`,
association.TaskID, association.RepositoryID, association.ProjectPath, association.MRIID,
); err != nil {
return fmt.Errorf("delete task MR automation options: %w", err)
}
if _, err = tx.ExecContext(ctx, `DELETE FROM gitlab_task_mrs WHERE id = ?`, association.ID); err != nil {
return fmt.Errorf("delete task MR association: %w", err)
}
@@ -67,6 +67,12 @@ func (h *Handlers) mrAutomationErrorResponse(msg *ws.Message, taskID, logMsg str
if errors.Is(err, repoerrors.ErrTaskNotFound) || errors.Is(err, repoerrors.ErrWorkspaceNotFound) {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "task not found", nil)
}
// Naming an MR that isn't linked to this task (or naming one only
// partially) is a caller mistake — report it as such instead of burying
// it in a generic internal error.
if errors.Is(err, gitlab.ErrTaskMRNotLinked) {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil)
}
h.logger.Error(logMsg, zap.String("task_id", taskID), zap.Error(err))
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "failed to process MR automation request", nil)
}
@@ -83,7 +89,13 @@ func (h *Handlers) handleUpdateTaskMRAutomation(ctx context.Context, msg *ws.Mes
if errResp != nil || err != nil {
return errResp, err
}
// repository_id/project_path/mr_iid optionally scope the switch changes
// to one linked MR; omitting all three applies them to every linked MR,
// which is what an agent with no MR identity to send gets.
var req struct {
RepositoryID *string `json:"repository_id"`
ProjectPath *string `json:"project_path"`
MRIID *int `json:"mr_iid"`
AutoFixEnabled *bool `json:"auto_fix_enabled"`
AutoMergeEnabled *bool `json:"auto_merge_enabled"`
AutoFixPromptOverride *string `json:"auto_fix_prompt_override"`
@@ -95,6 +107,9 @@ func (h *Handlers) handleUpdateTaskMRAutomation(ctx context.Context, msg *ws.Mes
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil)
}
patch := gitlab.TaskMRAutomationPatch{
RepositoryID: req.RepositoryID,
ProjectPath: req.ProjectPath,
MRIID: req.MRIID,
AutoFixEnabled: req.AutoFixEnabled,
AutoMergeEnabled: req.AutoMergeEnabled,
AutoFixPromptOverride: req.AutoFixPromptOverride,
@@ -91,3 +91,47 @@ func TestHandleGetTaskMRAutomation(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, ws.MessageTypeResponse, response.Type)
}
// TestHandleUpdateTaskMRAutomationForwardsMRIdentity covers the per-MR MCP
// contract: an agent that knows which MR it means can say so, and the
// identity reaches the service instead of being dropped into a fan-out.
func TestHandleUpdateTaskMRAutomationForwardsMRIdentity(t *testing.T) {
automation := &recordingTaskMRAutomationService{}
h := &Handlers{taskMRAutomation: automation, logger: testLogger(t).WithFields()}
msg := makeWSMessage(t, ws.ActionMCPUpdateTaskMRAutomation, map[string]any{
"task_id": "task-current",
"repository_id": "repo-1",
"project_path": "group/a",
"mr_iid": 7,
"auto_merge_enabled": true,
})
response, err := h.handleUpdateTaskMRAutomation(context.Background(), msg)
require.NoError(t, err)
assert.Equal(t, ws.MessageTypeResponse, response.Type)
require.Equal(t, 1, automation.calls)
require.True(t, automation.patch.HasMRIdentity())
assert.Equal(t, gitlab.MRIdentity{RepositoryID: "repo-1", ProjectPath: "group/a", MRIID: 7},
automation.patch.MRIdentity())
}
// TestHandleUpdateTaskMRAutomationRejectsIdentityOnlyPayload keeps MR
// identity from counting as a requested change on its own — it only says
// which MR the (absent) switch changes would have applied to.
func TestHandleUpdateTaskMRAutomationRejectsIdentityOnlyPayload(t *testing.T) {
automation := &recordingTaskMRAutomationService{}
h := &Handlers{taskMRAutomation: automation, logger: testLogger(t).WithFields()}
msg := makeWSMessage(t, ws.ActionMCPUpdateTaskMRAutomation, map[string]any{
"task_id": "task-current",
"repository_id": "repo-1",
"project_path": "group/a",
"mr_iid": 7,
})
response, err := h.handleUpdateTaskMRAutomation(context.Background(), msg)
require.NoError(t, err)
assert.Equal(t, ws.MessageTypeError, response.Type)
assert.Zero(t, automation.calls)
}
+42 -2
View File
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"math"
"strings"
"time"
@@ -366,6 +367,33 @@ func (s *Server) getTaskMRAutomationHandler() server.ToolHandlerFunc {
}
}
// copyMRIdentityArgs copies the optional repository_id/project_path/mr_iid
// triple that scopes a patch to one linked MR. repository_id is frequently
// an empty string on self-managed hosts without a numeric project ID (R6),
// so presence in args — not non-emptiness — is what marks it as sent;
// copyOptionalStringArg's "empty means absent" rule would silently turn a
// complete-but-empty identity into a partial one and get it rejected.
func copyMRIdentityArgs(payload, args map[string]interface{}) error {
for _, key := range []string{"repository_id", "project_path"} {
if value, ok := args[key]; ok {
if s, ok := value.(string); ok {
payload[key] = s
}
}
}
if value, ok := args["mr_iid"].(float64); ok {
if !isValidMRIID(value) {
return fmt.Errorf("mr_iid must be a positive integer")
}
payload["mr_iid"] = int(value)
}
return nil
}
func isValidMRIID(value float64) bool {
return !math.IsNaN(value) && !math.IsInf(value, 0) && value > 0 && math.Trunc(value) == value
}
func (s *Server) updateTaskMRAutomationHandler() server.ToolHandlerFunc {
return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
payload := map[string]interface{}{"task_id": s.taskID}
@@ -373,12 +401,24 @@ func (s *Server) updateTaskMRAutomationHandler() server.ToolHandlerFunc {
if hasLifecyclePromptOverrideArgument(args) {
return mcp.NewToolResultError("lifecycle prompt overrides are not supported"), nil
}
for _, key := range []string{"prompt_on_review_requested", "prompt_on_merged", "prompt_on_closed"} {
if err := copyMRIdentityArgs(payload, args); err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
fieldCount := 0
for _, key := range []string{
"auto_fix_enabled", "auto_merge_enabled",
"prompt_on_review_requested", "prompt_on_merged", "prompt_on_closed",
} {
if value, ok := args[key].(bool); ok {
payload[key] = value
fieldCount++
}
}
if len(payload) == 1 {
if value, ok := args["auto_fix_prompt_override"].(string); ok {
payload["auto_fix_prompt_override"] = value
fieldCount++
}
if fieldCount == 0 {
return mcp.NewToolResultError("at least one MR automation option is required"), nil
}
var result map[string]interface{}
@@ -979,6 +979,69 @@ func TestTaskMRAutomationToolsNoTaskIDArgument(t *testing.T) {
assert.NotContains(t, properties, "task_id")
}
// TestUpdateTaskMRAutomationToolForwardsMRIdentityAndAutoFixFields covers
// AC31: repository_id/project_path/mr_iid must reach the backend payload
// unchanged (including an explicit empty repository_id, R6) so the WS
// handler can scope the patch to one linked MR, and auto_fix_enabled /
// auto_merge_enabled / auto_fix_prompt_override must also be forwarded —
// the tool's input schema declares all six, but until this test regressed
// them, the handler only ever copied the three lifecycle booleans.
func TestUpdateTaskMRAutomationToolForwardsMRIdentityAndAutoFixFields(t *testing.T) {
backend := &testBackend{response: map[string]interface{}{"task_id": "task-current"}}
s := newTaskModeServer(t, backend, "task-current")
result := callTool(t, s, "update_task_mr_automation_kandev", map[string]interface{}{
"repository_id": "",
"project_path": "group/project",
"mr_iid": float64(7),
"auto_fix_enabled": true,
"auto_merge_enabled": false,
"auto_fix_prompt_override": "custom prompt",
})
assert.False(t, result.IsError)
assert.Equal(t, ws.ActionMCPUpdateTaskMRAutomation, backend.lastAction)
payload, ok := backend.lastPayload.(map[string]interface{})
require.True(t, ok)
assert.Equal(t, "", payload["repository_id"])
assert.Equal(t, "group/project", payload["project_path"])
assert.Equal(t, 7, payload["mr_iid"])
assert.Equal(t, true, payload["auto_fix_enabled"])
assert.Equal(t, false, payload["auto_merge_enabled"])
assert.Equal(t, "custom prompt", payload["auto_fix_prompt_override"])
}
// TestUpdateTaskMRAutomationToolRejectsIdentityAloneWithNoSwitch ensures MR
// identity by itself (no actual option change) is still rejected — mirrors
// TaskMRAutomationPatch.HasAny() treating identity as "which MR", not "a
// change".
func TestUpdateTaskMRAutomationToolRejectsIdentityAloneWithNoSwitch(t *testing.T) {
backend := &testBackend{}
s := newTaskModeServer(t, backend, "task-current")
result := callTool(t, s, "update_task_mr_automation_kandev", map[string]interface{}{
"repository_id": "",
"project_path": "group/project",
"mr_iid": float64(7),
})
assert.True(t, result.IsError)
assert.Empty(t, backend.lastAction, "identity-only calls must not reach the backend")
}
func TestUpdateTaskMRAutomationToolRejectsFractionalMRIID(t *testing.T) {
backend := &testBackend{}
s := newTaskModeServer(t, backend, "task-current")
result := callTool(t, s, "update_task_mr_automation_kandev", map[string]interface{}{
"repository_id": "",
"project_path": "group/project",
"mr_iid": float64(7.5),
"auto_merge_enabled": true,
})
assert.True(t, result.IsError)
assert.Empty(t, backend.lastAction, "invalid MR IID must not reach the backend")
}
func TestTaskMRAutomationToolsDoNotExposeLifecyclePromptOverrides(t *testing.T) {
backend := &testBackend{}
s := newTaskModeServer(t, backend, "task-current")
+11 -1
View File
@@ -1136,7 +1136,17 @@ func (s *Server) registerMRAutomationTools() {
)
s.mcpServer.AddTool(
mcp.NewTool("update_task_mr_automation_kandev",
mcp.WithDescription("Update this task's GitLab merge request lifecycle notification switches."),
mcp.WithDescription("Update this task's GitLab merge request automation options (auto-fix, auto-merge, "+
"and lifecycle notifications). The five switches are per merge request: pass repository_id, "+
"project_path and mr_iid together to target one linked MR, or omit all three to apply them to "+
"every MR linked to this task. auto_fix_prompt_override applies to every linked MR regardless "+
"of MR identity."),
mcp.WithString("repository_id", mcp.Description("Repository ID of the linked MR to target (omit to target every linked MR)")),
mcp.WithString("project_path", mcp.Description("Project path of the linked MR to target, e.g. group/project")),
mcp.WithNumber("mr_iid", mcp.Description("IID of the linked MR to target")),
mcp.WithBoolean("auto_fix_enabled", mcp.Description("Enable or disable auto-fix when the linked MR's pipeline fails")),
mcp.WithBoolean("auto_merge_enabled", mcp.Description("Enable or disable auto-merge when the linked MR is ready")),
mcp.WithString("auto_fix_prompt_override", mcp.Description("Task-level custom prompt for auto-fix; valid without linked MRs and not scoped by MR identity (empty string clears the override)")),
mcp.WithBoolean("prompt_on_review_requested", mcp.Description("Prompt this task's agent when a review is requested for the authenticated user")),
mcp.WithBoolean("prompt_on_merged", mcp.Description("Prompt this task's agent once when the linked MR becomes merged")),
mcp.WithBoolean("prompt_on_closed", mcp.Description("Prompt this task's agent once when the linked MR becomes closed without merge")),
@@ -21,7 +21,11 @@ import (
type mockGitLabMRAutomationService struct {
mu sync.Mutex
options *gitlab.TaskMRAutomationResponse
options *gitlab.TaskMRAutomationResponse
// optionsByMRIID, when set, returns per-MR evaluation options keyed by MR
// IID — the shape the real service produces now that the five switches
// are per linked MR. See optionsForMR.
optionsByMRIID map[int]*gitlab.TaskMRAutomationResponse
optionsErr error
checkpoint *gitlab.TaskMRLifecycleState
checkpointErr error
@@ -97,7 +101,7 @@ func (m *mockGitLabMRAutomationService) GetTaskMRAutomationResponse(ctx context.
}
func (m *mockGitLabMRAutomationService) GetTaskMRAutomationEvaluation(
ctx context.Context, _ string, _ string, _ string, _ int,
ctx context.Context, _ string, _ string, _ string, mrIID int,
) (*gitlab.TaskMRAutomationEvaluation, error) {
m.evaluationCalls.Add(1)
if m.optionsErr != nil {
@@ -112,13 +116,27 @@ func (m *mockGitLabMRAutomationService) GetTaskMRAutomationEvaluation(
if m.checkpointErr != nil {
return nil, m.checkpointErr
}
options := m.options
if options == nil {
options = &gitlab.TaskMRAutomationResponse{}
}
options := m.optionsForMR(mrIID)
return &gitlab.TaskMRAutomationEvaluation{Options: options, Checkpoint: m.checkpoint}, nil
}
// optionsForMR models the real service's per-MR resolution: the switches live
// per linked MR, so the evaluation snapshot for one MR carries that MR's own
// values. optionsByMRIID left nil keeps the single-options behaviour every
// other test in this package relies on.
func (m *mockGitLabMRAutomationService) optionsForMR(mrIID int) *gitlab.TaskMRAutomationResponse {
if m.optionsByMRIID != nil {
if options, ok := m.optionsByMRIID[mrIID]; ok {
return options
}
return &gitlab.TaskMRAutomationResponse{}
}
if m.options == nil {
return &gitlab.TaskMRAutomationResponse{}
}
return m.options
}
func (m *mockGitLabMRAutomationService) GetTaskMRLifecycleState(context.Context, string, string, string, int) (*gitlab.TaskMRLifecycleState, error) {
if m.checkpointCalls != nil {
select {
@@ -648,3 +648,62 @@ func TestHandleTaskMRCIAutomation_ExhaustedAutoFixStillBlocksUnreadyMerge(t *tes
t.Fatalf("MergeMRForAutomation calls = %d, want 0 — a failing pipeline must still block auto-merge", fake.mergeCalls.Load())
}
}
// TestHandleTaskMRLifecycleAutomation_AutoMergeOnOneMRDoesNotMergeAnother is
// the orchestrator-level statement of the whole per-MR change: auto-merge
// enabled on one linked MR must not merge a sibling MR on the same task.
//
// It deliberately enters through handleTaskMRLifecycleAutomation rather than
// calling handleTaskMRCIAutomation with hand-built options, because the
// per-MR resolution being verified happens in GetTaskMRAutomationEvaluation —
// options passed in directly would assume away the thing under test.
func TestHandleTaskMRLifecycleAutomation_AutoMergeOnOneMRDoesNotMergeAnother(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedTaskAndSession(t, repo, "task-1", "session-1", models.TaskSessionStateRunning)
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
mergeableSnapshot := func(iid int) *gitlab.MRAutomationSnapshot {
return &gitlab.MRAutomationSnapshot{
MR: &gitlab.MR{
State: gitlabMRStateOpen, IID: iid, ProjectPath: "group/widget",
MergeStatus: "can_be_merged", DetailedMergeStatus: "mergeable",
},
PipelineStatus: "success",
UnresolvedDiscussions: 0,
}
}
// Auto-merge is on for MR !1 only; !2 is left all-off.
fake := &mockGitLabMRAutomationService{
optionsByMRIID: map[int]*gitlab.TaskMRAutomationResponse{
1: {TaskID: "task-1", AutoMergeEnabled: true, WorkspaceID: "ws-1"},
2: {TaskID: "task-1", WorkspaceID: "ws-1"},
},
snapshot: mergeableSnapshot(2),
}
svc.SetGitLabMRAutomationService(fake)
mrTwo := &gitlab.TaskMR{
TaskID: "task-1", Host: "https://gitlab.example.com",
ProjectPath: "group/widget", MRIID: 2, State: gitlabMRStateOpen,
}
if err := svc.handleTaskMRLifecycleAutomation(ctx, mrTwo); err != nil {
t.Fatalf("evaluate MR !2: %v", err)
}
if got := fake.mergeCalls.Load(); got != 0 {
t.Fatalf("MergeMRForAutomation calls for MR !2 = %d, want 0 — a sibling MR's auto-merge must not merge this one", got)
}
// Same task, same fully-mergeable state, but this MR owns the switch.
fake.snapshot = mergeableSnapshot(1)
mrOne := &gitlab.TaskMR{
TaskID: "task-1", Host: "https://gitlab.example.com",
ProjectPath: "group/widget", MRIID: 1, State: gitlabMRStateOpen,
}
if err := svc.handleTaskMRLifecycleAutomation(ctx, mrOne); err != nil {
t.Fatalf("evaluate MR !1: %v", err)
}
if got := fake.mergeCalls.Load(); got != 1 {
t.Fatalf("MergeMRForAutomation calls after MR !1 = %d, want 1 — the MR that owns the switch must still merge", got)
}
}
@@ -2,7 +2,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { cleanup, render, screen, fireEvent } from "@testing-library/react";
import { TooltipProvider } from "@kandev/ui/tooltip";
import { ToastProvider } from "@/components/toast-provider";
import type { TaskMR, TaskMRAutomationOptions, TaskMRLifecycleState } from "@/lib/types/gitlab";
import type {
TaskMR,
TaskMRAutomationOptions,
TaskMRAutomationOptionsForMR,
TaskMRLifecycleState,
} from "@/lib/types/gitlab";
const hookMocks = vi.hoisted(() => ({
error: null as string | null,
@@ -36,6 +41,26 @@ const AUTO_FIX_LABEL = "Auto-fix CI and address comments";
const AUTO_MERGE_LABEL = "Auto-merge when ready";
const EDIT_PROMPT_LABEL = "Edit auto-fix prompt for this task";
const PROMPT_TEXTAREA_LABEL = "Task auto-fix prompt";
const PROJECT_PATH = "group/project";
function makeMROptions(
overrides: Partial<TaskMRAutomationOptionsForMR> = {},
): TaskMRAutomationOptionsForMR {
return {
task_id: "task-1",
repository_id: "",
project_path: PROJECT_PATH,
mr_iid: 7,
auto_fix_enabled: false,
auto_merge_enabled: false,
prompt_on_review_requested: false,
prompt_on_merged: false,
prompt_on_closed: false,
created_at: "",
updated_at: "",
...overrides,
};
}
function makeOptions(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAutomationOptions {
return {
@@ -51,6 +76,7 @@ function makeOptions(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAu
review_reviewer_username: "",
updated_at: "2026-06-18T10:00:00Z",
mr_states: [],
mr_options: [makeMROptions()],
...overrides,
};
}
@@ -60,7 +86,7 @@ function makeMR(overrides: Partial<TaskMR> = {}): TaskMR {
id: "assoc-1",
task_id: "task-1",
host: "https://gitlab.com",
project_path: "group/project",
project_path: PROJECT_PATH,
mr_iid: 7,
mr_url: "https://gitlab.com/group/project/-/merge_requests/7",
mr_title: "Test MR",
@@ -89,7 +115,7 @@ function makeState(overrides: Partial<TaskMRLifecycleState> = {}): TaskMRLifecyc
return {
task_id: "task-1",
repository_id: "",
project_path: "group/project",
project_path: PROJECT_PATH,
mr_iid: 7,
review_request_initialized: false,
last_review_requested: false,
@@ -105,7 +131,7 @@ function makeState(overrides: Partial<TaskMRLifecycleState> = {}): TaskMRLifecyc
};
}
function renderControls(mr: TaskMR | undefined = makeMR()) {
function renderControls(mr: TaskMR = makeMR()) {
return render(
<TooltipProvider>
<ToastProvider>
@@ -148,46 +174,69 @@ describe("MRAutomationControls — Automation section (AC1)", () => {
it("toggling auto-fix on patches auto_fix_enabled", () => {
renderControls();
fireEvent.click(screen.getByLabelText(AUTO_FIX_LABEL));
expect(hookMocks.updateMock).toHaveBeenCalledWith({ auto_fix_enabled: true });
expect(hookMocks.updateMock).toHaveBeenCalledWith({
repository_id: "",
project_path: PROJECT_PATH,
mr_iid: 7,
auto_fix_enabled: true,
});
});
it("toggling auto-merge on patches auto_merge_enabled", () => {
renderControls();
fireEvent.click(screen.getByLabelText(AUTO_MERGE_LABEL));
expect(hookMocks.updateMock).toHaveBeenCalledWith({ auto_merge_enabled: true });
expect(hookMocks.updateMock).toHaveBeenCalledWith({
repository_id: "",
project_path: PROJECT_PATH,
mr_iid: 7,
auto_merge_enabled: true,
});
});
it("shows the round-help button only when auto-fix is enabled and a single MR is known", () => {
hookMocks.options = makeOptions({ auto_fix_enabled: false });
hookMocks.options = makeOptions({ mr_options: [makeMROptions({ auto_fix_enabled: false })] });
renderControls();
expect(screen.queryByTestId("mr-auto-fix-round-help")).toBeNull();
cleanup();
hookMocks.options = makeOptions({
auto_fix_enabled: true,
mr_options: [makeMROptions({ auto_fix_enabled: true })],
mr_states: [makeState({ auto_fix_round_count: 3 })],
});
renderControls();
expect(screen.getByTestId("mr-auto-fix-round-help")).toBeTruthy();
});
it("still shows the round-help button when no single MR is provided", () => {
hookMocks.options = makeOptions({ auto_fix_enabled: true });
renderControls(undefined);
expect(screen.getByTestId("mr-auto-fix-round-help")).toBeTruthy();
});
it("shows the auto-merge readiness help button only when auto-merge is enabled", () => {
hookMocks.options = makeOptions({ auto_merge_enabled: false });
hookMocks.options = makeOptions({
mr_options: [makeMROptions({ auto_merge_enabled: false })],
});
renderControls();
expect(screen.queryByTestId("mr-auto-merge-help")).toBeNull();
cleanup();
hookMocks.options = makeOptions({ auto_merge_enabled: true });
hookMocks.options = makeOptions({ mr_options: [makeMROptions({ auto_merge_enabled: true })] });
renderControls();
expect(screen.getByTestId("mr-auto-merge-help")).toBeTruthy();
});
it("renders switches off when options have no entry for this MR", () => {
hookMocks.options = makeOptions({
mr_options: [
makeMROptions({
mr_iid: 99,
auto_fix_enabled: true,
auto_merge_enabled: true,
}),
],
});
renderControls();
expect(screen.getByLabelText(AUTO_FIX_LABEL).getAttribute("data-state")).toBe("unchecked");
expect(screen.getByLabelText(AUTO_MERGE_LABEL).getAttribute("data-state")).toBe("unchecked");
expect(screen.queryByTestId("mr-auto-fix-round-help")).toBeNull();
expect(screen.queryByTestId("mr-auto-merge-help")).toBeNull();
});
it("disables both automation switches while loading", () => {
hookMocks.loading = true;
hookMocks.options = null;
@@ -2,7 +2,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { cleanup, render, screen, fireEvent } from "@testing-library/react";
import { TooltipProvider } from "@kandev/ui/tooltip";
import { ToastProvider } from "@/components/toast-provider";
import type { TaskMRAutomationOptions } from "@/lib/types/gitlab";
import type {
TaskMR,
TaskMRAutomationOptions,
TaskMRAutomationOptionsForMR,
} from "@/lib/types/gitlab";
const hookMocks = vi.hoisted(() => ({
error: null as string | null,
@@ -44,6 +48,25 @@ const CLOSED_LABEL = "MR closed without merging";
const REVIEW_REQUESTED_LABEL = "Your review is requested";
const FOLLOW_UP_TRIGGER = "mr-review-follow-up-trigger";
function makeMROptions(
overrides: Partial<TaskMRAutomationOptionsForMR> = {},
): TaskMRAutomationOptionsForMR {
return {
task_id: "task-1",
repository_id: "",
project_path: "group/project",
mr_iid: 7,
auto_fix_enabled: false,
auto_merge_enabled: false,
prompt_on_review_requested: false,
prompt_on_merged: false,
prompt_on_closed: false,
created_at: "",
updated_at: "",
...overrides,
};
}
function makeOptions(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAutomationOptions {
return {
task_id: "task-1",
@@ -58,6 +81,37 @@ function makeOptions(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAu
review_reviewer_username: "",
updated_at: "2026-06-18T10:00:00Z",
mr_states: [],
mr_options: [makeMROptions()],
...overrides,
};
}
function makeMR(overrides: Partial<TaskMR> = {}): TaskMR {
return {
id: "assoc-1",
task_id: "task-1",
host: "https://gitlab.com",
project_path: "group/project",
mr_iid: 7,
mr_url: "https://gitlab.com/group/project/-/merge_requests/7",
mr_title: "Test MR",
head_branch: "feature",
base_branch: "main",
author_username: "alice",
state: "open",
approval_state: "",
pipeline_state: "",
merge_status: "",
draft: false,
approval_count: 0,
required_approvals: 0,
pipeline_jobs_total: 0,
pipeline_jobs_pass: 0,
reviewer_count: 0,
unapproved_reviewers: 0,
unresolved_discussions: 0,
created_at: "",
updated_at: "",
...overrides,
};
}
@@ -66,7 +120,7 @@ function renderControls() {
return render(
<TooltipProvider>
<ToastProvider>
<MRAutomationControls taskId="task-1" />
<MRAutomationControls taskId="task-1" mr={makeMR()} />
</ToastProvider>
</TooltipProvider>,
);
@@ -112,7 +166,7 @@ describe("MRAutomationControls", () => {
});
it("auto-expands when a switch is already on", () => {
hookMocks.options = makeOptions({ prompt_on_merged: true });
hookMocks.options = makeOptions({ mr_options: [makeMROptions({ prompt_on_merged: true })] });
renderControls();
expect(screen.getByTestId(FOLLOW_UP_TRIGGER).getAttribute("aria-expanded")).toBe("true");
expect(screen.getByLabelText(MERGED_LABEL)).not.toBeNull();
@@ -134,13 +188,13 @@ describe("MRAutomationControls", () => {
fireEvent.click(screen.getByTestId(FOLLOW_UP_TRIGGER));
expect(screen.getByLabelText(REVIEW_REQUESTED_LABEL).getAttribute("aria-describedby")).toBe(
"task-mr-review-requested-prompt-task-1-description",
"task-mr-review-requested-prompt-task-1-assoc-1-description",
);
expect(screen.getByLabelText(MERGED_LABEL).getAttribute("aria-describedby")).toBe(
"task-mr-terminal-help-task-1",
"task-mr-terminal-help-task-1-assoc-1",
);
expect(screen.getByLabelText(CLOSED_LABEL).getAttribute("aria-describedby")).toBe(
"task-mr-terminal-help-task-1",
"task-mr-terminal-help-task-1-assoc-1",
);
expect(
screen.getByText(
@@ -160,9 +214,23 @@ describe("MRAutomationControls", () => {
fireEvent.click(screen.getByLabelText(MERGED_LABEL));
fireEvent.click(screen.getByLabelText(CLOSED_LABEL));
expect(hookMocks.updateMock).toHaveBeenCalledWith({ prompt_on_review_requested: true });
expect(hookMocks.updateMock).toHaveBeenCalledWith({ prompt_on_merged: true });
expect(hookMocks.updateMock).toHaveBeenCalledWith({ prompt_on_closed: true });
const mrIdentity = { repository_id: "", project_path: "group/project", mr_iid: 7 };
expect(hookMocks.updateMock).toHaveBeenCalledWith({
...mrIdentity,
prompt_on_review_requested: true,
});
expect(hookMocks.updateMock).toHaveBeenCalledWith({ ...mrIdentity, prompt_on_merged: true });
expect(hookMocks.updateMock).toHaveBeenCalledWith({ ...mrIdentity, prompt_on_closed: true });
});
});
describe("MRAutomationControls error and loading states", () => {
beforeEach(() => {
resetHookMocks();
});
afterEach(() => {
cleanup();
});
it("surfaces an error from the hook", () => {
@@ -1,135 +1,35 @@
"use client";
import { useCallback, useEffect, useState, type ReactNode } from "react";
import { useCallback, useEffect, useState } from "react";
import { useTranslation } from "react-i18next";
import { IconChevronDown, IconEdit, IconInfoCircle } from "@tabler/icons-react";
import { Button } from "@kandev/ui/button";
import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@kandev/ui/collapsible";
import { Label } from "@kandev/ui/label";
import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
import { Switch } from "@kandev/ui/switch";
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
import { useToast } from "@/components/toast-provider";
import { useTaskMRAutomationOptions } from "@/hooks/domains/gitlab/use-task-mr-automation";
import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint";
import { autoFixRoundForState, findMRAutomationStateForMR } from "@/lib/gitlab/mr-automation";
import {
findMRAutomationOptionsForMR,
findMRAutomationStateForMR,
} from "@/lib/gitlab/mr-automation";
import type {
TaskMR,
TaskMRAutomationOptions,
TaskMRAutomationOptionsForMR,
TaskMRAutomationPatch,
TaskMRLifecycleState,
} from "@/lib/types/gitlab";
import { MRAutoFixPromptDialog } from "./mr-auto-fix-prompt-dialog";
import {
compactRowMinHeight,
MRAgentPromptRows,
MRAutomationOptionRows,
} from "./mr-automation-rows";
/** Shared compact-row height: taller touch target on mobile/coarse pointers. */
function compactRowMinHeight(isMobile: boolean, isFinePointer: boolean): string {
return isMobile || !isFinePointer ? "min-h-11" : "min-h-7";
}
/** True when any of the three #2125 lifecycle-notification switches is on. */
function hasLifecycleSwitchEnabled(options: TaskMRAutomationOptions | null): boolean {
return Boolean(
options?.prompt_on_review_requested || options?.prompt_on_merged || options?.prompt_on_closed,
);
}
/** Resolves the round-help state for a single MR, when one is known. */
function resolveAutomationState(
mr: TaskMR | undefined,
states: TaskMRLifecycleState[] | undefined,
): TaskMRLifecycleState | undefined {
return mr ? findMRAutomationStateForMR(states, mr) : undefined;
}
/**
* Dual-mode help affordance: a tap popover on coarse pointers, a hover
* tooltip on fine pointers. Mirrors CIAutomationHelpButton (GitHub).
*/
function MRAutomationHelpButton({
ariaLabel,
testId,
children,
}: {
ariaLabel: string;
testId: string;
children: ReactNode;
}) {
const { isFinePointer } = useResponsiveBreakpoint();
const [open, setOpen] = useState(false);
const trigger = (
<Button
type="button"
variant="ghost"
size="icon"
data-testid={testId}
className="h-5 w-5 cursor-help text-muted-foreground hover:text-foreground"
aria-label={ariaLabel}
>
<IconInfoCircle className="h-3.5 w-3.5" />
</Button>
);
if (!isFinePointer) {
return (
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger asChild>{trigger}</PopoverTrigger>
<PopoverContent
side="top"
align="start"
portal={false}
className="max-w-[280px] text-xs leading-relaxed"
>
{children}
</PopoverContent>
</Popover>
);
}
/** True when any of the three lifecycle-notification switches is on for this MR. */
function hasLifecycleSwitchEnabled(switches: TaskMRAutomationOptionsForMR): boolean {
return (
<Tooltip>
<TooltipTrigger asChild>{trigger}</TooltipTrigger>
<TooltipContent side="top" align="start" className="max-w-[280px] text-xs leading-relaxed">
{children}
</TooltipContent>
</Tooltip>
);
}
function MRAutomationRow({
id,
label,
checked,
disabled,
onCheckedChange,
help,
describedBy,
}: {
id: string;
label: string;
checked: boolean;
disabled: boolean;
onCheckedChange: (checked: boolean) => void;
help?: ReactNode;
describedBy?: string;
}) {
const { isFinePointer, isMobile } = useResponsiveBreakpoint();
const minHeight = compactRowMinHeight(isMobile, isFinePointer);
return (
<div className={`flex items-center justify-between gap-3 px-1 ${minHeight}`}>
<div className="flex min-w-0 flex-1 items-center gap-1.5">
<Label htmlFor={id} className="min-w-0 cursor-pointer text-xs leading-5">
{label}
</Label>
{help}
</div>
<Switch
id={id}
aria-label={label}
aria-describedby={describedBy}
checked={checked}
disabled={disabled}
onCheckedChange={onCheckedChange}
/>
</div>
switches.prompt_on_review_requested || switches.prompt_on_merged || switches.prompt_on_closed
);
}
@@ -172,50 +72,28 @@ function MRAutomationLoadErrorBanner({ error, onRetry }: { error: string; onRetr
);
}
function MRAutoFixRoundHelpButton({
state,
maxRounds,
}: {
state: TaskMRLifecycleState | undefined;
maxRounds: number | null | undefined;
}) {
const { t } = useTranslation();
const round = autoFixRoundForState(state, maxRounds);
return (
<MRAutomationHelpButton
testId="mr-auto-fix-round-help"
ariaLabel={t("gitlab:mrAutomationExplainAutoFixRounds")}
>
<span data-testid="mr-auto-fix-round-explanation">
{t("gitlab:mrAutoFixRoundExplanation", { current: round.current, max: round.max })}
</span>
</MRAutomationHelpButton>
);
}
function MRAutoMergeHelpButton() {
const { t } = useTranslation();
return (
<MRAutomationHelpButton
testId="mr-auto-merge-help"
ariaLabel={t("gitlab:mrAutomationExplainAutoMerge")}
>
<span data-testid="mr-auto-merge-explanation">{t("gitlab:mrAutoMergeReadyExplanation")}</span>
</MRAutomationHelpButton>
);
}
function MRAutomationHeader({
mrIID,
disabled,
onEditPrompt,
}: {
mrIID: number;
disabled: boolean;
onEditPrompt: () => void;
}) {
const { t } = useTranslation();
return (
<div className="flex items-center justify-between gap-2 px-1">
<div className="text-xs font-medium text-foreground">{t("gitlab:mrAutomation")}</div>
<div className="flex min-w-0 items-baseline gap-1.5">
<span className="text-xs font-medium text-foreground">{t("gitlab:mrAutomation")}</span>
{/* The switches below are scoped to this merge request, so say which. */}
<span
data-testid="mr-automation-scope-label"
className="truncate text-[11px] text-muted-foreground"
>
{t("gitlab:mrAutomationAppliesToMR", { iid: mrIID })}
</span>
</div>
<div className="flex items-center gap-1">
<Tooltip>
<TooltipTrigger asChild>
@@ -249,49 +127,6 @@ function MRAutomationHeader({
);
}
function MRAutomationOptionRows({
taskId,
options,
disabled,
patchOption,
automationState,
}: {
taskId: string;
options: TaskMRAutomationOptions | null;
disabled: boolean;
patchOption: (patch: TaskMRAutomationPatch) => void;
automationState: TaskMRLifecycleState | undefined;
}) {
const { t } = useTranslation();
return (
<>
<MRAutomationRow
id={`task-mr-auto-fix-${taskId}`}
label={t("gitlab:mrAutoFixCiAndAddressComments")}
checked={Boolean(options?.auto_fix_enabled)}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ auto_fix_enabled: checked })}
help={
options?.auto_fix_enabled ? (
<MRAutoFixRoundHelpButton
state={automationState}
maxRounds={options.auto_fix_max_rounds}
/>
) : null
}
/>
<MRAutomationRow
id={`task-mr-auto-merge-${taskId}`}
label={t("gitlab:mrAutoMergeWhenReady")}
checked={Boolean(options?.auto_merge_enabled)}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ auto_merge_enabled: checked })}
help={options?.auto_merge_enabled ? <MRAutoMergeHelpButton /> : null}
/>
</>
);
}
/**
* Encapsulates the auto-fix prompt editor's local state and save/reset
* handlers, split out of MRAutomationControls to keep that component under
@@ -328,6 +163,8 @@ function useMRAutoFixPromptEditor(
setPromptOpen(true);
}, [options]);
// The auto-fix prompt override is task-level (it applies to every linked
// MR), so these patches intentionally carry no MR identity.
const savePrompt = useCallback(() => {
const value = promptDraft.trim();
if (!value) return;
@@ -355,15 +192,26 @@ function useMRAutoFixPromptEditor(
/**
* Wraps `update` with the standard "toast on failure" handling shared by
* every switch row. Split out of MRAutomationControls to keep that
* every switch row, and stamps `mr`'s identity onto each patch so the backend
* applies the change to this merge request alone instead of fanning it out to
* every MR linked to the task. Split out of MRAutomationControls to keep that
* component under the file's complexity limit.
*/
function useMRAutomationPatch(update: (patch: TaskMRAutomationPatch) => Promise<unknown>) {
function useMRAutomationPatch(
mr: TaskMR,
update: (patch: TaskMRAutomationPatch) => Promise<unknown>,
) {
const { t } = useTranslation();
const { toast } = useToast();
return useCallback(
(patch: TaskMRAutomationPatch) => {
update(patch).catch((err) => {
const scoped: TaskMRAutomationPatch = {
...patch,
repository_id: mr.repository_id ?? "",
project_path: mr.project_path,
mr_iid: mr.mr_iid,
};
update(scoped).catch((err) => {
toast({
title: t("gitlab:mrAutomationUpdateFailedTitle"),
description:
@@ -372,127 +220,35 @@ function useMRAutomationPatch(update: (patch: TaskMRAutomationPatch) => Promise<
});
});
},
[t, toast, update],
);
}
function ReviewRequestedPromptRow({
taskId,
options,
disabled,
patchOption,
}: {
taskId: string;
options: TaskMRAutomationOptions | null;
disabled: boolean;
patchOption: (patch: TaskMRAutomationPatch) => void;
}) {
const { t } = useTranslation();
const helpID = `task-mr-review-requested-prompt-${taskId}-description`;
const help = t("gitlab:mrAutomationReviewRequestedHelp");
return (
<>
<span id={helpID} className="sr-only">
{help}
</span>
<MRAutomationRow
id={`task-mr-review-requested-prompt-${taskId}`}
label={t("gitlab:mrAutomationReviewRequestedLabel")}
describedBy={helpID}
checked={Boolean(options?.prompt_on_review_requested)}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ prompt_on_review_requested: checked })}
help={
<MRAutomationHelpButton
testId="mr-review-requested-help"
ariaLabel={t("gitlab:mrAutomationReviewRequestedHelpAriaLabel")}
>
{help}
</MRAutomationHelpButton>
}
/>
</>
);
}
function MRAgentPromptRows({
taskId,
options,
disabled,
patchOption,
}: {
taskId: string;
options: TaskMRAutomationOptions | null;
disabled: boolean;
patchOption: (patch: TaskMRAutomationPatch) => void;
}) {
const { t } = useTranslation();
const terminalHelpID = `task-mr-terminal-help-${taskId}`;
const terminalHelp = t("gitlab:mrAutomationTerminalHelp");
return (
<>
<ReviewRequestedPromptRow
taskId={taskId}
options={options}
disabled={disabled}
patchOption={patchOption}
/>
<span id={terminalHelpID} className="sr-only">
{terminalHelp}
</span>
<MRAutomationRow
id={`task-mr-merged-prompt-${taskId}`}
label={t("gitlab:mrAutomationMergedLabel")}
describedBy={terminalHelpID}
checked={Boolean(options?.prompt_on_merged)}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ prompt_on_merged: checked })}
help={
<MRAutomationHelpButton
testId="mr-terminal-help"
ariaLabel={t("gitlab:mrAutomationTerminalHelpAriaLabel")}
>
{terminalHelp}
</MRAutomationHelpButton>
}
/>
<MRAutomationRow
id={`task-mr-closed-prompt-${taskId}`}
label={t("gitlab:mrAutomationClosedLabel")}
describedBy={terminalHelpID}
checked={Boolean(options?.prompt_on_closed)}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ prompt_on_closed: checked })}
/>
</>
[mr, t, toast, update],
);
}
/**
* MR automation controls: an "Automation" section (auto-fix CI + auto-merge)
* above a collapsible "Review follow-up" group of three compact switch rows.
* Renders inside the GitLab MR topbar dropdown, below the per-MR items.
* Auto-expands "Review follow-up" when any of its switches is already on so
* a previously configured task doesn't hide its active switches. Mirrors
* PRCIAutomationControls + ReviewFollowUpSection (GitHub), AC1, AC29.
* MR automation controls for one linked merge request: an "Automation"
* section (auto-fix CI + auto-merge) above a collapsible "Review follow-up"
* group of three compact switch rows. Renders inside the GitLab MR topbar
* dropdown and hover popover. Auto-expands "Review follow-up" when any of its
* switches is already on so a previously configured MR doesn't hide its active
* switches. Mirrors PRCIAutomationControls + ReviewFollowUpSection (GitHub).
*
* `mr` is optional and only used to look up the per-MR auto-fix round state
* for the round-help button's count — when omitted (e.g. a task with
* multiple linked MRs and no single one to attribute rounds to), the
* round-help button still renders but reads 0 of max; the switches
* themselves remain task-scoped either way.
* `mr` is required: all five switches are per-MR, so there is no meaningful
* task-wide rendering of this group. A task with several linked MRs renders
* one instance per MR.
*/
export function MRAutomationControls({ taskId, mr }: { taskId: string; mr?: TaskMR }) {
export function MRAutomationControls({ taskId, mr }: { taskId: string; mr: TaskMR }) {
const { options, loading, saving, error, update, refresh, resetPrompt } =
useTaskMRAutomationOptions(taskId);
const { isFinePointer, isMobile } = useResponsiveBreakpoint();
const { t } = useTranslation();
const [open, setOpen] = useState(false);
const promptEditor = useMRAutoFixPromptEditor(options, update, resetPrompt);
const patchOption = useMRAutomationPatch(update);
const lifecycleEnabled = hasLifecycleSwitchEnabled(options);
const patchOption = useMRAutomationPatch(mr, update);
const switches = findMRAutomationOptionsForMR(options?.mr_options, mr);
const lifecycleEnabled = hasLifecycleSwitchEnabled(switches);
const minHeight = compactRowMinHeight(isMobile, isFinePointer);
const automationState = resolveAutomationState(mr, options?.mr_states);
const automationState = findMRAutomationStateForMR(options?.mr_states, mr);
const elementIDSuffix = `${taskId}-${mr.id}`;
useEffect(() => {
if (lifecycleEnabled) setOpen(true);
@@ -502,7 +258,7 @@ export function MRAutomationControls({ taskId, mr }: { taskId: string; mr?: Task
const disabled = saving || loading || !options;
return (
<div data-testid="mr-automation-controls">
<div data-testid="mr-automation-controls" data-mr-iid={mr.mr_iid}>
{loadFailed ? (
<MRAutomationLoadErrorBanner
error={error as string}
@@ -513,13 +269,18 @@ export function MRAutomationControls({ taskId, mr }: { taskId: string; mr?: Task
}}
/>
) : null}
<MRAutomationHeader disabled={!options} onEditPrompt={promptEditor.openPromptEditor} />
<MRAutomationHeader
mrIID={mr.mr_iid}
disabled={!options}
onEditPrompt={promptEditor.openPromptEditor}
/>
<MRAutomationOptionRows
taskId={taskId}
options={options}
elementIDSuffix={elementIDSuffix}
switches={switches}
disabled={disabled}
patchOption={patchOption}
automationState={automationState}
maxRounds={options?.auto_fix_max_rounds}
/>
<Collapsible open={open} onOpenChange={setOpen}>
<CollapsibleTrigger asChild>
@@ -543,8 +304,8 @@ export function MRAutomationControls({ taskId, mr }: { taskId: string; mr?: Task
{t("gitlab:mrAutomationDescription")}
</p>
<MRAgentPromptRows
taskId={taskId}
options={options}
elementIDSuffix={elementIDSuffix}
switches={switches}
disabled={disabled}
patchOption={patchOption}
/>
@@ -0,0 +1,280 @@
"use client";
import { useState, type ReactNode } from "react";
import { useTranslation } from "react-i18next";
import { IconInfoCircle } from "@tabler/icons-react";
import { Button } from "@kandev/ui/button";
import { Label } from "@kandev/ui/label";
import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
import { Switch } from "@kandev/ui/switch";
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint";
import { autoFixRoundForState } from "@/lib/gitlab/mr-automation";
import type {
TaskMRAutomationOptionsForMR,
TaskMRAutomationPatch,
TaskMRLifecycleState,
} from "@/lib/types/gitlab";
/** Shared compact-row height: taller touch target on mobile/coarse pointers. */
export function compactRowMinHeight(isMobile: boolean, isFinePointer: boolean): string {
return isMobile || !isFinePointer ? "min-h-11" : "min-h-7";
}
/**
* Dual-mode help affordance: a tap popover on coarse pointers, a hover
* tooltip on fine pointers. Mirrors CIAutomationHelpButton (GitHub).
*/
export function MRAutomationHelpButton({
ariaLabel,
testId,
children,
}: {
ariaLabel: string;
testId: string;
children: ReactNode;
}) {
const { isFinePointer } = useResponsiveBreakpoint();
const [open, setOpen] = useState(false);
const trigger = (
<Button
type="button"
variant="ghost"
size="icon"
data-testid={testId}
className="h-5 w-5 cursor-help text-muted-foreground hover:text-foreground"
aria-label={ariaLabel}
>
<IconInfoCircle className="h-3.5 w-3.5" />
</Button>
);
if (!isFinePointer) {
return (
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger asChild>{trigger}</PopoverTrigger>
<PopoverContent
side="top"
align="start"
portal={false}
className="max-w-[280px] text-xs leading-relaxed"
>
{children}
</PopoverContent>
</Popover>
);
}
return (
<Tooltip>
<TooltipTrigger asChild>{trigger}</TooltipTrigger>
<TooltipContent side="top" align="start" className="max-w-[280px] text-xs leading-relaxed">
{children}
</TooltipContent>
</Tooltip>
);
}
function MRAutomationRow({
id,
label,
checked,
disabled,
onCheckedChange,
help,
describedBy,
}: {
id: string;
label: string;
checked: boolean;
disabled: boolean;
onCheckedChange: (checked: boolean) => void;
help?: ReactNode;
describedBy?: string;
}) {
const { isFinePointer, isMobile } = useResponsiveBreakpoint();
const minHeight = compactRowMinHeight(isMobile, isFinePointer);
return (
<div className={`flex items-center justify-between gap-3 px-1 ${minHeight}`}>
<div className="flex min-w-0 flex-1 items-center gap-1.5">
<Label htmlFor={id} className="min-w-0 cursor-pointer text-xs leading-5">
{label}
</Label>
{help}
</div>
<Switch
id={id}
aria-label={label}
aria-describedby={describedBy}
checked={checked}
disabled={disabled}
onCheckedChange={onCheckedChange}
/>
</div>
);
}
function MRAutoFixRoundHelpButton({
state,
maxRounds,
}: {
state: TaskMRLifecycleState | undefined;
maxRounds: number | null | undefined;
}) {
const { t } = useTranslation();
const round = autoFixRoundForState(state, maxRounds);
return (
<MRAutomationHelpButton
testId="mr-auto-fix-round-help"
ariaLabel={t("gitlab:mrAutomationExplainAutoFixRounds")}
>
<span data-testid="mr-auto-fix-round-explanation">
{t("gitlab:mrAutoFixRoundExplanation", { current: round.current, max: round.max })}
</span>
</MRAutomationHelpButton>
);
}
function MRAutoMergeHelpButton() {
const { t } = useTranslation();
return (
<MRAutomationHelpButton
testId="mr-auto-merge-help"
ariaLabel={t("gitlab:mrAutomationExplainAutoMerge")}
>
<span data-testid="mr-auto-merge-explanation">{t("gitlab:mrAutoMergeReadyExplanation")}</span>
</MRAutomationHelpButton>
);
}
/**
* Props shared by every switch-row group. `elementIDSuffix` scopes the DOM ids
* to one merge request: the multi-MR dropdown renders one group per linked MR,
* so a task-only suffix would emit duplicate ids and point every `<Label>` at
* the first MR's switch.
*/
type MRAutomationRowsProps = {
elementIDSuffix: string;
switches: TaskMRAutomationOptionsForMR;
disabled: boolean;
patchOption: (patch: TaskMRAutomationPatch) => void;
};
export function MRAutomationOptionRows({
elementIDSuffix,
switches,
disabled,
patchOption,
automationState,
maxRounds,
}: MRAutomationRowsProps & {
automationState: TaskMRLifecycleState | undefined;
maxRounds: number | null | undefined;
}) {
const { t } = useTranslation();
return (
<>
<MRAutomationRow
id={`task-mr-auto-fix-${elementIDSuffix}`}
label={t("gitlab:mrAutoFixCiAndAddressComments")}
checked={switches.auto_fix_enabled}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ auto_fix_enabled: checked })}
help={
switches.auto_fix_enabled ? (
<MRAutoFixRoundHelpButton state={automationState} maxRounds={maxRounds} />
) : null
}
/>
<MRAutomationRow
id={`task-mr-auto-merge-${elementIDSuffix}`}
label={t("gitlab:mrAutoMergeWhenReady")}
checked={switches.auto_merge_enabled}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ auto_merge_enabled: checked })}
help={switches.auto_merge_enabled ? <MRAutoMergeHelpButton /> : null}
/>
</>
);
}
function ReviewRequestedPromptRow({
elementIDSuffix,
switches,
disabled,
patchOption,
}: MRAutomationRowsProps) {
const { t } = useTranslation();
const helpID = `task-mr-review-requested-prompt-${elementIDSuffix}-description`;
const help = t("gitlab:mrAutomationReviewRequestedHelp");
return (
<>
<span id={helpID} className="sr-only">
{help}
</span>
<MRAutomationRow
id={`task-mr-review-requested-prompt-${elementIDSuffix}`}
label={t("gitlab:mrAutomationReviewRequestedLabel")}
describedBy={helpID}
checked={switches.prompt_on_review_requested}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ prompt_on_review_requested: checked })}
help={
<MRAutomationHelpButton
testId="mr-review-requested-help"
ariaLabel={t("gitlab:mrAutomationReviewRequestedHelpAriaLabel")}
>
{help}
</MRAutomationHelpButton>
}
/>
</>
);
}
export function MRAgentPromptRows({
elementIDSuffix,
switches,
disabled,
patchOption,
}: MRAutomationRowsProps) {
const { t } = useTranslation();
const terminalHelpID = `task-mr-terminal-help-${elementIDSuffix}`;
const terminalHelp = t("gitlab:mrAutomationTerminalHelp");
return (
<>
<ReviewRequestedPromptRow
elementIDSuffix={elementIDSuffix}
switches={switches}
disabled={disabled}
patchOption={patchOption}
/>
<span id={terminalHelpID} className="sr-only">
{terminalHelp}
</span>
<MRAutomationRow
id={`task-mr-merged-prompt-${elementIDSuffix}`}
label={t("gitlab:mrAutomationMergedLabel")}
describedBy={terminalHelpID}
checked={switches.prompt_on_merged}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ prompt_on_merged: checked })}
help={
<MRAutomationHelpButton
testId="mr-terminal-help"
ariaLabel={t("gitlab:mrAutomationTerminalHelpAriaLabel")}
>
{terminalHelp}
</MRAutomationHelpButton>
}
/>
<MRAutomationRow
id={`task-mr-closed-prompt-${elementIDSuffix}`}
label={t("gitlab:mrAutomationClosedLabel")}
describedBy={terminalHelpID}
checked={switches.prompt_on_closed}
disabled={disabled}
onCheckedChange={(checked) => patchOption({ prompt_on_closed: checked })}
/>
</>
);
}
@@ -1,6 +1,11 @@
import { describe, expect, it } from "vitest";
import { chipAutomation, selectBadgeMR } from "./mr-status-chip-selection";
import type { TaskMR, TaskMRAutomationOptions, TaskMRLifecycleState } from "@/lib/types/gitlab";
import type {
TaskMR,
TaskMRAutomationOptions,
TaskMRAutomationOptionsForMR,
TaskMRLifecycleState,
} from "@/lib/types/gitlab";
function makeMR(overrides: Partial<TaskMR> = {}): TaskMR {
return {
@@ -52,6 +57,32 @@ function makeState(overrides: Partial<TaskMRLifecycleState> = {}): TaskMRLifecyc
};
}
/**
* One `mr_options` row. The switches are per MR, so a fixture that leaves
* `mr_options` empty while passing open MRs models a state the backend
* cannot produce (`taskMRAutomationOptionsList` emits one entry per linked
* MR) — and would silently assert nothing about the per-MR badge logic.
*/
function makeMROptions(
mrIID: number,
overrides: Partial<TaskMRAutomationOptionsForMR> = {},
): TaskMRAutomationOptionsForMR {
return {
task_id: "task-1",
repository_id: "",
project_path: "group/project",
mr_iid: mrIID,
auto_fix_enabled: true,
auto_merge_enabled: false,
prompt_on_review_requested: false,
prompt_on_merged: false,
prompt_on_closed: false,
created_at: "",
updated_at: "",
...overrides,
};
}
function makeAutomation(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAutomationOptions {
return {
task_id: "task-1",
@@ -65,6 +96,7 @@ function makeAutomation(overrides: Partial<TaskMRAutomationOptions> = {}): TaskM
prompt_on_closed: false,
review_reviewer_username: "",
updated_at: "",
mr_options: [],
mr_states: [],
...overrides,
};
@@ -81,6 +113,7 @@ describe("selectBadgeMR tiebreak", () => {
const exhausted = makeMR({ id: "a", mr_iid: 3 });
const notExhausted = makeMR({ id: "b", mr_iid: 9 });
const options = makeAutomation({
mr_options: [makeMROptions(3), makeMROptions(9)],
mr_states: [
makeState({
mr_iid: 3,
@@ -101,6 +134,7 @@ describe("selectBadgeMR tiebreak", () => {
const lowerRound = makeMR({ id: "a", mr_iid: 1 });
const higherRound = makeMR({ id: "b", mr_iid: 2 });
const options = makeAutomation({
mr_options: [makeMROptions(1), makeMROptions(2)],
mr_states: [
makeState({ mr_iid: 1, auto_fix_round_count: 1 }),
makeState({ mr_iid: 2, auto_fix_round_count: 4 }),
@@ -115,6 +149,7 @@ describe("selectBadgeMR tiebreak", () => {
const higherIID = makeMR({ id: "a", mr_iid: 12 });
const lowerIID = makeMR({ id: "b", mr_iid: 7 });
const options = makeAutomation({
mr_options: [makeMROptions(12), makeMROptions(7)],
mr_states: [
makeState({ mr_iid: 12, auto_fix_round_count: 2 }),
makeState({ mr_iid: 7, auto_fix_round_count: 2 }),
@@ -127,9 +162,53 @@ describe("selectBadgeMR tiebreak", () => {
expect(selectBadgeMR([lowerIID, higherIID], options)?.mr_iid).toBe(7);
});
it("returns null when auto-fix is disabled", () => {
it("returns null when this MR's own auto-fix switch is off", () => {
const mr = makeMR();
expect(selectBadgeMR([mr], makeAutomation({ auto_fix_enabled: false }))).toBeNull();
const options = makeAutomation({
mr_options: [makeMROptions(81, { auto_fix_enabled: false })],
});
expect(selectBadgeMR([mr], options)).toBeNull();
});
// The switches are per MR, so the task-level boolean is an aggregate
// ("every linked MR has it on"). Gating the badge on that aggregate hid
// the round counter for the MR that genuinely had auto-fix running.
it("still selects the one MR whose own auto-fix is on while a sibling's is off", () => {
const enabled = makeMR({ id: "a", mr_iid: 3 });
const disabled = makeMR({ id: "b", mr_iid: 9 });
const options = makeAutomation({
// Aggregate is false precisely because !9 is off — the old code read
// this and returned null, showing no badge for !3 at all.
auto_fix_enabled: false,
mr_options: [makeMROptions(3), makeMROptions(9, { auto_fix_enabled: false })],
mr_states: [
makeState({ mr_iid: 3, auto_fix_round_count: 2 }),
// A higher round on the disabled MR must not win the badge.
makeState({ mr_iid: 9, auto_fix_round_count: 7 }),
],
});
expect(selectBadgeMR([enabled, disabled], options)?.id).toBe("a");
expect(selectBadgeMR([disabled, enabled], options)?.id).toBe("a");
expect(chipAutomation(options, [enabled, disabled]).autoFixRound).toEqual({
current: 2,
max: 5,
exhausted: false,
});
});
// Payloads that predate per-MR scoping carry no mr_options at all; the
// task-level booleans are still the only signal there.
it("falls back to the task-level boolean when mr_options is absent", () => {
const mr = makeMR();
const legacy = makeAutomation({ mr_options: undefined as never, auto_fix_enabled: true });
expect(selectBadgeMR([mr], legacy)?.id).toBe("association-1");
expect(
selectBadgeMR(
[mr],
makeAutomation({ mr_options: undefined as never, auto_fix_enabled: false }),
),
).toBeNull();
});
});
@@ -140,6 +219,7 @@ describe("chipAutomation", () => {
const exhausted = makeMR({ id: "a", mr_iid: 3 });
const notExhausted = makeMR({ id: "b", mr_iid: 9 });
const options = makeAutomation({
mr_options: [makeMROptions(3), makeMROptions(9)],
mr_states: [
makeState({
mr_iid: 3,
@@ -154,4 +234,41 @@ describe("chipAutomation", () => {
expect(result.autoFixRound).toEqual({ current: 3, max: 5, exhausted: true });
});
// A badge lights up when ANY open MR has that switch on. Reading the
// task-level aggregate here left the chip silently under-reporting an
// auto-merge that was armed on one of two linked MRs.
it("lights each badge when any single open MR has that switch on", () => {
const armed = makeMR({ id: "a", mr_iid: 3 });
const idle = makeMR({ id: "b", mr_iid: 9 });
const options = makeAutomation({
auto_fix_enabled: false,
auto_merge_enabled: false,
mr_options: [
makeMROptions(3, { auto_fix_enabled: false, auto_merge_enabled: true }),
makeMROptions(9, { auto_fix_enabled: true, auto_merge_enabled: false }),
],
mr_states: [makeState({ mr_iid: 9, auto_fix_round_count: 1 })],
});
const result = chipAutomation(options, [armed, idle]);
expect(result.autoMergeEnabled).toBe(true);
expect(result.autoFixEnabled).toBe(true);
});
it("leaves both badges off when no open MR has either switch on", () => {
const mr = makeMR();
const options = makeAutomation({
auto_fix_enabled: true,
auto_merge_enabled: true,
mr_options: [makeMROptions(81, { auto_fix_enabled: false, auto_merge_enabled: false })],
});
const result = chipAutomation(options, [mr]);
expect(result.autoFixEnabled).toBe(false);
expect(result.autoMergeEnabled).toBe(false);
expect(result.autoFixRound).toBeNull();
});
});
@@ -4,6 +4,7 @@ import { useCallback, useEffect, useRef, useState } from "react";
import { compareChipMR } from "./mr-task-icon";
import {
autoFixRoundForState,
findMRAutomationOptionsForMR,
findMRAutomationStateForMR,
type AutoFixRoundInfo,
} from "@/lib/gitlab/mr-automation";
@@ -22,6 +23,29 @@ function autoFixRoundFor(options: TaskMRAutomationOptions | null, mr: TaskMR): A
);
}
/**
* One MR's own automation switches. The response's top-level booleans are an
* aggregate ("every linked MR has this on, and at least one MR is linked"),
* so reading them per MR would hide a switch the user enabled on only some
* of the linked MRs — the chip would show no auto-fix badge and no round
* counter while auto-fix was actively spending its round budget on one MR.
* Falls back to the aggregate only when `mr_options` is absent, i.e. a
* payload that predates per-MR scoping. Mirrors GitHub's automationForPR.
*/
function switchesForMR(
options: TaskMRAutomationOptions | null,
mr: TaskMR,
): { autoFix: boolean; autoMerge: boolean } {
if (!options?.mr_options) {
return {
autoFix: Boolean(options?.auto_fix_enabled),
autoMerge: Boolean(options?.auto_merge_enabled),
};
}
const row = findMRAutomationOptionsForMR(options.mr_options, mr);
return { autoFix: row.auto_fix_enabled, autoMerge: row.auto_merge_enabled };
}
function isBadgeMRBetter(
candidateRound: AutoFixRoundInfo,
candidate: TaskMR,
@@ -36,20 +60,25 @@ function isBadgeMRBetter(
/**
* The badge-selected MR: the most attention-worthy auto-fix round across the
* open MRs — an exhausted round beats a non-exhausted one, then a higher
* `current` wins, then the same `mr_iid` / `project_path` / `id` order
* `selectChipMR` uses (spec: Selection and ordering). Distinct from the live
* and frozen selections, and deliberately never frozen (spec: "The
* automation badges do NOT freeze").
* open MRs that have auto-fix enabled *for themselves* — an exhausted round
* beats a non-exhausted one, then a higher `current` wins, then the same
* `mr_iid` / `project_path` / `id` order `selectChipMR` uses (spec: Selection
* and ordering). Distinct from the live and frozen selections, and
* deliberately never frozen (spec: "The automation badges do NOT freeze").
*
* The per-MR gate replaces a task-level `auto_fix_enabled` check: now that
* the switches are per MR, that top-level boolean is an aggregate over every
* linked MR, so gating on it hid the round counter entirely whenever
* auto-fix was on for some linked MRs but not all.
*/
export function selectBadgeMR(
openMRs: TaskMR[],
options: TaskMRAutomationOptions | null,
): TaskMR | null {
if (!options?.auto_fix_enabled) return null;
let best: TaskMR | null = null;
let bestRound: AutoFixRoundInfo | null = null;
for (const mr of openMRs) {
if (!switchesForMR(options, mr).autoFix) continue;
const round = autoFixRoundFor(options, mr);
if (!best || !bestRound || isBadgeMRBetter(round, mr, bestRound, best)) {
best = mr;
@@ -59,15 +88,22 @@ export function selectBadgeMR(
return best;
}
/** Automation flags + round info for the chip's badges, live (never frozen). */
/**
* Automation flags + round info for the chip's badges, live (never frozen).
* A badge lights up when *any* open MR has that switch on, mirroring
* GitHub's automationForPRs — the task-level booleans read "on" only when
* every linked MR has the switch on, which would leave the chip silently
* under-reporting an armed auto-merge.
*/
export function chipAutomation(
options: TaskMRAutomationOptions | null,
openMRs: TaskMR[],
): ChipAutomation {
const badgeMR = selectBadgeMR(openMRs, options);
const perMR = openMRs.map((mr) => switchesForMR(options, mr));
return {
autoFixEnabled: Boolean(options?.auto_fix_enabled),
autoMergeEnabled: Boolean(options?.auto_merge_enabled),
autoFixEnabled: perMR.some((s) => s.autoFix),
autoMergeEnabled: perMR.some((s) => s.autoMerge),
autoFixRound: badgeMR ? autoFixRoundFor(options, badgeMR) : null,
};
}
@@ -3,7 +3,11 @@ import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { useHoverPopover } from "@/hooks/domains/github/use-hover-popover";
import { MRStatusChip } from "./mr-status-chip";
import type { TaskMR, TaskMRAutomationOptions } from "@/lib/types/gitlab";
import type {
TaskMR,
TaskMRAutomationOptions,
TaskMRAutomationOptionsForMR,
} from "@/lib/types/gitlab";
const OPEN_DELAY_MS = 150;
const CHIP_TESTID = "mr-status-chip";
@@ -130,11 +134,38 @@ function makeAutomation(overrides: Partial<TaskMRAutomationOptions> = {}): TaskM
prompt_on_closed: false,
review_reviewer_username: "",
updated_at: "",
mr_options: [],
mr_states: [],
...overrides,
};
}
/**
* One `mr_options` row for the default `makeMR()` MR. The badges read each
* MR's own switches (the top-level booleans are an aggregate over every
* linked MR), so a fixture with open MRs must carry a matching row —
* `mr_options: []` alongside an open MR is a state the backend cannot
* produce, since it emits one entry per linked MR.
*/
function makeMROptions(
overrides: Partial<TaskMRAutomationOptionsForMR> = {},
): TaskMRAutomationOptionsForMR {
return {
task_id: "task-1",
repository_id: "",
project_path: "group/project",
mr_iid: 81,
auto_fix_enabled: false,
auto_merge_enabled: false,
prompt_on_review_requested: false,
prompt_on_merged: false,
prompt_on_closed: false,
created_at: "",
updated_at: "",
...overrides,
};
}
function resetChipMocks() {
vi.useFakeTimers();
touchMocks.usesTouchDrawer = false;
@@ -197,6 +228,7 @@ describe("MRStatusChip rendering and selection", () => {
auto_fix_enabled: true,
auto_fix_max_rounds: 5,
auto_merge_enabled: true,
mr_options: [makeMROptions({ auto_fix_enabled: true, auto_merge_enabled: true })],
});
render(createElement(MRStatusChip, { taskId: "task-1" }));
const badge = screen.getByTestId("mr-status-auto-fix-chip");
@@ -192,7 +192,6 @@ function MRMenuTriggerButton({
function MRDropdownList({
taskId,
mrs,
single,
canLink,
onOpenReview,
onUnlink,
@@ -200,7 +199,6 @@ function MRDropdownList({
}: {
taskId: string;
mrs: TaskMR[];
single: TaskMR | null;
canLink: boolean;
onOpenReview: (mr: TaskMR) => void;
onUnlink: (associationId: string) => void;
@@ -230,10 +228,12 @@ function MRDropdownList({
{t("gitlab:unlink")}
{mr.mr_iid}
</DropdownMenuItem>
{/* The automation switches are per-MR, so each linked MR gets its
own group here rather than one shared group for the task. */}
<DropdownMenuSeparator />
<MRAutomationControls taskId={taskId} mr={mr} />
</div>
))}
<DropdownMenuSeparator />
<MRAutomationControls taskId={taskId} mr={single ?? undefined} />
{canLink ? (
<>
<DropdownMenuSeparator />
@@ -381,7 +381,6 @@ function MRMenuButton({
<MRDropdownList
taskId={taskId}
mrs={mrs}
single={single}
canLink={canLink}
onOpenReview={openReview}
onUnlink={onUnlink}
+65
View File
@@ -3,6 +3,14 @@ import type { ApiClient, MockGitLabMRSeed } from "./api-client";
export const GITLAB_HOST = "https://gitlab.example.test";
export const GITLAB_PROJECT = "platform/kandev";
type GitLabTaskSeedData = {
workspaceId: string;
repositoryId: string;
agentProfileId: string;
workflowId: string;
startStepId: string;
};
export function gitLabMR(
iid: number,
title: string,
@@ -46,6 +54,23 @@ export async function seedGitLabReview(
host = GITLAB_HOST,
): Promise<void> {
await apiClient.configureGitLab(workspaceId, host);
await seedGitLabMRData(apiClient, workspaceId, iid, title, host);
}
// The MR-data half of seedGitLabReview, without the `configureGitLab` call.
// Configuring the connection invalidates and rebuilds the workspace's cached
// GitLab client (SetConfigForWorkspace -> invalidateWorkspaceClient), which
// discards the in-memory MockClient's previously seeded state — so seeding
// two-or-more MRs on one workspace must configure once, then call this for
// each MR, rather than calling seedGitLabReview (and its configureGitLab)
// per MR.
export async function seedGitLabMRData(
apiClient: ApiClient,
workspaceId: string,
iid: number,
title: string,
host = GITLAB_HOST,
): Promise<void> {
const mr = gitLabMR(iid, title, {
url: `${host}/${GITLAB_PROJECT}/-/merge_requests/${iid}`,
web_url: `${host}/${GITLAB_PROJECT}/-/merge_requests/${iid}`,
@@ -108,3 +133,43 @@ export async function seedGitLabReview(
},
]);
}
export async function seedTaskWithLinkedGitLabMRs(
apiClient: ApiClient,
seedData: GitLabTaskSeedData,
title: string,
iids: number[],
mrTitlePrefix: string,
): Promise<string> {
// Configuring the connection invalidates and rebuilds the workspace's cached
// mock client, so it must happen before seeding every MR for this task.
await apiClient.configureGitLab(seedData.workspaceId, GITLAB_HOST);
for (const iid of iids) {
await seedGitLabMRData(apiClient, seedData.workspaceId, iid, `${mrTitlePrefix} ${iid}`);
}
await apiClient.updateRepository(seedData.repositoryId, {
provider: "gitlab",
provider_host: GITLAB_HOST,
provider_owner: "platform",
provider_name: "kandev",
});
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
title,
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
},
);
for (const iid of iids) {
await apiClient.linkTaskGitLabMR(seedData.workspaceId, {
task_id: task.id,
repository_id: seedData.repositoryId,
mr_url: `${GITLAB_HOST}/${GITLAB_PROJECT}/-/merge_requests/${iid}`,
});
}
return task.id;
}
@@ -0,0 +1,16 @@
import { describe, expect, it, vi } from "vitest";
import { retryGitIndexLock } from "../tests/review/submodule-review-helpers";
describe("retryGitIndexLock", () => {
it("retries a transient Git index lock before returning the operation result", async () => {
const operation = vi
.fn<() => string>()
.mockImplementationOnce(() => {
throw new Error("fatal: Unable to create index.lock: File exists");
})
.mockReturnValueOnce("complete");
await expect(retryGitIndexLock(operation)).resolves.toBe("complete");
expect(operation).toHaveBeenCalledTimes(2);
});
});
@@ -0,0 +1,112 @@
// One-off manual seed script for the "Scope GitLab MR automation switches
// per MR" task's isolated environment (STEP 3 of the Work phase). Not part
// of the automated test suite — run with `npx tsx` against a manually
// launched backend. Seeds a workspace/workflow/repo and a task with two
// linked GitLab MRs so the multi-MR dropdown independence UI can be
// exercised by hand.
import { execFileSync } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
import { ApiClient } from "./helpers/api-client";
import { seedGitLabMRData, GITLAB_HOST, GITLAB_PROJECT } from "./helpers/gitlab";
import { dwell } from "./helpers/causal-waits";
const BASE_URL = process.env.KANDEV_BASE_URL || "http://localhost:18500";
// Resolved from this file's own location so the script works in any checkout;
// `__dirname` matches how global-setup.ts and the e2e helpers locate paths.
const REPO_ROOT =
process.env.KANDEV_SEED_REPO_ROOT || path.resolve(__dirname, "../../backend/.manual-env/repos");
async function main() {
const apiClient = new ApiClient(BASE_URL);
const workspace = await apiClient.createWorkspace("GitLab MR Automation Demo");
const workflow = await apiClient.createWorkflow(workspace.id, "Demo Workflow", "simple");
const { steps } = await apiClient.listWorkflowSteps(workflow.id);
const sorted = steps.sort((a, b) => a.position - b.position);
const startStep = sorted.find((s) => s.is_start_step) ?? sorted[0];
const remoteDir = path.join(REPO_ROOT, "e2e-remote.git");
const repoDir = path.join(REPO_ROOT, "e2e-repo");
fs.mkdirSync(REPO_ROOT, { recursive: true });
if (!fs.existsSync(remoteDir)) {
execFileSync("git", ["init", "--bare", "-b", "main", remoteDir]);
fs.mkdirSync(repoDir, { recursive: true });
execFileSync("git", ["init", "-b", "main"], { cwd: repoDir });
execFileSync(
"git",
[
"-c",
"user.name=Demo",
"-c",
"user.email=demo@test.local",
"commit",
"--allow-empty",
"-m",
"init",
],
{ cwd: repoDir },
);
execFileSync("git", ["remote", "add", "origin", `file://${remoteDir}`], { cwd: repoDir });
execFileSync("git", ["push", "origin", "main"], { cwd: repoDir });
}
const repo = await apiClient.createRepository(workspace.id, repoDir);
let agentProfileId: string | undefined;
const deadline = Date.now() + 30_000;
while (Date.now() < deadline) {
const { agents } = await apiClient.listAgents();
agentProfileId = agents[0]?.profiles[0]?.id;
if (agentProfileId) break;
await dwell(
250,
"poll-interval",
"polling listAgents() until the backend's initial agent setup completes",
);
}
if (!agentProfileId) throw new Error("no agent profile available after 30s");
const iidA = 220;
const iidB = 221;
await apiClient.configureGitLab(workspace.id, GITLAB_HOST);
await seedGitLabMRData(apiClient, workspace.id, iidA, "Fix pagination bug");
await seedGitLabMRData(apiClient, workspace.id, iidB, "Add dark mode toggle");
await apiClient.updateRepository(repo.id, {
provider: "gitlab",
provider_host: GITLAB_HOST,
provider_owner: "platform",
provider_name: "kandev",
});
const task = await apiClient.createTaskWithAgent(
workspace.id,
"Demo: two linked GitLab MRs",
agentProfileId,
{
description: "Demo task seeded for manual verification of per-MR automation scoping.",
workflow_id: workflow.id,
workflow_step_id: startStep.id,
repository_ids: [repo.id],
},
);
await apiClient.linkTaskGitLabMR(workspace.id, {
task_id: task.id,
repository_id: repo.id,
mr_url: `${GITLAB_HOST}/${GITLAB_PROJECT}/-/merge_requests/${iidA}`,
});
await apiClient.linkTaskGitLabMR(workspace.id, {
task_id: task.id,
repository_id: repo.id,
mr_url: `${GITLAB_HOST}/${GITLAB_PROJECT}/-/merge_requests/${iidB}`,
});
console.log("Seed complete.");
console.log(`Workspace: ${workspace.id}`);
console.log(`Task: ${task.id}`);
console.log(`Open: ${BASE_URL}/t/${task.id}`);
}
main().catch((err) => {
console.error(err);
process.exit(1);
});
@@ -1,6 +1,11 @@
import { test, expect } from "../../fixtures/test-base";
import { SessionPage } from "../../pages/session-page";
import { seedGitLabReview, GITLAB_HOST, GITLAB_PROJECT } from "../../helpers/gitlab";
import {
seedGitLabReview,
seedTaskWithLinkedGitLabMRs,
GITLAB_HOST,
GITLAB_PROJECT,
} from "../../helpers/gitlab";
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
import type { ApiClient } from "../../helpers/api-client";
import type { SeedData } from "../../fixtures/test-base";
@@ -188,4 +193,59 @@ test.describe("mobile GitLab MR automation options", () => {
await assertNoDocumentHorizontalOverflow(testPage, "mobile MR automation load error");
});
test("dropdown renders one attributed automation block per linked MR, independently toggleable (AC1, AC26)", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(120_000);
const iidA = 222;
const iidB = 223;
const taskId = await seedTaskWithLinkedGitLabMRs(
apiClient,
seedData,
"Mobile MR automation independence",
[iidA, iidB],
"Mobile MR automation independence",
);
await testPage.goto(`/t/${taskId}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
const mrButton = testPage.getByTestId("mr-topbar-button");
await expect(mrButton).toBeVisible({ timeout: 15_000 });
await mrButton.tap();
await waitForDropdownSettled(testPage);
const controlsA = testPage.locator(
`[data-testid="mr-automation-controls"][data-mr-iid="${iidA}"]`,
);
const controlsB = testPage.locator(
`[data-testid="mr-automation-controls"][data-mr-iid="${iidB}"]`,
);
await expect(controlsA).toBeVisible();
await expect(controlsB).toBeVisible();
await expect(controlsA.getByTestId("mr-automation-scope-label")).toHaveText(
`Applies to !${iidA}`,
);
await expect(controlsB.getByTestId("mr-automation-scope-label")).toHaveText(
`Applies to !${iidB}`,
);
const autoFixA = controlsA.getByRole("switch", { name: "Auto-fix CI and address comments" });
const autoFixB = controlsB.getByRole("switch", { name: "Auto-fix CI and address comments" });
await autoFixA.tap();
await expect
.poll(async () => {
const options = await apiClient.getTaskMRAutomationOptions(taskId);
return options.mr_options?.find((o) => o.mr_iid === iidA)?.auto_fix_enabled;
})
.toBe(true);
const options = await apiClient.getTaskMRAutomationOptions(taskId);
expect(options.mr_options?.find((o) => o.mr_iid === iidB)?.auto_fix_enabled).toBe(false);
await expect(autoFixB).not.toBeChecked();
await assertNoDocumentHorizontalOverflow(testPage, "mobile MR automation multi-MR dropdown");
});
});
@@ -1,6 +1,11 @@
import { test, expect } from "../../fixtures/test-base";
import { SessionPage } from "../../pages/session-page";
import { seedGitLabReview, GITLAB_HOST, GITLAB_PROJECT } from "../../helpers/gitlab";
import {
seedGitLabReview,
seedTaskWithLinkedGitLabMRs,
GITLAB_HOST,
GITLAB_PROJECT,
} from "../../helpers/gitlab";
import type { ApiClient } from "../../helpers/api-client";
import type { SeedData } from "../../fixtures/test-base";
@@ -231,3 +236,149 @@ test.describe("GitLab MR automation options", () => {
).not.toBeChecked();
});
});
test.describe("GitLab MR automation — multi-MR independence (AC1-AC3, AC26)", () => {
test("toggling one linked MR's switches does not affect a second linked MR, and survives reload", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(120_000);
const iidA = 220;
const iidB = 221;
const taskId = await seedTaskWithLinkedGitLabMRs(
apiClient,
seedData,
"MR automation independence",
[iidA, iidB],
"MR automation independence",
);
await openTask(testPage, taskId);
// 2+ linked MRs always render the click-only dropdown — never the
// single-MR hover popover — with one attributed Automation block per MR.
await testPage.getByTestId("mr-topbar-button").click();
const controlsA = testPage.locator(
`[data-testid="mr-automation-controls"][data-mr-iid="${iidA}"]`,
);
const controlsB = testPage.locator(
`[data-testid="mr-automation-controls"][data-mr-iid="${iidB}"]`,
);
await expect(controlsA).toBeVisible();
await expect(controlsB).toBeVisible();
// AC25: each block states which MR it applies to.
await expect(controlsA.getByTestId("mr-automation-scope-label")).toHaveText(
`Applies to !${iidA}`,
);
await expect(controlsB.getByTestId("mr-automation-scope-label")).toHaveText(
`Applies to !${iidB}`,
);
const autoFixA = controlsA.getByRole("switch", { name: "Auto-fix CI and address comments" });
const autoFixB = controlsB.getByRole("switch", { name: "Auto-fix CI and address comments" });
await expect(autoFixA).not.toBeChecked();
await expect(autoFixB).not.toBeChecked();
// AC27: unique element ids per MR, not duplicated across simultaneously
// mounted blocks.
const autoFixAID = await autoFixA.getAttribute("id");
const autoFixBID = await autoFixB.getAttribute("id");
expect(autoFixAID).not.toBeNull();
expect(autoFixAID).not.toBe(autoFixBID);
// AC1: enabling MR A's auto-fix switch does not enable MR B's.
await autoFixA.click();
await expect
.poll(async () => {
const options = await apiClient.getTaskMRAutomationOptions(taskId);
return options.mr_options?.find((o) => o.mr_iid === iidA)?.auto_fix_enabled;
})
.toBe(true);
const midOptions = await apiClient.getTaskMRAutomationOptions(taskId);
expect(midOptions.mr_options?.find((o) => o.mr_iid === iidB)?.auto_fix_enabled).toBe(false);
await expect(autoFixB).not.toBeChecked();
// AC2: same independence for auto-merge.
const autoMergeA = controlsA.getByRole("switch", { name: "Auto-merge when ready" });
const autoMergeB = controlsB.getByRole("switch", { name: "Auto-merge when ready" });
await autoMergeA.click();
await expect
.poll(async () => {
const options = await apiClient.getTaskMRAutomationOptions(taskId);
return options.mr_options?.find((o) => o.mr_iid === iidA)?.auto_merge_enabled;
})
.toBe(true);
const afterMergeOptions = await apiClient.getTaskMRAutomationOptions(taskId);
expect(afterMergeOptions.mr_options?.find((o) => o.mr_iid === iidB)?.auto_merge_enabled).toBe(
false,
);
await expect(autoMergeB).not.toBeChecked();
// AC3: same independence for the three Review follow-up switches — MR A only.
await controlsA.getByTestId("mr-review-follow-up-trigger").click();
await controlsA.getByRole("switch", { name: "Your review is requested" }).click();
await controlsA.getByRole("switch", { name: "MR merged" }).click();
await controlsA.getByRole("switch", { name: "MR closed without merging" }).click();
await expect
.poll(async () => {
const options = await apiClient.getTaskMRAutomationOptions(taskId);
const option = options.mr_options?.find((o) => o.mr_iid === iidA);
return {
prompt_on_review_requested: option?.prompt_on_review_requested,
prompt_on_merged: option?.prompt_on_merged,
prompt_on_closed: option?.prompt_on_closed,
};
})
.toEqual({
prompt_on_review_requested: true,
prompt_on_merged: true,
prompt_on_closed: true,
});
const afterReviewOptions = await apiClient.getTaskMRAutomationOptions(taskId);
const optionB = afterReviewOptions.mr_options?.find((o) => o.mr_iid === iidB);
expect(optionB).toMatchObject({
prompt_on_review_requested: false,
prompt_on_merged: false,
prompt_on_closed: false,
});
// AC1 (reload): !A stays on, !B stays off after a fresh mount.
await testPage.reload();
await expect(testPage.getByTestId("mr-topbar-button")).toBeVisible({ timeout: 15_000 });
await testPage.getByTestId("mr-topbar-button").click();
const reloadedControlsA = testPage.locator(
`[data-testid="mr-automation-controls"][data-mr-iid="${iidA}"]`,
);
const reloadedControlsB = testPage.locator(
`[data-testid="mr-automation-controls"][data-mr-iid="${iidB}"]`,
);
await expect(
reloadedControlsA.getByRole("switch", { name: "Auto-fix CI and address comments" }),
).toBeChecked();
await expect(
reloadedControlsA.getByRole("switch", { name: "Auto-merge when ready" }),
).toBeChecked();
await expect(
reloadedControlsA.getByRole("switch", { name: "Your review is requested" }),
).toBeChecked();
await expect(reloadedControlsA.getByRole("switch", { name: "MR merged" })).toBeChecked();
await expect(
reloadedControlsA.getByRole("switch", { name: "MR closed without merging" }),
).toBeChecked();
await expect(
reloadedControlsB.getByRole("switch", { name: "Auto-fix CI and address comments" }),
).not.toBeChecked();
await expect(
reloadedControlsB.getByRole("switch", { name: "Auto-merge when ready" }),
).not.toBeChecked();
await reloadedControlsB.getByTestId("mr-review-follow-up-trigger").click();
await expect(
reloadedControlsB.getByRole("switch", { name: "Your review is requested" }),
).not.toBeChecked();
await expect(reloadedControlsB.getByRole("switch", { name: "MR merged" })).not.toBeChecked();
await expect(
reloadedControlsB.getByRole("switch", { name: "MR closed without merging" }),
).not.toBeChecked();
});
});
@@ -232,6 +232,65 @@ test.describe("GitLab MR status chip", () => {
await expect(chip.getByTestId("mr-status-auto-merge-chip")).toBeVisible();
});
// The switches are per MR, so the response's top-level booleans are an
// aggregate ("on for every linked MR"). The chip used to read that
// aggregate, so enabling automation on one of two linked MRs rendered no
// badge row at all (mr-status-chip-trigger.tsx returns null when both
// flags are false) while auto-fix was genuinely running on that MR.
test("renders the badges when only one of two linked MRs has automation on", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(120_000);
const ARMED_IID = 410;
const IDLE_IID = 411;
// Configure once, then seed both MRs: each configureGitLab call rebuilds
// the workspace's cached mock client and discards MRs seeded before it.
await apiClient.configureGitLab(seedData.workspaceId, GITLAB_HOST);
await apiClient.mockGitLabAddMRs(seedData.workspaceId, GITLAB_PROJECT, [
mrSeed(ARMED_IID, "Armed MR"),
mrSeed(IDLE_IID, "Idle MR"),
]);
await apiClient.mockGitLabAddPipelines(seedData.workspaceId, GITLAB_PROJECT, [
pipelineSeed(ARMED_IID, "success"),
]);
await apiClient.mockGitLabAddApprovals(seedData.workspaceId, GITLAB_PROJECT, ARMED_IID, [], 1);
await apiClient.mockGitLabAddApprovals(seedData.workspaceId, GITLAB_PROJECT, IDLE_IID, [], 1);
const task = await createTask(apiClient, seedData, "MR chip per-MR badges");
await linkMR(apiClient, seedData, task.id, ARMED_IID);
await linkMR(apiClient, seedData, task.id, IDLE_IID);
await apiClient.updateTaskMRAutomationOptions(task.id, {
repository_id: seedData.repositoryId,
project_path: GITLAB_PROJECT,
mr_iid: ARMED_IID,
auto_fix_enabled: true,
auto_merge_enabled: true,
});
// Pin the precondition: the aggregate the chip used to read is false,
// so a passing assertion below cannot come from the old code path.
const options = await apiClient.getTaskMRAutomationOptions(task.id);
expect(options.auto_fix_enabled).toBe(false);
expect(options.auto_merge_enabled).toBe(false);
expect(options.mr_options?.find((o) => o.mr_iid === ARMED_IID)?.auto_fix_enabled).toBe(true);
expect(options.mr_options?.find((o) => o.mr_iid === IDLE_IID)?.auto_fix_enabled).toBe(false);
const session = new SessionPage(testPage);
await openTask(testPage, session, task.id);
const chip = session.mrStatusChip();
await expect(chip).toBeVisible({ timeout: 15_000 });
await expect(chip).toHaveAttribute("data-mr-count", "2");
const autoFixBadge = chip.getByTestId("mr-status-auto-fix-chip");
await expect(autoFixBadge).toBeVisible();
// The round comes from the armed MR, which has no fix rounds yet.
await expect(autoFixBadge).toContainText("0/10");
await expect(chip.getByTestId("mr-status-auto-merge-chip")).toBeVisible();
});
test("DOM order: pr-status-chip precedes mr-status-chip when a task has both an open PR and MR", async ({
testPage,
apiClient,
@@ -25,7 +25,7 @@ test.describe("Nested submodule Review on mobile", () => {
await session.waitForChatIdle({ timeout: 45_000 });
const worktreePath = await fixture.waitForWorktree(apiClient);
fixture.applyNestedChanges(worktreePath);
await fixture.applyNestedChanges(worktreePath);
await testPage.getByRole("button", { name: "Changes" }).tap();
const changesPanel = testPage.getByTestId("mobile-changes-panel");
@@ -8,13 +8,15 @@ import type { SeedData } from "../../fixtures/test-base";
import { makeGitEnv } from "../../helpers/git-helper";
const GIT_PROTOCOL_ARGS = ["-c", "protocol.file.allow=always"];
const GIT_INDEX_LOCK_ATTEMPTS = 3;
const GIT_INDEX_LOCK_RETRY_MS = 300;
export type SubmoduleReviewFixture = {
taskId: string;
sessionId: string;
sourceRoot: string;
waitForWorktree: (apiClient: ApiClient) => Promise<string>;
applyNestedChanges: (worktreePath: string) => void;
applyNestedChanges: (worktreePath: string) => Promise<void>;
cleanup: () => void;
};
@@ -27,6 +29,26 @@ function runGit(repoPath: string, args: string[], env: NodeJS.ProcessEnv): strin
});
}
/** Retries the short-lived index lock taken by the backend's Git status refresh. */
export async function retryGitIndexLock<T>(operation: () => T): Promise<T> {
for (let attempt = 0; attempt < GIT_INDEX_LOCK_ATTEMPTS; attempt++) {
try {
return operation();
} catch (error) {
const isLastAttempt = attempt === GIT_INDEX_LOCK_ATTEMPTS - 1;
if (!(error instanceof Error) || !error.message.includes("index.lock") || isLastAttempt) {
throw error;
}
await dwell(
GIT_INDEX_LOCK_RETRY_MS,
"poll-interval",
"the backend's periodic Git status refresh can briefly hold the submodule index lock without publishing a completion event",
);
}
}
throw new Error("Git index lock retry exhausted");
}
function initializeRepository(
repoPath: string,
env: NodeJS.ProcessEnv,
@@ -48,9 +70,9 @@ export function readGitValue(repoPath: string, args: string[], tempRoot: string)
}).trim();
}
function commit(repoPath: string, env: NodeJS.ProcessEnv, message: string): void {
runGit(repoPath, ["add", "-A"], env);
runGit(repoPath, ["commit", "-m", message], env);
async function commit(repoPath: string, env: NodeJS.ProcessEnv, message: string): Promise<void> {
await retryGitIndexLock(() => runGit(repoPath, ["add", "-A"], env));
await retryGitIndexLock(() => runGit(repoPath, ["commit", "-m", message], env));
}
async function waitForWorktreePath(
@@ -92,7 +114,7 @@ export async function createSubmoduleReviewFixture(
initializeRepository(outerPath, env, "README.md", "outer base\n");
runGit(outerPath, [...GIT_PROTOCOL_ARGS, "submodule", "add", "../inner", "vendor/inner"], env);
commit(outerPath, env, "add nested inner submodule");
await commit(outerPath, env, "add nested inner submodule");
initializeRepository(parentPath, env, "README.md", "parent base\n");
runGit(parentPath, [...GIT_PROTOCOL_ARGS, "submodule", "add", "../outer", "vendor/outer"], env);
@@ -102,7 +124,7 @@ export async function createSubmoduleReviewFixture(
[...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"],
env,
);
commit(parentPath, env, "add outer submodule");
await commit(parentPath, env, "add outer submodule");
const repository = await apiClient.createRepository(seedData.workspaceId, parentPath, "main", {
name: "nested-submodule-parent",
@@ -131,22 +153,24 @@ export async function createSubmoduleReviewFixture(
sessionId: task.session_id,
sourceRoot,
waitForWorktree: (client) => waitForWorktreePath(client, task.id, task.session_id!),
applyNestedChanges(worktreePath: string) {
async applyNestedChanges(worktreePath: string) {
const outerWorktree = path.join(worktreePath, "vendor/outer");
const innerWorktree = path.join(outerWorktree, "vendor/inner");
if (!fs.existsSync(innerWorktree)) {
runGit(
worktreePath,
[...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"],
env,
await retryGitIndexLock(() =>
runGit(
worktreePath,
[...GIT_PROTOCOL_ARGS, "submodule", "update", "--init", "--recursive"],
env,
),
);
}
fs.appendFileSync(path.join(worktreePath, "README.md"), "parent working-tree change\n");
fs.appendFileSync(path.join(outerWorktree, "README.md"), "outer committed change\n");
commit(outerWorktree, env, "change outer submodule");
await commit(outerWorktree, env, "change outer submodule");
fs.appendFileSync(path.join(innerWorktree, "README.md"), "inner committed change\n");
commit(innerWorktree, env, "change inner submodule");
commit(outerWorktree, env, "record inner submodule change");
await commit(innerWorktree, env, "change inner submodule");
await commit(outerWorktree, env, "record inner submodule change");
},
cleanup,
};
@@ -47,7 +47,7 @@ test.describe("Nested submodule Review", () => {
await session.waitForChatIdle({ timeout: 45_000 });
const worktreePath = await fixture.waitForWorktree(apiClient);
fixture.applyNestedChanges(worktreePath);
await fixture.applyNestedChanges(worktreePath);
const parentBaseSha = readGitValue(worktreePath, ["rev-parse", "HEAD"], backend.tmpDir);
await session.clickTab("Changes");
+3
View File
@@ -1421,6 +1421,9 @@ export const i18nGuardFiles = [
"components/github/pr-shared.tsx",
"components/github/pr-status-chip.tsx",
"components/github/pr-topbar-button.tsx",
"components/gitlab/mr-auto-fix-prompt-dialog.tsx",
"components/gitlab/mr-automation-controls.tsx",
"components/gitlab/mr-automation-rows.tsx",
"components/gitlab/mr-commits-section.tsx",
"components/gitlab/mr-task-status-summary.tsx",
"components/gitlab/mr-detail-panel.tsx",
@@ -2,7 +2,7 @@ import { createElement, type ReactNode } from "react";
import { act, cleanup, renderHook, waitFor } from "@testing-library/react";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { StateProvider, useAppStore } from "@/components/state-provider";
import type { TaskMRAutomationOptions } from "@/lib/types/gitlab";
import type { TaskMRAutomationOptions, TaskMRAutomationOptionsForMR } from "@/lib/types/gitlab";
const api = vi.hoisted(() => ({
getTaskMRAutomation: vi.fn(),
@@ -17,6 +17,25 @@ function wrapper({ children }: { children: ReactNode }) {
return createElement(StateProvider, null, children);
}
function defaultMROption(
overrides: Partial<TaskMRAutomationOptionsForMR> = {},
): TaskMRAutomationOptionsForMR {
return {
task_id: "task-1",
repository_id: "",
project_path: "group/project",
mr_iid: 7,
auto_fix_enabled: false,
auto_merge_enabled: false,
prompt_on_review_requested: false,
prompt_on_merged: false,
prompt_on_closed: false,
created_at: "",
updated_at: "",
...overrides,
};
}
function baseOptions(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAutomationOptions {
return {
task_id: "task-1",
@@ -31,6 +50,7 @@ function baseOptions(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAu
review_reviewer_username: "",
updated_at: "2026-01-01T00:00:00Z",
mr_states: [],
mr_options: [defaultMROption()],
...overrides,
};
}
@@ -94,6 +114,30 @@ describe("useTaskMRAutomationOptions fetching", () => {
});
expect(api.getTaskMRAutomation).toHaveBeenCalledTimes(1);
});
it("issues one fetch when several instances mount together for the same task", async () => {
// The switches are per-MR, so a task with N linked MRs mounts N copies of
// MRAutomationControls — each with its own useTaskMRAutomationOptions.
// They all read the same store slot, so they must not each fire a GET.
const pending = deferred<TaskMRAutomationOptions>();
api.getTaskMRAutomation.mockReturnValue(pending.promise);
const { result } = renderHook(
() => {
useTaskMRAutomationOptions("task-1");
useTaskMRAutomationOptions("task-1");
return useTaskMRAutomationOptions("task-1");
},
{ wrapper },
);
await waitFor(() => expect(api.getTaskMRAutomation).toHaveBeenCalledTimes(1));
await act(async () => {
pending.resolve(baseOptions({ prompt_on_merged: true }));
await pending.promise;
});
await waitFor(() => expect(result.current.options?.prompt_on_merged).toBe(true));
expect(api.getTaskMRAutomation).toHaveBeenCalledTimes(1);
});
});
describe("useTaskMRAutomationOptions optimistic updates", () => {
@@ -108,32 +152,44 @@ describe("useTaskMRAutomationOptions optimistic updates", () => {
void result.current.update({ prompt_on_merged: true });
});
// Optimistic reflect happens synchronously within the update call.
await waitFor(() => expect(result.current.options?.prompt_on_merged).toBe(true));
// Optimistic reflect happens synchronously within the update call, into
// the targeted MR's entry in mr_options (the per-MR source of truth) —
// not the task-level aggregate, which is server-computed.
await waitFor(() =>
expect(result.current.options?.mr_options?.[0]?.prompt_on_merged).toBe(true),
);
expect(result.current.saving).toBe(true);
await act(async () => {
update.resolve(baseOptions({ prompt_on_merged: true }));
update.resolve(baseOptions({ mr_options: [defaultMROption({ prompt_on_merged: true })] }));
});
await waitFor(() => expect(result.current.saving).toBe(false));
expect(result.current.options?.prompt_on_merged).toBe(true);
expect(result.current.options?.mr_options?.[0]?.prompt_on_merged).toBe(true);
expect(result.current.error).toBeNull();
});
it("reverts the optimistic update and surfaces an error on failure (AC27)", async () => {
api.getTaskMRAutomation.mockResolvedValue(baseOptions());
api.updateTaskMRAutomation.mockRejectedValue(new Error(NETWORK_DOWN_ERROR));
const update = deferred<TaskMRAutomationOptions>();
api.updateTaskMRAutomation.mockImplementation(() => update.promise);
const { result } = renderHook(() => useTaskMRAutomationOptions("task-1"), { wrapper });
await waitFor(() => expect(result.current.options).not.toBeNull());
let pending: Promise<unknown>;
act(() => {
pending = result.current.update({ prompt_on_closed: true });
});
await waitFor(() =>
expect(result.current.options?.mr_options?.[0]?.prompt_on_closed).toBe(true),
);
await act(async () => {
await expect(result.current.update({ prompt_on_closed: true })).rejects.toThrow(
NETWORK_DOWN_ERROR,
);
update.reject(new Error(NETWORK_DOWN_ERROR));
await expect(pending!).rejects.toThrow(NETWORK_DOWN_ERROR);
});
expect(result.current.options?.prompt_on_closed).toBe(false);
expect(result.current.options?.mr_options?.[0]?.prompt_on_closed).toBe(false);
expect(result.current.error).toBe(NETWORK_DOWN_ERROR);
expect(result.current.saving).toBe(false);
});
@@ -199,6 +255,89 @@ describe("useTaskMRAutomationOptions races", () => {
});
expect(result.current.options?.prompt_on_merged).toBe(true);
});
// A task with N linked MRs mounts N MRAutomationControls, each with its own
// useTaskMRAutomationOptions("task-1") instance. All instances read/write
// the same store slot, so their save-ordering guards must also be shared —
// otherwise an older instance's private counter never learns about a
// newer instance's save and treats its own stale response as current.
it("shares update ordering across multiple mounted instances for the same task, so an older instance's stale response cannot clobber a newer one's committed result", async () => {
api.getTaskMRAutomation.mockResolvedValue(baseOptions());
const { result } = renderHook(
() => ({
a: useTaskMRAutomationOptions("task-1"),
b: useTaskMRAutomationOptions("task-1"),
}),
{ wrapper },
);
await waitFor(() => expect(result.current.a.options).not.toBeNull());
const updateA = deferred<TaskMRAutomationOptions>();
const updateB = deferred<TaskMRAutomationOptions>();
api.updateTaskMRAutomation
.mockImplementationOnce(() => updateA.promise)
.mockImplementationOnce(() => updateB.promise);
// Instance A (e.g. !1's block) starts a save...
act(() => {
void result.current.a.update({ auto_fix_prompt_override: "from A" });
});
// ...then instance B (e.g. !2's block) starts a later save for the same
// task before A's resolves.
act(() => {
void result.current.b.update({ auto_fix_prompt_override: "from B" });
});
// B's later, authoritative response lands first...
await act(async () => {
updateB.resolve(baseOptions({ auto_fix_prompt_override: "from B" }));
});
expect(result.current.a.options?.auto_fix_prompt_override).toBe("from B");
// ...then A's now-stale response resolves after. It must not win.
await act(async () => {
updateA.resolve(baseOptions({ auto_fix_prompt_override: "from A" }));
});
expect(result.current.a.options?.auto_fix_prompt_override).toBe("from B");
expect(result.current.b.options?.auto_fix_prompt_override).toBe("from B");
});
});
describe("useTaskMRAutomationOptions revision ordering", () => {
it("commits a higher-revision response even when its request started before a lower-revision response", async () => {
api.getTaskMRAutomation.mockResolvedValue(baseOptions({ automation_revision: 1 }));
const { result } = renderHook(() => useTaskMRAutomationOptions("task-1"), { wrapper });
await waitFor(() => expect(result.current.options).not.toBeNull());
const olderRequest = deferred<TaskMRAutomationOptions>();
const newerRequest = deferred<TaskMRAutomationOptions>();
api.updateTaskMRAutomation
.mockImplementationOnce(() => olderRequest.promise)
.mockImplementationOnce(() => newerRequest.promise);
act(() => {
void result.current.update({ auto_fix_prompt_override: "older request" });
void result.current.update({ auto_fix_prompt_override: "newer request" });
});
await act(async () => {
newerRequest.resolve(
baseOptions({ automation_revision: 2, auto_fix_prompt_override: "revision two" }),
);
});
expect(result.current.options?.automation_revision).toBe(2);
// The first request can commit later when the database transaction that
// owns it receives the next revision. Request-start order is not server
// revision order, so the higher revision must still win.
await act(async () => {
olderRequest.resolve(
baseOptions({ automation_revision: 3, auto_fix_prompt_override: "revision three" }),
);
});
expect(result.current.options?.automation_revision).toBe(3);
expect(result.current.options?.auto_fix_prompt_override).toBe("revision three");
});
});
describe("useTaskMRAutomationOptions task switching", () => {
@@ -420,13 +559,15 @@ describe("useTaskMRAutomationOptions refresh/save interaction", () => {
// The refresh's pre-patch response resolves while the save is still
// pending — it must not flip the optimistic switch back off.
await act(async () => {
refreshCall.resolve(baseOptions({ prompt_on_merged: false }));
refreshCall.resolve(
baseOptions({ mr_options: [defaultMROption({ prompt_on_merged: false })] }),
);
});
expect(result.current.options?.prompt_on_merged).toBe(true);
expect(result.current.options?.mr_options?.[0]?.prompt_on_merged).toBe(true);
await act(async () => {
update.resolve(baseOptions({ prompt_on_merged: true }));
update.resolve(baseOptions({ mr_options: [defaultMROption({ prompt_on_merged: true })] }));
});
expect(result.current.options?.prompt_on_merged).toBe(true);
expect(result.current.options?.mr_options?.[0]?.prompt_on_merged).toBe(true);
});
});
@@ -1,14 +1,40 @@
"use client";
import { useCallback, useEffect, useRef, type RefObject } from "react";
import { useCallback, useEffect, type RefObject } from "react";
import { getTaskMRAutomation, updateTaskMRAutomation } from "@/lib/api/domains/gitlab-api";
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
import type { AppState } from "@/lib/state/store";
import type { TaskMRAutomationOptions, TaskMRAutomationPatch } from "@/lib/types/gitlab";
import type {
TaskMRAutomationOptions,
TaskMRAutomationOptionsForMR,
TaskMRAutomationPatch,
} from "@/lib/types/gitlab";
import { t } from "@/lib/i18n";
type AppStoreApi = ReturnType<typeof useAppStoreApi>;
type MRAutomationRequestRefs = Pick<
MRAutomationRequestContext,
"refreshRequestRef" | "updateRequestRef" | "updateSettleCounterRef"
>;
// One task can mount a control for every linked MR. Those controls share a
// store, so they must also share request ordering: an older control's full
// response must not overwrite a newer control's response for another MR.
const requestRefsByStore = new WeakMap<AppStoreApi, MRAutomationRequestRefs>();
function requestRefsForStore(storeApi: AppStoreApi): MRAutomationRequestRefs {
const existing = requestRefsByStore.get(storeApi);
if (existing) return existing;
const refs: MRAutomationRequestRefs = {
refreshRequestRef: { current: {} },
updateRequestRef: { current: {} },
updateSettleCounterRef: { current: {} },
};
requestRefsByStore.set(storeApi, refs);
return refs;
}
function errorMessage(error: unknown): string {
return error instanceof Error ? error.message : t("gitlab:failedToLoadMrAutomationOptions");
}
@@ -54,6 +80,14 @@ async function performRefresh(
setLoading,
setError,
} = ctx;
// A task with several linked MRs mounts one MRAutomationControls per MR,
// and every instance's mount effect sees the same pre-fetch render snapshot
// (options null, loading false) — so without this guard, opening the
// dropdown would fire one identical GET per linked MR. The in-flight
// request commits to the shared store slot that all of them read.
if (storeApi.getState().taskMRAutomation.loading[taskId]) {
return storeApi.getState().taskMRAutomation.byTaskId[taskId] ?? null;
}
const requestId = (refreshRequestRef.current[taskId] ?? 0) + 1;
refreshRequestRef.current[taskId] = requestId;
const settleCounterAtStart = updateSettleCounterRef.current[taskId] ?? 0;
@@ -89,6 +123,84 @@ async function performRefresh(
}
}
/**
* Applies a patch to the cached options the way the backend will. The five
* switches live per-MR in `mr_options`, so a naive `{...previous, ...patch}`
* would write them onto the task-level *aggregate* instead — showing the
* switch as on for every linked MR until the response landed. Switch fields
* are therefore merged into the targeted MR's entry (or every entry, when the
* patch names no MR, which is the fan-out the backend performs); the
* remaining task-level fields merge at the top level as before.
*/
function applyMRAutomationPatchOptimistically(
previous: TaskMRAutomationOptions,
patch: TaskMRAutomationPatch,
): TaskMRAutomationOptions {
const {
repository_id: repositoryID,
project_path: projectPath,
mr_iid: mrIID,
...fields
} = patch;
const switchKeys = [
"auto_fix_enabled",
"auto_merge_enabled",
"prompt_on_review_requested",
"prompt_on_merged",
"prompt_on_closed",
] as const;
const switchPatch: Partial<TaskMRAutomationOptionsForMR> = {};
const taskLevel: Partial<TaskMRAutomationOptions> = {};
for (const [key, value] of Object.entries(fields)) {
if ((switchKeys as readonly string[]).includes(key)) {
Object.assign(switchPatch, { [key]: value });
} else {
Object.assign(taskLevel, { [key]: value });
}
}
if (Object.keys(switchPatch).length === 0) {
return { ...previous, ...taskLevel };
}
const targetsOneMR =
repositoryID !== undefined && projectPath !== undefined && mrIID !== undefined;
const mrOptions = (previous.mr_options ?? []).map((option) => {
const isTarget =
!targetsOneMR ||
(option.repository_id === repositoryID &&
option.project_path === projectPath &&
option.mr_iid === mrIID);
return isTarget ? { ...option, ...switchPatch } : option;
});
return { ...previous, ...taskLevel, mr_options: mrOptions };
}
type UpdateResponseCommitContext = {
automation: AppState["taskMRAutomation"];
updateRequestRef: Record<string, number>;
taskId: string;
requestId: number;
externalGenAtStart: number;
response: TaskMRAutomationOptions;
};
function shouldCommitUpdateResponse({
automation,
updateRequestRef,
taskId,
requestId,
externalGenAtStart,
response,
}: UpdateResponseCommitContext): boolean {
const currentRevision = automation.byTaskId[taskId]?.automation_revision ?? 0;
const responseRevision = response.automation_revision ?? 0;
const isLatestRequest = updateRequestRef[taskId] === requestId;
const isNewerRevision = responseRevision > currentRevision;
return (
(automation.externalGeneration[taskId] ?? 0) === externalGenAtStart &&
(isLatestRequest || isNewerRevision)
);
}
async function performUpdate(
ctx: MRAutomationRequestContext,
patch: TaskMRAutomationPatch,
@@ -108,21 +220,25 @@ async function performUpdate(
const previous = storeApi.getState().taskMRAutomation.byTaskId[taskId] ?? null;
// Optimistic update: apply immediately, revert on failure (AC27).
if (previous) {
setOptions(taskId, { ...previous, ...patch });
setOptions(taskId, applyMRAutomationPatchOptimistically(previous, patch));
}
setSaving(taskId, true);
setError(taskId, null);
try {
const response = await updateTaskMRAutomation(taskId, patch, { cache: "no-store" });
const automation = storeApi.getState().taskMRAutomation;
// Request-start order is not server revision order. The helper allows an
// older request through only when its server revision is strictly newer;
// the store then applies its monotonic revision guard.
if (
isCurrentAndUnchangedExternally(
shouldCommitUpdateResponse({
automation,
updateRequestRef.current,
updateRequestRef: updateRequestRef.current,
taskId,
requestId,
externalGenAtStart,
)
response,
})
) {
setOptions(taskId, response);
}
@@ -189,11 +305,10 @@ async function performUpdate(
* corrects it again.
*/
export function useTaskMRAutomationOptions(taskId: string | null) {
const refreshRequestRef = useRef<Record<string, number>>({});
const updateRequestRef = useRef<Record<string, number>>({});
const updateSettleCounterRef = useRef<Record<string, number>>({});
const storeApi = useAppStoreApi();
const { refreshRequestRef, updateRequestRef, updateSettleCounterRef } =
requestRefsForStore(storeApi);
const options = useAppStore((state) =>
taskId ? (state.taskMRAutomation.byTaskId[taskId] ?? null) : null,
);
+44 -1
View File
@@ -1,7 +1,50 @@
import type { TaskMR, TaskMRLifecycleState } from "@/lib/types/gitlab";
import type {
TaskMR,
TaskMRAutomationOptionsForMR,
TaskMRLifecycleState,
} from "@/lib/types/gitlab";
const DEFAULT_AUTO_FIX_MAX_ROUNDS = 10;
export const DISABLED_MR_AUTOMATION_SWITCHES: Omit<
TaskMRAutomationOptionsForMR,
"task_id" | "repository_id" | "project_path" | "mr_iid" | "created_at" | "updated_at"
> = {
auto_fix_enabled: false,
auto_merge_enabled: false,
prompt_on_review_requested: false,
prompt_on_merged: false,
prompt_on_closed: false,
};
/**
* Selects the given MR's own automation switches out of the task-scoped
* `mr_options` array, falling back to all-off defaults when the MR has no
* stored row yet (never configured). Mirrors findMRAutomationStateForMR.
*/
export function findMRAutomationOptionsForMR(
options: TaskMRAutomationOptionsForMR[] | undefined,
mr: TaskMR,
): TaskMRAutomationOptionsForMR {
const repositoryID = mr.repository_id ?? "";
const found = options?.find(
(option) =>
option.mr_iid === mr.mr_iid &&
option.project_path === mr.project_path &&
option.repository_id === repositoryID,
);
if (found) return found;
return {
task_id: mr.task_id,
repository_id: repositoryID,
project_path: mr.project_path,
mr_iid: mr.mr_iid,
created_at: "",
updated_at: "",
...DISABLED_MR_AUTOMATION_SWITCHES,
};
}
export type AutoFixRoundInfo = {
current: number;
max: number;
@@ -49,6 +49,7 @@ function makeOptions(overrides: Partial<TaskMRAutomationOptions> = {}): TaskMRAu
review_reviewer_username: "",
updated_at: "2026-01-01T00:00:00Z",
mr_states: [],
mr_options: [],
...overrides,
};
}
@@ -215,6 +215,10 @@ function taskMRAutomationActions(set: ImmerSet) {
options: GitLabSliceState["taskMRAutomation"]["byTaskId"][string],
) =>
set((draft) => {
const existing = draft.taskMRAutomation.byTaskId[taskId];
if (existing && (options.automation_revision ?? 0) < (existing.automation_revision ?? 0)) {
return;
}
draft.taskMRAutomation.byTaskId[taskId] = options;
}),
setTaskMRAutomationLoading: (taskId: string, loading: boolean) =>
+31 -1
View File
@@ -411,9 +411,33 @@ export type TaskMRLifecycleState = {
updated_at: string;
};
/** Task-level MR automation preferences: lifecycle switches (#2125) plus auto-fix CI and auto-merge. */
/**
* The five automation switches for one linked merge request. This is the
* per-MR source of truth; the same-named booleans on TaskMRAutomationOptions
* are an aggregate that only reports "every linked MR has this on".
*/
export type TaskMRAutomationOptionsForMR = {
task_id: string;
repository_id: string;
project_path: string;
mr_iid: number;
auto_fix_enabled: boolean;
auto_merge_enabled: boolean;
prompt_on_review_requested: boolean;
prompt_on_merged: boolean;
prompt_on_closed: boolean;
created_at: string;
updated_at: string;
};
/**
* Task MR automation preferences: the task-level auto-fix prompt override and
* reviewer username, the per-MR switches in `mr_options`, and an aggregate of
* those switches in the top-level booleans.
*/
export type TaskMRAutomationOptions = {
task_id: string;
automation_revision?: number;
auto_fix_enabled: boolean;
auto_merge_enabled: boolean;
auto_fix_prompt_override?: string | null;
@@ -426,10 +450,16 @@ export type TaskMRAutomationOptions = {
review_reviewer_username: string;
updated_at: string;
mr_states: TaskMRLifecycleState[];
mr_options: TaskMRAutomationOptionsForMR[];
};
/** Partial update for task MR automation options. */
export type TaskMRAutomationPatch = {
// Target one linked MR's switches; omit all three to apply them to every MR
// currently linked to the task.
repository_id?: string;
project_path?: string;
mr_iid?: number;
auto_fix_enabled?: boolean;
auto_merge_enabled?: boolean;
auto_fix_prompt_override?: string;
+21 -1
View File
@@ -6,12 +6,13 @@ import { registerGitLabHandlers } from "./gitlab";
const WORKSPACE_A = "workspace-a";
function makeStore(activeWorkspaceId: string | null) {
function makeStore(activeWorkspaceId: string | null, options?: TaskMRAutomationOptions) {
const setTaskMR = vi.fn();
const setTaskMRAutomationOptions = vi.fn();
const markTaskMRAutomationExternalUpdate = vi.fn();
const state = {
workspaces: { activeId: activeWorkspaceId },
taskMRAutomation: { byTaskId: options ? { [options.task_id]: options } : {} },
setTaskMR,
setTaskMRAutomationOptions,
markTaskMRAutomationExternalUpdate,
@@ -40,6 +41,7 @@ function taskMRAutomationOptions(
review_reviewer_username: "",
updated_at: "2026-01-01T00:00:00Z",
mr_states: [],
mr_options: [],
...overrides,
};
}
@@ -93,6 +95,24 @@ describe("GitLab WebSocket handlers", () => {
expect(markTaskMRAutomationExternalUpdate).toHaveBeenCalledWith("task-1");
});
it("drops an older MR automation snapshot pushed after a newer one", () => {
const current = taskMRAutomationOptions({ automation_revision: 2 });
const { store, setTaskMRAutomationOptions, markTaskMRAutomationExternalUpdate } = makeStore(
WORKSPACE_A,
current,
);
const handler = registerGitLabHandlers(store)["gitlab.task_mr_options.updated"]!;
handler({
type: "notification",
action: "gitlab.task_mr_options.updated",
payload: taskMRAutomationOptions({ automation_revision: 1 }),
});
expect(setTaskMRAutomationOptions).not.toHaveBeenCalled();
expect(markTaskMRAutomationExternalUpdate).not.toHaveBeenCalled();
});
it("ignores a task MR automation options update with no task_id", () => {
const { store, setTaskMRAutomationOptions } = makeStore(WORKSPACE_A);
const handler = registerGitLabHandlers(store)["gitlab.task_mr_options.updated"]!;
+4
View File
@@ -14,6 +14,10 @@ export function registerGitLabHandlers(store: StoreApi<AppState>): WsHandlers {
"gitlab.task_mr_options.updated": (message) => {
const options = message.payload as TaskMRAutomationOptions;
if (options.task_id) {
const current = store.getState().taskMRAutomation.byTaskId[options.task_id];
if (current && (options.automation_revision ?? 0) < (current.automation_revision ?? 0)) {
return;
}
store.getState().setTaskMRAutomationOptions(options.task_id, options);
// Marks this write as externally-sourced so a slower in-flight local
// refresh()/update() in useTaskMRAutomationOptions knows not to
+1
View File
@@ -300,6 +300,7 @@
"mrAutomationUpdateFailedDescription": "The setting was not saved.",
"mrAutomationUpdateFailedTitle": "Failed to update MR automation",
"mrAutomation": "Automation",
"mrAutomationAppliesToMR": "Applies to !{{iid}}",
"mrAutoFixCiAndAddressComments": "Auto-fix CI and address comments",
"mrAutoFixPrompt": "Auto-fix prompt",
"mrAutoFixPromptDescription": "This prompt is used only for this task. Leave it blank to use the default prompt. Add <0>{{placeholder}}</0> when you want Kandev to include its MR feedback snapshot.",
+1
View File
@@ -300,6 +300,7 @@
"mrAutomationUpdateFailedDescription": "Ţĥē śēţţĩńĝ ŵàś ńōţ śàvēď.",
"mrAutomationUpdateFailedTitle": "Ƒàĩĺēď ţō ũƥďàţē ḾŔ àũţōḿàţĩōń",
"mrAutomation": "Àũţōḿàţĩōń",
"mrAutomationAppliesToMR": "Àƥƥĺĩēś ţō !{{iid}}",
"mrAutoFixCiAndAddressComments": "Àũţō-ƒĩx ĆĨ àńď àďďŕēśś ćōḿḿēńţś",
"mrAutoFixPrompt": "Àũţō-ƒĩx ƥŕōḿƥţ",
"mrAutoFixPromptDescription": "Ţĥĩś ƥŕōḿƥţ ĩś ũśēď ōńĺŷ ƒōŕ ţĥĩś ţàśķ. Ĺēàvē ĩţ ƀĺàńķ ţō ũśē ţĥē ďēƒàũĺţ ƥŕōḿƥţ. Àďď <0>{{placeholder}}</0> ŵĥēń ŷōũ ŵàńţ Ķàńďēv ţō ĩńćĺũďē ĩţś ḾŔ ƒēēďƀàćķ śńàƥśĥōţ.",
+1
View File
@@ -287,6 +287,7 @@
"workflowThatReceivesNewTasks": "Fluxo de trabalho que recebe as novas tarefas.",
"yourTokenMayStillBeValid": "O seu token pode continuar válido; isto parece um problema de rede ou do serviço.",
"mrAutomationClosedLabel": "MR fechado sem merge",
"mrAutomationAppliesToMR": "Aplica-se a !{{iid}}",
"mrAutomationDescription": "Acorda o agente desta tarefa quando a sua revisão é pedida num merge request associado ou quando um deles é integrado ou fechado. As notificações vão para a fila de sessão que estiver ativa nesta tarefa.",
"mrAutomationMergedLabel": "MR integrado",
"mrAutomationRetry": "Tentar novamente",
+1
View File
@@ -287,6 +287,7 @@
"workflowThatReceivesNewTasks": "接收新任务的工作流。",
"yourTokenMayStillBeValid": "你的令牌可能仍然有效--这看起来是网络或上游服务问题。",
"mrAutomationClosedLabel": "MR 已关闭未合并",
"mrAutomationAppliesToMR": "适用于 !{{iid}}",
"mrAutomationDescription": "当关联的合并请求请求你的审阅,或该合并请求被合并、关闭时,唤醒此任务的智能体。通知会发送到该任务当前处于活动状态的会话队列。",
"mrAutomationMergedLabel": "MR 已合并",
"mrAutomationRetry": "重试",
+1
View File
@@ -287,6 +287,7 @@
"workflowThatReceivesNewTasks": "接收新任務的工作流程。",
"yourTokenMayStillBeValid": "你的權杖可能仍然有效--這看起來是網絡或上游服務問題。",
"mrAutomationClosedLabel": "MR 已關閉未合併",
"mrAutomationAppliesToMR": "適用於 !{{iid}}",
"mrAutomationDescription": "當關聯的合併請求請求你的審閱,或該合併請求被合併、關閉時,喚醒此任務的代理程式。通知會傳送到該任務目前處於活動狀態的工作階段佇列。",
"mrAutomationMergedLabel": "MR 已合併",
"mrAutomationRetry": "重試",
+1
View File
@@ -287,6 +287,7 @@
"workflowThatReceivesNewTasks": "接收新任務的工作流程。",
"yourTokenMayStillBeValid": "你的權杖可能仍然有效--這看起來是網路或上游服務問題。",
"mrAutomationClosedLabel": "MR 已關閉未合併",
"mrAutomationAppliesToMR": "適用於 !{{iid}}",
"mrAutomationDescription": "當關聯的合併請求請求你的審閱,或該合併請求被合併、關閉時,喚醒此任務的代理程式。通知會傳送到該任務目前處於活動狀態的工作階段佇列。",
"mrAutomationMergedLabel": "MR 已合併",
"mrAutomationRetry": "重試",
@@ -75,6 +75,22 @@ dispatch functions turned out byte-for-byte identical below the decision layer.
- **`locked` state.** GitLab's MR state machine has a fourth value with no GitHub
analogue. It is treated as non-terminal (fires neither `merged` nor `closed`) and
is exercised as its own regression case in the decision-function tests.
- **Amended: the five switches are now per linked MR, not per task.** As written,
this ADR put `prompt_on_review_requested` / `prompt_on_merged` / `prompt_on_closed`
(and later auto-fix / auto-merge) on `gitlab_task_mr_options`, keyed by `task_id`
alone. That is wrong for a task with more than one linked MR: enabling a switch on
one MR silently enabled it on all of them. The five switches moved to
`gitlab_task_mr_automation_options`, keyed by the same
`(task_id, repository_id, project_path, mr_iid)` identity as
`gitlab_task_mr_state`, seeded once from the legacy task-wide values by an
`mr_scope_migrated_at`-guarded fan-out. `gitlab_task_mr_options` keeps only what is
genuinely task-level — the auto-fix prompt override and the server-resolved
reviewer username — so a reviewer-identity change still clears every linked MR's
review-request baseline at once, while a switch flip clears only its own MR's
checkpoints. A `PATCH` / `update_task_mr_automation_kandev` call naming an MR
targets it alone; omitting MR identity fans out to every linked MR, which preserves
the behavior of agents that have no MR identity to send. See
`docs/specs/gitlab-integration/spec.md`'s "MR automation" section.
- **Discovered while integrating:** `executeQueuedMessage`'s lifecycle-prompt
detection (`event_handlers_agent.go`) was hardcoded to the GitHub PR automation
origin string. A GitLab-originated durable lifecycle entry was queued but never
+3 -1
View File
@@ -372,7 +372,9 @@ These actions use the connected GitLab user's permissions and do not bypass prot
### Automate a linked merge request
For a task with a linked GitLab merge request, open the MR topbar control. The **Automation** group has the same two task-level controls as GitHub's PRs: **Auto-fix CI and address comments** and **Auto-merge when ready**. Below it, expand **Review follow-up** for three lifecycle booleans: **Your review is requested**, **MR merged**, and **MR closed without merging**. Enabling any control applies it to every MR linked to that task; Kandev tracks delivery and deduplication separately for each linked MR.
For a task with a linked GitLab merge request, open the MR topbar control. The **Automation** group has the same two controls as GitHub's PRs: **Auto-fix CI and address comments** and **Auto-merge when ready**. Below it, expand **Review follow-up** for three lifecycle booleans: **Your review is requested**, **MR merged**, and **MR closed without merging**.
All five belong to a single merge request. A task with several linked MRs shows one **Automation** group per MR, each labelled with its MR number, so you can automate one MR and leave the rest untouched; Kandev tracks delivery and deduplication separately for each. The auto-fix prompt override is the one setting that stays task-level; editing it applies to every linked MR. An agent calling `update_task_mr_automation_kandev` can name a merge request to target it alone, or omit the merge-request fields to apply the change to every MR linked to the task.
Kandev reuses the existing lightweight task MR poller, which checks linked MRs roughly once per minute; it does not add a separate scheduler. Saving enabled options also evaluates the task's current linked MRs without waiting for the next poll.
+1 -1
View File
@@ -274,7 +274,7 @@ The GitLab MR topbar control has an **Automation** group with the same two actio
- **Auto-fix CI and address comments** sends the agent a new or changed failing pipeline job or unresolved discussion note once the pipeline settles, and stops after 10 repair rounds for that MR. Disable and re-enable it to reset the limit.
- **Auto-merge when ready** merges only after the pipeline passes, unresolved discussions are cleared, and GitLab's own merge-readiness check agrees.
Below that, open **Review follow-up** for the same three notification switches GitHub uses, task-level and applying to every merge request linked to the task:
Below that, open **Review follow-up** for the same three notification switches GitHub uses. Every switch above belongs to one merge request: a task with several linked MRs shows an **Automation** group per MR, each labelled with its MR number, and turning a switch on for one leaves the others alone.
- **Your review is requested** wakes the agent when the workspace's connected GitLab account is newly added as a reviewer on the MR. Staying assigned across MR updates does not re-fire it; being removed and re-added (for example, for a re-review after changes) does.
- **MR merged** and **MR closed without merging** independently wake the agent when review work ends.
+47 -23
View File
@@ -108,16 +108,18 @@ workflows are not usable end to end.
switches — `Auto-fix CI and address comments` and `Auto-merge when ready` —
above a collapsible `Review follow-up` group holding the three lifecycle
notification switches (`Your review is requested`, `MR merged`, `MR closed
without merging`) introduced for MR lifecycle notifications. These switches
currently use task-level settings, so changing one linked MR's control also
affects the task's other linked MRs. Per-MR switch scoping, including
per-MR round/attempt state and identity-aware `PATCH`/MCP updates, is tracked
in the "Scope GitLab MR automation switches per MR" follow-up task. The
auto-fix prompt override remains task-level. See "Automation (lifecycle,
auto-fix, auto-merge)" below.
without merging`) introduced for MR lifecycle notifications. All five
switches are scoped per linked MR (see the per-MR amendment in
[the GitLab MR lifecycle ADR](../../decisions/2026-08-01-gitlab-mr-lifecycle-notifications.md)):
enabling a switch on one linked MR does not affect
any other linked MR's switches, and a `PATCH`/MCP update that omits MR
identity fans out to every linked MR, preserving prior agent behavior.
Auto-fix and auto-merge additionally track per-MR round/attempt state. The
auto-fix prompt override remains task-level, as does the resolved review
reviewer username. See "Automation (lifecycle, auto-fix, auto-merge)" below.
- `Auto-fix CI and address comments` sends or queues an agent prompt when a
linked MR's pipeline has a new or changed failing job, or a new or changed
unresolved discussion note, capped at 10 accepted rounds per task.
unresolved discussion note, capped at 10 accepted rounds per linked MR.
`Auto-merge when ready` merges a linked MR only when it is open, not a
draft, its pipeline succeeded, it has zero unresolved discussions, and
GitLab's own merge-readiness verdict agrees.
@@ -130,8 +132,11 @@ workflows are not usable end to end.
Clicking the button opens the MR detail panel directly (no intermediate
dropdown), also mirroring GitHub's single-PR topbar button. A task with
2+ linked MRs, and touch/coarse-pointer surfaces regardless of MR count,
keep the click-only dropdown (per-MR review/open/unlink rows, the
Automation group, and "Link another merge request") with no hover popover.
keep the click-only dropdown (per-MR review/open/unlink rows, one
Automation block per linked MR — each labeled with that MR's number, its
auto-fix/auto-merge rows always visible and its nested `Review follow-up`
group collapsed unless one of that MR's own three lifecycle switches is
already on — and "Link another merge request") with no hover popover.
- The Kanban card shows a merge-request badge (`IconGitMerge`, coloured by
state/pipeline/approval) next to the existing pull-request badge when the
task has at least one linked MR. Multiple linked MRs collapse into one badge
@@ -184,12 +189,23 @@ into the secret store.
notable transitions.
- GitLab notification subscription state is owned by GitLab. Kandev reads it
live and does not duplicate it in SQLite.
- `gitlab_task_mr_options` is a per-task row: `task_id` (PK), the three
lifecycle booleans (`prompt_on_review_requested`, `prompt_on_merged`,
`prompt_on_closed`), `review_reviewer_username`, `auto_fix_enabled`,
`auto_merge_enabled`, `auto_fix_prompt_override` (nullable; empty/`NULL`
means use the built-in `mr-auto-fix` prompt), and timestamps. One row covers
every MR linked to the task.
- `gitlab_task_mr_options` is a per-task row: `task_id` (PK), the genuinely
task-level fields `review_reviewer_username` and `auto_fix_prompt_override`
(nullable; empty/`NULL` means use the built-in `mr-auto-fix` prompt),
`mr_scope_migrated_at` (nullable; guards the one-time fan-out into
`gitlab_task_mr_automation_options` below so a replay never re-enables a
switch a user has since turned off for one MR), and timestamps. Its five
boolean columns (`auto_fix_enabled`, `auto_merge_enabled`,
`prompt_on_review_requested`, `prompt_on_merged`, `prompt_on_closed`) are
legacy: no longer written, read only by that one-time migration.
- `gitlab_task_mr_automation_options` is the per-MR source of truth for the
five automation switches, keyed by `(task_id, repository_id, project_path,
mr_iid)`. A `PATCH`/MCP update naming one linked MR's identity writes only
that row; omitting MR identity fans the patch out to every row currently
linked to the task. The public `GET` response's top-level switch booleans
stay an aggregate ("on for every linked MR, and at least one MR linked")
for MCP/API read compatibility; the `mr_options` array in that response is
the per-MR source of truth the UI renders from.
- `gitlab_task_mr_state` is a per-`(task_id, repository_id, project_path,
mr_iid)` row carrying lifecycle dedupe fields (from MR lifecycle
notifications) plus `last_fix_signature`, `last_fix_checkpoint_json`,
@@ -265,12 +281,16 @@ validated against any supplied value.
### MR automation (lifecycle, auto-fix, auto-merge)
- `GET /tasks/:taskID/mr-automation` returns the task's `TaskMRAutomationOptions`:
the three lifecycle booleans, `review_reviewer_username`, `auto_fix_enabled`,
`auto_merge_enabled`, `auto_fix_prompt_override` (`null` when unset),
`auto_fix_max_rounds` (`10`), `effective_auto_fix_prompt`,
`using_default_prompt`, `updated_at`, and `mr_states` (one
`TaskMRLifecycleState` per linked MR, carrying both the lifecycle dedupe
fields and the auto-fix/auto-merge checkpoint fields).
`automation_revision`, the three lifecycle booleans,
`review_reviewer_username`, `auto_fix_enabled`, `auto_merge_enabled`,
`auto_fix_prompt_override` (`null` when unset), `auto_fix_max_rounds` (`10`),
`effective_auto_fix_prompt`, `using_default_prompt`, `updated_at`,
`mr_options` (one row of the five switches per linked MR), and `mr_states`
(one `TaskMRLifecycleState` per linked MR, carrying both the lifecycle dedupe
fields and the auto-fix/auto-merge checkpoint fields). The top-level switch
booleans are compatibility aggregates: they are true only when at least one
MR is linked and every linked MR has the switch enabled. Clients that need
one MR's exact value must read its `mr_options` row.
- `PATCH /tasks/:taskID/mr-automation` accepts a partial body with any of the
same fields (excluding `auto_fix_max_rounds`, `effective_auto_fix_prompt`,
`using_default_prompt`, `updated_at`, and `mr_states`, which are
@@ -278,7 +298,11 @@ validated against any supplied value.
field`; `auto_fix_enabled`/`auto_merge_enabled`/the three lifecycle booleans
reject an explicit `null` (they are switches, not clearable values);
`auto_fix_prompt_override: null` or `""` restores the built-in `mr-auto-fix`
prompt.
prompt. To target one linked MR, clients pass the complete
`repository_id`, `project_path`, and `mr_iid` selector tuple. If all three
selectors are omitted, the switch patch fans out to every linked MR. A
partial selector tuple or an unlinked MR returns `400` without a write.
The prompt override remains task-level and does not use these selectors.
- Current-task MCP exposes `get_task_mr_automation_kandev` and
`update_task_mr_automation_kandev` with the same shape, scoped to the
connected task.