[v2] Workflow Agent: DAG scheduling with dependencies - #53
Merged
Conversation
Introduce the Workflow Agent package (builder, step wrappers, two-dispatch schedule/verify lifecycle) and expose work item source methods on runner execution context instead of a separate scheduler component.
Contributor
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughAdds the ChangesWorkflow Agent
Work-item Source RPC and Lifecycle Execution
Task Agent and Examples
Sequence Diagram(s)sequenceDiagram
participant WorkItemSource
participant Orchestrator
participant Runner
participant WorkflowAgent
WorkItemSource->>Orchestrator: yield runnable workflow work item
Orchestrator->>Runner: dispatch workflow work item
Runner->>WorkflowAgent: execute registered workflow script
WorkflowAgent->>Orchestrator: create children and set dependencies through RPC
Orchestrator->>WorkItemSource: apply lifecycle and dependency operations
WorkItemSource-->>Orchestrator: report child status
Orchestrator->>Runner: redispatch workflow for verification
WorkflowAgent->>WorkItemSource: pause or fail workflow
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
orchestrator-v2/packages/agent-4-workflow/src/types.ts (1)
42-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify
parseWorkflowStatevalidation to avoid redundant double-push.The presence check (lines 47–51) and the type/length check (lines 60–65) can both push the same field name into
missing, requiringSetdeduplication at lines 68 and 86. Consolidating into a single validation pass per field would improve clarity and eliminate the dedup workaround.Additionally, the
childIdscheck at line 78 (childIds === null || typeof childIds !== "object") accepts arrays, sincetypeof [] === "object". An array would be silently cast toRecord<string, string>at line 95. Consider adding anArray.isArray()guard if only plain objects are expected.♻️ Proposed refactor
export function parseWorkflowState(taskState: Record<string, unknown>): ParsedWorkflowState { const missing: string[] = []; - for (const field of REQUIRED_FIELDS) { - if (!(field in taskState) || taskState[field] === undefined) { - missing.push(field); - } - } - - if (missing.length > 0) { - return { ok: false, missing }; - } - - const workingDir = taskState.workingDir; - const definitionName = taskState.definitionName; - - if (typeof workingDir !== "string" || workingDir.length === 0) { - missing.push("workingDir"); - } - if (typeof definitionName !== "string" || definitionName.length === 0) { - missing.push("definitionName"); - } - - if (missing.length > 0) { - return { ok: false, missing: [...new Set(missing)] }; - } + const workingDir = taskState.workingDir; + if (typeof workingDir !== "string" || workingDir.length === 0) { + missing.push("workingDir"); + } + + const definitionName = taskState.definitionName; + if (typeof definitionName !== "string" || definitionName.length === 0) { + missing.push("definitionName"); + } + + if (missing.length > 0) { + return { ok: false, missing }; + } const phase = taskState.phase; const childIds = taskState.childIds; const rewindTarget = taskState.rewindTarget; if (phase !== undefined && phase !== "schedule" && phase !== "verify") { missing.push("phase"); } - if (childIds !== undefined && (childIds === null || typeof childIds !== "object")) { + if (childIds !== undefined && (childIds === null || typeof childIds !== "object" || Array.isArray(childIds))) { missing.push("childIds"); } if (rewindTarget !== undefined && typeof rewindTarget !== "string") { missing.push("rewindTarget"); } if (missing.length > 0) { - return { ok: false, missing: [...new Set(missing)] }; + return { ok: false, missing }; } return { ok: true, state: { workingDir: workingDir as string, definitionName: definitionName as string, ...(phase !== undefined ? { phase: phase as WorkflowPhase } : {}), ...(childIds !== undefined ? { childIds: childIds as Record<string, string> } : {}), ...(rewindTarget !== undefined ? { rewindTarget: rewindTarget as string } : {}), }, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts` around lines 42 - 99, Simplify parseWorkflowState by validating each required field only once so the same name is not pushed into missing twice; refactor the checks in parseWorkflowState to combine presence, type, and length validation for workingDir and definitionName, and remove the need for Set deduplication. Also tighten the childIds validation in parseWorkflowState to reject arrays by adding an Array.isArray guard before treating it as a Record<string, string>, while keeping the existing handling for phase and rewindTarget.orchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.ts (1)
96-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting child execution order for the linear workflow.
The test verifies that at least 3 children were started (
startedOrder.length >= 3) but doesn't assert they executed in dependency order (a→b→c). Since this is a linear workflow where each step depends on the previous, verifying the order would strengthen the test against a regression where dependencies are ignored but all items still run.💚 Suggested addition
function workflow_and_children_completed(this: Context) { expect(this.source.completed).toContain("workflow-1"); expect(this.source.startedOrder.length).toBeGreaterThanOrEqual(3); + expect(this.source.startedOrder.slice(0, 3)).toEqual( + expect.arrayContaining([ + expect.stringContaining("a"), + expect.stringContaining("b"), + expect.stringContaining("c"), + ]), + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.ts` around lines 96 - 118, The linear workflow assertion in workflow_and_children_completed only checks that at least three children started, but it should also verify the dependency order. Update the test to assert that the entries recorded in this.source.startedOrder reflect the expected a → b → c execution sequence, using the existing Context fields and workflow_and_children_completed helper so regressions that ignore dependencies are caught.orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts (1)
16-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
parseStepWrapperStateis called twice per execution.
createStepWrapperHandler.runparses at line 17 to extractcwd, thenrunStepWrapperre-parses the same state at line 43. SincerunStepWrapperis exported and may be called directly, consider accepting a pre-parsed state or caching the first result to avoid redundant validation.Also applies to: 43-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts` around lines 16 - 30, `createStepWrapperHandler.run` and `runStepWrapper` are parsing the same state twice in one execution path, so update the flow to reuse the first `parseStepWrapperState` result instead of re-validating. Either pass the parsed state (or derived `cwd`) into `runStepWrapper`, or add an optional pre-parsed state parameter and skip parsing when it is already available. Keep `runStepWrapper` compatible for direct callers while ensuring `run` does not invoke `parseStepWrapperState` redundantly.orchestrator-v2/docs/orchestrator.md (1)
114-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse full signature for
setDependencyin the RPC table.All other rows in the table show complete parameter lists (e.g.,
createDraftWorkItem(input),startWorkItem(workItemId)), butsetDependencyusessetDependency(...). Expanding it to match the style would improve consistency.♻️ Consistency fix
-| `workItemSource.setDependency` | `setDependency(...)` | +| `workItemSource.setDependency` | `setDependency(workItemId, dependsOnWorkItemId, type?)` |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/docs/orchestrator.md` around lines 114 - 120, Update the RPC mapping table entry for setDependency in orchestrator.md so it uses the full method signature instead of an ellipsis, matching the style of createDraftWorkItem, startWorkItem, getDependencies, and getWorkItemStatus. Locate the row for workItemSource.setDependency and replace the placeholder parameter list with the complete setDependency(...) signature used by the workItemSource API.orchestrator-v2/packages/orchestrator/src/rpc-router.ts (1)
82-99: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider validating optional
state/metadatatypes inreadCreateDraftParams.The parser correctly validates required fields (
kind,nameas strings) but passesstateandmetadatathrough unchecked. If a non-object value is supplied for either, it reachesworkItemSource.createDraftWorkItemwith an unexpected type. While the runner is a trusted client and constructs input programmatically, adding type guards for optional fields would make the parser more robust and catch malformed payloads early.♻️ Optional validation improvement
function readCreateDraftParams(params: unknown): { input: CreateDraftWorkItemInput } | null { if (params === null || typeof params !== "object") { return null; } const record = params as { input?: unknown }; if (record.input === null || typeof record.input !== "object") { return null; } const input = record.input as Partial<CreateDraftWorkItemInput>; if (typeof input.kind !== "string" || typeof input.name !== "string") { return null; } + if (input.state !== undefined && (input.state === null || typeof input.state !== "object")) { + return null; + } + if (input.metadata !== undefined && (input.metadata === null || typeof input.metadata !== "object")) { + return null; + } return { input: record.input as CreateDraftWorkItemInput }; }Also applies to: 206-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/orchestrator/src/rpc-router.ts` around lines 82 - 99, `readCreateDraftParams` should also validate the optional `state` and `metadata` fields instead of forwarding them unchecked, since malformed payloads can otherwise reach `workItemSource.createDraftWorkItem`. Update the parser to accept these fields only when they have the expected object-like shape, and return null for invalid values just like the required `kind` and `name` checks. Apply the same validation logic anywhere the create-draft input is parsed, including the other referenced parser block, so `handleCreateDraftWorkItem` only receives well-formed input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts`:
- Around line 79-89: `buildStepId` is generating duplicate IDs for items that
share the same `name` or `displayName` within a group because it ignores
`itemIndex`. Update `buildStepId` in `flatten-workflow.ts` to incorporate
`itemIndex` into the returned ID for both script and non-script
`WorkflowStepInput` cases, and keep the change localized so downstream consumers
like `schedulePass` continue to receive unique `step.id` values.
- Around line 5-77: flattenWorkflowBuilder and flattenWorkflowGroup duplicate
the same traversal, nesting, and step-construction logic; consolidate them so
only one implementation owns the workflow flattening behavior. Make
flattenWorkflowBuilder a thin wrapper that delegates to flattenWorkflowGroup (or
a shared helper) and returns the appropriate WorkflowDefinition shape, while
preserving the existing buildStepId, nested Workflow handling, and dependsOn
behavior.
In `@orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts`:
- Around line 111-131: The completed-workflow telemetry path is dead code
because `telemetryList` in `run-workflow-agent` is never populated before
`aggregateTelemetry(telemetryList)` is called. Update the `runWorkflowAgent`
flow to either remove `telemetryList` and the aggregation call if child
telemetry is unavailable, or fetch and append each child’s telemetry from the
relevant source while iterating over `definition.steps` so the final `outcome:
"completed"` result includes real aggregated telemetry.
- Around line 119-125: The verify pass in run-workflow-agent.ts is treating
non-terminal child statuses as a failure, which causes still-running children to
return a misleading failed result. Update the status handling around
getWorkItemStatus in verifyPass so that "failed" is checked first, but any
non-terminal status like "live" returns "paused" instead of "failed", and keep
the existing "completed" path unchanged. Use the verifyPass logic and the
childId/status checks to locate the fix.
In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts`:
- Around line 54-60: The rewind path in step-wrapper’s handling of transition
=== "rewind" calls source.setState without error handling, so a thrown state
update can escape as an unhandled rejection. Update the rewind branch in
step-wrapper to wrap source.setState in the same kind of try-catch used around
innerHandler.run, and on failure return a failed WorkItemResult with a clear
message instead of letting the error propagate. Use the existing wrapperState
and source symbols to keep the behavior consistent with the rest of the method.
In `@orchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.ts`:
- Around line 156-160: pauseWorkItem in graph-memory-work-item-source is only
removing the item from queued, so a work item already present in readyQueue can
still be emitted by watchWorkItems on the next drain. Update pauseWorkItem to
also remove the workItemId from readyQueue, or add a status check in
watchWorkItems before yielding items from the queue, using the existing paused,
queued, statuses, and readyQueue state to keep paused items from being
delivered.
In
`@orchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.ts`:
- Line 6: Add direct table-driven unit tests for the status-mapping helpers in
Bifrost, specifically `mapRuneStatus` and `getWorkItemStatus`, since
`bifrost-work-item-source.spec.ts` currently only covers `watchWorkItems` with
`open` and `draft` fixtures. Exercise the remaining status branches and the
default fallback by calling those helpers directly and asserting each expected
`WorkItemStatus` result, using the existing exported symbols to locate the logic
even if the implementation moves.
---
Nitpick comments:
In `@orchestrator-v2/docs/orchestrator.md`:
- Around line 114-120: Update the RPC mapping table entry for setDependency in
orchestrator.md so it uses the full method signature instead of an ellipsis,
matching the style of createDraftWorkItem, startWorkItem, getDependencies, and
getWorkItemStatus. Locate the row for workItemSource.setDependency and replace
the placeholder parameter list with the complete setDependency(...) signature
used by the workItemSource API.
In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts`:
- Around line 16-30: `createStepWrapperHandler.run` and `runStepWrapper` are
parsing the same state twice in one execution path, so update the flow to reuse
the first `parseStepWrapperState` result instead of re-validating. Either pass
the parsed state (or derived `cwd`) into `runStepWrapper`, or add an optional
pre-parsed state parameter and skip parsing when it is already available. Keep
`runStepWrapper` compatible for direct callers while ensuring `run` does not
invoke `parseStepWrapperState` redundantly.
In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts`:
- Around line 42-99: Simplify parseWorkflowState by validating each required
field only once so the same name is not pushed into missing twice; refactor the
checks in parseWorkflowState to combine presence, type, and length validation
for workingDir and definitionName, and remove the need for Set deduplication.
Also tighten the childIds validation in parseWorkflowState to reject arrays by
adding an Array.isArray guard before treating it as a Record<string, string>,
while keeping the existing handling for phase and rewindTarget.
In `@orchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.ts`:
- Around line 96-118: The linear workflow assertion in
workflow_and_children_completed only checks that at least three children
started, but it should also verify the dependency order. Update the test to
assert that the entries recorded in this.source.startedOrder reflect the
expected a → b → c execution sequence, using the existing Context fields and
workflow_and_children_completed helper so regressions that ignore dependencies
are caught.
In `@orchestrator-v2/packages/orchestrator/src/rpc-router.ts`:
- Around line 82-99: `readCreateDraftParams` should also validate the optional
`state` and `metadata` fields instead of forwarding them unchecked, since
malformed payloads can otherwise reach `workItemSource.createDraftWorkItem`.
Update the parser to accept these fields only when they have the expected
object-like shape, and return null for invalid values just like the required
`kind` and `name` checks. Apply the same validation logic anywhere the
create-draft input is parsed, including the other referenced parser block, so
`handleCreateDraftWorkItem` only receives well-formed input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d12cca2a-46ea-4945-bd77-d22c8a367348
⛔ Files ignored due to path filters (1)
orchestrator-v2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
orchestrator-v2/README.mdorchestrator-v2/docs/README.mdorchestrator-v2/docs/agent-4-workflow.mdorchestrator-v2/docs/orchestrator.mdorchestrator-v2/docs/protocol.mdorchestrator-v2/docs/runner.mdorchestrator-v2/packages/agent-3-task/src/run-task-agent.spec.tsorchestrator-v2/packages/agent-4-workflow/package.jsonorchestrator-v2/packages/agent-4-workflow/src/augment.tsorchestrator-v2/packages/agent-4-workflow/src/create-workflow-agent.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.tsorchestrator-v2/packages/agent-4-workflow/src/index.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.spec.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.tsorchestrator-v2/packages/agent-4-workflow/src/step-refs.tsorchestrator-v2/packages/agent-4-workflow/src/step-wrapper.tsorchestrator-v2/packages/agent-4-workflow/src/types.tsorchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.tsorchestrator-v2/packages/agent-4-workflow/src/workflow.tsorchestrator-v2/packages/agent-4-workflow/tsconfig.jsonorchestrator-v2/packages/agent-4-workflow/vite.config.tsorchestrator-v2/packages/interfaces-work/src/index.tsorchestrator-v2/packages/interfaces-work/src/types.tsorchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.tsorchestrator-v2/packages/orchestrator/src/index.tsorchestrator-v2/packages/orchestrator/src/orchestrator.tsorchestrator-v2/packages/orchestrator/src/rpc-router.tsorchestrator-v2/packages/orchestrator/src/test-helpers.tsorchestrator-v2/packages/orchestrator/src/types.tsorchestrator-v2/packages/runner/src/index.tsorchestrator-v2/packages/runner/src/work-item-execution-context.tsorchestrator-v2/packages/runner/src/work-item-source-client.tsorchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.tsorchestrator-v2/publish.js
💤 Files with no reviewable changes (1)
- orchestrator-v2/packages/orchestrator/src/types.ts
Address duplicate step IDs, verify-pass status handling, pause queue leaks, and related test/doc gaps from PR review.
…ate. Introduce StepResult with continue/fail/rewind helpers so the step wrapper parses the inner step return value directly.
Scripts now succeed or throw; failOnError and completeOnSuccess call workItemSource directly, agents use assertion verifiers for state, and the legacy script adapter is removed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
orchestrator-v2/docs/runner.md (1)
128-128: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
ctx.workItemSourcehere.ScriptContextexposesworkItemSource, so the docs snippet should match the actual API inorchestrator-v2/docs/runner.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/docs/runner.md` at line 128, Update the runner documentation statement to reference ctx.workItemSource instead of ctx.source, matching the ScriptContext API while preserving the existing distinction that the engine runs locally and is not proxied.orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts (1)
31-92: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConsume
rewindTargetduring rewind
rewindTargetis written to workflow state, but nothing inagent-4-workflowreads it. As written, a rewind just flips the parent back toscheduleand then back toverifywith the samechildIds, so the targeted step never gets re-selected. Wire this into the schedule path or another consumer so the intended step(s) actually rerun.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts` around lines 31 - 92, The schedulePass flow currently ignores workflow state’s rewindTarget, so rewinds reuse existing childIds without rerunning the targeted step. Update schedulePass or its immediate scheduling path to consume rewindTarget, identify the requested step(s), and recreate or reset their child work items as needed before verification; clear or otherwise mark rewindTarget consumed while preserving normal dependency scheduling and non-rewind behavior.orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts (1)
12-27: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNamespace nested workflow instances by their position.
nestedPrefixuses onlyitem.name, so same-named nestedWorkflowinstances can flatten to identical child IDs. For example, twoinnerworkflows containingtask("x")both produce...:inner:step1-1[x], allowing downstream child maps or dependency wiring to overwrite one instance. Include bothgroupIndexanditemIndexin the nested prefix, and add a regression test for duplicate nested names.Based on
flatten-workflow.ts:50-60, direct steps already include both indexes while nested prefixes do not.🐛 Proposed fix
- const nestedPrefix = `${prefix}:${item.name}`; + const nestedPrefix = + `${prefix}:${item.name}:group${groupIndex + 1}-${itemIndex + 1}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts` around lines 12 - 27, Update nestedPrefix in flattenWorkflowGroup to include both groupIndex and itemIndex alongside item.name, matching the positional namespace used for direct steps and ensuring duplicate nested workflow names produce distinct child IDs. Add a regression test covering two same-named nested workflows and verify their flattened steps and dependency wiring remain separate.
🧹 Nitpick comments (4)
orchestrator-v2/examples/lvl4/mappers/map-task-work-item.ts (1)
12-14: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winValidate required task state instead of asserting it.
as stringonly satisfies TypeScript; it does not prevent missingworkingDirorengineNamefrom reaching the runner. Validate these fields and fail with a descriptive error before constructingTaskAgentState.Proposed fix
const rune = workItem.metadata; + if (typeof workItem.state.workingDir !== "string") { + throw new Error("Task work item is missing workingDir"); + } + if (typeof workItem.state.engineName !== "string") { + throw new Error("Task work item is missing engineName"); + } return { @@ - workingDir: workItem.state.workingDir as string, - engineName: workItem.state.engineName as string, + workingDir: workItem.state.workingDir, + engineName: workItem.state.engineName,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/examples/lvl4/mappers/map-task-work-item.ts` around lines 12 - 14, Update the mapper that constructs TaskAgentState to validate workItem.state.workingDir and workItem.state.engineName before construction, rejecting missing or invalid required values with descriptive errors; remove the unsafe string assertions for these fields while preserving sessionId’s optional behavior.orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.ts (1)
66-84: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a test for the
"pause"transition.
applyStepResult(step-wrapper.ts:40-43) handles the"pause"transition by callingpauseWorkItem, but no test covers this path. TheMockSource.pauseWorkItemcurrently throws"not implemented", so a test would also require updating the mock to track pause calls.🧪 Suggested test addition
test("rewind step result rewinds workflow", { given: { rewind_step_fixture }, when: { running_decorator }, then: { workflow_is_rewound }, }); + + test("pause step result pauses work item", { + given: { pause_step_fixture }, + when: { running_decorator }, + then: { work_item_is_paused }, + }); });Add the fixture and assertion:
+function pause_step_fixture(this: Context) { + this.workItemSource = new MockSource(); + this.innerResult = { transition: "pause" }; +} + +function work_item_is_paused(this: Context) { + expect(this.error).toBeNull(); + expect(this.workItemSource.paused).toEqual(["step-child-1"]); +}Update
MockSource.pauseWorkItemto track calls:- async pauseWorkItem(): Promise<void> { - throw new Error("not implemented"); + public paused: string[] = []; + async pauseWorkItem(workItemId: string): Promise<void> { + this.paused.push(workItemId); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.ts` around lines 66 - 84, Extend the runStepDecorator tests with a pause transition case using a pause-step fixture and assertion that the workflow pauses. Update MockSource.pauseWorkItem to record invocations instead of throwing, and have the assertion verify the expected pause call.orchestrator-v2/packages/agent-4-workflow/src/types.ts (1)
89-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep
aggregateTelemetryonly if it’s part of the public API. There are no in-repo callers left; the only remaining reference is the barrel export insrc/index.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts` around lines 89 - 117, Verify whether aggregateTelemetry is part of the intended public API; if not, remove the function and its barrel export from src/index.ts. If it is public, retain both the implementation and export.orchestrator-v2/packages/runner/src/conventions/fail-on-error.ts (1)
5-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging the original error before swallowing it.
failOnErrorcatches the error and reports it to the work-item source viafailWorkItem, but never logs it locally. IffailWorkItem's RPC/report path is itself degraded or the message field is truncated upstream, the original stack trace/context is lost for local debugging.🩹 Suggested addition
export const failOnError: DecoratorFn = async (workItem, ctx, next) => { try { await next(); } catch (error) { const message = error instanceof Error ? error.message : String(error); + console.error(`Work item ${workItem.workItemId} failed:`, error); await ctx.workItemSource.failWorkItem(workItem.workItemId, message); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/runner/src/conventions/fail-on-error.ts` around lines 5 - 12, Update failOnError to log the caught original error, including its stack or full error context, before calling ctx.workItemSource.failWorkItem. Preserve the existing failure-reporting behavior and message conversion while ensuring local logging occurs even if the RPC/report path loses details.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.ts`:
- Around line 86-90: Update parallel_step_ids_are_unique to assert ids has
length 2 before the uniqueness and pairwise ID assertions, preserving the
existing checks for distinct emitted step IDs.
In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts`:
- Around line 45-50: Update the rewind branch in the step-wrapper transition
handling to merge the existing workflow state into the setState update before
overriding rewindTarget and phase. Preserve fields such as workingDir and
definitionName so the resulting state continues to satisfy
verifyIsWorkflowState(), then retain the existing error behavior.
In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts`:
- Line 5: Update the exported StepTransition union in types.ts to include
"pause", keeping it aligned with the existing StepResult transition values while
preserving the current transitions.
In `@orchestrator-v2/packages/runner/src/runner.ts`:
- Line 17: Update DEFAULT_CONVENTIONS so completion handling is composed outside
FAIL_ON_ERROR_DECORATOR, ensuring failOnError only catches script execution
errors and lifecycle RPC failures from getWorkItemStatus or completeWorkItem do
not mark successful work as failed.
In `@orchestrator-v2/packages/runner/src/script-context.ts`:
- Around line 34-39: Update setState so it persists the proposed state through
rpc.call before mutating liveWorkItem.state. Only assign nextState to
liveWorkItem.state after the RPC succeeds, while ensuring the persisted payload
contains the resulting merged state.
In `@orchestrator-v2/README.md`:
- Around line 168-176: Update the “RPC surface” table in the README to document
workItemSource.createDraftWorkItem, workItemSource.startWorkItem,
workItemSource.setDependency, workItemSource.getDependencies, and
workItemSource.getWorkItemStatus alongside the existing workItemSource.setState
entry. Include each method’s parameters and purpose based on the RPC handlers
and client usage, without changing unrelated documentation.
---
Outside diff comments:
In `@orchestrator-v2/docs/runner.md`:
- Line 128: Update the runner documentation statement to reference
ctx.workItemSource instead of ctx.source, matching the ScriptContext API while
preserving the existing distinction that the engine runs locally and is not
proxied.
In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts`:
- Around line 12-27: Update nestedPrefix in flattenWorkflowGroup to include both
groupIndex and itemIndex alongside item.name, matching the positional namespace
used for direct steps and ensuring duplicate nested workflow names produce
distinct child IDs. Add a regression test covering two same-named nested
workflows and verify their flattened steps and dependency wiring remain
separate.
In `@orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts`:
- Around line 31-92: The schedulePass flow currently ignores workflow state’s
rewindTarget, so rewinds reuse existing childIds without rerunning the targeted
step. Update schedulePass or its immediate scheduling path to consume
rewindTarget, identify the requested step(s), and recreate or reset their child
work items as needed before verification; clear or otherwise mark rewindTarget
consumed while preserving normal dependency scheduling and non-rewind behavior.
---
Nitpick comments:
In `@orchestrator-v2/examples/lvl4/mappers/map-task-work-item.ts`:
- Around line 12-14: Update the mapper that constructs TaskAgentState to
validate workItem.state.workingDir and workItem.state.engineName before
construction, rejecting missing or invalid required values with descriptive
errors; remove the unsafe string assertions for these fields while preserving
sessionId’s optional behavior.
In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.ts`:
- Around line 66-84: Extend the runStepDecorator tests with a pause transition
case using a pause-step fixture and assertion that the workflow pauses. Update
MockSource.pauseWorkItem to record invocations instead of throwing, and have the
assertion verify the expected pause call.
In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts`:
- Around line 89-117: Verify whether aggregateTelemetry is part of the intended
public API; if not, remove the function and its barrel export from src/index.ts.
If it is public, retain both the implementation and export.
In `@orchestrator-v2/packages/runner/src/conventions/fail-on-error.ts`:
- Around line 5-12: Update failOnError to log the caught original error,
including its stack or full error context, before calling
ctx.workItemSource.failWorkItem. Preserve the existing failure-reporting
behavior and message conversion while ensuring local logging occurs even if the
RPC/report path loses details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 460a4af6-a279-4549-a4e1-302639a1fce4
⛔ Files ignored due to path filters (1)
orchestrator-v2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (58)
orchestrator-v2/README.mdorchestrator-v2/docs/orchestrator.mdorchestrator-v2/docs/protocol.mdorchestrator-v2/docs/runner.mdorchestrator-v2/examples/lvl3/doSomething.tsorchestrator-v2/examples/lvl3/orchestrator.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/index.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/prepare.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/summarize.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/workflow.tsorchestrator-v2/examples/lvl4/agents/cowsay/AGENT.mdorchestrator-v2/examples/lvl4/agents/cowsay/index.tsorchestrator-v2/examples/lvl4/mappers/map-task-work-item.tsorchestrator-v2/examples/lvl4/mappers/map-workflow-work-item.tsorchestrator-v2/examples/lvl4/orchestrator.tsorchestrator-v2/examples/lvl4/package.jsonorchestrator-v2/examples/lvl4/runner.tsorchestrator-v2/packages/agent-3-task/src/index.tsorchestrator-v2/packages/agent-3-task/src/run-task-agent.spec.tsorchestrator-v2/packages/agent-3-task/src/run-task-agent.tsorchestrator-v2/packages/agent-3-task/src/types.tsorchestrator-v2/packages/agent-4-workflow/src/augment.tsorchestrator-v2/packages/agent-4-workflow/src/create-workflow-agent.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.tsorchestrator-v2/packages/agent-4-workflow/src/index.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.spec.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.tsorchestrator-v2/packages/agent-4-workflow/src/step-refs.tsorchestrator-v2/packages/agent-4-workflow/src/step-result.tsorchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.tsorchestrator-v2/packages/agent-4-workflow/src/step-wrapper.tsorchestrator-v2/packages/agent-4-workflow/src/types.tsorchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.tsorchestrator-v2/packages/agent-4-workflow/src/workflow.tsorchestrator-v2/packages/engine-claude-code/src/claude-code-engine.tsorchestrator-v2/packages/engine-cursor/src/cursor-engine.spec.tsorchestrator-v2/packages/engine-cursor/src/stream-preview.tsorchestrator-v2/packages/interfaces-work/src/index.tsorchestrator-v2/packages/interfaces-work/src/types.tsorchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.tsorchestrator-v2/packages/orchestrator/src/rpc-router.tsorchestrator-v2/packages/orchestrator/src/test-helpers.tsorchestrator-v2/packages/runner/README.mdorchestrator-v2/packages/runner/src/conventions/complete-on-success.tsorchestrator-v2/packages/runner/src/conventions/fail-on-error.tsorchestrator-v2/packages/runner/src/conventions/lifecycle-decorators.spec.tsorchestrator-v2/packages/runner/src/dispatch-handler.tsorchestrator-v2/packages/runner/src/index.tsorchestrator-v2/packages/runner/src/runner.spec.tsorchestrator-v2/packages/runner/src/runner.tsorchestrator-v2/packages/runner/src/script-agent.tsorchestrator-v2/packages/runner/src/script-context.tsorchestrator-v2/packages/runner/src/script-stack.spec.tsorchestrator-v2/packages/runner/src/script-stack.tsorchestrator-v2/packages/runner/src/work-item-source-client.tsorchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.spec.tsorchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.ts
💤 Files with no reviewable changes (1)
- orchestrator-v2/packages/runner/src/script-agent.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- orchestrator-v2/packages/agent-4-workflow/src/workflow.ts
- orchestrator-v2/packages/interfaces-work/src/index.ts
- orchestrator-v2/docs/protocol.md
- orchestrator-v2/packages/agent-4-workflow/src/step-refs.ts
- orchestrator-v2/packages/runner/src/work-item-source-client.ts
- orchestrator-v2/packages/agent-4-workflow/src/augment.ts
- orchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.ts
- orchestrator-v2/docs/orchestrator.md
- orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.spec.ts
- orchestrator-v2/packages/orchestrator/src/test-helpers.ts
- orchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.ts
- orchestrator-v2/packages/orchestrator/src/rpc-router.ts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
orchestrator-v2/docs/runner.md (1)
128-128: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
ctx.workItemSourcehere.ScriptContextexposesworkItemSource, so the docs snippet should match the actual API inorchestrator-v2/docs/runner.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/docs/runner.md` at line 128, Update the runner documentation statement to reference ctx.workItemSource instead of ctx.source, matching the ScriptContext API while preserving the existing distinction that the engine runs locally and is not proxied.orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts (1)
31-92: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConsume
rewindTargetduring rewind
rewindTargetis written to workflow state, but nothing inagent-4-workflowreads it. As written, a rewind just flips the parent back toscheduleand then back toverifywith the samechildIds, so the targeted step never gets re-selected. Wire this into the schedule path or another consumer so the intended step(s) actually rerun.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts` around lines 31 - 92, The schedulePass flow currently ignores workflow state’s rewindTarget, so rewinds reuse existing childIds without rerunning the targeted step. Update schedulePass or its immediate scheduling path to consume rewindTarget, identify the requested step(s), and recreate or reset their child work items as needed before verification; clear or otherwise mark rewindTarget consumed while preserving normal dependency scheduling and non-rewind behavior.orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts (1)
12-27: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNamespace nested workflow instances by their position.
nestedPrefixuses onlyitem.name, so same-named nestedWorkflowinstances can flatten to identical child IDs. For example, twoinnerworkflows containingtask("x")both produce...:inner:step1-1[x], allowing downstream child maps or dependency wiring to overwrite one instance. Include bothgroupIndexanditemIndexin the nested prefix, and add a regression test for duplicate nested names.Based on
flatten-workflow.ts:50-60, direct steps already include both indexes while nested prefixes do not.🐛 Proposed fix
- const nestedPrefix = `${prefix}:${item.name}`; + const nestedPrefix = + `${prefix}:${item.name}:group${groupIndex + 1}-${itemIndex + 1}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts` around lines 12 - 27, Update nestedPrefix in flattenWorkflowGroup to include both groupIndex and itemIndex alongside item.name, matching the positional namespace used for direct steps and ensuring duplicate nested workflow names produce distinct child IDs. Add a regression test covering two same-named nested workflows and verify their flattened steps and dependency wiring remain separate.
🧹 Nitpick comments (4)
orchestrator-v2/examples/lvl4/mappers/map-task-work-item.ts (1)
12-14: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winValidate required task state instead of asserting it.
as stringonly satisfies TypeScript; it does not prevent missingworkingDirorengineNamefrom reaching the runner. Validate these fields and fail with a descriptive error before constructingTaskAgentState.Proposed fix
const rune = workItem.metadata; + if (typeof workItem.state.workingDir !== "string") { + throw new Error("Task work item is missing workingDir"); + } + if (typeof workItem.state.engineName !== "string") { + throw new Error("Task work item is missing engineName"); + } return { @@ - workingDir: workItem.state.workingDir as string, - engineName: workItem.state.engineName as string, + workingDir: workItem.state.workingDir, + engineName: workItem.state.engineName,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/examples/lvl4/mappers/map-task-work-item.ts` around lines 12 - 14, Update the mapper that constructs TaskAgentState to validate workItem.state.workingDir and workItem.state.engineName before construction, rejecting missing or invalid required values with descriptive errors; remove the unsafe string assertions for these fields while preserving sessionId’s optional behavior.orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.ts (1)
66-84: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a test for the
"pause"transition.
applyStepResult(step-wrapper.ts:40-43) handles the"pause"transition by callingpauseWorkItem, but no test covers this path. TheMockSource.pauseWorkItemcurrently throws"not implemented", so a test would also require updating the mock to track pause calls.🧪 Suggested test addition
test("rewind step result rewinds workflow", { given: { rewind_step_fixture }, when: { running_decorator }, then: { workflow_is_rewound }, }); + + test("pause step result pauses work item", { + given: { pause_step_fixture }, + when: { running_decorator }, + then: { work_item_is_paused }, + }); });Add the fixture and assertion:
+function pause_step_fixture(this: Context) { + this.workItemSource = new MockSource(); + this.innerResult = { transition: "pause" }; +} + +function work_item_is_paused(this: Context) { + expect(this.error).toBeNull(); + expect(this.workItemSource.paused).toEqual(["step-child-1"]); +}Update
MockSource.pauseWorkItemto track calls:- async pauseWorkItem(): Promise<void> { - throw new Error("not implemented"); + public paused: string[] = []; + async pauseWorkItem(workItemId: string): Promise<void> { + this.paused.push(workItemId); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.ts` around lines 66 - 84, Extend the runStepDecorator tests with a pause transition case using a pause-step fixture and assertion that the workflow pauses. Update MockSource.pauseWorkItem to record invocations instead of throwing, and have the assertion verify the expected pause call.orchestrator-v2/packages/agent-4-workflow/src/types.ts (1)
89-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep
aggregateTelemetryonly if it’s part of the public API. There are no in-repo callers left; the only remaining reference is the barrel export insrc/index.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts` around lines 89 - 117, Verify whether aggregateTelemetry is part of the intended public API; if not, remove the function and its barrel export from src/index.ts. If it is public, retain both the implementation and export.orchestrator-v2/packages/runner/src/conventions/fail-on-error.ts (1)
5-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging the original error before swallowing it.
failOnErrorcatches the error and reports it to the work-item source viafailWorkItem, but never logs it locally. IffailWorkItem's RPC/report path is itself degraded or the message field is truncated upstream, the original stack trace/context is lost for local debugging.🩹 Suggested addition
export const failOnError: DecoratorFn = async (workItem, ctx, next) => { try { await next(); } catch (error) { const message = error instanceof Error ? error.message : String(error); + console.error(`Work item ${workItem.workItemId} failed:`, error); await ctx.workItemSource.failWorkItem(workItem.workItemId, message); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/runner/src/conventions/fail-on-error.ts` around lines 5 - 12, Update failOnError to log the caught original error, including its stack or full error context, before calling ctx.workItemSource.failWorkItem. Preserve the existing failure-reporting behavior and message conversion while ensuring local logging occurs even if the RPC/report path loses details.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.ts`:
- Around line 86-90: Update parallel_step_ids_are_unique to assert ids has
length 2 before the uniqueness and pairwise ID assertions, preserving the
existing checks for distinct emitted step IDs.
In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts`:
- Around line 45-50: Update the rewind branch in the step-wrapper transition
handling to merge the existing workflow state into the setState update before
overriding rewindTarget and phase. Preserve fields such as workingDir and
definitionName so the resulting state continues to satisfy
verifyIsWorkflowState(), then retain the existing error behavior.
In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts`:
- Line 5: Update the exported StepTransition union in types.ts to include
"pause", keeping it aligned with the existing StepResult transition values while
preserving the current transitions.
In `@orchestrator-v2/packages/runner/src/runner.ts`:
- Line 17: Update DEFAULT_CONVENTIONS so completion handling is composed outside
FAIL_ON_ERROR_DECORATOR, ensuring failOnError only catches script execution
errors and lifecycle RPC failures from getWorkItemStatus or completeWorkItem do
not mark successful work as failed.
In `@orchestrator-v2/packages/runner/src/script-context.ts`:
- Around line 34-39: Update setState so it persists the proposed state through
rpc.call before mutating liveWorkItem.state. Only assign nextState to
liveWorkItem.state after the RPC succeeds, while ensuring the persisted payload
contains the resulting merged state.
In `@orchestrator-v2/README.md`:
- Around line 168-176: Update the “RPC surface” table in the README to document
workItemSource.createDraftWorkItem, workItemSource.startWorkItem,
workItemSource.setDependency, workItemSource.getDependencies, and
workItemSource.getWorkItemStatus alongside the existing workItemSource.setState
entry. Include each method’s parameters and purpose based on the RPC handlers
and client usage, without changing unrelated documentation.
---
Outside diff comments:
In `@orchestrator-v2/docs/runner.md`:
- Line 128: Update the runner documentation statement to reference
ctx.workItemSource instead of ctx.source, matching the ScriptContext API while
preserving the existing distinction that the engine runs locally and is not
proxied.
In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts`:
- Around line 12-27: Update nestedPrefix in flattenWorkflowGroup to include both
groupIndex and itemIndex alongside item.name, matching the positional namespace
used for direct steps and ensuring duplicate nested workflow names produce
distinct child IDs. Add a regression test covering two same-named nested
workflows and verify their flattened steps and dependency wiring remain
separate.
In `@orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts`:
- Around line 31-92: The schedulePass flow currently ignores workflow state’s
rewindTarget, so rewinds reuse existing childIds without rerunning the targeted
step. Update schedulePass or its immediate scheduling path to consume
rewindTarget, identify the requested step(s), and recreate or reset their child
work items as needed before verification; clear or otherwise mark rewindTarget
consumed while preserving normal dependency scheduling and non-rewind behavior.
---
Nitpick comments:
In `@orchestrator-v2/examples/lvl4/mappers/map-task-work-item.ts`:
- Around line 12-14: Update the mapper that constructs TaskAgentState to
validate workItem.state.workingDir and workItem.state.engineName before
construction, rejecting missing or invalid required values with descriptive
errors; remove the unsafe string assertions for these fields while preserving
sessionId’s optional behavior.
In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.ts`:
- Around line 66-84: Extend the runStepDecorator tests with a pause transition
case using a pause-step fixture and assertion that the workflow pauses. Update
MockSource.pauseWorkItem to record invocations instead of throwing, and have the
assertion verify the expected pause call.
In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts`:
- Around line 89-117: Verify whether aggregateTelemetry is part of the intended
public API; if not, remove the function and its barrel export from src/index.ts.
If it is public, retain both the implementation and export.
In `@orchestrator-v2/packages/runner/src/conventions/fail-on-error.ts`:
- Around line 5-12: Update failOnError to log the caught original error,
including its stack or full error context, before calling
ctx.workItemSource.failWorkItem. Preserve the existing failure-reporting
behavior and message conversion while ensuring local logging occurs even if the
RPC/report path loses details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 460a4af6-a279-4549-a4e1-302639a1fce4
⛔ Files ignored due to path filters (1)
orchestrator-v2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (58)
orchestrator-v2/README.mdorchestrator-v2/docs/orchestrator.mdorchestrator-v2/docs/protocol.mdorchestrator-v2/docs/runner.mdorchestrator-v2/examples/lvl3/doSomething.tsorchestrator-v2/examples/lvl3/orchestrator.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/index.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/prepare.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/summarize.tsorchestrator-v2/examples/lvl4/agents/cowsay-flow/workflow.tsorchestrator-v2/examples/lvl4/agents/cowsay/AGENT.mdorchestrator-v2/examples/lvl4/agents/cowsay/index.tsorchestrator-v2/examples/lvl4/mappers/map-task-work-item.tsorchestrator-v2/examples/lvl4/mappers/map-workflow-work-item.tsorchestrator-v2/examples/lvl4/orchestrator.tsorchestrator-v2/examples/lvl4/package.jsonorchestrator-v2/examples/lvl4/runner.tsorchestrator-v2/packages/agent-3-task/src/index.tsorchestrator-v2/packages/agent-3-task/src/run-task-agent.spec.tsorchestrator-v2/packages/agent-3-task/src/run-task-agent.tsorchestrator-v2/packages/agent-3-task/src/types.tsorchestrator-v2/packages/agent-4-workflow/src/augment.tsorchestrator-v2/packages/agent-4-workflow/src/create-workflow-agent.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.tsorchestrator-v2/packages/agent-4-workflow/src/index.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.spec.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.tsorchestrator-v2/packages/agent-4-workflow/src/step-refs.tsorchestrator-v2/packages/agent-4-workflow/src/step-result.tsorchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.tsorchestrator-v2/packages/agent-4-workflow/src/step-wrapper.tsorchestrator-v2/packages/agent-4-workflow/src/types.tsorchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.tsorchestrator-v2/packages/agent-4-workflow/src/workflow.tsorchestrator-v2/packages/engine-claude-code/src/claude-code-engine.tsorchestrator-v2/packages/engine-cursor/src/cursor-engine.spec.tsorchestrator-v2/packages/engine-cursor/src/stream-preview.tsorchestrator-v2/packages/interfaces-work/src/index.tsorchestrator-v2/packages/interfaces-work/src/types.tsorchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.tsorchestrator-v2/packages/orchestrator/src/rpc-router.tsorchestrator-v2/packages/orchestrator/src/test-helpers.tsorchestrator-v2/packages/runner/README.mdorchestrator-v2/packages/runner/src/conventions/complete-on-success.tsorchestrator-v2/packages/runner/src/conventions/fail-on-error.tsorchestrator-v2/packages/runner/src/conventions/lifecycle-decorators.spec.tsorchestrator-v2/packages/runner/src/dispatch-handler.tsorchestrator-v2/packages/runner/src/index.tsorchestrator-v2/packages/runner/src/runner.spec.tsorchestrator-v2/packages/runner/src/runner.tsorchestrator-v2/packages/runner/src/script-agent.tsorchestrator-v2/packages/runner/src/script-context.tsorchestrator-v2/packages/runner/src/script-stack.spec.tsorchestrator-v2/packages/runner/src/script-stack.tsorchestrator-v2/packages/runner/src/work-item-source-client.tsorchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.spec.tsorchestrator-v2/packages/work-item-source-bifrost/src/bifrost-work-item-source.ts
💤 Files with no reviewable changes (1)
- orchestrator-v2/packages/runner/src/script-agent.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- orchestrator-v2/packages/agent-4-workflow/src/workflow.ts
- orchestrator-v2/packages/interfaces-work/src/index.ts
- orchestrator-v2/docs/protocol.md
- orchestrator-v2/packages/agent-4-workflow/src/step-refs.ts
- orchestrator-v2/packages/runner/src/work-item-source-client.ts
- orchestrator-v2/packages/agent-4-workflow/src/augment.ts
- orchestrator-v2/packages/agent-4-workflow/src/workflow.integration.spec.ts
- orchestrator-v2/docs/orchestrator.md
- orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.spec.ts
- orchestrator-v2/packages/orchestrator/src/test-helpers.ts
- orchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.ts
- orchestrator-v2/packages/orchestrator/src/rpc-router.ts
🛑 Comments failed to post (6)
orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.ts (1)
86-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that both parallel items are emitted.
The uniqueness checks do not prove cardinality: if flattening returned only one step, the current assertions could still pass because
ids[1]would beundefined. Addexpect(ids).toHaveLength(2)before checking uniqueness.Based on
parallel_same_name_workflowat Lines 81-83, the test input contains exactly two tasks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.ts` around lines 86 - 90, Update parallel_step_ids_are_unique to assert ids has length 2 before the uniqueness and pairwise ID assertions, preserving the existing checks for distinct emitted step IDs.orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts (1)
45-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Find the setState implementation in the work item source client and graph memory source rg -nP 'setState' --type=ts orchestrator-v2/packages/runner/src/work-item-source-client.ts orchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.ts orchestrator-v2/packages/interfaces-work/src/types.ts -C5Repository: devzeebo/bifrost
Length of output: 6376
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the rewind path and workflow-state validation. sed -n '1,220p' orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts printf '\n---\n' sed -n '1,240p' orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts printf '\n---\n' rg -n "verifyIsWorkflowState|workingDir|definitionName|rewindTarget|transition === \"rewind\"" orchestrator-v2/packages/agent-4-workflow/src -C 3 printf '\n---\n' sed -n '1,220p' orchestrator-v2/packages/orchestrator/src/graph-memory-work-item-source.tsRepository: devzeebo/bifrost
Length of output: 27310
Preserve workflow state on rewind
setState(...)replaces the stored state here, so passing onlyrewindTargetandphasedropsworkingDiranddefinitionName. The next workflow dispatch will failverifyIsWorkflowState(). Merge the existing state into this update.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/step-wrapper.ts` around lines 45 - 50, Update the rewind branch in the step-wrapper transition handling to merge the existing workflow state into the setState update before overriding rewindTarget and phase. Preserve fields such as workingDir and definitionName so the resulting state continues to satisfy verifyIsWorkflowState(), then retain the existing error behavior.orchestrator-v2/packages/agent-4-workflow/src/types.ts (1)
5-5: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Search for usages of StepTransition across the codebase rg -nP '\bStepTransition\b' --type=ts orchestrator-v2/packages/agent-4-workflow/src/Repository: devzeebo/bifrost
Length of output: 348
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant type definitions and nearby exports/usages. printf '\n== types.ts ==\n' cat -n orchestrator-v2/packages/agent-4-workflow/src/types.ts printf '\n== step-result.ts ==\n' cat -n orchestrator-v2/packages/agent-4-workflow/src/step-result.ts printf '\n== index.ts ==\n' cat -n orchestrator-v2/packages/agent-4-workflow/src/index.ts printf '\n== StepTransition usages ==\n' rg -nP '\bStepTransition\b' orchestrator-v2/packages/agent-4-workflow/src/Repository: devzeebo/bifrost
Length of output: 7571
Add
"pause"toStepTransition.StepResultalready includes it, and this exported union should stay aligned so consumers don’t reject a valid transition.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/types.ts` at line 5, Update the exported StepTransition union in types.ts to include "pause", keeping it aligned with the existing StepResult transition values while preserving the current transitions.orchestrator-v2/packages/runner/src/runner.ts (1)
17-17: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep completion infrastructure errors outside
failOnError.With outermost-first composition, this order makes
failOnErrorcatch failures fromgetWorkItemStatusorcompleteWorkItem. A successful script can therefore be marked failed due to a transient lifecycle RPC error. Put completion outside failure handling so only execution errors transition the item to failed.Proposed fix
-const DEFAULT_CONVENTIONS = [FAIL_ON_ERROR_DECORATOR, COMPLETE_ON_SUCCESS_DECORATOR] as const; +const DEFAULT_CONVENTIONS = [COMPLETE_ON_SUCCESS_DECORATOR, FAIL_ON_ERROR_DECORATOR] as const;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const DEFAULT_CONVENTIONS = [COMPLETE_ON_SUCCESS_DECORATOR, FAIL_ON_ERROR_DECORATOR] as const;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/runner/src/runner.ts` at line 17, Update DEFAULT_CONVENTIONS so completion handling is composed outside FAIL_ON_ERROR_DECORATOR, ensuring failOnError only catches script execution errors and lifecycle RPC failures from getWorkItemStatus or completeWorkItem do not mark successful work as failed.orchestrator-v2/packages/runner/src/script-context.ts (1)
34-39: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the live state only after persistence succeeds.
A rejected
setStateRPC leavesliveWorkItem.statecontaining unpersisted values. A retrying decorator can then make decisions from state the source never stored.Proposed fix
async setState(nextState) { - Object.assign(liveWorkItem.state, nextState); + const state = { ...liveWorkItem.state, ...nextState }; await rpc.call("workItemSource.setState", { workItemId: workItem.workItemId, - state: liveWorkItem.state, + state, }); + Object.assign(liveWorkItem.state, nextState); },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.async setState(nextState) { const state = { ...liveWorkItem.state, ...nextState }; await rpc.call("workItemSource.setState", { workItemId: workItem.workItemId, state, }); Object.assign(liveWorkItem.state, nextState);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/runner/src/script-context.ts` around lines 34 - 39, Update setState so it persists the proposed state through rpc.call before mutating liveWorkItem.state. Only assign nextState to liveWorkItem.state after the RPC succeeds, while ensuring the persisted payload contains the resulting merged state.orchestrator-v2/README.md (1)
168-176: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
RPC surface table is missing the new
workItemSourceRPC methods.The table documents only
workItemSource.setState, butrpc-router.tsand the runner'swork-item-source-client.tsalso handleworkItemSource.createDraftWorkItem,workItemSource.startWorkItem,workItemSource.setDependency,workItemSource.getDependencies, andworkItemSource.getWorkItemStatus. Since this PR's primary objective is replacing the scheduler with these work-item-source RPCs, they should be documented here.📝 Proposed addition to the RPC surface table
| Method | Params | Purpose | | ------------------------- | ------------------------- | ------------------------ | | `dispatch` | `WorkItem` | Execute work on runner | | `workItem.complete` | `{ workItemId }` | Mark work item completed | | `workItem.fail` | `{ workItemId, message }` | Mark work item failed | | `workItem.pause` | `{ workItemId }` | Mark work item paused | | `workItemSource.setState` | `{ workItemId, state }` | Persist handler state | +| `workItemSource.createDraftWorkItem` | `{ input }` | Create a draft work item | +| `workItemSource.startWorkItem` | `{ workItemId }` | Start a draft work item | +| `workItemSource.setDependency` | `{ workItemId, dependsOnWorkItemId, type? }` | Wire a dependency | +| `workItemSource.getDependencies` | `{ workItemId }` | Query dependencies | +| `workItemSource.getWorkItemStatus` | `{ workItemId }` | Query work item status |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.## RPC surface | Method | Params | Purpose | | ------------------------- | ------------------------- | ------------------------ | | `dispatch` | `WorkItem` | Execute work on runner | | `workItem.complete` | `{ workItemId }` | Mark work item completed | | `workItem.fail` | `{ workItemId, message }` | Mark work item failed | | `workItem.pause` | `{ workItemId }` | Mark work item paused | | `workItemSource.setState` | `{ workItemId, state }` | Persist handler state | | `workItemSource.createDraftWorkItem` | `{ input }` | Create a draft work item | | `workItemSource.startWorkItem` | `{ workItemId }` | Start a draft work item | | `workItemSource.setDependency` | `{ workItemId, dependsOnWorkItemId, type? }` | Wire a dependency | | `workItemSource.getDependencies` | `{ workItemId }` | Query dependencies | | `workItemSource.getWorkItemStatus` | `{ workItemId }` | Query work item status |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/README.md` around lines 168 - 176, Update the “RPC surface” table in the README to document workItemSource.createDraftWorkItem, workItemSource.startWorkItem, workItemSource.setDependency, workItemSource.getDependencies, and workItemSource.getWorkItemStatus alongside the existing workItemSource.setState entry. Include each method’s parameters and purpose based on the RPC handlers and client usage, without changing unrelated documentation.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
orchestrator-v2/README.md (1)
108-108: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep the runner key ID consistent with the YAML example.
This code now generates
runner, but the example configuration at Line 157 still usesrunner-1. If both snippets are followed, the orchestrator authorizes one key ID while the runner presents another, so authentication fails. Update the YAML example torunner(or userunner-1consistently).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/README.md` at line 108, Align the YAML example’s runner key ID with the generateKeyPair call using "runner": update the example from "runner-1" to "runner" so authorization and runner identity match.
🧹 Nitpick comments (1)
orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.ts (1)
112-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid mutating the object under test.
The use of
Array.prototype.sort()mutates thedependsOnarray in place. While this might not cause an issue here since it is the final assertion, it is generally best practice to avoid mutating test data to prevent accidental side effects. Consider sorting a shallow copy of the array instead.♻️ Proposed refactor
const afterStep = this.definition.steps.find((step) => step.id.includes("[after]")); - expect(afterStep?.dependsOn.sort()).toEqual(innerSteps.map((step) => step.id).sort()); + expect([...(afterStep?.dependsOn ?? [])].sort()).toEqual(innerSteps.map((step) => step.id).sort());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.ts` around lines 112 - 113, Update the assertion for afterStep in the flatten-workflow test to sort a shallow copy of dependsOn rather than mutating the object under test; keep the existing comparison with the sorted innerSteps IDs unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@orchestrator-v2/README.md`:
- Line 108: Align the YAML example’s runner key ID with the generateKeyPair call
using "runner": update the example from "runner-1" to "runner" so authorization
and runner identity match.
---
Nitpick comments:
In `@orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.ts`:
- Around line 112-113: Update the assertion for afterStep in the
flatten-workflow test to sort a shallow copy of dependsOn rather than mutating
the object under test; keep the existing comparison with the sorted innerSteps
IDs unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3c7e349f-e866-4aad-87f3-5a629d20a3e4
📒 Files selected for processing (15)
orchestrator-v2/README.mdorchestrator-v2/docs/runner.mdorchestrator-v2/examples/lvl4/mappers/map-task-work-item.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.spec.tsorchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.tsorchestrator-v2/packages/agent-4-workflow/src/index.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.spec.tsorchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.tsorchestrator-v2/packages/agent-4-workflow/src/step-result.tsorchestrator-v2/packages/agent-4-workflow/src/step-wrapper.spec.tsorchestrator-v2/packages/agent-4-workflow/src/step-wrapper.tsorchestrator-v2/packages/agent-4-workflow/src/types.tsorchestrator-v2/packages/runner/src/conventions/fail-on-error.tsorchestrator-v2/packages/runner/src/runner.tsorchestrator-v2/packages/runner/src/script-context.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- orchestrator-v2/packages/runner/src/conventions/fail-on-error.ts
- orchestrator-v2/docs/runner.md
- orchestrator-v2/examples/lvl4/mappers/map-task-work-item.ts
- orchestrator-v2/packages/agent-4-workflow/src/flatten-workflow.ts
- orchestrator-v2/packages/runner/src/script-context.ts
- orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.ts
- orchestrator-v2/packages/runner/src/runner.ts
- orchestrator-v2/packages/agent-4-workflow/src/run-workflow-agent.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@bifrost-ai/agent-4-workflowwith a two-dispatch schedule/verify lifecycle, fluentWorkflowbuilder (including nested workflows), and per-step Success/Fail/Rewind wrappers.ctx.source(createDraftWorkItem,startWorkItem,setDependency,getDependencies,getWorkItemStatus).createGraphMemoryWorkItemSourcefor integration tests and implements child-failure unblock semantics (terminal failed children clear workflow blockers so verify can fail the workflow).Fulfills #39.
Test plan
vp run -r test— unit tests for flatten, schedule/verify, and end-to-end linear workflow integrationvp run -r check— format, lint, typecheck (agent-4-workflow, orchestrator, runner, interfaces-work, work-item-source-bifrost)