merge main: per-column agent assignment (binding validation, policy-escalation handshake, override notes) into workflow editor consolidation
Resolved 4 conflicts preserving both feature sets: - WorkflowNodeEditor.tsx: kept card-node/edge/dialog/dirty-guard/auto-layout/ onboarding/template/AI-edit features; wired main's columnAgentsEnabled flag gate, override-column agent registry load, and policy-escalation save retry (finishSave helper wraps both update payloads). - WorkflowNodeEditor.test.tsx: kept U2/U4/AI-design describes plus main's U6 column-agent describe; merged api mock + import lists. - register-workflow-routes.ts: merged import sets (design DI seam + column-agent validators). - agent-tools.ts: strip-approval-flags and column-agent binding assertion now both run in create/update tools. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -41,6 +41,7 @@ import type { ToastType } from "../hooks/useToast";
|
||||
import { useOverlayDismiss } from "../hooks/useOverlayDismiss";
|
||||
import { useConfirm } from "../hooks/useConfirm";
|
||||
import { useModalResizePersist } from "../hooks/useModalResizePersist";
|
||||
import { useAppSettings } from "../hooks/useAppSettings";
|
||||
import { workflowNodeTypes, type WorkflowFlowNodeData, type WorkflowEditorNodeKind } from "./nodes/WorkflowNodeTypes";
|
||||
import { WorkflowEditorCatalogContext } from "./nodes/WorkflowEditorCatalogContext";
|
||||
import type { NodeSummaryCatalogs } from "./nodes/node-summary";
|
||||
@@ -757,6 +758,14 @@ function InnerEditor({
|
||||
return !nodes.some((n) => USER_NODE_KINDS.has(n.data.kind));
|
||||
}, [activeWorkflow, isBuiltin, nodes]);
|
||||
|
||||
// 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(() => {
|
||||
@@ -1574,47 +1583,71 @@ function InnerEditor({
|
||||
// loaded workflow (KTD-10 inline rename/description persist here).
|
||||
const nameChanged = trimmedName !== activeWorkflow.name;
|
||||
const descChanged = description !== (activeWorkflow.description ?? "");
|
||||
const updated = await updateWorkflow(
|
||||
activeWorkflow.id,
|
||||
{
|
||||
ir,
|
||||
layout,
|
||||
...(nameChanged ? { name: trimmedName } : {}),
|
||||
...(descChanged ? { description } : {}),
|
||||
},
|
||||
projectId,
|
||||
);
|
||||
setWorkflows((ws) => ws.map((w) => (w.id === updated.id ? updated : w)));
|
||||
// Re-baseline the dirty snapshot to the just-saved state so the editor is
|
||||
// clean immediately after a successful save.
|
||||
loadedSnapshotRef.current = serializeGraph(
|
||||
updated.name,
|
||||
updated.description ?? "",
|
||||
nodes,
|
||||
edges,
|
||||
columns,
|
||||
fields,
|
||||
);
|
||||
setName(updated.name);
|
||||
setDescription(updated.description ?? "");
|
||||
// Validate by compiling — surfaces non-linear graphs as a banner.
|
||||
try {
|
||||
await compileWorkflow(updated.id, projectId);
|
||||
addToast(t("workflows.saved", "Workflow saved"), "success");
|
||||
} catch (compileErr) {
|
||||
const compileMsg = getErrorMessage(compileErr) || "";
|
||||
// KTD-4: branching graphs reject with this shared suffix from
|
||||
// workflow-compiler.ts (both the fan-out and off-main-path messages).
|
||||
// Such a graph still runs on the interpreter — present it as info, not a
|
||||
// warning. NOTE: this string is coupled to the compiler's message; if
|
||||
// that wording changes, update both sites (see compiler message site).
|
||||
if (compileMsg.includes("require the workflow interpreter (deferred)")) {
|
||||
setInterpreterOnly(true);
|
||||
} else {
|
||||
setValidationError(
|
||||
compileMsg || t("workflows.savedNotCompilable", "Workflow saved but cannot be compiled"),
|
||||
);
|
||||
const finishSave = async (updated: Awaited<ReturnType<typeof updateWorkflow>>) => {
|
||||
setWorkflows((ws) => ws.map((w) => (w.id === updated.id ? updated : w)));
|
||||
// Re-baseline the dirty snapshot to the just-saved state so the editor is
|
||||
// clean immediately after a successful save.
|
||||
loadedSnapshotRef.current = serializeGraph(
|
||||
updated.name,
|
||||
updated.description ?? "",
|
||||
nodes,
|
||||
edges,
|
||||
columns,
|
||||
fields,
|
||||
);
|
||||
setName(updated.name);
|
||||
setDescription(updated.description ?? "");
|
||||
// Validate by compiling — surfaces non-linear graphs as a banner.
|
||||
try {
|
||||
await compileWorkflow(updated.id, projectId);
|
||||
addToast(t("workflows.saved", "Workflow saved"), "success");
|
||||
} catch (compileErr) {
|
||||
const compileMsg = getErrorMessage(compileErr) || "";
|
||||
// KTD-4: branching graphs reject with this shared suffix from
|
||||
// workflow-compiler.ts (both the fan-out and off-main-path messages).
|
||||
// Such a graph still runs on the interpreter — present it as info, not a
|
||||
// warning. NOTE: this string is coupled to the compiler's message; if
|
||||
// that wording changes, update both sites (see compiler message site).
|
||||
if (compileMsg.includes("require the workflow interpreter (deferred)")) {
|
||||
setInterpreterOnly(true);
|
||||
} else {
|
||||
setValidationError(
|
||||
compileMsg || t("workflows.savedNotCompilable", "Workflow saved but cannot be compiled"),
|
||||
);
|
||||
}
|
||||
}
|
||||
};
|
||||
const savePayload = {
|
||||
ir,
|
||||
layout,
|
||||
...(nameChanged ? { name: trimmedName } : {}),
|
||||
...(descChanged ? { description } : {}),
|
||||
};
|
||||
try {
|
||||
await finishSave(await updateWorkflow(activeWorkflow.id, savePayload, projectId));
|
||||
} catch (err) {
|
||||
// Policy-escalation handshake (R13, PR #1432 review): the route rejects a
|
||||
// binding to a broader-than-default agent until the author explicitly
|
||||
// confirms. Surface the server's explanation, then retry with the flag —
|
||||
// otherwise such bindings would be unsavable from the dashboard.
|
||||
// Shape-checked rather than `instanceof ApiRequestError` so test doubles
|
||||
// (and any error wrapper) that carry the details payload still route here.
|
||||
const escalation =
|
||||
(err as { details?: { policyEscalation?: boolean } } | null)?.details?.policyEscalation === true;
|
||||
if (!escalation) throw err;
|
||||
const proceed = window.confirm(
|
||||
`${getErrorMessage(err)}\n\n${t(
|
||||
"workflowColumns.confirmPolicyEscalation",
|
||||
"Bind it anyway? The column agent will run with broader permissions than this project's default.",
|
||||
)}`,
|
||||
);
|
||||
if (!proceed) {
|
||||
addToast(t("workflowColumns.escalationDeclined", "Save cancelled — column agent binding not confirmed"), "error");
|
||||
return;
|
||||
}
|
||||
await finishSave(
|
||||
await updateWorkflow(activeWorkflow.id, { ...savePayload, confirmPolicyEscalation: true }, projectId),
|
||||
);
|
||||
}
|
||||
} catch (err) {
|
||||
const message = getErrorMessage(err) || t("workflows.saveFailed", "Failed to save workflow");
|
||||
@@ -1693,6 +1726,13 @@ function InnerEditor({
|
||||
// non-fatal — summaries fall back to raw ids — so the prefetch is toastless.
|
||||
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[]>([]);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -1733,6 +1773,33 @@ 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(() => {
|
||||
// Foreach template children don't carry their own column in irToFlow — they
|
||||
// inherit the enclosing foreach group's column at execution (R4). Mirror that
|
||||
// inheritance here so a step-execute prompt inside an override-bound foreach
|
||||
// still shows the note (PR #1432 review).
|
||||
const columnId =
|
||||
selectedNode?.data.column
|
||||
?? (selectedNode?.parentId
|
||||
? nodes.find((n) => n.id === selectedNode.parentId)?.data.column
|
||||
: undefined);
|
||||
if (!columnId) return undefined;
|
||||
const col = columns.find((c) => c.id === columnId);
|
||||
if (!col?.agent || col.agent.mode !== "override") return undefined;
|
||||
return col.agent;
|
||||
}, [selectedNode?.data.column, selectedNode?.parentId, nodes, columns]);
|
||||
|
||||
// Resolve the override agent's display name from the loaded registry; when the
|
||||
// id is stale (not in the list) fall back to the not-found treatment.
|
||||
const overrideAgent = useMemo(
|
||||
() => (overrideColumnBinding ? agents.find((a) => a.id === overrideColumnBinding.agentId) : undefined),
|
||||
[overrideColumnBinding, agents],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
// step-review offers an optional review model picker (KTD-4).
|
||||
if (selectedNode?.data.kind === "step-review" && models.length === 0) {
|
||||
@@ -1747,7 +1814,10 @@ function InnerEditor({
|
||||
addToast(getErrorMessage(err) || "Failed to load models", "error");
|
||||
});
|
||||
} else if (currentExecutor === "agent" && agents.length === 0) {
|
||||
fetchAgents().then(setAgents).catch((err) => {
|
||||
// Project-scoped, matching WorkflowColumnPanel's fetchAgents(undefined,
|
||||
// projectId) — an unscoped fetch returns the wrong registry in
|
||||
// multi-project deployments (PR #1432 review).
|
||||
fetchAgents(undefined, projectId).then(setAgents).catch((err) => {
|
||||
addToast(getErrorMessage(err) || "Failed to load agents", "error");
|
||||
});
|
||||
} else if (currentExecutor === "skill" && skills.length === 0) {
|
||||
@@ -1807,6 +1877,25 @@ function InnerEditor({
|
||||
[guardedDismiss, activeId],
|
||||
);
|
||||
|
||||
// When the selected node sits in an override column, eagerly load the agent
|
||||
// registry so the "overridden by column agent <name>" note can resolve the
|
||||
// name even if this node's own executor isn't "agent".
|
||||
useEffect(() => {
|
||||
if (!overrideColumnBinding || agents.length > 0) return;
|
||||
let cancelled = false;
|
||||
// Project-scoped (PR #1432 review): without projectId this resolves from the
|
||||
// wrong scope in multi-project deployments — the override note would show a
|
||||
// false "not found" for a perfectly valid project agent.
|
||||
Promise.resolve(fetchAgents(undefined, projectId)).then((list) => {
|
||||
if (!cancelled) setAgents(list ?? []);
|
||||
}).catch((err) => {
|
||||
if (!cancelled) addToast(getErrorMessage(err) || "Failed to load agents", "error");
|
||||
});
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [overrideColumnBinding, agents.length, projectId, addToast]);
|
||||
|
||||
const overlayProps = useOverlayDismiss(requestClose);
|
||||
|
||||
return (
|
||||
@@ -1951,6 +2040,7 @@ function InnerEditor({
|
||||
readOnly={isBuiltin}
|
||||
projectId={projectId}
|
||||
addToast={addToast}
|
||||
columnAgentsEnabled={columnAgentsEnabled}
|
||||
/>
|
||||
)}
|
||||
</section>
|
||||
@@ -2483,6 +2573,19 @@ function InnerEditor({
|
||||
</select>
|
||||
</label>
|
||||
|
||||
{overrideColumnBinding && (
|
||||
<p className="wf-inspector-note wf-inspector-note--warn" data-testid="wf-node-overridden-by-column-agent">
|
||||
{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 }),
|
||||
},
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
|
||||
{currentExecutor === "model" && (
|
||||
<label className="wf-field">
|
||||
<span>Model</span>
|
||||
@@ -2501,20 +2604,37 @@ function InnerEditor({
|
||||
</label>
|
||||
)}
|
||||
|
||||
{currentExecutor === "agent" && (
|
||||
<label className="wf-field">
|
||||
<span>Agent</span>
|
||||
<select
|
||||
value={String(selectedNode.data.config?.agentId ?? "")}
|
||||
onChange={(e) => updateSelectedData({ config: { agentId: e.target.value || undefined } })}
|
||||
>
|
||||
<option value="">— select agent —</option>
|
||||
{agents.map((a) => (
|
||||
<option key={a.id} value={a.id}>{a.name}</option>
|
||||
))}
|
||||
</select>
|
||||
</label>
|
||||
)}
|
||||
{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 (
|
||||
<label className="wf-field">
|
||||
<span>Agent</span>
|
||||
<select
|
||||
value={nodeAgentId}
|
||||
onChange={(e) => updateSelectedData({ config: { agentId: e.target.value || undefined } })}
|
||||
>
|
||||
<option value="">— select agent —</option>
|
||||
{nodeAgentStale && (
|
||||
<option value={nodeAgentId}>
|
||||
{t("workflowColumns.agentNotFound", "Agent not found — {{id}}", { id: nodeAgentId })}
|
||||
</option>
|
||||
)}
|
||||
{agents.map((a) => (
|
||||
<option key={a.id} value={a.id}>{a.name}</option>
|
||||
))}
|
||||
</select>
|
||||
{nodeAgentStale && (
|
||||
<p className="wf-inspector-note wf-inspector-note--warn" data-testid="wf-node-agent-stale">
|
||||
{t("workflowColumns.agentNotFound", "Agent not found — {{id}}", { id: nodeAgentId })}
|
||||
</p>
|
||||
)}
|
||||
</label>
|
||||
);
|
||||
})()}
|
||||
|
||||
{currentExecutor === "skill" && (
|
||||
<label className="wf-field">
|
||||
|
||||
Reference in New Issue
Block a user