diff --git a/apps/backend/config/workflows/loader_test.go b/apps/backend/config/workflows/loader_test.go index 308eefdb4b..e1de16e83f 100644 --- a/apps/backend/config/workflows/loader_test.go +++ b/apps/backend/config/workflows/loader_test.go @@ -192,8 +192,8 @@ func TestLoadTemplates_ExpectedTemplateIDs(t *testing.T) { } // TestLoadTemplates_HiddenFlag verifies that the YAML loader propagates -// the `hidden` field into WorkflowTemplate.Hidden. Only improve-kandev -// is expected to be hidden; all user-facing templates must be visible. +// the `hidden` field into WorkflowTemplate.Hidden. System-only templates +// must be hidden while user-pickable templates remain visible. func TestLoadTemplates_HiddenFlag(t *testing.T) { templates, err := LoadTemplates() if err != nil { @@ -205,8 +205,10 @@ func TestLoadTemplates_HiddenFlag(t *testing.T) { hiddenByID[tmpl.ID] = tmpl.Hidden } - if !hiddenByID["improve-kandev"] { - t.Errorf("expected template %q to be hidden", "improve-kandev") + for _, id := range []string{"improve-kandev", "office-default", "routine"} { + if !hiddenByID[id] { + t.Errorf("expected template %q to be hidden", id) + } } for _, id := range []string{"simple", "standard", "architecture", "pr-review", "feature-dev"} { if hiddenByID[id] { diff --git a/apps/backend/config/workflows/office-default.yml b/apps/backend/config/workflows/office-default.yml index beb99cf995..5de847d21a 100644 --- a/apps/backend/config/workflows/office-default.yml +++ b/apps/backend/config/workflows/office-default.yml @@ -9,6 +9,7 @@ description: >- Five-step office workflow with a primary agent doing the work, multi-agent reviewers, and multi-agent approvers. Tasks flow Backlog -> Work -> Review -> Approval -> Done with quorum-gated transitions. +hidden: true steps: - id: backlog diff --git a/apps/backend/config/workflows/routine.yml b/apps/backend/config/workflows/routine.yml index 8dd9e9e2c6..6a2a08bf8f 100644 --- a/apps/backend/config/workflows/routine.yml +++ b/apps/backend/config/workflows/routine.yml @@ -6,6 +6,7 @@ id: routine name: Routine description: System workflow for routine-fired tasks. Single auto-completing step. +hidden: true steps: - id: in_progress name: In Progress diff --git a/apps/backend/internal/task/repository/sqlite/base_schema.go b/apps/backend/internal/task/repository/sqlite/base_schema.go index 8a216fb708..5111d4e2c2 100644 --- a/apps/backend/internal/task/repository/sqlite/base_schema.go +++ b/apps/backend/internal/task/repository/sqlite/base_schema.go @@ -27,6 +27,7 @@ func (r *Repository) initSchema() error { r.ensureDefaultWorkspace, r.ensureDefaultExecutorsAndEnvironments, r.runMigrations, + r.hideBuiltinWorkflows, r.backfillTaskEnvironments, r.backfillTaskEnvironmentRepos, r.healTaskEnvironmentWorkspacePaths, diff --git a/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go b/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go index 922644f0e8..9af9ce8811 100644 --- a/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go +++ b/apps/backend/internal/task/repository/sqlite/builtin_workflow_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "path/filepath" "testing" + "time" "github.com/jmoiron/sqlx" @@ -73,6 +74,21 @@ func loadStepsForWorkflow(t *testing.T, repo *Repository, workflowID string) []* return out } +func assertBuiltinWorkflowHidden(t *testing.T, repo *Repository, workflowID string) { + t.Helper() + var hidden int + if err := repo.db.QueryRowContext( + context.Background(), + repo.db.Rebind(`SELECT hidden FROM workflows WHERE id = ?`), + workflowID, + ).Scan(&hidden); err != nil { + t.Fatalf("query workflow visibility: %v", err) + } + if hidden != 1 { + t.Errorf("workflow %q hidden = %d, want 1", workflowID, hidden) + } +} + func TestEnsureOfficeDefaultWorkflow_CreatesFiveSteps(t *testing.T) { repo := newRepoForBuiltinWorkflowTests(t) ctx := context.Background() @@ -84,6 +100,7 @@ func TestEnsureOfficeDefaultWorkflow_CreatesFiveSteps(t *testing.T) { if id == "" { t.Fatalf("expected non-empty workflow id") } + assertBuiltinWorkflowHidden(t, repo, id) steps := loadStepsForWorkflow(t, repo, id) if len(steps) != 5 { @@ -201,6 +218,7 @@ func TestEnsureOfficeDefaultWorkflow_Idempotent(t *testing.T) { if first != second { t.Errorf("expected idempotent id, got %q vs %q", first, second) } + assertBuiltinWorkflowHidden(t, repo, first) steps := loadStepsForWorkflow(t, repo, first) if len(steps) != 5 { t.Errorf("expected 5 steps after idempotent ensure, got %d", len(steps)) @@ -226,6 +244,7 @@ func TestEnsureRoutineWorkflow_AutoCompletingShape(t *testing.T) { if first != second { t.Errorf("expected idempotent id, got %q vs %q", first, second) } + assertBuiltinWorkflowHidden(t, repo, first) steps := loadStepsForWorkflow(t, repo, first) if len(steps) != 2 { @@ -246,6 +265,102 @@ func TestEnsureRoutineWorkflow_AutoCompletingShape(t *testing.T) { } } +func TestEnsureRoutineWorkflow_HealsExistingWorkflowVisibility(t *testing.T) { + repo := newRepoForBuiltinWorkflowTests(t) + ctx := context.Background() + + _, err := repo.db.ExecContext(ctx, ` + INSERT INTO workflows ( + id, workspace_id, name, workflow_template_id, is_system, hidden, created_at, updated_at + ) VALUES ( + 'stale-routine', 'ws-1', 'Routine', 'routine', 1, 0, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + ) + `) + if err != nil { + t.Fatalf("insert stale routine workflow: %v", err) + } + + id, err := repo.EnsureRoutineWorkflow(ctx, "ws-1") + if err != nil { + t.Fatalf("ensure routine: %v", err) + } + if id != "stale-routine" { + t.Fatalf("EnsureRoutineWorkflow() id = %q, want stale-routine", id) + } + assertBuiltinWorkflowHidden(t, repo, id) +} + +func TestEnsureRoutineWorkflow_KeepsUserWorkflowVisible(t *testing.T) { + repo := newRepoForBuiltinWorkflowTests(t) + ctx := context.Background() + + _, err := repo.db.ExecContext(ctx, ` + INSERT INTO workflows ( + id, workspace_id, name, workflow_template_id, is_system, hidden, created_at, updated_at + ) VALUES ( + 'user-routine', 'ws-1', 'My Routine', 'routine', 0, 0, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + ) + `) + if err != nil { + t.Fatalf("insert user routine workflow: %v", err) + } + + id, err := repo.EnsureRoutineWorkflow(ctx, "ws-1") + if err != nil { + t.Fatalf("ensure routine: %v", err) + } + if id == "user-routine" { + t.Fatal("EnsureRoutineWorkflow() reused a user workflow") + } + + var hidden int + if err := repo.db.QueryRowContext(ctx, `SELECT hidden FROM workflows WHERE id = 'user-routine'`).Scan(&hidden); err != nil { + t.Fatalf("query user routine visibility: %v", err) + } + if hidden != 0 { + t.Errorf("user routine hidden = %d, want 0", hidden) + } +} + +func TestRepositoryInitialization_HealsBuiltinWorkflowVisibility(t *testing.T) { + repo := newRepoForBuiltinWorkflowTests(t) + ctx := context.Background() + legacyTime := time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC) + + for _, workflow := range []struct { + id string + templateID string + }{ + {id: "stale-office", templateID: "office-default"}, + {id: "stale-routine", templateID: "routine"}, + } { + _, err := repo.db.ExecContext(ctx, repo.db.Rebind(` + INSERT INTO workflows ( + id, workspace_id, name, workflow_template_id, is_system, hidden, created_at, updated_at + ) VALUES (?, 'ws-1', 'System workflow', ?, 1, 0, ?, ?) + `), workflow.id, workflow.templateID, legacyTime, legacyTime) + if err != nil { + t.Fatalf("insert stale %s workflow: %v", workflow.templateID, err) + } + } + + if err := repo.initSchema(); err != nil { + t.Fatalf("reinitialize repository: %v", err) + } + assertBuiltinWorkflowHidden(t, repo, "stale-office") + assertBuiltinWorkflowHidden(t, repo, "stale-routine") + + for _, workflowID := range []string{"stale-office", "stale-routine"} { + var updatedAt time.Time + if err := repo.db.QueryRowContext(ctx, `SELECT updated_at FROM workflows WHERE id = ?`, workflowID).Scan(&updatedAt); err != nil { + t.Fatalf("query workflow updated_at: %v", err) + } + if !updatedAt.After(legacyTime) { + t.Errorf("workflow %q updated_at = %v, want after %v", workflowID, updatedAt, legacyTime) + } + } +} + // TestRoutineWorkflowTasks_FlaggedSystem verifies tasks created in the // routine workflow are excluded from ListTasksByWorkspace by default // (template_id="routine" lives in SystemWorkflowTemplateIDs). diff --git a/apps/backend/internal/task/repository/sqlite/defaults.go b/apps/backend/internal/task/repository/sqlite/defaults.go index 35996b0af6..da49832a54 100644 --- a/apps/backend/internal/task/repository/sqlite/defaults.go +++ b/apps/backend/internal/task/repository/sqlite/defaults.go @@ -8,6 +8,7 @@ import ( "time" "github.com/google/uuid" + "github.com/jmoiron/sqlx" workflowcfg "github.com/kandev/kandev/config/workflows" "github.com/kandev/kandev/internal/common/config" @@ -33,6 +34,36 @@ var SystemWorkflowTemplateIDs = []string{ templateIDRoutine, } +// hideBuiltinWorkflows reconciles system workflow rows created before their +// embedded templates were marked hidden. User-created workflows are left +// untouched even if they reference the same template. +func (r *Repository) hideBuiltinWorkflows() error { + hidden, err := workflowcfg.HiddenTemplateIDs() + if err != nil { + return fmt.Errorf("load hidden workflow templates: %w", err) + } + templateIDs := make([]string, 0, len(hidden)) + for templateID, isHidden := range hidden { + if isHidden { + templateIDs = append(templateIDs, templateID) + } + } + if len(templateIDs) == 0 { + return nil + } + query, args, err := sqlx.In(` + UPDATE workflows SET hidden = 1, updated_at = ? + WHERE is_system = 1 AND workflow_template_id IN (?) AND hidden != 1 + `, time.Now().UTC(), templateIDs) + if err != nil { + return fmt.Errorf("build hidden workflow reconciliation: %w", err) + } + if _, err := r.db.Exec(r.db.Rebind(query), args...); err != nil { + return fmt.Errorf("hide builtin workflows: %w", err) + } + return nil +} + // ensureDefaultWorkspace creates a default workspace if none exists func (r *Repository) ensureDefaultWorkspace() error { ctx := context.Background() @@ -314,27 +345,34 @@ func (r *Repository) ensureBuiltinWorkflow(ctx context.Context, workspaceID, tem if workspaceID == "" { return "", fmt.Errorf("workspace_id is required") } - existing, err := r.findWorkflowByTemplate(ctx, workspaceID, templateID) - if err != nil { - return "", err - } - if existing != "" { - return existing, nil - } tmpl, err := loadBuiltinTemplate(templateID) if err != nil { return "", err } + existing, err := r.findBuiltinWorkflowByTemplate(ctx, workspaceID, templateID) + if err != nil { + return "", err + } + if existing != "" { + hidden := dialect.BoolToInt(tmpl.Hidden) + if _, err := r.db.ExecContext(ctx, r.db.Rebind(` + UPDATE workflows SET hidden = ?, updated_at = ? + WHERE id = ? AND is_system = 1 AND hidden != ? + `), hidden, time.Now().UTC(), existing, hidden); err != nil { + return "", fmt.Errorf("update builtin workflow visibility: %w", err) + } + return existing, nil + } return r.createWorkflowFromTemplate(ctx, workspaceID, name, description, tmpl) } -// findWorkflowByTemplate returns the workflow id in workspaceID with the -// given workflow_template_id, or empty string when none exists. -func (r *Repository) findWorkflowByTemplate(ctx context.Context, workspaceID, templateID string) (string, error) { +// findBuiltinWorkflowByTemplate returns the system workflow id in workspaceID +// with the given workflow_template_id, or empty string when none exists. +func (r *Repository) findBuiltinWorkflowByTemplate(ctx context.Context, workspaceID, templateID string) (string, error) { var id string err := r.db.QueryRowContext(ctx, r.db.Rebind(` SELECT id FROM workflows - WHERE workspace_id = ? AND workflow_template_id = ? + WHERE workspace_id = ? AND workflow_template_id = ? AND is_system = 1 LIMIT 1 `), workspaceID, templateID).Scan(&id) if err == sql.ErrNoRows { @@ -370,9 +408,9 @@ func (r *Repository) createWorkflowFromTemplate( workflowID := uuid.New().String() if _, err := r.db.ExecContext(ctx, r.db.Rebind(` - INSERT INTO workflows (id, workspace_id, name, description, workflow_template_id, is_system, sort_order, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, 1, 999, ?, ?) - `), workflowID, workspaceID, name, description, tmpl.ID, now, now); err != nil { + INSERT INTO workflows (id, workspace_id, name, description, workflow_template_id, is_system, sort_order, hidden, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, 1, 999, ?, ?, ?) + `), workflowID, workspaceID, name, description, tmpl.ID, dialect.BoolToInt(tmpl.Hidden), now, now); err != nil { return "", fmt.Errorf("insert builtin workflow %s: %w", tmpl.ID, err) } diff --git a/apps/backend/internal/workflow/service/service_test.go b/apps/backend/internal/workflow/service/service_test.go index 7b81d8c52a..195b2b933e 100644 --- a/apps/backend/internal/workflow/service/service_test.go +++ b/apps/backend/internal/workflow/service/service_test.go @@ -114,8 +114,8 @@ func createStep(t *testing.T, svc *Service, step *models.WorkflowStep) { } // TestListTemplates_FiltersHidden verifies that templates marked -// `hidden: true` in their embedded YAML (improve-kandev) are excluded -// from the picker shown by the create-workflow dialog and the settings UI. +// `hidden: true` in their embedded YAML are excluded from the picker shown by +// the create-workflow dialog and the settings UI. func TestListTemplates_FiltersHidden(t *testing.T) { svc, _ := setupTestService(t) @@ -123,8 +123,9 @@ func TestListTemplates_FiltersHidden(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, templates, "ListTemplates returned no templates; the assertion below would vacuously pass") + hiddenIDs := []string{"improve-kandev", "office-default", "routine"} for _, tmpl := range templates { - assert.NotEqual(t, "improve-kandev", tmpl.ID, "hidden template must not be returned by ListTemplates") + assert.NotContains(t, hiddenIDs, tmpl.ID, "hidden template must not be returned by ListTemplates") } } diff --git a/apps/web/e2e/tests/workflow/workflow-settings.spec.ts b/apps/web/e2e/tests/workflow/workflow-settings.spec.ts index 56f0bcfacb..71e3accb05 100644 --- a/apps/web/e2e/tests/workflow/workflow-settings.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-settings.spec.ts @@ -2,6 +2,19 @@ import { test, expect } from "../../fixtures/test-base"; import { WorkflowSettingsPage } from "../../pages/workflow-settings-page"; test.describe("Workflow settings", () => { + test("hides system-only templates from the add workflow dialog", async ({ + testPage, + seedData, + }) => { + const page = new WorkflowSettingsPage(testPage); + await page.goto(seedData.workspaceId); + + await page.addWorkflowButton.click(); + await expect(page.createDialog).toBeVisible(); + await expect(page.createDialog.getByText("Office Default", { exact: true })).toHaveCount(0); + await expect(page.createDialog.getByText("Routine", { exact: true })).toHaveCount(0); + }); + test("displays existing workflows on the settings page", async ({ testPage, seedData }) => { const page = new WorkflowSettingsPage(testPage); await page.goto(seedData.workspaceId);