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

Address PR review feedback + compact lifecycle warnings banner (#2006)

Browse Source 
- fix: edge-targeted fragment and "insert as optional group" picks from the
  add-step dialog now splice the inserted subgraph into the targeted edge
  (entries inherit the original routing condition, exits feed the old
  target, subgraph moves into the source's column band) instead of dropping
  disconnected fixed-position nodes (Greptile P1).
- feat: the lifecycle warnings banner is now a one-line collapsible
  disclosure (count summary, details on expand) so it no longer dominates
  the editor header.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-11 17:28:12 -07:00
parent a25cf9cfe3
commit d79e2d62af
11 changed files with 275 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
packages/dashboard/app/components/WorkflowNodeEditor.css
View File

@@ -1642,6 +1642,12 @@ Workflow authors need store-produced lifecycle warnings visible in the editor be
color: var(--text);
}
/*
FNXC:WorkflowEditor 2026-07-12-10:30:
The banner is a <details> disclosure: collapsed it reads as one quiet count
line ("2 lifecycle warnings"); expanding reveals the full list. Hide the
native marker and rotate the chevron on open.
*/
.wf-lifecycle-warnings-title {
display: inline-flex;
align-items: center;
@@ -1649,6 +1655,32 @@ Workflow authors need store-produced lifecycle warnings visible in the editor be
font-size: 0.78rem;
font-weight: 650;
color: var(--ws-warning);
cursor: pointer;
list-style: none;
user-select: none;
}
.wf-lifecycle-warnings-title::-webkit-details-marker {
display: none;
}
.wf-lifecycle-warnings-title:focus-visible {
outline: 2px solid var(--accent);
outline-offset: 2px;
border-radius: var(--radius-sm);
}
.wf-lifecycle-warnings-chevron {
transition: transform var(--transition-fast);
opacity: 0.75;
}
.wf-lifecycle-warnings[open] .wf-lifecycle-warnings-chevron {
transform: rotate(180deg);
}
.wf-lifecycle-warnings[open] ul {
margin-top: var(--space-xs);
}
.wf-lifecycle-warnings ul {

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

@@ -91,7 +91,7 @@ import {
FOREACH_CHILD_Y,
} from "./workflow-flow-mapping";
import { autoLayout, applyAutoLayout } from "./workflow-auto-layout";
import { insertNodeOnEdge, findAppendEdgeId } from "./workflow-simple-layout";
import { insertNodeOnEdge, findAppendEdgeId, spliceInsertedSubgraphOnEdge } from "./workflow-simple-layout";
import { WorkflowSimpleCanvas } from "./WorkflowSimpleCanvas";
import { WorkflowAddStepModal, type AddStepPaletteEntry } from "./WorkflowAddStepModal";
import { fetchTraits, fetchStepParsers, type TraitCatalogEntry } from "../api";
@@ -1620,7 +1620,10 @@ function InnerEditor({
surfaces label it.
*/
const handleInsertStepTemplateAsOptionalGroup = useCallback(
(tpl: WorkflowStepTemplate) => {
// FNXC:WorkflowSimpleView 2026-07-12-10:30: PR #2006 review — when the
// add-step dialog targeted an edge "+", this path must splice the new
// optional-group into that edge instead of dropping it free-floating.
(tpl: WorkflowStepTemplate, targetEdgeId?: string | null) => {
if (isBuiltin) return;
const { kind, config } = stepTemplateToNode(tpl);
const fragmentIr = optionalGroupFragmentIr(
@@ -1631,8 +1634,11 @@ function InnerEditor({
x: 240,
y: 200 + (nodes.length % 4) * 40,
});
setNodes(result.nodes);
setEdges(result.edges);
const spliced = targetEdgeId
? spliceInsertedSubgraphOnEdge(result.nodes, result.edges, targetEdgeId, result.insertedNodeIds)
: null;
setNodes(spliced?.nodes ?? result.nodes);
setEdges(spliced?.edges ?? result.edges);
setSelectedNodeId(result.insertedNodeIds[0] ?? null);
},
[isBuiltin, nodes, edges, setNodes, setEdges],
@@ -1644,7 +1650,11 @@ function InnerEditor({
// insertFragment remaps ids + rewires internal edges, landing nodes at a fixed
// offset from the canvas origin.
const handleInsertFragment = useCallback(
(fragment: WorkflowDefinition) => {
// FNXC:WorkflowSimpleView 2026-07-12-10:30: PR #2006 review — an
// edge-targeted pick ("+" on an edge) splices the fragment's entry/exit
// boundary into that edge; the toolbar/free path keeps the classic
// fixed-position landing.
(fragment: WorkflowDefinition, targetEdgeId?: string | null) => {
if (isBuiltin) return false;
const loadedIrFallback = canvasNodesMaterializedRef.current ? undefined : activeWorkflow?.ir;
const conflicts = fragmentSeamConflicts(fragment.ir, nodes, loadedIrFallback);
@@ -1660,8 +1670,11 @@ function InnerEditor({
{ x: 240, y: 200 + (nodes.length % 4) * 40 },
fragment.layout,
);
setNodes(result.nodes);
setEdges(result.edges);
const spliced = targetEdgeId
? spliceInsertedSubgraphOnEdge(result.nodes, result.edges, targetEdgeId, result.insertedNodeIds)
: null;
setNodes(spliced?.nodes ?? result.nodes);
setEdges(spliced?.edges ?? result.edges);
setSelectedNodeId(result.insertedNodeIds[0] ?? null);
return true;
},
@@ -3077,11 +3090,21 @@ function InnerEditor({
)}
</div>
{lifecycleWarnings.length > 0 && (
<div className="wf-lifecycle-warnings" role="status" data-testid="wf-lifecycle-warnings">
<div className="wf-lifecycle-warnings-title">
/* FNXC:WorkflowEditor 2026-07-12-10:30: the lifecycle
warnings banner previously rendered fully expanded and
dominated the editor header (every fresh workflow starts
with two warnings). It is now a one-line disclosure —
count summary, details on demand — in every view mode. */
<details className="wf-lifecycle-warnings" data-testid="wf-lifecycle-warnings">
<summary className="wf-lifecycle-warnings-title" data-testid="wf-lifecycle-warnings-toggle">
<Shield size={14} aria-hidden />
<span>{t("workflows.lifecycleWarningsTitle", "Lifecycle warnings")}</span>
</div>
<span role="status">
{t("workflows.lifecycleWarningsCount", "{{count}} lifecycle warnings", {
count: lifecycleWarnings.length,
})}
</span>
<ChevronDown size={12} className="wf-lifecycle-warnings-chevron" aria-hidden />
</summary>
<ul>
{lifecycleWarnings.map((warning, index) => (
<li key={`${warning.code}:${warning.nodeId ?? ""}:${index}`}>
@@ -3091,7 +3114,7 @@ function InnerEditor({
</li>
))}
</ul>
</div>
</details>
)}
{simpleLayoutEnabled && (
<div className="wf-mobile-shell" data-testid="wf-mobile-shell">
@@ -5479,11 +5502,11 @@ function InnerEditor({
templateConflict={templateConflict}
onPickPalette={handleAddStepPalettePick}
onPickFragment={(fragment) => {
if (handleInsertFragment(fragment)) setAddStepTarget(null);
if (handleInsertFragment(fragment, addStepTarget?.edgeId)) setAddStepTarget(null);
}}
onPickStepTemplate={handleAddStepTemplatePick}
onPickStepTemplateAsOptionalGroup={(tpl) => {
handleInsertStepTemplateAsOptionalGroup(tpl);
handleInsertStepTemplateAsOptionalGroup(tpl, addStepTarget?.edgeId);
setAddStepTarget(null);
}}
/>

35
packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx
View File

@@ -570,8 +570,13 @@ describe("WorkflowNodeEditor", () => {
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
// FNXC:WorkflowEditor 2026-07-12-10:30: banner is a collapsed-by-default
// disclosure: a count summary line, with details revealed on expand.
const banner = await screen.findByTestId("wf-lifecycle-warnings");
expect(banner).toHaveTextContent("Lifecycle warnings");
expect(banner).toHaveTextContent("2 lifecycle warnings");
expect(banner).not.toHaveAttribute("open");
fireEvent.click(screen.getByTestId("wf-lifecycle-warnings-toggle"));
expect(banner).toHaveAttribute("open");
expect(banner).toHaveTextContent("missing-merge-region");
expect(banner).toHaveTextContent("optional-group-after-execution");
expect(banner).toHaveTextContent("plan-review");
@@ -4160,6 +4165,34 @@ describe("WorkflowNodeEditor simplified view modes", () => {
expect(document.querySelector('[data-testid^="wf-simple-insert-"]')).toBeNull();
});
it("splices an edge-targeted fragment pick into the targeted edge", async () => {
// FNXC:WorkflowSimpleView 2026-07-12-10:30: PR #2006 review — fragment
// picks from an edge-targeted add-step dialog must rewire
// source→fragment→target instead of dropping a disconnected subgraph.
vi.mocked(fetchWorkflows).mockResolvedValue([def(), fragmentDef()]);
vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...def(), ...(updates as object) }));
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
await screen.findByTestId("wf-simple-canvas");
// def() has a single edge into end, so the toolbar add targets that edge.
fireEvent.click(screen.getByTestId("wf-simple-toolbar-add-step"));
const dialog = await screen.findByTestId("wf-add-step-modal");
fireEvent.click(within(dialog).getByTestId("wf-add-step-fragment-WF-FRAG"));
await waitFor(() => expect(screen.queryByTestId("wf-add-step-modal")).not.toBeInTheDocument());
// Save and inspect the serialized IR: merge no longer feeds end directly;
// the fragment's lint gate sits between them.
fireEvent.click(screen.getByText("Save").closest("button")!);
await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1));
const [, updates] = vi.mocked(updateWorkflow).mock.calls[0];
const ir = (updates as { ir: { nodes: Array<{ id: string; kind: string }>; edges: Array<{ from: string; to: string; condition?: string }> } }).ir;
const insertedGate = ir.nodes.find((n) => n.kind === "gate" && n.id !== "lint");
expect(insertedGate).toBeDefined();
expect(ir.edges.some((e) => e.from === "merge" && e.to === "end")).toBe(false);
expect(ir.edges.some((e) => e.from === "merge" && e.to === insertedGate!.id)).toBe(true);
expect(ir.edges.some((e) => e.from === insertedGate!.id && e.to === "end")).toBe(true);
});
it("defaults the mobile graph tab to the simplified canvas with a list fallback", async () => {
mockWorkflowEditorViewport("mobile");
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);

65
packages/dashboard/app/components/__tests__/workflow-simple-layout.test.ts
View File

@@ -6,6 +6,7 @@ import {
edgeSupportsSimpleInsert,
insertNodeOnEdge,
findAppendEdgeId,
spliceInsertedSubgraphOnEdge,
SIMPLE_NODE_WIDTH,
} from "../workflow-simple-layout";
@@ -203,3 +204,67 @@ describe("findAppendEdgeId", () => {
expect(findAppendEdgeId(nodes, [edge("e1", "start", "a")])).toBeNull();
});
});
/*
FNXC:WorkflowSimpleView 2026-07-12-10:30:
PR #2006 review coverage: edge-targeted fragment / optional-group inserts must
splice the already-inserted subgraph into the targeted edge (entries inherit
the original condition, exits feed the old target, the original edge is
removed, the subgraph moves into the source's y band).
*/
describe("spliceInsertedSubgraphOnEdge", () => {
const baseNodes = (): N[] => [
node("start", "start", 0, 120),
node("a", "prompt", 300, 120),
node("end", "end", 900, 120),
];
const baseEdges = (): FlowEdge[] => [edge("e1", "start", "a"), edge("e2", "a", "end", "failure")];
it("wires source→entry and exit→target, removes the edge, preserves condition", () => {
const nodes = [
...baseNodes(),
node("f1", "gate", 240, 700),
node("f2", "script", 520, 700),
];
const edges = [...baseEdges(), edge("f-e", "f1", "f2")];
const result = spliceInsertedSubgraphOnEdge(nodes, edges, "e2", ["f1", "f2"]);
expect(result).not.toBeNull();
const { nodes: nextNodes, edges: nextEdges } = result!;
expect(nextEdges.find((e) => e.id === "e2")).toBeUndefined();
const inbound = nextEdges.find((e) => e.source === "a" && e.target === "f1");
const outbound = nextEdges.find((e) => e.source === "f2" && e.target === "end");
expect(inbound?.data?.condition).toBe("failure");
expect(outbound?.data?.condition).toBe("success");
// Subgraph translated into the source's y band, preserving relative layout.
const f1 = nextNodes.find((n) => n.id === "f1")!;
const f2 = nextNodes.find((n) => n.id === "f2")!;
expect(f1.position.y).toBe(120 + 8);
expect(f2.position.x - f1.position.x).toBe(280);
});
it("wires an optional-group container while leaving its template children alone", () => {
const nodes = [
...baseNodes(),
node("grp", "optional-group", 240, 700, { style: { width: 560, height: 220 } }),
node("grp-child", "prompt", 30, 56, { parentId: "grp" }),
];
const result = spliceInsertedSubgraphOnEdge(nodes, baseEdges(), "e2", ["grp", "grp-child"]);
expect(result).not.toBeNull();
const inbound = result!.edges.find((e) => e.source === "a" && e.target === "grp");
const outbound = result!.edges.find((e) => e.source === "grp" && e.target === "end");
expect(inbound).toBeDefined();
expect(outbound).toBeDefined();
// Child keeps its parent-relative position.
const child = result!.nodes.find((n) => n.id === "grp-child")!;
expect(child.position).toEqual({ x: 30, y: 56 });
});
it("returns null when the target edge is gone or ineligible", () => {
const nodes = [...baseNodes(), node("f1", "gate", 240, 700)];
expect(spliceInsertedSubgraphOnEdge(nodes, baseEdges(), "missing", ["f1"])).toBeNull();
const rework = [edge("r1", "a", "start", "failure", "rework")];
expect(spliceInsertedSubgraphOnEdge(nodes, rework, "r1", ["f1"])).toBeNull();
expect(spliceInsertedSubgraphOnEdge(baseNodes(), baseEdges(), "e2", ["not-present"])).toBeNull();
});
});

95
packages/dashboard/app/components/workflow-simple-layout.ts
View File

@@ -269,6 +269,101 @@ export function insertNodeOnEdge(
return { nodes: refreshed.nodes, edges: refreshed.edges, newNodeId: id };
}
/*
FNXC:WorkflowSimpleView 2026-07-12-10:30:
PR #2006 review: fragment and "insert as optional group" picks from an
edge-targeted add-step dialog previously fell through to the advanced
fixed-position insertFragment path, leaving the "+" edge untouched and the
inserted subgraph disconnected. This splice helper wires an ALREADY-inserted
subgraph into the targeted edge: source→(subgraph entries) preserving the
original routing condition, (subgraph exits)→target as success, original edge
removed, and the subgraph translated next to the source node's y so v2
column banding stays valid for simple-view authors who cannot drag.
*/
/**
* Wire an inserted subgraph (from insertFragment) into an existing edge.
* Entries/exits derive from in/out degree over the subgraph's internal
* non-rework edges (mirroring template boundary semantics). Returns null when
* the edge is gone/ineligible or the subgraph has no top-level nodes — the
* caller keeps the plain fixed-position insert in that case.
*/
export function spliceInsertedSubgraphOnEdge(
nodes: LayoutNode[],
edges: FlowEdge[],
edgeId: string,
insertedNodeIds: readonly string[],
): { nodes: LayoutNode[]; edges: FlowEdge[] } | null {
const edge = edges.find((e) => e.id === edgeId);
if (!edge || !edgeSupportsSimpleInsert(edge)) return null;
const source = nodes.find((n) => n.id === edge.source);
const target = nodes.find((n) => n.id === edge.target);
if (!source || !target) return null;
const insertedSet = new Set(insertedNodeIds);
const insertedTop = nodes.filter(
(n) => insertedSet.has(n.id) && !n.parentId && !isColumnBandNode(n.id),
);
if (insertedTop.length === 0) return null;
const topIds = new Set(insertedTop.map((n) => n.id));
const indegree = new Map<string, number>();
const outdegree = new Map<string, number>();
for (const id of topIds) {
indegree.set(id, 0);
outdegree.set(id, 0);
}
for (const e of edges) {
if (isVisualOnlyWorkflowEdge(e)) continue;
if ((e.data?.kind as string | undefined) === "rework") continue;
if (!topIds.has(e.source) || !topIds.has(e.target)) continue;
outdegree.set(e.source, (outdegree.get(e.source) ?? 0) + 1);
indegree.set(e.target, (indegree.get(e.target) ?? 0) + 1);
}
const entries = insertedTop.filter((n) => (indegree.get(n.id) ?? 0) === 0);
const exits = insertedTop.filter((n) => (outdegree.get(n.id) ?? 0) === 0);
const entryNodes = entries.length > 0 ? entries : insertedTop;
const exitNodes = exits.length > 0 ? exits : insertedTop;
// Translate the subgraph beside the wiring point: same y band as the source
// (column validity), x around the source/target midpoint. Relative layout
// inside the subgraph is preserved.
const minX = Math.min(...insertedTop.map((n) => n.position.x));
const minY = Math.min(...insertedTop.map((n) => n.position.y));
const deltaX = (source.position.x + target.position.x) / 2 + 24 - minX;
const deltaY = source.position.y + 8 - minY;
const nextNodes = nodes.map((n) =>
topIds.has(n.id)
? { ...n, position: { x: n.position.x + deltaX, y: n.position.y + deltaY } }
: n,
);
const inboundCondition = (edge.data?.condition as string | undefined) ?? "success";
const newEdges: FlowEdge[] = [
...entryNodes.map((entry) => ({
id: `e-${newNodeId()}`,
source: edge.source,
target: entry.id,
label: shortConditionLabel(inboundCondition),
data: { condition: inboundCondition, kind: undefined },
className: edgeClassName(inboundCondition, false),
interactionWidth: WF_EDGE_INTERACTION_WIDTH,
})),
...exitNodes.map((exit) => ({
id: `e-${newNodeId()}`,
source: exit.id,
target: edge.target,
label: shortConditionLabel("success"),
data: { condition: "success", kind: undefined },
className: edgeClassName("success", false),
interactionWidth: WF_EDGE_INTERACTION_WIDTH,
})),
];
const nextEdges = [...edges.filter((e) => e.id !== edgeId), ...newEdges];
return refreshTemplateContainerVisualBoundaries(nextNodes, nextEdges);
}
/**
* The simple view's "+ Add step" (no specific edge): insert before the `end`
* node when a single wiring point is unambiguous, otherwise signal the caller

2
packages/i18n/locales/en/app.json
View File

@@ -8806,6 +8806,8 @@
"importInvalidJson": "That file isn't valid JSON.",
"importStripped": "Auto-approval flags were removed from imported nodes",
"importTooltip": "Import a workflow from a JSON file",
"lifecycleWarningsCount_one": "{{count}} lifecycle warning",
"lifecycleWarningsCount_other": "{{count}} lifecycle warnings",
"loadFailed": "Failed to load workflows",
"loading": "Loading…",
"migrationNotice": "Your legacy workflow steps were converted — find them as templates in the palette and as the \"Migrated steps\" workflow.",

2
packages/i18n/locales/es/app.json
View File

@@ -8795,6 +8795,8 @@
"importInvalidJson": "Ese archivo no es JSON válido.",
"importStripped": "Se eliminaron los indicadores de aprobación automática de los nodos importados",
"importTooltip": "Importar un flujo de trabajo desde un archivo JSON",
"lifecycleWarningsCount_one": "",
"lifecycleWarningsCount_other": "",
"loadFailed": "",
"loading": "",
"migrationNotice": "Los pasos del flujo de trabajo heredado fueron convertidos — encuéntralos como plantillas en la paleta y como el flujo de trabajo «Pasos migrados».",

2
packages/i18n/locales/fr/app.json
View File

@@ -8796,6 +8796,8 @@
"importInvalidJson": "Ce fichier n'est pas un JSON valide.",
"importStripped": "Les indicateurs d'approbation automatique ont été supprimés des nœuds importés",
"importTooltip": "Importer un workflow depuis un fichier JSON",
"lifecycleWarningsCount_one": "",
"lifecycleWarningsCount_other": "",
"loadFailed": "",
"loading": "",
"migrationNotice": "Vos anciennes étapes de workflow ont été converties — retrouvez-les sous forme de modèles dans la palette et sous le workflow « Étapes migrées ».",

2
packages/i18n/locales/ko/app.json
View File

@@ -8795,6 +8795,8 @@
"importInvalidJson": "이 파일은 유효한 JSON이 아닙니다.",
"importStripped": "가져온 노드에서 자동 승인 플래그가 제거되었습니다",
"importTooltip": "JSON 파일에서 워크플로 가져오기",
"lifecycleWarningsCount_one": "",
"lifecycleWarningsCount_other": "",
"loadFailed": "",
"loading": "",
"migrationNotice": "이전 워크플로 단계가 변환되었습니다 — 팔레트의 템플릿과 \"마이그레이션된 단계\" 워크플로에서 찾을 수 있습니다.",

2
packages/i18n/locales/zh-CN/app.json
View File

@@ -8795,6 +8795,8 @@
"importInvalidJson": "该文件不是有效的 JSON。",
"importStripped": "已从导入的节点中移除自动审批标志",
"importTooltip": "从 JSON 文件导入工作流",
"lifecycleWarningsCount_one": "",
"lifecycleWarningsCount_other": "",
"loadFailed": "",
"loading": "",
"migrationNotice": "您的旧版工作流步骤已转换——在面板模板和“已迁移步骤”工作流中查找。",

2
packages/i18n/locales/zh-TW/app.json
View File

@@ -8795,6 +8795,8 @@
"importInvalidJson": "該檔案不是有效的 JSON。",
"importStripped": "已從匯入的節點移除自動核准旗標",
"importTooltip": "從 JSON 檔案匯入工作流程",
"lifecycleWarningsCount_one": "",
"lifecycleWarningsCount_other": "",
"loadFailed": "",
"loading": "",
"migrationNotice": "你的舊版工作流程步驟已轉換 — 可在選盤中以範本形式找到它們，以及名為「已遷移步驟」的工作流程。",