fix(workflows): hide system templates (#1765)
This commit is contained in:
@@ -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] {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -27,6 +27,7 @@ func (r *Repository) initSchema() error {
|
||||
r.ensureDefaultWorkspace,
|
||||
r.ensureDefaultExecutorsAndEnvironments,
|
||||
r.runMigrations,
|
||||
r.hideBuiltinWorkflows,
|
||||
r.backfillTaskEnvironments,
|
||||
r.backfillTaskEnvironmentRepos,
|
||||
r.healTaskEnvironmentWorkspacePaths,
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user