root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Address PR review feedback round 2 (#1432)

Browse Source 
- R10 kill-switch: workflowColumns flag now gates the execution path (resolver
  installation + resume pass 2), making the documented rollback real
- resolver: skip parse candidates whose templateNodeId doesn't exist under the
  foreach (disambiguation guard)
- editor: reset the project-scoped agents cache on projectId change
- tests: kill-switch inertness, defer→own-settings release, candidate skip
This commit is contained in:
gsxdsm
2026-06-05 13:59:50 -07:00
parent 4209195743
commit 59364e7c58
5 changed files with 88 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
packages/core/src/__tests__/column-agent-resolver.test.ts
View File

@@ -201,6 +201,14 @@ describe("resolveColumnAgentBinding — foreach instance inheritance (U2)", () =
expect(resolveColumnAgentBinding(ir, nodeId)).toBeUndefined();
});
it("skips a candidate whose templateNodeId doesn't exist under the foreach", () => {
// PR #1432 review: a bogus prefix candidate can name a real foreach while its
// parsed templateNodeId resolves to nothing — it must be skipped, not treated
// as inheriting the foreach's column.
const ir = foreachIr({ foreachColumn: "review", reviewAgent: overrideBinding });
expect(resolveColumnAgentBinding(ir, instanceNodeId("fe", 0, "nope"))).toBeUndefined();
});
it("resolves bindings when the foreach node id itself contains '#'", () => {
// The instance-id format is delimiter-ambiguous; the resolver validates each
// candidate split against real foreach nodes instead of trusting the first '#'

4
packages/core/src/column-agent-resolver.ts
View File

@@ -141,6 +141,10 @@ export function resolveColumnAgentBinding(
const cfg = foreachNode.config as Partial<WorkflowForeachConfig> | undefined;
const templateNodes = cfg?.template?.nodes ?? [];
const templateNode = templateNodes.find((n) => n.id === parsed.templateNodeId);
// Disambiguation guard (PR #1432 review): a bogus prefix candidate can name a
// real foreach while its templateNodeId doesn't exist under it — skip it so a
// later exact parse isn't masked. A template with no nodes still inherits.
if (templateNodes.length > 0 && !templateNode) continue;
// Template node's own column wins; otherwise inherit the foreach node's column.
if (templateNode?.column !== undefined) {

7
packages/dashboard/app/components/WorkflowNodeEditor.tsx
View File

@@ -586,6 +586,13 @@ function InnerEditor({
// Lazy-loaded executor resources
const [models, setModels] = useState<ModelInfo[]>([]);
const [agents, setAgents] = useState<Agent[]>([]);
// The agent fetches are project-scoped, but this cache survives project
// switches — both load paths short-circuit on agents.length > 0, which would
// keep showing (and let the editor bind) the PREVIOUS project's registry.
// Reset on project change so the next consumer refetches (PR #1432 review).
useEffect(() => {
setAgents([]);
}, [projectId]);
const [skills, setSkills] = useState<DiscoveredSkill[]>([]);
const currentExecutor = (selectedNode?.data.config?.executor as ExecutorKind | undefined) ?? "model";

49
packages/engine/src/__tests__/executor-column-agent-principal.test.ts
View File

@@ -228,7 +228,9 @@ describe("column-agent principal alignment (plan U5)", () => {
store.getSettings.mockResolvedValue({
globalPause: false,
enginePaused: false,
experimentalFeatures: { workflowGraphExecutor: true },
// R10: column agents require BOTH flags — pass 2 is gated on
// workflowColumns too (kill-switch, PR #1432 review).
experimentalFeatures: { workflowGraphExecutor: true, workflowColumns: true },
} as any);
store.listTasks.mockResolvedValue([task] as any);
store.getTaskWorkflowSelection = vi.fn().mockReturnValue({ workflowId: "wf-1", stepIds: [] });
@@ -290,6 +292,21 @@ describe("column-agent principal alignment (plan U5)", () => {
expect(executeSpy).not.toHaveBeenCalled();
});
it("kill-switch: workflowColumns off → pass 2 is inert even with a live override binding (R10)", async () => {
// The documented rollback is disabling workflowColumns alone; pass 2
// resolves the IR directly (not via the per-run resolver map), so it
// carries its own flag guard (PR #1432 review).
const task = singleSessionTask({ assignedAgentId: "agent-Y" });
const store = resumeStore(task, irWithExecuteSeamColumn(OVERRIDE_COL));
store.getSettings.mockResolvedValue({
globalPause: false,
enginePaused: false,
experimentalFeatures: { workflowGraphExecutor: true, workflowColumns: false },
} as any);
const { executor } = makeExecutor(store, { "agent-X": makeColumnAgent() });
await expect((executor as any).taskEffectiveAgentMatches(task, "agent-X")).resolves.toBe(false);
});
it("step-execute template node binding governs → pass 2 matches a foreach-template-bound column agent (walks template subgraphs)", async () => {
// R6: step-execute seam nodes live ONLY inside a foreach template, never in
// ir.nodes. Pass 2 must walk foreach template subgraphs to find them; before
@@ -527,6 +544,36 @@ describe("column-agent principal alignment (plan U5)", () => {
expect(loggedLines(store).some((l) => l.includes("binding released"))).toBe(true);
});
it("defer binding stays but the task regains own settings → release path fires (FN-5893)", async () => {
// Second release surface: the binding is still present, but a mid-flight
// task edit gave it a complete own model pair, so `defer` now resolves to
// own-settings. The watcher must release exactly like binding removal.
const store = createMockStore();
const find = vi.fn().mockReturnValue({ provider: "openai", modelId: "gpt-own" });
const task = singleSessionTask({
assignedAgentId: "agent-Y",
modelProvider: "openai",
modelId: "gpt-own",
});
const { executor } = makeExecutor(store, {
"agent-Y": makeAssignedAgent({ id: "agent-Y", runtimeConfig: { model: "openai/gpt-own" } }),
});
(executor as any)._modelRegistry = { find };
const { setModel } = activeGraphSession(executor, task.id, "exec-node", {
agentId: "agent-X",
mode: "defer",
});
(executor as any).effectiveColumnAgentByTask.set(task.id, "agent-X");
await store._triggerAsync("task:updated", task);
expect(setModel).toHaveBeenCalledWith({ provider: "openai", modelId: "gpt-own" });
expect((executor as any).activeSessions.get(task.id).lastEffectiveColumnAgentId).toBeNull();
expect(executor.isAgentEffectivelyExecuting("agent-X")).toBe(false);
expect(loggedLines(store).some((l) => l.includes("binding released"))).toBe(true);
});
it("legacy entry (no effective column agent) → the column-invalidation block is skipped", async () => {
const store = createMockStore();
const find = vi.fn();

27
packages/engine/src/executor.ts
View File

@@ -9,7 +9,7 @@ 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, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId } from "@fusion/core";
import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, isWorkflowColumnsEnabled, resolveWorkflowIrForTask, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId } from "@fusion/core";
import type { TaskStep, WorkflowIr, WorkflowFieldDefinition, WorkflowColumnAgent, EffectiveAgentInput } from "@fusion/core";
import {
buildWorkflowObservationFromTask,
@@ -3295,6 +3295,11 @@ export class TaskExecutor {
* `resumeTaskForAgent` second pass to re-dispatch column-bound tasks the
* `assignedAgentId` filter misses. Best-effort: an unresolvable IR yields false. */
private async taskEffectiveAgentMatches(task: Task, agentId: string): Promise<boolean> {
// R10 kill-switch (PR #1432 review): this path resolves the IR directly (it
// does not go through the per-run resolver map), so it needs its own flag
// guard — resume pass 2 must be inert when workflowColumns is off.
const settings = await this.store.getSettings();
if (!isWorkflowColumnsEnabled(settings)) return false;
const ir = await resolveWorkflowIrForTask(this.store, task.id);
if (!ir || ir.version !== "v2") return false;
@@ -3597,11 +3602,19 @@ export class TaskExecutor {
// 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).
// R10 kill-switch (PR #1432 review): column agents require BOTH flags. The
// graph executor gate above covers workflowGraphExecutor; this guard makes
// disabling workflowColumns alone actually render bindings inert at
// execution time (the documented rollback) — no resolver installed means
// every downstream consumer (custom nodes, seams, watcher) sees no binding.
const columnAgentsEnabled = isWorkflowColumnsEnabled(settings);
let columnAgentIr: WorkflowIr | undefined;
try {
columnAgentIr = await resolveWorkflowIrForTask(this.store, task.id);
} catch {
columnAgentIr = undefined;
if (columnAgentsEnabled) {
try {
columnAgentIr = await resolveWorkflowIrForTask(this.store, task.id);
} catch {
columnAgentIr = undefined;
}
}
const resolveBindingForNode = (nodeId: string): WorkflowColumnAgent | undefined =>
columnAgentIr ? resolveColumnAgentBinding(columnAgentIr, nodeId) : undefined;
@@ -3609,7 +3622,9 @@ export class TaskExecutor {
// execute / step-execute seams (which key off a governing node id stamped
// into context), so the coding/step session runs as the column agent under
// the SAME binding lookup the custom-node seam uses (KTD-2 single resolver).
this.graphColumnAgentResolver.set(task.id, resolveBindingForNode);
if (columnAgentsEnabled) {
this.graphColumnAgentResolver.set(task.id, resolveBindingForNode);
}
const runner = new WorkflowGraphTaskRunner({
store: this.store,