From 295d726768e552dbb5a36e1a317be45913c9d0d5 Mon Sep 17 00:00:00 2001 From: Tom Durrant Date: Fri, 5 Jun 2026 10:07:27 +1000 Subject: [PATCH 01/21] fix: pass OPENCODE_API_KEY env var when syncing opencode-go models discoverOpencodeGoModels() spawns 'opencode models opencode --refresh' but never passed the saved API key as OPENCODE_API_KEY. The opencode CLI's internal OpencodePlugin checks this env var to decide whether to show paid models; without it, only free (cost.input === 0) models appear. Now threads the apiKey from auth storage through to the spawned process environment, so the CLI sees the user's Go subscription and returns the full model catalog including paid models like Claude, GPT-5.x, Gemini, etc. Callers in serve.ts, daemon.ts, and dashboard.ts all updated to read the key from dashboardAuthStorage and pass it to refreshOpencodeGoModels. syncStartupModels reads from authStorage in StartupSyncOptions. --- packages/cli/src/commands/daemon.ts | 2 ++ packages/cli/src/commands/dashboard.ts | 4 ++++ packages/cli/src/commands/serve.ts | 2 ++ packages/cli/src/commands/startup-model-sync.ts | 15 +++++++++++---- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/daemon.ts b/packages/cli/src/commands/daemon.ts index 27d1af4f67..ead75bf96b 100644 --- a/packages/cli/src/commands/daemon.ts +++ b/packages/cli/src/commands/daemon.ts @@ -724,9 +724,11 @@ export async function runDaemon(opts: DaemonOptions = {}) { if (settings.opencodeGoModelSync === false) { return { registeredCount: 0, reason: "disabled-by-settings" }; } + const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); return await refreshOpencodeGoModels({ modelRegistry, log: (scope, message) => console.log(`[${scope}] ${message}`), + apiKey: opencodeGoKey, }); }, getClaudeCliExtensionStatus: () => { diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index 8df4be7de2..a41f7b8055 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -1764,9 +1764,11 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: if (settings.opencodeGoModelSync === false) { return { registeredCount: 0, reason: "disabled-by-settings" }; } + const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); return await refreshOpencodeGoModels({ modelRegistry, log: (scope, message) => logSink.log(message, scope), + apiKey: opencodeGoKey, }); }, getClaudeCliExtensionStatus: () => { @@ -2085,9 +2087,11 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: if (settings.opencodeGoModelSync === false) { return { registeredCount: 0, reason: "disabled-by-settings" }; } + const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); return await refreshOpencodeGoModels({ modelRegistry, log: (scope, message) => logSink.log(message, scope), + apiKey: opencodeGoKey, }); }, getClaudeCliExtensionStatus: () => { diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 8f01b2c798..2d6a315d6c 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -831,9 +831,11 @@ export async function runServe( if (settings.opencodeGoModelSync === false) { return { registeredCount: 0, reason: "disabled-by-settings" }; } + const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); return await refreshOpencodeGoModels({ modelRegistry, log: (scope, message) => console.log(`[${scope}] ${message}`), + apiKey: opencodeGoKey, }); }, getClaudeCliExtensionStatus: () => { diff --git a/packages/cli/src/commands/startup-model-sync.ts b/packages/cli/src/commands/startup-model-sync.ts index 79d3b88220..7322d6cf5b 100644 --- a/packages/cli/src/commands/startup-model-sync.ts +++ b/packages/cli/src/commands/startup-model-sync.ts @@ -233,10 +233,15 @@ export function parseOpencodeModelsOutput(stdout: string): string[] { return [...ids]; } -export async function discoverOpencodeGoModels(): Promise { +export async function discoverOpencodeGoModels(apiKey?: string): Promise { return await new Promise((resolve, reject) => { + const env: Record = { ...process.env as Record }; + if (apiKey) { + env.OPENCODE_API_KEY = apiKey; + } const proc = spawn("opencode", ["models", "opencode", "--refresh"], { stdio: ["ignore", "pipe", "pipe"], + env, }); let stdout = ""; @@ -272,10 +277,11 @@ export async function discoverOpencodeGoModels(): Promise { export async function refreshOpencodeGoModels(options: { modelRegistry: ModelRegistryLike; log: (scope: string, message: string) => void; + apiKey?: string; }): Promise { try { - const { modelRegistry, log } = options; - const modelIds = await discoverOpencodeGoModels(); + const { modelRegistry, log, apiKey } = options; + const modelIds = await discoverOpencodeGoModels(apiKey); if (modelIds.length === 0) { log("opencode-go", "No models discovered from opencode CLI refresh"); return { registeredCount: 0, reason: "no-models-from-cli" }; @@ -310,6 +316,7 @@ export async function syncStartupModels(options: StartupSyncOptions): Promise Date: Fri, 5 Jun 2026 10:40:01 +1000 Subject: [PATCH 02/21] fix: strip provider prefix from opencode-go model IDs normalizeOpencodeGoModel was prefixing model IDs with 'opencode-go/' (e.g. 'opencode-go/deepseek-v4-flash'), but the Pi SDK sends the model id field as the model name in API requests. The OpenCode API expects bare model names (e.g. 'deepseek-v4-flash'), not prefixed ones. Now strips the 'opencode/' or 'opencode-go/' prefix entirely so the registered model ID matches what the API expects. --- packages/cli/src/commands/startup-model-sync.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/commands/startup-model-sync.ts b/packages/cli/src/commands/startup-model-sync.ts index 7322d6cf5b..ef5a1d30c2 100644 --- a/packages/cli/src/commands/startup-model-sync.ts +++ b/packages/cli/src/commands/startup-model-sync.ts @@ -205,15 +205,18 @@ async function syncOpenRouterModels(options: StartupSyncOptions, settings: Setti export function normalizeOpencodeGoModel(modelId: string): ModelConfig { const trimmed = modelId.trim(); - const normalizedId = trimmed.startsWith("opencode/") - ? `opencode-go/${trimmed.slice("opencode/".length)}` - : trimmed.startsWith("opencode-go/") - ? trimmed - : `opencode-go/${trimmed}`; + // Strip the provider prefix (opencode/ or opencode-go/) — the Pi SDK + // already routes requests by provider, and the OpenCode API expects the + // bare model name (e.g. "deepseek-v4-flash", not "opencode-go/deepseek-v4-flash"). + const bareModel = trimmed.startsWith("opencode-go/") + ? trimmed.slice("opencode-go/".length) + : trimmed.startsWith("opencode/") + ? trimmed.slice("opencode/".length) + : trimmed; return { - id: normalizedId, - name: normalizedId, + id: bareModel, + name: bareModel, reasoning: false, input: ["text"], cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, From 978d07c96caa873db07ad27f2699cb005a337ac6 Mon Sep 17 00:00:00 2001 From: Tom Durrant Date: Fri, 5 Jun 2026 11:12:40 +1000 Subject: [PATCH 03/21] Address PR review comments - Update test assertions for bare model IDs - Add deduplication guard for models with same bare ID - Add validation for empty model IDs after prefix stripping - Add test for API key forwarded as env var to spawn - Add test for deduplication and empty model ID guard - Extract shared handleOpencodeGoApiKeySaved helper - Add changeset for the published package --- .changeset/fix-opencode-go-api-key-env.md | 15 +++++ .../__tests__/startup-model-sync.test.ts | 56 +++++++++++++++++-- packages/cli/src/commands/daemon.ts | 16 ++---- packages/cli/src/commands/dashboard.ts | 30 ++++------ packages/cli/src/commands/serve.ts | 16 ++---- .../cli/src/commands/startup-model-sync.ts | 34 ++++++++++- 6 files changed, 123 insertions(+), 44 deletions(-) create mode 100644 .changeset/fix-opencode-go-api-key-env.md diff --git a/.changeset/fix-opencode-go-api-key-env.md b/.changeset/fix-opencode-go-api-key-env.md new file mode 100644 index 0000000000..d43e43e884 --- /dev/null +++ b/.changeset/fix-opencode-go-api-key-env.md @@ -0,0 +1,15 @@ +--- +"@runfusion/fusion": patch +--- + +Fix opencode-go model sync: pass API key to CLI and strip provider prefix from model IDs + +Two bugs when using OpenCode Go as a provider: + +1. **Model discovery only returned free models** — the saved Go API key was never passed as `OPENCODE_API_KEY` to the spawned `opencode models opencode --refresh` process. The CLI's internal plugin checks this env var and, when absent, disables all paid models (those with `cost.input > 0`). Only 20 free models appeared instead of all 67. + +2. **API requests failed with 401** — `normalizeOpencodeGoModel` was registering models with prefixed IDs like `opencode-go/deepseek-v4-flash`. The Pi SDK sends `model.id` verbatim in API requests; the OpenCode API expects bare model names (e.g. `deepseek-v4-flash`). The prefix is now stripped during normalization. + +Also deduplicates models when the CLI emits both `opencode/foo` and `opencode-go/foo` for the same model, guards against empty model IDs, and refactors the duplicated `onApiKeySaved` handler into a shared `handleOpencodeGoApiKeySaved` helper. + +After this change, users must re-select their opencode-go model in Settings because model IDs have changed from prefixed to bare names. diff --git a/packages/cli/src/commands/__tests__/startup-model-sync.test.ts b/packages/cli/src/commands/__tests__/startup-model-sync.test.ts index 4daa09537b..9e2d09ac07 100644 --- a/packages/cli/src/commands/__tests__/startup-model-sync.test.ts +++ b/packages/cli/src/commands/__tests__/startup-model-sync.test.ts @@ -9,7 +9,7 @@ vi.mock("node:child_process", () => ({ spawn: mockSpawn, })); -import { parseOpencodeModelsOutput, refreshOpencodeGoModels, syncStartupModels } from "../startup-model-sync.js"; +import { normalizeOpencodeGoModel, parseOpencodeModelsOutput, refreshOpencodeGoModels, syncStartupModels } from "../startup-model-sync.js"; type MockProcess = EventEmitter & { stdout: EventEmitter; @@ -75,8 +75,8 @@ describe("startup-model-sync", () => { expect(registerProvider).toHaveBeenCalledWith("openrouter", expect.objectContaining({ models: expect.any(Array) })); expect(registerProvider).toHaveBeenCalledWith("opencode-go", expect.objectContaining({ models: expect.arrayContaining([ - expect.objectContaining({ id: "opencode-go/gpt-5" }), - expect.objectContaining({ id: "opencode-go/custom" }), + expect.objectContaining({ id: "gpt-5" }), + expect.objectContaining({ id: "custom" }), ]), })); expect(log).toHaveBeenCalledWith("openrouter", expect.stringContaining("Synced")); @@ -257,7 +257,7 @@ describe("startup-model-sync", () => { expect(result).toEqual({ registeredCount: 1 }); expect(registerProvider).toHaveBeenCalledWith("opencode-go", expect.objectContaining({ - models: [expect.objectContaining({ id: "opencode-go/gpt-5" })], + models: [expect.objectContaining({ id: "gpt-5" })], })); }); @@ -319,4 +319,52 @@ describe("startup-model-sync", () => { "opencode-go/custom", ]); }); + + it("deduplicates models when CLI emits both prefix forms", async () => { + mockSpawn.mockImplementation(() => { + const proc = createSpawnProcess(); + queueMicrotask(() => { + proc.stdout.emit("data", Buffer.from("opencode/foo\nopencode-go/foo\nopencode/bar\n")); + proc.emit("exit", 0); + }); + return proc; + }); + + const registerProvider = vi.fn(); + await refreshOpencodeGoModels({ modelRegistry: { registerProvider }, log: vi.fn() }); + + expect(registerProvider).toHaveBeenCalledWith("opencode-go", expect.objectContaining({ + models: [ + expect.objectContaining({ id: "foo" }), + expect.objectContaining({ id: "bar" }), + ], + })); + }); + + it("throws on empty model ID after prefix stripping", () => { + expect(() => normalizeOpencodeGoModel("opencode/")).toThrow("no model name"); + expect(() => normalizeOpencodeGoModel("opencode-go/")).toThrow("no model name"); + }); + + it("accepts apiKey and passes it as env var to spawn", async () => { + mockSpawn.mockImplementation(() => { + const proc = createSpawnProcess(); + queueMicrotask(() => { + proc.stdout.emit("data", Buffer.from("opencode/foo\n")); + proc.emit("exit", 0); + }); + return proc; + }); + + const registerProvider = vi.fn(); + await refreshOpencodeGoModels({ modelRegistry: { registerProvider }, log: vi.fn(), apiKey: "test-key" }); + + expect(mockSpawn).toHaveBeenCalledWith( + "opencode", + ["models", "opencode", "--refresh"], + expect.objectContaining({ + env: expect.objectContaining({ OPENCODE_API_KEY: "test-key" }), + }), + ); + }); }); diff --git a/packages/cli/src/commands/daemon.ts b/packages/cli/src/commands/daemon.ts index ead75bf96b..31172e6bb7 100644 --- a/packages/cli/src/commands/daemon.ts +++ b/packages/cli/src/commands/daemon.ts @@ -71,7 +71,7 @@ import { createReadOnlyAuthFileStorage, mergeAuthStorageReads, wrapAuthStorageWi import { getClaudeCodeCredentialPaths, getCodexCliAuthPath, getFusionAuthPath, getLegacyAuthPaths, getModelRegistryModelsPath, getPackageManagerAgentDir } from "./auth-paths.js"; import { resolveProject } from "../project-context.js"; import { ensureBundledDependencyGraphPluginInstalled } from "../plugins/bundled-plugin-install.js"; -import { refreshOpencodeGoModels, syncStartupModels } from "./startup-model-sync.js"; +import { handleOpencodeGoApiKeySaved, syncStartupModels } from "./startup-model-sync.js"; import { ensureCwdProjectRegistered } from "./ensure-project-registered.js"; const DIAGNOSTIC_INTERVAL_MS = 30 * 60 * 1000; // 30 minutes @@ -720,16 +720,12 @@ export async function runDaemon(opts: DaemonOptions = {}) { if (providerId !== "opencode" && providerId !== "opencode-go") { return undefined; } - const settings = await store.getSettings(); - if (settings.opencodeGoModelSync === false) { - return { registeredCount: 0, reason: "disabled-by-settings" }; - } - const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); - return await refreshOpencodeGoModels({ + return await handleOpencodeGoApiKeySaved( + dashboardAuthStorage, + store, modelRegistry, - log: (scope, message) => console.log(`[${scope}] ${message}`), - apiKey: opencodeGoKey, - }); + (scope, message) => console.log(`[${scope}] ${message}`), + ); }, getClaudeCliExtensionStatus: () => { const r = getCachedClaudeCliResolution(); diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index a41f7b8055..21cbfbba65 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -83,7 +83,7 @@ import { getCachedUpdateStatus, isUpdateCheckEnabled } from "../update-cache.js" import { resolveSelfExtension } from "./self-extension.js"; import { ensureBundledDependencyGraphPluginInstalled, ensureBundledPluginInstalled, isBundledPluginId } from "../plugins/bundled-plugin-install.js"; import { registerCustomProviders, reregisterCustomProviders } from "./custom-provider-registry.js"; -import { refreshOpencodeGoModels, syncStartupModels } from "./startup-model-sync.js"; +import { handleOpencodeGoApiKeySaved, syncStartupModels } from "./startup-model-sync.js"; import { DashboardTUI, DashboardLogSink, isTTYAvailable, type SystemInfo, type GitStatus, type GitCommit, type GitCommitDetail, type GitBranch, type GitWorktree, type FileEntry, type FileReadResult, type TaskStep as TUITaskStep, type TaskLogEntry as TUITaskLogEntry, type TaskDetailData, type TaskEvent } from "./dashboard-tui/index.js"; import { DASHBOARD_STARTUP_STATUS, runTuiStartupPrelude } from "./dashboard-startup-chain.js"; @@ -1760,16 +1760,12 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: if (providerId !== "opencode" && providerId !== "opencode-go") { return undefined; } - const settings = await store.getSettings(); - if (settings.opencodeGoModelSync === false) { - return { registeredCount: 0, reason: "disabled-by-settings" }; - } - const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); - return await refreshOpencodeGoModels({ + return await handleOpencodeGoApiKeySaved( + dashboardAuthStorage, + store, modelRegistry, - log: (scope, message) => logSink.log(message, scope), - apiKey: opencodeGoKey, - }); + (scope, message) => logSink.log(message, scope), + ); }, getClaudeCliExtensionStatus: () => { const r = getCachedClaudeCliResolution(); @@ -2083,16 +2079,12 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: if (providerId !== "opencode" && providerId !== "opencode-go") { return undefined; } - const settings = await store.getSettings(); - if (settings.opencodeGoModelSync === false) { - return { registeredCount: 0, reason: "disabled-by-settings" }; - } - const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); - return await refreshOpencodeGoModels({ + return await handleOpencodeGoApiKeySaved( + dashboardAuthStorage, + store, modelRegistry, - log: (scope, message) => logSink.log(message, scope), - apiKey: opencodeGoKey, - }); + (scope, message) => logSink.log(message, scope), + ); }, getClaudeCliExtensionStatus: () => { const r = getCachedClaudeCliResolution(); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 2d6a315d6c..a19b35813a 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -71,7 +71,7 @@ import { } from "./llama-cpp-extension.js"; import { resolveSelfExtension } from "./self-extension.js"; import { registerCustomProviders, reregisterCustomProviders } from "./custom-provider-registry.js"; -import { refreshOpencodeGoModels, syncStartupModels } from "./startup-model-sync.js"; +import { handleOpencodeGoApiKeySaved, syncStartupModels } from "./startup-model-sync.js"; import { ensureBundledDependencyGraphPluginInstalled, ensureBundledPluginInstalled, isBundledPluginId } from "../plugins/bundled-plugin-install.js"; import { ensureCwdProjectRegistered } from "./ensure-project-registered.js"; @@ -827,16 +827,12 @@ export async function runServe( if (providerId !== "opencode" && providerId !== "opencode-go") { return undefined; } - const settings = await store.getSettings(); - if (settings.opencodeGoModelSync === false) { - return { registeredCount: 0, reason: "disabled-by-settings" }; - } - const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); - return await refreshOpencodeGoModels({ + return await handleOpencodeGoApiKeySaved( + dashboardAuthStorage, + store, modelRegistry, - log: (scope, message) => console.log(`[${scope}] ${message}`), - apiKey: opencodeGoKey, - }); + (scope, message) => console.log(`[${scope}] ${message}`), + ); }, getClaudeCliExtensionStatus: () => { const r = getCachedClaudeCliResolution(); diff --git a/packages/cli/src/commands/startup-model-sync.ts b/packages/cli/src/commands/startup-model-sync.ts index ef5a1d30c2..2700c448d6 100644 --- a/packages/cli/src/commands/startup-model-sync.ts +++ b/packages/cli/src/commands/startup-model-sync.ts @@ -214,6 +214,10 @@ export function normalizeOpencodeGoModel(modelId: string): ModelConfig { ? trimmed.slice("opencode/".length) : trimmed; + if (!bareModel) { + throw new Error(`Invalid opencode-go model ID: "${modelId}" has no model name after provider prefix`); + } + return { id: bareModel, name: bareModel, @@ -290,7 +294,15 @@ export async function refreshOpencodeGoModels(options: { return { registeredCount: 0, reason: "no-models-from-cli" }; } - const models = modelIds.map(normalizeOpencodeGoModel); + const normalized = modelIds.map(normalizeOpencodeGoModel); + // Deduplicate: CLI can emit both "opencode/foo" and "opencode-go/foo" + // which normalize to the same bare ID. + const seen = new Set(); + const models = normalized.filter((m) => { + if (seen.has(m.id)) return false; + seen.add(m.id); + return true; + }); modelRegistry.registerProvider("opencode-go", { baseUrl: "https://api.opencode.ai/v1", apiKey: "OPENCODE_API_KEY", @@ -323,3 +335,23 @@ export async function syncStartupModels(options: StartupSyncOptions): Promise Promise }, + modelRegistry: ModelRegistryLike, + log: (scope: string, message: string) => void, +): Promise { + const settings = await store.getSettings(); + if (settings.opencodeGoModelSync === false) { + return { registeredCount: 0, reason: "disabled-by-settings" }; + } + const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode"); + return await refreshOpencodeGoModels({ modelRegistry, log, apiKey: opencodeGoKey }); +} From 4426eb8d743075646e864c2d7296b8f700bd14a3 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 21:57:46 -0700 Subject: [PATCH 04/21] Fix Settings plugin installs registering unloadable directory paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugins installed from Settings → Built-in Plugins registered the manifest directory as the plugin path, but since FN-4128 the loader requires a loadable entry FILE (Node ESM cannot import directories), so enabling failed with "Plugin entry must be a file, got directory". Only the CLI startup path had been migrated to entry-file resolution, which is why CLI-auto-installed plugins worked and Settings installs never did. - Add resolvePluginEntryPath (bundled.js → dist/index.js → src/index.ts) to @fusion/core; the CLI keeps its local copy (its test fs mocks don't reach externalized core) with sync comments both ways. - Register the resolved entry file in both dashboard install routes; 400 with a clear message when a package has no loadable entry. - Heal legacy directory-path registrations on enable, mirroring the CLI's startup heal, so existing broken rows recover from the UI without a restart. - Route tests: assert installs register entry files, cover the enable-route heal, and update existing install tests to the entry-file contract. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...rkflow-graph-editor-and-bundled-plugins.md | 1 + .../bundled-plugin-registration-drift.md | 4 + .../cli/src/plugins/bundled-plugin-install.ts | 3 + packages/core/src/index.ts | 2 +- packages/core/src/plugin-loader.ts | 32 ++++- .../src/__tests__/plugin-routes.test.ts | 119 ++++++++++++++++-- packages/dashboard/src/plugin-routes.ts | 13 +- packages/dashboard/src/routes.ts | 27 +++- 8 files changed, 183 insertions(+), 18 deletions(-) diff --git a/.changeset/workflow-graph-editor-and-bundled-plugins.md b/.changeset/workflow-graph-editor-and-bundled-plugins.md index ebef3b8410..d8b6bb60fc 100644 --- a/.changeset/workflow-graph-editor-and-bundled-plugins.md +++ b/.changeset/workflow-graph-editor-and-bundled-plugins.md @@ -7,3 +7,4 @@ Fix the workflow graph editor opening invisibly and bundle the Compound Engineer - The "Graph editor" button now actually shows the editor: its overlay was rendered without the `open` class, leaving it `display: none`, so opening it looked like the workflow steps view was just dismissed. - `fusion-plugin-compound-engineering` and `fusion-plugin-roadmap` are now listed in the dashboard's built-in plugins, so they appear under Settings → Built-in Plugins (they were implemented and registered but missing from the list). - Installing Compound Engineering (and CLI Printing Press) from Settings → Built-in Plugins no longer fails with "Plugin manifest not found": both ids are now in the dashboard's bundled-plugin fallback set, and the Compound Engineering plugin is staged into `dist/plugins/` so packaged installs can resolve it. +- Plugins installed from Settings now load instead of erroring with "Plugin entry must be a file, got directory": the dashboard install routes register the plugin's loadable entry file (`bundled.js`/`dist/index.js`/`src/index.ts`) rather than the package directory, and enabling a plugin heals legacy directory-path registrations in place. diff --git a/docs/solutions/integration-issues/bundled-plugin-registration-drift.md b/docs/solutions/integration-issues/bundled-plugin-registration-drift.md index b9b485a893..a7973caad5 100644 --- a/docs/solutions/integration-issues/bundled-plugin-registration-drift.md +++ b/docs/solutions/integration-issues/bundled-plugin-registration-drift.md @@ -63,6 +63,10 @@ await bundlePluginEntry({ }); ``` +## Follow-up failure: directory registered as plugin path + +Fixing the fallback surfaced a second, independent bug: both dashboard install routes registered the **manifest directory** as the plugin path, but since FN-4128 the loader requires a loadable entry **file** (Node ESM cannot import directories) — enable then failed with `Plugin entry must be a file, got directory: `. Only the CLI startup path had been migrated to `resolvePluginEntryPath` (`bundled.js` → `dist/index.js` → `src/index.ts`), which is why CLI-auto-installed plugins worked and Settings-installed ones never did. Fix: the install routes now resolve and register the entry file (helper added to `@fusion/core`), and the enable route heals legacy directory-path rows in place — mirroring the CLI's startup heal. + ## Why This Works The Settings card sends a relative `./plugins/` path. The server resolves it against `process.cwd()` — normally the user's project dir, not the Fusion repo — so it 404s and falls back to `extractBundledPluginId()`, which only recognizes ids in routes.ts's `BUNDLED_PLUGIN_IDS`. Adding the id makes the fallback resolve the staged bundled copy; the tsup staging block guarantees that copy exists in packaged installs. diff --git a/packages/cli/src/plugins/bundled-plugin-install.ts b/packages/cli/src/plugins/bundled-plugin-install.ts index 7ddf24a102..6f5beede25 100644 --- a/packages/cli/src/plugins/bundled-plugin-install.ts +++ b/packages/cli/src/plugins/bundled-plugin-install.ts @@ -78,6 +78,9 @@ function resolveBundledPluginDir(pluginId: string): string | null { * Returns null when the directory exists but none of the loadable entry files * are present. Callers must treat that as a missing bundle rather than * persisting a directory path that Node cannot import. + * + * Keep in sync with resolvePluginEntryPath in @fusion/core (plugin-loader.ts), + * which the dashboard install/enable routes use for the same contract. */ export function resolvePluginEntryPath(pluginDir: string): string | null { const candidates = [ diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index dc731acb8f..7d89c6fa08 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -587,7 +587,7 @@ export type { export { validatePluginManifest, normalizePluginUiContributionSurface, normalizePluginUiContributionDefinition } from "./plugin-types.js"; export { PluginStore } from "./plugin-store.js"; export type { PluginStoreEvents, PluginRegistrationInput, PluginUpdateInput } from "./plugin-store.js"; -export { PluginLoader } from "./plugin-loader.js"; +export { PluginLoader, resolvePluginEntryPath } from "./plugin-loader.js"; export { scanPluginSecurity } from "./plugin-security-scan.js"; export type { PluginSecurityScanResult, PluginSecurityFinding } from "./plugin-security-scan.js"; export type { diff --git a/packages/core/src/plugin-loader.ts b/packages/core/src/plugin-loader.ts index 4d06bb6532..c9a5b28472 100644 --- a/packages/core/src/plugin-loader.ts +++ b/packages/core/src/plugin-loader.ts @@ -9,7 +9,8 @@ * - Error isolation (plugin crashes don't crash the loader) */ -import { basename, dirname, extname, isAbsolute, resolve } from "node:path"; +import { basename, dirname, extname, isAbsolute, join, resolve } from "node:path"; +import { existsSync } from "node:fs"; import { stat } from "node:fs/promises"; import { copyFile } from "node:fs/promises"; import { pathToFileURL } from "node:url"; @@ -47,6 +48,35 @@ import { scanPluginSecurity } from "./plugin-security-scan.js"; const MINIMUM_FUSION_VERSION = "0.1.0"; let moduleImportVersion = 0; +/** + * Resolve the actual loadable entry FILE path for a plugin directory. Node ESM + * does not allow directory imports, so the registered plugin path must be the + * explicit file the loader will dynamic-import. Preference order: + * 1. ./bundled.js (esbuild-bundled, shipped in npm tarball) + * 2. ./dist/index.js (legacy prebuilt fallback) + * 3. ./src/index.ts (workspace/dev fallback when no bundle exists) + * + * Returns null when the directory exists but none of the loadable entry files + * are present. Callers must treat that as a missing/unloadable plugin rather + * than persisting a directory path that Node cannot import. + * + * Keep in sync with resolvePluginEntryPath in the CLI's + * bundled-plugin-install.ts, which keeps a local copy so its fs mocks work. + */ +export function resolvePluginEntryPath(pluginDir: string): string | null { + const candidates = [ + join(pluginDir, "bundled.js"), + join(pluginDir, "dist", "index.js"), + join(pluginDir, "src", "index.ts"), + ]; + for (const candidate of candidates) { + if (existsSync(candidate)) { + return candidate; + } + } + return null; +} + export interface PluginLoaderOptions { /** Plugin store for persistence */ pluginStore: PluginStore; diff --git a/packages/dashboard/src/__tests__/plugin-routes.test.ts b/packages/dashboard/src/__tests__/plugin-routes.test.ts index 62dce6a2f6..b6e7f54eb2 100644 --- a/packages/dashboard/src/__tests__/plugin-routes.test.ts +++ b/packages/dashboard/src/__tests__/plugin-routes.test.ts @@ -280,6 +280,9 @@ describe("POST /api/plugins mode:install — package root path", () => { beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ @@ -313,7 +316,8 @@ describe("POST /api/plugins mode:install — package root path", () => { expect(pluginStore.registerPlugin).toHaveBeenCalledWith( expect.objectContaining({ manifest: expect.objectContaining({ id: "my-plugin" }), - path: pkgRoot, + // Registered path is the loadable entry file inside the package root + path: `${pkgRoot}/bundled.js`, }), ); }); @@ -338,7 +342,7 @@ describe("POST /api/plugins mode:install — package root path", () => { expect(res.status).toBe(201); expect(res.body).toMatchObject({ id: "my-plugin" }); expect(pluginStore.registerPlugin).toHaveBeenCalledWith( - expect.objectContaining({ path: distPath }), + expect.objectContaining({ path: `${distPath}/bundled.js` }), ); }); @@ -434,6 +438,7 @@ describe("POST /api/plugins central persistence integration", () => { if (p === pluginPath || p === `${pluginPath}/manifest.json`) return Promise.resolve(); return Promise.reject(new Error("not found")); }); + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); mockReadFile.mockResolvedValueOnce(JSON.stringify(VALID_MANIFEST)); const app = buildRealApp(pluginStore); @@ -471,6 +476,9 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ @@ -491,7 +499,7 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = id: "fusion-plugin-dependency-graph", name: "Dependency Graph", }; - mockExistsSync.mockImplementation((p: string) => p.includes("fusion-plugin-dependency-graph/manifest.json")); + mockExistsSync.mockImplementation((p: string) => p.includes("fusion-plugin-dependency-graph/manifest.json") || p.endsWith("bundled.js")); mockAccess.mockImplementation((p: string) => { if (p.includes("fusion-plugin-dependency-graph")) return Promise.resolve(); return Promise.reject(new Error("not found")); @@ -523,7 +531,7 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = id: "fusion-plugin-reports", name: "Reports", }; - mockExistsSync.mockImplementation((p: string) => p.includes("fusion-plugin-reports/manifest.json")); + mockExistsSync.mockImplementation((p: string) => p.includes("fusion-plugin-reports/manifest.json") || p.endsWith("bundled.js")); mockAccess.mockImplementation((p: string) => { if (p.includes("fusion-plugin-reports")) return Promise.resolve(); return Promise.reject(new Error("not found")); @@ -557,7 +565,9 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = }; // Only the staged bundled copy under dist/plugins exists — the // cwd-relative path must miss so the bundled fallback is exercised. - mockExistsSync.mockImplementation((p: string) => p.includes("dist/plugins/fusion-plugin-compound-engineering/manifest.json")); + mockExistsSync.mockImplementation((p: string) => + p.includes("dist/plugins/fusion-plugin-compound-engineering/manifest.json") + || p.includes("dist/plugins/fusion-plugin-compound-engineering/bundled.js")); mockAccess.mockImplementation((p: string) => { if (p.includes("dist/plugins/fusion-plugin-compound-engineering")) return Promise.resolve(); return Promise.reject(new Error("not found")); @@ -575,10 +585,12 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = }); expect(res.status).toBe(201); + // The registered path must be the loadable entry FILE, not the + // package directory — the loader rejects directory imports. expect(pluginStore.registerPlugin).toHaveBeenCalledWith( expect.objectContaining({ manifest: expect.objectContaining({ id: "fusion-plugin-compound-engineering" }), - path: expect.stringContaining("fusion-plugin-compound-engineering"), + path: expect.stringMatching(/fusion-plugin-compound-engineering[\\/]bundled\.js$/), }), ); }); @@ -591,7 +603,9 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = }; // Only the staged bundled copy under dist/plugins exists — the // cwd-relative path must miss so the bundled fallback is exercised. - mockExistsSync.mockImplementation((p: string) => p.includes("dist/plugins/fusion-plugin-cli-printing-press/manifest.json")); + mockExistsSync.mockImplementation((p: string) => + p.includes("dist/plugins/fusion-plugin-cli-printing-press/manifest.json") + || p.includes("dist/plugins/fusion-plugin-cli-printing-press/bundled.js")); mockAccess.mockImplementation((p: string) => { if (p.includes("dist/plugins/fusion-plugin-cli-printing-press")) return Promise.resolve(); return Promise.reject(new Error("not found")); @@ -612,7 +626,7 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = expect(pluginStore.registerPlugin).toHaveBeenCalledWith( expect.objectContaining({ manifest: expect.objectContaining({ id: "fusion-plugin-cli-printing-press" }), - path: expect.stringContaining("fusion-plugin-cli-printing-press"), + path: expect.stringMatching(/fusion-plugin-cli-printing-press[\\/]bundled\.js$/), }), ); }); @@ -648,6 +662,64 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = }); }); +describe("POST /api/plugins/:id/enable — legacy directory path heal", () => { + let pluginStore: PluginStore; + let pluginLoader: PluginLoader; + let store: TaskStore; + + beforeEach(() => { + vi.clearAllMocks(); + pluginStore = createMockPluginStore(); + pluginLoader = createMockPluginLoader(); + store = createMockTaskStore({ + getPluginStore: vi.fn().mockReturnValue(pluginStore), + }); + }); + + function buildApp() { + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store, { pluginStore, pluginLoader })); + return app; + } + + it("re-points a directory plugin path at its loadable entry before loading", async () => { + // Legacy registration stored the package directory; the loader rejects + // directory imports, so enable must heal the path first. + const dirPath = "/home/user/plugins/my-plugin"; + mockStatSync.mockReturnValue({ isDirectory: () => true }); + mockExistsSync.mockImplementation((p: string) => p === `${dirPath}/bundled.js`); + (pluginStore.enablePlugin as ReturnType).mockResolvedValue({ + ...INSTALLED_PLUGIN, + path: dirPath, + }); + (pluginStore.updatePlugin as ReturnType).mockResolvedValue({ + ...INSTALLED_PLUGIN, + path: `${dirPath}/bundled.js`, + }); + + const res = await REQUEST(buildApp(), "POST", "/api/plugins/my-plugin/enable", {}); + + expect(res.status).toBe(200); + expect(pluginStore.updatePlugin).toHaveBeenCalledWith("my-plugin", { path: `${dirPath}/bundled.js` }); + expect(pluginLoader.loadPlugin).toHaveBeenCalledWith("my-plugin"); + }); + + it("leaves file paths untouched on enable", async () => { + mockStatSync.mockReturnValue({ isDirectory: () => false }); + (pluginStore.enablePlugin as ReturnType).mockResolvedValue({ + ...INSTALLED_PLUGIN, + path: "/home/user/plugins/my-plugin/bundled.js", + }); + + const res = await REQUEST(buildApp(), "POST", "/api/plugins/my-plugin/enable", {}); + + expect(res.status).toBe(200); + expect(pluginStore.updatePlugin).not.toHaveBeenCalled(); + expect(pluginLoader.loadPlugin).toHaveBeenCalledWith("my-plugin"); + }); +}); + describe("POST /api/plugins mode:install — negative paths", () => { let pluginStore: PluginStore; let pluginLoader: PluginLoader; @@ -655,6 +727,9 @@ describe("POST /api/plugins mode:install — negative paths", () => { beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ @@ -835,6 +910,9 @@ describe("POST /api/plugins mode:install — manifest validation edge cases", () beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ @@ -921,6 +999,9 @@ describe("POST /api/plugins mode:install — dist-folder parent resolution", () beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore({ registerPlugin: vi.fn().mockResolvedValue(INSTALLED_PLUGIN), }); @@ -954,7 +1035,7 @@ describe("POST /api/plugins mode:install — dist-folder parent resolution", () expect(res.status).toBe(201); expect(pluginStore.registerPlugin).toHaveBeenCalledWith( - expect.objectContaining({ path: parentPath }), + expect.objectContaining({ path: `${parentPath}/bundled.js` }), ); }); @@ -974,7 +1055,7 @@ describe("POST /api/plugins mode:install — dist-folder parent resolution", () expect(res.status).toBe(201); expect(pluginStore.registerPlugin).toHaveBeenCalledWith( - expect.objectContaining({ path: parentPath }), + expect.objectContaining({ path: `${parentPath}/bundled.js` }), ); }); @@ -994,7 +1075,7 @@ describe("POST /api/plugins mode:install — dist-folder parent resolution", () expect(res.status).toBe(201); expect(pluginStore.registerPlugin).toHaveBeenCalledWith( - expect.objectContaining({ path: parentPath }), + expect.objectContaining({ path: `${parentPath}/bundled.js` }), ); }); @@ -1032,9 +1113,9 @@ describe("POST /api/plugins mode:install — dist-folder parent resolution", () }); expect(res.status).toBe(201); - // Should use the dist dir path since it has its own manifest + // Should use the dist dir entry since it has its own manifest expect(pluginStore.registerPlugin).toHaveBeenCalledWith( - expect.objectContaining({ path: distPath }), + expect.objectContaining({ path: `${distPath}/bundled.js` }), ); }); }); @@ -1049,6 +1130,9 @@ describe("GET /api/plugins/dashboard-views", () => { beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ @@ -1166,6 +1250,9 @@ describe("GET /api/plugins/ui-slots", () => { beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ @@ -1335,6 +1422,9 @@ describe("GET /api/plugins/ui-contributions", () => { beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ @@ -1842,6 +1932,9 @@ describe("GET /api/plugins/runtimes", () => { beforeEach(() => { vi.clearAllMocks(); + // Install now registers the loadable entry file; pretend each + // package ships an esbuild bundle. + mockExistsSync.mockImplementation((p: string) => p.endsWith("bundled.js")); pluginStore = createMockPluginStore(); pluginLoader = createMockPluginLoader(); store = createMockTaskStore({ diff --git a/packages/dashboard/src/plugin-routes.ts b/packages/dashboard/src/plugin-routes.ts index d12cf471d8..883498d78a 100644 --- a/packages/dashboard/src/plugin-routes.ts +++ b/packages/dashboard/src/plugin-routes.ts @@ -24,7 +24,7 @@ import type { PluginStore, PluginContext, } from "@fusion/core"; -import { validatePluginManifest } from "@fusion/core"; +import { resolvePluginEntryPath, validatePluginManifest } from "@fusion/core"; import { ApiError, badRequest, @@ -251,7 +251,16 @@ export function createPluginRouter( if (source.path) { const resolved = await resolvePluginManifest(source.path); manifest = resolved.manifest; - installPath = resolved.manifestDir; + // Register the loadable entry FILE, not the package directory — Node + // ESM cannot import directories, so the loader rejects directory paths. + const entryPath = resolvePluginEntryPath(resolved.manifestDir); + if (!entryPath) { + throw badRequest( + `Plugin at ${resolved.manifestDir} has no loadable entry file ` + + "(expected bundled.js, dist/index.js, or src/index.ts)", + ); + } + installPath = entryPath; } else if (source.package) { // npm packages not yet supported throw badRequest("Installing plugins from npm packages is not yet implemented"); diff --git a/packages/dashboard/src/routes.ts b/packages/dashboard/src/routes.ts index 57ec951ebd..47e8bff7d1 100644 --- a/packages/dashboard/src/routes.ts +++ b/packages/dashboard/src/routes.ts @@ -29,6 +29,7 @@ import { listAgentMemoryFiles, readAgentMemoryFile, resolvePlanningSettingsModel, + resolvePluginEntryPath, resolveProjectDefaultModel, resolveTitleSummarizerSettingsModel, writeAgentMemoryFile, @@ -3618,10 +3619,20 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout // Resolve manifest — supports package root and dist-folder selections const { manifestDir, manifest } = await resolvePluginManifest(manifestPathForInstall); + // Register the loadable entry FILE, not the package directory — Node ESM + // cannot import directories, so the loader rejects directory paths. + const entryPath = resolvePluginEntryPath(manifestDir); + if (!entryPath) { + throw badRequest( + `Plugin at ${manifestDir} has no loadable entry file ` + + "(expected bundled.js, dist/index.js, or src/index.ts)", + ); + } + try { const plugin = await pluginStore.registerPlugin({ manifest, - path: manifestDir, + path: entryPath, ...(typeof aiScanOnLoad === "boolean" ? { aiScanOnLoad } : {}), }); @@ -3668,6 +3679,20 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout let plugin = await pluginStore.enablePlugin(id); + // Heal legacy registrations that stored the package directory instead of + // a loadable entry file (Node ESM cannot import directories). Mirrors the + // CLI's startup heal in ensureBundledPluginInstalled. + try { + if (nodeFs.statSync(plugin.path).isDirectory()) { + const entryPath = resolvePluginEntryPath(plugin.path); + if (entryPath) { + plugin = await pluginStore.updatePlugin(id, { path: entryPath }); + } + } + } catch { + // Path missing or unreadable — let loadPlugin surface the real error. + } + // Start the plugin if loader is available if (options?.pluginLoader) { try { From 7dde43a63b0321961eceda35ad2abe714929d41b Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 22:10:20 -0700 Subject: [PATCH 05/21] Address PR review feedback (#1428) - Add heal block to createPluginRouter's enable handler so it matches routes.ts (directory-path registrations re-pointed at entry files) - Test the 400 "no loadable entry file" install branch - Add a real-fs drift-guard test asserting the CLI and @fusion/core copies of resolvePluginEntryPath resolve identically Co-Authored-By: Claude Opus 4.8 (1M context) --- .../resolve-plugin-entry-path-sync.test.ts | 57 +++++++++++++++++++ .../src/__tests__/plugin-routes.test.ts | 43 ++++++++++++++ packages/dashboard/src/plugin-routes.ts | 14 +++++ 3 files changed, 114 insertions(+) create mode 100644 packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts diff --git a/packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts b/packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts new file mode 100644 index 0000000000..0fa5e80258 --- /dev/null +++ b/packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts @@ -0,0 +1,57 @@ +/** + * Drift guard for the intentionally duplicated resolvePluginEntryPath. + * + * The CLI keeps a local copy in bundled-plugin-install.ts (so its fs mocks + * work in tests) while @fusion/core owns the copy used by the dashboard + * install/enable routes. This test runs both against real on-disk layouts and + * asserts identical results, so a candidate-list change applied to one copy + * but not the other fails CI instead of silently diverging. + * + * No fs mocks here on purpose — vitest module mocks don't reach the + * externalized @fusion/core import, so real temp directories are the only + * seam that exercises both implementations equally. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { resolvePluginEntryPath as cliResolve } from "../bundled-plugin-install.js"; +import { resolvePluginEntryPath as coreResolve } from "@fusion/core"; + +describe("resolvePluginEntryPath: CLI copy stays in sync with @fusion/core", () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "entry-path-sync-")); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + function touch(relative: string) { + const full = join(dir, relative); + mkdirSync(join(full, ".."), { recursive: true }); + writeFileSync(full, "// entry\n"); + } + + const layouts: Array<{ name: string; files: string[]; expected: string | null }> = [ + { name: "bundled.js only", files: ["bundled.js"], expected: "bundled.js" }, + { name: "dist/index.js only", files: ["dist/index.js"], expected: "dist/index.js" }, + { name: "src/index.ts only", files: ["src/index.ts"], expected: "src/index.ts" }, + { name: "bundled.js preferred over src", files: ["bundled.js", "src/index.ts"], expected: "bundled.js" }, + { name: "dist preferred over src", files: ["dist/index.js", "src/index.ts"], expected: "dist/index.js" }, + { name: "all three → bundled.js", files: ["bundled.js", "dist/index.js", "src/index.ts"], expected: "bundled.js" }, + { name: "no entry files", files: ["README.md"], expected: null }, + ]; + + for (const layout of layouts) { + it(`resolves identically for: ${layout.name}`, () => { + for (const f of layout.files) touch(f); + const expected = layout.expected === null ? null : join(dir, layout.expected); + + expect(cliResolve(dir)).toBe(expected); + expect(coreResolve(dir)).toBe(expected); + }); + } +}); diff --git a/packages/dashboard/src/__tests__/plugin-routes.test.ts b/packages/dashboard/src/__tests__/plugin-routes.test.ts index b6e7f54eb2..6d09caf506 100644 --- a/packages/dashboard/src/__tests__/plugin-routes.test.ts +++ b/packages/dashboard/src/__tests__/plugin-routes.test.ts @@ -705,6 +705,29 @@ describe("POST /api/plugins/:id/enable — legacy directory path heal", () => { expect(pluginLoader.loadPlugin).toHaveBeenCalledWith("my-plugin"); }); + it("heals directory paths in createPluginRouter's enable handler too", async () => { + const dirPath = "/home/user/plugins/my-plugin"; + mockStat.mockResolvedValue({ isDirectory: () => true }); + mockExistsSync.mockImplementation((p: string) => p === `${dirPath}/bundled.js`); + (pluginStore.enablePlugin as ReturnType).mockResolvedValue({ + ...INSTALLED_PLUGIN, + path: dirPath, + }); + (pluginStore.updatePlugin as ReturnType).mockResolvedValue({ + ...INSTALLED_PLUGIN, + path: `${dirPath}/bundled.js`, + }); + + const app = express(); + app.use(express.json()); + app.use("/api/plugins", createPluginRouter(pluginStore, pluginLoader)); + const res = await REQUEST(app, "POST", "/api/plugins/my-plugin/enable", {}); + + expect(res.status).toBe(200); + expect(pluginStore.updatePlugin).toHaveBeenCalledWith("my-plugin", { path: `${dirPath}/bundled.js` }); + expect(pluginLoader.loadPlugin).toHaveBeenCalledWith("my-plugin"); + }); + it("leaves file paths untouched on enable", async () => { mockStatSync.mockReturnValue({ isDirectory: () => false }); (pluginStore.enablePlugin as ReturnType).mockResolvedValue({ @@ -744,6 +767,26 @@ describe("POST /api/plugins mode:install — negative paths", () => { return app; } + it("returns 400 when the package has no loadable entry file", async () => { + const pkgRoot = "/home/user/plugins/my-plugin"; + mockAccess.mockImplementation((p: string) => { + if (p === pkgRoot || p === `${pkgRoot}/manifest.json`) return Promise.resolve(); + return Promise.reject(new Error("not found")); + }); + mockReadFile.mockResolvedValue(JSON.stringify(VALID_MANIFEST)); + // Manifest resolves, but no bundled.js / dist/index.js / src/index.ts exists. + mockExistsSync.mockReturnValue(false); + + const res = await REQUEST(buildApp(), "POST", "/api/plugins", { + mode: "install", + path: pkgRoot, + }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain("no loadable entry file"); + expect(pluginStore.registerPlugin).not.toHaveBeenCalled(); + }); + it("returns 404 when path does not exist", async () => { mockAccess.mockRejectedValue(new Error("not found")); diff --git a/packages/dashboard/src/plugin-routes.ts b/packages/dashboard/src/plugin-routes.ts index 883498d78a..9720c9bf84 100644 --- a/packages/dashboard/src/plugin-routes.ts +++ b/packages/dashboard/src/plugin-routes.ts @@ -307,6 +307,20 @@ export function createPluginRouter( // Enable in store let plugin = await pluginStore.enablePlugin(id); + // Heal legacy registrations that stored the package directory instead of + // a loadable entry file (Node ESM cannot import directories). Mirrors the + // heal in routes.ts's enable handler and the CLI's startup heal. + try { + if ((await stat(plugin.path)).isDirectory()) { + const entryPath = resolvePluginEntryPath(plugin.path); + if (entryPath) { + plugin = await pluginStore.updatePlugin(id, { path: entryPath }); + } + } + } catch { + // Path missing or unreadable — let loadPlugin surface the real error. + } + // Start the plugin try { await pluginLoader.loadPlugin(id); From 784021e95c2e0300000bd90f70007fe6f50880a6 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 22:12:36 -0700 Subject: [PATCH 06/21] Tighten plugin install route tests to full entry-resolution contract (#1428) - Bundled fallback assertions for dependency-graph/reports now require the bundled.js entry-file suffix instead of just containing the id - Add route-level fallback cases for dist/index.js and src/index.ts Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/__tests__/plugin-routes.test.ts | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/dashboard/src/__tests__/plugin-routes.test.ts b/packages/dashboard/src/__tests__/plugin-routes.test.ts index 6d09caf506..8a74f0b5f3 100644 --- a/packages/dashboard/src/__tests__/plugin-routes.test.ts +++ b/packages/dashboard/src/__tests__/plugin-routes.test.ts @@ -322,6 +322,48 @@ describe("POST /api/plugins mode:install — package root path", () => { ); }); + it("falls back to dist/index.js when no bundled.js exists", async () => { + const pkgRoot = "/home/user/plugins/my-plugin"; + mockAccess.mockImplementation((p: string) => { + if (p === pkgRoot || p === `${pkgRoot}/manifest.json`) return Promise.resolve(); + return Promise.reject(new Error("not found")); + }); + mockReadFile.mockResolvedValue(JSON.stringify(VALID_MANIFEST)); + mockExistsSync.mockImplementation((p: string) => p === `${pkgRoot}/dist/index.js`); + (pluginStore.registerPlugin as ReturnType).mockResolvedValue(INSTALLED_PLUGIN); + + const res = await REQUEST(buildApp(), "POST", "/api/plugins", { + mode: "install", + path: pkgRoot, + }); + + expect(res.status).toBe(201); + expect(pluginStore.registerPlugin).toHaveBeenCalledWith( + expect.objectContaining({ path: `${pkgRoot}/dist/index.js` }), + ); + }); + + it("falls back to src/index.ts for workspace-dev packages without build outputs", async () => { + const pkgRoot = "/home/user/plugins/my-plugin"; + mockAccess.mockImplementation((p: string) => { + if (p === pkgRoot || p === `${pkgRoot}/manifest.json`) return Promise.resolve(); + return Promise.reject(new Error("not found")); + }); + mockReadFile.mockResolvedValue(JSON.stringify(VALID_MANIFEST)); + mockExistsSync.mockImplementation((p: string) => p === `${pkgRoot}/src/index.ts`); + (pluginStore.registerPlugin as ReturnType).mockResolvedValue(INSTALLED_PLUGIN); + + const res = await REQUEST(buildApp(), "POST", "/api/plugins", { + mode: "install", + path: pkgRoot, + }); + + expect(res.status).toBe(201); + expect(pluginStore.registerPlugin).toHaveBeenCalledWith( + expect.objectContaining({ path: `${pkgRoot}/src/index.ts` }), + ); + }); + it("accepts a dist folder path with valid manifest.json and returns 201", async () => { const distPath = "/home/user/plugins/my-plugin/dist"; mockAccess.mockImplementation((p: string) => { @@ -520,7 +562,7 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = expect(pluginStore.registerPlugin).toHaveBeenCalledWith( expect.objectContaining({ manifest: expect.objectContaining({ id: "fusion-plugin-dependency-graph" }), - path: expect.stringContaining("fusion-plugin-dependency-graph"), + path: expect.stringMatching(/fusion-plugin-dependency-graph[\\/]bundled\.js$/), }), ); }); @@ -552,7 +594,7 @@ describe("POST /api/plugins mode:install — bundled plugin path fallback", () = expect(pluginStore.registerPlugin).toHaveBeenCalledWith( expect.objectContaining({ manifest: expect.objectContaining({ id: "fusion-plugin-reports" }), - path: expect.stringContaining("fusion-plugin-reports"), + path: expect.stringMatching(/fusion-plugin-reports[\\/]bundled\.js$/), }), ); }); From 3bf803e6f3997345a76133d604c1a8ca67c62a28 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 22:33:23 -0700 Subject: [PATCH 07/21] Extend plugin registration-drift learning with entry-file contract details - Expand the directory-path follow-up section: PR #1428, 400 no-entry branch, heal-on-enable in both routers - Document the vitest fs-mock vs externalized workspace-dep trap and the real-fs drift-guard test pattern - Add Plugin Entry to CONCEPTS.md Plugins cluster Co-Authored-By: Claude Opus 4.8 (1M context) --- CONCEPTS.md | 4 ++++ .../bundled-plugin-registration-drift.md | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/CONCEPTS.md b/CONCEPTS.md index 7d892fe025..0a5bdce4e1 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -141,6 +141,10 @@ A plugin that ships inside the Fusion distribution itself rather than being inst *Avoid:* built-in plugin (as a distinct concept; the Settings label uses "Built-in" for the same thing) A Bundled Plugin must be registered in several independently maintained surfaces — the Settings catalog, the dashboard server's bundled-id fallback set, the CLI's startup auto-install list, and the build step that stages a loadable copy into the distribution. The surfaces do not cross-check each other: a plugin registered in some but not all appears installable yet fails to install or load, so adding one means mirroring an existing bundled plugin across every surface. + +### Plugin Entry +The single loadable file persisted as a plugin's path and dynamically imported by the loader. The contract is strict: a package directory is never a valid entry (ESM cannot import directories), so every install surface must resolve a concrete file before persisting, preferring the shipped bundle, then a prebuilt output, then raw workspace source. Legacy registrations that stored a directory are healed in place — re-pointed at a resolved entry — the next time the plugin is enabled or auto-installed. + ## Workflow columns & traits *Behind the `experimentalFeatures.workflowColumns` flag. With the flag off, the legacy fixed pipeline (the closed column enum + `VALID_TRANSITIONS`) is authoritative and unchanged.* diff --git a/docs/solutions/integration-issues/bundled-plugin-registration-drift.md b/docs/solutions/integration-issues/bundled-plugin-registration-drift.md index a7973caad5..7c8ba38f33 100644 --- a/docs/solutions/integration-issues/bundled-plugin-registration-drift.md +++ b/docs/solutions/integration-issues/bundled-plugin-registration-drift.md @@ -12,7 +12,8 @@ symptoms: root_cause: incomplete_setup resolution_type: code_fix severity: medium -tags: [plugins, bundled-plugins, settings, install, tsup, registration-drift] +last_updated: 2026-06-05 +tags: [plugins, bundled-plugins, settings, install, tsup, registration-drift, entry-file, fs-mock] --- # Bundled plugins must be registered in 4 independent places — they drift @@ -65,7 +66,11 @@ await bundlePluginEntry({ ## Follow-up failure: directory registered as plugin path -Fixing the fallback surfaced a second, independent bug: both dashboard install routes registered the **manifest directory** as the plugin path, but since FN-4128 the loader requires a loadable entry **file** (Node ESM cannot import directories) — enable then failed with `Plugin entry must be a file, got directory: `. Only the CLI startup path had been migrated to `resolvePluginEntryPath` (`bundled.js` → `dist/index.js` → `src/index.ts`), which is why CLI-auto-installed plugins worked and Settings-installed ones never did. Fix: the install routes now resolve and register the entry file (helper added to `@fusion/core`), and the enable route heals legacy directory-path rows in place — mirroring the CLI's startup heal. +Fixing the fallback surfaced a second, independent bug (fixed in PR #1428): both dashboard install routes registered the **manifest directory** as the plugin path, but since FN-4128 the loader requires a loadable entry **file** (Node ESM cannot import directories) — enable then failed with `Plugin entry must be a file, got directory: `. Only the CLI startup path had been migrated to `resolvePluginEntryPath` (`bundled.js` → `dist/index.js` → `src/index.ts`), which is why CLI-auto-installed plugins worked and Settings-installed ones never did. Fix: both install routes now resolve and register the entry file (helper added to `@fusion/core`; 400 with "no loadable entry file" when none exists), and **both** enable routes heal legacy directory-path rows in place before `loadPlugin` — mirroring the CLI's startup heal — so pre-fix broken registrations self-repair on first enable without a migration. + +### Trap: vitest fs mocks don't reach externalized workspace deps + +Moving `resolvePluginEntryPath` to `@fusion/core` and re-exporting from the CLI broke the CLI's tests: `vi.mock("node:fs")` in the CLI package does **not** intercept fs calls made inside the externalized `@fusion/core` import (vitest only inlines/mocks modules in the test package's transform graph — the dashboard package inlines core, the CLI doesn't). Resolution: the CLI keeps an intentionally duplicated local copy (its fs mocks work against it), both copies carry keep-in-sync comments, and a **real-fs drift-guard test** (`packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts`) imports both copies and asserts identical resolution across real temp-dir layouts — each candidate alone, precedence pairs, all three, and the no-entry → `null` case. Real directories are the only seam that exercises both implementations equally; a candidate-list change applied to one copy but not the other now fails CI. ## Why This Works @@ -75,9 +80,13 @@ The Settings card sends a relative `./plugins/` path. The server resolves it - **When adding a bundled plugin, grep for an existing one** (e.g. `rg -l "fusion-plugin-roadmap" packages/` ) and mirror every hit — that surfaces all four lists plus view registration. - Route tests must force the fallback: mock fs so the cwd-relative path **misses** and only `dist/plugins/` exists (see "installs bundled compound engineering plugin when relative path misses cwd" in `packages/dashboard/src/__tests__/plugin-routes.test.ts`). A mock that matches any path containing the plugin id tests nothing. +- **Pin assertions to the exact contract, not substring containment.** `stringContaining(pluginId)` passed for both the correct entry-file path and the buggy directory path — when a mock or matcher can satisfy both the correct and the buggy value, the test proves nothing. Route tests now assert the registered path ends in an entry-file suffix, cover the `dist/index.js` and `src/index.ts` fallbacks, and the 400 no-entry branch. +- When duplicating a helper is forced by test infrastructure (fs mocks vs externalized deps), add a real-fs drift-guard test that runs every copy against the same on-disk fixtures and asserts identical output. - Consider a future consistency test asserting every `BUILTIN_PLUGINS` UI entry with a `path` is present in both server-side `BUNDLED_PLUGIN_IDS` sets. ## Related Issues -- PR #1423 — the fix +- PR #1423 — the registration-drift fix +- PR #1428 — the entry-file/heal follow-up fix +- Issue #1096 — same Settings-install bundled-plugin failure family (missing-bundle symptom for the Paperclip runtime in global npm installs); different root cause - Commit `ff0750cd1` — added CE/Roadmap to the UI list (2 of 4 registrations) From 66f1db122b9613e87c1dca442dc08daadca95e3d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 23:02:58 -0700 Subject: [PATCH 08/21] Fix quality-backfill suites broken by fast-tests x workflow-columns merge race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main went red when the fast-tests quality-backfill projects (PR #1385) landed alongside the workflow-columns stream (PR #1424) — the new test projects were written against pre-stream code: - TaskFieldsSection.css toggle knob used background: #fff, violating the theme-token assertion in AgentListModal's styling-parity test; use var(--card) per the SkillsView toggle convention - ListView.test.tsx api mock lacked fetchBoardWorkflows (TaskDetailModal now calls it on mount) - chat.test.ts and routes-agent-import.test.ts @fusion/core mocks lacked registerTraitHookImpl (engine merge-trait registers hooks at import) - auto-merge-toggle-blank.mobile and board-mobile-initial-render used vi.runAllTimers(), which never terminates now that sse-bus starts a keepalive setInterval; use vi.runOnlyPendingTimers() Both quality-backfill projects now pass fully: 7151/7151 across 414 files. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app/components/TaskFieldsSection.css | 2 +- .../app/components/__tests__/ListView.test.tsx | 1 + .../auto-merge-toggle-blank.mobile.test.tsx | 18 +++++++++--------- .../board-mobile-initial-render.test.tsx | 8 ++++---- packages/dashboard/src/__tests__/chat.test.ts | 1 + .../src/__tests__/routes-agent-import.test.ts | 1 + 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/dashboard/app/components/TaskFieldsSection.css b/packages/dashboard/app/components/TaskFieldsSection.css index 6e0d69f31e..95181aef57 100644 --- a/packages/dashboard/app/components/TaskFieldsSection.css +++ b/packages/dashboard/app/components/TaskFieldsSection.css @@ -139,7 +139,7 @@ width: 14px; height: 14px; border-radius: 50%; - background: #fff; + background: var(--card); transition: transform 0.15s ease; } diff --git a/packages/dashboard/app/components/__tests__/ListView.test.tsx b/packages/dashboard/app/components/__tests__/ListView.test.tsx index a835b80587..c80aacc9a3 100644 --- a/packages/dashboard/app/components/__tests__/ListView.test.tsx +++ b/packages/dashboard/app/components/__tests__/ListView.test.tsx @@ -23,6 +23,7 @@ vi.mock("../../api", () => ({ fetchTaskDetail: vi.fn(), batchUpdateTaskModels: vi.fn(), fetchNodes: vi.fn().mockResolvedValue([]), + fetchBoardWorkflows: vi.fn().mockResolvedValue({ flagEnabled: false, defaultWorkflowId: "", workflows: [], taskWorkflowIds: {} }), })); import { fetchTaskDetail, batchUpdateTaskModels, fetchNodes } from "../../api"; diff --git a/packages/dashboard/app/components/__tests__/auto-merge-toggle-blank.mobile.test.tsx b/packages/dashboard/app/components/__tests__/auto-merge-toggle-blank.mobile.test.tsx index b825adf972..b639379313 100644 --- a/packages/dashboard/app/components/__tests__/auto-merge-toggle-blank.mobile.test.tsx +++ b/packages/dashboard/app/components/__tests__/auto-merge-toggle-blank.mobile.test.tsx @@ -209,13 +209,13 @@ describe("auto-merge toggle mobile blank regression", () => { expectBoardVisible(); act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); board.scrollLeft = 240; act(() => { visualViewport.dispatchResize(); - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expect(board.scrollLeft).toBe(0); @@ -227,7 +227,7 @@ describe("auto-merge toggle mobile blank regression", () => { board.scrollLeft = 240; act(() => { visualViewport.dispatchResize(); - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expectBoardVisible(); @@ -248,7 +248,7 @@ describe("auto-merge toggle mobile blank regression", () => { const board = document.querySelector("main.board") as HTMLElement; act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); const toggle = screen.getByRole("checkbox", { name: "Auto-merge" }); @@ -260,7 +260,7 @@ describe("auto-merge toggle mobile blank regression", () => { board.scrollLeft = 180; act(() => { visualViewport.dispatchResize(); - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expectBoardVisible(); expect(board.scrollLeft).toBe(0); @@ -270,7 +270,7 @@ describe("auto-merge toggle mobile blank regression", () => { board.scrollLeft = 180; act(() => { visualViewport.dispatchResize(); - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expectBoardVisible(); expect(board.scrollLeft).toBe(0); @@ -296,7 +296,7 @@ describe("auto-merge toggle mobile blank regression", () => { ); act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expect(screen.getByTestId("task-card-FN-5936")).toHaveTextContent("true"); @@ -322,7 +322,7 @@ describe("auto-merge toggle mobile blank regression", () => { const board = document.querySelector("main.board") as HTMLElement; act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); fireEvent.click(screen.getByRole("checkbox", { name: "Auto-merge" })); @@ -332,7 +332,7 @@ describe("auto-merge toggle mobile blank regression", () => { Object.defineProperty(pageShow, "persisted", { configurable: true, value: true }); act(() => { window.dispatchEvent(pageShow); - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expectBoardVisible(); diff --git a/packages/dashboard/app/components/__tests__/board-mobile-initial-render.test.tsx b/packages/dashboard/app/components/__tests__/board-mobile-initial-render.test.tsx index 038e12dc0f..b13d9b4e6d 100644 --- a/packages/dashboard/app/components/__tests__/board-mobile-initial-render.test.tsx +++ b/packages/dashboard/app/components/__tests__/board-mobile-initial-render.test.tsx @@ -109,7 +109,7 @@ describe("Board mobile initial render stabilization (FN-4574)", () => { board.scrollLeft = 500; act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expect(board.scrollLeft).toBe(0); expect(raf).toHaveBeenCalled(); @@ -130,7 +130,7 @@ describe("Board mobile initial render stabilization (FN-4574)", () => { const board = document.querySelector("main.board") as HTMLElement; act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expect(board.scrollLeft).toBe(0); @@ -140,7 +140,7 @@ describe("Board mobile initial render stabilization (FN-4574)", () => { window.dispatchEvent(pageShow); act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expect(board.scrollLeft).toBe(0); @@ -190,7 +190,7 @@ describe("Board mobile initial render stabilization (FN-4574)", () => { window.dispatchEvent(pageShow); act(() => { - vi.runAllTimers(); + vi.runOnlyPendingTimers(); }); expect(board.scrollLeft).toBe(500); expect(addEventListenerSpy).not.toHaveBeenCalledWith("pageshow", expect.any(Function)); diff --git a/packages/dashboard/src/__tests__/chat.test.ts b/packages/dashboard/src/__tests__/chat.test.ts index 1fadb23015..e4b205d143 100644 --- a/packages/dashboard/src/__tests__/chat.test.ts +++ b/packages/dashboard/src/__tests__/chat.test.ts @@ -28,6 +28,7 @@ vi.mock("@fusion/core", () => ({ summarizeTitle: vi.fn(), AgentStore: vi.fn(), ChatStore: vi.fn(), + registerTraitHookImpl: vi.fn(), })); describe("resolveFileReferences", () => { diff --git a/packages/dashboard/src/__tests__/routes-agent-import.test.ts b/packages/dashboard/src/__tests__/routes-agent-import.test.ts index 82f9997ff4..5df5cb43a2 100644 --- a/packages/dashboard/src/__tests__/routes-agent-import.test.ts +++ b/packages/dashboard/src/__tests__/routes-agent-import.test.ts @@ -75,6 +75,7 @@ vi.mock("@fusion/core", () => { isEphemeralAgent: (agent: { metadata?: Record }) => agent?.metadata?.agentKind === "task-worker", deterministicGuardLocks: new Map(), + registerTraitHookImpl: () => {}, }; }); From f5f09c6e37c99261b5dccb335307d0515a28a82d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 23:26:44 -0700 Subject: [PATCH 09/21] docs(plans): column agent assignment plan --- ...4-002-feat-column-agent-assignment-plan.md | 345 ++++++++++++++++++ 1 file changed, 345 insertions(+) create mode 100644 docs/plans/2026-06-04-002-feat-column-agent-assignment-plan.md 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..fff98413ed --- /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: active +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`). From e421774f6aff9b9b166d9d719ec6db3e73e029d2 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 23:35:32 -0700 Subject: [PATCH 10/21] feat(core): column agent IR schema, validation, and effective-agent resolver U1+U2 of the column-agent plan: WorkflowColumnAgent on WorkflowIrColumn (defer/override), template-subgraph column validation, v2-only-feature registration, plugin-sdk type parity, and the shared core resolver with instanceNodeId format ownership moved to core. --- .../__tests__/column-agent-resolver.test.ts | 258 ++++++++++++++++++ .../workflow-ir-column-agent.test.ts | 224 +++++++++++++++ packages/core/src/column-agent-resolver.ts | 186 +++++++++++++ packages/core/src/index.ts | 12 + packages/core/src/workflow-ir-types.ts | 21 ++ packages/core/src/workflow-ir.ts | 43 ++- packages/engine/src/workflow-graph-foreach.ts | 10 +- packages/plugin-sdk/src/index.ts | 4 + 8 files changed, 750 insertions(+), 8 deletions(-) create mode 100644 packages/core/src/__tests__/column-agent-resolver.test.ts create mode 100644 packages/core/src/__tests__/workflow-ir-column-agent.test.ts create mode 100644 packages/core/src/column-agent-resolver.ts 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..5fd19903ee --- /dev/null +++ b/packages/core/src/__tests__/column-agent-resolver.test.ts @@ -0,0 +1,258 @@ +// @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 × 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(); + }); +}); + +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/column-agent-resolver.ts b/packages/core/src/column-agent-resolver.ts new file mode 100644 index 0000000000..6ec7dc579b --- /dev/null +++ b/packages/core/src/column-agent-resolver.ts @@ -0,0 +1,186 @@ +/** + * 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. */ +export function parseInstanceNodeId(nodeId: string): ParsedInstanceNodeId | undefined { + const hashIndex = nodeId.indexOf("#"); + if (hashIndex < 0) return 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 }; +} + +// ── 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. + const parsed = parseInstanceNodeId(nodeId); + if (!parsed) return undefined; + + const foreachNode = nodesById.get(parsed.foreachNodeId); + if (!foreachNode || foreachNode.kind !== "foreach") return undefined; + + const cfg = foreachNode.config as Partial | undefined; + const templateNodes = cfg?.template?.nodes ?? []; + const templateNode = templateNodes.find((n) => n.id === parsed.templateNodeId); + + // 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); +} + +// ── 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 999eb211cb..c14ef74fde 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"; 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/engine/src/workflow-graph-foreach.ts b/packages/engine/src/workflow-graph-foreach.ts index 9c0ee826c0..67160499e3 100644 --- a/packages/engine/src/workflow-graph-foreach.ts +++ b/packages/engine/src/workflow-graph-foreach.ts @@ -1,5 +1,5 @@ import type { TaskDetail, TaskStep, WorkflowIrEdge, WorkflowIrNode } from "@fusion/core"; -import { WorkflowIrError } from "@fusion/core"; +import { WorkflowIrError, instanceNodeId } from "@fusion/core"; import type { WorkflowNodeOutcome, WorkflowNodeResult } from "./workflow-graph-executor.js"; import { @@ -247,10 +247,10 @@ export interface ForeachRunResult { visitedNodeIds: string[]; } -/** Materialize a deterministic instance node id (KTD-3) — pure, no IR mutation. */ -export function instanceNodeId(foreachNodeId: string, stepIndex: number, templateNodeId: string): string { - return `${foreachNodeId}#${stepIndex}:${templateNodeId}`; -} +// `instanceNodeId` now lives in `@fusion/core` (column-agent plan KTD-2) so the +// instance-id format has exactly one owner. Re-exported here (the imported binding) +// for back-compat with any local callers; the format is unchanged. +export { instanceNodeId }; /** Resolve the foreach config, validating the bits this module relies on. */ function resolveForeachConfig(node: WorkflowIrNode): { diff --git a/packages/plugin-sdk/src/index.ts b/packages/plugin-sdk/src/index.ts index d9e87b586e..a53b4267a7 100644 --- a/packages/plugin-sdk/src/index.ts +++ b/packages/plugin-sdk/src/index.ts @@ -109,6 +109,10 @@ export type { WorkflowIrNode, WorkflowIrEdge, WorkflowIrNodeKind, + // Columns + per-column permanent-agent binding (column-agent plan KTD-1, R12). + WorkflowIrColumn, + WorkflowIrColumnTrait, + WorkflowColumnAgent, // Foreach / artifacts / custom fields (step inversion). WorkflowForeachConfig, WorkflowIrArtifact, From d88bfc4c75594880b2eea4a609b8563e1754a7e8 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 23:53:22 -0700 Subject: [PATCH 11/21] feat(engine): custom workflow nodes run as their column's agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit U3: per-run IR resolution feeds the core column-agent resolver at the runCustomNode seam; override supersedes node agent/model/persona wholesale, defer fills bare nodes only; adoption and fallback are audited via logEntry; raw-CLI nodes log a skip. Also fixes the customInstructions persona drift — node-level executor:"agent" persona injection now uses the typed soul/instructionsText fields (KTD-6). --- .../executor-column-agent-custom-node.test.ts | 217 ++++++++++++++++++ packages/engine/src/executor.ts | 143 +++++++++++- 2 files changed, 354 insertions(+), 6 deletions(-) create mode 100644 packages/engine/src/__tests__/executor-column-agent-custom-node.test.ts diff --git a/packages/engine/src/__tests__/executor-column-agent-custom-node.test.ts b/packages/engine/src/__tests__/executor-column-agent-custom-node.test.ts new file mode 100644 index 0000000000..c7a854c3d2 --- /dev/null +++ b/packages/engine/src/__tests__/executor-column-agent-custom-node.test.ts @@ -0,0 +1,217 @@ +// Column-agent custom-node resolution (plan U3, R2/R3/R4/R8, KTD-2/KTD-3/KTD-6). +// +// `runGraphCustomNode` synthesizes a `WorkflowStep` and runs it on the proven +// WorkflowStep machinery. The seam wiring (maybeExecuteWorkflowGraph) resolves +// the per-node column-agent binding and threads it in as a parameter. These +// tests call `runGraphCustomNode` directly with that binding and assert the +// synthesized step's model/persona plus the audit log entries — mirroring the +// established executor harness (executor-workflow-step-scope.test.ts): build a +// real TaskExecutor over a mock store and spy on `executeWorkflowStep` / +// `executeScriptWorkflowStep` to capture the synthesized step. + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; +import type { WorkflowColumnAgent } from "@fusion/core"; + +function makeAgent(overrides: Record = {}) { + return { + id: "agent-col", + name: "Column Agent", + soul: "I am the senior reviewer.", + instructionsText: "Always be thorough.", + runtimeConfig: { executorProvider: "anthropic", executorModelId: "claude-col" }, + ...overrides, + }; +} + +function makeExecutor(store: ReturnType, agent: unknown | null) { + const agentStore = { + getAgent: vi.fn().mockResolvedValue(agent), + }; + const executor = new TaskExecutor(store as any, "/tmp/test", { agentStore } as any); + return { executor, agentStore }; +} + +/** Spy both session-running paths; return the captured synthesized step. */ +function spyStep(executor: TaskExecutor) { + const captured: { step?: any } = {}; + vi.spyOn(executor as any, "executeWorkflowStep").mockImplementation(async (...args: any[]) => { + captured.step = args[1]; + return { success: true, output: "ok" }; + }); + vi.spyOn(executor as any, "executeScriptWorkflowStep").mockImplementation(async (...args: any[]) => { + captured.step = args[1]; + return { success: true, output: "ok" }; + }); + return captured; +} + +function loggedLines(store: ReturnType): string[] { + return store.logEntry.mock.calls.map((call: any[]) => String(call[1] ?? "")); +} + +const OVERRIDE: WorkflowColumnAgent = { agentId: "agent-col", mode: "override" }; +const DEFER: WorkflowColumnAgent = { agentId: "agent-col", mode: "defer" }; + +describe("runGraphCustomNode column-agent resolution (plan U3)", () => { + beforeEach(() => { + resetExecutorMocks(); + }); + + it("override column: node with own cfg.agentId runs as column agent (model+persona) and logs substitution+mode", async () => { + const store = createMockStore(); + store.getTask.mockResolvedValue({ id: "FN-001", worktree: "/tmp/wt", log: [] } as any); + const { executor, agentStore } = makeExecutor(store, makeAgent()); + const captured = spyStep(executor); + + const node = { + id: "review", + kind: "prompt", + column: "review", + config: { + executor: "agent", + agentId: "node-own-agent", + modelProvider: "openai", + modelId: "gpt-node", + prompt: "Review the diff.", + }, + }; + + const result = await (executor as any).runGraphCustomNode(node, { id: "FN-001" }, {}, OVERRIDE); + + expect(result.outcome).toBe("success"); + // Column agent fetched (not the node's own agent). + expect(agentStore.getAgent).toHaveBeenCalledWith("agent-col"); + // Column agent's model wins over the node's own pair. + expect(captured.step.modelProvider).toBe("anthropic"); + expect(captured.step.modelId).toBe("claude-col"); + // Column agent's persona (soul + instructionsText) prepended to the prompt. + expect(captured.step.prompt).toContain("I am the senior reviewer."); + expect(captured.step.prompt).toContain("Always be thorough."); + expect(captured.step.prompt).toContain("Review the diff."); + // Audit log records substitution + mode. + expect( + loggedLines(store).some((l) => l.includes("running as column agent 'agent-col' (override)")), + ).toBe(true); + }); + + it("defer column: node with own cfg.agentId keeps it; bare node adopts the column agent", async () => { + // (a) own agentId present → defer yields own settings, column agent untouched. + { + const store = createMockStore(); + store.getTask.mockResolvedValue({ id: "FN-001", worktree: "/tmp/wt", log: [] } as any); + const nodeOwnAgent = makeAgent({ id: "node-own-agent", soul: "node persona", instructionsText: "", runtimeConfig: { executorProvider: "openai", executorModelId: "gpt-node" } }); + const { executor, agentStore } = makeExecutor(store, nodeOwnAgent); + const captured = spyStep(executor); + + const node = { + id: "review", + kind: "prompt", + column: "review", + config: { executor: "agent", agentId: "node-own-agent", prompt: "Do it." }, + }; + await (executor as any).runGraphCustomNode(node, { id: "FN-001" }, {}, DEFER); + + // Own agent fetched, NOT the column agent. + expect(agentStore.getAgent).toHaveBeenCalledWith("node-own-agent"); + expect(agentStore.getAgent).not.toHaveBeenCalledWith("agent-col"); + expect(captured.step.modelProvider).toBe("openai"); + expect(captured.step.modelId).toBe("gpt-node"); + expect( + loggedLines(store).some((l) => l.includes("running as column agent")), + ).toBe(false); + } + + // (b) bare node (no own agent/model) → defer adopts the column agent. + { + const store = createMockStore(); + store.getTask.mockResolvedValue({ id: "FN-001", worktree: "/tmp/wt", log: [] } as any); + const { executor, agentStore } = makeExecutor(store, makeAgent()); + const captured = spyStep(executor); + + const node = { id: "review", kind: "prompt", column: "review", config: { prompt: "Plain." } }; + await (executor as any).runGraphCustomNode(node, { id: "FN-001" }, {}, DEFER); + + expect(agentStore.getAgent).toHaveBeenCalledWith("agent-col"); + expect(captured.step.modelProvider).toBe("anthropic"); + expect(captured.step.modelId).toBe("claude-col"); + expect(captured.step.prompt).toContain("I am the senior reviewer."); + expect( + loggedLines(store).some((l) => l.includes("running as column agent 'agent-col' (defer)")), + ).toBe(true); + } + }); + + it("missing column agent in registry → logged, node falls back, step still executes", async () => { + const store = createMockStore(); + store.getTask.mockResolvedValue({ id: "FN-001", worktree: "/tmp/wt", log: [] } as any); + // agentStore returns null for the column agent. + const { executor } = makeExecutor(store, null); + const captured = spyStep(executor); + + const node = { id: "review", kind: "prompt", column: "review", config: { prompt: "Plain." } }; + const result = await (executor as any).runGraphCustomNode(node, { id: "FN-001" }, {}, OVERRIDE); + + expect(result.outcome).toBe("success"); + // No column-agent model adopted (agent missing) → step has no model pair. + expect(captured.step.modelProvider).toBeUndefined(); + expect(captured.step.modelId).toBeUndefined(); + expect( + loggedLines(store).some((l) => l.includes("column agent 'agent-col' not found")), + ).toBe(true); + }); + + it("node with no declared column → untouched resolution even when a binding is passed as undefined", async () => { + const store = createMockStore(); + store.getTask.mockResolvedValue({ id: "FN-001", worktree: "/tmp/wt", log: [] } as any); + const { executor, agentStore } = makeExecutor(store, makeAgent()); + const captured = spyStep(executor); + + // No declared column → the seam wiring resolves no binding (undefined). + const node = { + id: "review", + kind: "prompt", + config: { executor: "model", modelProvider: "openai", modelId: "gpt-node", prompt: "Plain." }, + }; + await (executor as any).runGraphCustomNode(node, { id: "FN-001" }, {}, undefined); + + // Column agent never fetched; node's own model preserved. + expect(agentStore.getAgent).not.toHaveBeenCalled(); + expect(captured.step.modelProvider).toBe("openai"); + expect(captured.step.modelId).toBe("gpt-node"); + expect(loggedLines(store).some((l) => l.includes("column agent"))).toBe(false); + }); + + it("CLI-executor node (raw command) in override column → mechanics unchanged, audit notes the skip", async () => { + const store = createMockStore(); + store.getTask.mockResolvedValue({ id: "FN-001", worktree: "/tmp/wt", log: [] } as any); + store.isWorkflowCliCommandApproved = vi.fn().mockResolvedValue(true); + const { executor, agentStore } = makeExecutor(store, makeAgent()); + // Raw CLI runs runRawCliCommand, not a session — stub it. + const rawSpy = vi.spyOn(executor as any, "runRawCliCommand").mockResolvedValue({ success: true }); + + const node = { + id: "lint", + kind: "script", + column: "review", + config: { executor: "cli", cliCommand: "npm run lint", cliSkipApproval: true, prompt: "" }, + }; + const result = await (executor as any).runGraphCustomNode(node, { id: "FN-001" }, {}, OVERRIDE); + + expect(result.outcome).toBe("success"); + // Raw CLI mechanics unchanged: command still ran. + expect(rawSpy).toHaveBeenCalled(); + // Column agent NOT fetched/adopted for raw CLI execution. + expect(agentStore.getAgent).not.toHaveBeenCalled(); + // Audit explains the skip. + expect( + loggedLines(store).some( + (l) => + l.includes("column agent 'agent-col' (override) not applied") && + l.includes("raw CLI execution runs no session"), + ), + ).toBe(true); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 3a29414187..e1f3028606 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -9,8 +9,8 @@ import { delimiter, isAbsolute, join, relative, resolve as resolvePath } from "n import { existsSync, realpathSync } from "node:fs"; import { readFile, rm, writeFile } from "node:fs/promises"; import type { TaskStore, Task, TaskDetail, TaskTokenUsage, StepStatus, Settings, WorkflowStep, MissionStore, Slice, AgentState, AgentCapability, RunMutationContext, AgentHeartbeatConfig, Agent, AgentMemoryInclusionMode, ProjectSettings, MergeResult, WorkflowIrNode } from "@fusion/core"; -import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, resolveWorkflowIrForTask } from "@fusion/core"; -import type { TaskStep, WorkflowIr, WorkflowFieldDefinition } from "@fusion/core"; +import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, resolveWorkflowIrForTask, resolveColumnAgentBinding, resolveEffectiveAgent } from "@fusion/core"; +import type { TaskStep, WorkflowIr, WorkflowFieldDefinition, WorkflowColumnAgent } from "@fusion/core"; import { buildWorkflowObservationFromTask, buildWorkflowObservation, @@ -3320,11 +3320,27 @@ export class TaskExecutor { // Definition load failure — leave undefined; deps/runner use fallbacks. } + // Column-agent binding (plan U3): the IR is NOT in scope inside + // runGraphCustomNode, so resolve it here (the seam wiring) where the + // selection is known, and thread a per-node binding lookup into the custom + // node callback. Resolve the IR ONCE per run (never an uncached per-node + // fetch — mirrors the hold-release.ts irCache posture); best-effort, so a + // resolution failure simply yields no bindings (R8 graceful degradation). + let columnAgentIr: WorkflowIr | undefined; + try { + columnAgentIr = await resolveWorkflowIrForTask(this.store, task.id); + } catch { + columnAgentIr = undefined; + } + const resolveBindingForNode = (nodeId: string): WorkflowColumnAgent | undefined => + columnAgentIr ? resolveColumnAgentBinding(columnAgentIr, nodeId) : undefined; + const runner = new WorkflowGraphTaskRunner({ store: this.store, runId: resolvedRunId, seams: this.createGraphSeams(settings), - runCustomNode: (node, nodeTask) => this.runGraphCustomNode(node, nodeTask, settings), + runCustomNode: (node, nodeTask) => + this.runGraphCustomNode(node, nodeTask, settings, resolveBindingForNode(node.id)), onEvent: (event) => executorLog.log(`[workflow-graph] ${event.type} ${event.taskId}: ${event.detail}`), // Wire SQLite-backed per-branch persistence in production (#1407): the // executor writes each branch's currentNodeId/status to @@ -4495,11 +4511,77 @@ export class TaskExecutor { } } - /** Run a custom (non-seam) graph node on the proven WorkflowStep machinery. */ + /** Build the persona prefix for an agent from its TYPED identity fields (KTD-6). + * Reads `soul` and `instructionsText` — the fields the `Agent` type actually + * exposes (`packages/core/src/types.ts`) — and joins them. The custom-node + * `"agent"` branch historically read a non-existent `customInstructions` + * field (silently undefined); this is the single consistent source used by + * both the node-agent and column-agent paths. */ + private buildAgentPersona(agent: Agent): string | undefined { + const parts = [agent.soul, agent.instructionsText] + .map((p) => (typeof p === "string" ? p.trim() : "")) + .filter((p) => p.length > 0); + return parts.length > 0 ? parts.join("\n\n") : undefined; + } + + /** Fetch the column agent and surface its model + persona for adoption by a + * custom node (plan U3). Best-effort, mirroring the node-agent posture at the + * `"agent"` branch: on null/throw, log and return undefined so the caller + * falls back to the node's own/default resolution (R8). Emits a logEntry + * naming the substitution and mode so the audit trail explains who ran. */ + private async adoptColumnAgentForNode( + node: WorkflowIrNode, + live: TaskDetail, + columnAgentId: string, + mode: WorkflowColumnAgent["mode"] | undefined, + ): Promise<{ modelProvider?: string; modelId?: string; persona?: string } | undefined> { + try { + const agent = await this.options.agentStore?.getAgent(columnAgentId); + if (!agent) { + await this.store.logEntry( + live.id, + `Workflow node '${node.id}': column agent '${columnAgentId}' not found — falling back to node/default resolution`, + undefined, + this.getRunContextFor(live.id), + ); + return undefined; + } + const rc = (agent.runtimeConfig ?? {}) as { executorProvider?: string; executorModelId?: string }; + await this.store.logEntry( + live.id, + `Workflow node '${node.id}': running as column agent '${columnAgentId}' (${mode})`, + undefined, + this.getRunContextFor(live.id), + ); + return { + modelProvider: rc.executorProvider, + modelId: rc.executorModelId, + persona: this.buildAgentPersona(agent), + }; + } catch { + // Agent lookup is best-effort; fall back to node/default resolution (R8). + await this.store.logEntry( + live.id, + `Workflow node '${node.id}': column agent '${columnAgentId}' lookup failed — falling back to node/default resolution`, + undefined, + this.getRunContextFor(live.id), + ); + return undefined; + } + } + + /** Run a custom (non-seam) graph node on the proven WorkflowStep machinery. + * + * `columnBinding` (plan U3) is the agent binding governing this node's + * declared column, resolved by the seam wiring in maybeExecuteWorkflowGraph + * (the IR is not in scope here). When present, the core resolver decides + * whether the column agent supersedes (override) or defers to the node's own + * `cfg.agentId`/model pair — never a reimplemented precedence. */ private async runGraphCustomNode( node: WorkflowIrNode, nodeTask: TaskDetail, settings: Settings, + columnBinding?: WorkflowColumnAgent, ): Promise { const cfg = node.config ?? {}; const live = await this.store.getTask(nodeTask.id); @@ -4536,6 +4618,51 @@ export class TaskExecutor { let modelProvider = typeof cfg.modelProvider === "string" && cfg.modelProvider.trim() ? cfg.modelProvider : undefined; let modelId = typeof cfg.modelId === "string" && cfg.modelId.trim() ? cfg.modelId : undefined; + // ── Column-agent binding (plan U3, KTD-2/KTD-3) ────────────────────────── + // When the node's declared column names an agent, the CORE resolver decides + // whether the column agent supersedes (override) or defers to the node's own + // settings — we never reimplement precedence. The node's own `cfg.agentId` + // and complete model pair feed the resolver as "own settings" (KTD-5). + const ownModelComplete = Boolean(modelProvider && modelId); + const effective = resolveEffectiveAgent({ + binding: columnBinding, + ownAgentId: typeof cfg.agentId === "string" && cfg.agentId.trim() ? cfg.agentId.trim() : undefined, + ownModelProvider: ownModelComplete ? modelProvider : undefined, + ownModelId: ownModelComplete ? modelId : undefined, + }); + // The effective executor identity: a column agent supersedes the node's own + // `executor: "agent"` adoption wholesale (identity + model + persona). When + // the resolver yields the column agent, we run the column-agent adoption + // path below INSTEAD of the node's own agent branch. + const columnAgentId = effective.source === "column-agent" ? effective.agentId : undefined; + const columnAgentMode = columnBinding?.mode; + + if (columnAgentId) { + // CLI executor with a raw command runs no session — the column agent + // cannot contribute a model/persona to raw process execution, so it is a + // no-op here. Log the skip so the audit trail explains why the column + // agent did not apply (plan U3). Skill / model / script-via-session nodes + // DO adopt the column agent below. + if (executorKind === "cli" && rawCliCommand) { + await this.store.logEntry( + live.id, + `Workflow node '${node.id}': column agent '${columnAgentId}' (${columnAgentMode}) not applied — raw CLI execution runs no session`, + undefined, + this.getRunContextFor(live.id), + ); + } else { + const adopted = await this.adoptColumnAgentForNode(node, live, columnAgentId, columnAgentMode); + if (adopted) { + modelProvider = adopted.modelProvider ?? modelProvider; + modelId = adopted.modelId ?? modelId; + if (adopted.persona) prompt = `${adopted.persona}\n\n${prompt}`; + } + // Whether or not the agent resolved, the column agent SUPERSEDES the + // node's own `executor: "agent"` adoption — skip that branch so we never + // blend the column agent's model with the node agent's persona. + } + } + // Executor kinds for prompt nodes: // - "model" (default): run the prompt on the configured/override model. // - "agent": run as a named agent — adopt its model and persona prompt. @@ -4543,14 +4670,18 @@ export class TaskExecutor { // - "cli": run a named project script with the prompt passed via env // (FUSION_NODE_PROMPT). Named scripts only — raw commands are // never accepted from node config. - if (executorKind === "agent" && typeof cfg.agentId === "string" && cfg.agentId.trim()) { + if (!columnAgentId && executorKind === "agent" && typeof cfg.agentId === "string" && cfg.agentId.trim()) { try { const agent = await this.options.agentStore?.getAgent(cfg.agentId); if (agent) { const rc = (agent.runtimeConfig ?? {}) as { executorProvider?: string; executorModelId?: string }; modelProvider = rc.executorProvider ?? modelProvider; modelId = rc.executorModelId ?? modelId; - const persona = (agent as { customInstructions?: string }).customInstructions; + // KTD-6: read the TYPED persona fields (soul / instructionsText), not + // the non-existent `customInstructions` (which was silently undefined, + // so node-agent persona injection never actually fired). Same fields + // the column-agent path uses — one consistent persona source. + const persona = this.buildAgentPersona(agent); if (persona) prompt = `${persona}\n\n${prompt}`; } else { await this.store.logEntry(live.id, `Workflow node '${node.id}': agent '${cfg.agentId}' not found — using default model`, undefined, this.getRunContextFor(live.id)); From 75ebe23b4ffe429b4796357c43573f4148ed08d6 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 4 Jun 2026 23:53:22 -0700 Subject: [PATCH 12/21] feat(dashboard): column agent picker, override visibility, write-time validation U6: WorkflowColumnPanel agent picker + defer/override toggle with specified interaction states (flags-off hint, loading, fetch-error, stale-agent warning, bound-column badge); WorkflowNodeEditor overridden-by-column-agent note + stale-id treatment; assertColumnAgentsExist + confirmPolicyEscalation gate (R13) on workflow save routes; flowToIr now preserves column agent bindings through the editor round-trip. --- .../app/components/WorkflowColumnPanel.tsx | 197 +++++++++++++++++- .../app/components/WorkflowNodeEditor.tsx | 103 +++++++-- .../__tests__/WorkflowNodeEditor.test.tsx | 163 ++++++++++++++- .../app/components/workflow-flow-mapping.ts | 13 +- .../src/__tests__/workflow-routes.test.ts | 136 ++++++++++++ .../src/routes/register-workflow-routes.ts | 120 ++++++++++- 6 files changed, 707 insertions(+), 25 deletions(-) diff --git a/packages/dashboard/app/components/WorkflowColumnPanel.tsx b/packages/dashboard/app/components/WorkflowColumnPanel.tsx index 10c3b3ca1a..74c11558d8 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,90 @@ 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], + ); + + const agentPickerDisabled = readOnly || !columnAgentsEnabled || agentsLoading; + const workflowWide = violations.filter((v) => v.columnId === null); const violationsFor = useCallback( (columnId: string) => violations.filter((v) => v.columnId === columnId), @@ -135,6 +230,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..bc5f094515 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(() => { @@ -554,6 +563,25 @@ function InnerEditor({ 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(() => { + const columnId = selectedNode?.data.column; + 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, 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) { @@ -587,6 +615,22 @@ 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; + Promise.resolve(fetchAgents()).then((list) => { + if (!cancelled) setAgents(list ?? []); + }).catch((err) => { + if (!cancelled) addToast(getErrorMessage(err) || "Failed to load agents", "error"); + }); + return () => { + cancelled = true; + }; + }, [overrideColumnBinding, agents.length, addToast]); + const overlayProps = useOverlayDismiss(onClose); return ( @@ -722,6 +766,7 @@ function InnerEditor({ readOnly={isBuiltin} projectId={projectId} addToast={addToast} + columnAgentsEnabled={columnAgentsEnabled} /> )} @@ -777,6 +822,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" && (