Skip to content

Commit 2592ac5

Browse files
committed
Report submit_output from complete() and drop autoAdvance
The handler reconstructed the cursor before the director mutated on tool.done, so parallel submit_output could both claim an advance. complete() now returns already-complete vs not-current and the handler reports that result. autoAdvance is removed from the plugin contract because the coordinator no longer reads it.
1 parent e938653 commit 2592ac5

14 files changed

Lines changed: 117 additions & 68 deletions

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1313

1414
## [Unreleased]
1515

16+
### Changed
17+
18+
- Completing a workflow step is a `submit_output` tagged with that step's id.
19+
`advance_workflow` is gone. Already-complete and not-current ids are
20+
acknowledged without advancing. The unused `autoAdvance` workflow field is
21+
removed.
22+
1623
## [0.3.10] - 2026-08-30
1724

1825
### Fixed

docs/ARCHITECTURE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ Compaction replaces older turns with a structured, workflow-aware summary rather
173173

174174
- `ask_operator` — Pauses for a clarifying question with a list of options (and optional shell pre-approval via `command`).
175175
- `present` — Renders structured UI from a JSON view spec instead of pasting tables into chat.
176-
- `submit_output` — Completes a workflow step when `step` is set (observed by the workflow coordinator). The step id is compared atomically against the current step; duplicate or stale ids are acknowledged without advancing. Always advertised so activating a workflow does not grow the tools array.
176+
- `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.
177177

178178
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.
179179

@@ -185,7 +185,7 @@ Workflows are named, ordered recipes the agent follows step by step — a thin l
185185
- `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.
186186
- `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.
187187

188-
- `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. Shared by both directors. Fresh and resumed runs share one listener path.
188+
- `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.
189189
- 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.
190190

191191
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`).

src/agent/director.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,8 @@ export const submitOutputDefinition: ToolDefinition = {
256256
name: "submit_output",
257257
description:
258258
"Call this when the task is fully complete (include summary) or to complete " +
259-
"a workflow step (step id is required to advance; duplicate or stale step " +
260-
"ids are acknowledged without advancing).",
259+
"a workflow step (step id is required to advance; already-complete and " +
260+
"not-current step ids are acknowledged without advancing).",
261261
inputSchema: {
262262
type: "object",
263263
properties: {

src/agent/tools.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { sessionModeEnablesSubAgents } from "../config/session-mode.js";
3434
import { advertisedToolNamesForSessionMode, type ToolAvailability } from "./tool-search.js";
3535
import type { ProviderCatalogEntry } from "../config/index.js";
3636
import type { AgentProfile } from "./profiles.js";
37+
import type { WorkflowCompleteResult } from "../workflows/types.js";
3738
import {
3839
createTaskTool,
3940
runSubAgent,
@@ -137,12 +138,10 @@ export interface AgentToolsetArgs {
137138
// every turn (workflow or not), so the model can call it with nothing active;
138139
// this lets its handler report an honest no-op instead of a false advance.
139140
isWorkflowActive?: () => boolean;
140-
// Current workflow step id, read live so the handler can distinguish a
141-
// matching complete from a duplicate, stale, or future id without advancing.
142-
getCurrentWorkflowStepId?: () => string | null;
143-
// True when `stepId` is behind the cursor in the active workflow frame.
144-
// Omitted (tests, exec) treats a non-current id as unknown/future, not stale.
145-
isPastWorkflowStep?: (stepId: string) => boolean;
141+
// Compare-and-advance the live workflow. The handler reports this result
142+
// instead of reconstructing the cursor; omitted (exec, tests) never claims
143+
// an advance.
144+
completeWorkflowStep?: (stepId: string) => WorkflowCompleteResult;
146145
// Primary session mode (always orchestrator; kept for call-site wiring).
147146
sessionMode?: SessionMode;
148147
// Session-start facts gating lsp advertisement. Omitted callers (tests,
@@ -473,11 +472,11 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
473472
}),
474473
stringTool({
475474
definition: submitOutputDefinition,
476-
// The director observes this call and compare-and-advances the workflow
477-
// runtime; the handler only acknowledges so the model gets a clean tool
478-
// result. Duplicate, stale, and future step ids succeed without claiming
479-
// an advance. Copy distinguishes those cases so a future id is not
480-
// reported as already complete.
475+
// The director also observes this call on tool.done; complete() is
476+
// compare-and-advance so a second pass is a no-op. The handler reports
477+
// complete()'s result so parallel submit_output cannot both claim an
478+
// advance. Already-complete and not-current ids succeed without
479+
// claiming one.
481480
handler: async (rawArgs: Record<string, unknown>): Promise<string> => {
482481
const parsed = SubmitOutputArgs(rawArgs);
483482
const step = parsed instanceof type.errors ? undefined : parsed.step;
@@ -487,12 +486,12 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
487486
if (step === undefined || step.length === 0) {
488487
return "Error: workflow completion requires a step identifier.";
489488
}
490-
const current = args.getCurrentWorkflowStepId?.() ?? null;
491-
if (current === step) {
489+
const result = args.completeWorkflowStep?.(step) ?? "not-current";
490+
if (result === "advanced") {
492491
const note = summary !== undefined && summary.length > 0 ? ` (${summary})` : "";
493492
return `Workflow step marked complete${note}. Advancing to the next step.`;
494493
}
495-
if (args.isPastWorkflowStep?.(step) === true) {
494+
if (result === "already-complete") {
496495
return "This workflow step is already complete. No advance.";
497496
}
498497
return "This workflow step is not current. No advance.";

src/director.test.ts

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,7 @@ describe("updateToolDefinitions rewrites infer tools", () => {
814814
describe("submit_output workflow handler", () => {
815815
const buildToolset = (opts: {
816816
isWorkflowActive: () => boolean;
817-
getCurrentWorkflowStepId?: () => string | null;
818-
isPastWorkflowStep?: (stepId: string) => boolean;
817+
completeWorkflowStep?: (stepId: string) => "advanced" | "already-complete" | "not-current";
819818
}) =>
820819
createAgentToolset({
821820
cwd: process.cwd(),
@@ -826,11 +825,8 @@ describe("submit_output workflow handler", () => {
826825
}),
827826
onOperatorGate: async () => ({ kind: "cancel" }),
828827
isWorkflowActive: opts.isWorkflowActive,
829-
...(opts.getCurrentWorkflowStepId !== undefined
830-
? { getCurrentWorkflowStepId: opts.getCurrentWorkflowStepId }
831-
: {}),
832-
...(opts.isPastWorkflowStep !== undefined
833-
? { isPastWorkflowStep: opts.isPastWorkflowStep }
828+
...(opts.completeWorkflowStep !== undefined
829+
? { completeWorkflowStep: opts.completeWorkflowStep }
834830
: {}),
835831
});
836832

@@ -858,51 +854,86 @@ describe("submit_output workflow handler", () => {
858854
const content = await runSubmit(
859855
await buildToolset({
860856
isWorkflowActive: () => true,
861-
getCurrentWorkflowStepId: () => "a",
857+
completeWorkflowStep: () => "advanced",
862858
}),
863859
{ summary: "done" },
864860
);
865861
expect(content).toContain("requires a step identifier");
866862
expect(content).not.toContain("Advancing");
867863
});
868864

869-
test("acknowledges advancement when the step matches the current step", async () => {
865+
test("reports complete() when the step advances", async () => {
870866
const content = await runSubmit(
871867
await buildToolset({
872868
isWorkflowActive: () => true,
873-
getCurrentWorkflowStepId: () => "a",
869+
completeWorkflowStep: (id) => (id === "a" ? "advanced" : "not-current"),
874870
}),
875871
{ step: "a" },
876872
);
877873
expect(content).toContain("Advancing to the next step");
878874
});
879875

880-
test("acknowledges duplicate or stale completions without claiming an advance", async () => {
876+
test("reports already-complete without claiming an advance", async () => {
881877
const content = await runSubmit(
882878
await buildToolset({
883879
isWorkflowActive: () => true,
884-
getCurrentWorkflowStepId: () => "b",
885-
isPastWorkflowStep: (id) => id === "a",
880+
completeWorkflowStep: () => "already-complete",
886881
}),
887882
{ step: "a" },
888883
);
889884
expect(content).toContain("already complete");
890885
expect(content).not.toContain("Advancing");
891886
});
892887

893-
test("does not report a future step as already complete", async () => {
888+
test("does not report a not-current step as already complete", async () => {
894889
const content = await runSubmit(
895890
await buildToolset({
896891
isWorkflowActive: () => true,
897-
getCurrentWorkflowStepId: () => "a",
898-
isPastWorkflowStep: () => false,
892+
completeWorkflowStep: () => "not-current",
899893
}),
900894
{ step: "b" },
901895
);
902896
expect(content).toContain("not current");
903897
expect(content).not.toContain("already complete");
904898
expect(content).not.toContain("Advancing");
905899
});
900+
901+
test("omitted completeWorkflowStep does not claim an advance", async () => {
902+
const content = await runSubmit(await buildToolset({ isWorkflowActive: () => true }), {
903+
step: "a",
904+
});
905+
expect(content).toContain("not current");
906+
expect(content).not.toContain("Advancing");
907+
});
908+
909+
test("parallel submit_output only one reports Advancing", async () => {
910+
const { WorkflowRuntime } = await import("./workflows/runtime.js");
911+
const workflow = {
912+
name: "simple",
913+
description: "two steps",
914+
steps: [
915+
{ id: "a", label: "A" },
916+
{ id: "b", label: "B" },
917+
],
918+
};
919+
const runtime = new WorkflowRuntime(new Map(), () => workflow);
920+
runtime.start(workflow);
921+
const toolset = await buildToolset({
922+
isWorkflowActive: () => true,
923+
completeWorkflowStep: (stepId) => runtime.complete(stepId),
924+
});
925+
const run = (id: string, step: string) =>
926+
toolset.dynamicRunner.run(
927+
{ id, name: "submit_output", arguments: { step } },
928+
new AbortController().signal,
929+
);
930+
const [first, second] = await Promise.all([run("so-1", "a"), run("so-2", "a")]);
931+
await toolset.dispose();
932+
const contents = [String(first.content), String(second.content)];
933+
expect(contents.filter((c) => c.includes("Advancing"))).toHaveLength(1);
934+
expect(contents.filter((c) => c.includes("already complete"))).toHaveLength(1);
935+
expect(runtime.currentStep()?.id).toBe("b");
936+
});
906937
});
907938

908939
describe("transient nudges", () => {

src/tui/runner.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,7 +1252,7 @@ export async function runTUI(initialConfig: Config): Promise<number> {
12521252
toolAvailability,
12531253
);
12541254
// The workflow controller is built below, after the toolset; the holder lets
1255-
// submit_output's handler read live workflow-active state without a
1255+
// submit_output's handler complete the live workflow without a
12561256
// construction-order cycle.
12571257
const workflowControllerHolder: { instance?: WorkflowController } = {};
12581258

@@ -1276,9 +1276,8 @@ export async function runTUI(initialConfig: Config): Promise<number> {
12761276
getContextDir: () => workdir,
12771277

12781278
isWorkflowActive: () => workflowControllerHolder.instance?.isActive() === true,
1279-
getCurrentWorkflowStepId: () => workflowControllerHolder.instance?.currentStepId() ?? null,
1280-
isPastWorkflowStep: (stepId) =>
1281-
workflowControllerHolder.instance?.isPastStep(stepId) === true,
1279+
completeWorkflowStep: (stepId) =>
1280+
workflowControllerHolder.instance?.complete(stepId) ?? "not-current",
12821281
...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}),
12831282
onOperatorGate: (question, options) =>
12841283
new Promise<OperatorResult>((resolve) => {

src/tui/workflow-controller.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ import {
1212
saveWorkflowState,
1313
warnWorkflowPersistenceFailure,
1414
} from "../workflows/state.js";
15-
import type { CapabilityName, StepStatus, Workflow } from "../workflows/types.js";
15+
import type {
16+
CapabilityName,
17+
StepStatus,
18+
Workflow,
19+
WorkflowCompleteResult,
20+
} from "../workflows/types.js";
1621
import type { WorkflowEvent } from "../workflows/runtime.js";
1722

1823
export interface CapabilityStatus {
@@ -105,12 +110,8 @@ export class WorkflowController {
105110
return this.runtime?.isActive() === true;
106111
}
107112

108-
currentStepId(): string | null {
109-
return this.coordinator?.currentStepId() ?? null;
110-
}
111-
112-
isPastStep(stepId: string): boolean {
113-
return this.coordinator?.isPastStep(stepId) === true;
113+
complete(stepId: string): WorkflowCompleteResult {
114+
return this.coordinator?.complete(stepId) ?? "not-current";
114115
}
115116

116117
list(): { name: string; description: string }[] {

src/workflows/coordinator.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { WorkflowRuntime } from "./runtime.js";
2-
import type { WorkflowStep } from "./types.js";
2+
import type { WorkflowCompleteResult, WorkflowStep } from "./types.js";
33

44
// Bridges the workflow runtime and a director. A director consults the
55
// coordinator for the directive to inject into each turn's system prompt and
@@ -78,16 +78,23 @@ export class WorkflowCoordinator {
7878
// the runtime, and only via compare-and-advance against the current step.
7979
// Returns true when the runtime advanced (used by tests; the directors
8080
// already reset their idle counters on any tool call, so a workflow
81-
// advance is never seen as a stall). Duplicate or stale completions are
82-
// acknowledged here without moving the cursor.
81+
// advance is never seen as a stall). Already-complete and not-current
82+
// completions are acknowledged here without moving the cursor.
8383
handleToolDone(name: string | undefined, args: unknown, isError: boolean): boolean {
8484
if (isError || !this.runtime.isActive()) return false;
8585
if (name !== "submit_output") return false;
8686
const stepId = stepIdOf(args);
8787
if (stepId === null) return false;
88-
if (this.runtime.complete(stepId) !== "advanced") return false;
89-
this.persist();
90-
return true;
88+
return this.complete(stepId) === "advanced";
89+
}
90+
91+
// Compare-and-advance, persist on a real move, and return the complete()
92+
// result so callers (submit_output's handler) report it instead of
93+
// reconstructing the cursor.
94+
complete(stepId: string): WorkflowCompleteResult {
95+
const result = this.runtime.complete(stepId);
96+
if (result === "advanced") this.persist();
97+
return result;
9198
}
9299
}
93100

src/workflows/definition.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ export interface Workflow {
2323
description: string;
2424
autoInvoke?: string;
2525
stepThrough?: boolean;
26-
autoAdvance?: boolean;
2726
steps: WorkflowStep[];
2827
}
2928

src/workflows/runtime.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
type WorkflowFrame,
99
type WorkflowState,
1010
type WorkflowStep,
11+
type WorkflowCompleteResult,
1112
} from "./types.js";
1213

1314
export type WorkflowEvent =
@@ -96,16 +97,22 @@ export class WorkflowRuntime {
9697
}
9798

9899
// Compare-and-advance against the current step. Matching `stepId` advances
99-
// atomically (check and move happen in this call). Any other id — duplicate
100-
// of a step already left behind, a future step, or no active step — is
101-
// acknowledged without moving the cursor, so a retry cannot skip ahead.
102-
complete(stepId: string): "advanced" | "acknowledged" {
100+
// atomically (check and move happen in this call). A step already behind the
101+
// cursor is already-complete; a future, unknown, or inactive id is
102+
// not-current. Neither acknowledged case moves the cursor, so a retry cannot
103+
// skip ahead.
104+
complete(stepId: string): WorkflowCompleteResult {
103105
const current = this.currentStep();
104106
if (current !== null && current.id === stepId) {
105107
this.advance();
106108
return "advanced";
107109
}
108-
return "acknowledged";
110+
const view = this.view();
111+
if (view !== null) {
112+
const idx = view.steps.findIndex((s) => s.step.id === stepId);
113+
if (idx !== -1 && idx < view.stepIndex) return "already-complete";
114+
}
115+
return "not-current";
109116
}
110117

111118
// Mark the current step complete and move to the next runnable step. Pops

0 commit comments

Comments
 (0)