|
| 1 | +# Workflow Domain Modeling Gap (Coupled to YAML schema / Scattered normalization & validation) |
| 2 | + |
| 3 | +Status: proposal |
| 4 | + |
| 5 | +## Problem |
| 6 | + |
| 7 | +After `parseWorkflow()` reads the workflow YAML, the resulting structure is used directly as the runtime domain model. |
| 8 | +As a result, the boundaries between "configuration (config)", "execution plan (compiled/resolved)", and "execution results (runtime state)" are blurry, and transformations/validation tend to be spread across multiple layers. |
| 9 | + |
| 10 | +Concrete symptoms: |
| 11 | + |
| 12 | +- Similar normalization logic is duplicated across both supervised and non-supervised paths |
| 13 | + - e.g. duration parsing for `timeout`, `path.resolve()` for `workspace`, budget assembly |
| 14 | + - Similar logic exists in `src/workflow/multi-worker-step-runner.ts` and `src/workflow/core-ipc-step-runner.ts` |
| 15 | +- Validation is scattered across multiple places, making it easy for behavior to diverge whenever the spec evolves |
| 16 | + - `src/workflow/parser.ts` (schema/field validation) |
| 17 | + - `src/workflow/dag-validator.ts` (DAG consistency + partial safety checks) |
| 18 | + - `src/workflow/executor.ts` (runtime safety/convergence/infra-failure guards) |
| 19 | +- `WORKER_TASK` over Core IPC has a weak payload type contract |
| 20 | + - Core relies on casts like `job.payload as { ... }` (see `src/core/agentcore.ts`) |
| 21 | + - Runner/Core do not share a typed contract for expected fields, so breaking changes can slip in more easily |
| 22 | + |
| 23 | +## Current Structure (Summary) |
| 24 | + |
| 25 | +- YAML -> `WorkflowDefinition` / `StepDefinition` (`src/workflow/types.ts`) |
| 26 | +- Runner: load/parse/validateDag -> feed into `WorkflowExecutor` (`src/workflow/run.ts`) |
| 27 | +- Executor: DAG state management + loop (`completion_check`) + convergence guards, etc. (`src/workflow/executor.ts`) |
| 28 | +- Step execution: delegated to the `StepRunner` port |
| 29 | + - direct spawn: `MultiWorkerStepRunner` (`src/workflow/multi-worker-step-runner.ts`) |
| 30 | + - supervised: `CoreIpcStepRunner` (`src/workflow/core-ipc-step-runner.ts`) |
| 31 | + |
| 32 | +This split is healthy, but the lack of separation between "YAML types" and "resolved execution model" is the root cause of duplication and dispersion. |
| 33 | + |
| 34 | +## Root Causes |
| 35 | + |
| 36 | +1) `StepDefinition` is used as both "user input (YAML)" and "execution plan (resolved)" |
| 37 | + |
| 38 | +2) Normalization (duration/path/capabilities/budget/env) is repeatedly performed in each StepRunner |
| 39 | + |
| 40 | +3) IPC payload types are not shared between Core and Runner, leaving the boundary as an implicit contract |
| 41 | + |
| 42 | +## Goals |
| 43 | + |
| 44 | +- Separate configuration (YAML) from execution plan (resolved/compiled), and consolidate normalization/validation into a single path |
| 45 | +- Guarantee identical resolved results (timeout/workspace/budget, etc.) across supervised and non-supervised execution |
| 46 | +- Fix the Core IPC `WORKER_TASK` payload contract as a shared type so breaking changes are caught at build time |
| 47 | +- Preserve backwards compatibility with existing YAML (no breaking changes) |
| 48 | + |
| 49 | +## Proposal |
| 50 | + |
| 51 | +### 1) Introduce a Resolved/Compiled Workflow model |
| 52 | + |
| 53 | +Produce a resolved execution model from `WorkflowDefinition` (YAML schema). |
| 54 | + |
| 55 | +Example (conceptual): |
| 56 | + |
| 57 | +- `ResolvedWorkflow`: |
| 58 | + - `workflowTimeoutMs: number` |
| 59 | + - `steps: Record<string, ResolvedStep>` |
| 60 | +- `ResolvedStep`: |
| 61 | + - `stepId: string` |
| 62 | + - `workerKind: WorkerKind` (runtime enum, not a raw string like `"OPENCODE"`) |
| 63 | + - `workspaceRef: string` (absolute path) |
| 64 | + - `timeoutMs: number` |
| 65 | + - `budget: { deadlineAt: number; maxSteps?; maxCommandTimeMs? }` |
| 66 | + - `capabilities: WorkerCapability[]` (runtime enum list, not raw strings) |
| 67 | + - `model?: string` |
| 68 | + - `completionCheck?: ResolvedCheck` |
| 69 | + |
| 70 | +`WorkflowExecutor` and StepRunners consume this compiled model, and StepRunners no longer do duration/path resolving. |
| 71 | + |
| 72 | +### 2) Re-organize the validation pipeline |
| 73 | + |
| 74 | +Split validation into: "schema validation", "semantic validation", and "runtime safety validation", and make responsibilities explicit. |
| 75 | + |
| 76 | +- Schema validation: `parser.ts` |
| 77 | +- Semantic validation: DAG, `depends_on`/inputs consistency, `completion_check` + `max_iterations`, etc. |
| 78 | +- Runtime safety validation: drift/convergence/infra failure, etc. (`executor`) |
| 79 | + |
| 80 | +At minimum, static constraints like DAG and `max_iterations` should run only once before generating the compiled model. |
| 81 | + |
| 82 | +### 3) Share the Core IPC `WORKER_TASK` payload type |
| 83 | + |
| 84 | +Share the same type/validator for the payload sent by `CoreIpcStepRunner` and received by Core. |
| 85 | + |
| 86 | +Example: |
| 87 | + |
| 88 | +- Add `src/types/worker-task-payload.ts` |
| 89 | +- `CoreIpcStepRunner` constructs a `WorkerTaskJobPayload` |
| 90 | +- `AgentCore.executeWorkerTask` validates via a type guard and avoids `as { ... }` |
| 91 | + |
| 92 | +### 4) Observability: persist resolved values as artifacts |
| 93 | + |
| 94 | +Persist resolved timeout/budget/workspaceRef/model/agent (minimum necessary) into artifacts like `context/_workflow.json` and `context/<step>/_meta.json`, so the effective settings can be reconstructed after the run. |
| 95 | + |
| 96 | +## Implementation Tasks (Phased) |
| 97 | + |
| 98 | +### Phase 0: Nail down the spec/boundaries (keep it small) |
| 99 | + |
| 100 | +- [ ] Decide the shape of the resolved model |
| 101 | + - Recommended: keep `timeoutMs`, and compute `deadlineAt = now + timeoutMs` right before step execution (so long workflows do not drift timeouts) |
| 102 | +- [ ] Decide the default timeout for `completion_check` |
| 103 | + - Recommended: if `check.timeout` is not specified, use `step.timeoutMs / 4` (aligned with existing docs). If compatibility risk exists, gate with a backwards-compat flag |
| 104 | +- [ ] Fix where agent catalog resolution happens |
| 105 | + - Recommended: keep resolving at parse time (`src/workflow/parser.ts`), and keep resolved/compiled focused on execution normalization |
| 106 | + |
| 107 | +### Phase 1: Consolidate normalization in one place (introduce a shared resolver) |
| 108 | + |
| 109 | +- [ ] Add `src/workflow/resolve-worker-task.ts` (name is flexible) |
| 110 | + - Implement `resolveTaskLike(def, workspaceRoot, env)` |
| 111 | + - duration string -> ms (`timeoutMs`, `maxCommandTimeMs`) |
| 112 | + - `workspace` -> `workspaceRef` (`path.resolve`) |
| 113 | + - capability string[] -> `WorkerCapability[]` |
| 114 | + - `worker` -> `WorkerKind` |
| 115 | + - apply `model` / `instructions` / `env` |
| 116 | + - Implement `buildWorkerTask(resolved, abortSignal)` (`deadlineAt = now + timeoutMs`) |
| 117 | +- [ ] Add unit tests: `test/unit/workflow/resolve-worker-task.test.ts` |
| 118 | + - duration parsing / workspace resolving / capability mapping / default timeout |
| 119 | + |
| 120 | +### Phase 2: Remove StepRunner duplication (replace with the resolver) |
| 121 | + |
| 122 | +- [ ] Replace `src/workflow/multi-worker-step-runner.ts` to use the resolver |
| 123 | + - remove duplicated `buildWorkerTask` / ad-hoc type definitions |
| 124 | +- [ ] Replace `src/workflow/core-ipc-step-runner.ts` to use the resolver |
| 125 | + - remove duplicated `ResolvedWorkerTaskDef` / conversion logic |
| 126 | +- [ ] Add tests ensuring "same StepDefinition -> same resolved spec" across both runners |
| 127 | + - e.g. compare only resolved fields (exclude `workerTaskId` / `deadlineAt`) |
| 128 | + |
| 129 | +### Phase 3: Lock the Core IPC boundary type contract (shared payload + validation) |
| 130 | + |
| 131 | +- [ ] Add `src/types/worker-task-job-payload.ts` (name is flexible) |
| 132 | + - `WorkerTaskJobPayload` type + a type guard (at minimum: required fields presence and types) |
| 133 | +- [ ] In `src/workflow/core-ipc-step-runner.ts`, build payload as `WorkerTaskJobPayload` |
| 134 | +- [ ] In `src/core/agentcore.ts`, validate the payload via a type guard and remove `as { ... }` |
| 135 | + - invalid payload must reliably terminate as `job_completed(outcome=failed, errorClass=NON_RETRYABLE)` (avoid hangs) |
| 136 | +- [ ] Add tests |
| 137 | + - invalid payload fail-fast in Core; no hang or unfinished state |
| 138 | + |
| 139 | +### Phase 4: (Optional) Promote to a Compiled Workflow model |
| 140 | + |
| 141 | +- [ ] Introduce `ResolvedWorkflow` / `ResolvedStepStatic` to resolve static fields ahead of time |
| 142 | + - Note: `instructions` can change dynamically due to convergence logic, so apply instructions each iteration (exclude from static resolution) |
| 143 | +- [ ] Refine boundaries between Executor/Runner and share a single resolved model |
| 144 | + |
| 145 | +### Phase 5: Observability/artifacts (persist resolved values) |
| 146 | + |
| 147 | +- [ ] Record resolved values (timeoutMs/workspaceRef/workerKind/model, etc.) minimally into `context/<step>/_meta.json` |
| 148 | +- [ ] Update docs (this issue + relevant parts of `docs/guide/workflow.md`) |
| 149 | + |
| 150 | +## Acceptance Criteria |
| 151 | + |
| 152 | +- duration/path/budget/capabilities resolution logic is consolidated into a single place |
| 153 | +- normalization logic is removed from both `MultiWorkerStepRunner` and `CoreIpcStepRunner` (they only execute compiled models) |
| 154 | +- Core validates typed payload and the Runner/Core contract is explicit |
| 155 | +- existing examples work and `make typecheck` / `make test` pass |
| 156 | + |
| 157 | +## References (Current code) |
| 158 | + |
| 159 | +- YAML parse / schema: `src/workflow/parser.ts`, `src/workflow/types.ts` |
| 160 | +- DAG validation: `src/workflow/dag-validator.ts` |
| 161 | +- Executor: `src/workflow/executor.ts` |
| 162 | +- StepRunner (direct): `src/workflow/multi-worker-step-runner.ts` |
| 163 | +- StepRunner (supervised): `src/workflow/core-ipc-step-runner.ts` |
| 164 | +- Core worker delegation: `src/core/agentcore.ts` |
0 commit comments