From ec99615ccf386f52b31ad5f5290055d0970bef98 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Sun, 14 Jun 2026 16:00:39 +0300 Subject: [PATCH 1/5] docs(roadmap): mark 1.P (node-type handlers) Done (PR #20); 1.Q is the next workstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #20 merged — the six non-agent NodeExecutor arms (condition / transform / fan_out / fan_in / input / output) behind a dispatching executor, executor-only, with the pre-merge BLOCKER secret-leak fixed via a secretInputNames masking gate on NodeExecContext. - phase-1-engine-and-llm.md: §1.P → ✅ Done (PR #20); top status block updated; 1.Q/1.R/1.S/1.AC remain. - current.md: 1.P ✅ Done in both status passages; the human gate (1.Q) named as next. - CLAUDE.md + README.md: status lines advanced to 1.P done, 1.Q next. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 10 ++++++---- README.md | 7 ++++--- docs/roadmap/current.md | 20 ++++++++++++------- docs/roadmap/phases/phase-1-engine-and-llm.md | 20 ++++++++++++++----- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f2d1e6e9..013d2c2c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,7 +38,7 @@ A run executes in one of **three execution modes** behind the one `LLMProvider` engine is identical across all three. See [ADR-0012](docs/decisions/0012-managed-inference-dual-mode.md) to [ADR-0015](docs/decisions/0015-managed-mode-data-handling-and-compliance.md) and [docs/architecture/managed-inference.md](docs/architecture/managed-inference.md). -**Status: Phase 1 in progress — milestone M1 (LLM seam proven) reached (PR #9, 2026-06-07); the `FallbackChain` runner (1.K) landed, completing 1.m2 with the cost tracker (PR #13, 2026-06-11); the run loop (1.N — `WorkflowEngine` + `RunEventBus`) landed (PR #17, 2026-06-13) **completing 1.m3** (parse → DAG → run loop emits the canonical event stream), with the built-in `ToolRegistry` (1.T, a 1.m4 component) landing alongside it as the other `AgentRunner` (1.O) join prerequisite; and the **`AgentRunner` (1.O) — per-node LLM execution behind the seam — landed (PR #18, 2026-06-14)**. The node-type handlers (1.P) are next, toward M2.** +**Status: Phase 1 in progress — milestone M1 (LLM seam proven) reached (PR #9, 2026-06-07); the `FallbackChain` runner (1.K) landed, completing 1.m2 with the cost tracker (PR #13, 2026-06-11); the run loop (1.N — `WorkflowEngine` + `RunEventBus`) landed (PR #17, 2026-06-13) **completing 1.m3** (parse → DAG → run loop emits the canonical event stream), with the built-in `ToolRegistry` (1.T, a 1.m4 component) landing alongside it as the other `AgentRunner` (1.O) join prerequisite; the **`AgentRunner` (1.O) — per-node LLM execution behind the seam — landed (PR #18, 2026-06-14)**; and the **node-type handlers (1.P) — the six non-agent `NodeExecutor` arms (condition / transform / fan_out / fan_in / input / output) behind a dispatching executor — landed (PR #20, 2026-06-14)**. The human gate (1.Q) is next, toward M2.** Phase 0 (M0, 2026-06-04) landed the monorepo, strict toolchain + CI, `@relavium/shared` (the full Zod contract set), the no-vendor-type seam fence, and `@relavium/db`. Phase 1 has since landed `@relavium/llm` — the `LLMProvider` seam + all three adapters (Anthropic, OpenAI/DeepSeek, @@ -51,10 +51,12 @@ reserved generator methods, shape-only, landed before the seam's exhaustive cons **`WorkflowYAMLParser` (1.L, PR #14)**, the **`{{ … }}` interpolation engine + parse-time secret-taint gate (1.L2, PR #15)**, the **DAG builder + `RunPlan` (1.M)** plus the **QuickJS-wasm expression sandbox (1.AB)** (PR #16, 2026-06-13), and the **run loop — `WorkflowEngine` + `RunEventBus` (1.N)** -together with the **built-in `ToolRegistry` (1.T)** (PR #17, 2026-06-13), and the **`AgentRunner` (1.O)** +together with the **built-in `ToolRegistry` (1.T)** (PR #17, 2026-06-13), the **`AgentRunner` (1.O)** join (PR #18, 2026-06-14 — host-injected provider resolution behind the seam, the correlation-agnostic -turn core, the tool-call loop, and the same-provider reasoning replay). Active work is now the remaining -1.m4 lane — the node-type handlers (1.P), the human gate (1.Q), checkpoint/resume (1.R), node retry +turn core, the tool-call loop, and the same-provider reasoning replay), and the **node-type handlers +(1.P)** (PR #20, 2026-06-14 — the six non-agent `NodeExecutor` arms behind a dispatching executor, +executor-only with a `secretInputNames` masking gate on `NodeExecContext`). Active work is now the +remaining 1.m4 lane — the human gate (1.Q), checkpoint/resume (1.R), node retry (1.S), and the pre-egress budget governor (1.AC) — toward **M2**; see [docs/roadmap/current.md](docs/roadmap/current.md). See [README.md](README.md) for the public overview. diff --git a/README.md b/README.md index 0bdb713e..428bacec 100644 --- a/README.md +++ b/README.md @@ -43,8 +43,9 @@ secret-taint gate** (1.L2, PR #15), and the **DAG builder + `RunPlan`** (1.M) to **QuickJS-wasm expression sandbox** (1.AB) (PR #16, 2026-06-13) — all with zero platform imports. The **run loop** (1.N — `WorkflowEngine` + `RunEventBus`) landed (PR #17, 2026-06-13), **completing milestone 1.m3** (parse → DAG → run loop emits the canonical event stream); the **built-in -`ToolRegistry`** (1.T, a 1.m4 component) landed alongside it; and the **`AgentRunner`** (1.O — -per-node LLM execution behind the seam) landed (PR #18, 2026-06-14). Next on the critical path: the -**node-type handlers** (1.P), the human gate, checkpoint/resume and retry, plus the **AgentSession** +`ToolRegistry`** (1.T, a 1.m4 component) landed alongside it; the **`AgentRunner`** (1.O — +per-node LLM execution behind the seam) landed (PR #18, 2026-06-14); and the **node-type handlers** +(1.P — the six non-agent handlers behind a dispatching executor) landed (PR #20, 2026-06-14). Next on +the critical path: the **human gate** (1.Q), checkpoint/resume and retry, plus the **AgentSession** runtime + export-to-workflow sub-spine. See [docs/roadmap/current.md](docs/roadmap/current.md) for live status. diff --git a/docs/roadmap/current.md b/docs/roadmap/current.md index bdb76ca1..40206d1a 100644 --- a/docs/roadmap/current.md +++ b/docs/roadmap/current.md @@ -140,11 +140,16 @@ separate `AgentRunner` (1.O) join prerequisite — it does **not** close 1.m3. T landed the **1.O `AgentRunner` join ✅ Done (PR #18, 2026-06-14)** — per-node LLM execution behind the `@relavium/llm` seam: the host-injected provider-resolution boundary ([ADR-0038](../decisions/0038-agentrunner-llm-call-boundary.md)), the correlation-agnostic turn core (reused by 1.V), the tool-call loop + classified failure ladder, and -the same-provider signed-reasoning replay ([ADR-0039](../decisions/0039-same-provider-reasoning-replay.md)). -The lane now continues at the remaining **1.m4** workstreams toward **M2** — the **node-type handlers -(1.P)**, the **human gate (1.Q)**, **checkpoint/resume (1.R)**, **node retry (1.S)**, and the -**pre-egress budget governor (1.AC)** — and the agent-first sub-spine (**1.V–1.AA**, Lane C) opens now -that 1.O exists. +the same-provider signed-reasoning replay ([ADR-0039](../decisions/0039-same-provider-reasoning-replay.md)) — +and the **node-type handlers (1.P) ✅ Done (PR #20, 2026-06-14)**: the six non-agent `NodeExecutor` arms +(condition / transform / fan_out / fan_in / input / output) composed by a `createDispatchingNodeExecutor` +alongside the 1.O agent arm — executor-only, no `engine.ts` change (the run loop already owns readiness, +skip-propagation, fan-in join scheduling, events, cancellation), `wait_first` executor-only (true +loser-cancel deferred), and a pre-merge BLOCKER secret-leak (the `input` handler emitting raw +`secret`-typed inputs into events) fixed via a `secretInputNames` masking gate on `NodeExecContext`. +The lane now continues at the remaining **1.m4** workstreams toward **M2** — the **human gate (1.Q)**, +**checkpoint/resume (1.R)**, **node retry (1.S)**, and the **pre-egress budget governor (1.AC)** — and the +agent-first sub-spine (**1.V–1.AA**, Lane C) opens now that 1.O exists. > **Multimodal I/O — the shape is landed (1.AD ✅ Done, PR #11, 2026-06-10).** First-class > image/audio/video I/O (input **and** output, incl. generate-media-by-rule) was decided on 2026-06-08: @@ -171,8 +176,9 @@ that 1.O exists. > `RunPlan`) and 1.AB (the expression sandbox) have since landed (PR #16, merged 2026-06-13)**; and > **1.N (`WorkflowEngine` + `RunEventBus`) and 1.T (the built-in `ToolRegistry`) are ✅ Done (PR #17, > merged 2026-06-13)** — **1.N closes 1.m3** (its last component); **1.T** (a 1.m4 component) is the -> other 1.O join prerequisite; and **the `AgentRunner` join (1.O) is ✅ Done (PR #18, 2026-06-14)**. The -> **node-type handlers (1.P)** are the next workstream. +> other 1.O join prerequisite; **the `AgentRunner` join (1.O) is ✅ Done (PR #18, 2026-06-14)**; and the +> **node-type handlers (1.P) are ✅ Done (PR #20, 2026-06-14)**. The **human gate (1.Q)** is the next +> workstream. Carry-over hardening is tracked in [deferred-tasks.md](deferred-tasks.md) — pick items up as Phase 1 first touches each file. diff --git a/docs/roadmap/phases/phase-1-engine-and-llm.md b/docs/roadmap/phases/phase-1-engine-and-llm.md index ae1954a8..a9193718 100644 --- a/docs/roadmap/phases/phase-1-engine-and-llm.md +++ b/docs/roadmap/phases/phase-1-engine-and-llm.md @@ -18,9 +18,13 @@ > ✅ Done (PR #16, 2026-06-13)** — the plan layer and the deterministic `condition`/`transform`/`merge_fn` > evaluator. **1.N (`WorkflowEngine` + `RunEventBus`) and 1.T (built-in `ToolRegistry`) are ✅ Done > (PR #17, 2026-06-13)**, completing **1.m3** (parse → DAG → run loop emits the canonical event stream). -> With 1.K, 1.N, and 1.T all landed, the **1.O `AgentRunner` join is ✅ Done (PR #18, 2026-06-14)** — the -> lane now continues at the remaining 1.m4 handlers (1.P/1.Q/1.R/1.S/1.AC) toward **M2**, and Lane C -> (1.V–1.AA) opens. *(Session persistence, 1.X/1.Z, must exclude the reasoning signature — non-persisting.)* +> With 1.K, 1.N, and 1.T all landed, the **1.O `AgentRunner` join is ✅ Done (PR #18, 2026-06-14)**, and +> the **node-type handlers (1.P) are ✅ Done (PR #20, 2026-06-14)** — the six non-agent `NodeExecutor` arms +> behind a dispatching executor, executor-only (no `engine.ts` change), with a pre-merge BLOCKER secret-leak +> fixed by a `secretInputNames` masking gate on `NodeExecContext`. The lane now continues at the remaining +> 1.m4 workstreams (**1.Q** human gate, **1.R** checkpoint/resume, **1.S** node retry, **1.AC** budget +> governor) toward **M2**, and Lane C (1.V–1.AA) opens. *(Session persistence, 1.X/1.Z, must exclude the +> reasoning signature — non-persisting.)* > > **Multimodal I/O decided (2026-06-08).** First-class image/audio/video I/O (input **and** output) is a > second pre-freeze seam amendment in the ADR-0030 mould — [ADR-0031](../../decisions/0031-llm-seam-shape-amendment-multimodal-io.md) @@ -654,9 +658,15 @@ recorded follow-up needing a validator dependency behind an ADR.)* > transform/sandbox-node follow-up ([deferred-tasks.md](../deferred-tasks.md)); 1.O's structural > guarantee is `system` = authored text only, the resolved prompt in a `user` position. -### 1.P — Node-type handlers (condition / fan-out / fan-in / transform / input / output) +### 1.P — Node-type handlers (condition / fan-out / fan-in / transform / input / output) — ✅ **Done (PR #20, 2026-06-14)** -The dispatch table for the non-agent node types the DAG can contain. +The dispatch table for the non-agent node types the DAG can contain. **Landed** as a +`createDispatchingNodeExecutor` composing the six handlers (and the 1.O agent arm) into the one +injected `NodeExecutor` — executor-only, no `engine.ts` change for the merge/branch work (the run loop +already owns readiness, skip-propagation, fan-in join scheduling, events, cancellation). `wait_first` is +executor-only (true loser-cancel deferred). A pre-merge review surfaced and fixed a **BLOCKER secret-leak** +(the `input` handler emitted raw `secret`-typed inputs into events) by threading `secretInputNames` onto +`NodeExecContext` and masking in the input handler + the expression scope. **Tasks:** - Implement handlers for `condition` (evaluate the expression, activate exactly one From 2b19f7f7f7dc0f5193018a76526053a729841e36 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Sun, 14 Jun 2026 16:25:33 +0300 Subject: [PATCH 2/5] =?UTF-8?q?fix(core):=20deferred-tasks=20hardening=20?= =?UTF-8?q?=E2=80=94=20reject=20handle-less=20edge=20from=20a=20condition?= =?UTF-8?q?=20(1.M)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pre-1.Q triage of deferred-tasks.md. Most open items are legitimately blocked (a future workstream, an external key/license, or a recorded decision); two were actionable now. - dag.ts (1.M validation): `validateStructuralEdge` now rejects a plain (handle-less) edge whose `from` is a `condition` node — such an edge would wire a dependent the handler's `selected` never names, a silently-dead downstream. Reuses the `invalid_handle` issue; safe (no fixture/spec/test used one — conditions route via `branches` + `nodeId:when` handles). Pinned by dag.test.ts and documented in workflow-yaml-spec.md §edges. - Bookkeeping: the **secret-into-`run.outputs` runtime-taint** item (ADR-0029(c)) is closed at the source by 1.P — `buildExpressionScope` masks secret inputs, so a transform/condition/merge_fn can never read a raw secret to launder; no runtime taint needed. Checked off. deferred-tasks.md triage recorded: `ctx.*` threading is the highest-value open engine gap but is a run-lifecycle + seam change (async `resolveContext` at run start) — best folded into 1.Q/1.R, not a drive-by. Everything else stays legitimately deferred. pnpm turbo run lint typecheck test build format:check: green (577 core tests). Leakwatch: 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../reference/contracts/workflow-yaml-spec.md | 2 +- docs/roadmap/deferred-tasks.md | 23 +++++++++++++++---- packages/core/src/dag.test.ts | 15 ++++++++++++ packages/core/src/dag.ts | 12 ++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/docs/reference/contracts/workflow-yaml-spec.md b/docs/reference/contracts/workflow-yaml-spec.md index ea9c47ef..67a7721c 100644 --- a/docs/reference/contracts/workflow-yaml-spec.md +++ b/docs/reference/contracts/workflow-yaml-spec.md @@ -324,7 +324,7 @@ The cost cap is **pre-egress**: before each LLM call the engine checks `cumulati ## Edges -Edges are explicit directed connections. An edge from a `condition`/branching node uses the `nodeId:handleName` form to reference a named output handle/branch. +Edges are explicit directed connections. An edge from a `condition`/branching node uses the `nodeId:handleName` form to reference a named output handle/branch. **A plain (handle-less) edge whose `from` is a `condition` node is rejected at parse** — a `condition` routes only via its `branches[].target_node` (and the optional `nodeId:when` handle edge), so a handle-less edge from it would name a downstream the branch selection never activates (a silently dead node). Use the `nodeId:when` form, or rely on `branches[].target_node` alone (the routing edge is materialized either way). ```yaml edges: diff --git a/docs/roadmap/deferred-tasks.md b/docs/roadmap/deferred-tasks.md index 5b86be32..aee2ae41 100644 --- a/docs/roadmap/deferred-tasks.md +++ b/docs/roadmap/deferred-tasks.md @@ -250,12 +250,17 @@ Severity is the review's verified rating. Check an item off in the PR that resol - [ ] **Multi-tool result ordering in the turn core** — `dispatchToolCalls` appends tool-result messages in dispatch-completion order; for v1.0 (single tool call per `tool_use` stop) this is moot, but a parallel-tool provider should order by the accumulator's `toolOrder` before 1.V reuses the core. *(low · packages/core/src/engine/agent-turn.ts; 1.V)* -- [ ] **Secret-into-`run.outputs` runtime taint (ADR-0029(c) follow-up)** — an `agent` node cannot launder a +- [x] **Secret-into-`run.outputs` runtime taint (ADR-0029(c) follow-up)** — an `agent` node cannot launder a secret into `run.outputs` (it emits LLM text only), so this is **not** 1.O's to own; it belongs to the `transform` / sandbox node (1.P / 1.AB) that can return a secret-derived value. 1.O's only obligation is to refuse a tainted `{{ run.outputs[…] }}` reference *if* such a marker reaches it; the static parse-time `analyzeSecretTaint` gate covers the authored template graph. Record as a scoped ADR-0029 amendment when 1.P/1.AB - lands. *(medium · packages/core/src/interpolation/analyze.ts; ADR-0029(c), 1.P/1.AB)* + lands. *(medium · packages/core/src/interpolation/analyze.ts; ADR-0029(c), 1.P/1.AB)* **✅ Closed at the source + by 1.P (PR #20):** `buildExpressionScope` (scope.ts) masks `secret`-typed inputs out of the sandbox scope, so a + `transform`/`condition`/`merge_fn` reads the `{ secret, ref }` marker — never the raw secret — and therefore + cannot derive a secret value to launder into `run.outputs`. The vector is cut at the read, so no runtime taint + on the output is needed. (The only remaining secret-into-egress path is the agent prompt — tracked separately + below as a 1.O policy item, and it is provider egress, not an event-payload leak.) > **2026-06-14 (PR #18 final review follow-ups).** Confirmed by the multi-dimensional pre-merge review; > non-blocking, recorded so they aren't dropped. @@ -305,7 +310,12 @@ Severity is the review's verified rating. Check an item off in the PR that resol `merge_fn` sandbox scope (and the AgentRunner's prompt `RunScope`) currently bind `ctx: {}` — the authored `context:` namespace is not yet resolved and threaded to handlers (a `{{ctx.key}}` template still resolves, but a bare `ctx.key` JS-expression read sees `{}`). A cross-cutting change for **both** 1.O and 1.P when the - engine resolves the workflow `context:` map. *(medium · packages/core/src/engine/node-handlers/scope.ts, packages/core/src/engine/agent-runner.ts)* + engine resolves the workflow `context:` map. **NB (2026-06-14 triage):** this is not a drive-by — `resolveContext` + is **async** (a context value may `read_file`), so it needs a new **run-start async resolution step** in the + engine (with resolver capabilities + a context-resolution-failure path) plus the `NodeExecContext` seam field. + Best done as its own focused task or folded into **1.Q/1.R** (which already touch the run lifecycle); it is the + highest-value open engine gap (a bare `ctx.key` silently reads `undefined` today — a mis-route risk). *(medium · + packages/core/src/engine/engine.ts, node-handlers/scope.ts, agent-runner.ts; resolveContext is 1.L2)* - [ ] **`secret`-typed input flowing into an agent prompt (1.O parallel to the 1.P fix)** — the AgentRunner resolves `{{ inputs. }}` in a `prompt_template` against the **raw** `RunScope` (agent-runner.ts), so a `secret`-typed input interpolates raw into a USER message sent to the provider. This is provider **egress** @@ -313,13 +323,16 @@ Severity is the review's verified rating. Check an item off in the PR that resol enforces), but whether a `secret`-typed input should be silently interpolated into a prompt — vs masked / rejected at parse — is a policy call. Evaluate alongside the secret-handling story; if masked, reuse `maskSecretInputs`. *(low · packages/core/src/engine/agent-runner.ts; security-review.md)* -- [ ] **Reject a plain (handle-less) edge whose `from` is a `condition` node (1.M validation)** — a `condition` +- [x] **Reject a plain (handle-less) edge whose `from` is a `condition` node (1.M validation)** — a `condition` routes only via `branches[].target_node`/`default` (materialized edges); a separately-authored plain edge `from: ` (no `:handle`) makes its target a dependent that the handler's `selected` never names, so the run loop skip-propagates it — a silently-dead downstream rather than a parse error. Add a structural validation in `dag.ts` (`validateStructuralEdge`) rejecting a handle-less edge out of a condition (reuse `invalid_handle`, or a `condition_requires_handle` kind). Pre-existing 1.M edge-validation gap, not a 1.P - handler defect. *(low · packages/core/src/dag.ts; workflow-yaml-spec.md §edges)* + handler defect. *(low · packages/core/src/dag.ts; workflow-yaml-spec.md §edges)* **✅ Fixed (2026-06-14 + hardening pass):** `validateStructuralEdge` rejects a handle-less edge from a `condition` with an + `invalid_handle` issue (no existing fixture/spec used one — the spec routes via `branches` + `nodeId:when` + handles); pinned by `dag.test.ts` and documented in workflow-yaml-spec.md §edges. ## Schema / validation hardening diff --git a/packages/core/src/dag.test.ts b/packages/core/src/dag.test.ts index ce825362..db3b7ca4 100644 --- a/packages/core/src/dag.test.ts +++ b/packages/core/src/dag.test.ts @@ -556,6 +556,21 @@ describe('buildRunPlan — endpoint and handle validation', () => { expect(err.issues.some((i) => i.kind === 'invalid_handle')).toBe(true); }); + it('rejects a plain (handle-less) edge from a condition node (routes nowhere)', () => { + const err = expectGraphError( + doc(` id: plaincondedge + nodes: + - { id: gate, type: condition, expression: 'x', branches: [{ when: true, target_node: out }] } + - { id: out, type: output } + - { id: stray, type: output } + edges: + - { from: gate, to: stray }`), + ); + // A handle-less edge from a condition would make `stray` a dependent the branch `selected` never + // names — rejected at parse rather than left as a silently-dead node. + expect(err.issues.some((i) => i.kind === 'invalid_handle')).toBe(true); + }); + it('accepts a numeric condition handle (when value stringified)', () => { const p = plan( doc(` id: numhandle diff --git a/packages/core/src/dag.ts b/packages/core/src/dag.ts index 6c9169dd..8a9093bf 100644 --- a/packages/core/src/dag.ts +++ b/packages/core/src/dag.ts @@ -279,6 +279,18 @@ function validateStructuralEdge( } return; // a handled edge's dependency comes from branch materialization, never a second edge here } + if (fromNode?.type === 'condition') { + // A `condition` routes ONLY via `branches[].target_node` (materialized) + the `nodeId:when` handle + // edge. A plain (handle-less) edge from it would wire a dependent the handler's `selected` never + // names — a silently dead downstream the run loop always skips. Require the handle form. (The + // condition node id is safe to echo; the unconstrained handle suffix is absent on this path.) + issues.push({ + kind: 'invalid_handle', + field: locator, + message: `a plain edge from condition \`${fromBase}\` must name a branch handle (\`${fromBase}:\`); a handle-less edge from a condition routes to a node its branch selection never activates`, + }); + return; + } addEdge(fromBase, edge.to); } From d3eca05a28a0763ffbf92c7a674e3bf6aa30fab3 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Sun, 14 Jun 2026 17:19:09 +0300 Subject: [PATCH 3/5] fix(core): fold the Sonnet hardening-pass review findings (MERGE-WITH-NITS) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Sonnet review of the hardening pass returned MERGE-WITH-NITS (zero blocker/high; the secret-taint closure confirmed substantively correct). Folds the confirmed mediums/lows. - DOC-1 (medium): the handle-less-condition-edge rejection message + the workflow-yaml-spec §edges note overstated "silently dead node" — a plain edge to a node that IS a branch target is ALSO rejected, but that target IS activated (via materialization), so it's redundant, not dead. Reworded both (dag.ts message + the spec note) to cover both cases (redundant-with-a-branch-target OR a never-activated node). - T-1 (medium): added a dag.test.ts case pinning that a plain edge from a condition to its OWN branch target is also rejected — the rejection is unconditional (unlike the `parallel_of` "redundant-if-agrees" leniency); the existing test only covered the dead-downstream (stray) topology. - DOC-2 (low): run-plan.md's `invalid_handle` enumeration now lists the new plain-edge-from-condition case. - TRIAGE-02 (nit): clarified `merge_fn` → fan_in `merge_fn` in the secret-taint closure entry. Skipped (with reason): the double-push of unknown_edge_target + invalid_handle for a missing `to` (both true, harmless, parse fails regardless); assertion-strength + test-title nits (subsumed by the new test); TRIAGE-01/03/04 doc-annotation polish (the substance is already accurate). pnpm turbo run lint typecheck test build format:check: green (578 core tests). Leakwatch: 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/reference/contracts/workflow-yaml-spec.md | 2 +- docs/reference/shared-core/run-plan.md | 2 +- docs/roadmap/deferred-tasks.md | 4 ++-- packages/core/src/dag.test.ts | 15 +++++++++++++++ packages/core/src/dag.ts | 9 +++++---- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/docs/reference/contracts/workflow-yaml-spec.md b/docs/reference/contracts/workflow-yaml-spec.md index 67a7721c..47146be2 100644 --- a/docs/reference/contracts/workflow-yaml-spec.md +++ b/docs/reference/contracts/workflow-yaml-spec.md @@ -324,7 +324,7 @@ The cost cap is **pre-egress**: before each LLM call the engine checks `cumulati ## Edges -Edges are explicit directed connections. An edge from a `condition`/branching node uses the `nodeId:handleName` form to reference a named output handle/branch. **A plain (handle-less) edge whose `from` is a `condition` node is rejected at parse** — a `condition` routes only via its `branches[].target_node` (and the optional `nodeId:when` handle edge), so a handle-less edge from it would name a downstream the branch selection never activates (a silently dead node). Use the `nodeId:when` form, or rely on `branches[].target_node` alone (the routing edge is materialized either way). +Edges are explicit directed connections. An edge from a `condition`/branching node uses the `nodeId:handleName` form to reference a named output handle/branch. **A plain (handle-less) edge whose `from` is a `condition` node is rejected at parse** — a `condition` routes only via its `branches[].target_node` (and the optional `nodeId:when` handle edge), so a handle-less edge from it is either redundant with a dependency already materialized from a branch target, or it names a downstream the branch selection never activates (a silently dead node). Use the `nodeId:when` form, or rely on `branches[].target_node` alone (the routing edge is materialized either way). ```yaml edges: diff --git a/docs/reference/shared-core/run-plan.md b/docs/reference/shared-core/run-plan.md index 81194557..7d409280 100644 --- a/docs/reference/shared-core/run-plan.md +++ b/docs/reference/shared-core/run-plan.md @@ -77,7 +77,7 @@ The builder owns the structural checks the pure parser defers (it has the full n - **Cycle** — the dependency graph has a directed cycle; the message names it (`a → b → c → a`). - **Unknown edge target** — an `edges[]` endpoint, a `condition` `branches[].target_node` / `default`, or a `parallel_of` member names a node that does not exist. -- **Invalid handle** — a `nodeId:handle` edge whose source is not a `condition`, or whose handle matches no branch `when` value (the only named output handles in v1.0; `fan_out` uses plain edges). +- **Invalid handle** — a `nodeId:handle` edge whose source is not a `condition`, or whose handle matches no branch `when` value (the only named output handles in v1.0; `fan_out` uses plain edges); **also** a *plain* (handle-less) edge whose `from` is a `condition` node — a condition routes only via `branches[].target_node` + the `nodeId:when` handle form, so a handle-less edge from it is rejected (redundant with a branch target, or a node the branch selection never activates). - **Dangling ref** — an `agent_ref` resolves to no agent. Only checked when a **resolved-agent registry** is supplied (`agent_ref` resolution against the workspace registry is a host concern — the pure builder never reads files); otherwise resolution is deferred. When resolution was deferred and an `agent` vertex reaches dispatch with **no** `resolvedAgent`, the `AgentRunner` (1.O) fails the node with `code: 'validation'` naming the unresolved `agent_ref` — never a crash ([agent-runner.md](agent-runner.md)). Separately, a resolved `$ref`/registry agent's `system_prompt` is re-run through the secret-taint gate (a `$ref` agent's prompt lives in another file the pure parser never reads): a secret reaching it throws **`WorkflowSecretLeakError`** (ADR-0029(c)), exactly as for an inline agent. diff --git a/docs/roadmap/deferred-tasks.md b/docs/roadmap/deferred-tasks.md index aee2ae41..b64945a2 100644 --- a/docs/roadmap/deferred-tasks.md +++ b/docs/roadmap/deferred-tasks.md @@ -257,8 +257,8 @@ Severity is the review's verified rating. Check an item off in the PR that resol `analyzeSecretTaint` gate covers the authored template graph. Record as a scoped ADR-0029 amendment when 1.P/1.AB lands. *(medium · packages/core/src/interpolation/analyze.ts; ADR-0029(c), 1.P/1.AB)* **✅ Closed at the source by 1.P (PR #20):** `buildExpressionScope` (scope.ts) masks `secret`-typed inputs out of the sandbox scope, so a - `transform`/`condition`/`merge_fn` reads the `{ secret, ref }` marker — never the raw secret — and therefore - cannot derive a secret value to launder into `run.outputs`. The vector is cut at the read, so no runtime taint + `transform` / `condition` / fan_in `merge_fn` reads the `{ secret, ref }` marker — never the raw secret — and + therefore cannot derive a secret value to launder into `run.outputs`. The vector is cut at the read, so no runtime taint on the output is needed. (The only remaining secret-into-egress path is the agent prompt — tracked separately below as a 1.O policy item, and it is provider egress, not an event-payload leak.) diff --git a/packages/core/src/dag.test.ts b/packages/core/src/dag.test.ts index db3b7ca4..b7fa9b35 100644 --- a/packages/core/src/dag.test.ts +++ b/packages/core/src/dag.test.ts @@ -571,6 +571,21 @@ describe('buildRunPlan — endpoint and handle validation', () => { expect(err.issues.some((i) => i.kind === 'invalid_handle')).toBe(true); }); + it('rejects a plain edge from a condition even to its OWN branch target (rejection is unconditional)', () => { + const err = expectGraphError( + doc(` id: plaincondedgetotarget + nodes: + - { id: gate, type: condition, expression: 'x', branches: [{ when: true, target_node: out }] } + - { id: out, type: output } + edges: + - { from: gate, to: out }`), + ); + // Unlike `parallel_of` (a redundant fan-out edge that AGREES is allowed), a plain edge from a + // condition is ALWAYS rejected — even to the branch's own target — because routing from a condition + // must use the `nodeId:when` handle form (the dependency is materialized from `branches` regardless). + expect(err.issues.some((i) => i.kind === 'invalid_handle')).toBe(true); + }); + it('accepts a numeric condition handle (when value stringified)', () => { const p = plan( doc(` id: numhandle diff --git a/packages/core/src/dag.ts b/packages/core/src/dag.ts index 8a9093bf..ea83b54e 100644 --- a/packages/core/src/dag.ts +++ b/packages/core/src/dag.ts @@ -281,13 +281,14 @@ function validateStructuralEdge( } if (fromNode?.type === 'condition') { // A `condition` routes ONLY via `branches[].target_node` (materialized) + the `nodeId:when` handle - // edge. A plain (handle-less) edge from it would wire a dependent the handler's `selected` never - // names — a silently dead downstream the run loop always skips. Require the handle form. (The - // condition node id is safe to echo; the unconstrained handle suffix is absent on this path.) + // edge. A plain (handle-less) edge from it is always rejected: it is either redundant with a + // dependency already materialized from a branch target, or it wires a dependent the handler's + // `selected` never names (a node the run loop always skips). Require the handle form. (The condition + // node id is safe to echo; the unconstrained handle suffix is absent on this path.) issues.push({ kind: 'invalid_handle', field: locator, - message: `a plain edge from condition \`${fromBase}\` must name a branch handle (\`${fromBase}:\`); a handle-less edge from a condition routes to a node its branch selection never activates`, + message: `a plain edge from condition \`${fromBase}\` is not allowed — route via \`branches[].target_node\` or the \`${fromBase}:\` handle form; a handle-less edge from a condition is either redundant with a branch target or names a node its branch selection never activates`, }); return; } From 01b59339cd7a74a1d7a2e399537c105b238a1e0d Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Sun, 14 Jun 2026 17:27:16 +0300 Subject: [PATCH 4/5] docs(core): make the secret-output invariant explicit (Sonnet re-run findings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-run of the two socket-failed Sonnet dimensions confirmed every ✅ claim GENUINE — an exhaustive (a)–(g) trace verified the secret-taint closure is airtight (no pipe/filter unwrap, no scope bypass), and the audited resolved-items (handle-less edge, six handlers + dispatcher + secretInputNames gate, tool-output spill, Sonar fixes, concurrent-agent test, reportDuplicates) all match the code. No real defects; the actionable items are doc/comment improvements that make implicit invariants explicit. - AUDIT-9 (medium): scope.ts `outputsRecord` now documents the load-bearing secret invariant — it does NOT re-mask; `run.outputs` stays raw-secret-free solely because the `input` handler masks at the ingress and expression handlers read through `buildExpressionScope`. A future handler emitting `ctx.inputs`-derived data MUST `maskSecretInputs` first. (Defense against a future tool/human-gate/subworkflow handler.) - SEC-3 (low): sse-event-schema.md §security now states a `MaskedSecret` marker can appear in `node:completed.output` / `run:completed.outputs` (the input node emits masked inputs) and surfaces must render it as a redacted placeholder. - AUDIT-6 (nit): corrected the `readBracket` description in deferred-tasks.md (it's a thin dispatcher, not literally "3-line"). AUDIT-2 was a false positive (no `node-handlers.test.ts:618-655` citation exists in deferred-tasks.md). pnpm turbo run lint typecheck test build format:check: green (578 core tests). Leakwatch: 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/reference/contracts/sse-event-schema.md | 2 ++ docs/roadmap/deferred-tasks.md | 3 ++- packages/core/src/engine/node-handlers/scope.ts | 7 +++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/reference/contracts/sse-event-schema.md b/docs/reference/contracts/sse-event-schema.md index 552639d3..0a4fbd35 100644 --- a/docs/reference/contracts/sse-event-schema.md +++ b/docs/reference/contracts/sse-event-schema.md @@ -136,6 +136,8 @@ export interface HumanGatePausedEvent extends BaseEvent { `agent:tool_call.toolInput` is sanitized (no secrets) and `agent:tool_result.outputSummary` is truncated. `run:started.inputs` carries workflow inputs, but any **secret-typed** input is **masked** — the value is replaced with `{ secret: true, ref }` (the keychain/env reference), never the raw value. API keys and other secrets never appear in any event payload — this holds across the in-process bus, HTTP SSE, and any persisted run log. (On the desktop the raw provider key never even reaches the WebView: egress is Rust-delegated, [ADR-0018](../../decisions/0018-desktop-execution-and-rust-egress.md).) +The same `{ secret: true, ref }` **`MaskedSecret`** marker can also appear in **`node:completed.output`** (for an `input` node, which emits the masked inputs) and therefore in **`run:completed.outputs`** / **`run:failed.partialOutputs`** wherever a `secret`-typed input would otherwise surface — the engine masks `secret` inputs at the ingress so a raw secret never reaches an output payload (see [run-plan.md §output capture](../shared-core/run-plan.md)). **A surface rendering node/run outputs must treat a `MaskedSecret` object as a redacted placeholder, not displayable data.** + ## Consuming the stream The consumer pattern is identical for every surface, local or cloud: diff --git a/docs/roadmap/deferred-tasks.md b/docs/roadmap/deferred-tasks.md index b64945a2..cd23e33d 100644 --- a/docs/roadmap/deferred-tasks.md +++ b/docs/roadmap/deferred-tasks.md @@ -502,7 +502,8 @@ Severity is the review's verified rating. Check an item off in the PR that resol - [x] **`readBracket` cognitive complexity (1.L2)** — Sonar 17 > 15; extract the numeric-index vs quoted-key branches into helpers. *(critical · packages/core/src/interpolation/path.ts:96)* **✅ Fixed:** - extracted `readQuotedKey` + `readNumericIndex`; `readBracket` is now a 3-line dispatcher. + extracted `readQuotedKey` + `readNumericIndex`; `readBracket` is now a thin dispatcher that delegates + to them (cognitive complexity well under the threshold). - [x] **`splitTopLevel` cognitive complexity (1.L)** — Sonar 16 > 15; extract the quote/bracket depth-tracking into a small state helper. *(critical · packages/core/src/interpolation/references.ts:217)* **✅ Fixed:** extracted a `SplitState` + `splitStep`/`splitStepOutsideQuote` pair; the loop body is one call. diff --git a/packages/core/src/engine/node-handlers/scope.ts b/packages/core/src/engine/node-handlers/scope.ts index 14a61453..5b35e389 100644 --- a/packages/core/src/engine/node-handlers/scope.ts +++ b/packages/core/src/engine/node-handlers/scope.ts @@ -35,6 +35,13 @@ export function cancelled(): NodeOutcome { * checkpoint/resume replay — a determinism obligation the sandbox cannot enforce itself * (expression-sandbox-spec.md §run.outputs ordering; ADR-0027). `ctx.runOutputs` holds only * `completed` upstream outputs, so a skipped/failed/not-yet-run producer is simply absent. + * + * **Secret invariant (load-bearing):** this does NOT re-mask — it passes node outputs through verbatim. + * The guarantee that `run.outputs` never carries a raw `secret`-typed value rests entirely on the + * **`input` handler masking at the ingress** (io.ts `maskSecretInputs`) and on every expression handler + * reading inputs through {@link buildExpressionScope} (also masked). Any future node-type handler that + * writes `ctx.inputs`-derived data into its returned output MUST mask it (`maskSecretInputs`) first, or a + * raw secret would enter `run.outputs` here unmasked and then a `node:completed` event payload. */ export function outputsRecord(runOutputs: ReadonlyMap): Record { // A null-prototype record (defense-in-depth: a node id cannot be `__proto__` under the kebab grammar, From 5acd6f5334fa78d579387ec423cebc13c6a70d8e Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Sun, 14 Jun 2026 17:51:34 +0300 Subject: [PATCH 5/5] fix(core): guard the condition-edge check against double-reporting (PR #21 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the PR #21 review of the new handle-less-condition-edge validation. - dag.ts (the valid finding): the condition-routing check now also requires `nodesById.has(edge.to)`, so an edge with a missing `to` (or `from`) — already reported as `unknown_edge_target` — does NOT also push a redundant/confusing `invalid_handle` for the same edge. `addEdge` no-ops on a phantom endpoint, so the fall-through is safe. Pinned by a new dag.test.ts case (condition edge to a nonexistent target ⇒ only `unknown_edge_target`, never `invalid_handle`). - dag.ts: shortened the `invalid_handle` message to the actionable instruction (route via `branches[].target_node` or the `nodeId:when` handle form); the redundant-vs-dead rationale lives in the code comment + workflow-yaml-spec.md §edges, not the error string. - sse-event-schema.md: grammar fix ("A surface rendering node/run outputs" → "Any surface rendering of node/run outputs"). Skipped (with reason): a shared error-message constant (over-engineering for one call site); factoring the two condition-edge tests into a fixture (dag.test.ts's idiom is self-contained inline YAML per test — ~40 tests follow it; a 2-test helper would break that consistency for ~3 lines). pnpm turbo run lint typecheck test build format:check: green (579 core tests). Leakwatch: 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/reference/contracts/sse-event-schema.md | 2 +- packages/core/src/dag.test.ts | 15 +++++++++++++++ packages/core/src/dag.ts | 13 +++++++------ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/docs/reference/contracts/sse-event-schema.md b/docs/reference/contracts/sse-event-schema.md index 0a4fbd35..a280aade 100644 --- a/docs/reference/contracts/sse-event-schema.md +++ b/docs/reference/contracts/sse-event-schema.md @@ -136,7 +136,7 @@ export interface HumanGatePausedEvent extends BaseEvent { `agent:tool_call.toolInput` is sanitized (no secrets) and `agent:tool_result.outputSummary` is truncated. `run:started.inputs` carries workflow inputs, but any **secret-typed** input is **masked** — the value is replaced with `{ secret: true, ref }` (the keychain/env reference), never the raw value. API keys and other secrets never appear in any event payload — this holds across the in-process bus, HTTP SSE, and any persisted run log. (On the desktop the raw provider key never even reaches the WebView: egress is Rust-delegated, [ADR-0018](../../decisions/0018-desktop-execution-and-rust-egress.md).) -The same `{ secret: true, ref }` **`MaskedSecret`** marker can also appear in **`node:completed.output`** (for an `input` node, which emits the masked inputs) and therefore in **`run:completed.outputs`** / **`run:failed.partialOutputs`** wherever a `secret`-typed input would otherwise surface — the engine masks `secret` inputs at the ingress so a raw secret never reaches an output payload (see [run-plan.md §output capture](../shared-core/run-plan.md)). **A surface rendering node/run outputs must treat a `MaskedSecret` object as a redacted placeholder, not displayable data.** +The same `{ secret: true, ref }` **`MaskedSecret`** marker can also appear in **`node:completed.output`** (for an `input` node, which emits the masked inputs) and therefore in **`run:completed.outputs`** / **`run:failed.partialOutputs`** wherever a `secret`-typed input would otherwise surface — the engine masks `secret` inputs at the ingress so a raw secret never reaches an output payload (see [run-plan.md §output capture](../shared-core/run-plan.md)). **Any surface rendering of node/run outputs must treat a `MaskedSecret` object as a redacted placeholder, not displayable data.** ## Consuming the stream diff --git a/packages/core/src/dag.test.ts b/packages/core/src/dag.test.ts index b7fa9b35..123cefce 100644 --- a/packages/core/src/dag.test.ts +++ b/packages/core/src/dag.test.ts @@ -586,6 +586,21 @@ describe('buildRunPlan — endpoint and handle validation', () => { expect(err.issues.some((i) => i.kind === 'invalid_handle')).toBe(true); }); + it('does not double-report: a condition edge to a NONEXISTENT target is only unknown_edge_target', () => { + const err = expectGraphError( + doc(` id: condedgemissingtarget + nodes: + - { id: gate, type: condition, expression: 'x', branches: [{ when: true, target_node: out }] } + - { id: out, type: output } + edges: + - { from: gate, to: ghost }`), + ); + // The missing `to` is the real fault; the condition-routing check is guarded on a real target, so it + // does NOT also push a redundant/confusing invalid_handle for the same edge. + expect(err.issues.some((i) => i.kind === 'unknown_edge_target')).toBe(true); + expect(err.issues.some((i) => i.kind === 'invalid_handle')).toBe(false); + }); + it('accepts a numeric condition handle (when value stringified)', () => { const p = plan( doc(` id: numhandle diff --git a/packages/core/src/dag.ts b/packages/core/src/dag.ts index ea83b54e..051b290b 100644 --- a/packages/core/src/dag.ts +++ b/packages/core/src/dag.ts @@ -279,16 +279,17 @@ function validateStructuralEdge( } return; // a handled edge's dependency comes from branch materialization, never a second edge here } - if (fromNode?.type === 'condition') { + // Validate condition routing only when BOTH endpoints are real nodes — a missing `from`/`to` is + // already reported as `unknown_edge_target` above, so checking here too would double-report one edge. + if (fromNode?.type === 'condition' && nodesById.has(edge.to)) { // A `condition` routes ONLY via `branches[].target_node` (materialized) + the `nodeId:when` handle - // edge. A plain (handle-less) edge from it is always rejected: it is either redundant with a - // dependency already materialized from a branch target, or it wires a dependent the handler's - // `selected` never names (a node the run loop always skips). Require the handle form. (The condition - // node id is safe to echo; the unconstrained handle suffix is absent on this path.) + // edge. A plain (handle-less) edge from it is always rejected — either redundant with a dependency + // already materialized from a branch target, or it wires a dependent the handler's `selected` never + // names (a node the run loop always skips). (The condition node id is safe to echo.) issues.push({ kind: 'invalid_handle', field: locator, - message: `a plain edge from condition \`${fromBase}\` is not allowed — route via \`branches[].target_node\` or the \`${fromBase}:\` handle form; a handle-less edge from a condition is either redundant with a branch target or names a node its branch selection never activates`, + message: `a plain edge from condition \`${fromBase}\` is not allowed — route via \`branches[].target_node\` or the \`${fromBase}:\` handle form`, }); return; }