Merge pull request #731 from corbitsdev/cl-7266-replace-advance_workflow-with-idempotent-step-tagged

Replace workflow advance with step-tagged submit_output

Author: TheGreatAxios

CHANGELOG.md

@@ -13,6 +13,13 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
 
 ## [Unreleased]
 
+### Changed
+
+- Completing a workflow step is a `submit_output` tagged with that step's id.
+  `advance_workflow` is gone. Already-complete and not-current ids are
+  acknowledged without advancing. The unused `autoAdvance` workflow field is
+  removed.
+
 ### Fixed
 
 - Failed sessions with an `error` string in `run.json` are valid resume

docs/ARCHITECTURE.md

@@ -173,8 +173,7 @@ Compaction replaces older turns with a structured, workflow-aware summary rather
 
 - `ask_operator` — Pauses for a clarifying question with a list of options.
 - `present` — Renders structured UI from a JSON view spec instead of pasting tables into chat.
-- `submit_output` — Workflow step advancement when `step` is set (observed by the workflow coordinator).
-- `advance_workflow` — Advances the active workflow to its next step (observed by the director). Only advertised while a workflow is running.
+- `submit_output` — Completes a workflow step when `step` is set. The step id is compared atomically against the current step (`complete()`); already-complete ids (behind the cursor) and not-current ids (future or unknown) are acknowledged without advancing. Always advertised so activating a workflow does not grow the tools array.
 
 Core agent tools (advertised in every chat turn) include `manage_tasks`, `tool_search`, `use_skill`, **`task`** (spawn a sub-agent), and **`search_agents`** when sub-agent profiles are available — see Sub-agents below.
 
@@ -186,7 +185,7 @@ Workflows are named, ordered recipes the agent follows step by step — a thin l
 - `capabilities.ts` — `detectCapabilities` maps the live tool surface to abstract capabilities (`ticket-tracker`, `code-host`, `doc-search`) by name pattern; `resolveStep` decides whether a step runs. A capability override set forces integrations off per run. Adding a capability is a data edit, not a logic change.
 - `runtime.ts` — `WorkflowRuntime` drives execution on a call stack: it skips capability-unsatisfied steps, descends into sub-workflow references, emits step lifecycle events, and snapshots `WorkflowState`. `state.ts` persists that snapshot atomically to `workflow.json` under the session state root for resume.
 
-- `coordinator.ts` — bridges runtime and director: produces the `[WORKFLOW STEP i/total: label]` directive injected into each turn's system prompt, and advances the runtime when `advance_workflow` (or a `submit_output` tagged `{ step }`) completes. Shared by both directors.
+- `coordinator.ts` — bridges runtime and director: produces the `[WORKFLOW STEP i/total: label]` directive injected into each turn's system prompt, and compare-and-advances the runtime when a `submit_output` tagged `{ step }` completes. Already-complete and not-current ids are acknowledged without moving the cursor. Shared by both directors. Fresh and resumed runs share one listener path.
 - The built-in recipes: the atomics `update-ticket`, `improve-docs`, `write-tests`, `triage-bug`, `code-review`, `scope-project`, and the `build-feature` composite that chains them.
 
 Invocation: workflows are **not** top-level slash commands. Recipe definitions load into the `WORKFLOWS` registry from **enabled workflow/command plugins** at startup; command surfaces on those plugins (e.g. a workflow plugin's command prefix such as `/mywf scope`). Slash commands may also be authored as data-only markdown (`commands/*.md`, no `index.ts`); see PLUGINS.md. The model never suggests or auto-starts workflows from ordinary chat. Skills (bundled `corbits-skills`, enabled plugins, or `.agents/skills/`) load on demand via `use_skill` or as `/<skill-name>` slash commands when `user-invocable` is not `false` (see Skills below). The TUI surfaces state via `src/tui/workflow-controller.ts` (lifecycle, capability overrides, resume) — the header shows step progress (`⟳ name · step/total label`).

docs/IMPLEMENTATION.md

@@ -342,7 +342,7 @@ Positional arguments after flags are joined into the optional initial task deliv
 ### Inference
 
 - OpenAI-compatible chat completions, streamed via `@intx/inference`
-- JSON-schema tool definitions for director-layer tools (`ask_operator`, `present`, `submit_output`, `advance_workflow`) and agent tools (`manage_tasks`, `tool_search`, `use_skill`, `search_agents`, …)
+- JSON-schema tool definitions for director-layer tools (`ask_operator`, `present`, `submit_output`) and agent tools (`manage_tasks`, `tool_search`, `use_skill`, `search_agents`, …)
 
 ### State Persistence
 

src/agent/director.ts

@@ -99,9 +99,9 @@ const IDLE_OPEN_TASK_NUDGE =
 
 const WORKFLOW_OPEN_TASK_NUDGE =
   "\n\nYou are ending your turn while tasks are still open (todo/doing) and a " +
-  "workflow step is active. Continue working with tools, call advance_workflow " +
-  "once the step is complete, or mark finished tasks done with manage_tasks. " +
-  "Do not end your turn with tasks still open.";
+  "workflow step is active. Continue working with tools, call submit_output " +
+  "with this step's id once the step is complete, or mark finished tasks done " +
+  "with manage_tasks. Do not end your turn with tasks still open.";
 
 const DECLINED_OPEN_TASK_NUDGE =
   "\n\nThe operator declined the tool call. Do not retry the declined action. " +
@@ -243,27 +243,12 @@ export const presentDefinition: ToolDefinition = {
   },
 };
 
-export const advanceWorkflowDefinition: ToolDefinition = {
-  name: "advance_workflow",
-  description:
-    "Call this when the current workflow step is finished to advance to the next step. " +
-    "Include an optional note summarizing what the step accomplished.",
-  inputSchema: {
-    type: "object",
-    properties: {
-      note: {
-        type: "string",
-        description: "Optional summary of what this step accomplished",
-      },
-    },
-  },
-};
-
 export const submitOutputDefinition: ToolDefinition = {
   name: "submit_output",
   description:
-    "Call this when the task is fully complete (include summary) or to advance " +
-    "a workflow step (include step id).",
+    "Call this when the task is fully complete (include summary) or to complete " +
+    "a workflow step (step id is required to advance; already-complete and " +
+    "not-current step ids are acknowledged without advancing).",
   inputSchema: {
     type: "object",
     properties: {
@@ -274,8 +259,8 @@ export const submitOutputDefinition: ToolDefinition = {
       step: {
         type: "string",
         description:
-          "Workflow step ID to advance. When present this is a " +
-          "step-advancement signal, not a terminal task submission.",
+          "Workflow step ID to complete. Required to advance a workflow. " +
+          "Compared atomically against the current step.",
       },
     },
   },
@@ -480,12 +465,13 @@ class ChatDirectorImpl extends DefaultDirector {
     result: ReactorAction | ReactorAction[],
   ): ReactorAction | ReactorAction[] {
     const active = this.workflowCoordinator?.isActive() === true;
-    // advance_workflow rides on the wire every turn, workflow or not, so
+    // submit_output rides on the wire every turn, workflow or not, so
     // activating a workflow never grows the tools array and busts the cache
-    // prefix. Outside a workflow it is a harmless no-op the director ignores.
-    const tools = this._toolDefinitions.some((t) => t.name === advanceWorkflowDefinition.name)
+    // prefix. Outside a workflow it is a harmless no-op the director ignores
+    // unless the call is a terminal task submission.
+    const tools = this._toolDefinitions.some((t) => t.name === submitOutputDefinition.name)
       ? this._toolDefinitions
-      : [...this._toolDefinitions, advanceWorkflowDefinition];
+      : [...this._toolDefinitions, submitOutputDefinition];
 
     const directive = active ? (this.workflowCoordinator?.directive() ?? null) : null;
 
@@ -694,7 +680,7 @@ class ChatDirectorImpl extends DefaultDirector {
           const path = pathResult instanceof type.errors ? "" : pathResult.path;
           if (isCodeFile(path)) this.lspTriggerCalls.add(block.id);
         }
-        if (block.name === "advance_workflow" || block.name === "submit_output") {
+        if (block.name === "submit_output") {
           this.workflowCalls.set(block.id, { name: block.name, args: block.arguments });
         }
         if (block.name === "ask_operator") {
@@ -791,10 +777,15 @@ class ChatDirectorImpl extends DefaultDirector {
             ),
           ];
         }
+        const stepId = coordinator.currentStepId();
+        const stepClause =
+          stepId !== null
+            ? `call submit_output with { "step": "${stepId}" } now`
+            : "call submit_output with this step's id now";
         const nudge =
-          "\n\nYou have not yet called advance_workflow. " +
-          "If this step is complete, call advance_workflow now. " +
-          "Otherwise continue working with tools.";
+          `\n\nYou have not yet completed this workflow step. ` +
+          `If this step is complete, ${stepClause}. ` +
+          `Otherwise continue working with tools.`;
         const passThrough = actions.filter(
           (a): a is Exclude<ReactorAction, { type: "wait" } | { type: "reply" }> =>
             a.type !== "wait" && a.type !== "reply",
@@ -816,8 +807,9 @@ class ChatDirectorImpl extends DefaultDirector {
             (a): a is Exclude<ReactorAction, { type: "wait" } | { type: "reply" }> =>
               a.type !== "wait" && a.type !== "reply",
           );
-          // Inside a workflow the terminal action is advance_workflow, so point
-          // the nudge at it rather than the general manage_tasks guidance.
+          // Inside a workflow the terminal action is submit_output with the
+          // current step id, so point the nudge at it rather than the general
+          // manage_tasks guidance.
           const nudge =
             coordinator?.isActive() === true ? WORKFLOW_OPEN_TASK_NUDGE : IDLE_OPEN_TASK_NUDGE;
           return [...passThrough, inferWithNudge(capabilities, nudge)];

src/agent/prompts.ts

@@ -219,7 +219,7 @@ const TOOL_SUMMARIES: Record<string, string> = {
   search_agents:
     "find agent profiles by role or team before spawning with task(agent=...); results include full system prompt / body so you need not read_file plugin roots outside the workspace",
   manage_tasks: "maintain your work checklist — create/replace, update status, append, cancel",
-  submit_output: "signal the task is complete — the only way to finish",
+  submit_output: "signal the task is complete, or complete a workflow step by passing its step id",
   ask_operator:
     "pause and ask the user when blocked or genuinely ambiguous; put long rationale in a transcript reply first, then call with a short question and short option labels only",
   present:

src/agent/tools.ts

@@ -4,9 +4,9 @@ import type { ToolDefinition } from "@intx/types/runtime";
 import { type } from "arktype";
 import { createPosixTools, type ToolPlugin } from "@intx/tools-posix";
 import {
-  advanceWorkflowDefinition,
   askOperatorDefinition,
   presentDefinition,
+  submitOutputDefinition,
 } from "../agent/director.js";
 import { manageTasksDefinition } from "./tasks.js";
 import { validateView } from "../tui/view/index.js";
@@ -34,6 +34,7 @@ import { sessionModeEnablesSubAgents } from "../config/session-mode.js";
 import { advertisedToolNamesForSessionMode, type ToolAvailability } from "./tool-search.js";
 import type { ProviderCatalogEntry } from "../config/index.js";
 import type { AgentProfile } from "./profiles.js";
+import type { WorkflowCompleteResult } from "../workflows/types.js";
 import {
   createTaskTool,
   runSubAgent,
@@ -74,8 +75,9 @@ const AskOperatorArgs = type({
   options: "string[]",
 });
 
-const AdvanceWorkflowArgs = type({
-  "note?": "string",
+const SubmitOutputArgs = type({
+  "summary?": "string",
+  "step?": "string",
 });
 
 // The operator can pick one of the offered options, type a free-form answer, or
@@ -131,10 +133,14 @@ export interface AgentToolsetArgs {
   getContextDir?: () => string | undefined;
   // Per-project settings.env, merged into the run_shell tool's spawn environment.
   shellEnv?: Record<string, string>;
-  // Whether a workflow is currently running. advance_workflow rides the wire
+  // Whether a workflow is currently running. submit_output rides the wire
   // every turn (workflow or not), so the model can call it with nothing active;
   // this lets its handler report an honest no-op instead of a false advance.
   isWorkflowActive?: () => boolean;
+  // Compare-and-advance the live workflow. The handler reports this result
+  // instead of reconstructing the cursor; omitted (exec, tests) never claims
+  // an advance.
+  completeWorkflowStep?: (stepId: string) => WorkflowCompleteResult;
   // Primary session mode (always orchestrator; kept for call-site wiring).
   sessionMode?: SessionMode;
   // Session-start facts gating lsp advertisement. Omitted callers (tests,
@@ -459,21 +465,35 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
       },
     }),
     stringTool({
-      definition: advanceWorkflowDefinition,
-      // The director observes this call and advances the workflow runtime; the
-      // handler only needs to acknowledge so the model gets a clean tool result.
-      // Since the tool is always advertised, the model can call it with no
-      // workflow active — report the honest no-op rather than a false advance.
+      definition: submitOutputDefinition,
+      // The director also observes this call on tool.done; complete() is
+      // compare-and-advance so a second pass is a no-op. The handler reports
+      // complete()'s result so parallel submit_output cannot both claim an
+      // advance. Already-complete and not-current ids succeed without
+      // claiming one.
       handler: async (rawArgs: Record<string, unknown>): Promise<string> => {
-        if (args.isWorkflowActive?.() === false) {
-          return "No active workflow — nothing to advance.";
+        const parsed = SubmitOutputArgs(rawArgs);
+        const step = parsed instanceof type.errors ? undefined : parsed.step;
+        const summary = parsed instanceof type.errors ? undefined : parsed.summary;
+        const workflowActive = args.isWorkflowActive?.() === true;
+        if (workflowActive) {
+          if (step === undefined || step.length === 0) {
+            return "Error: workflow completion requires a step identifier.";
+          }
+          const result = args.completeWorkflowStep?.(step) ?? "not-current";
+          if (result === "advanced") {
+            const note = summary !== undefined && summary.length > 0 ? ` (${summary})` : "";
+            return `Workflow step marked complete${note}. Advancing to the next step.`;
+          }
+          if (result === "already-complete") {
+            return "This workflow step is already complete. No advance.";
+          }
+          return "This workflow step is not current. No advance.";
         }
-        const parsed = AdvanceWorkflowArgs(rawArgs);
-        if (parsed instanceof type.errors) {
-          return "Acknowledged.";
+        if (step !== undefined && step.length > 0) {
+          return "No active workflow — nothing to advance.";
         }
-        const note = parsed.note !== undefined ? ` (${parsed.note})` : "";
-        return `Workflow step marked complete${note}. Advancing to the next step.`;
+        return "Acknowledged.";
       },
     }),
   ];