diff --git a/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx b/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx
index 397892ffd..d873891f3 100644
--- a/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx
+++ b/src/components/shared/ComponentEditor/ComponentEditorDialog.test.tsx
@@ -337,5 +337,54 @@ describe("", () => {
expect(onCloseMock).toHaveBeenCalledTimes(1);
});
});
+
+ test("calls onComponentSaved instead of importing to the library when provided", async () => {
+ const onCloseMock = vi.fn();
+ const onComponentSavedMock = vi.fn();
+ const mockHydratedComponent = {
+ spec: {
+ implementation: { container: { image: "test" } },
+ metadata: { annotations: {} },
+ },
+ name: "test-component",
+ digest: "abc123",
+ text: "name: test-component",
+ };
+
+ vi.mocked(hydrateComponentReference).mockResolvedValue(
+ mockHydratedComponent,
+ );
+
+ renderWithProviders(
+ ,
+ );
+
+ await waitFor(() => {
+ expect(
+ screen.getByRole("button", { name: /Save/i }),
+ ).toBeInTheDocument();
+ });
+
+ fireEvent.click(screen.getByRole("button", { name: /Save/i }));
+
+ await waitFor(() => {
+ // The edited component is handed to the caller to apply.
+ expect(onComponentSavedMock).toHaveBeenCalledWith(
+ mockHydratedComponent,
+ );
+ expect(onCloseMock).toHaveBeenCalledTimes(1);
+ });
+
+ // It must NOT fall back to the library-import behavior.
+ expect(mockAddToComponentLibrary).not.toHaveBeenCalled();
+ expect(mockToast).not.toHaveBeenCalledWith(
+ expect.stringContaining("imported successfully"),
+ "success",
+ );
+ });
});
});
diff --git a/src/components/shared/ComponentEditor/ComponentEditorDialog.tsx b/src/components/shared/ComponentEditor/ComponentEditorDialog.tsx
index 3579bf65a..933087ece 100644
--- a/src/components/shared/ComponentEditor/ComponentEditorDialog.tsx
+++ b/src/components/shared/ComponentEditor/ComponentEditorDialog.tsx
@@ -11,7 +11,10 @@ import useToastNotification from "@/hooks/useToastNotification";
import { useAnalytics } from "@/providers/AnalyticsProvider";
import { useComponentLibrary } from "@/providers/ComponentLibraryProvider";
import { hydrateComponentReference } from "@/services/componentService";
-import { isContainerImplementation } from "@/utils/componentSpec";
+import {
+ type HydratedComponentReference,
+ isContainerImplementation,
+} from "@/utils/componentSpec";
import { saveComponent } from "@/utils/localforage";
import { FullscreenElement } from "../FullscreenElement";
@@ -72,10 +75,21 @@ export const ComponentEditorDialog = withSuspenseWrapper(
text,
templateName = "empty",
onClose,
+ onComponentSaved,
}: {
text?: string;
templateName?: SupportedTemplate;
onClose: () => void;
+ /**
+ * When provided, the editor is being used to edit an existing target (e.g.
+ * a selected task's component) rather than to import a brand new component
+ * into the library. The callback receives the hydrated, edited component
+ * and is responsible for applying it (and any user feedback). When omitted,
+ * the editor falls back to importing the component into the library.
+ */
+ onComponentSaved?: (
+ hydratedComponent: HydratedComponentReference,
+ ) => void | Promise;
}) => {
const notify = useToastNotification();
const { track } = useAnalytics();
@@ -190,16 +204,23 @@ export const ComponentEditorDialog = withSuspenseWrapper(
onClose();
- await addToComponentLibrary(hydratedComponent, "editor_save");
+ if (onComponentSaved) {
+ // Editing an existing target (e.g. a selected task): apply the edit to
+ // that target instead of importing a new library component. The caller
+ // owns the success/feedback messaging.
+ await onComponentSaved(hydratedComponent);
+ } else {
+ await addToComponentLibrary(hydratedComponent, "editor_save");
+ notify(
+ `Component ${hydratedComponent.name} imported successfully`,
+ "success",
+ );
+ }
track("component_editor.save.completed", {
mode,
selected_template: templateName,
});
- notify(
- `Component ${hydratedComponent.name} imported successfully`,
- "success",
- );
};
const handleClose = () => {
diff --git a/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.test.ts b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.test.ts
new file mode 100644
index 000000000..58dc1555a
--- /dev/null
+++ b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.test.ts
@@ -0,0 +1,163 @@
+import { describe, expect, it } from "vitest";
+
+import type { ComponentReference, GraphSpec } from "@/utils/componentSpec";
+
+import { replaceTaskComponentRef } from "./replaceTaskComponentRef";
+
+const taxiRef = (command: string): ComponentReference => ({
+ name: "Chicago Taxi Trips dataset",
+ digest: "old-digest",
+ spec: {
+ name: "Chicago Taxi Trips dataset",
+ inputs: [
+ { name: "Limit", type: "Integer", default: "1000" },
+ { name: "Select", type: "String" },
+ ],
+ outputs: [{ name: "Table" }],
+ implementation: { container: { image: "alpine/curl", command: [command] } },
+ },
+});
+
+const baseGraphSpec = (): GraphSpec => ({
+ tasks: {
+ dataset: {
+ componentRef: taxiRef("sed -E 's/x/'\\1'/g'"),
+ annotations: { "editor.position": '{"x":10,"y":20}' },
+ arguments: {
+ Limit: { graphInput: { inputName: "Input" } },
+ Select: "tips,trip_seconds",
+ },
+ },
+ train: {
+ componentRef: { name: "Train" },
+ arguments: {
+ training_data: {
+ taskOutput: { taskId: "dataset", outputName: "Table" },
+ },
+ },
+ },
+ },
+});
+
+describe("replaceTaskComponentRef", () => {
+ it("swaps the componentRef in place without renaming the task", () => {
+ const graphSpec = baseGraphSpec();
+ const fixedRef: ComponentReference = {
+ ...taxiRef("sed -E 's/x/\\1/g'"),
+ digest: "new-digest",
+ };
+
+ const { updatedGraphSpec, lostInputs } = replaceTaskComponentRef(
+ "dataset",
+ fixedRef,
+ graphSpec,
+ );
+
+ // Same key, no "dataset 2".
+ expect(Object.keys(updatedGraphSpec.tasks)).toEqual(["dataset", "train"]);
+ expect(updatedGraphSpec.tasks.dataset.componentRef.digest).toBe(
+ "new-digest",
+ );
+ expect(lostInputs).toEqual([]);
+ });
+
+ it("preserves the task's arguments, annotations and downstream wiring", () => {
+ const graphSpec = baseGraphSpec();
+ const fixedRef = { ...taxiRef("fixed"), digest: "new-digest" };
+
+ const { updatedGraphSpec } = replaceTaskComponentRef(
+ "dataset",
+ fixedRef,
+ graphSpec,
+ );
+
+ expect(updatedGraphSpec.tasks.dataset.arguments).toEqual({
+ Limit: { graphInput: { inputName: "Input" } },
+ Select: "tips,trip_seconds",
+ });
+ expect(updatedGraphSpec.tasks.dataset.annotations).toEqual({
+ "editor.position": '{"x":10,"y":20}',
+ });
+ // Downstream consumer still wired to the (unchanged) output/taskId.
+ expect(updatedGraphSpec.tasks.train.arguments).toEqual({
+ training_data: {
+ taskOutput: { taskId: "dataset", outputName: "Table" },
+ },
+ });
+ });
+
+ it("does not mutate the original graph spec", () => {
+ const graphSpec = baseGraphSpec();
+ const fixedRef = { ...taxiRef("fixed"), digest: "new-digest" };
+
+ replaceTaskComponentRef("dataset", fixedRef, graphSpec);
+
+ expect(graphSpec.tasks.dataset.componentRef.digest).toBe("old-digest");
+ });
+
+ it("drops arguments for inputs the edited component no longer defines", () => {
+ const graphSpec = baseGraphSpec();
+ const refWithoutSelect: ComponentReference = {
+ name: "Chicago Taxi Trips dataset",
+ digest: "new-digest",
+ spec: {
+ name: "Chicago Taxi Trips dataset",
+ inputs: [{ name: "Limit", type: "Integer" }],
+ outputs: [{ name: "Table" }],
+ implementation: { container: { image: "alpine/curl" } },
+ },
+ };
+
+ const { updatedGraphSpec, lostInputs } = replaceTaskComponentRef(
+ "dataset",
+ refWithoutSelect,
+ graphSpec,
+ );
+
+ expect(lostInputs.map((input) => input.name)).toEqual(["Select"]);
+ expect(updatedGraphSpec.tasks.dataset.arguments).toEqual({
+ Limit: { graphInput: { inputName: "Input" } },
+ });
+ });
+
+ it("drops downstream bindings to outputs the edited component no longer produces", () => {
+ const graphSpec = baseGraphSpec();
+ const refWithoutTable: ComponentReference = {
+ name: "Chicago Taxi Trips dataset",
+ digest: "new-digest",
+ spec: {
+ name: "Chicago Taxi Trips dataset",
+ inputs: [
+ { name: "Limit", type: "Integer" },
+ { name: "Select", type: "String" },
+ ],
+ outputs: [{ name: "RenamedTable" }],
+ implementation: { container: { image: "alpine/curl" } },
+ },
+ };
+
+ const { updatedGraphSpec } = replaceTaskComponentRef(
+ "dataset",
+ refWithoutTable,
+ graphSpec,
+ );
+
+ expect(updatedGraphSpec.tasks.train.arguments).toEqual({});
+ });
+
+ it("returns the graph unchanged when the task does not exist", () => {
+ const graphSpec = baseGraphSpec();
+ const fixedRef = { ...taxiRef("fixed"), digest: "new-digest" };
+
+ const { updatedGraphSpec, lostInputs } = replaceTaskComponentRef(
+ "missing",
+ fixedRef,
+ graphSpec,
+ );
+
+ expect(lostInputs).toEqual([]);
+ expect(updatedGraphSpec.tasks.dataset.componentRef.digest).toBe(
+ "old-digest",
+ );
+ });
+});
diff --git a/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.ts b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.ts
new file mode 100644
index 000000000..5a448cac7
--- /dev/null
+++ b/src/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef.ts
@@ -0,0 +1,100 @@
+import type {
+ ArgumentType,
+ ComponentReference,
+ GraphSpec,
+ InputSpec,
+} from "@/utils/componentSpec";
+import { deepClone } from "@/utils/deepClone";
+
+/**
+ * Replaces a task's componentRef in place (keeping the same taskId, position,
+ * annotations and execution options) while preserving as much of the task's
+ * existing wiring as possible.
+ *
+ * This is the right primitive for "Edit Component Definition": the user has
+ * selected a task and edited its component, so their intent is to update that
+ * specific task and keep everything they already configured.
+ *
+ * Unlike `replaceTaskNode` (used by "Upgrade"), this does NOT rename the task:
+ * renaming would re-key the task to a new unique id derived from the component
+ * name and is surprising for an in-place edit.
+ *
+ * Wiring is preserved with two safe exceptions, mirroring the upgrade flow:
+ * - argument bindings on this task for inputs that no longer exist in the
+ * edited component are dropped (and reported via `lostInputs`);
+ * - argument bindings on downstream tasks that consume an output this
+ * component no longer produces are dropped, to avoid dangling references.
+ *
+ * In the common case (editing the command/image, no input/output changes)
+ * nothing is dropped and every argument and default is preserved.
+ */
+export const replaceTaskComponentRef = (
+ taskId: string,
+ newComponentRef: ComponentReference,
+ graphSpec: GraphSpec,
+) => {
+ const updatedGraphSpec = deepClone(graphSpec);
+ const task = updatedGraphSpec.tasks[taskId];
+
+ if (!task) {
+ return { updatedGraphSpec, lostInputs: [] as InputSpec[] };
+ }
+
+ const oldInputs = task.componentRef.spec?.inputs ?? [];
+ const newInputNames = new Set(
+ (newComponentRef.spec?.inputs ?? []).map((input) => input.name),
+ );
+ const newOutputNames = new Set(
+ (newComponentRef.spec?.outputs ?? []).map((output) => output.name),
+ );
+
+ // Inputs present on the old component but missing from the edited one.
+ const lostInputs = oldInputs.filter(
+ (input) => !newInputNames.has(input.name),
+ );
+
+ task.componentRef = newComponentRef;
+
+ // Drop this task's argument bindings for inputs that were removed.
+ if (task.arguments) {
+ task.arguments = Object.entries(task.arguments).reduce(
+ (acc, [inputName, argument]) => {
+ if (newInputNames.has(inputName)) {
+ acc[inputName] = argument;
+ }
+ return acc;
+ },
+ {} as Record,
+ );
+ }
+
+ // Drop downstream argument bindings that consume an output this component no
+ // longer produces. Outputs that still exist keep working because the taskId
+ // is unchanged.
+ Object.values(updatedGraphSpec.tasks).forEach((downstreamTask) => {
+ if (!downstreamTask.arguments) {
+ return;
+ }
+
+ downstreamTask.arguments = Object.entries(downstreamTask.arguments).reduce(
+ (acc, [inputName, argument]) => {
+ if (
+ typeof argument === "object" &&
+ argument !== null &&
+ "taskOutput" in argument &&
+ argument.taskOutput &&
+ argument.taskOutput.taskId === taskId &&
+ !newOutputNames.has(argument.taskOutput.outputName)
+ ) {
+ return acc;
+ }
+
+ acc[inputName] = argument;
+ return acc;
+ },
+ {} as Record,
+ );
+ });
+
+ return { updatedGraphSpec, lostInputs } as const;
+};
diff --git a/src/components/shared/TaskDetails/Actions.tsx b/src/components/shared/TaskDetails/Actions.tsx
index 88de44bc3..a64abc96b 100644
--- a/src/components/shared/TaskDetails/Actions.tsx
+++ b/src/components/shared/TaskDetails/Actions.tsx
@@ -56,7 +56,7 @@ const TaskActions = ({
/>
);
const editComponent = !readOnly && (
-
+
);
// Canvas Actions
diff --git a/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx b/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx
index 5b6d241b1..c3d1d4a46 100644
--- a/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx
+++ b/src/components/shared/TaskDetails/Actions/EditComponentButton.tsx
@@ -1,5 +1,8 @@
import { useState } from "react";
+import { replaceTaskComponentRef } from "@/components/shared/ReactFlow/FlowCanvas/utils/replaceTaskComponentRef";
+import useToastNotification from "@/hooks/useToastNotification";
+import { useComponentSpec } from "@/providers/ComponentSpecProvider";
import type { HydratedComponentReference } from "@/utils/componentSpec";
import { tracking } from "@/utils/tracking";
@@ -8,12 +11,46 @@ import { ComponentEditorDialog } from "../../ComponentEditor/ComponentEditorDial
interface EditComponentButtonProps {
componentRef: HydratedComponentReference;
+ taskId?: string;
}
export const EditComponentButton = ({
componentRef,
+ taskId,
}: EditComponentButtonProps) => {
const [isEditDialogOpen, setIsEditDialogOpen] = useState(false);
+ const notify = useToastNotification();
+ const { currentGraphSpec, updateGraphSpec } = useComponentSpec();
+
+ const handleComponentSaved = (
+ hydratedComponent: HydratedComponentReference,
+ ) => {
+ if (!taskId || !currentGraphSpec?.tasks[taskId]) {
+ notify(
+ "Could not update the component: the edited task was not found.",
+ "error",
+ );
+ return;
+ }
+
+ const { updatedGraphSpec, lostInputs } = replaceTaskComponentRef(
+ taskId,
+ hydratedComponent,
+ currentGraphSpec,
+ );
+
+ updateGraphSpec(updatedGraphSpec);
+
+ if (lostInputs.length > 0) {
+ const inputNames = lostInputs.map((input) => input.name).join(", ");
+ notify(
+ `Component updated. Removed ${lostInputs.length} input(s) no longer defined: ${inputNames}.`,
+ "warning",
+ );
+ } else {
+ notify("Component updated", "success");
+ }
+ };
return (
<>
@@ -27,6 +64,7 @@ export const EditComponentButton = ({
setIsEditDialogOpen(false)}
+ onComponentSaved={taskId ? handleComponentSaved : undefined}
/>
)}
>