diff --git a/.changeset/workflow-column-agent-assignment.md b/.changeset/workflow-column-agent-assignment.md new file mode 100644 index 0000000000..8dbc8fb081 --- /dev/null +++ b/.changeset/workflow-column-agent-assignment.md @@ -0,0 +1,11 @@ +--- +"@runfusion/fusion": minor +--- + +Add per-column agent assignment for workflow columns, behind the combined `experimentalFeatures.workflowColumns` + `experimentalFeatures.workflowGraphExecutor` flags. + +A workflow column can now name a permanent agent from the registry plus a mode — `defer` (the column agent is the default for work in that column that carries no agent/model settings of its own) or `override` (the column agent supersedes node- and task-level agent/model settings). The binding applies to all session-running work attributable to the column's nodes: custom prompt/gate/script nodes, the execute seam's coding session, and step-execute sessions. Precedence is resolved by one shared `@fusion/core` resolver (`resolveColumnAgentBinding` + `resolveEffectiveAgent`) consumed by every reader, with defer/override expressed as explicit named rules and defer granularity all-or-nothing (an own agent identity OR a complete `modelProvider`+`modelId` pair suppresses the column agent). The binding keys off the node's declared IR column; foreach template nodes inherit the enclosing foreach node's column. A missing/deleted agent at resolution time logs and falls back to normal resolution — a live session is never aborted. The built-in default workflow carries no column agents and stays byte-identical (parity oracle); with either flag off, column agents are inert. + +The effective column agent is also the principal for the subsystems that previously assumed the running agent is always `task.assignedAgentId`: action gating (`buildActionGateContext` / `buildPermanentAgentGatingContext`) is computed for the agent actually running; heartbeat serialization honors it in both directions (the execute deferral gate, a second `resumeTaskForAgent` pass that re-dispatches tasks whose effective column agent matches, and a reverse-direction heartbeat-scheduler guard so an `allowParallelExecution=false` column agent never heartbeats concurrently with its own session); and a workflow-definition edit or agent runtimeConfig change that re-keys the column-effective agent/model hot-swaps the running graph session, while an agent deleted mid-session falls back without a restart. + +Authoring lands in the workflow editor: the column panel gains a registry-backed per-column agent picker plus a defer/override mode toggle, bound columns are badged on their headers, and a node inside an override column shows that its own executor settings are superseded (so override never reads as a bug). Picker interaction states are explicit — flags off disables the picker with a tooltip naming both required flags, an in-flight fetch disables it, a failed fetch shows an inline error, and a stored `agentId` missing from the registry renders an "Agent not found" warning that preserves the IR until the author clears or replaces it. Agent references are validated at save time: the `POST`/`PATCH` workflow routes reject an unknown `agentId` with a typed 4xx naming the offending column, and binding an agent whose permission policy is broader than the project default requires an explicit `confirmPolicyEscalation` flag so override cannot silently re-key action gates to a more-privileged agent. diff --git a/docs/plans/2026-06-04-002-feat-column-agent-assignment-plan.md b/docs/plans/2026-06-04-002-feat-column-agent-assignment-plan.md new file mode 100644 index 0000000000..4fd644a866 --- /dev/null +++ b/docs/plans/2026-06-04-002-feat-column-agent-assignment-plan.md @@ -0,0 +1,345 @@ +--- +title: "feat: Per-column agent assignment — permanent agents for workflow columns" +type: feat +status: completed +date: 2026-06-04 +depth: standard +origin: none (solo planning bootstrap; extends docs/plans/2026-06-03-003-feat-workflow-custom-columns-traits-plan.md) +--- + +# feat: Per-column agent assignment — permanent agents for workflow columns + +## Summary + +Let a workflow-defined column name a **permanent agent** from the agent registry, with a per-column mode: **defer** (column agent is the default for work in that column that carries no agent/model settings of its own) or **override** (column agent wins over node-level and task-level agent/model settings). The binding applies to all session-running work attributable to the column — custom prompt/gate/script nodes, the execute seam's coding session, and step-execute sessions — and the column-resolved agent becomes the *principal* for action gating, heartbeat deferral, and session-restart detection, not merely a model source. The built-in default workflow carries no column agents and stays byte-identical (parity oracle). + +--- + +## Problem Frame + +The columns/traits track (`docs/plans/2026-06-03-003-feat-workflow-custom-columns-traits-plan.md`, PR #1418) made columns first-class workflow IR entities with composable traits, and the step-inversion track (active on this branch) is making steps workflow-modelable. But **who does the work** in a column is still decided node-by-node or task-by-task: a custom node can set `executor: "agent"` + `agentId` in its config (`packages/engine/src/executor.ts:4546`), and a task can carry `assignedAgentId` / `modelProvider` + `modelId` — there is no way to say "everything that runs in my Review column runs as the senior-reviewer agent." + +A user authoring a workflow with specialized columns (planning, implementation, review, docs) wants to staff each column once and have every card flowing through inherit that staffing — while still being able to either respect finer-grained node settings (defer) or enforce the column's agent unconditionally (override). + +--- + +## Requirements + +**Binding & precedence** + +- R1. A workflow column can optionally name an agent from the agent registry plus a mode, `defer` or `override`. +- R2. Defer: the column agent applies only when the work carries no agent/model settings of its own — for custom nodes, no `cfg.agentId` and no `cfg.modelProvider`+`cfg.modelId` pair; for coding seams, no `task.assignedAgentId` and no `task.modelProvider`+`task.modelId` pair. Granularity is all-or-nothing: any own agent identity or complete model pair suppresses the column agent entirely. +- R3. Override: the column agent supersedes node-level and task-level agent/model settings — identity, model, and persona. +- R4. The binding keys off the node's **declared** IR column (`node.column`), never the task's current board lane. Foreach template nodes inherit the enclosing foreach node's column unless they declare their own. A node with no declared column resolves normally (no column agent), even in override mode. + +**Principal semantics** + +- R5. Under an effective column agent, action gating (`buildPermanentAgentGatingContext` / `buildActionGateContext`) is computed for the column agent — the agent actually running — not `task.assignedAgentId`. +- R6. Heartbeat deferral (`shouldDeferForHeartbeat`) and resume (`resumeTaskForAgent`) honor the effective column agent: a column agent with `allowParallelExecution=false` is serialized the same way an assigned agent is. This includes `resumeTaskForAgent`'s task-selection query (it must re-dispatch tasks whose *effective* agent matches, not only `assignedAgentId` matches) and the heartbeat scheduler's reverse-direction guards keyed on `agent.taskId`. +- R7. Column-agent-driven changes to the effective model/agent (workflow-definition edit, agent `runtimeConfig` change) hot-swap a running session with the same user-visible effect as a `task.modelProvider` change today, via save-event invalidation feeding the restart watcher (KTD-4). Agent deletion falls back without a restart storm. + +**Resilience & parity** + +- R8. A missing/deleted agent at resolution time logs and falls back to normal resolution (mirrors the existing best-effort posture at `packages/engine/src/executor.ts:4555`); a live session is never aborted because its column agent was deleted mid-flight. +- R9. The built-in default workflow is untouched: the new IR field is omitted entirely when unset (never serialized as `agent: null` / explicit defaults), v2-only-feature detection registers it, and the existing parity suites stay green. +- R10. Feature behavior requires both `experimentalFeatures.workflowColumns` and `experimentalFeatures.workflowGraphExecutor`; with either off, column agents are inert and the editor surfaces that. + +**Authoring surface** + +- R11. The workflow editor's column panel gets a per-column agent picker (registry-backed) plus a defer/override mode toggle; agent references are validated at write time with a clear error for unknown agents. Bound columns are visibly indicated, and a node inside an override column shows that its own executor settings are superseded — override must never look like a bug to the author. +- R12. New IR types are re-exported type-only from `@fusion/plugin-sdk` (`WorkflowColumnAgent`; verify whether `WorkflowIrColumn`/`WorkflowIrColumnTrait` are already reachable through the existing core re-export block and add them only if absent). +- R13. Binding an agent whose permission policy is broader than the project default requires explicit confirmation at save time — override cannot silently re-key action gates to a more-privileged agent. + +--- + +## Key Technical Decisions + +- **KTD-1 — First-class optional field on `WorkflowIrColumn`, not a trait.** Traits are board-transition policy (flags + lifecycle hooks consumed by the move machinery); the agent binding is *execution identity* consumed by the executor's session-building paths. A typed `agent?: { agentId: string; mode: "defer" | "override" }` field gets schema validation, plugin-sdk type parity, and a purpose-built picker UI — a trait would bury it in an opaque `config: Record` and overload the trait registry with a concept the transition machinery never reads. Follows the additive-optional-field precedent of `artifacts?`/`fields?` on `WorkflowIrV2`. + +- **KTD-2 — One shared resolver in `@fusion/core`; defer/override are explicit named rules, never a `??` collapse.** A single `resolveColumnAgentBinding(ir, nodeId)` (declared-column lookup + foreach template inheritance) and an effective-agent precedence function live in core and are consumed by every reader — the three engine resolution sites and the dashboard write-validation route. Two institutional learnings drive this: the per-task auto-merge override died because the override was honored at the action site but not at the 20+ trigger-layer gates (`docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md`), and route-vs-engine predicate duplication drifted into a data-loss hazard (`docs/solutions/integration-issues/branch-group-single-pr-synthetic-id-dead-wiring.md`). + +- **KTD-3 — The effective column agent is the principal.** Several subsystems assume the running agent is `task.assignedAgentId` today: the restart watcher (`packages/engine/src/executor.ts:2060`), heartbeat deferral/resume (defined `:3031`/`:3056`; the deferral gate call is `:4723`, and `resumeTaskForAgent`'s task-selection query filters on `assignedAgentId`), the heartbeat scheduler's reverse-direction guards keyed on `agent.taskId` (`packages/engine/src/agent-heartbeat.ts`), and permanent-agent action gating (`:1515`, `:1581`). Under override, the agent actually running differs from `assignedAgentId` — computing permission gates for the wrong principal is a security boundary error, and bypassing `allowParallelExecution=false` violates the agent's own contract. All of these must consult the effective agent. The gating-context builders already accept an `Agent` object parameter (callers resolve and pass it in), so principal substitution there is a call-site object swap, not gating-internals surgery — the real risk is resolving the right agent per session and closing the resume/heartbeat reverse-mapping gaps (U5). + +- **KTD-4 — Mid-flight edits hot-swap via save-event invalidation; no new edit guard.** The existing restart watcher diffs cached *task* fields — a workflow-definition edit or agent `runtimeConfig` change mutates nothing it observes, so "just feed the diff" is not a mechanism that exists. The primary mechanism is event-driven invalidation: workflow-definition saves and agent-config updates re-resolve the column-effective provider/model/agent into the watcher's tracked state, which then triggers the same restart path a `task.modelProvider` change does today. (Per-tick IR re-resolution is the fallback only if event hooks prove insufficient — it is hot-path-expensive, not the default.) The invalidation hook distinguishes agent-deleted (fall back per R8, no restart) from agent-changed (restart). We deliberately do not mirror `packages/core/src/node-override-guard.ts` (which blocks node-override edits while in-progress): hot-swap is the established posture for model/agent changes, and blocking workflow saves because some card somewhere is in a bound column would make workflow editing unusably brittle. Pause of the effective agent routes through heartbeat deferral (R6). + +- **KTD-5 — Defer granularity is all-or-nothing.** "Own settings" means an own agent identity OR a complete `modelProvider`+`modelId` pair; either suppresses column defer entirely. An incomplete model pair with no agent identity does not count (the existing resolver already ignores incomplete pairs — `resolveExecutorSessionModel`'s both-present semantics, `packages/engine/src/agent-session-helpers.ts:147-150`). The column agent is never blended with own settings: filling "only the missing half" would create hybrid identities (column agent's model with the task agent's persona) that are impossible to reason about in audit. + +- **KTD-6 — Persona injection follows the coding-session path, and reconciles the field drift.** The custom-node `"agent"` branch reads `agent.customInstructions` (`packages/engine/src/executor.ts:4553`) while the `Agent` type exposes `soul`/`instructionsText` (`packages/core/src/types.ts:5955-5957`) and the coding session resolves persona via `resolveInstructionsForRole` + `buildPromptLayers` (`executor.ts:5800-5840`). The column-agent path uses the typed fields consistently in both places; U3 fixes the custom-node branch to read the same fields rather than perpetuating the drift. + +- **KTD-7 — No store schema bump.** The binding lives inside the JSON-serialized workflow IR (parsed by `parseWorkflowIr`); workflow definitions are stored as blobs, so no `SCHEMA_VERSION` change is needed. Write-time validation happens in the dashboard route; read-time misses degrade gracefully (R8). + +--- + +## High-Level Technical Design + +Effective-agent resolution — one core function, three engine consumers, one dashboard consumer: + +```mermaid +flowchart TB + subgraph core["@fusion/core (new: column-agent-resolver)"] + B[resolveColumnAgentBinding\nir + nodeId → binding?] + P[resolveEffectiveAgent\nbinding × own settings → principal] + B --> P + end + subgraph engine["@fusion/engine consumers"] + N["runGraphCustomNode\n(custom prompt/gate/script nodes)"] + E["execute seam\n(single coding session)"] + S["step-execute\n(StepSessionExecutor)"] + end + D["dashboard route\n(write-time validation)"] + P --> N + P --> E + P --> S + B --> D + P --> G["principal subsystems:\naction gating · heartbeat deferral\nrestart watcher"] +``` + +Precedence per node (the two named rules): + +```mermaid +flowchart TB + A[node executes] --> C{node.column declared?\nforeach templates inherit\nthe foreach node's column} + C -->|no| F[normal resolution\nnode cfg → task → settings] + C -->|yes| H{column has agent binding?} + H -->|no| F + H -->|yes| M{mode} + M -->|override| O[column agent wins:\nidentity + model + persona\n+ gating principal] + M -->|defer| Q{work has own settings?\nagentId OR complete\nmodelProvider+modelId pair} + Q -->|yes| F + Q -->|no| O + O --> R{agent resolves\nin registry?} + R -->|yes| Z[session runs as column agent] + R -->|no| L[log + fall back] --> F +``` + +Directional guidance, refined during implementation — the prose requirements are authoritative. + +--- + +## Implementation Units + +### U1. IR schema, validation, and parity registration + +**Goal:** `WorkflowIrColumn` gains an optional, additively-validated `agent` binding that never perturbs legacy or default workflows. + +**Requirements:** R1, R9, R12 + +**Dependencies:** none + +**Files:** +- `packages/core/src/workflow-ir-types.ts` — `WorkflowColumnAgent` interface; `agent?: WorkflowColumnAgent` on `WorkflowIrColumn` +- `packages/core/src/workflow-ir.ts` — extend `validateColumns` (`:729`); register in v2-only-feature detection (`:858-878`); ensure serialization omits the field when unset +- `packages/plugin-sdk/src/index.ts` — type-only re-export `WorkflowColumnAgent`; check whether `WorkflowIrColumn`/`WorkflowIrColumnTrait` are already reachable through the existing `@fusion/core` re-export block and add them only if absent (R12) +- `packages/core/src/__tests__/workflow-ir-column-agent.test.ts` (new) + +**Approach:** Mirror the `validateFields` pattern (`workflow-ir.ts:660` — early return when absent). Validation when present: `agentId` non-empty string, `mode` one of `defer`/`override`. Additionally validate that every `node.column` reference — **including nodes inside foreach `template` subgraphs** — resolves to a declared column id, so a template node with a dangling column is a typed validation error rather than a silent no-binding no-op at runtime. Agent *existence* is not an IR-validation concern (the IR layer has no agent store) — that's write-time route validation (U6) and read-time fallback (U3/U4). + +**Patterns to follow:** `artifacts?`/`fields?` additive-optional precedent on `WorkflowIrV2`; `validateFields` early-return validator shape. + +**Test scenarios:** +- Column with `agent: { agentId: "agent-001", mode: "defer" }` parses and round-trips; field absent → parses identically to today. +- `agent` with empty `agentId`, missing `mode`, or unknown `mode` value → typed validation error naming the column id. +- v1 graph upgrade via `synthesizeDefaultColumns` produces columns with no `agent` field (absent, not null). +- Template-subgraph node with a `column` value matching no declared column id → typed validation error naming the node. +- Default workflow IR (`builtin-coding-workflow-ir.ts`) round-trips byte-identically; v2-only-feature detection flags a graph with a column agent as non-default. +- Serialization of a column whose binding was removed omits the key entirely. + +**Verification:** core IR tests green; existing `workflow-ir.test.ts`, `migration-workflow-columns.test.ts`, and the cli `plugin-sdk-export` test untouched-green. + +--- + +### U2. Core effective-agent resolver + +**Goal:** A single `@fusion/core` module owns "which agent does this node's work" — binding lookup and the two named precedence rules — so engine and dashboard can never drift. + +**Requirements:** R2, R3, R4 + +**Dependencies:** U1 + +**Files:** +- `packages/core/src/column-agent-resolver.ts` (new) +- `packages/core/src/index.ts` — export +- `packages/engine/src/workflow-graph-foreach.ts` — re-point `instanceNodeId` import to core (format ownership moves) +- `packages/core/src/__tests__/column-agent-resolver.test.ts` (new) + +**Approach:** Two pure functions. `resolveColumnAgentBinding(ir, nodeId)` resolves the node's `column` against `ir.columns` and returns the binding or undefined (a column without an `agent` field yields no binding — that, not "column undeclared," is the operative guarantee, since v1→v2 upgrade synthesizes a column for every node); for foreach instance node ids (`#:`) it resolves the *enclosing foreach node's* column, honoring a template node's own declared column when present. The instance-id format currently lives engine-side (`workflow-graph-foreach.ts` `instanceNodeId`): move `instanceNodeId` plus a paired `parseInstanceNodeId` into `@fusion/core` and re-point the engine import, so the format has exactly one owner (the route/engine predicate-drift learning). Parse defensively — split on the first `#`, then the first `:`, since `templateNodeId` is not sanitized against containing `:`. `resolveEffectiveAgent({ binding, ownAgentId, ownModelPair })` implements R2/R3 as explicit branches (per the auto-merge-override learning: distinct named rules, no effective-value `??` collapse) and returns a discriminated result (`column-agent` | `own-settings` | `none`) so callers and audit logs can state *why* an agent was chosen. + +**Test scenarios:** +- Override × own settings present → column agent. Override × no own settings → column agent. +- Defer × own agentId only → own settings win. Defer × complete own model pair only → own settings win. Defer × lone provider with no modelId and no agentId → column agent wins (an incomplete pair does not count as own settings, matching `resolveExecutorSessionModel`'s both-present rule, KTD-5). Pin all three explicitly. +- No `node.column` → no binding, even when other columns carry override agents. +- Foreach instance id resolves to the foreach node's column; template node with its own `column` wins over inheritance. +- Two tasks differing only in column binding diverge (the divergence-assertion pattern from the auto-merge learning). + +**Verification:** resolver tests enumerate the full mode × own-settings matrix; no engine import in the module (core stays DI-clean). + +--- + +### U3. Custom-node resolution honors the column binding + +**Goal:** Prompt/gate/script/skill nodes in a bound column run as the column agent per mode. + +**Requirements:** R2, R3, R4, R8 + +**Dependencies:** U2 + +**Files:** +- `packages/engine/src/executor.ts` — `runGraphCustomNode` (`:4498-4644`) +- `packages/engine/src/__tests__/executor-column-agent-custom-node.test.ts` (new) + +**Approach:** The IR is *not* in scope inside `runGraphCustomNode` — resolve the column binding in the `runCustomNode` seam wiring (`executor.ts:3327`, where the graph runner's callbacks are constructed and the resolved IR is available) and pass the binding into `runGraphCustomNode` as a parameter; if resolution must happen inside instead, use `resolveWorkflowIrForTask` with the `hold-release.ts` irCache pattern — never an uncached per-node store fetch. On `column-agent`: fetch via `agentStore.getAgent` (best-effort, log + fall back on null — same posture as `:4555`), adopt `runtimeConfig.executorProvider/executorModelId` and persona, and emit a `logEntry` naming the substitution and mode (e.g., "running as column agent X (override)") so the audit trail explains who ran and why — mirroring the `:4556` fallback-log pattern. Override replaces the node's own `agentId`/model/persona wholesale; defer fires only when the resolver said so. Persona uses the typed `soul`/`instructionsText` fields and this unit fixes the existing `customInstructions` drift (KTD-6). `executorKind: "cli"`/`"skill"` nodes keep their execution mechanics; the column agent contributes model/persona where a session runs (skill prompt sessions), and is a no-op for raw CLI script execution — log the skip so audit explains it. + +**Patterns to follow:** the existing `"agent"` branch at `executor.ts:4546-4560` (model adoption + persona prepend + best-effort fallback). + +**Test scenarios:** +- Override column: node with its own `cfg.agentId` runs as the column agent (model + persona from column agent asserted on the synthesized `WorkflowStep`), and the task log records the substitution and mode. +- Defer column: node with own `cfg.agentId` keeps it; bare node adopts the column agent. +- Missing column agent in registry → logged, node falls back to its own/default resolution, node still executes. +- Node with no declared column in a graph that has bound columns → untouched resolution. +- CLI-executor node in an override column → mechanics unchanged, audit log notes the skip. + +**Verification:** new tests green; existing `workflow-graph-executor-handlers.test.ts` and `workflow-node-handlers.test.ts` untouched-green. + +--- + +### U4. Coding seams: execute + step-execute sessions + +**Goal:** The main coding session and per-step sessions run as the column agent when the seam node's column is bound — the "does whatever work for that column's steps" half. + +**Requirements:** R2, R3, R4, R8 + +**Dependencies:** U2 + +**Files:** +- `packages/engine/src/executor.ts` — execute-seam session build (`:5649-5767`), step-session branch (`:5154-5183`), graph seam wiring (`:4203-4223`) +- `packages/engine/src/step-session-executor.ts` — model/agent resolution (`:985-1021`) +- `packages/engine/src/agent-session-helpers.ts` — only if the effective-agent input needs threading into `resolveExecutorSessionModel` callers +- `packages/engine/src/__tests__/executor-column-agent-seams.test.ts` (new) + +**Approach:** At the graph seams the executor knows the seam node and the resolved IR. Resolve the effective agent once per seam invocation; when it yields `column-agent`, substitute that agent where `assignedAgentId`'s agent flows today — `resolveExecutorSessionModel`'s `assignedAgentRuntimeConfig` argument, `extractRuntimeHint`, persona via `resolveInstructionsForRole`/`buildPromptLayers`, memory tools, and the session's `agentId` attribution in `StepSessionExecutor`. Adoption is audited via `logEntry` at the seam (same contract and wording shape as U3). Defer mode maps onto the resolver verdict computed from `task.assignedAgentId` + `task.modelProvider/modelId`. Foreach instances inherit the foreach node's column (resolver handles id parsing, U2). Flag-OFF and legacy (non-graph) execution never reach this code path — the legacy executor doesn't read `node.column` at all, preserving R10 structurally. + +**Execution note:** characterization-first — pin the current `assignedAgentId` session-identity behavior for both seams before introducing the substitution, so the no-binding path is provably byte-identical. + +**Test scenarios:** +- Execute seam, override column, task with `assignedAgentId` Y → session built with column agent X's model/persona/identity; audit shows the `column-agent` reason. +- Execute seam, defer column, task with complete `modelProvider/modelId` → task settings win. +- Step sessions: foreach template `step-execute` node inherits the foreach node's bound column; each instance session carries the column agent's identity (`agentId` attribution asserted). +- No binding anywhere → session construction byte-identical to the pinned characterization (parity). +- Column agent missing from registry at seam time → fallback to `assignedAgentId` path, logged, run proceeds. +- Integration scenario (per the plugin-skills learning — prove with a real resolver, not a scripted session): a real session-build path carries the column agent's `executorProvider/executorModelId` end-to-end into `createResolvedAgentSession` options. + +**Verification:** new seam tests green; `step-session-executor.test.ts`, `agent-session-helpers.test.ts`, and `workflow-graph-executor-parity.test.ts` untouched-green. + +--- + +### U5. Principal alignment: gating, heartbeat deferral, restart watcher + +**Goal:** The three subsystems that assume "the running agent is `task.assignedAgentId`" consult the effective column agent instead, closing the security and serialization gaps. + +**Requirements:** R5, R6, R7 + +**Dependencies:** U4 + +**Files:** +- `packages/engine/src/executor.ts` — restart watcher (`:2060-2090`), `shouldDeferForHeartbeat` (defined `:3031`; the deferral gate call site that must consult the effective principal is `:4723`), `resumeTaskForAgent` (defined `:3056` — both its gate input AND its task-selection query change), gating-context builders (`:1515`, `:1581`) +- `packages/engine/src/agent-heartbeat.ts` — reverse-direction `agent.taskId` parallel-execution guards +- `packages/engine/src/__tests__/executor-column-agent-principal.test.ts` (new) + +**Approach:** Introduce `resolveEffectivePrincipal(task, resolvedBinding)` — **session-scoped**, receiving the binding already computed by the U2 resolver for the specific governing node (not a task-wide lookup), returning the principal (column agent when the binding governs, else `assignedAgentId`). Feed it to: (a) `buildPermanentAgentGatingContext`/`buildActionGateContext` — both already accept an `Agent` object, so this is a call-site object swap at the session-build sites; (b) heartbeat serialization in **both directions**: the deferral gate at `:4723` consults the effective principal, `resumeTaskForAgent`'s task-selection query gains a second pass that re-dispatches tasks whose effective column agent matches (after the existing `assignedAgentId` filter), and the heartbeat scheduler's `agent.taskId`-keyed guards in `agent-heartbeat.ts` learn that an agent may be effectively executing column-bound tasks it is not assigned to — otherwise an `allowParallelExecution=false` column agent heartbeats concurrently with its own override session; (c) the restart watcher via save-event invalidation (KTD-4): workflow-definition saves and agent-config updates re-resolve the column-effective provider/model/agent into the watcher's tracked state, distinguishing agent-deleted (fall back per R8, no restart) from agent-changed (restart). Per-node resolution means a task may have >1 effective agent across concurrent split-branch sessions — deferral/gating evaluate per session, not per task. + +**Test scenarios:** +- Override column, task assigned to Y, column agent X with `allowParallelExecution=false` and an active heartbeat run → execute defers; `resumeTaskForAgent(X)` re-dispatches it via the effective-agent pass (the `assignedAgentId` filter alone would miss it — assert the second pass fires). +- Reverse direction: agent X (`allowParallelExecution=false`) is executing an override-column task it is not assigned to → X's heartbeat timer does not fire concurrently. +- Action gating context built for X (not Y) when the column binding governs; built for Y when no binding. +- Workflow edit changes the column's agent while a session runs → restart watcher fires (mirrors the existing model-change restart assertion shape at `executor.ts:2062-2075` tests). +- Column agent deleted mid-session → no restart-storm, session finishes, next resolution falls back (R8). +- Split branches with different bound columns → two sessions, two principals, each gated independently. + +**Verification:** principal tests green; no regression in existing heartbeat/gating suites (`agent-*` engine tests). + +--- + +### U6. Dashboard: column agent picker, mode toggle, write-time validation + +**Goal:** Workflow authors staff a column from the editor; invalid agent references are rejected at save. + +**Requirements:** R10, R11 + +**Dependencies:** U1 + +**Files:** +- `packages/dashboard/app/components/WorkflowColumnPanel.tsx` — agent picker + defer/override toggle per column, bound-column indicator +- `packages/dashboard/app/components/WorkflowNodeEditor.tsx` — "overridden by column agent" note on nodes in override columns; stale-agentId treatment shared with the column picker +- `packages/dashboard/src/routes/register-workflow-routes.ts` — extend the POST `/api/workflows` and PATCH `/api/workflows/:id` handlers: `assertColumnAgentsExist(ir, agentStore)` helper parallel to `assertCodeNodesCompile`, plus the policy-escalation confirmation (R13) +- `packages/dashboard/src/__tests__/workflow-routes.test.ts` — extend +- `packages/dashboard/src/routes/__tests__/board-workflows.test.ts` — extend if column payloads surface there + +**Approach:** Mirror the `fetchAgents()` dropdown pattern from `WorkflowNodeEditor.tsx:560-571, 800-803`, loading eagerly on panel mount. Picker renders "(none)" + registry agents; selecting one reveals the defer/override toggle (default `defer` — the less surprising mode). Interaction states are specified, not implementer-invented: **flags off** → disabled (not hidden) with a tooltip naming both required flags, matching the existing `readOnly` title-hint pattern (`WorkflowColumnPanel.tsx:113-115`); **fetch in flight** → picker disabled; **fetch failed** → inline error on the picker, not only a toast; **stored `agentId` absent from the registry** → render "Agent not found — \" warning instead of a blank select, preserving the IR until the author explicitly clears or replaces it (apply the same stale-id treatment to the node-level picker). **Override visibility (R11):** a bound column shows the agent name/badge on its header, and a node inside an override column shows an "overridden by column agent" note beside its own executor settings — without this, authors diagnose override as a bug. **Write-time validation (R13):** `assertColumnAgentsExist` returns a typed 4xx naming the column for unknown agents; when the bound agent's `permissionPolicy` is broader than the project default, the save requires an explicit `confirmPolicyEscalation` flag in the request body so override cannot silently re-key action gates to a more-privileged agent. Per the SWR-identity learning, key any selection/reset state on agent ids, not cached array identity. + +**Test scenarios:** +- Save with valid `agent` binding persists and round-trips through the definition GET. +- Save referencing an unknown `agentId` → typed 4xx naming the column; definition unchanged. +- Save binding a more-privileged agent without `confirmPolicyEscalation` → typed 4xx naming the policy gap; with the flag → persists (R13). +- Save with binding absent → stored IR has no `agent` key (omission asserted, R9). +- Stored `agentId` missing from the registry response → picker renders the not-found warning with the stale id; IR untouched until explicitly cleared (component-level). +- Node inside an override column renders the overridden-by-column-agent note (component-level). +- Flags off → picker disabled with the flag-naming hint (component-level), and the route still accepts/round-trips bindings (config is data; execution is what's gated). + +**Verification:** dashboard route tests green; manual editor check via the worktree-safe dashboard flow (`docs/solutions/` browser-testing note) if UI verification is wanted. + +--- + +### U7. Surface-enumeration test matrix, parity proof, changeset, docs + +**Goal:** Prove the invariant across every surface and both modes; document the feature. + +**Requirements:** R9, plus cross-cutting assertions for R1-R8 + +**Dependencies:** U3, U4, U5, U6 + +**Files:** +- `packages/engine/src/__tests__/workflow-graph-executor-parity.test.ts` — extend: default workflow with no bindings is byte-identical +- new matrix coverage distributed into the U3/U4/U5 test files (this unit audits completeness rather than duplicating) +- `.changeset/*.md` — minor, `@runfusion/fusion` +- docs: workflow-authoring docs section covering column agents, defer/override semantics, and the foreach inheritance rule + +**Approach:** Per FN-5893 surface enumeration, the matrix is mode (`defer`/`override`) × surface (custom node, execute seam, step-execute, heartbeat-deferred, missing-agent fallback) × own-settings (present/absent). Most cells land in U3-U5; this unit's job is the completeness audit, the parity extension, and the explicit two-tasks-differing-only-in-binding divergence test if not already present. + +**Test scenarios:** +- Matrix audit: every mode × surface × own-settings cell has an assertion somewhere (enumerate in a comment block or table in the parity test). +- Default workflow parity: graph with zero bindings produces identical observations via `compareWorkflowRunObservations`. + +**Verification:** `pnpm test` (changed) green; `pnpm lint` and `pnpm build` green; changeset present. + +--- + +## Scope Boundaries + +### Deferred to Follow-Up Work + +- **Legacy (non-graph) executor support** — column agents only act under `workflowGraphExecutor`; teaching the legacy fixed pipeline about column staffing is not planned (the legacy path is slated for post-graduation removal per the columns track). +- **Per-column agent *pools*** (multiple agents per column with load-balancing) — single agent per column this round; the IR field shape (`agent?` object) leaves room to widen. +- **Exclusive reservation semantics** — the binding is execution identity, not a scheduling reservation; the column agent can still do unrelated work. Capacity remains the `wip` trait + `AgentSemaphore`'s job. +- **Plugin-authored column agents in manifests** — plugins get the types (R12) but no manifest contribution surface for column bindings this round. + +### Outside this product's identity + +- Human assignee semantics (columns "assigned" to people, approvals routing) — agents only; human gates remain the `human-review` trait's territory. + +--- + +## Risks & Dependencies + +- **Step-inversion track is active on this branch.** U4 touches the same seam code (`step-execute`, `StepSessionExecutor`) that track is building. Sequence this plan's U4 after the step-inversion units that establish `runTaskStep` land, or coordinate in the same PR series — implementer should check branch state at execution time. +- **Principal substitution (U5) is the highest-risk unit** — it alters permission-gating identity. The characterization-first posture in U4 plus the no-binding byte-identical assertions are the guardrails; any ambiguity during implementation should resolve toward "gate as the agent actually running." +- **Restart-watcher integration** is event-driven (KTD-4): workflow-definition saves and agent-config updates are the invalidation triggers. If an event path proves unreliable, per-tick IR re-resolution is the (hot-path-expensive) fallback — a contained implementation decision inside U5. Note the weaker guarantee either way: a stale session restarts on the *event*, not on an arbitrary-time diff. + +--- + +## Sources & Research + +- Node-level agent adoption template: `packages/engine/src/executor.ts:4546-4560`; canonical model precedence: `packages/engine/src/agent-session-helpers.ts:134-164`. +- Column IR + validation: `packages/core/src/workflow-ir-types.ts:103-148`, `packages/core/src/workflow-ir.ts:660, 729-798, 858-878`. +- Graph executor never reads `node.column` today (confirmed by sweep) — the binding lookup is net-new plumbing at the seams, not a change to walk routing. +- Editor patterns: `WorkflowNodeEditor.tsx` agent dropdown (`:560-571, 800-803`); `WorkflowColumnPanel.tsx` (traits-only today). +- Institutional learnings applied: per-task auto-merge override trigger-gap (`docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md`), route/engine predicate drift (`docs/solutions/integration-issues/branch-group-single-pr-synthetic-id-dead-wiring.md`), registry-declared-but-unwired no-op (`docs/solutions/integration-issues/plugin-bundled-skills-not-loading-in-interactive-sessions.md`), SSE/store enrichment authority (`docs/solutions/logic-errors/queued-chat-message-flush-trusts-stale-isgenerating.md`), SWR identity churn (`docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`). diff --git a/docs/residual-review-findings/feat-column-agent-assignment.md b/docs/residual-review-findings/feat-column-agent-assignment.md new file mode 100644 index 0000000000..4ca692eb67 --- /dev/null +++ b/docs/residual-review-findings/feat-column-agent-assignment.md @@ -0,0 +1,16 @@ +# Residual Review Findings — feat/column-agent-assignment + +Source: ce-code-review autofix run `20260605-003401-136dbfd5` (artifact: `/tmp/compound-engineering/ce-code-review/20260605-003401-136dbfd5/`), reviewing the column-agent-assignment feature against `docs/plans/2026-06-04-002-feat-column-agent-assignment-plan.md`. + +All other actionable findings from the review (validated P1 correctness bugs, R13 agent-path gate bypass, reliability guards, test gaps — findings #1-#10, #12-#18) were fixed on-branch in commit `fix(review): apply autofix feedback` before PR creation. One design-level residual was deferred to the tracker: + +## Residual Review Findings + +- **[P2]** `packages/dashboard/src/routes/register-workflow-routes.ts:119` — Column-agent policy-escalation gate is save-time-only (TOCTOU): broadening a bound agent's policy (or narrowing the project default) after save silently escalates the substituted principal with no re-confirmation. Filed: https://github.com/Runfusion/Fusion/issues/1431 + +## Advisory notes (report-only, no action required) + +- `confirmPolicyEscalation` is a transient per-request flag; no persisted record of which policy state was confirmed. +- KTD-4 hot-swap covers execute-seam sessions; step-session tasks are not hot-swapped mid-flight (documented limitation). +- `resumeTaskForAgent` pass-2 performs sequential per-candidate IR resolution; consider a fast-path skip when no bindings are active if it shows up in profiles. +- `effectiveColumnAgentByTask` is per-executor-instance while `graphRouting` is process-static — a second `TaskExecutor` instance would not see the first's column-bound sessions in the heartbeat reverse guard. diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index b093b0723f..448644370b 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -61,6 +61,36 @@ FN-5769 evaluated whether those conventions required a `1.1.0` schema bump and r The `workflowColumns` track introduces **IR v2** (`version: "v2"`), where a workflow additionally defines its own **columns** (`{ id, name, traits: [{ trait, config }] }`), places nodes in columns (`node.column`), and gains `hold`, `split`, and `join` node kinds. Columns become first-class, workflow-defined task state carrying composable **traits** (declarative flags + lifecycle hooks); this generalizes the fixed pipeline + the `gateMode` semantics documented below into per-column trait configuration. v1 graphs still parse and upgrade by synthesizing default-workflow columns. The column/trait model — the trait vocabulary, the substrate/policy line, the transition authority, and the graduation gate — is documented in **`docs/architecture.md` § 9 "Workflow-defined columns & traits"** and the **Concepts** glossary (column, trait, lane, hold node, split/join, default workflow, `transitionPending`). The whole v2 model is gated behind `experimentalFeatures.workflowColumns`; with the flag off, the v1 IR and the quality-gate `WorkflowStep` model below are unchanged. +### Workflow IR v2 — per-column agent assignment + +A v2 column can optionally name a **permanent agent** from the agent registry, staffing every card that flows through it once instead of node-by-node or task-by-task. The binding is a first-class optional field on the column (not a trait — traits are board-transition policy; this is execution identity): + +```ts +{ id: "review", name: "Review", traits: [], + agent: { agentId: "agent-001", mode: "defer" | "override" } } +``` + +**Binding shape.** `agent.agentId` is a non-empty registry agent id; `agent.mode` is `defer` or `override`. The field is omitted entirely when unset — a column with no `agent` key yields no binding, and the built-in default workflow carries none (it stays byte-identical, the parity oracle). Adding a binding forces the workflow to v2. + +**Which column governs.** The binding keys off the node's **declared** IR column (`node.column`), never the task's current board lane. A node with no declared column resolves normally (no column agent), even when other columns carry override bindings. + +**`defer` vs `override`.** + +- **`defer`** — the column agent is the default *only* when the work carries no agent/model settings of its own. "Own settings" is all-or-nothing: an own agent identity **or** a complete `modelProvider`+`modelId` pair suppresses the column agent entirely. An incomplete model pair (provider with no model id) does **not** count as own settings, so the column agent still wins (matching the executor's both-present model rule). The column agent is never blended with own settings — filling only the missing half would create hybrid identities that are impossible to audit. +- **`override`** — the column agent supersedes node-level and task-level agent/model settings: identity, model, **and** persona. + +**Where it applies.** The effective agent governs all session-running work attributable to the column's nodes: custom prompt/gate/script nodes, the execute seam's coding session, and step-execute sessions. Raw CLI script nodes run no session, so the binding is a no-op there (the skip is audited). Every adoption is logged (`running as column agent '' ()`) so the audit trail explains who ran and why. + +**Foreach template inheritance.** A node inside a `foreach` template subgraph inherits the **enclosing foreach node's** column, unless the template node declares its own `column` (which then wins). Each per-step instance session is attributed to the resolved column agent. + +**Principal semantics.** The effective column agent becomes the **principal**, not merely a model source. Action gating is computed for the agent actually running (a security boundary — never `task.assignedAgentId` when an override governs). Heartbeat serialization follows it in both directions: a column agent with `allowParallelExecution=false` is serialized like an assigned agent, the engine re-dispatches tasks whose *effective* column agent matches (not only `assignedAgentId` matches), and the heartbeat scheduler never lets a column agent heartbeat concurrently with its own override session. A workflow-definition edit or agent `runtimeConfig` change that re-keys the effective agent/model hot-swaps a running session, the same way a `task.modelProvider` change does today. + +**Missing-agent fallback.** A missing or deleted agent at resolution time logs and falls back to normal resolution — a live session is never aborted because its column agent was deleted mid-flight. + +**Flag requirements.** Column agents act only when **both** `experimentalFeatures.workflowColumns` and `experimentalFeatures.workflowGraphExecutor` are on; with either off the binding is inert (config is still stored and round-trips — only execution is gated), and the editor surfaces that the picker is disabled with a tooltip naming both flags. + +**Write-time validation.** Saving a workflow validates agent references: an unknown `agentId` is rejected with a typed 4xx naming the column. Binding an agent whose permission policy is broader than the project default requires an explicit policy-escalation confirmation (`confirmPolicyEscalation`) at save time, so override cannot silently re-key action gates to a more-privileged agent. + ### Workflow IR v2 — step inversion (foreach, step-review, parse-steps, code) The **step-inversion** track makes task *steps* themselves workflow-modelable. Today the engine owns step policy end-to-end (PROMPT.md parsing, per-step review verdicts, RETHINK/REVISE control flow, merge blocking). Step inversion extracts exactly one new substrate capability — *run one step inside a task's session, and reset one step to its baseline* — and exposes everything else as authored graph structure. It is additive to IR v2 and gated by `experimentalFeatures.workflowGraphExecutor`. The default coding workflow is untouched and byte-identical (it keeps its monolithic `execute` seam and is the parity oracle); inversion is opt-in via custom workflows and a new built-in **stepwise coding workflow**. diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index a54b69519d..65e1f58474 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -1972,6 +1972,11 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: }); }, store, + // Dev-mode scheduler: no TaskExecutor runs here (engine not started), so + // neither `isTaskExecuting` nor the U5 reverse-direction + // `isAgentEffectivelyExecuting` guard has a source — both stay unwired (the + // guards simply never fire), matching the prior `isTaskExecuting` omission. + // The real wiring is the InProcessRuntime construction site. ); triggerScheduler.start(); diff --git a/packages/core/src/__tests__/column-agent-resolver.test.ts b/packages/core/src/__tests__/column-agent-resolver.test.ts new file mode 100644 index 0000000000..d8c32cc76a --- /dev/null +++ b/packages/core/src/__tests__/column-agent-resolver.test.ts @@ -0,0 +1,288 @@ +// @vitest-environment node +// +// column-agent plan U2 — the shared effective-agent resolver. +// +// Proves the full mode × own-settings matrix (KTD-2/KTD-5): +// - override × own-settings present → column agent; override × bare → column. +// - defer × own agentId → own; defer × complete model pair → own; +// defer × lone provider (incomplete pair, no agentId) → column agent wins. +// - no node.column / column without binding → own-settings or none. +// - foreach instance inheritance + template-node own column wins. +// - parseInstanceNodeId round-trip incl. templateNodeId containing ':'. +// - two graphs differing only in binding diverge. + +import { describe, expect, it } from "vitest"; +import { + instanceNodeId, + parseInstanceNodeId, + resolveColumnAgentBinding, + resolveEffectiveAgent, +} from "../column-agent-resolver.js"; +import type { + WorkflowColumnAgent, + WorkflowIrEdge, + WorkflowIrNode, + WorkflowIrV2, +} from "../workflow-ir-types.js"; + +function v2( + columns: WorkflowIrV2["columns"], + nodes: WorkflowIrNode[], + edges: WorkflowIrEdge[] = [], +): WorkflowIrV2 { + return { version: "v2", name: "test", columns, nodes, edges }; +} + +const overrideBinding: WorkflowColumnAgent = { agentId: "col-agent", mode: "override" }; +const deferBinding: WorkflowColumnAgent = { agentId: "col-agent", mode: "defer" }; + +describe("resolveEffectiveAgent — precedence matrix (U2)", () => { + it("override × own settings present → column agent", () => { + expect( + resolveEffectiveAgent({ + binding: overrideBinding, + ownAgentId: "own-agent", + ownModelProvider: "anthropic", + ownModelId: "claude-x", + }), + ).toEqual({ source: "column-agent", agentId: "col-agent" }); + }); + + it("override × bare → column agent", () => { + expect(resolveEffectiveAgent({ binding: overrideBinding })).toEqual({ + source: "column-agent", + agentId: "col-agent", + }); + }); + + it("defer × own agentId only → own settings win", () => { + expect(resolveEffectiveAgent({ binding: deferBinding, ownAgentId: "own-agent" })).toEqual({ + source: "own-settings", + }); + }); + + it("defer × complete own model pair only → own settings win", () => { + expect( + resolveEffectiveAgent({ + binding: deferBinding, + ownModelProvider: "anthropic", + ownModelId: "claude-x", + }), + ).toEqual({ source: "own-settings" }); + }); + + it("defer × lone provider (incomplete pair, no agentId) → column agent wins", () => { + // An incomplete pair does NOT count as own settings (KTD-5; matches + // resolveExecutorSessionModel's both-present rule). + expect( + resolveEffectiveAgent({ binding: deferBinding, ownModelProvider: "anthropic" }), + ).toEqual({ source: "column-agent", agentId: "col-agent" }); + }); + + it("defer × lone modelId (incomplete pair, no agentId) → column agent wins", () => { + // Symmetric incomplete-pair surface (FN-5893: assert the invariant across + // ALL known surfaces, not only the provider-only reproduction). + expect(resolveEffectiveAgent({ binding: deferBinding, ownModelId: "claude-x" })).toEqual({ + source: "column-agent", + agentId: "col-agent", + }); + }); + + it("defer × bare → column agent wins", () => { + expect(resolveEffectiveAgent({ binding: deferBinding })).toEqual({ + source: "column-agent", + agentId: "col-agent", + }); + }); + + it("no binding × own settings → own-settings", () => { + expect(resolveEffectiveAgent({ binding: undefined, ownAgentId: "own-agent" })).toEqual({ + source: "own-settings", + }); + }); + + it("no binding × bare → none", () => { + expect(resolveEffectiveAgent({ binding: undefined })).toEqual({ source: "none" }); + }); +}); + +describe("resolveColumnAgentBinding — lookup (U2)", () => { + const ir = v2( + [ + { id: "todo", name: "todo", traits: [] }, + { id: "review", name: "review", traits: [], agent: overrideBinding }, + ], + [ + { id: "start", kind: "start", column: "todo" }, + { id: "work", kind: "prompt", column: "review", config: { prompt: "do" } }, + { id: "plain", kind: "prompt", column: "todo", config: { prompt: "do" } }, + { id: "nocol", kind: "prompt", config: { prompt: "do" } }, + { id: "end", kind: "end", column: "review" }, + ], + ); + + it("resolves the bound column's agent for a node declared in it", () => { + expect(resolveColumnAgentBinding(ir, "work")).toEqual(overrideBinding); + }); + + it("returns undefined for a node in a column without a binding", () => { + expect(resolveColumnAgentBinding(ir, "plain")).toBeUndefined(); + }); + + it("returns undefined for a node with no declared column, even when other columns bind", () => { + expect(resolveColumnAgentBinding(ir, "nocol")).toBeUndefined(); + }); + + it("returns undefined for an unknown node id", () => { + expect(resolveColumnAgentBinding(ir, "ghost")).toBeUndefined(); + }); +}); + +describe("resolveColumnAgentBinding — foreach instance inheritance (U2)", () => { + function foreachIr(opts: { + foreachColumn?: string; + templateNodeColumn?: string; + reviewAgent?: WorkflowColumnAgent; + todoAgent?: WorkflowColumnAgent; + }): WorkflowIrV2 { + return v2( + [ + { id: "todo", name: "todo", traits: [], ...(opts.todoAgent ? { agent: opts.todoAgent } : {}) }, + { id: "review", name: "review", traits: [], ...(opts.reviewAgent ? { agent: opts.reviewAgent } : {}) }, + ], + [ + { id: "start", kind: "start" }, + { + id: "fe", + kind: "foreach", + ...(opts.foreachColumn ? { column: opts.foreachColumn } : {}), + config: { + source: "task-steps", + template: { + nodes: [ + { + id: "se", + kind: "prompt", + ...(opts.templateNodeColumn ? { column: opts.templateNodeColumn } : {}), + config: { seam: "step-execute" }, + }, + { id: "rev", kind: "step-review", config: { type: "code" } }, + { id: "exit", kind: "prompt" }, + ], + edges: [], + }, + }, + }, + { id: "end", kind: "end" }, + ], + ); + } + + it("instance node inherits the enclosing foreach node's column binding", () => { + const ir = foreachIr({ foreachColumn: "review", reviewAgent: overrideBinding }); + const nodeId = instanceNodeId("fe", 0, "se"); + expect(resolveColumnAgentBinding(ir, nodeId)).toEqual(overrideBinding); + }); + + it("template node's own declared column wins over inheritance", () => { + const ir = foreachIr({ + foreachColumn: "review", + reviewAgent: overrideBinding, + templateNodeColumn: "todo", + todoAgent: deferBinding, + }); + const nodeId = instanceNodeId("fe", 1, "se"); + expect(resolveColumnAgentBinding(ir, nodeId)).toEqual(deferBinding); + }); + + it("instance node with no foreach column and no template column → no binding", () => { + const ir = foreachIr({ reviewAgent: overrideBinding }); + const nodeId = instanceNodeId("fe", 0, "se"); + expect(resolveColumnAgentBinding(ir, nodeId)).toBeUndefined(); + }); + + it("skips a candidate whose templateNodeId doesn't exist under the foreach", () => { + // PR #1432 review: a bogus prefix candidate can name a real foreach while its + // parsed templateNodeId resolves to nothing — it must be skipped, not treated + // as inheriting the foreach's column. + const ir = foreachIr({ foreachColumn: "review", reviewAgent: overrideBinding }); + expect(resolveColumnAgentBinding(ir, instanceNodeId("fe", 0, "nope"))).toBeUndefined(); + }); + + it("resolves bindings when the foreach node id itself contains '#'", () => { + // The instance-id format is delimiter-ambiguous; the resolver validates each + // candidate split against real foreach nodes instead of trusting the first '#' + // (PR #1432 review). + const ir = foreachIr({ foreachColumn: "review", reviewAgent: overrideBinding }); + const fe = ir.nodes.find((n) => n.id === "fe"); + if (!fe) throw new Error("fixture foreach missing"); + fe.id = "fe#a"; + const nodeId = instanceNodeId("fe#a", 0, "se"); + expect(nodeId).toBe("fe#a#0:se"); + expect(resolveColumnAgentBinding(ir, nodeId)).toEqual(overrideBinding); + }); +}); + +describe("instanceNodeId / parseInstanceNodeId round-trip (U2)", () => { + it("round-trips a simple instance id", () => { + const id = instanceNodeId("fe", 3, "se"); + expect(id).toBe("fe#3:se"); + expect(parseInstanceNodeId(id)).toEqual({ + foreachNodeId: "fe", + stepIndex: 3, + templateNodeId: "se", + }); + }); + + it("round-trips when the templateNodeId itself contains ':'", () => { + // Defensive: split on the FIRST ':' of the remainder, keep the rest. + const id = instanceNodeId("fe", 2, "ns:inner:node"); + expect(id).toBe("fe#2:ns:inner:node"); + expect(parseInstanceNodeId(id)).toEqual({ + foreachNodeId: "fe", + stepIndex: 2, + templateNodeId: "ns:inner:node", + }); + }); + + it("returns undefined for non-instance ids", () => { + expect(parseInstanceNodeId("plain")).toBeUndefined(); + expect(parseInstanceNodeId("fe#3")).toBeUndefined(); + expect(parseInstanceNodeId("fe#:se")).toBeUndefined(); + expect(parseInstanceNodeId("fe#x:se")).toBeUndefined(); + }); +}); + +describe("two graphs differing only in binding diverge (U2)", () => { + function graph(reviewAgent?: WorkflowColumnAgent): WorkflowIrV2 { + return v2( + [ + { id: "todo", name: "todo", traits: [] }, + { id: "review", name: "review", traits: [], ...(reviewAgent ? { agent: reviewAgent } : {}) }, + ], + [ + { id: "start", kind: "start", column: "todo" }, + { id: "work", kind: "prompt", column: "review", config: { prompt: "do" } }, + { id: "end", kind: "end", column: "review" }, + ], + ); + } + + it("the effective agent diverges when only the binding differs", () => { + const bound = graph(overrideBinding); + const unbound = graph(); + // Same node, same own settings, different graph binding → different verdict. + const own = { ownAgentId: "task-agent" } as const; + const boundResult = resolveEffectiveAgent({ + binding: resolveColumnAgentBinding(bound, "work"), + ...own, + }); + const unboundResult = resolveEffectiveAgent({ + binding: resolveColumnAgentBinding(unbound, "work"), + ...own, + }); + expect(boundResult).toEqual({ source: "column-agent", agentId: "col-agent" }); + expect(unboundResult).toEqual({ source: "own-settings" }); + expect(boundResult).not.toEqual(unboundResult); + }); +}); diff --git a/packages/core/src/__tests__/workflow-ir-column-agent.test.ts b/packages/core/src/__tests__/workflow-ir-column-agent.test.ts new file mode 100644 index 0000000000..a0c73512c5 --- /dev/null +++ b/packages/core/src/__tests__/workflow-ir-column-agent.test.ts @@ -0,0 +1,224 @@ +// @vitest-environment node +// +// column-agent plan U1 — IR schema, validation, and parity registration for the +// per-column permanent-agent binding (`WorkflowIrColumn.agent`). +// +// Proves: +// - a column `agent` binding parses + round-trips; absent field parses as today. +// - typed validation errors for empty agentId / missing mode / unknown mode. +// - v1 upgrade synthesizes columns with NO `agent` field (absent, not null). +// - a template-subgraph node with a dangling `column` is a typed error. +// - the default workflow IR round-trips byte-identically; a graph carrying a +// column agent is flagged non-default (forces v2 — KTD-1/R9). +// - a removed binding omits the `agent` key entirely on serialization. + +import { describe, expect, it } from "vitest"; +import { + parseWorkflowIr, + serializeWorkflowIr, + downgradeIrToV1IfPure, + WorkflowIrError, +} from "../workflow-ir.js"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js"; +import type { + WorkflowColumnAgent, + WorkflowIrEdge, + WorkflowIrNode, + WorkflowIrV1, + WorkflowIrV2, +} from "../workflow-ir-types.js"; + +const baseColumns: WorkflowIrV2["columns"] = [ + { id: "todo", name: "todo", traits: [] }, + { id: "review", name: "review", traits: [] }, +]; + +function v2( + columns: WorkflowIrV2["columns"], + nodes: WorkflowIrNode[], + edges: WorkflowIrEdge[], + extra: Partial = {}, +): WorkflowIrV2 { + return { version: "v2", name: "test", columns, nodes, edges, ...extra }; +} + +/** start → work → end, work in the second column. */ +function simpleGraph(reviewAgent?: WorkflowColumnAgent): WorkflowIrV2 { + const columns: WorkflowIrV2["columns"] = [ + { id: "todo", name: "todo", traits: [] }, + { id: "review", name: "review", traits: [], ...(reviewAgent ? { agent: reviewAgent } : {}) }, + ]; + return v2( + columns, + [ + { id: "start", kind: "start", column: "todo" }, + { id: "work", kind: "prompt", column: "review", config: { prompt: "do" } }, + { id: "end", kind: "end", column: "review" }, + ], + [ + { from: "start", to: "work" }, + { from: "work", to: "end" }, + ], + ); +} + +describe("column-agent IR schema + validation (U1)", () => { + it("parses and round-trips a column with a defer agent binding", () => { + const ir = simpleGraph({ agentId: "agent-001", mode: "defer" }); + const parsed = parseWorkflowIr(serializeWorkflowIr(ir)) as WorkflowIrV2; + const col = parsed.columns.find((c) => c.id === "review")!; + expect(col.agent).toEqual({ agentId: "agent-001", mode: "defer" }); + }); + + it("parses identically to today when no agent field is present", () => { + const ir = simpleGraph(); + const parsed = parseWorkflowIr(serializeWorkflowIr(ir)) as WorkflowIrV2; + const col = parsed.columns.find((c) => c.id === "review")!; + expect("agent" in col).toBe(false); + }); + + it("rejects an empty agentId (typed error naming the column)", () => { + const ir = simpleGraph({ agentId: "", mode: "defer" }); + expect(() => parseWorkflowIr(ir)).toThrow(WorkflowIrError); + expect(() => parseWorkflowIr(ir)).toThrow(/column 'review'.*non-empty agentId/); + }); + + it("rejects a missing mode", () => { + const ir = simpleGraph({ agentId: "agent-001" } as unknown as WorkflowColumnAgent); + expect(() => parseWorkflowIr(ir)).toThrow(/column 'review'.*mode must be/); + }); + + it("rejects an unknown mode value", () => { + const ir = simpleGraph({ agentId: "agent-001", mode: "always" as "defer" }); + expect(() => parseWorkflowIr(ir)).toThrow(/column 'review'.*mode must be/); + }); + + it("v1 upgrade synthesizes columns with no agent field (absent, not null)", () => { + const v1: WorkflowIrV1 = { + version: "v1", + name: "legacy", + nodes: [ + { id: "start", kind: "start" }, + { id: "p", kind: "prompt", config: { prompt: "hi" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "p" }, + { from: "p", to: "end" }, + ], + }; + const upgraded = parseWorkflowIr(v1) as WorkflowIrV2; + for (const col of upgraded.columns) { + expect("agent" in col).toBe(false); + } + // And serialization carries no `agent` key at all. + expect(serializeWorkflowIr(upgraded)).not.toContain('"agent"'); + }); + + it("rejects a foreach template node whose column does not resolve (typed, names node)", () => { + const ir = v2( + baseColumns, + [ + { id: "start", kind: "start" }, + { + id: "ps", + kind: "parse-steps", + config: { artifact: "PROMPT.md", parser: "step-headings" }, + }, + { + id: "fe", + kind: "foreach", + config: { + source: "task-steps", + template: { + nodes: [ + // Dangling column reference on a template node. + { id: "se", kind: "prompt", column: "nope", config: { seam: "step-execute" } }, + { id: "rev", kind: "step-review", config: { type: "code" } }, + { id: "exit", kind: "prompt" }, + ], + edges: [ + { from: "se", to: "rev" }, + { from: "rev", to: "exit", condition: "outcome:approve" }, + { from: "rev", to: "se", condition: "outcome:revise", kind: "rework" }, + ], + }, + }, + }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "ps" }, + { from: "ps", to: "fe" }, + { from: "fe", to: "end" }, + ], + ); + expect(() => parseWorkflowIr(ir)).toThrow(/node 'se' references undefined column 'nope'/); + }); + + it("accepts a foreach template node whose column resolves to a declared column", () => { + const ir = v2( + baseColumns, + [ + { id: "start", kind: "start" }, + { + id: "ps", + kind: "parse-steps", + config: { artifact: "PROMPT.md", parser: "step-headings" }, + }, + { + id: "fe", + kind: "foreach", + column: "review", + config: { + source: "task-steps", + template: { + nodes: [ + { id: "se", kind: "prompt", column: "todo", config: { seam: "step-execute" } }, + { id: "rev", kind: "step-review", config: { type: "code" } }, + { id: "exit", kind: "prompt" }, + ], + edges: [ + { from: "se", to: "rev" }, + { from: "rev", to: "exit", condition: "outcome:approve" }, + { from: "rev", to: "se", condition: "outcome:revise", kind: "rework" }, + ], + }, + }, + }, + { id: "end", kind: "end" }, + ], + [ + { from: "start", to: "ps" }, + { from: "ps", to: "fe" }, + { from: "fe", to: "end" }, + ], + ); + expect(() => parseWorkflowIr(ir)).not.toThrow(); + }); +}); + +describe("column-agent parity registration (U1, R9)", () => { + it("default workflow IR round-trips byte-identically", () => { + const serialized = serializeWorkflowIr(BUILTIN_CODING_WORKFLOW_IR); + const reparsed = parseWorkflowIr(serialized); + expect(serializeWorkflowIr(reparsed)).toBe(serialized); + }); + + it("a graph carrying a column agent is flagged non-default (forces v2)", () => { + // A pure default-shaped graph downgrades to v1; adding an agent binding must + // keep it v2 (the v2-only-feature gate registers the field). + const bound = simpleGraph({ agentId: "agent-001", mode: "override" }); + expect(downgradeIrToV1IfPure(bound).version).toBe("v2"); + }); + + it("serialization of a column whose binding was removed omits the key entirely", () => { + const bound = simpleGraph({ agentId: "agent-001", mode: "defer" }); + const col = bound.columns.find((c) => c.id === "review")!; + delete col.agent; + const serialized = serializeWorkflowIr(bound); + expect(serialized).not.toContain('"agent"'); + const reparsed = parseWorkflowIr(serialized) as WorkflowIrV2; + expect("agent" in reparsed.columns.find((c) => c.id === "review")!).toBe(false); + }); +}); diff --git a/packages/core/src/agent-permission-policy.ts b/packages/core/src/agent-permission-policy.ts index e7d4484960..3f0d013e0a 100644 --- a/packages/core/src/agent-permission-policy.ts +++ b/packages/core/src/agent-permission-policy.ts @@ -156,3 +156,62 @@ export function resolveEffectiveAgentPermissionPolicy( rules: policy.rules, }); } + +/** + * Disposition strictness rank for column-agent policy-escalation comparison + * (R13). A LOWER rank is *broader* (more privileged): `allow` lets an action + * through unconditionally, `require-approval` gates it, `block` denies it. An + * agent whose policy is broader than the project default on ANY action category + * is an escalation that must be explicitly confirmed at save time. + */ +const DISPOSITION_BREADTH_RANK: Record = { + allow: 0, + "require-approval": 1, + block: 2, +}; + +/** + * The broadest (most-privileged) rank — used as the fallback when a category is + * absent from a policy's rules map. Treating a missing category as the broadest + * possible disposition (`allow`) ensures an absent key can never silently + * *suppress* a genuine escalation: the comparison only flags when the agent is + * at least as broad as the default, so an unknown agent-side category errs + * toward flagging, and an unknown default-side category errs toward the most + * permissive default (the conservative direction for escalation detection). + */ +const BROADEST_RANK = DISPOSITION_BREADTH_RANK.allow; + +function dispositionRank( + rules: AgentPermissionPolicyRules, + category: (typeof AGENT_PERMISSION_POLICY_ACTION_CATEGORIES)[number], +): number { + const disposition = rules[category]; + if (disposition === undefined) { + // An absent category must not suppress escalation. Treat the agent side as + // broadest (most privileged) so a missing key never narrows the comparison. + return BROADEST_RANK; + } + return DISPOSITION_BREADTH_RANK[disposition]; +} + +/** + * True when `agentPolicy`'s effective policy is broader (more privileged) than + * the project `defaultPolicy` on at least one action category (R13). + * + * Both arguments should already be resolved via + * {@link resolveEffectiveAgentPermissionPolicy}, which fills every category. The + * defensive per-category handling here guards against a partial/custom rules + * map slipping through with a missing category key — an absent key must never + * silently suppress a genuine escalation. + */ +export function isPolicyBroaderThanDefault( + agentPolicy: AgentPermissionPolicy, + defaultPolicy: AgentPermissionPolicy, +): boolean { + for (const category of AGENT_PERMISSION_POLICY_ACTION_CATEGORIES) { + const agentRank = dispositionRank(agentPolicy.rules, category); + const defaultRank = dispositionRank(defaultPolicy.rules, category); + if (agentRank < defaultRank) return true; + } + return false; +} diff --git a/packages/core/src/column-agent-binding-validation.ts b/packages/core/src/column-agent-binding-validation.ts new file mode 100644 index 0000000000..1dd0c91544 --- /dev/null +++ b/packages/core/src/column-agent-binding-validation.ts @@ -0,0 +1,104 @@ +import type { AgentStore } from "./agent-store.js"; +import type { Settings } from "./types.js"; +import type { WorkflowIr, WorkflowIrColumn } from "./workflow-ir-types.js"; +import { + isPolicyBroaderThanDefault, + resolveEffectiveAgentPermissionPolicy, +} from "./agent-permission-policy.js"; + +/** + * Typed error raised when a workflow IR binds a column to an agent that fails a + * write-time check (existence or policy escalation, R11/R13). Carries the + * offending column id and a `reason` discriminant so each write surface can map + * it to its own transport (the dashboard route → an HTTP 400; the agent tools → + * a structured tool error) without re-deriving the message. + * + * Shared between the dashboard workflow route and the `fn_workflow_create` / + * `fn_workflow_update` agent tools so both write paths enforce the SAME gate — + * an agent must not be able to persist a binding the UI would reject. + */ +export class ColumnAgentBindingError extends Error { + readonly columnId: string; + readonly agentId: string; + readonly reason: "unknown-agent" | "policy-escalation"; + + constructor(args: { + message: string; + columnId: string; + agentId: string; + reason: "unknown-agent" | "policy-escalation"; + }) { + super(args.message); + this.name = "ColumnAgentBindingError"; + this.columnId = args.columnId; + this.agentId = args.agentId; + this.reason = args.reason; + } +} + +/** + * Write-time column-agent validation (U6, R11/R13), shared by every write + * surface. Inspects an IR's columns BEFORE it is persisted and throws a typed + * {@link ColumnAgentBindingError} naming the offending column. Never mutates the + * IR and never touches the store/scheduler. + * + * Two checks per bound column: + * 1. Existence — every `column.agent.agentId` must resolve in the agent + * registry; an unknown id throws (`reason: "unknown-agent"`) so the binding + * can't be saved and silently fall back at execution time. + * 2. Policy escalation (R13) — if the bound agent's effective permission policy + * is broader (more privileged) than the project default on any action + * category, the write requires an explicit `confirmPolicyEscalation` flag, + * else it throws (`reason: "policy-escalation"`). Override must never + * silently re-key action gates to a more-privileged agent. + * + * Config is data: bindings are accepted regardless of feature flags — flags gate + * execution, not storage. A null/non-object IR or columns array is left to the + * store's own validator (this only inspects shapes it can read). + */ +export async function validateColumnAgentBindings(args: { + ir: WorkflowIr | unknown; + agentStore: AgentStore; + settings: Pick; + confirmPolicyEscalation: boolean; +}): Promise { + const { ir, agentStore, settings, confirmPolicyEscalation } = args; + const columns = (ir as { columns?: unknown })?.columns; + if (!Array.isArray(columns)) return; + const bound = (columns as WorkflowIrColumn[]).filter( + (col) => col && typeof col === "object" && col.agent && typeof col.agent.agentId === "string", + ); + if (bound.length === 0) return; + + const defaultPolicy = resolveEffectiveAgentPermissionPolicy( + undefined, + settings.defaultAgentPermissionPolicy, + ); + + for (const col of bound) { + const agentId = col.agent!.agentId; + const agent = await agentStore.getAgent(agentId); + if (!agent) { + throw new ColumnAgentBindingError({ + message: `Column '${col.id}' binds unknown agent '${agentId}'`, + columnId: col.id, + agentId, + reason: "unknown-agent", + }); + } + const agentPolicy = resolveEffectiveAgentPermissionPolicy( + agent.permissionPolicy, + settings.defaultAgentPermissionPolicy, + ); + if (isPolicyBroaderThanDefault(agentPolicy, defaultPolicy) && !confirmPolicyEscalation) { + throw new ColumnAgentBindingError({ + message: + `Column '${col.id}' binds agent '${agentId}' whose permission policy is broader than ` + + `the project default; set confirmPolicyEscalation: true to confirm`, + columnId: col.id, + agentId, + reason: "policy-escalation", + }); + } + } +} diff --git a/packages/core/src/column-agent-resolver.ts b/packages/core/src/column-agent-resolver.ts new file mode 100644 index 0000000000..21c956e2ab --- /dev/null +++ b/packages/core/src/column-agent-resolver.ts @@ -0,0 +1,219 @@ +/** + * Column-agent effective resolution (column-agent plan KTD-2). + * + * One shared resolver in `@fusion/core` consumed by every reader (the three engine + * resolution sites and the dashboard write-validation route) so engine and route + * can never drift — the route/engine predicate-drift learning + * (`docs/solutions/integration-issues/branch-group-single-pr-synthetic-id-dead-wiring.md`). + * + * Two pure functions: + * - `resolveColumnAgentBinding(ir, nodeId)` — declared-column lookup with foreach + * template inheritance — answers "which column binding (if any) governs this + * node's work?". + * - `resolveEffectiveAgent(...)` — defer/override precedence as EXPLICIT named + * branches (never a `??` effective-value collapse), per the per-task + * auto-merge-override learning + * (`docs/solutions/logic-errors/per-task-auto-merge-override-ignored-by-trigger-gates.md`). + * Returns a discriminated result so callers and audit logs can state *why* an + * agent was chosen. + * + * This module must stay DI-clean: `@fusion/core` never imports from `@fusion/engine`. + */ + +import type { WorkflowColumnAgent, WorkflowForeachConfig, WorkflowIr } from "./workflow-ir-types.js"; + +// ── Foreach instance node-id ownership (column-agent plan KTD-2) ────────────── +// The instance-id FORMAT (`#:`) now has +// exactly one owner here in core. The engine re-points its import (was +// `workflow-graph-foreach.ts`). The format itself is unchanged. + +/** Materialize a deterministic foreach instance node id (step-inversion KTD-3). + * Pure, no IR mutation. Format: `#:`. */ +export function instanceNodeId( + foreachNodeId: string, + stepIndex: number, + templateNodeId: string, +): string { + return `${foreachNodeId}#${stepIndex}:${templateNodeId}`; +} + +/** Parsed components of a foreach instance node id. */ +export interface ParsedInstanceNodeId { + foreachNodeId: string; + stepIndex: number; + templateNodeId: string; +} + +/** Parse a foreach instance node id back into its components, or `undefined` when + * `nodeId` is not in instance form. Defensive against `templateNodeId` itself + * containing `:` — split on the FIRST `#`, then the FIRST `:` of the remainder, + * and keep everything after that as the template node id. The `templateNodeId` is + * not sanitized against `:`, so a greedy/last-delimiter split would corrupt it. + * + * NOTE: a `foreachNodeId` that itself contains `#` is ambiguous under any single + * split. Callers that hold the IR should use {@link parseInstanceNodeIdCandidates} + * and validate each candidate's `foreachNodeId` against the graph (as + * `resolveColumnAgentBinding` does) instead of trusting one split position. */ +export function parseInstanceNodeId(nodeId: string): ParsedInstanceNodeId | undefined { + const hashIndex = nodeId.indexOf("#"); + if (hashIndex < 0) return undefined; + return parseInstanceNodeIdAt(nodeId, hashIndex); +} + +/** Parse treating the `#` at `hashIndex` as the instance-id delimiter. */ +function parseInstanceNodeIdAt(nodeId: string, hashIndex: number): ParsedInstanceNodeId | undefined { + const foreachNodeId = nodeId.slice(0, hashIndex); + const remainder = nodeId.slice(hashIndex + 1); + const colonIndex = remainder.indexOf(":"); + if (colonIndex < 0) return undefined; + const stepIndexRaw = remainder.slice(0, colonIndex); + const templateNodeId = remainder.slice(colonIndex + 1); + if (foreachNodeId === "" || templateNodeId === "") return undefined; + // stepIndex must be a non-negative integer; reject anything else as non-instance. + if (!/^\d+$/.test(stepIndexRaw)) return undefined; + const stepIndex = Number(stepIndexRaw); + return { foreachNodeId, stepIndex, templateNodeId }; +} + +/** Every plausible parse of `nodeId` as an instance id — one candidate per `#` + * whose suffix matches the `:` shape. The id format is ambiguous when + * node ids themselves contain `#` (e.g. foreach `f#a`, instance `f#a#0:t` — both + * the first and second `#` look like delimiters), so callers with access to the + * graph validate each candidate's `foreachNodeId` against real foreach nodes + * rather than committing to a single split position. Ordered left-to-right. */ +export function parseInstanceNodeIdCandidates(nodeId: string): ParsedInstanceNodeId[] { + const candidates: ParsedInstanceNodeId[] = []; + for (let i = nodeId.indexOf("#"); i >= 0; i = nodeId.indexOf("#", i + 1)) { + const parsed = parseInstanceNodeIdAt(nodeId, i); + if (parsed) candidates.push(parsed); + } + return candidates; +} + +// ── Binding lookup ─────────────────────────────────────────────────────────── + +/** Index a graph's top-level nodes by id (handles v1 + v2 shapes). */ +function topLevelNodesById(ir: WorkflowIr): Map { + return new Map(ir.nodes.map((n) => [n.id, n])); +} + +/** Resolve the agent binding (if any) that governs the work of `nodeId`. + * + * A column WITHOUT an `agent` field yields `undefined` — that, not "column + * undeclared," is the operative guarantee, since v1→v2 upgrade synthesizes a + * column for every node (column-agent plan KTD-2). + * + * Foreach instance ids (`#:`) resolve against the + * ENCLOSING foreach node's column, but a template node that declares its OWN + * `column` wins over inheritance (R4). */ +export function resolveColumnAgentBinding( + ir: WorkflowIr, + nodeId: string, +): WorkflowColumnAgent | undefined { + // v1 graphs have no columns and therefore no bindings. (Callers normally parse + // to v2 first, but stay defensive.) + if (ir.version !== "v2") return undefined; + + const columnsById = new Map(ir.columns.map((c) => [c.id, c])); + const bindingForColumn = (columnId: string | undefined): WorkflowColumnAgent | undefined => { + if (columnId === undefined) return undefined; + return columnsById.get(columnId)?.agent; + }; + + const nodesById = topLevelNodesById(ir); + + // Direct (top-level) node. + const direct = nodesById.get(nodeId); + if (direct) { + return bindingForColumn(direct.column); + } + + // Foreach instance node: resolve against the enclosing foreach, honoring a + // template node's own declared column. The instance-id format is ambiguous when + // node ids contain `#`, so try every plausible split and accept the first whose + // foreachNodeId names a REAL foreach node in this graph — a single fixed split + // (first-# or last-#) silently bypasses bindings for ids on the other side of + // the ambiguity (PR #1432 review). + for (const parsed of parseInstanceNodeIdCandidates(nodeId)) { + const foreachNode = nodesById.get(parsed.foreachNodeId); + if (!foreachNode || foreachNode.kind !== "foreach") continue; + + const cfg = foreachNode.config as Partial | undefined; + const templateNodes = cfg?.template?.nodes ?? []; + const templateNode = templateNodes.find((n) => n.id === parsed.templateNodeId); + // Disambiguation guard (PR #1432 review): a bogus prefix candidate can name a + // real foreach while its templateNodeId doesn't exist under it — skip it so a + // later exact parse isn't masked. A template with no nodes still inherits. + if (templateNodes.length > 0 && !templateNode) continue; + + // Template node's own column wins; otherwise inherit the foreach node's column. + if (templateNode?.column !== undefined) { + return bindingForColumn(templateNode.column); + } + return bindingForColumn(foreachNode.column); + } + return undefined; +} + +// ── Effective-agent precedence (defer / override) ──────────────────────────── + +/** Inputs to the effective-agent decision. `ownAgentId` is the work's own agent + * identity (node `cfg.agentId` or `task.assignedAgentId`); `ownModelProvider` / + * `ownModelId` are the work's own model pair (node cfg or task model fields). */ +export interface EffectiveAgentInput { + /** The binding governing this node, from `resolveColumnAgentBinding`. */ + binding: WorkflowColumnAgent | undefined; + /** The work's own agent identity, if any. */ + ownAgentId?: string; + /** The work's own model provider, if any. */ + ownModelProvider?: string; + /** The work's own model id, if any. */ + ownModelId?: string; +} + +/** Discriminated result of effective-agent resolution: callers and audit logs can + * state *why* an agent was (or was not) chosen (column-agent plan KTD-2). */ +export type EffectiveAgentResult = + | { source: "column-agent"; agentId: string } + | { source: "own-settings" } + | { source: "none" }; + +/** Does the work carry "own settings" that suppress a `defer` column agent + * (column-agent plan KTD-5)? All-or-nothing: an own agent identity OR a COMPLETE + * modelProvider+modelId pair counts. A lone provider with no modelId and no + * agentId does NOT count — matching `resolveExecutorSessionModel`'s both-present + * rule (`packages/engine/src/agent-session-helpers.ts:147-150`). */ +function hasOwnSettings(input: EffectiveAgentInput): boolean { + const hasOwnAgent = typeof input.ownAgentId === "string" && input.ownAgentId !== ""; + const hasCompletePair = + typeof input.ownModelProvider === "string" && + input.ownModelProvider !== "" && + typeof input.ownModelId === "string" && + input.ownModelId !== ""; + return hasOwnAgent || hasCompletePair; +} + +/** Decide the effective agent for a node's work using the two EXPLICIT named rules + * (column-agent plan KTD-2/KTD-5): + * - No binding → `own-settings` if the work has any, else `none`. + * - `override` → the column agent ALWAYS (identity + model + persona). + * - `defer` → the column agent ONLY when the work has no own settings; otherwise + * own settings win. + * No `??` collapse: each branch is named so audit can explain the choice. */ +export function resolveEffectiveAgent(input: EffectiveAgentInput): EffectiveAgentResult { + const { binding } = input; + + if (!binding) { + return hasOwnSettings(input) ? { source: "own-settings" } : { source: "none" }; + } + + if (binding.mode === "override") { + return { source: "column-agent", agentId: binding.agentId }; + } + + // mode === "defer": column agent only when the work carries no own settings. + if (hasOwnSettings(input)) { + return { source: "own-settings" }; + } + return { source: "column-agent", agentId: binding.agentId }; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a8d0d77420..33edfa9488 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -60,6 +60,7 @@ export type { WorkflowIrNodeKind, WorkflowIrColumn, WorkflowIrColumnTrait, + WorkflowColumnAgent, WorkflowHoldRelease, WorkflowJoinMode, WorkflowJoinBranchFailure, @@ -71,6 +72,17 @@ export type { WorkflowFieldOption, WorkflowFieldRender, } from "./workflow-ir-types.js"; +export { + instanceNodeId, + parseInstanceNodeId, + resolveColumnAgentBinding, + resolveEffectiveAgent, +} from "./column-agent-resolver.js"; +export type { + ParsedInstanceNodeId, + EffectiveAgentInput, + EffectiveAgentResult, +} from "./column-agent-resolver.js"; export { BUILTIN_CODING_WORKFLOW_IR } from "./builtin-coding-workflow-ir.js"; export { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./builtin-stepwise-coding-workflow-ir.js"; @@ -292,8 +304,13 @@ export { normalizeAgentPermissionPolicy, resolveEffectiveAgentPermissionPolicy, isAgentPermissionPolicyPresetId, + isPolicyBroaderThanDefault, } from "./agent-permission-policy.js"; export type { BuiltInAgentPermissionPolicyPreset } from "./agent-permission-policy.js"; +export { + validateColumnAgentBindings, + ColumnAgentBindingError, +} from "./column-agent-binding-validation.js"; export { AgentStore, DEFAULT_AGENT_HEARTBEAT_INTERVAL_MS } from "./agent-store.js"; export type { AgentStoreEvents } from "./agent-store.js"; export { diff --git a/packages/core/src/workflow-definition-types.ts b/packages/core/src/workflow-definition-types.ts index 60aee809e4..412bfca6c0 100644 --- a/packages/core/src/workflow-definition-types.ts +++ b/packages/core/src/workflow-definition-types.ts @@ -48,6 +48,13 @@ export interface WorkflowDefinitionUpdate { * the `workflowColumns` flag is ON. */ rehomeTo?: string; + /** + * Column-agent policy escalation (column-agent plan R13): set true to confirm + * binding a column agent whose permission policy is broader than the project + * default. Without it, the write surfaces (dashboard routes, fn_workflow_* + * tools) reject such bindings with a typed policy-escalation error. + */ + confirmPolicyEscalation?: boolean; /** * U11/KTD-13: when an IR update changes a custom field's type incompatibly for * tasks that already hold a value under that field, the update is blocked with diff --git a/packages/core/src/workflow-ir-types.ts b/packages/core/src/workflow-ir-types.ts index b96e538fb5..b3d5765367 100644 --- a/packages/core/src/workflow-ir-types.ts +++ b/packages/core/src/workflow-ir-types.ts @@ -105,11 +105,32 @@ export interface WorkflowIrColumnTrait { config?: Record; } +/** Per-column permanent-agent binding (column-agent plan KTD-1). A column may name + * one agent from the registry plus a mode that decides precedence against + * node-level / task-level agent and model settings: + * - `defer`: the column agent applies only when the work carries no own settings + * (no agent identity and no complete modelProvider+modelId pair — KTD-5). + * - `override`: the column agent supersedes node/task settings wholesale. + * This is execution identity (consumed by the executor's session-building paths), + * not a board-transition trait — hence a first-class typed field, not a trait + * config blob (KTD-1). Agent *existence* is not an IR concern (no agent store at + * this layer); it is enforced at write time (route) and falls back at read time. */ +export interface WorkflowColumnAgent { + /** Registry agent id that staffs the column. Non-empty. */ + agentId: string; + /** Precedence mode against node/task settings. */ + mode: "defer" | "override"; +} + /** A workflow-defined board column. */ export interface WorkflowIrColumn { id: string; name: string; traits: WorkflowIrColumnTrait[]; + /** Optional permanent-agent binding (column-agent plan KTD-1). Additive and + * omitted entirely when unset — never serialized as `agent: null` — so legacy + * and default workflows stay byte-identical (R9). */ + agent?: WorkflowColumnAgent; } /** Release conditions for a `hold` node (KTD-2, R3). */ diff --git a/packages/core/src/workflow-ir.ts b/packages/core/src/workflow-ir.ts index 6a3cc5eff8..793c4e30ae 100644 --- a/packages/core/src/workflow-ir.ts +++ b/packages/core/src/workflow-ir.ts @@ -271,7 +271,11 @@ function reachableFrom( * - rework edges legal only when both endpoints are inside this template; * - step-review verdict routing rules (KTD-4). */ -function validateForeach(node: WorkflowIrNode, topLevelNodeIds: Set): void { +function validateForeach( + node: WorkflowIrNode, + topLevelNodeIds: Set, + columnIds: Set, +): void { const cfg = node.config as Partial | undefined; if (!cfg || cfg.source !== "task-steps") { throw new WorkflowIrError( @@ -341,13 +345,20 @@ function validateForeach(node: WorkflowIrNode, topLevelNodeIds: Set): vo ); } - // No nested foreach. + // No nested foreach. Also: a template node's declared `column` must resolve to a + // top-level column id (column-agent plan KTD-1) — otherwise a dangling reference + // is a silent no-binding no-op at runtime instead of a typed authoring error. for (const inner of templateNodes) { if (inner.kind === "foreach") { throw new WorkflowIrError( `foreach node '${node.id}' template may not contain a nested foreach ('${inner.id}')`, ); } + if (inner.column !== undefined && !columnIds.has(inner.column)) { + throw new WorkflowIrError( + `Workflow node '${inner.id}' references undefined column '${inner.column}'`, + ); + } } // Edge endpoints must reference template nodes; rework edges must stay intra-template. @@ -742,6 +753,29 @@ function validateColumns(ir: WorkflowIrV2): void { if (!Array.isArray(column.traits)) { throw new WorkflowIrError(`Workflow IR column '${column.id}' traits must be an array`); } + validateColumnAgent(column); + } +} + +/** Validate a column's optional permanent-agent binding (column-agent plan KTD-1). + * Mirrors the `validateFields` early-return shape: absent → no-op; present → + * `agentId` must be a non-empty string and `mode` exactly `defer`/`override`. + * Agent existence is NOT checked here (no agent store at the IR layer). */ +function validateColumnAgent(column: WorkflowIrColumn): void { + const agent = column.agent; + if (agent === undefined) return; + if (!agent || typeof agent !== "object") { + throw new WorkflowIrError(`Workflow IR column '${column.id}' agent must be an object`); + } + if (typeof agent.agentId !== "string" || agent.agentId === "") { + throw new WorkflowIrError( + `Workflow IR column '${column.id}' agent must have a non-empty agentId`, + ); + } + if (agent.mode !== "defer" && agent.mode !== "override") { + throw new WorkflowIrError( + `Workflow IR column '${column.id}' agent mode must be 'defer' or 'override' (got '${String(agent.mode)}')`, + ); } } @@ -775,7 +809,7 @@ function validateV2(ir: WorkflowIrV2): void { const topLevelIds = new Set(ir.nodes.map((n) => n.id)); validateStepExecutePlacement(ir.nodes); for (const node of ir.nodes) { - if (node.kind === "foreach") validateForeach(node, topLevelIds); + if (node.kind === "foreach") validateForeach(node, topLevelIds, columnIds); } validateStepReviewRouting(ir.nodes, outgoing, nodesById, false); validateParseStepsNodes(ir); @@ -884,6 +918,9 @@ export function downgradeIrToV1IfPure(ir: WorkflowIr): WorkflowIr { if (col.id !== expectedId || col.name !== expectedId || col.traits.length !== 0) { return ir; } + // A permanent-agent binding is a v2-only feature (column-agent plan, R9): a + // graph that staffs a column can never round-trip through a pre-v2 binary. + if (col.agent !== undefined) return ir; } // Every node must sit in its default seam-derived column. A node placed diff --git a/packages/dashboard/app/components/WorkflowColumnPanel.tsx b/packages/dashboard/app/components/WorkflowColumnPanel.tsx index 10c3b3ca1a..e712a43eba 100644 --- a/packages/dashboard/app/components/WorkflowColumnPanel.tsx +++ b/packages/dashboard/app/components/WorkflowColumnPanel.tsx @@ -1,8 +1,9 @@ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; -import { Plus, Trash2, ChevronUp, ChevronDown, AlertTriangle } from "lucide-react"; -import type { WorkflowIrColumn, TraitViolation } from "@fusion/core"; -import { fetchTraits, type TraitCatalogEntry } from "../api"; +import { Plus, Trash2, ChevronUp, ChevronDown, AlertTriangle, Bot } from "lucide-react"; +import type { WorkflowIrColumn, WorkflowColumnAgent, TraitViolation } from "@fusion/core"; +import { fetchTraits, fetchAgents, type TraitCatalogEntry } from "../api"; +import type { Agent } from "../api"; import { getErrorMessage } from "@fusion/core"; import type { ToastType } from "../hooks/useToast"; @@ -16,6 +17,12 @@ interface WorkflowColumnPanelProps { readOnly: boolean; projectId?: string; addToast: (message: string, type?: ToastType) => void; + /** True only when BOTH `experimentalFeatures.workflowColumns` AND + * `experimentalFeatures.workflowGraphExecutor` are on. When false, the + * per-column agent picker is disabled (not hidden) with a hint naming both + * flags — config is data, so bindings still round-trip, but column agents are + * inert at execution time (R10). */ + columnAgentsEnabled: boolean; } let columnSeq = 0; @@ -31,9 +38,13 @@ export function WorkflowColumnPanel({ readOnly, projectId, addToast, + columnAgentsEnabled, }: WorkflowColumnPanelProps) { const { t } = useTranslation("app"); const [catalog, setCatalog] = useState([]); + const [agents, setAgents] = useState([]); + const [agentsLoading, setAgentsLoading] = useState(true); + const [agentsError, setAgentsError] = useState(null); useEffect(() => { let cancelled = false; @@ -49,6 +60,94 @@ export function WorkflowColumnPanel({ }; }, [projectId, addToast, t]); + // Eagerly load the agent registry for the per-column picker (R11). Mirrors the + // fetchTraits-on-mount pattern above (cancelled guard + toast), but ALSO keeps + // an inline error near the picker rather than only a toast, so a failed fetch + // is visible at the point of use. + useEffect(() => { + let cancelled = false; + setAgentsLoading(true); + setAgentsError(null); + // Promise.resolve guards against test mocks that return undefined. + Promise.resolve(fetchAgents(undefined, projectId)) + .then((list) => { + if (cancelled) return; + setAgents(list ?? []); + setAgentsLoading(false); + }) + .catch((err) => { + if (cancelled) return; + const message = getErrorMessage(err) || t("workflowColumns.agentsLoadFailed", "Failed to load agents"); + setAgentsError(message); + setAgentsLoading(false); + addToast(message, "error"); + }); + return () => { + cancelled = true; + }; + }, [projectId, addToast, t]); + + // Key derived agent lookups on the joined id string, never on array identity — + // SWR/dedupe can hand back a fresh array with identical ids and we must not + // churn selection/derived state on that (skill-autocomplete SWR learning). + const agentIdsKey = useMemo(() => agents.map((a) => a.id).join(","), [agents]); + const agentById = useMemo(() => { + const map = new Map(); + for (const a of agents) map.set(a.id, a); + return map; + // Keyed on the joined id string (not array identity) per the SWR-identity + // learning: a fresh array with identical ids must not churn derived state. + // (exhaustive-deps is not enforced in this package; the omission of `agents` + // from the dep array is intentional — agentIdsKey is the stable identity.) + }, [agentIdsKey]); + + const setColumnAgent = useCallback( + (id: string, agent: WorkflowColumnAgent | undefined) => { + onChange( + columns.map((c) => { + if (c.id !== id) return c; + if (!agent) { + // Clearing to "(none)" REMOVES the key entirely — never write + // `agent: null` (R9 parity: omitted-when-unset). + const { agent: _omit, ...rest } = c; + return rest; + } + return { ...c, agent }; + }), + ); + }, + [columns, onChange], + ); + + const selectColumnAgentId = useCallback( + (id: string, agentId: string) => { + if (!agentId) { + setColumnAgent(id, undefined); + return; + } + const existing = columns.find((c) => c.id === id)?.agent; + // Preserve an existing mode; default new selections to "defer" (the less + // surprising mode). + setColumnAgent(id, { agentId, mode: existing?.mode ?? "defer" }); + }, + [columns, setColumnAgent], + ); + + const setColumnAgentMode = useCallback( + (id: string, mode: "defer" | "override") => { + const existing = columns.find((c) => c.id === id)?.agent; + if (!existing) return; + setColumnAgent(id, { ...existing, mode }); + }, + [columns, setColumnAgent], + ); + + // `!!agentsError` (PR #1432 review): when the registry fetch failed, the select + // would render enabled with only "(none)" while the bound id has no matching + // option — interacting with it could silently clear a binding. Disabled while + // the registry is unavailable, consistent with the loading guard. + const agentPickerDisabled = readOnly || !columnAgentsEnabled || agentsLoading || !!agentsError; + const workflowWide = violations.filter((v) => v.columnId === null); const violationsFor = useCallback( (columnId: string) => violations.filter((v) => v.columnId === columnId), @@ -135,6 +234,16 @@ export function WorkflowColumnPanel({
    {columns.map((col, index) => { const colViolations = violationsFor(col.id); + const boundAgentId = col.agent?.agentId; + const boundAgent = boundAgentId ? agentById.get(boundAgentId) : undefined; + // A stored id that is not in the loaded registry list is "stale": + // render a not-found warning and PRESERVE the IR value until the + // author explicitly clears or replaces it (R11). + const boundAgentStale = !!boundAgentId && !agentsLoading && !agentsError && !boundAgent; + const boundAgentLabel = boundAgent?.name + ?? (boundAgentStale + ? t("workflowColumns.agentNotFound", "Agent not found — {{id}}", { id: boundAgentId ?? "" }) + : boundAgentId); return (
  • renameColumn(col.id, e.target.value)} /> + {boundAgentId && ( + + {boundAgentLabel} + + )}
    + +
    + {t("workflowColumns.agent", "Column agent")} + + + {agentsError && ( +

    + {agentsError} +

    + )} + {boundAgentStale && ( +

    + {" "} + {t("workflowColumns.agentNotFound", "Agent not found — {{id}}", { id: boundAgentId ?? "" })} +

    + )} + + {boundAgentId && ( +
    + + +
    + )} +
    • ); })} diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index 2f97816815..d40be781f1 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -34,6 +34,7 @@ import type { DiscoveredSkill } from "../api"; import type { ToastType } from "../hooks/useToast"; import { useOverlayDismiss } from "../hooks/useOverlayDismiss"; import { useModalResizePersist } from "../hooks/useModalResizePersist"; +import { useAppSettings } from "../hooks/useAppSettings"; import { workflowNodeTypes, type WorkflowFlowNodeData, type WorkflowEditorNodeKind } from "./nodes/WorkflowNodeTypes"; import { irToFlow, @@ -147,6 +148,14 @@ function InnerEditor({ const activeWorkflow = useMemo(() => workflows.find((w) => w.id === activeId), [workflows, activeId]); const isBuiltin = !!activeWorkflow && isBuiltinWorkflowId(activeWorkflow.id); + // Column-agent authoring requires BOTH flags (R10). When either is off, the + // picker is disabled (not hidden) and bound columns are inert at execution + // time; config still round-trips (flags gate execution, not storage). + const { experimentalFeatures } = useAppSettings(projectId); + const columnAgentsEnabled = + experimentalFeatures?.workflowColumns === true && + experimentalFeatures?.workflowGraphExecutor === true; + // Trait catalog (for client-side composition validation; the panel fetches its // own copy for the picker, but the editor needs the flags to validate). useEffect(() => { @@ -466,15 +475,42 @@ function InnerEditor({ columns.length ? columns : undefined, fields.length ? fields : undefined, ); - const updated = await updateWorkflow(activeWorkflow.id, { ir, layout }, projectId); - setWorkflows((ws) => ws.map((w) => (w.id === updated.id ? updated : w))); - // Validate by compiling — surfaces non-linear graphs as a banner. + const finishSave = async (updated: Awaited>) => { + setWorkflows((ws) => ws.map((w) => (w.id === updated.id ? updated : w))); + // Validate by compiling — surfaces non-linear graphs as a banner. + try { + await compileWorkflow(updated.id, projectId); + addToast(t("workflows.saved", "Workflow saved"), "success"); + } catch (compileErr) { + setValidationError( + getErrorMessage(compileErr) || t("workflows.savedNotCompilable", "Workflow saved but cannot be compiled"), + ); + } + }; try { - await compileWorkflow(updated.id, projectId); - addToast(t("workflows.saved", "Workflow saved"), "success"); - } catch (compileErr) { - setValidationError( - getErrorMessage(compileErr) || t("workflows.savedNotCompilable", "Workflow saved but cannot be compiled"), + await finishSave(await updateWorkflow(activeWorkflow.id, { ir, layout }, projectId)); + } catch (err) { + // Policy-escalation handshake (R13, PR #1432 review): the route rejects a + // binding to a broader-than-default agent until the author explicitly + // confirms. Surface the server's explanation, then retry with the flag — + // otherwise such bindings would be unsavable from the dashboard. + // Shape-checked rather than `instanceof ApiRequestError` so test doubles + // (and any error wrapper) that carry the details payload still route here. + const escalation = + (err as { details?: { policyEscalation?: boolean } } | null)?.details?.policyEscalation === true; + if (!escalation) throw err; + const proceed = window.confirm( + `${getErrorMessage(err)}\n\n${t( + "workflowColumns.confirmPolicyEscalation", + "Bind it anyway? The column agent will run with broader permissions than this project's default.", + )}`, + ); + if (!proceed) { + addToast(t("workflowColumns.escalationDeclined", "Save cancelled — column agent binding not confirmed"), "error"); + return; + } + await finishSave( + await updateWorkflow(activeWorkflow.id, { ir, layout, confirmPolicyEscalation: true }, projectId), ); } } catch (err) { @@ -550,10 +586,44 @@ function InnerEditor({ // Lazy-loaded executor resources const [models, setModels] = useState([]); const [agents, setAgents] = useState([]); + // The agent fetches are project-scoped, but this cache survives project + // switches — both load paths short-circuit on agents.length > 0, which would + // keep showing (and let the editor bind) the PREVIOUS project's registry. + // Reset on project change so the next consumer refetches (PR #1432 review). + useEffect(() => { + setAgents([]); + }, [projectId]); const [skills, setSkills] = useState([]); const currentExecutor = (selectedNode?.data.config?.executor as ExecutorKind | undefined) ?? "model"; + // The override binding governing the selected node, if any: its declared + // column carries an `agent` in `override` mode. Drives the "overridden by + // column agent" note so authors don't diagnose override as a bug (R11). Keyed + // on the column id + binding, not array identity. + const overrideColumnBinding = useMemo(() => { + // Foreach template children don't carry their own column in irToFlow — they + // inherit the enclosing foreach group's column at execution (R4). Mirror that + // inheritance here so a step-execute prompt inside an override-bound foreach + // still shows the note (PR #1432 review). + const columnId = + selectedNode?.data.column + ?? (selectedNode?.parentId + ? nodes.find((n) => n.id === selectedNode.parentId)?.data.column + : undefined); + if (!columnId) return undefined; + const col = columns.find((c) => c.id === columnId); + if (!col?.agent || col.agent.mode !== "override") return undefined; + return col.agent; + }, [selectedNode?.data.column, selectedNode?.parentId, nodes, columns]); + + // Resolve the override agent's display name from the loaded registry; when the + // id is stale (not in the list) fall back to the not-found treatment. + const overrideAgent = useMemo( + () => (overrideColumnBinding ? agents.find((a) => a.id === overrideColumnBinding.agentId) : undefined), + [overrideColumnBinding, agents], + ); + useEffect(() => { // step-review offers an optional review model picker (KTD-4). if (selectedNode?.data.kind === "step-review" && models.length === 0) { @@ -568,7 +638,10 @@ function InnerEditor({ addToast(getErrorMessage(err) || "Failed to load models", "error"); }); } else if (currentExecutor === "agent" && agents.length === 0) { - fetchAgents().then(setAgents).catch((err) => { + // Project-scoped, matching WorkflowColumnPanel's fetchAgents(undefined, + // projectId) — an unscoped fetch returns the wrong registry in + // multi-project deployments (PR #1432 review). + fetchAgents(undefined, projectId).then(setAgents).catch((err) => { addToast(getErrorMessage(err) || "Failed to load agents", "error"); }); } else if (currentExecutor === "skill" && skills.length === 0) { @@ -587,6 +660,25 @@ function InnerEditor({ skills.length, ]); + // When the selected node sits in an override column, eagerly load the agent + // registry so the "overridden by column agent " note can resolve the + // name even if this node's own executor isn't "agent". + useEffect(() => { + if (!overrideColumnBinding || agents.length > 0) return; + let cancelled = false; + // Project-scoped (PR #1432 review): without projectId this resolves from the + // wrong scope in multi-project deployments — the override note would show a + // false "not found" for a perfectly valid project agent. + Promise.resolve(fetchAgents(undefined, projectId)).then((list) => { + if (!cancelled) setAgents(list ?? []); + }).catch((err) => { + if (!cancelled) addToast(getErrorMessage(err) || "Failed to load agents", "error"); + }); + return () => { + cancelled = true; + }; + }, [overrideColumnBinding, agents.length, projectId, addToast]); + const overlayProps = useOverlayDismiss(onClose); return ( @@ -722,6 +814,7 @@ function InnerEditor({ readOnly={isBuiltin} projectId={projectId} addToast={addToast} + columnAgentsEnabled={columnAgentsEnabled} /> )} @@ -777,6 +870,19 @@ function InnerEditor({ + {overrideColumnBinding && ( +

    + {t( + "workflowColumns.overriddenByColumnAgent", + "Overridden by column agent {{name}} — this node's executor settings are superseded.", + { + name: overrideAgent?.name + ?? t("workflowColumns.agentNotFound", "Agent not found — {{id}}", { id: overrideColumnBinding.agentId }), + }, + )} +

    + )} + {currentExecutor === "model" && ( )} - {currentExecutor === "agent" && ( - - )} + {currentExecutor === "agent" && (() => { + const nodeAgentId = String(selectedNode.data.config?.agentId ?? ""); + // A stored id absent from the loaded registry would render the + // select blank; instead surface a not-found option that + // preserves the IR value until the author clears/replaces it. + const nodeAgentStale = nodeAgentId !== "" && !agents.some((a) => a.id === nodeAgentId); + return ( + + ); + })()} {currentExecutor === "skill" && (