Address PR review feedback (#1433)

- always dispose AI design session (try/finally) + rejecting-agent test
- prune expired design rate-limit entries per check
- apply workflowId precedence in createTaskWithReservedId
- compare-and-set migrated project default (no pre-transaction snapshot)
- reject fragment ids in setDefaultWorkflowId at the write boundary
- guard stripApprovalBypassFlags against malformed template nodes
- hoist duplicate stepToFragmentIr call in migration loop
- auto-layout preserves unplaced nodes (strictColumnForY)
- second same-pair connection births a failure edge for condition-capable sources
- insertFragment expands foreach templates into child nodes (lossless round-trip)
- copyIrWithFreshIds remaps namespaced foreach child layout keys
- discard stale AI-edit results after workflow switch
- i18n: returnEmptyString:false so empty locale placeholders fall back to en
- test fixes: schema-version titles, migrate idempotency assertion, TaskForm post-resolve + cross-mode regression, deterministic waits

packages/core/src/__tests__/db-migrate.test.ts

@@ -1073,6 +1073,8 @@ describe("schema migration", () => {
     expect(reopened.getSchemaVersion()).toBe(109);
     const workflowColumns = reopened.prepare("PRAGMA table_info(workflows)").all() as Array<{ name: string }>;
     expect(workflowColumns.filter((c) => c.name === "kind")).toHaveLength(1);
+    const stepColumns = reopened.prepare("PRAGMA table_info(workflow_steps)").all() as Array<{ name: string }>;
+    expect(stepColumns.filter((c) => c.name === "migrated_fragment_id")).toHaveLength(1);
     reopened.close();
   });
 });

packages/core/src/__tests__/goals-schema.test.ts

@@ -90,7 +90,7 @@ describe("goals schema", () => {
     expect(table?.name).toBe("goals");
   });
 
-  it("reports schema version 101", () => {
+  it("reports schema version 109", () => {
     expect(db.getSchemaVersion()).toBe(109);
   });
 });

packages/core/src/__tests__/mission-store.test.ts

@@ -3745,7 +3745,7 @@ describe("MissionStore", () => {
   // ── Loop State & Validator Run Schema Tests ───────────────────────────
 
   describe("Loop State & Validator Run Schema (v31)", () => {
-    it("schema version is 101 after migration", () => {
+    it("schema version is 109 after migration", () => {
       expect(db.getSchemaVersion()).toBe(109);
     });
 

packages/core/src/__tests__/run-audit.test.ts

@@ -583,7 +583,7 @@ describe("Run Audit", () => {
       expect(indexNames).toContain("idxRunAuditEventsTimestamp");
     });
 
-    it("schema version is bumped to 40", () => {
+    it("schema version is bumped to 109", () => {
       expect(db.getSchemaVersion()).toBe(109);
     });
   });

packages/core/src/__tests__/strip-approval-bypass-flags.test.ts

@@ -76,4 +76,36 @@ describe("stripApprovalBypassFlags", () => {
     const ir = { version: "v1", name: "wf" } as unknown as WorkflowIr;
     expect(stripApprovalBypassFlags(ir).stripped).toBe(false);
   });
+
+  it("tolerates non-object entries in nodes (untrusted input)", () => {
+    const ir = {
+      version: "v1",
+      name: "wf",
+      nodes: [null, "bogus", 42, { id: "n1", kind: "prompt", config: { cliSkipApproval: true } }],
+      edges: [],
+    } as unknown as WorkflowIr;
+    const { ir: out, stripped } = stripApprovalBypassFlags(ir);
+    expect(stripped).toBe(true);
+    expect((out as any).nodes[3].config.cliSkipApproval).toBeUndefined();
+  });
+
+  it("tolerates non-object entries in nested template.nodes", () => {
+    const ir = {
+      version: "v1",
+      name: "wf",
+      nodes: [
+        {
+          id: "fe",
+          kind: "foreach",
+          config: {
+            template: { nodes: [null, 0, "x", { id: "inner", kind: "prompt", config: { autoApprove: true } }] },
+          },
+        },
+      ],
+      edges: [],
+    } as unknown as WorkflowIr;
+    const { ir: out, stripped } = stripApprovalBypassFlags(ir);
+    expect(stripped).toBe(true);
+    expect((out as any).nodes[0].config.template.nodes[3].config.autoApprove).toBeUndefined();
+  });
 });

packages/core/src/__tests__/workflow-definition-store.test.ts

@@ -273,4 +273,39 @@ describe("TaskStore workflow definitions (U1)", () => {
     const task = await store.createTask({ description: "t" });
     await expect(store.selectTaskWorkflow(task.id, frag.id)).rejects.toThrow(/fragment/i);
   });
+
+  it("setDefaultWorkflowId rejects a fragment id at the write boundary", async () => {
+    const frag = await store.createWorkflowDefinition({ name: "Frag", ir: fragmentIr(), kind: "fragment" });
+    await expect(store.setDefaultWorkflowId(frag.id)).rejects.toThrow(/fragment/i);
+    expect(await store.getDefaultWorkflowId()).toBeUndefined();
+  });
+
+  it("setDefaultWorkflowId accepts a real workflow and clears with null", async () => {
+    const wf = await store.createWorkflowDefinition({ name: "W", ir: makeIr() });
+    await store.setDefaultWorkflowId(wf.id);
+    expect(await store.getDefaultWorkflowId()).toBe(wf.id);
+    await store.setDefaultWorkflowId(null);
+    expect(await store.getDefaultWorkflowId()).toBeUndefined();
+  });
+
+  it("createTaskWithReservedId honors an explicit workflowId (precedence over default)", async () => {
+    const def = await store.createWorkflowDefinition({ name: "Explicit", ir: makeIr() });
+    const task = await store.createTaskWithReservedId(
+      { description: "t", workflowId: def.id },
+      { taskId: "task-explicit-wf" },
+    );
+    const sel = store.getTaskWorkflowSelection(task.id);
+    expect(sel?.workflowId).toBe(def.id);
+  });
+
+  it("createTaskWithReservedId treats workflowId:null as explicit opt-out", async () => {
+    const def = await store.createWorkflowDefinition({ name: "Def", ir: makeIr() });
+    await store.setDefaultWorkflowId(def.id);
+    const task = await store.createTaskWithReservedId(
+      { description: "t", workflowId: null },
+      { taskId: "task-optout-wf" },
+    );
+    const sel = store.getTaskWorkflowSelection(task.id);
+    expect(sel?.workflowId ?? undefined).toBeUndefined();
+  });
 });

packages/core/src/__tests__/workflow-step-migration.test.ts

@@ -155,6 +155,36 @@ describe("TaskStore.migrateLegacyWorkflowSteps (U2/R5)", () => {
     expect(await store.getDefaultWorkflowId()).toBe(existing.id);
   });
 
+  it("compare-and-set: re-reads the default after the transaction and skips when a concurrent writer set one", async () => {
+    await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true });
+
+    const concurrent = await store.createWorkflowDefinition({
+      name: "Concurrent",
+      ir: {
+        version: "v1",
+        name: "Concurrent",
+        nodes: [
+          { id: "start", kind: "start" },
+          { id: "end", kind: "end" },
+        ],
+        edges: [{ from: "start", to: "end", condition: "success" }],
+      },
+      kind: "workflow",
+    });
+
+    // A project default exists when migration's post-transaction compare-and-set
+    // re-reads it. Because the set is gated on the re-read (not a pre-transaction
+    // snapshot), an existing default is observed and never clobbered.
+    await store.setDefaultWorkflowId(concurrent.id);
+
+    const result = await store.migrateLegacyWorkflowSteps();
+
+    expect(result.combinedWorkflowId).toBeTruthy();
+    expect(result.combinedWorkflowId).not.toBe(concurrent.id);
+    // The compare-and-set re-read observed the existing default and did NOT clobber it.
+    expect(await store.getDefaultWorkflowId()).toBe(concurrent.id);
+  });
+
   it("is a no-op with zero user steps", async () => {
     const result = await store.migrateLegacyWorkflowSteps();
     expect(result).toEqual({ migrated: 0, skipped: 0, combinedWorkflowId: undefined });

packages/core/src/store.ts

@@ -4161,7 +4161,24 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
       : undefined;
 
     let pendingWorkflowSelection: { workflowId: string; stepIds: string[] } | undefined;
-    if (input.enabledWorkflowSteps === undefined && options.applyDefaultWorkflowSteps !== false) {
+    // U6/R3/KTD-4: an explicit create-time workflowId beats the project default,
+    // mirroring createTask(). `null` is an explicit opt-out, `string` materializes
+    // that workflow, `undefined` falls through to the default-workflow behavior.
+    // Explicit enabledWorkflowSteps still wins over workflowId for trusted callers.
+    const explicitWorkflowId =
+      input.enabledWorkflowSteps === undefined ? input.workflowId : undefined;
+    if (explicitWorkflowId !== undefined) {
+      if (explicitWorkflowId === null) {
+        // Explicit "No workflow": skip default materialization entirely.
+        resolvedWorkflowSteps = undefined;
+      } else {
+        // Compile + materialize up front so unknown/fragment ids throw BEFORE
+        // the task row is created (no orphaned steps, no half-created task).
+        const selected = await this.materializeExplicitWorkflowSteps(explicitWorkflowId);
+        resolvedWorkflowSteps = selected.stepIds;
+        pendingWorkflowSelection = selected;
+      }
+    } else if (input.enabledWorkflowSteps === undefined && options.applyDefaultWorkflowSteps !== false) {
       // Mirror createTask: a configured project default workflow takes
       // precedence over legacy default-on steps on this creation path too.
       try {
@@ -13010,6 +13027,13 @@ ${stepsSection}`;
     if (workflowId) {
       const exists = await this.getWorkflowDefinition(workflowId);
       if (!exists) throw new Error(`Workflow '${workflowId}' not found`);
+      // KTD-1/R6: a fragment is a reusable palette piece, not a selectable
+      // workflow. Reject it at the write boundary so a fragment can never be
+      // persisted as the project default (the read-side skip in
+      // materializeDefaultWorkflowSteps remains as defense in depth).
+      if (exists.kind === "fragment") {
+        throw new Error(`Workflow '${workflowId}' is a fragment and cannot be set as the project default`);
+      }
     }
     // null is updateSettings' explicit-delete sentinel for project keys.
     await this.updateSettings({ defaultWorkflowId: workflowId } as unknown as Partial<Settings>);
@@ -13090,10 +13114,10 @@ ${stepsSection}`;
     combinedWorkflowId?: string;
   }> {
     // Resolve async prerequisites BEFORE the synchronous transaction: the
-    // workflow-columns flag (for flag-aware persistence) and the current project
-    // default (for the no-clobber guard).
+    // workflow-columns flag (for flag-aware persistence). The project default is
+    // re-read AFTER the transaction (compare-and-set) so a concurrently-set
+    // default is never clobbered.
     const flagOn = await this.workflowColumnsFlagOn();
-    const existingDefaultId = await this.getDefaultWorkflowId();
 
     const result = this.db.transactionImmediate(() => {
       // Write lock is now held. Read the raw step rows directly (the cached,
@@ -13118,13 +13142,16 @@ ${stepsSection}`;
 
       // Every unmigrated user step → a single-node fragment; stamp the source row.
       for (const step of unmigrated) {
+        // parseWorkflowIr runs inside both insertWorkflowDefinitionSync and
+        // layoutForIr, so compute the fragment IR once and reuse it.
+        const fragmentIr = stepToFragmentIr(step);
         const fragment = this.insertWorkflowDefinitionSync(
           {
             name: step.name,
             description: step.description,
             kind: "fragment",
-            ir: stepToFragmentIr(step),
-            layout: layoutForIr(stepToFragmentIr(step)),
+            ir: fragmentIr,
+            layout: layoutForIr(fragmentIr),
           },
           flagOn,
         );
@@ -13159,10 +13186,25 @@ ${stepsSection}`;
     // Set the combined workflow as the project default — only when one was
     // created AND no explicit default is already set (don't clobber a user
     // choice). Done outside the transaction via the async setter so the project
-    // default-workflow hooks run. Racing re-runs are harmless: the second run
-    // creates no combined workflow, so this branch is skipped.
-    if (result.combinedWorkflowId && !existingDefaultId) {
-      await this.setDefaultWorkflowId(result.combinedWorkflowId);
+    // default-workflow hooks run. Compare-and-set against the CURRENT default
+    // (re-read immediately before writing, not the pre-transaction snapshot) so
+    // a default set concurrently by another writer is never overwritten. If the
+    // set fails, swallow the error: a missing migrated default is recoverable
+    // (the user can set one), but throwing here would surface the whole
+    // migration as failed even though the definitions were written.
+    if (result.combinedWorkflowId) {
+      const currentDefaultId = await this.getDefaultWorkflowId();
+      if (!currentDefaultId) {
+        try {
+          await this.setDefaultWorkflowId(result.combinedWorkflowId);
+        } catch (err) {
+          storeLog.warn("Failed to set migrated combined workflow as project default", {
+            phase: "migrateLegacyWorkflowSteps:set-default",
+            combinedWorkflowId: result.combinedWorkflowId,
+            error: err instanceof Error ? err.message : String(err),
+          });
+        }
+      }
     }
 
     return result;

packages/core/src/workflow-ir.ts

@@ -923,6 +923,9 @@ export function stripApprovalBypassFlags(ir: WorkflowIr): { ir: WorkflowIr; stri
   if (!Array.isArray(nodes)) return { ir, stripped: false };
   let stripped = false;
   const stripNode = (node: WorkflowIrNode): void => {
+    // Untrusted input may contain non-object entries (null, strings, numbers)
+    // in `nodes` / `template.nodes`; skip them rather than dereferencing.
+    if (!node || typeof node !== "object") return;
     const cfg = node.config as Record<string, unknown> | undefined;
     if (cfg && typeof cfg === "object") {
       if ("cliSkipApproval" in cfg) {

packages/dashboard/app/components/WorkflowNodeEditor.tsx

@@ -743,6 +743,11 @@ function InnerEditor({
   const activeWorkflow = useMemo(() => workflows.find((w) => w.id === activeId), [workflows, activeId]);
   const isBuiltin = !!activeWorkflow && isBuiltinWorkflowId(activeWorkflow.id);
 
+  // Live mirror of the active workflow id, readable inside async callbacks that
+  // captured an earlier value before an await (e.g. the AI-design round-trip).
+  const activeIdRef = useRef(activeId);
+  activeIdRef.current = activeId;
+
   // Trivial-graph palette hint (R9): a user-owned workflow whose graph carries no
   // user-authored node yet (everything is start/end/column-band — column bands map
   // to data.kind "start"). Disappears as soon as any user node exists; never shows
@@ -1216,14 +1221,26 @@ function InnerEditor({
     }
     const controller = new AbortController();
     aiEditAbortRef.current = controller;
+    // Capture the target workflow up-front: if the user switches the active
+    // workflow during the (long) design round-trip, we must NOT apply the result
+    // to whatever workflow happens to be active when it resolves.
+    const targetWorkflow = activeWorkflow;
     setAiEditBusy(true);
     setAiEditError(null);
     try {
       const result = await designWorkflow(
-        { prompt: trimmed, workflowId: activeWorkflow.id },
+        { prompt: trimmed, workflowId: targetWorkflow.id },
         projectId,
         controller.signal,
       );
+      // The active workflow changed mid-flight → discard the stale result.
+      if (activeIdRef.current !== targetWorkflow.id) {
+        addToast(
+          t("workflows.aiStaleDiscarded", "Discarded AI design — you switched workflows"),
+          "warning",
+        );
+        return;
+      }
       // Always confirm before the destructive replace.
       const ok = await confirm({
         title: t("workflows.aiReplaceTitle", "Replace graph?"),
@@ -1238,14 +1255,14 @@ function InnerEditor({
       // Map the returned IR through irToFlow on a definition-shaped object,
       // mirroring the active-workflow load effect's mapping.
       const flow = irToFlow({
-        ...activeWorkflow,
+        ...targetWorkflow,
         ir: result.ir,
         layout: result.layout,
       });
       setNodes(flow.nodes);
       setEdges(flow.edges);
-      setColumns(columnsOf({ ...activeWorkflow, ir: result.ir }));
-      setFields(fieldsOf({ ...activeWorkflow, ir: result.ir }));
+      setColumns(columnsOf({ ...targetWorkflow, ir: result.ir }));
+      setFields(fieldsOf({ ...targetWorkflow, ir: result.ir }));
       setSelectedNodeId(null);
       setSelectedEdgeId(null);
       setValidationError(null);

packages/dashboard/app/components/__tests__/TaskForm.test.tsx

@@ -1108,23 +1108,39 @@ describe("TaskForm workflow picker (U6/R3)", () => {
     renderTaskForm({ onWorkflowIdChange: vi.fn() });
 
     expect(screen.getByTestId("task-workflow-loading")).toBeTruthy();
-    resolveFn([]);
-  });
-
-  it("regression: no per-step checkboxes and no fetchWorkflowSteps usage", async () => {
-    await mockWorkflows([{ id: "WF-1", name: "QA" }]);
-    renderTaskForm({ onWorkflowIdChange: vi.fn() });
+    resolveFn([{ id: "WF-1", name: "QA" }]);
 
+    // After the promise resolves, the loading placeholder is replaced by the
+    // populated select containing the fetched workflow option.
     await waitFor(() => {
-      expect(screen.getByTestId("task-workflow-select")).toBeTruthy();
+      expect(screen.queryByTestId("task-workflow-loading")).toBeNull();
     });
-    // The old per-step checkbox UI and execution-order controls are gone.
-    expect(screen.queryByTestId("workflow-step-order")).toBeNull();
-    expect(document.querySelector('[data-testid^="workflow-step-checkbox-"]')).toBeNull();
-    // The api mock no longer needs a fetchWorkflowSteps export — TaskForm never
-    // calls it. (If it still did, rendering above would have thrown on the
-    // missing mock export, so reaching this point is itself the regression proof.)
+    const select = screen.getByTestId("task-workflow-select") as HTMLSelectElement;
+    const optionValues = Array.from(select.options).map((o) => o.value);
+    expect(optionValues).toContain("WF-1");
   });
+
+  it.each([
+    ["create", { mode: "create" as const }],
+    ["edit", { mode: "edit" as const, title: "Existing task", onTitleChange: vi.fn() }],
+  ])(
+    "regression: no per-step checkboxes and no fetchWorkflowSteps usage (%s mode)",
+    async (_label, modeProps) => {
+      await mockWorkflows([{ id: "WF-1", name: "QA" }]);
+      renderTaskForm({ onWorkflowIdChange: vi.fn(), ...modeProps });
+
+      await waitFor(() => {
+        expect(screen.getByTestId("task-workflow-select")).toBeTruthy();
+      });
+      // The old per-step checkbox UI and execution-order controls are gone on
+      // every TaskForm surface (create and edit).
+      expect(screen.queryByTestId("workflow-step-order")).toBeNull();
+      expect(document.querySelector('[data-testid^="workflow-step-checkbox-"]')).toBeNull();
+      // The api mock no longer needs a fetchWorkflowSteps export — TaskForm never
+      // calls it. (If it still did, rendering above would have thrown on the
+      // missing mock export, so reaching this point is itself the regression proof.)
+    },
+  );
 });
 
 describe("TaskForm focus behavior (FN-1459)", () => {

packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx

@@ -1016,7 +1016,9 @@ describe("WorkflowNodeEditor — U4 create dialog / delete / inline rename / dir
     render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
     fireEvent.click((await screen.findByText("Delete")).closest("button")!);
     // The no-op fallback resolves false → deleteWorkflow is never called.
-    await new Promise((r) => setTimeout(r, 20));
+    // Let the async fallback settle deterministically (no wall-clock delay).
+    await Promise.resolve();
+    await Promise.resolve();
     expect(deleteWorkflow).not.toHaveBeenCalled();
   });
 
@@ -1167,6 +1169,7 @@ describe("WorkflowNodeEditor — U4 create dialog / delete / inline rename / dir
 function trivialUserDef(): WorkflowDefinition {
   return {
     id: "WF-TRIVIAL",
+    kind: "workflow",
     name: "Trivial",
     description: "",
     ir: {
@@ -1536,11 +1539,14 @@ describe("WorkflowNodeEditor — U9 palette Templates section", () => {
 
     render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
     await screen.findByTestId("wf-palette-templates");
+    // Canvas can lag the palette under cold-transform shard load.
+    await screen.findByTestId("wf-node-gate", undefined, { timeout: 3000 });
 
     const before = screen.queryAllByTestId("wf-node-prompt").length;
     fireEvent.click(screen.getByTestId("wf-tpl-step-qa-check"));
-    await waitFor(() =>
-      expect(screen.queryAllByTestId("wf-node-prompt").length).toBe(before + 1),
+    await waitFor(
+      () => expect(screen.queryAllByTestId("wf-node-prompt").length).toBe(before + 1),
+      { timeout: 3000 },
     );
   });
 

packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts

@@ -201,6 +201,25 @@ describe("autoLayout — foreach / unreachable / cycles", () => {
     expect(strictColumnForY(o.y, COLUMNS_3)).toBe("done");
   });
 
+  it("leaves an unplaced node (no column, parked outside all bands) unplaced", () => {
+    // y far below the last band, with no explicit column → strictColumnForY undefined.
+    const outsideY = bandTop(COLUMNS_3.length) + 5000;
+    const nodes: N[] = [
+      node("start", "start", 0, midBand(0), { column: "triage" }),
+      node("a", "prompt", 0, midBand(1), { column: "in-progress" }),
+      node("loose", "prompt", 0, outsideY),
+    ];
+    const edges = [edge("start", "a"), edge("a", "loose")];
+    const pos = autoLayout(nodes, edges, COLUMNS_3);
+
+    const loose = pos.get("loose")!;
+    // y is preserved (still outside every band) — NOT clamped into a band.
+    expect(loose.y).toBe(outsideY);
+    expect(strictColumnForY(loose.y, COLUMNS_3)).toBeUndefined();
+    // x still flows left-to-right with the layering tidy.
+    expect(loose.x).toBeGreaterThan(pos.get("a")!.x);
+  });
+
   it("terminates and stays sane with a rework cycle edge present", () => {
     const nodes: N[] = [
       node("start", "start", 0, midBand(0), { column: "triage" }),

packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts

@@ -584,11 +584,36 @@ describe("edge-condition authoring (U2)", () => {
       error: "missing-endpoint",
     });
 
-    // duplicate of the same condition.
-    expect(buildConnectionEdge({ source: "a", target: "b" }, edges, nodes)).toEqual({
+    // second connect of an existing success pair (prompt source supports
+    // conditions) → births the parallel failure edge rather than rejecting.
+    const failureBirth = buildConnectionEdge({ source: "a", target: "b" }, edges, nodes);
+    expect("edge" in failureBirth).toBe(true);
+    if ("edge" in failureBirth) {
+      expect(failureBirth.edge.data?.condition).toBe("failure");
+    }
+
+    // once BOTH success and failure exist, a third connect is a duplicate.
+    const bothConditions = [
+      ...edges,
+      { id: "3", source: "a", target: "b", data: { condition: "failure" } },
+    ];
+    expect(buildConnectionEdge({ source: "a", target: "b" }, bothConditions, nodes)).toEqual({
       error: "duplicate",
     });
 
+    // a source kind that does NOT support conditions stays a hard duplicate.
+    const readonlyNodes: FlowNode<WorkflowFlowNodeData>[] = [
+      { id: "a", type: "start", position: { x: 0, y: 0 }, data: { kind: "start", label: "a" } },
+      { id: "b", type: "prompt", position: { x: 100, y: 0 }, data: { kind: "prompt", label: "b" } },
+    ];
+    expect(
+      buildConnectionEdge(
+        { source: "a", target: "b" },
+        [{ id: "1", source: "a", target: "b", data: { condition: "success" } }],
+        readonlyNodes,
+      ),
+    ).toEqual({ error: "duplicate" });
+
     // cycle: c→a closes a→b→c→a.
     expect(buildConnectionEdge({ source: "c", target: "a" }, edges, nodes)).toEqual({
       error: "cycle",
@@ -867,6 +892,59 @@ describe("insertFragment", () => {
     const allIds = second.nodes.map((n) => n.id);
     expect(new Set(allIds).size).toBe(allIds.length);
   });
+
+  it("expands a foreach fragment's template so flowToIr round-trips the full template", () => {
+    // Fragment: start → loop(foreach with a 2-node template) → end.
+    const foreachFragment: WorkflowDefinition["ir"] = {
+      version: "v1",
+      name: "frag",
+      nodes: [
+        { id: "start", kind: "start" },
+        {
+          id: "loop",
+          kind: "foreach",
+          config: {
+            source: "task-steps",
+            template: {
+              nodes: [
+                { id: "t1", kind: "prompt", config: { prompt: "inner1" } },
+                { id: "t2", kind: "prompt", config: { prompt: "inner2" } },
+              ],
+              edges: [{ from: "t1", to: "t2", condition: "success" }],
+            },
+          },
+        },
+        { id: "end", kind: "end" },
+      ],
+      edges: [
+        { from: "start", to: "loop", condition: "success" },
+        { from: "loop", to: "end", condition: "success" },
+      ],
+    };
+
+    const existing = irToFlow(u8ChainDef());
+    const { nodes, edges, insertedNodeIds } = insertFragment(
+      existing.nodes,
+      existing.edges,
+      foreachFragment,
+      { x: 400, y: 200 },
+    );
+
+    // The foreach group's template children were expanded as parented nodes.
+    const groupId = insertedNodeIds[0];
+    const children = nodes.filter((n) => n.parentId === groupId);
+    expect(children).toHaveLength(2);
+
+    // Round-trip the live canvas back to IR — the inserted foreach must carry its
+    // full template (not an empty one) with both inner nodes and the inner edge.
+    const { ir: out } = flowToIr("wf", nodes, edges);
+    const loop = out.nodes.find((n) => n.kind === "foreach")!;
+    expect(loop).toBeTruthy();
+    const template = (loop.config as { template?: { nodes: unknown[]; edges: unknown[] } })
+      .template;
+    expect(template?.nodes).toHaveLength(2);
+    expect(template?.edges).toHaveLength(1);
+  });
 });
 
 describe("fragmentSeamConflicts", () => {
@@ -999,4 +1077,37 @@ describe("copyIrWithFreshIds", () => {
       expect(result.ir.nodes.every((n) => n.column === "in-progress")).toBe(true);
     }
   });
+
+  it("remaps namespaced foreach child layout keys consistently with the template ids", () => {
+    const ir = v2WithForeach();
+    const layout = {
+      start: { x: 0, y: 0 },
+      loop: { x: 100, y: 0 },
+      "loop::t1": { x: 10, y: 20 },
+      "loop::t2": { x: 270, y: 20 },
+      end: { x: 400, y: 0 },
+    };
+    const result = copyIrWithFreshIds(ir, layout);
+
+    const loop = result.ir.nodes.find((n) => n.kind === "foreach")!;
+    const template = (loop.config as { template: { nodes: { id: string }[] } }).template;
+    const newGroupId = loop.id;
+    const childKeys = Object.keys(result.layout).filter((k) => k.includes("::"));
+
+    // Both namespaced child keys survive (count preserved).
+    expect(childKeys).toHaveLength(2);
+    // Each child key is `${newGroupId}::${newTemplateId}` for a real template id.
+    const newInnerIds = new Set(template.nodes.map((n) => n.id));
+    for (const k of childKeys) {
+      const [g, inner] = k.split("::");
+      expect(g).toBe(newGroupId);
+      expect(newInnerIds.has(inner)).toBe(true);
+      // No stale original ids leak through.
+      expect(g).not.toBe("loop");
+      expect(["t1", "t2"]).not.toContain(inner);
+    }
+    // Values preserved by position (t1's offset stays with the remapped t1 key).
+    const t1NewId = result.layout[`${newGroupId}::${template.nodes[0].id}`];
+    expect(t1NewId).toEqual({ x: 10, y: 20 });
+  });
 });

packages/dashboard/app/components/workflow-auto-layout.ts

@@ -6,7 +6,7 @@ import {
   WF_CARD_HEIGHT,
   COLUMN_BAND_HEIGHT,
   bandTop,
-  columnForY,
+  strictColumnForY,
   isColumnBandNode,
 } from "./workflow-flow-mapping";
 
@@ -174,8 +174,20 @@ export function autoLayout(
     const rowsPerColumn = new Map<string, number>();
     for (const id of sorted) {
       const node = byId.get(id)!;
-      const colId = node.data.column ?? columnForY(node.position.y, columns);
-      const colIndex = colId ? columns.findIndex((c) => c.id === colId) : -1;
+      // Resolve the node's column WITHOUT clamping: an explicit column id, or a
+      // strict band hit-test. A node with neither (parked outside every band and
+      // carrying no column) is "unplaced" and must stay that way — auto-layout
+      // never silently re-columns it into the nearest band. We give it the new
+      // layer x (so the tidy still flows it left-to-right) but preserve its y so
+      // strictColumnForY(newY) remains undefined.
+      const colId =
+        node.data.column ??
+        (strictColumnForY(node.position.y, columns) ? strictColumnForY(node.position.y, columns) : undefined);
+      if (!colId) {
+        positions.set(id, { x: layerX, y: node.position.y });
+        continue;
+      }
+      const colIndex = columns.findIndex((c) => c.id === colId);
       const safeColIndex = colIndex >= 0 ? colIndex : 0;
 
       const top = bandTop(safeColIndex);

packages/dashboard/app/components/workflow-flow-mapping.ts

@@ -574,23 +574,38 @@ export function buildConnectionEdge(
   const target = connection.target ?? undefined;
   if (!source || !target) return { error: "missing-endpoint" };
 
-  const condition = "success";
-  // Skip exact duplicates of the SAME condition (a second identical edge is
-  // pointless); different conditions between the same pair are allowed.
-  const isDuplicate = edges.some(
-    (e) =>
-      e.source === source &&
-      e.target === target &&
-      ((e.data?.condition as string | undefined) ?? "success") === condition,
+  const srcNode = nodes.find((n) => n.id === source);
+  const tgtNode = nodes.find((n) => n.id === target);
+
+  // Existing conditions already authored between this exact pair.
+  const existingConditions = new Set(
+    edges
+      .filter((e) => e.source === source && e.target === target)
+      .map((e) => (e.data?.condition as string | undefined) ?? "success"),
   );
-  if (isDuplicate) return { error: "duplicate" };
+
+  // New edges are normally born "success". But a second connect gesture between a
+  // pair that already has a success edge should author the *parallel* "failure"
+  // edge (rather than being rejected as a duplicate), so users can build a
+  // success/failure split with two connect gestures — but only when the source
+  // kind actually exposes a condition select. Block only when both conditions
+  // already exist (or the only available condition is already taken).
+  const supportsConditions = edgeConditionEditability(srcNode?.data.kind) === "conditions";
+  let condition = "success";
+  if (existingConditions.has("success")) {
+    if (supportsConditions && !existingConditions.has("failure")) {
+      condition = "failure";
+    } else {
+      return { error: "duplicate" };
+    }
+  } else if (existingConditions.has(condition)) {
+    return { error: "duplicate" };
+  }
 
   // Cycle guard (KTD-9). Exempt connections where both endpoints are children of
   // the same foreach template — those may legitimately be rework cycles authored
   // separately; the simplest correct rule applies the guard only to non-template
   // connections.
-  const srcNode = nodes.find((n) => n.id === source);
-  const tgtNode = nodes.find((n) => n.id === target);
   const bothTemplateChildren =
     !!srcNode?.parentId && srcNode.parentId === tgtNode?.parentId;
   if (!bothTemplateChildren && wouldCreateCycle(edges, source, target)) {
@@ -899,6 +914,12 @@ export function insertFragment(
   const minY = placed.length ? Math.min(...placed.map((p) => p.y)) : 0;
 
   const insertedNodeIds: string[] = [];
+  // foreach template children are expanded into parented child flow nodes (the
+  // same way irToFlow does), so an inserted foreach round-trips its full template
+  // through flowToIr instead of dropping config.template (which flowToIr would
+  // otherwise rebuild as an empty template from the absent children).
+  const childNodes: FlowNode<WorkflowFlowNodeData>[] = [];
+  const childEdges: FlowEdge[] = [];
   const newNodes = bodyNodes.map((node, index): FlowNode<WorkflowFlowNodeData> => {
     const id = idMap.get(node.id)!;
     insertedNodeIds.push(id);
@@ -906,6 +927,40 @@ export function insertFragment(
     const pos = fromLayout
       ? { x: position.x + (fromLayout.x - minX), y: position.y + (fromLayout.y - minY) }
       : { x: position.x + index * 180, y: position.y };
+    const foreachCfg = foreachConfigOf(node);
+    if (foreachCfg) {
+      const template = foreachCfg.template;
+      template.nodes.forEach((inner, innerIdx) => {
+        const innerKind = editorKind(inner);
+        childNodes.push({
+          id: foreachChildFlowId(id, inner.id),
+          type: innerKind,
+          position: { x: FOREACH_CHILD_X + innerIdx * FOREACH_CHILD_STEP_X, y: FOREACH_CHILD_Y },
+          parentId: id,
+          extent: "parent",
+          data: { kind: innerKind, label: nodeLabel(inner), config: { ...(inner.config ?? {}) } },
+          deletable: true,
+        });
+      });
+      template.edges.forEach((edge, eIdx) => {
+        childEdges.push(irEdgeToFlow(edge, eIdx, `${id}${FOREACH_CHILD_SEP}`));
+      });
+      // The group node keeps everything except the template (children carry it).
+      const { template: _t, ...restCfg } = (node.config ?? {}) as Record<string, unknown>;
+      return {
+        id,
+        type: "foreach",
+        position: pos,
+        data: {
+          kind: "foreach",
+          label: nodeLabel(node),
+          config: { ...restCfg },
+          templateEmpty: template.nodes.length === 0,
+        },
+        style: { width: FOREACH_GROUP_WIDTH, height: FOREACH_GROUP_HEIGHT },
+        deletable: true,
+      };
+    }
     return irNodeToFlowNode(node, id, pos);
   });
 
@@ -925,8 +980,9 @@ export function insertFragment(
     });
 
   return {
-    nodes: [...nodes, ...newNodes],
-    edges: [...edges, ...newEdges],
+    // Group nodes (in newNodes) must precede their children (childNodes).
+    nodes: [...nodes, ...newNodes, ...childNodes],
+    edges: [...edges, ...newEdges, ...childEdges],
     insertedNodeIds,
   };
 }
@@ -935,11 +991,10 @@ export function insertFragment(
  *  new template object; the original is untouched. Template-local ids are scoped
  *  to the template, so a fresh local id space suffices (and keeps config compact
  *  rather than reusing global ids). */
-function copyForeachTemplate(template: {
-  nodes: WorkflowIrNode[];
-  edges: WorkflowIrEdge[];
-}): { nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[] } {
-  const innerMap = new Map<string, string>();
+function copyForeachTemplate(
+  template: { nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[] },
+  innerMap: Map<string, string>,
+): { nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[] } {
   for (const n of template.nodes) innerMap.set(n.id, newNodeId());
   const nodes = template.nodes.map((n) => copyIrNode(n, innerMap.get(n.id)!));
   const edges = template.edges.map((e) => ({
@@ -951,12 +1006,21 @@ function copyForeachTemplate(template: {
 }
 
 /** Deep-ish copy of an IR node under a new id, recursing into a foreach
- *  template's internal node references so they remain self-consistent. */
-function copyIrNode(node: WorkflowIrNode, newId: string): WorkflowIrNode {
+ *  template's internal node references so they remain self-consistent. When the
+ *  node is a foreach, its template-local id remap is recorded in `templateMaps`
+ *  keyed by the node's ORIGINAL id, so the caller can remap namespaced
+ *  `${groupId}::${templateNodeId}` layout keys consistently. */
+function copyIrNode(
+  node: WorkflowIrNode,
+  newId: string,
+  templateMaps?: Map<string, Map<string, string>>,
+): WorkflowIrNode {
   const config = node.config ? { ...node.config } : undefined;
   const foreach = foreachConfigOf(node);
   if (foreach && config) {
-    config.template = copyForeachTemplate(foreach.template);
+    const innerMap = new Map<string, string>();
+    config.template = copyForeachTemplate(foreach.template, innerMap);
+    templateMaps?.set(node.id, innerMap);
   }
   const copy: WorkflowIrNode = { id: newId, kind: node.kind };
   if (node.column !== undefined) copy.column = node.column;
@@ -978,16 +1042,34 @@ export function copyIrWithFreshIds(
   const idMap = new Map<string, string>();
   for (const n of ir.nodes) idMap.set(n.id, newNodeId());
 
-  const nodes = ir.nodes.map((n) => copyIrNode(n, idMap.get(n.id)!));
+  // Per foreach group (by ORIGINAL group id): its template-local id remap, so
+  // namespaced layout keys `${groupId}::${templateNodeId}` can be remapped to
+  // `${newGroupId}::${newTemplateNodeId}` consistently.
+  const templateMaps = new Map<string, Map<string, string>>();
+  const nodes = ir.nodes.map((n) => copyIrNode(n, idMap.get(n.id)!, templateMaps));
   const edges = ir.edges.map((e) => ({
     ...e,
     from: idMap.get(e.from) ?? e.from,
     to: idMap.get(e.to) ?? e.to,
   }));
 
-  // Remap layout keys for top-level nodes; leave any unrelated keys as-is.
+  // Remap layout keys. Top-level node keys remap via idMap; namespaced foreach
+  // child keys remap via the owning group's idMap entry + its inner map; any
+  // unrelated keys pass through unchanged.
   const newLayout: Record<string, { x: number; y: number }> = {};
   for (const [key, pos] of Object.entries(layout)) {
+    const sepIdx = key.indexOf(FOREACH_CHILD_SEP);
+    if (sepIdx >= 0) {
+      const groupId = key.slice(0, sepIdx);
+      const innerId = key.slice(sepIdx + FOREACH_CHILD_SEP.length);
+      const newGroupId = idMap.get(groupId);
+      const innerMap = templateMaps.get(groupId);
+      const newInnerId = innerMap?.get(innerId);
+      if (newGroupId && newInnerId) {
+        newLayout[foreachChildFlowId(newGroupId, newInnerId)] = { ...pos };
+        continue;
+      }
+    }
     const mapped = idMap.get(key);
     newLayout[mapped ?? key] = { ...pos };
   }

packages/dashboard/src/routes/__tests__/workflow-design-route.test.ts

@@ -45,6 +45,26 @@ function makeFakeAgent(text: string) {
   return { factory, captured };
 }
 
+/** A fake agent whose prompt rejects; tracks whether dispose() was called so we
+ *  can assert the route releases the session even when the model turn throws. */
+function makeRejectingAgent() {
+  const state = { disposed: false };
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const factory: any = async () => {
+    const session = {
+      on(_event: "text", _listener: (delta: string) => void) {},
+      async prompt() {
+        throw new Error("model turn failed");
+      },
+      dispose() {
+        state.disposed = true;
+      },
+    };
+    return { session };
+  };
+  return { factory, state };
+}
+
 /** A minimal valid v1 linear IR (start → prompt → end). */
 function linearIr(overrides?: { nodeConfig?: Record<string, unknown> }): WorkflowIr {
   return {
@@ -174,6 +194,17 @@ describe("POST /api/workflows/design (U7/R11/KTD-6)", () => {
     expect(await userDefCount()).toBe(before);
   });
 
+  it("prompt rejects → 5xx but session is still disposed (no leak)", async () => {
+    const { factory, state } = makeRejectingAgent();
+    __setCreateFnAgentForDesign(factory);
+
+    const before = await userDefCount();
+    const res = await postJson("/api/workflows/design", { prompt: "x" });
+    expect(res.status).toBeGreaterThanOrEqual(400);
+    expect(state.disposed).toBe(true);
+    expect(await userDefCount()).toBe(before);
+  });
+
   it("JSON failing parseWorkflowIr (missing start) → 422 with parser message, nothing persisted", async () => {
     const noStart = {
       version: "v1",

packages/dashboard/src/routes/register-workflow-routes.ts

@@ -52,6 +52,13 @@ const designRateLimits = new Map<string, DesignRateLimitEntry>();
  *  when the 10/hour window is exhausted. Same shape as ai-refine.checkRateLimit. */
 function checkDesignRateLimit(ip: string): boolean {
   const now = Date.now();
+  // Prune expired-window entries so the map can't grow unbounded across many
+  // distinct IPs (each request triggers a cheap sweep of stale entries).
+  for (const [key, value] of designRateLimits) {
+    if (now - value.firstRequestAt > DESIGN_RATE_LIMIT_WINDOW_MS) {
+      designRateLimits.delete(key);
+    }
+  }
   const entry = designRateLimits.get(ip);
   if (!entry || now - entry.firstRequestAt > DESIGN_RATE_LIMIT_WINDOW_MS) {
     designRateLimits.set(ip, { count: 1, firstRequestAt: now });
@@ -688,8 +695,11 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void {
       const userPrompt = baseIrJson
         ? `Modify the following base workflow per the request below. Output the full updated WorkflowIr.\n\nBASE WORKFLOW IR:\n${baseIrJson}\n\nREQUEST:\n${prompt}`
         : `Design a workflow for the following request:\n\n${prompt}`;
-      await designSession.prompt(userPrompt);
-      designSession.dispose();
+      try {
+        await designSession.prompt(userPrompt);
+      } finally {
+        designSession.dispose();
+      }
 
       // Extract JSON (handles fences/prose) → JSON.parse → parseWorkflowIr.
       const candidate = extractJsonFromText(output);

packages/i18n/src/config.ts

@@ -86,5 +86,12 @@ export function baseInitOptions(): InitOptions {
     // React (and Ink) escape on render; double-escaping mangles output.
     interpolation: { escapeValue: false },
     returnNull: false,
+    // Untranslated keys are backfilled with "" placeholders across the non-en
+    // catalogs (hundreds of them). With i18next's default returnEmptyString:true
+    // those render blank — even when a component passes an inline English default
+    // to t() — because an empty string is still a "found" value. Setting this
+    // false makes empty values fall through the fallback chain to en, which is
+    // the intended behavior for the placeholder convention.
+    returnEmptyString: false,
   };
 }