Fix workflow step execution wiring

2026-04-05 22:28:22 -07:00
parent ef3136d601
commit 6ff6e69e60
9 changed files with 414 additions and 11 deletions
--- a/packages/core/src/store.test.ts
+++ b/packages/core/src/store.test.ts
@@ -4762,6 +4762,40 @@ Task with acceptance criteria
      expect(task.enabledWorkflowSteps).toEqual([ws1.id, ws2.id]);
    });

+    it("should materialize built-in workflow templates when creating a task", async () => {
+      const task = await store.createTask({
+        description: "Task with browser verification",
+        enabledWorkflowSteps: ["browser-verification"],
+      });
+
+      expect(task.enabledWorkflowSteps).toEqual(["WS-001"]);
+
+      const step = await store.getWorkflowStep("WS-001");
+      expect(step).toMatchObject({
+        id: "WS-001",
+        templateId: "browser-verification",
+        name: "Browser Verification",
+        toolMode: "coding",
+      });
+    });
+
+    it("should reuse an existing materialized built-in workflow step", async () => {
+      const first = await store.createTask({
+        description: "First browser verification task",
+        enabledWorkflowSteps: ["browser-verification"],
+      });
+      const second = await store.createTask({
+        description: "Second browser verification task",
+        enabledWorkflowSteps: ["browser-verification"],
+      });
+
+      expect(first.enabledWorkflowSteps).toEqual(["WS-001"]);
+      expect(second.enabledWorkflowSteps).toEqual(["WS-001"]);
+
+      const steps = await store.listWorkflowSteps();
+      expect(steps.filter((step) => step.templateId === "browser-verification")).toHaveLength(1);
+    });
+
    it("should not set enabledWorkflowSteps when empty array provided", async () => {
      const task = await store.createTask({
        description: "Task without workflow steps",
@@ -4994,6 +5028,32 @@ Task with acceptance criteria
      expect(task.enabledWorkflowSteps).toEqual(["WS-001", "WS-002"]);
    });

+    it("should update task workflow steps and materialize built-in templates", async () => {
+      const task = await store.createTask({ description: "Editable task" });
+
+      const updated = await store.updateTask(task.id, {
+        enabledWorkflowSteps: ["browser-verification"],
+      });
+
+      expect(updated.enabledWorkflowSteps).toEqual(["WS-001"]);
+
+      const persisted = await store.getTask(task.id);
+      expect(persisted.enabledWorkflowSteps).toEqual(["WS-001"]);
+    });
+
+    it("should resolve built-in workflow templates from getWorkflowStep", async () => {
+      const step = await store.getWorkflowStep("browser-verification");
+
+      expect(step).toMatchObject({
+        id: "browser-verification",
+        templateId: "browser-verification",
+        name: "Browser Verification",
+        mode: "prompt",
+        phase: "pre-merge",
+        toolMode: "coding",
+      });
+    });
+
    // ── Workflow Step Phase ──────────────────────────────────────────────

    it("should default phase to 'pre-merge' when creating a workflow step", async () => {
--- a/packages/core/src/store.ts
+++ b/packages/core/src/store.ts
@@ -4,7 +4,7 @@ import { appendFile, mkdir, readFile, writeFile, rename, unlink } from "node:fs/
 import { join } from "node:path";
 import { existsSync, watch, type FSWatcher } from "node:fs";
 import type { Task, TaskDetail, TaskCreateInput, TaskAttachment, AgentLogEntry, BoardConfig, Column, MergeResult, Settings, GlobalSettings, ProjectSettings, ActivityLogEntry, ActivityEventType } from "./types.js";
-import { VALID_TRANSITIONS, DEFAULT_SETTINGS, GLOBAL_SETTINGS_KEYS } from "./types.js";
+import { VALID_TRANSITIONS, DEFAULT_SETTINGS, GLOBAL_SETTINGS_KEYS, WORKFLOW_STEP_TEMPLATES } from "./types.js";
 import { GlobalSettingsStore } from "./global-settings.js";
 import { Database, toJson, toJsonNullable, fromJson } from "./db.js";
 import { detectLegacyData, migrateFromLegacy } from "./db-migrate.js";
@@ -657,6 +657,83 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
    return join(this.tasksDir, id);
  }

+  private getBuiltInWorkflowTemplate(templateId: string): import("./types.js").WorkflowStepTemplate | undefined {
+    return WORKFLOW_STEP_TEMPLATES.find((template) => template.id === templateId);
+  }
+
+  private toBuiltInWorkflowStep(template: import("./types.js").WorkflowStepTemplate): import("./types.js").WorkflowStep {
+    const now = new Date().toISOString();
+    return {
+      id: template.id,
+      templateId: template.id,
+      name: template.name,
+      description: template.description,
+      mode: "prompt",
+      phase: "pre-merge",
+      prompt: template.prompt,
+      toolMode: template.toolMode || "readonly",
+      enabled: true,
+      createdAt: now,
+      updatedAt: now,
+    };
+  }
+
+  private async ensureWorkflowStepForTemplate(templateId: string): Promise<import("./types.js").WorkflowStep> {
+    return this.withConfigLock(async () => {
+      const template = this.getBuiltInWorkflowTemplate(templateId);
+      if (!template) {
+        throw new Error(`Workflow step template '${templateId}' not found`);
+      }
+
+      const config = await this.readConfig();
+      const existing = (config.workflowSteps || []).find((ws) =>
+        ws.id === templateId
+        || ws.templateId === templateId
+        || ws.name.toLowerCase() === template.name.toLowerCase(),
+      );
+      if (existing) {
+        return existing;
+      }
+
+      const nextWsId = config.nextWorkflowStepId || 1;
+      const step = this.toBuiltInWorkflowStep(template);
+      step.id = `WS-${String(nextWsId).padStart(3, "0")}`;
+
+      if (!config.workflowSteps) {
+        config.workflowSteps = [];
+      }
+      config.workflowSteps.push(step);
+      config.nextWorkflowStepId = nextWsId + 1;
+      await this.writeConfig(config);
+
+      return step;
+    });
+  }
+
+  private async resolveEnabledWorkflowSteps(stepIds?: string[]): Promise<string[] | undefined> {
+    if (!stepIds?.length) return undefined;
+
+    const resolved: string[] = [];
+    const seen = new Set<string>();
+
+    for (const rawId of stepIds) {
+      const stepId = rawId.trim();
+      if (!stepId) continue;
+
+      const template = this.getBuiltInWorkflowTemplate(stepId);
+      const resolvedId = template
+        ? (await this.ensureWorkflowStepForTemplate(stepId)).id
+        : stepId;
+
+      if (!seen.has(resolvedId)) {
+        seen.add(resolvedId);
+        resolved.push(resolvedId);
+      }
+    }
+
+    return resolved.length > 0 ? resolved : undefined;
+  }
+
  async createTask(
    input: TaskCreateInput,
    options?: {
@@ -695,7 +772,9 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
    }

    // Determine enabledWorkflowSteps: explicit input takes precedence, otherwise auto-apply default-on steps
-    let resolvedWorkflowSteps: string[] | undefined = input.enabledWorkflowSteps?.length ? input.enabledWorkflowSteps : undefined;
+    let resolvedWorkflowSteps: string[] | undefined = input.enabledWorkflowSteps?.length
+      ? await this.resolveEnabledWorkflowSteps(input.enabledWorkflowSteps)
+      : undefined;

    // When enabledWorkflowSteps is not provided at all (undefined), auto-apply default-on workflow steps
    if (input.enabledWorkflowSteps === undefined) {
@@ -1016,7 +1095,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {

  async updateTask(
    id: string,
-    updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; blockedBy?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; stuckKillCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
+    updates: { title?: string; description?: string; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; blockedBy?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; stuckKillCount?: number | null; recoveryRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
  ): Promise<Task> {
    return this.withTaskLock(id, async () => {
      // Validate that task doesn't depend on itself
@@ -1101,6 +1180,9 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
      } else if (updates.nextRecoveryAt !== undefined) {
        task.nextRecoveryAt = updates.nextRecoveryAt;
      }
+      if (updates.enabledWorkflowSteps !== undefined) {
+        task.enabledWorkflowSteps = await this.resolveEnabledWorkflowSteps(updates.enabledWorkflowSteps);
+      }
      if (updates.modelProvider === null) {
        task.modelProvider = undefined;
      } else if (updates.modelProvider !== undefined) {
@@ -2680,11 +2762,13 @@ ${stepsSection}`;
      const now = new Date().toISOString();
      const step: import("./types.js").WorkflowStep = {
        id,
+        templateId: input.templateId,
        name: input.name,
        description: input.description,
        mode,
        phase: input.phase || "pre-merge",
        prompt: mode === "prompt" ? (input.prompt || "") : "",
+        toolMode: mode === "prompt" ? (input.toolMode || "readonly") : undefined,
        scriptName: mode === "script" ? input.scriptName : undefined,
        enabled: input.enabled !== undefined ? input.enabled : true,
        defaultOn: input.defaultOn === true ? true : undefined,
@@ -2718,7 +2802,11 @@ ${stepsSection}`;
   */
  async getWorkflowStep(id: string): Promise<import("./types.js").WorkflowStep | undefined> {
    const config = await this.readConfig();
-    return (config.workflowSteps || []).find((ws) => ws.id === id);
+    const stored = (config.workflowSteps || []).find((ws) => ws.id === id);
+    if (stored) return stored;
+
+    const template = this.getBuiltInWorkflowTemplate(id);
+    return template ? this.toBuiltInWorkflowStep(template) : undefined;
  }

  /**
@@ -2748,12 +2836,14 @@ ${stepsSection}`;
        // When switching to script mode, clear prompt and model overrides
        if (newMode === "script") {
          step.prompt = "";
+          step.toolMode = undefined;
          step.modelProvider = undefined;
          step.modelId = undefined;
        }
        // When switching to prompt mode, clear scriptName
        if (newMode === "prompt") {
          step.scriptName = undefined;
+          step.toolMode = step.toolMode || "readonly";
        }
      }

@@ -2761,6 +2851,7 @@ ${stepsSection}`;
      if (updates.description !== undefined) step.description = updates.description;
      if (updates.phase !== undefined) step.phase = updates.phase;
      if (updates.prompt !== undefined && step.mode === "prompt") step.prompt = updates.prompt;
+      if (updates.toolMode !== undefined && step.mode === "prompt") step.toolMode = updates.toolMode;
      if (updates.scriptName !== undefined && step.mode === "script") step.scriptName = updates.scriptName;
      if (updates.enabled !== undefined) step.enabled = updates.enabled;
      if (updates.defaultOn !== undefined) step.defaultOn = updates.defaultOn;
--- a/packages/core/src/types.ts
+++ b/packages/core/src/types.ts
@@ -59,6 +59,7 @@ export interface ModelPreset {
 /** A reusable workflow step definition that can run after task implementation. */
 /** Execution mode for a workflow step. */
 export type WorkflowStepMode = "prompt" | "script";
+export type WorkflowStepToolMode = "readonly" | "coding";

 /** Lifecycle phase for workflow step execution. */
 export type WorkflowStepPhase = "pre-merge" | "post-merge";
@@ -66,6 +67,8 @@ export type WorkflowStepPhase = "pre-merge" | "post-merge";
 export interface WorkflowStep {
  /** Unique identifier (e.g., "WS-001") */
  id: string;
+  /** Built-in template source ID when this step was materialized from a template. */
+  templateId?: string;
  /** Display name (e.g., "Documentation Review") */
  name: string;
  /** Short description for UI display */
@@ -76,6 +79,8 @@ export interface WorkflowStep {
  phase?: WorkflowStepPhase;
  /** Full agent prompt to execute when this step runs (used when mode is "prompt") */
  prompt: string;
+  /** Tool set available to prompt-mode workflow agents. Defaults to readonly. */
+  toolMode?: WorkflowStepToolMode;
  /** Name of a script from project settings `scripts` map to execute (required when mode is "script") */
  scriptName?: string;
  /** Whether this step is available for selection on new tasks */
@@ -102,6 +107,8 @@ export interface WorkflowStep {
 export type NtfyNotificationEvent = "in-review" | "merged" | "failed";

 export interface WorkflowStepInput {
+  /** Built-in template source ID when creating a concrete step from a template. */
+  templateId?: string;
  name: string;
  description: string;
  /** Execution mode — defaults to "prompt" if not specified */
@@ -110,6 +117,8 @@ export interface WorkflowStepInput {
  phase?: WorkflowStepPhase;
  /** Agent prompt (used when mode is "prompt"). Optional — can be AI-generated later via refinement. */
  prompt?: string;
+  /** Tool set available to prompt-mode workflow agents. Defaults to readonly. */
+  toolMode?: WorkflowStepToolMode;
  /** Script name from project settings (required when mode is "script").
   *  Must reference a named script in `settings.scripts` — no raw commands. */
  scriptName?: string;
@@ -152,6 +161,8 @@ export interface WorkflowStepTemplate {
  description: string;
  /** Full agent prompt template */
  prompt: string;
+  /** Tool set available when the template runs as a prompt-mode step. */
+  toolMode?: WorkflowStepToolMode;
  /** Grouping category (e.g., "Quality", "Security") */
  category: string;
  /** Optional icon identifier for UI (e.g., "file-text", "shield") */
@@ -166,6 +177,7 @@ export const WORKFLOW_STEP_TEMPLATES: WorkflowStepTemplate[] = [
    description: "Verify all public APIs, functions, and complex logic have appropriate documentation",
    category: "Quality",
    icon: "file-text",
+    toolMode: "readonly",
    prompt: `You are a documentation reviewer. Review the completed task and verify documentation quality.

 Review Criteria:
@@ -191,6 +203,7 @@ Output Requirements:
    description: "Run tests and verify they pass, check for obvious bugs",
    category: "Quality",
    icon: "check-circle",
+    toolMode: "coding",
    prompt: `You are a QA tester. Verify the task implementation by running tests and checking for bugs.

 Test Execution:
@@ -215,6 +228,7 @@ Output Requirements:
    description: "Check for common security vulnerabilities and anti-patterns",
    category: "Security",
    icon: "shield",
+    toolMode: "readonly",
    prompt: `You are a security auditor. Review the task changes for common security vulnerabilities.

 Security Checklist:
@@ -242,6 +256,7 @@ Output Requirements:
    description: "Check for performance anti-patterns and optimization opportunities",
    category: "Quality",
    icon: "zap",
+    toolMode: "readonly",
    prompt: `You are a performance reviewer. Analyze the task changes for performance implications.

 Performance Checklist:
@@ -268,6 +283,7 @@ Output Requirements:
    description: "Verify UI changes meet accessibility standards (WCAG 2.1)",
    category: "Quality",
    icon: "eye",
+    toolMode: "readonly",
    prompt: `You are an accessibility reviewer. Check UI changes for WCAG 2.1 compliance.

 Accessibility Checklist:
@@ -294,6 +310,7 @@ Output Requirements:
    description: "Verify web application functionality using browser automation",
    category: "Quality",
    icon: "globe",
+    toolMode: "coding",
    prompt: `You are a browser verification specialist. Verify web application functionality after task implementation using the agent-browser CLI tool.

 ## Prerequisites