fix: resolve mobile Add folder executor race in workspace sources (#2696)
Co-authored-by: ayattara <alassane.yattara@savoirfairelinux.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -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<typeof getWorkspaceSourceCapabilities>;
|
||||
onAdd: (kind: NonNullable<WorkspaceSourceRow["sourceType"]>) => void;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
if (!capabilities.canAddFolders && capabilities.executorCapabilitiesKnown) return null;
|
||||
return (
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
disabled={!capabilities.canAddFolders}
|
||||
className={cn("cursor-pointer", isMobile ? "min-h-11" : "h-9 px-3")}
|
||||
onClick={() => onAdd("folder")}
|
||||
>
|
||||
<IconFolderPlus className="h-4 w-4" />
|
||||
<span className="flex flex-col items-start">
|
||||
<span>{t("task:addFolder")}</span>
|
||||
{!capabilities.executorCapabilitiesKnown && (
|
||||
<span className="text-xs text-muted-foreground normal-case">
|
||||
{t("task:addFolderExecutorUnknown")}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
</Button>
|
||||
);
|
||||
}
|
||||
+24
-8
@@ -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 (
|
||||
<StateProvider>
|
||||
<HarnessContent makeTurnActive={makeTurnActive} executorType={executorType} />
|
||||
@@ -68,7 +64,7 @@ function HarnessContent({
|
||||
executorType,
|
||||
}: {
|
||||
makeTurnActive: boolean;
|
||||
executorType: string;
|
||||
executorType: string | null | undefined;
|
||||
}) {
|
||||
const [open, setOpen] = useState(false);
|
||||
const [opener, setOpener] = useState<HTMLElement | null>(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(<Harness executorType={executorType} />);
|
||||
|
||||
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(<Harness executorType="local_docker" />);
|
||||
|
||||
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"],
|
||||
|
||||
@@ -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({
|
||||
<div className="space-y-4 py-1" data-testid="add-workspace-sources-form">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<RepositorySourceMenu isMobile={isMobile} onAdd={onAdd} />
|
||||
{capabilities.canAddFolders && (
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
className={cn("cursor-pointer", isMobile ? "min-h-11" : "h-9 px-3")}
|
||||
onClick={() => onAdd("folder")}
|
||||
>
|
||||
<IconFolderPlus className="h-4 w-4" />
|
||||
{t("task:addFolder")}
|
||||
</Button>
|
||||
)}
|
||||
<AddFolderButton isMobile={isMobile} capabilities={capabilities} onAdd={onAdd} />
|
||||
</div>
|
||||
{capabilities.requiresCloneableLocalRepository && (
|
||||
<p className="text-sm text-muted-foreground">
|
||||
|
||||
@@ -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) => {
|
||||
|
||||
@@ -1,5 +1,17 @@
|
||||
import type { ExecutorType } from "@/lib/types/http";
|
||||
|
||||
const KNOWN_EXECUTOR_TYPES = new Set<ExecutorType>([
|
||||
"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),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<string, unknown>) {
|
||||
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<string, unknown>) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<AppState>) => {
|
||||
hydrateState(d, snapshotToState(snapshotWithExecutor()));
|
||||
});
|
||||
|
||||
expect(result.kanban.tasks[0]?.primaryExecutorType).toBe(EXECUTOR_TYPE);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<AppState> {
|
||||
// Handle empty snapshot (ephemeral tasks have no workflow)
|
||||
if (!snapshot.workflow) {
|
||||
@@ -64,6 +73,7 @@ export function snapshotToState(snapshot: WorkflowSnapshot): Partial<AppState> {
|
||||
})),
|
||||
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,
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -24,6 +24,7 @@
|
||||
"addBlocker": "+ Àďď ƀĺōćķēŕ",
|
||||
"addDetailsAboutThisChange": "Àďď ďēţàĩĺś àƀōũţ ţĥĩś ćĥàńĝē...",
|
||||
"addFolder": "Àďď ƒōĺďēŕ",
|
||||
"addFolderExecutorUnknown": "Ŵàĩţĩńĝ ţō ćōńƒĩŕḿ ţĥĩś ţàśķ'ś ēxēćũţōŕ śũƥƥōŕţś ƒōĺďēŕś",
|
||||
"addLabel": "Àďď ĺàƀēĺ",
|
||||
"addLabels": "+ Àďď ĺàƀēĺś",
|
||||
"addMore": "Àďď ḿōŕē",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -24,6 +24,7 @@
|
||||
"addBlocker": "+ 添加阻塞项",
|
||||
"addDetailsAboutThisChange": "添加此变更的详细说明…",
|
||||
"addFolder": "添加文件夹",
|
||||
"addFolderExecutorUnknown": "尚未确认此任务的执行器",
|
||||
"addLabel": "添加标签",
|
||||
"addLabels": "+ 添加标签",
|
||||
"addMore": "添加更多",
|
||||
|
||||
@@ -24,6 +24,7 @@
|
||||
"addBlocker": "+ 新增阻塞項",
|
||||
"addDetailsAboutThisChange": "新增此變更的詳細說明…",
|
||||
"addFolder": "新增資料夾",
|
||||
"addFolderExecutorUnknown": "尚未確認此任務的執行器",
|
||||
"addLabel": "新增標籤",
|
||||
"addLabels": "+ 新增標籤",
|
||||
"addMore": "新增更多",
|
||||
|
||||
@@ -24,6 +24,7 @@
|
||||
"addBlocker": "+ 新增阻塞項",
|
||||
"addDetailsAboutThisChange": "新增此變更的詳細說明…",
|
||||
"addFolder": "新增資料夾",
|
||||
"addFolderExecutorUnknown": "尚未確認此任務的執行器",
|
||||
"addLabel": "新增標籤",
|
||||
"addLabels": "+ 新增標籤",
|
||||
"addMore": "新增更多",
|
||||
|
||||
Reference in New Issue
Block a user