From faed669371bf5111f47e8b0be6b4db8c4a11af91 Mon Sep 17 00:00:00 2001 From: Alassane <79718216+yattdev@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:30:45 +0000 Subject: [PATCH] fix: resolve mobile Add folder executor race in workspace sources (#2696) Co-authored-by: ayattara Co-authored-by: Claude Sonnet 5 Co-authored-by: Carlos Florencio --- .../add-folder-button.tsx | 38 ++++ .../add-workspace-sources-dialog.test.tsx | 32 ++- .../add-workspace-sources-dialog.tsx | 14 +- .../executor-capabilities.test.ts | 31 +++ .../executor-capabilities.ts | 19 ++ apps/web/e2e/helpers/api-client.ts | 1 + .../tests/task/add-workspace-sources.spec.ts | 13 ++ .../task/mobile-add-workspace-sources.spec.ts | 24 ++- .../kanban/use-all-workflow-snapshots.test.ts | 185 ++++++++++++++++-- .../kanban/use-all-workflow-snapshots.ts | 21 +- apps/web/lib/ssr/mapper.test.ts | 87 ++++++++ apps/web/lib/ssr/mapper.ts | 10 + apps/web/src/locales/en/task.json | 1 + apps/web/src/locales/pseudo/task.json | 1 + apps/web/src/locales/pt-pt/task.json | 1 + apps/web/src/locales/zh-cn/task.json | 1 + apps/web/src/locales/zh-hk/task.json | 1 + apps/web/src/locales/zh-tw/task.json | 1 + 18 files changed, 430 insertions(+), 51 deletions(-) create mode 100644 apps/web/components/task/add-workspace-sources/add-folder-button.tsx diff --git a/apps/web/components/task/add-workspace-sources/add-folder-button.tsx b/apps/web/components/task/add-workspace-sources/add-folder-button.tsx new file mode 100644 index 0000000000..02837322cf --- /dev/null +++ b/apps/web/components/task/add-workspace-sources/add-folder-button.tsx @@ -0,0 +1,38 @@ +import { IconFolderPlus } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { useTranslation } from "react-i18next"; +import { cn } from "@/lib/utils"; +import { getWorkspaceSourceCapabilities } from "@/components/workspace-source-picker/executor-capabilities"; +import { type WorkspaceSourceRow } from "@/components/workspace-source-picker/workspace-source-state"; + +export function AddFolderButton({ + isMobile, + capabilities, + onAdd, +}: { + isMobile: boolean; + capabilities: ReturnType; + onAdd: (kind: NonNullable) => void; +}) { + const { t } = useTranslation(); + if (!capabilities.canAddFolders && capabilities.executorCapabilitiesKnown) return null; + return ( + + ); +} diff --git a/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.test.tsx b/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.test.tsx index 033f44f5ba..b383177ac1 100644 --- a/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.test.tsx +++ b/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.test.tsx @@ -49,13 +49,9 @@ function openRepositoryMenu() { return trigger; } -function Harness({ - makeTurnActive = false, - executorType = "worktree", -}: { - makeTurnActive?: boolean; - executorType?: string; -}) { +function Harness(props: { makeTurnActive?: boolean; executorType?: string | null }) { + const makeTurnActive = props.makeTurnActive ?? false; + const executorType = Object.hasOwn(props, "executorType") ? props.executorType : "worktree"; return ( @@ -68,7 +64,7 @@ function HarnessContent({ executorType, }: { makeTurnActive: boolean; - executorType: string; + executorType: string | null | undefined; }) { const [open, setOpen] = useState(false); const [opener, setOpener] = useState(null); @@ -199,6 +195,26 @@ describe("AddWorkspaceSourcesDialog", () => { expect(form.querySelectorAll('[role="alert"]')).toHaveLength(2); }); + it.each([null, undefined])( + "shows Add folder disabled with a touch-visible reason while the executor is unresolved (%s)", + async (executorType) => { + render(); + + fireEvent.click(screen.getByRole("button", { name: ADD_SOURCES_LABEL })); + const addFolder = screen.getByRole("button", { name: /Add folder/ }) as HTMLButtonElement; + expect(addFolder.disabled).toBe(true); + const hint = screen.getByText("Waiting to confirm this task's executor supports folders"); + expect(hint.className).toContain("text-xs"); + }, + ); + + it("keeps Add folder absent once the executor is known not to support folders", async () => { + render(); + + fireEvent.click(screen.getByRole("button", { name: ADD_SOURCES_LABEL })); + expect(screen.queryByRole("button", { name: /Add folder/ })).toBeNull(); + }); + it.each([ ["desktop", false, "add-workspace-sources-dialog"], ["mobile", true, "add-workspace-sources-drawer"], diff --git a/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.tsx b/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.tsx index 3470e8e796..d37535e16f 100644 --- a/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.tsx +++ b/apps/web/components/task/add-workspace-sources/add-workspace-sources-dialog.tsx @@ -4,7 +4,6 @@ import { useCallback, useEffect, useState, type RefObject, type ReactNode } from import { IconChevronDown, IconCloudDownload, - IconFolderPlus, IconGitBranch, IconPlus, IconStack2, @@ -47,6 +46,7 @@ import { getWorkspaceSourceCapabilities, hasCloneableSavedRepository, } from "@/components/workspace-source-picker/executor-capabilities"; +import { AddFolderButton } from "./add-folder-button"; import { SavedRepositorySourceRow } from "./saved-repository-source-row"; import { useDialogOpenerFocus } from "./use-dialog-opener-focus"; import { useSubmitWorkspaceSources } from "./use-submit-workspace-sources"; @@ -318,17 +318,7 @@ function SourceForm({
- {capabilities.canAddFolders && ( - - )} +
{capabilities.requiresCloneableLocalRepository && (

diff --git a/apps/web/components/workspace-source-picker/executor-capabilities.test.ts b/apps/web/components/workspace-source-picker/executor-capabilities.test.ts index 531af287e4..c74c63cc4e 100644 --- a/apps/web/components/workspace-source-picker/executor-capabilities.test.ts +++ b/apps/web/components/workspace-source-picker/executor-capabilities.test.ts @@ -17,6 +17,37 @@ describe("getWorkspaceSourceCapabilities", () => { }, ); + it.each([ + "local", + "local_pc", + "worktree", + "local_docker", + "remote_docker", + "ssh", + "sprites", + "remote_vps", + "k8s", + ] as const)("marks executor capabilities as known once resolved for %s", (executorType) => { + expect(getWorkspaceSourceCapabilities(executorType).executorCapabilitiesKnown).toBe(true); + }); + + it.each([null, undefined])( + "marks executor capabilities as unknown, not unsupported, for %s", + (executorType) => { + expect(getWorkspaceSourceCapabilities(executorType)).toMatchObject({ + canAddFolders: false, + executorCapabilitiesKnown: false, + }); + }, + ); + + it("keeps an unrecognized executor type unresolved", () => { + expect(getWorkspaceSourceCapabilities("future_executor")).toMatchObject({ + canAddFolders: false, + executorCapabilitiesKnown: false, + }); + }); + it.each(["local", "local_pc"] as const)( "uses the live checkout and does not offer a checkout branch for %s", (executorType) => { diff --git a/apps/web/components/workspace-source-picker/executor-capabilities.ts b/apps/web/components/workspace-source-picker/executor-capabilities.ts index 4dfa0b2799..d416b3abf5 100644 --- a/apps/web/components/workspace-source-picker/executor-capabilities.ts +++ b/apps/web/components/workspace-source-picker/executor-capabilities.ts @@ -1,5 +1,17 @@ import type { ExecutorType } from "@/lib/types/http"; +const KNOWN_EXECUTOR_TYPES = new Set([ + "local", + "local_pc", + "local_docker", + "sprites", + "ssh", + "remote_docker", + "remote_vps", + "k8s", + "worktree", +]); + type SavedRepositoryIdentity = { remote_url?: string; provider?: string; @@ -15,6 +27,12 @@ export function getWorkspaceSourceCapabilities( canAddFolders: boolean; canChooseCheckoutBranch: boolean; requiresCloneableLocalRepository: boolean; + /** + * False while the primary executor binding has not resolved yet or the type + * is unknown. Callers must not treat that the same as a known executor that + * cannot host folders — an unresolved capability means "not yet known," not "no." + */ + executorCapabilitiesKnown: boolean; } { const usesLiveCheckout = executorType === "local" || executorType === "local_pc"; const requiresClone = @@ -26,6 +44,7 @@ export function getWorkspaceSourceCapabilities( canAddFolders: usesLiveCheckout || executorType === "worktree", canChooseCheckoutBranch: !usesLiveCheckout, requiresCloneableLocalRepository: requiresClone, + executorCapabilitiesKnown: KNOWN_EXECUTOR_TYPES.has(executorType as ExecutorType), }; } diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 2758d1364a..d6f8f9505b 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -2202,6 +2202,7 @@ export class ApiClient { title: string; autopilot?: boolean; primary_session_id?: string | null; + primary_executor_type?: string | null; state?: string; workflow_step_id?: string; parent_id?: string; diff --git a/apps/web/e2e/tests/task/add-workspace-sources.spec.ts b/apps/web/e2e/tests/task/add-workspace-sources.spec.ts index 32b9079bc2..a73f8809a7 100644 --- a/apps/web/e2e/tests/task/add-workspace-sources.spec.ts +++ b/apps/web/e2e/tests/task/add-workspace-sources.spec.ts @@ -104,6 +104,19 @@ test.describe("Attach local workspace sources", () => { if (!task.session_id) throw new Error("task creation did not return a session id"); await waitForWorkspaceReady(apiClient, task.id, task.session_id); + + // The "Add folder" control is gated on the task's primary executor + // binding. Poll the backend directly for it rather than relying on the + // later "Add folder" click landing after it incidentally. + // `primary_executor_type` is `omitempty`, so an unbound task omits the key + // entirely: assert truthiness, never `not.toBeNull()`, which `undefined` + // satisfies on the first poll. + await expect + .poll(async () => (await apiClient.getTask(task.id)).primary_executor_type, { + timeout: 30_000, + }) + .toBeTruthy(); + await testPage.goto(`/t/${task.id}`); const session = new SessionPage(testPage); await session.waitForLoad(); diff --git a/apps/web/e2e/tests/task/mobile-add-workspace-sources.spec.ts b/apps/web/e2e/tests/task/mobile-add-workspace-sources.spec.ts index fa2fc3b3ac..23277522c0 100644 --- a/apps/web/e2e/tests/task/mobile-add-workspace-sources.spec.ts +++ b/apps/web/e2e/tests/task/mobile-add-workspace-sources.spec.ts @@ -56,6 +56,19 @@ test("mobile Files drawer attaches sources with fixed controls and persisted wor }, ); + // The "Add folder" control is gated on the task's primary executor binding. + // Poll the backend directly for it rather than budgeting the later UI + // assertion for the async session launch that produces it. The budget sits on + // the backend cause, not on a UI shadow of it, so the "Add folder" assertion + // below keeps the default timeout. `primary_executor_type` is `omitempty`, so + // an unbound task omits the key entirely: assert truthiness, never + // `not.toBeNull()`, which `undefined` satisfies on the first poll. + await expect + .poll(async () => (await apiClient.getTask(task.id)).primary_executor_type, { + timeout: 30_000, + }) + .toBeTruthy(); + await testPage.goto(`/t/${task.id}`); const session = new SessionPage(testPage); await session.waitForLoad(); @@ -147,14 +160,11 @@ test("mobile Files drawer attaches sources with fixed controls and persisted wor const addRepository = drawer.getByRole("button", { name: "Add repository" }); const addFolder = drawer.getByRole("button", { name: "Add folder" }); const submit = drawer.getByTestId("add-workspace-sources-submit"); - // The "Add folder" control is gated on activeTask.primaryExecutorType, which - // arrives from a follow-up office.task.updated WS event once the primary - // session's executor is bound and can lag the drawer mount on a loaded CI - // shard. The drawer re-renders reactively when that field hydrates, so wait - // for the button with a generous timeout instead of measuring a not-yet- - // rendered button (a null box that otherwise stalls until the suite timeout). + // The "Add folder" control is gated on activeTask.primaryExecutorType. The + // poll before navigation already confirmed the backend has resolved it, so + // the drawer renders it on first paint and this needs no extended budget. await expect(addRepository).toBeVisible(); - await expect(addFolder).toBeVisible({ timeout: 30_000 }); + await expect(addFolder).toBeVisible(); const [addRepositoryBox, addFolderBox] = await Promise.all([ addRepository.boundingBox(), addFolder.boundingBox(), diff --git a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts index 83bee49a54..420eb7ea32 100644 --- a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts +++ b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts @@ -222,6 +222,25 @@ describe("useAllWorkflowSnapshots — fetch guards", () => { }); }); +const WORKTREE_EXECUTOR_FIELDS = { + primaryExecutorId: "exec-worktree", + primaryExecutorType: "worktree", + primaryExecutorName: "Worktree", + isRemoteExecutor: false, +}; + +function seedCachedTask(taskOverrides: Record) { + mockState.kanbanMulti.snapshots = { + "wf-A": { + workflowId: "wf-A", + workflowName: "A", + isPlaceholder: true, + steps: [], + tasks: [{ id: "task-1", workflowStepId: "step-1", ...taskOverrides }], + }, + }; +} + describe("useAllWorkflowSnapshots — snapshot mapping", () => { beforeEach(() => { resetMocks([{ id: "wf-A", workspaceId: "ws-A", name: "A" }]); @@ -255,15 +274,7 @@ describe("useAllWorkflowSnapshots — snapshot mapping", () => { }); it("preserves a cached autopilot marker when a fresh snapshot omits it", async () => { - mockState.kanbanMulti.snapshots = { - "wf-A": { - workflowId: "wf-A", - workflowName: "A", - isPlaceholder: true, - steps: [], - tasks: [{ id: "task-1", workflowStepId: "step-1", autopilot: true }], - }, - }; + seedCachedTask({ autopilot: true }); mockFetchWorkflowSnapshot.mockResolvedValueOnce({ steps: [{ id: "step-1", name: "Review", position: 1 }], tasks: [{ id: "task-1", workflow_step_id: "step-1", title: "Task" }], @@ -276,15 +287,7 @@ describe("useAllWorkflowSnapshots — snapshot mapping", () => { }); it("keeps an explicit false autopilot value from the fresh snapshot", async () => { - mockState.kanbanMulti.snapshots = { - "wf-A": { - workflowId: "wf-A", - workflowName: "A", - isPlaceholder: true, - steps: [], - tasks: [{ id: "task-1", workflowStepId: "step-1", autopilot: true }], - }, - }; + seedCachedTask({ autopilot: true }); mockFetchWorkflowSnapshot.mockResolvedValueOnce({ steps: [{ id: "step-1", name: "Review", position: 1 }], tasks: [{ id: "task-1", workflow_step_id: "step-1", title: "Task", autopilot: false }], @@ -297,6 +300,152 @@ describe("useAllWorkflowSnapshots — snapshot mapping", () => { }); }); +describe("useAllWorkflowSnapshots — executor binding merge", () => { + beforeEach(() => { + resetMocks([{ id: "wf-A", workspaceId: "ws-A", name: "A" }]); + }); + + it("preserves a newer live executor binding when a delayed snapshot omits it", async () => { + seedCachedTask({}); + let resolveSnapshot: (value: { steps: unknown[]; tasks: unknown[] }) => void = () => {}; + mockFetchWorkflowSnapshot.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSnapshot = resolve; + }), + ); + + renderHook(() => useAllWorkflowSnapshots("ws-A")); + await waitFor(() => expect(mockFetchWorkflowSnapshot).toHaveBeenCalled()); + + // A task.updated event adds the executor while the snapshot is in flight. + mockState.kanbanMulti.snapshots = { + "wf-A": { + workflowId: "wf-A", + workflowName: "A", + isPlaceholder: true, + steps: [], + tasks: [{ id: "task-1", workflowStepId: "step-1", ...WORKTREE_EXECUTOR_FIELDS }], + }, + }; + resolveSnapshot({ + steps: [{ id: "step-1", name: "Review", position: 1 }], + tasks: [{ id: "task-1", workflow_step_id: "step-1", title: "Task" }], + }); + + await waitFor(() => expect(mockSetWorkflowSnapshot).toHaveBeenCalled()); + expect(mockSetWorkflowSnapshot.mock.calls[0][1].tasks[0]).toMatchObject( + WORKTREE_EXECUTOR_FIELDS, + ); + }); + + it("clears a cached executor when a current snapshot omits the primary session", async () => { + seedCachedTask({ primarySessionId: "session-1", ...WORKTREE_EXECUTOR_FIELDS }); + mockFetchWorkflowSnapshot.mockResolvedValueOnce({ + steps: [{ id: "step-1", name: "Review", position: 1 }], + // The backend omits primary_session_id and primary_executor_* when the + // task has no primary session because those DTO fields use omitempty. + tasks: [{ id: "task-1", workflow_step_id: "step-1", title: "Task" }], + }); + + renderHook(() => useAllWorkflowSnapshots("ws-A")); + + await waitFor(() => expect(mockSetWorkflowSnapshot).toHaveBeenCalled()); + expect(mockSetWorkflowSnapshot.mock.calls[0][1].tasks[0].primaryExecutorType).toBeUndefined(); + }); + + it("preserves each omitted executor field from a newer live binding independently", async () => { + seedCachedTask({}); + let resolveSnapshot: (value: { steps: unknown[]; tasks: unknown[] }) => void = () => {}; + mockFetchWorkflowSnapshot.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSnapshot = resolve; + }), + ); + + renderHook(() => useAllWorkflowSnapshots("ws-A")); + await waitFor(() => expect(mockFetchWorkflowSnapshot).toHaveBeenCalled()); + + mockState.kanbanMulti.snapshots = { + "wf-A": { + workflowId: "wf-A", + workflowName: "A", + isPlaceholder: true, + steps: [], + tasks: [{ id: "task-1", workflowStepId: "step-1", ...WORKTREE_EXECUTOR_FIELDS }], + }, + }; + resolveSnapshot({ + steps: [{ id: "step-1", name: "Review", position: 1 }], + tasks: [ + { + id: "task-1", + workflow_step_id: "step-1", + title: "Task", + primary_executor_type: "worktree", + }, + ], + }); + + await waitFor(() => expect(mockSetWorkflowSnapshot).toHaveBeenCalled()); + expect(mockSetWorkflowSnapshot.mock.calls[0][1].tasks[0]).toMatchObject( + WORKTREE_EXECUTOR_FIELDS, + ); + }); +}); + +describe("useAllWorkflowSnapshots — executor binding mapping", () => { + beforeEach(() => { + resetMocks([{ id: "wf-A", workspaceId: "ws-A", name: "A" }]); + }); + + it("clears a cached executor binding when the response explicitly clears the primary session", async () => { + seedCachedTask({ primarySessionId: "session-1", ...WORKTREE_EXECUTOR_FIELDS }); + mockFetchWorkflowSnapshot.mockResolvedValueOnce({ + steps: [{ id: "step-1", name: "Review", position: 1 }], + tasks: [ + { + id: "task-1", + workflow_step_id: "step-1", + title: "Task", + primary_session_id: null, + }, + ], + }); + + renderHook(() => useAllWorkflowSnapshots("ws-A")); + + await waitFor(() => expect(mockSetWorkflowSnapshot).toHaveBeenCalled()); + expect(mockSetWorkflowSnapshot.mock.calls[0][1].tasks[0].primaryExecutorType).toBeUndefined(); + }); + + it("adopts a fresh primary executor binding when the response includes one", async () => { + seedCachedTask({}); + mockFetchWorkflowSnapshot.mockResolvedValueOnce({ + steps: [{ id: "step-1", name: "Review", position: 1 }], + tasks: [ + { + id: "task-1", + workflow_step_id: "step-1", + title: "Task", + primary_executor_id: "exec-worktree", + primary_executor_type: "worktree", + primary_executor_name: "Worktree", + is_remote_executor: false, + }, + ], + }); + + renderHook(() => useAllWorkflowSnapshots("ws-A")); + + await waitFor(() => expect(mockSetWorkflowSnapshot).toHaveBeenCalled()); + expect(mockSetWorkflowSnapshot.mock.calls[0][1].tasks[0]).toMatchObject( + WORKTREE_EXECUTOR_FIELDS, + ); + }); +}); + const REMOTE_EXECUTOR_NAME = "Remote box"; function seedExecutor(executor: Record) { diff --git a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts index 6c2e46cae2..cc2f66fcef 100644 --- a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts +++ b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts @@ -55,10 +55,23 @@ function hasNewerLiveExecutor(existing: KanbanTask, fetchStart: KanbanTask | und ); } +function preserveLiveExecutorBinding( + merged: KanbanTask, + existing: KanbanTask, + fetchStart: KanbanTask | undefined, + source: Task, +): void { + // An explicit primary-session clear is authoritative, even if a newer live + // update changed the cached executor while this snapshot was in flight. + if (source.primary_session_id === null || !hasNewerLiveExecutor(existing, fetchStart)) return; + copyPrimaryExecutorFields(merged, existing); +} + function mergeFetchedTask( mapped: KanbanTask, existing: KanbanTask, fetchStart: KanbanTask | undefined, + source: Task, statusSummaryInvalidated: boolean, ): KanbanTask { // Production snapshot payloads can be surfaced through read-only objects. @@ -100,12 +113,7 @@ function mergeFetchedTask( if (merged.autoStartFailed === undefined || hasNewerLiveAutoStartFailed(existing, fetchStart)) { merged.autoStartFailed = existing.autoStartFailed; } - // An omitted executor bundle in a current full snapshot represents a - // legitimate detach. Only copy live fields when a live update changed the - // executor after this request began, or when the task appeared in flight. - if (hasNewerLiveExecutor(existing, fetchStart)) { - copyPrimaryExecutorFields(merged, existing); - } + preserveLiveExecutorBinding(merged, existing, fetchStart, source); return merged; } @@ -132,6 +140,7 @@ function mergeSnapshotTasks( mapped, existing, fetchStartById.get(mapped.id), + task, task.status_summary_invalidated === true, ) : mapped; diff --git a/apps/web/lib/ssr/mapper.test.ts b/apps/web/lib/ssr/mapper.test.ts index c127f28a45..f7514013d8 100644 --- a/apps/web/lib/ssr/mapper.test.ts +++ b/apps/web/lib/ssr/mapper.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest"; +import { produce } from "immer"; +import type { Draft } from "immer"; import { snapshotToState } from "./mapper"; +import { hydrateState } from "@/lib/state/hydration/hydrator"; +import { defaultState } from "@/lib/state/default-state"; +import type { AppState } from "@/lib/state/store"; import { taskId, workflowId, workspaceId } from "@/lib/types/ids"; import type { WorkflowSnapshot } from "@/lib/types/http"; import { partitionWipTasks } from "@/lib/kanban/wip-queue"; @@ -47,6 +52,18 @@ function snapshotWithPendingAction(action: unknown): WorkflowSnapshot { }; } +const EXECUTOR_ID = "exec-worktree"; +const EXECUTOR_TYPE = "worktree"; +const EXECUTOR_NAME = "Worktree"; + +function snapshotWithExecutor(): WorkflowSnapshot { + const snapshot = snapshotWithPendingAction(undefined); + snapshot.tasks[0].primary_executor_id = EXECUTOR_ID; + snapshot.tasks[0].primary_executor_type = EXECUTOR_TYPE; + snapshot.tasks[0].primary_executor_name = EXECUTOR_NAME; + return snapshot; +} + // eslint-disable-next-line max-lines-per-function -- test describe block, splitting hurts readability describe("snapshotToState", () => { it("keeps known primary session pending action values", () => { @@ -115,6 +132,45 @@ describe("snapshotToState", () => { expect(state.kanban?.tasks[0]?.statusSummary).toEqual(snapshot.tasks[0].status_summary); }); + it("maps the primary executor binding onto the kanban task", () => { + const snapshot = snapshotWithExecutor(); + snapshot.tasks[0].is_remote_executor = false; + + const state = snapshotToState(snapshot); + + expect(state.kanban?.tasks[0]).toMatchObject({ + primaryExecutorId: EXECUTOR_ID, + primaryExecutorType: EXECUTOR_TYPE, + primaryExecutorName: EXECUTOR_NAME, + isRemoteExecutor: false, + }); + }); + + it("leaves the executor binding unset when the snapshot omits it", () => { + // The backend marks every executor field `omitempty`, so a task whose + // primary session has not bound an executor yet arrives with the keys + // absent rather than null. The mapper must yield undefined here, never + // crash and never invent a binding. + const state = snapshotToState(snapshotWithPendingAction(undefined)); + + expect(state.kanban?.tasks[0]?.primaryExecutorId).toBeUndefined(); + expect(state.kanban?.tasks[0]?.primaryExecutorType).toBeUndefined(); + expect(state.kanban?.tasks[0]?.primaryExecutorName).toBeUndefined(); + expect(state.kanban?.tasks[0]?.isRemoteExecutor).toBe(false); + }); + + it("leaves an explicitly undefined executor binding unset", () => { + // Reviewer-requested wire-contract coverage. JSON omits undefined values, + // but typed snapshot fixtures can retain the property; both representations + // must produce the same unresolved client state. + const snapshot = snapshotWithPendingAction(undefined); + snapshot.tasks[0].primary_executor_type = undefined; + + const state = snapshotToState(snapshot); + + expect(state.kanban?.tasks[0]?.primaryExecutorType).toBeUndefined(); + }); + it("preserves workflow step WIP fields", () => { const state = snapshotToState({ workflow: { @@ -182,3 +238,34 @@ describe("snapshotToState", () => { ]); }); }); + +describe("snapshotToState feeding hydrateState", () => { + it("does not blank an already-known executor when a re-hydration lands", () => { + // Regression guard for the bug this change fixes, exercised through the + // real chain: use-workflow-snapshot calls hydrate(snapshotToState(...)) on + // (re)mount, and mergeKanbanTasks replaces the cached task wholesale when + // the incoming updatedAt is not older. While snapshotToState dropped the + // executor fields, that replacement silently reset primaryExecutorType to + // undefined and the "Add folder" control disappeared. + const draft = structuredClone(defaultState) as AppState; + draft.kanban.tasks = [ + { + id: taskId("task-1"), + workflowId: workflowID, + workflowStepId: "step-1", + title: "Task", + position: 0, + updatedAt: now, + primaryExecutorId: EXECUTOR_ID, + primaryExecutorType: EXECUTOR_TYPE, + primaryExecutorName: EXECUTOR_NAME, + }, + ] as AppState["kanban"]["tasks"]; + + const result = produce(draft, (d: Draft) => { + hydrateState(d, snapshotToState(snapshotWithExecutor())); + }); + + expect(result.kanban.tasks[0]?.primaryExecutorType).toBe(EXECUTOR_TYPE); + }); +}); diff --git a/apps/web/lib/ssr/mapper.ts b/apps/web/lib/ssr/mapper.ts index 9ea55940c9..e0dbc256e8 100644 --- a/apps/web/lib/ssr/mapper.ts +++ b/apps/web/lib/ssr/mapper.ts @@ -16,6 +16,15 @@ function resolveAutoStartFailed(task: WorkflowSnapshot["tasks"][number]): boolea return task.auto_start_failed ?? false; } +function primaryExecutorFields(task: Task) { + return { + primaryExecutorId: task.primary_executor_id ?? undefined, + primaryExecutorType: task.primary_executor_type ?? undefined, + primaryExecutorName: task.primary_executor_name ?? undefined, + isRemoteExecutor: task.is_remote_executor ?? false, + }; +} + export function snapshotToState(snapshot: WorkflowSnapshot): Partial { // Handle empty snapshot (ephemeral tasks have no workflow) if (!snapshot.workflow) { @@ -64,6 +73,7 @@ export function snapshotToState(snapshot: WorkflowSnapshot): Partial { })), primarySessionId: task.primary_session_id ?? undefined, primarySessionState: task.primary_session_state ?? undefined, + ...primaryExecutorFields(task), primarySessionPendingAction: pickPendingAction(task.primary_session_pending_action), taskPendingAction: pickPendingAction(task.task_pending_action), foregroundActivity: task.foreground_activity ?? undefined, diff --git a/apps/web/src/locales/en/task.json b/apps/web/src/locales/en/task.json index cf4f7df1be..cd46eeb993 100644 --- a/apps/web/src/locales/en/task.json +++ b/apps/web/src/locales/en/task.json @@ -24,6 +24,7 @@ "addBlocker": "+ Add blocker", "addDetailsAboutThisChange": "Add details about this change...", "addFolder": "Add folder", + "addFolderExecutorUnknown": "Waiting to confirm this task's executor supports folders", "addLabel": "Add label", "addLabels": "+ Add labels", "addMore": "Add more", diff --git a/apps/web/src/locales/pseudo/task.json b/apps/web/src/locales/pseudo/task.json index 5e7a88a267..995ed9dc92 100644 --- a/apps/web/src/locales/pseudo/task.json +++ b/apps/web/src/locales/pseudo/task.json @@ -24,6 +24,7 @@ "addBlocker": "+ Àďď ƀĺōćķēŕ", "addDetailsAboutThisChange": "Àďď ďēţàĩĺś àƀōũţ ţĥĩś ćĥàńĝē...", "addFolder": "Àďď ƒōĺďēŕ", + "addFolderExecutorUnknown": "Ŵàĩţĩńĝ ţō ćōńƒĩŕḿ ţĥĩś ţàśķ'ś ēxēćũţōŕ śũƥƥōŕţś ƒōĺďēŕś", "addLabel": "Àďď ĺàƀēĺ", "addLabels": "+ Àďď ĺàƀēĺś", "addMore": "Àďď ḿōŕē", diff --git a/apps/web/src/locales/pt-pt/task.json b/apps/web/src/locales/pt-pt/task.json index b31545bca5..ea69c720aa 100644 --- a/apps/web/src/locales/pt-pt/task.json +++ b/apps/web/src/locales/pt-pt/task.json @@ -24,6 +24,7 @@ "addBlocker": "+ Adicionar bloqueio", "addDetailsAboutThisChange": "Adicione detalhes sobre esta alteração...", "addFolder": "Adicionar pasta", + "addFolderExecutorUnknown": "O executor desta tarefa ainda não foi confirmado", "addLabel": "Adicionar etiqueta", "addLabels": "+ Adicionar etiquetas", "addMore": "Adicionar mais", diff --git a/apps/web/src/locales/zh-cn/task.json b/apps/web/src/locales/zh-cn/task.json index 2e71a3e174..e65918827e 100644 --- a/apps/web/src/locales/zh-cn/task.json +++ b/apps/web/src/locales/zh-cn/task.json @@ -24,6 +24,7 @@ "addBlocker": "+ 添加阻塞项", "addDetailsAboutThisChange": "添加此变更的详细说明…", "addFolder": "添加文件夹", + "addFolderExecutorUnknown": "尚未确认此任务的执行器", "addLabel": "添加标签", "addLabels": "+ 添加标签", "addMore": "添加更多", diff --git a/apps/web/src/locales/zh-hk/task.json b/apps/web/src/locales/zh-hk/task.json index bca0a5bef5..62e9d514ac 100644 --- a/apps/web/src/locales/zh-hk/task.json +++ b/apps/web/src/locales/zh-hk/task.json @@ -24,6 +24,7 @@ "addBlocker": "+ 新增阻塞項", "addDetailsAboutThisChange": "新增此變更的詳細說明…", "addFolder": "新增資料夾", + "addFolderExecutorUnknown": "尚未確認此任務的執行器", "addLabel": "新增標籤", "addLabels": "+ 新增標籤", "addMore": "新增更多", diff --git a/apps/web/src/locales/zh-tw/task.json b/apps/web/src/locales/zh-tw/task.json index 02afa725f2..8ebdc7de52 100644 --- a/apps/web/src/locales/zh-tw/task.json +++ b/apps/web/src/locales/zh-tw/task.json @@ -24,6 +24,7 @@ "addBlocker": "+ 新增阻塞項", "addDetailsAboutThisChange": "新增此變更的詳細說明…", "addFolder": "新增資料夾", + "addFolderExecutorUnknown": "尚未確認此任務的執行器", "addLabel": "新增標籤", "addLabels": "+ 新增標籤", "addMore": "新增更多",