diff --git a/docs/architecture.md b/docs/architecture.md index 1255b21e2..b4bad9af1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -758,6 +758,12 @@ Custom-provider settings routes are registered in `register-custom-provider-rout This API surface is intentionally separate from `projects.nodeId` (runtime host placement metadata) and from task-level routing defaults (`defaultNodeId` / `Task.nodeId`). +Dashboard node onboarding (`AddNodeModal` → `useNodes.register`) uses a two-phase flow: +1. Register node metadata first via `POST /api/nodes`. +2. Persist selected project↔node path mappings with per-project `PUT /api/projects/:id/path-mappings/:nodeId` upserts. + +The client treats mapping persistence as part of onboarding success. If mapping writes fail after node creation, onboarding attempts rollback via `DELETE /api/nodes/:id` and refreshes node state to avoid a silent half-configured node. + ### Node settings sync and update-check endpoints | Method | Path | Description | diff --git a/docs/multi-project.md b/docs/multi-project.md index e750ded8b..cee6c1bcf 100644 --- a/docs/multi-project.md +++ b/docs/multi-project.md @@ -115,6 +115,19 @@ Dashboard and node workflows should use dedicated mapping endpoints rather than These APIs persist/read `projectNodePathMappings` (`projectId` + `nodeId` key). They do **not** assign runtime hosting, and they do **not** change task routing defaults. +### Node onboarding path-capture flow + +When adding a node from the dashboard, onboarding now supports attaching already-registered projects and capturing a node-specific absolute path for each selected project. + +- Step 1: register the node (`POST /api/nodes`) +- Step 2: upsert one `projectNodePathMappings` record per selected project (`PUT /api/projects/:id/path-mappings/:nodeId`) + +This onboarding mapping capture is intentionally separate from: +- `projects.nodeId` (runtime host-node assignment) +- `projects.path` / `ProjectInfo.path` (canonical registered project path) + +So node onboarding records where a given node can access a project on disk, without changing which node hosts the runtime or task-routing defaults. + ### Runtime placement (`projects.nodeId`) `ProjectManager` uses project registration data plus isolation mode to pick runtime type: diff --git a/packages/dashboard/app/__tests__/api-node.test.ts b/packages/dashboard/app/__tests__/api-node.test.ts index f4a85b1b1..21a0856bf 100644 --- a/packages/dashboard/app/__tests__/api-node.test.ts +++ b/packages/dashboard/app/__tests__/api-node.test.ts @@ -10,21 +10,25 @@ import { fetchNodeSettingsSyncStatus, syncNodeAuth, fetchNodeProjectPathMappings, + persistNodeProjectPathMappings, } from "../api-node"; import * as apiModule from "../api"; vi.mock("../api", () => ({ proxyApi: vi.fn(), api: vi.fn(), + upsertProjectPathMapping: vi.fn(), })); const mockProxyApi = vi.mocked(apiModule.proxyApi); const mockApi = vi.mocked(apiModule.api); +const mockUpsertProjectPathMapping = vi.mocked(apiModule.upsertProjectPathMapping); describe("api-node", () => { beforeEach(() => { mockProxyApi.mockReset(); mockApi.mockReset(); + mockUpsertProjectPathMapping.mockReset(); }); describe("fetchRemoteNodeHealth", () => { @@ -231,6 +235,45 @@ describe("api-node", () => { }); }); + describe("persistNodeProjectPathMappings", () => { + it("upserts one mapping per selected project", async () => { + mockUpsertProjectPathMapping + .mockResolvedValueOnce({ projectId: "proj-1", nodeId: "node-1", path: "/node/proj-1", createdAt: "t", updatedAt: "t" }) + .mockResolvedValueOnce({ projectId: "proj-2", nodeId: "node-1", path: "/node/proj-2", createdAt: "t", updatedAt: "t" }); + + const result = await persistNodeProjectPathMappings("node-1", [ + { projectId: "proj-1", path: "/node/proj-1" }, + { projectId: "proj-2", path: "/node/proj-2" }, + ]); + + expect(mockUpsertProjectPathMapping).toHaveBeenNthCalledWith(1, "proj-1", "node-1", "/node/proj-1"); + expect(mockUpsertProjectPathMapping).toHaveBeenNthCalledWith(2, "proj-2", "node-1", "/node/proj-2"); + expect(result).toHaveLength(2); + }); + + it("preserves encoded project ids through delegated helper calls", async () => { + mockUpsertProjectPathMapping.mockResolvedValueOnce({ + projectId: "proj/1+2", + nodeId: "node/1+2", + path: "/path", + createdAt: "t", + updatedAt: "t", + }); + + await persistNodeProjectPathMappings("node/1+2", [{ projectId: "proj/1+2", path: "/path" }]); + + expect(mockUpsertProjectPathMapping).toHaveBeenCalledWith("proj/1+2", "node/1+2", "/path"); + }); + + it("propagates the first upsert failure", async () => { + mockUpsertProjectPathMapping.mockRejectedValueOnce(new Error("mapping failed")); + + await expect( + persistNodeProjectPathMappings("node-1", [{ projectId: "proj-1", path: "/node/proj-1" }]), + ).rejects.toThrow("mapping failed"); + }); + }); + describe("error handling", () => { it("propagates errors from proxyApi", async () => { mockProxyApi.mockRejectedValueOnce(new Error("Network error")); diff --git a/packages/dashboard/app/api-node.ts b/packages/dashboard/app/api-node.ts index 08a7de021..3c566f669 100644 --- a/packages/dashboard/app/api-node.ts +++ b/packages/dashboard/app/api-node.ts @@ -3,9 +3,9 @@ * All functions route through /api/proxy/:nodeId/... when a remote node is targeted. */ -import type { ProjectInfo } from "./api"; +import type { NodeProjectMappingInput, ProjectInfo } from "./api"; import type { ProjectHealth, ProjectNodePathMapping, Task } from "@fusion/core"; -import { api, proxyApi } from "./api"; +import { api, proxyApi, upsertProjectPathMapping } from "./api"; /** Health information for a remote node */ export interface RemoteNodeHealth { @@ -125,3 +125,13 @@ export async function syncNodeAuth(nodeId: string): Promise export async function fetchNodeProjectPathMappings(nodeId: string): Promise { return api(`/nodes/${encodeURIComponent(nodeId)}/path-mappings`); } + +/** Persist one mapping per selected project for a newly-created node. */ +export async function persistNodeProjectPathMappings( + nodeId: string, + projectMappings: NodeProjectMappingInput[], +): Promise { + return Promise.all( + projectMappings.map(({ projectId, path }) => upsertProjectPathMapping(projectId, nodeId, path)), + ); +} diff --git a/packages/dashboard/app/api.ts b/packages/dashboard/app/api.ts index cc2b2c8f2..1a129debf 100644 --- a/packages/dashboard/app/api.ts +++ b/packages/dashboard/app/api.ts @@ -5,3 +5,4 @@ * while implementation lives under `app/api/*` modules. */ export * from "./api/legacy"; +export * from "./api-node"; diff --git a/packages/dashboard/app/api/legacy.ts b/packages/dashboard/app/api/legacy.ts index e1d3b56d5..05c69453c 100644 --- a/packages/dashboard/app/api/legacy.ts +++ b/packages/dashboard/app/api/legacy.ts @@ -5542,6 +5542,22 @@ export interface NodeCreateInput { dockerConfig?: DockerNodeConfigInfo; } +/** Input for assigning a project path for a specific node during onboarding. */ +export interface NodeProjectMappingInput { + projectId: string; + path: string; +} + +/** + * Node onboarding payload used by dashboard UI. + * + * `projectMappings` is intentionally separate from `ProjectInfo.path` and `projects.nodeId`. + * It captures node-specific filesystem paths for selected existing projects. + */ +export interface NodeOnboardingInput extends NodeCreateInput { + projectMappings: NodeProjectMappingInput[]; +} + /** Input for updating an existing node */ export type NodeUpdateInput = Partial> & { status?: NodeStatus; diff --git a/packages/dashboard/app/components/AddNodeModal.css b/packages/dashboard/app/components/AddNodeModal.css index 07cc0043c..0108c0e0b 100644 --- a/packages/dashboard/app/components/AddNodeModal.css +++ b/packages/dashboard/app/components/AddNodeModal.css @@ -149,6 +149,34 @@ gap: var(--space-sm); } +.add-node-modal__projects { + display: flex; + flex-direction: column; + gap: var(--space-sm); +} + +.add-node-modal__projects-title { + margin: 0; + font-size: calc(var(--space-md) + var(--space-xs) * 0.5); +} + +.add-node-modal__project-list { + display: flex; + flex-direction: column; + gap: var(--space-sm); +} + +.add-node-modal__project-card { + padding: var(--space-sm); + display: flex; + flex-direction: column; + gap: var(--space-sm); +} + +.add-node-modal__project-toggle { + margin: 0; +} + @media (max-width: 768px) { .add-node-modal { width: calc(100vw - (var(--space-md) * 2)); @@ -171,4 +199,8 @@ .add-node-modal__row { grid-template-columns: 1fr; } + + .add-node-modal__project-card { + padding: var(--space-md); + } } diff --git a/packages/dashboard/app/components/AddNodeModal.tsx b/packages/dashboard/app/components/AddNodeModal.tsx index 00c194bb8..2215b641e 100644 --- a/packages/dashboard/app/components/AddNodeModal.tsx +++ b/packages/dashboard/app/components/AddNodeModal.tsx @@ -1,4 +1,6 @@ import { useCallback, useEffect, useMemo, useState } from "react"; +import type { NodeProjectMappingInput, ProjectInfo } from "../api"; +import { validateProjectPath } from "../utils/projectDetection"; import type { ToastType } from "../hooks/useToast"; import { useMobileScrollLock } from "../hooks/useMobileScrollLock"; import "./AddNodeModal.css"; @@ -9,6 +11,7 @@ export interface AddNodeInput { url?: string; apiKey?: string; maxConcurrent: number; + projectMappings: NodeProjectMappingInput[]; apiKeyMode?: "auto-generate" | "provide"; extraClis?: Array<"claude-cli" | "droid-cli">; persistentStorage?: boolean; @@ -30,19 +33,21 @@ interface AddNodeModalProps { onClose: () => void; onSubmit: (input: AddNodeInput) => Promise; addToast: (message: string, type?: ToastType) => void; + projects: ProjectInfo[]; } interface FormErrors { name?: string; url?: string; maxConcurrent?: string; + projectMappings: Record; } const MAX_CONCURRENT_MIN = 1; const MAX_CONCURRENT_MAX = 10; function validateInput(input: AddNodeInput): FormErrors { - const errors: FormErrors = {}; + const errors: FormErrors = { projectMappings: {} }; if (!input.name.trim()) { errors.name = "Name is required"; @@ -56,10 +61,17 @@ function validateInput(input: AddNodeInput): FormErrors { errors.maxConcurrent = `Concurrency must be between ${MAX_CONCURRENT_MIN} and ${MAX_CONCURRENT_MAX}`; } + for (const mapping of input.projectMappings) { + const validation = validateProjectPath(mapping.path); + if (!validation.valid) { + errors.projectMappings[mapping.projectId] = validation.error ?? "Path is invalid"; + } + } + return errors; } -export function AddNodeModal({ isOpen, onClose, onSubmit, addToast }: AddNodeModalProps) { +export function AddNodeModal({ isOpen, onClose, onSubmit, addToast, projects }: AddNodeModalProps) { useMobileScrollLock(isOpen); const [name, setName] = useState(""); const [type, setType] = useState<"local" | "remote">("local"); @@ -67,7 +79,8 @@ export function AddNodeModal({ isOpen, onClose, onSubmit, addToast }: AddNodeMod const [apiKey, setApiKey] = useState(""); const [maxConcurrent, setMaxConcurrent] = useState(2); const [apiKeyMode, setApiKeyMode] = useState<"auto-generate" | "provide">("auto-generate"); - const [errors, setErrors] = useState({}); + const [selectedProjectPaths, setSelectedProjectPaths] = useState>({}); + const [errors, setErrors] = useState({ projectMappings: {} }); const [isSubmitting, setIsSubmitting] = useState(false); const resetForm = useCallback(() => { @@ -77,7 +90,8 @@ export function AddNodeModal({ isOpen, onClose, onSubmit, addToast }: AddNodeMod setApiKey(""); setMaxConcurrent(2); setApiKeyMode("auto-generate"); - setErrors({}); + setSelectedProjectPaths({}); + setErrors({ projectMappings: {} }); setIsSubmitting(false); }, []); @@ -113,7 +127,8 @@ export function AddNodeModal({ isOpen, onClose, onSubmit, addToast }: AddNodeMod apiKey: type === "remote" && apiKeyMode === "provide" ? apiKey || undefined : undefined, maxConcurrent, apiKeyMode, - }), [apiKey, apiKeyMode, maxConcurrent, name, type, url]); + projectMappings: Object.entries(selectedProjectPaths).map(([projectId, path]) => ({ projectId, path: path.trim() })), + }), [apiKey, apiKeyMode, maxConcurrent, name, selectedProjectPaths, type, url]); const handleSubmit = useCallback(async () => { if (isSubmitting) return; @@ -121,7 +136,12 @@ export function AddNodeModal({ isOpen, onClose, onSubmit, addToast }: AddNodeMod const validationErrors = validateInput(input); setErrors(validationErrors); - if (Object.keys(validationErrors).length > 0) { + if ( + validationErrors.name + || validationErrors.url + || validationErrors.maxConcurrent + || Object.keys(validationErrors.projectMappings).length > 0 + ) { return; } @@ -139,6 +159,20 @@ export function AddNodeModal({ isOpen, onClose, onSubmit, addToast }: AddNodeMod } }, [addToast, closeModal, input, isSubmitting, onSubmit]); + const toggleProjectSelection = (project: ProjectInfo) => { + setSelectedProjectPaths((current) => { + if (project.id in current) { + const { [project.id]: _removed, ...remaining } = current; + return remaining; + } + return { ...current, [project.id]: project.path }; + }); + }; + + const updateProjectPath = (projectId: string, path: string) => { + setSelectedProjectPaths((current) => ({ ...current, [projectId]: path })); + }; + if (!isOpen) return null; return ( @@ -253,6 +287,49 @@ export function AddNodeModal({ isOpen, onClose, onSubmit, addToast }: AddNodeMod {errors.maxConcurrent && {errors.maxConcurrent}} +
+

Attach Existing Projects

+

Select existing projects to run on this node and provide the node-specific absolute path for each one.

+ {projects.length === 0 ? ( +

No projects are currently registered.

+ ) : ( +
+ {projects.map((project) => { + const selected = project.id in selectedProjectPaths; + const error = errors.projectMappings[project.id]; + return ( +
+ + {selected && ( + + )} +
+ ); + })} +
+ )} +
+
diff --git a/packages/dashboard/app/components/NodesView.tsx b/packages/dashboard/app/components/NodesView.tsx index 93b47826f..c8c30e122 100644 --- a/packages/dashboard/app/components/NodesView.tsx +++ b/packages/dashboard/app/components/NodesView.tsx @@ -32,7 +32,7 @@ export function NodesView({ addToast, onClose }: NodesViewProps) { patchDockerConfig, fetchDockerDiff, } = useNodes(); - const { projects } = useProjects(); + const { projects, refresh: refreshProjects } = useProjects(); const { syncStatusMap, pushSettings, pullSettings, syncAuth, trackNode, getAuthSyncState, getAuthProviders } = useNodeSettingsSync(); const { dockerNodes, @@ -74,7 +74,8 @@ export function NodesView({ addToast, onClose }: NodesViewProps) { const handleRegister = useCallback(async (input: AddNodeInput) => { await register(input); - }, [register]); + await refreshProjects(); + }, [refreshProjects, register]); const handleCreateDockerNode = useCallback(async (input: ManagedDockerNodeInput) => { try { @@ -249,6 +250,7 @@ export function NodesView({ addToast, onClose }: NodesViewProps) { onClose={() => setAddModalOpen(false)} onSubmit={handleRegister} addToast={addToast} + projects={projects} /> { const defaultProps = { @@ -9,6 +8,24 @@ describe("AddNodeModal", () => { onClose: vi.fn(), onSubmit: vi.fn().mockResolvedValue(undefined), addToast: vi.fn(), + projects: [ + { + id: "proj-1", + name: "Project One", + path: "/workspace/project-one", + status: "active" as const, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }, + { + id: "proj-2", + name: "Project Two", + path: "/workspace/project-two", + status: "active" as const, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }, + ], }; beforeEach(() => { @@ -92,6 +109,7 @@ describe("AddNodeModal", () => { url: undefined, apiKey: undefined, maxConcurrent: 2, + projectMappings: [], })); expect(defaultProps.addToast).toHaveBeenCalledWith('Node "Test Node" registered', "success"); expect(defaultProps.onClose).toHaveBeenCalled(); @@ -249,7 +267,74 @@ describe("AddNodeModal", () => { url: "https://node.example.com", apiKey: "secret-key", maxConcurrent: 2, + projectMappings: [], })); }); }); + + it("validates selected project path is required", async () => { + render(); + + fireEvent.change(screen.getByPlaceholderText("Build Machine"), { + target: { value: "Node With Project" }, + }); + fireEvent.click(screen.getByRole("checkbox", { name: "Project One" })); + fireEvent.change(screen.getByDisplayValue("/workspace/project-one"), { + target: { value: "" }, + }); + + fireEvent.click(screen.getByRole("button", { name: "Add Node" })); + + expect(await screen.findByText("Path is required")).toBeInTheDocument(); + expect(defaultProps.onSubmit).not.toHaveBeenCalled(); + }); + + it("validates selected project path is absolute", async () => { + render(); + + fireEvent.change(screen.getByPlaceholderText("Build Machine"), { + target: { value: "Node With Project" }, + }); + fireEvent.click(screen.getByRole("checkbox", { name: "Project One" })); + fireEvent.change(screen.getByDisplayValue("/workspace/project-one"), { + target: { value: "relative/path" }, + }); + + fireEvent.click(screen.getByRole("button", { name: "Add Node" })); + + expect(await screen.findByText("Path must be absolute")).toBeInTheDocument(); + expect(defaultProps.onSubmit).not.toHaveBeenCalled(); + }); + + it("submits selected project mappings", async () => { + render(); + + fireEvent.change(screen.getByPlaceholderText("Build Machine"), { + target: { value: "Node With Project" }, + }); + fireEvent.click(screen.getByRole("checkbox", { name: "Project One" })); + fireEvent.change(screen.getByDisplayValue("/workspace/project-one"), { + target: { value: "/mnt/node/project-one" }, + }); + + fireEvent.click(screen.getByRole("button", { name: "Add Node" })); + + await waitFor(() => { + expect(defaultProps.onSubmit).toHaveBeenCalledWith(expect.objectContaining({ + name: "Node With Project", + projectMappings: [{ projectId: "proj-1", path: "/mnt/node/project-one" }], + })); + }); + }); + + it("removes path input when project is deselected", () => { + render(); + + const checkbox = screen.getByRole("checkbox", { name: "Project One" }); + fireEvent.click(checkbox); + expect(screen.getByDisplayValue("/workspace/project-one")).toBeInTheDocument(); + + fireEvent.click(checkbox); + expect(screen.queryByDisplayValue("/workspace/project-one")).not.toBeInTheDocument(); + }); }); diff --git a/packages/dashboard/app/components/__tests__/NodesView.test.tsx b/packages/dashboard/app/components/__tests__/NodesView.test.tsx index 0f7c73d03..15c07a7be 100644 --- a/packages/dashboard/app/components/__tests__/NodesView.test.tsx +++ b/packages/dashboard/app/components/__tests__/NodesView.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, fireEvent } from "@testing-library/react"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; import { NodesView } from "../NodesView"; import type { NodeInfo, ProjectInfo } from "../../api"; import { useNodes } from "../../hooks/useNodes"; @@ -217,6 +217,35 @@ describe("NodesView", () => { expect(screen.getByRole("dialog", { name: "Add Node" })).toBeDefined(); }); + it("refreshes projects after node registration succeeds", async () => { + const register = vi.fn().mockResolvedValue(makeNode({ id: "node-new", name: "New Node" })); + const refreshProjects = vi.fn().mockResolvedValue(undefined); + mockUseNodes.mockReturnValue(makeUseNodesResult({ nodes: [], register })); + mockUseProjects.mockReturnValue({ + projects: [makeProject()], + loading: false, + error: null, + refresh: refreshProjects, + register: vi.fn(), + update: vi.fn(), + unregister: vi.fn(), + }); + + render(); + + fireEvent.click(screen.getByText("Add Node")); + fireEvent.change(screen.getByPlaceholderText("Build Machine"), { target: { value: "New Node" } }); + fireEvent.click(screen.getByTestId("add-node-submit")); + + await waitFor(() => { + expect(register).toHaveBeenCalledWith(expect.objectContaining({ + name: "New Node", + projectMappings: [], + })); + expect(refreshProjects).toHaveBeenCalledTimes(1); + }); + }); + it("opens Node Detail modal when a node card is clicked", () => { mockUseProjects.mockReturnValue({ projects: [makeProject({ nodeId: "node-1" })], diff --git a/packages/dashboard/app/hooks/__tests__/useNodes.test.ts b/packages/dashboard/app/hooks/__tests__/useNodes.test.ts index a63af7ade..82223efb5 100644 --- a/packages/dashboard/app/hooks/__tests__/useNodes.test.ts +++ b/packages/dashboard/app/hooks/__tests__/useNodes.test.ts @@ -2,7 +2,8 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { renderHook, act } from "@testing-library/react"; import { useNodes } from "../useNodes"; import * as api from "../../api"; -import type { NodeInfo, NodeCreateInput } from "../../api"; +import * as nodeApi from "../../api-node"; +import type { NodeInfo, NodeOnboardingInput } from "../../api"; vi.mock("../../api", () => ({ fetchNodes: vi.fn(), @@ -12,11 +13,16 @@ vi.mock("../../api", () => ({ checkNodeHealth: vi.fn(), })); +vi.mock("../../api-node", () => ({ + persistNodeProjectPathMappings: vi.fn(), +})); + const mockFetchNodes = vi.mocked(api.fetchNodes); const mockRegisterNode = vi.mocked(api.registerNode); const mockUpdateNode = vi.mocked(api.updateNode); const mockUnregisterNode = vi.mocked(api.unregisterNode); const mockCheckNodeHealth = vi.mocked(api.checkNodeHealth); +const mockPersistNodeProjectPathMappings = vi.mocked(nodeApi.persistNodeProjectPathMappings); function makeNode(overrides: Partial = {}): NodeInfo { return { @@ -45,6 +51,7 @@ describe("useNodes", () => { mockUpdateNode.mockReset(); mockUnregisterNode.mockReset(); mockCheckNodeHealth.mockReset(); + mockPersistNodeProjectPathMappings.mockReset(); }); afterEach(() => { @@ -78,9 +85,14 @@ describe("useNodes", () => { expect(result.current.error).toBe("boom"); }); - it("register adds node optimistically", async () => { - mockFetchNodes.mockResolvedValueOnce([]); - const nodeInput: NodeCreateInput = { name: "Remote Node", type: "remote", url: "https://node.test" }; + it("register creates node and persists selected path mappings", async () => { + mockFetchNodes.mockResolvedValueOnce([]).mockResolvedValueOnce([]); + const nodeInput: NodeOnboardingInput = { + name: "Remote Node", + type: "remote", + url: "https://node.test", + projectMappings: [{ projectId: "proj-1", path: "/mnt/proj-1" }], + }; const createdNode = makeNode({ id: "node_remote", name: "Remote Node", @@ -89,6 +101,7 @@ describe("useNodes", () => { status: "connecting", }); mockRegisterNode.mockResolvedValueOnce(createdNode); + mockPersistNodeProjectPathMappings.mockResolvedValueOnce([]); const { result } = renderHook(() => useNodes()); @@ -100,9 +113,84 @@ describe("useNodes", () => { await result.current.register(nodeInput); }); - expect(mockRegisterNode).toHaveBeenCalledWith(nodeInput); - expect(result.current.nodes).toHaveLength(1); - expect(result.current.nodes[0].id).toBe("node_remote"); + expect(mockRegisterNode).toHaveBeenCalledWith({ + name: "Remote Node", + type: "remote", + url: "https://node.test", + }); + expect(mockPersistNodeProjectPathMappings).toHaveBeenCalledWith("node_remote", nodeInput.projectMappings); + expect(mockFetchNodes).toHaveBeenCalledTimes(2); + }); + + it("register rolls back node when mapping write fails", async () => { + mockFetchNodes.mockResolvedValueOnce([]).mockResolvedValueOnce([]); + const createdNode = makeNode({ id: "node_remote", type: "remote", status: "connecting" }); + mockRegisterNode.mockResolvedValueOnce(createdNode); + mockPersistNodeProjectPathMappings.mockRejectedValueOnce(new Error("mapping failed")); + mockUnregisterNode.mockResolvedValueOnce(undefined); + + const { result } = renderHook(() => useNodes()); + + await act(async () => { + await flushPromises(); + }); + + await expect(result.current.register({ + name: "Remote Node", + type: "remote", + url: "https://node.test", + projectMappings: [{ projectId: "proj-1", path: "/mnt/proj-1" }], + })).rejects.toThrow("mapping failed"); + + expect(mockUnregisterNode).toHaveBeenCalledWith("node_remote"); + expect(mockFetchNodes).toHaveBeenCalledTimes(2); + }); + + it("register appends cleanup failure message when rollback also fails", async () => { + mockFetchNodes.mockResolvedValueOnce([]).mockResolvedValueOnce([]); + const createdNode = makeNode({ id: "node_remote", type: "remote", status: "connecting" }); + mockRegisterNode.mockResolvedValueOnce(createdNode); + mockPersistNodeProjectPathMappings.mockRejectedValueOnce(new Error("mapping failed")); + mockUnregisterNode.mockRejectedValueOnce(new Error("cleanup failed")); + + const { result } = renderHook(() => useNodes()); + + await act(async () => { + await flushPromises(); + }); + + await expect(result.current.register({ + name: "Remote Node", + type: "remote", + url: "https://node.test", + projectMappings: [{ projectId: "proj-1", path: "/mnt/proj-1" }], + })).rejects.toThrow("mapping failed. Cleanup also failed: cleanup failed"); + + expect(mockUnregisterNode).toHaveBeenCalledWith("node_remote"); + }); + + it("does not rollback when no mappings are selected even if post-success refresh fails", async () => { + mockFetchNodes.mockResolvedValueOnce([]).mockRejectedValueOnce(new Error("refresh failed")); + const createdNode = makeNode({ id: "node_remote", type: "remote", status: "connecting" }); + mockRegisterNode.mockResolvedValueOnce(createdNode); + + const { result } = renderHook(() => useNodes()); + + await act(async () => { + await flushPromises(); + }); + + await act(async () => { + await expect(result.current.register({ + name: "Remote Node", + type: "remote", + url: "https://node.test", + projectMappings: [], + })).resolves.toEqual(createdNode); + }); + + expect(mockPersistNodeProjectPathMappings).not.toHaveBeenCalled(); + expect(mockUnregisterNode).not.toHaveBeenCalled(); }); it("update modifies node optimistically", async () => { diff --git a/packages/dashboard/app/hooks/useNodes.ts b/packages/dashboard/app/hooks/useNodes.ts index 9bc361ae9..417c1e0e9 100644 --- a/packages/dashboard/app/hooks/useNodes.ts +++ b/packages/dashboard/app/hooks/useNodes.ts @@ -1,5 +1,5 @@ import { useState, useEffect, useCallback, useRef } from "react"; -import type { DockerNodeConfigInfo, NodeCreateInput, NodeInfo, NodeUpdateInput } from "../api"; +import type { DockerNodeConfigInfo, NodeCreateInput, NodeInfo, NodeOnboardingInput, NodeUpdateInput } from "../api"; import { fetchDockerConfigDiff, fetchDockerNodeConfig, @@ -10,13 +10,14 @@ import { unregisterNode, checkNodeHealth, } from "../api"; +import { persistNodeProjectPathMappings } from "../api-node"; export interface UseNodesResult { nodes: NodeInfo[]; loading: boolean; error: string | null; refresh: () => Promise; - register: (input: NodeCreateInput) => Promise; + register: (input: NodeOnboardingInput) => Promise; update: (id: string, updates: NodeUpdateInput) => Promise; unregister: (id: string) => Promise; healthCheck: (id: string) => Promise; @@ -112,11 +113,36 @@ export function useNodes(): UseNodesResult { }; }, [loading, refresh]); - const register = useCallback(async (input: NodeCreateInput): Promise => { - const node = await registerNode(input); - setNodes((prev) => [...prev, node]); + const register = useCallback(async (input: NodeOnboardingInput): Promise => { + const { projectMappings, ...nodeInput } = input; + const node = await registerNode(nodeInput as NodeCreateInput); + + if (projectMappings.length > 0) { + try { + await persistNodeProjectPathMappings(node.id, projectMappings); + } catch (error) { + const mappingError = error instanceof Error ? error.message : "Failed to persist project mappings"; + let cleanupErrorMessage = ""; + try { + await unregisterNode(node.id); + } catch (cleanupError) { + cleanupErrorMessage = cleanupError instanceof Error + ? cleanupError.message + : "Failed to unregister node after mapping failure"; + } + + await refresh(); + + if (cleanupErrorMessage) { + throw new Error(`${mappingError}. Cleanup also failed: ${cleanupErrorMessage}`); + } + throw new Error(mappingError); + } + } + + await refresh(); return node; - }, []); + }, [refresh]); const update = useCallback(async (id: string, updates: NodeUpdateInput): Promise => { const node = await updateNode(id, updates);