fix(web): preserve repo metadata in kanban.update and check legacy repositoryId (#1258)
This commit is contained in:
@@ -75,6 +75,10 @@ import type { Terminal } from "@/hooks/domains/session/use-terminals";
|
||||
import { setPanelTitle } from "@/lib/layout/panel-portal-manager";
|
||||
import { PanelPortalHost, usePortalSlot } from "@/lib/layout/panel-portal-host";
|
||||
import { ENV_SCOPED_DOCKVIEW_COMPONENTS } from "@/lib/state/dockview-env-scoped-components";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import { createDebugLogger, isDebug } from "@/lib/debug/log";
|
||||
|
||||
const debugChangesVisibility = createDebugLogger("changes:visibility");
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// PORTAL SLOT — generic dockview component that adopts a persistent portal
|
||||
@@ -301,11 +305,36 @@ function DiffViewerContent({
|
||||
);
|
||||
}
|
||||
|
||||
function describeTaskRepositoriesForDebug(state: AppState, taskId: string | null) {
|
||||
if (!taskId) return { source: "none", repositoryId: "-", repoCount: -1, repoIds: "-" };
|
||||
const task = state.kanban.tasks.find((item) => item.id === taskId);
|
||||
if (task) {
|
||||
return {
|
||||
source: "kanban",
|
||||
repositoryId: task.repositoryId ?? "-",
|
||||
repoCount: task.repositories?.length ?? -1,
|
||||
repoIds: task.repositories?.map((repo) => repo.repository_id).join(",") || "-",
|
||||
};
|
||||
}
|
||||
for (const [workflowId, snapshot] of Object.entries(state.kanbanMulti.snapshots)) {
|
||||
const snapshotTask = snapshot.tasks.find((item) => item.id === taskId);
|
||||
if (!snapshotTask) continue;
|
||||
return {
|
||||
source: `kanbanMulti:${workflowId}`,
|
||||
repositoryId: snapshotTask.repositoryId ?? "-",
|
||||
repoCount: snapshotTask.repositories?.length ?? -1,
|
||||
repoIds: snapshotTask.repositories?.map((repo) => repo.repository_id).join(",") || "-",
|
||||
};
|
||||
}
|
||||
return { source: "missing", repositoryId: "-", repoCount: -1, repoIds: "-" };
|
||||
}
|
||||
|
||||
function ChangesContent({ panelId }: { panelId: string }) {
|
||||
const addDiffViewerPanel = useDockviewStore((s) => s.addDiffViewerPanel);
|
||||
const addFileDiffPanel = useDockviewStore((s) => s.addFileDiffPanel);
|
||||
const addCommitDetailPanel = useDockviewStore((s) => s.addCommitDetailPanel);
|
||||
const { openFile } = useFileEditors();
|
||||
const appStore = useAppStoreApi();
|
||||
|
||||
// Dynamic title with file count — use environment-stable sessionId so the
|
||||
// tab title doesn't re-fetch on same-environment session tab switches.
|
||||
@@ -318,11 +347,32 @@ function ChangesContent({ panelId }: { panelId: string }) {
|
||||
// that window is unrecoverable in the same session.
|
||||
const taskHasRepos = useActiveTaskHasRepos();
|
||||
useEffect(() => {
|
||||
if (taskHasRepos !== false) return;
|
||||
const dockApi = useDockviewStore.getState().api;
|
||||
const panel = dockApi?.getPanel(panelId);
|
||||
if (isDebug()) {
|
||||
const state = appStore.getState();
|
||||
const activeTaskId = state.tasks.activeTaskId;
|
||||
const repoDebug = describeTaskRepositoriesForDebug(state, activeTaskId);
|
||||
let action = "keep";
|
||||
if (taskHasRepos === false) {
|
||||
action = panel ? "remove" : "remove-missing-panel";
|
||||
}
|
||||
debugChangesVisibility("auto-close decision", {
|
||||
panelId,
|
||||
taskHasRepos: taskHasRepos === null ? "unknown" : String(taskHasRepos),
|
||||
action,
|
||||
activeTaskId: activeTaskId ?? "-",
|
||||
sessionId: state.tasks.activeSessionId ?? "-",
|
||||
taskSource: repoDebug.source,
|
||||
repositoryId: repoDebug.repositoryId,
|
||||
repoCount: repoDebug.repoCount,
|
||||
repoIds: repoDebug.repoIds,
|
||||
livePanelIds: dockApi?.panels.map((p) => p.id).join(",") ?? "-",
|
||||
});
|
||||
}
|
||||
if (taskHasRepos !== false) return;
|
||||
if (dockApi && panel) dockApi.removePanel(panel);
|
||||
}, [taskHasRepos, panelId]);
|
||||
}, [taskHasRepos, panelId, appStore]);
|
||||
|
||||
useEffect(() => {
|
||||
const title = totalCount > 0 ? `Changes (${totalCount})` : "Changes";
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { taskHasRepositories } from "./use-active-task-has-repos";
|
||||
|
||||
describe("taskHasRepositories", () => {
|
||||
it("treats legacy repositoryId as a repo association", () => {
|
||||
expect(taskHasRepositories({ repositoryId: "repo-a" })).toBe(true);
|
||||
});
|
||||
|
||||
it("treats a non-empty repositories array as a repo association", () => {
|
||||
expect(
|
||||
taskHasRepositories({
|
||||
repositories: [
|
||||
{
|
||||
id: "link-a",
|
||||
repository_id: "repo-a",
|
||||
base_branch: "main",
|
||||
position: 0,
|
||||
},
|
||||
],
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false when both repository fields are absent", () => {
|
||||
expect(taskHasRepositories({})).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for an empty repositories array with no repositoryId", () => {
|
||||
expect(taskHasRepositories({ repositories: [] })).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for null input", () => {
|
||||
expect(taskHasRepositories(null)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,13 @@
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import type { KanbanState } from "@/lib/state/slices";
|
||||
import { useTask } from "@/hooks/use-task";
|
||||
|
||||
type RepoBearingTask = Pick<KanbanState["tasks"][number], "repositories" | "repositoryId">;
|
||||
|
||||
export function taskHasRepositories(task: RepoBearingTask | null | undefined): boolean {
|
||||
return Boolean(task?.repositoryId || (task?.repositories && task.repositories.length > 0));
|
||||
}
|
||||
|
||||
/**
|
||||
* Repo-status of the active task, distinguishing three states:
|
||||
* - `null` — unknown (no active task, or task hasn't loaded into the
|
||||
@@ -17,5 +24,5 @@ export function useActiveTaskHasRepos(): boolean | null {
|
||||
const taskId = useAppStore((s) => s.tasks.activeTaskId);
|
||||
const task = useTask(taskId);
|
||||
if (!task) return null;
|
||||
return Boolean(task.repositories && task.repositories.length > 0);
|
||||
return taskHasRepositories(task);
|
||||
}
|
||||
|
||||
@@ -48,6 +48,9 @@
|
||||
* when adding new loggers — it's the cheat-sheet for triage):
|
||||
*
|
||||
* Git / Changes panel pipeline (bug: stale until refresh)
|
||||
* [changes:visibility] Changes tab auto-close/keep decision:
|
||||
* taskHasRepos, source task object, repositoryId,
|
||||
* repoCount, repoIds, and live Dockview panel IDs.
|
||||
* [git-status:subscribe] useSessionGitStatus subscribe/unsubscribe cycle
|
||||
* [ws:dispatch] every WS message — inbound notifications and
|
||||
* outbound sends (`message="send"`). Streaming
|
||||
|
||||
@@ -3,6 +3,16 @@ import type { StoreApi } from "zustand";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import { registerKanbanHandlers } from "./kanban";
|
||||
|
||||
const WORKFLOW_ID = "wf1";
|
||||
const TASK_ID = "t1";
|
||||
const STEP_ID = "s1";
|
||||
const TASK_TITLE = "T1";
|
||||
const UPDATED_TITLE = "T1 updated";
|
||||
const REPO_A_ID = "repo-a";
|
||||
const REPO_B_ID = "repo-b";
|
||||
const REPO_LINK_A_ID = "link-a";
|
||||
const FEATURE_BRANCH = "feature/x";
|
||||
|
||||
function makeStore(initial: Partial<AppState> = {}) {
|
||||
let state = {
|
||||
kanban: { workflowId: null, steps: [], tasks: [] },
|
||||
@@ -36,13 +46,13 @@ describe("kanban.update handler — primarySessionId preservation", () => {
|
||||
it("preserves primarySessionId from existing tasks", () => {
|
||||
const store = makeStore({
|
||||
kanban: {
|
||||
workflowId: "wf1",
|
||||
workflowId: WORKFLOW_ID,
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: "t1",
|
||||
workflowStepId: "s1",
|
||||
title: "T1",
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: TASK_TITLE,
|
||||
position: 0,
|
||||
primarySessionId: "sess-primary",
|
||||
},
|
||||
@@ -52,26 +62,32 @@ describe("kanban.update handler — primarySessionId preservation", () => {
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [
|
||||
{ id: "t1", workflowStepId: "s1", title: "T1 updated", position: 0, state: "IN_PROGRESS" },
|
||||
makeUpdateMessage(WORKFLOW_ID, [
|
||||
{
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: UPDATED_TITLE,
|
||||
position: 0,
|
||||
state: "IN_PROGRESS",
|
||||
},
|
||||
]),
|
||||
);
|
||||
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === "t1");
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === TASK_ID);
|
||||
expect(task?.primarySessionId).toBe("sess-primary");
|
||||
expect(task?.title).toBe("T1 updated");
|
||||
expect(task?.title).toBe(UPDATED_TITLE);
|
||||
});
|
||||
|
||||
it("preserves primarySessionState from existing tasks", () => {
|
||||
const store = makeStore({
|
||||
kanban: {
|
||||
workflowId: "wf1",
|
||||
workflowId: WORKFLOW_ID,
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: "t1",
|
||||
workflowStepId: "s1",
|
||||
title: "T1",
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: TASK_TITLE,
|
||||
position: 0,
|
||||
primarySessionId: "sess-primary",
|
||||
primarySessionState: "RUNNING",
|
||||
@@ -82,22 +98,24 @@ describe("kanban.update handler — primarySessionId preservation", () => {
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [{ id: "t1", workflowStepId: "s1", title: "T1", position: 0 }]),
|
||||
makeUpdateMessage(WORKFLOW_ID, [
|
||||
{ id: TASK_ID, workflowStepId: STEP_ID, title: TASK_TITLE, position: 0 },
|
||||
]),
|
||||
);
|
||||
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === "t1");
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === TASK_ID);
|
||||
expect(task?.primarySessionState).toBe("RUNNING");
|
||||
});
|
||||
|
||||
it("new tasks start with undefined primarySessionId", () => {
|
||||
const store = makeStore({
|
||||
kanban: { workflowId: "wf1", steps: [], tasks: [] },
|
||||
kanban: { workflowId: WORKFLOW_ID, steps: [], tasks: [] },
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [
|
||||
{ id: "t-new", workflowStepId: "s1", title: "New Task", position: 0 },
|
||||
makeUpdateMessage(WORKFLOW_ID, [
|
||||
{ id: "t-new", workflowStepId: STEP_ID, title: "New Task", position: 0 },
|
||||
]),
|
||||
);
|
||||
|
||||
@@ -107,6 +125,129 @@ describe("kanban.update handler — primarySessionId preservation", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("kanban.update handler — repository preservation", () => {
|
||||
it("preserves existing repositories when kanban.update omits repo metadata", () => {
|
||||
const store = makeStore({
|
||||
kanban: {
|
||||
workflowId: WORKFLOW_ID,
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: TASK_TITLE,
|
||||
position: 0,
|
||||
repositoryId: REPO_A_ID,
|
||||
repositories: [
|
||||
{
|
||||
id: REPO_LINK_A_ID,
|
||||
repository_id: REPO_A_ID,
|
||||
base_branch: "main",
|
||||
checkout_branch: FEATURE_BRANCH,
|
||||
position: 0,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage(WORKFLOW_ID, [
|
||||
{
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: UPDATED_TITLE,
|
||||
position: 0,
|
||||
state: "CREATED",
|
||||
},
|
||||
]),
|
||||
);
|
||||
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === TASK_ID);
|
||||
expect(task?.repositoryId).toBe(REPO_A_ID);
|
||||
expect(task?.repositories).toEqual([
|
||||
{
|
||||
id: REPO_LINK_A_ID,
|
||||
repository_id: REPO_A_ID,
|
||||
base_branch: "main",
|
||||
checkout_branch: FEATURE_BRANCH,
|
||||
position: 0,
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("kanban.update handler — repository switch", () => {
|
||||
it("does not restore stale snapshot repositories when repository_id changes", () => {
|
||||
const repoA = [
|
||||
{
|
||||
id: REPO_LINK_A_ID,
|
||||
repository_id: REPO_A_ID,
|
||||
base_branch: "main",
|
||||
checkout_branch: FEATURE_BRANCH,
|
||||
position: 0,
|
||||
},
|
||||
];
|
||||
const store = makeStore({
|
||||
kanban: {
|
||||
workflowId: WORKFLOW_ID,
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: TASK_TITLE,
|
||||
position: 0,
|
||||
repositoryId: REPO_A_ID,
|
||||
repositories: repoA,
|
||||
},
|
||||
],
|
||||
},
|
||||
kanbanMulti: {
|
||||
isLoading: false,
|
||||
snapshots: {
|
||||
[WORKFLOW_ID]: {
|
||||
workflowId: WORKFLOW_ID,
|
||||
workflowName: "WF1",
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: TASK_TITLE,
|
||||
position: 0,
|
||||
repositoryId: REPO_A_ID,
|
||||
repositories: repoA,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage(WORKFLOW_ID, [
|
||||
{
|
||||
id: TASK_ID,
|
||||
workflowStepId: STEP_ID,
|
||||
title: UPDATED_TITLE,
|
||||
position: 0,
|
||||
repository_id: REPO_B_ID,
|
||||
},
|
||||
]),
|
||||
);
|
||||
|
||||
const snapshotTask = store
|
||||
.getState()
|
||||
.kanbanMulti.snapshots[WORKFLOW_ID]?.tasks.find((t) => t.id === TASK_ID);
|
||||
expect(snapshotTask?.repositoryId).toBe(REPO_B_ID);
|
||||
expect(snapshotTask?.repositories).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("kanban.update handler — explicit-null primary preservation", () => {
|
||||
it("does not restore stale snapshot value when primarySessionId is explicitly cleared", () => {
|
||||
// Repro for the multi-snapshot null-preservation bug: when task.updated
|
||||
|
||||
@@ -6,6 +6,35 @@ import type { KanbanState } from "@/lib/state/slices/kanban/types";
|
||||
type KanbanTask = KanbanState["tasks"][number];
|
||||
type KanbanStep = KanbanState["steps"][number];
|
||||
|
||||
type KanbanUpdateTask = {
|
||||
id: string;
|
||||
workflowStepId: string;
|
||||
title: string;
|
||||
description?: string;
|
||||
position?: number;
|
||||
state?: KanbanTask["state"];
|
||||
repository_id?: string;
|
||||
repositories?: KanbanTask["repositories"];
|
||||
is_ephemeral?: boolean;
|
||||
};
|
||||
|
||||
function resolveRepositories(
|
||||
task: KanbanUpdateTask,
|
||||
existing: KanbanTask | undefined,
|
||||
): KanbanTask["repositories"] {
|
||||
if (task.repositories !== undefined) return task.repositories;
|
||||
if (task.repository_id && task.repository_id !== existing?.repositoryId) return undefined;
|
||||
return existing?.repositories;
|
||||
}
|
||||
|
||||
function resolveRepositoryId(
|
||||
task: KanbanUpdateTask,
|
||||
repositories: KanbanTask["repositories"],
|
||||
existing: KanbanTask | undefined,
|
||||
): string | undefined {
|
||||
return task.repository_id ?? repositories?.[0]?.repository_id ?? existing?.repositoryId;
|
||||
}
|
||||
|
||||
export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
|
||||
return {
|
||||
"kanban.update": (message) => {
|
||||
@@ -28,11 +57,10 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
|
||||
const existingById = new Map(state.kanban.tasks.map((t) => [t.id, t]));
|
||||
const tasks: KanbanTask[] = message.payload.tasks
|
||||
// Filter out ephemeral tasks (e.g., quick chat)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
.filter((task: any) => !task.is_ephemeral)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
.map((task: any) => {
|
||||
.filter((task: KanbanUpdateTask) => !task.is_ephemeral)
|
||||
.map((task: KanbanUpdateTask) => {
|
||||
const existing = existingById.get(task.id);
|
||||
const repositories = resolveRepositories(task, existing);
|
||||
return {
|
||||
id: task.id,
|
||||
workflowStepId: task.workflowStepId,
|
||||
@@ -40,6 +68,8 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
|
||||
description: task.description,
|
||||
position: task.position ?? 0,
|
||||
state: task.state,
|
||||
repositoryId: resolveRepositoryId(task, repositories, existing),
|
||||
repositories,
|
||||
primarySessionId: existing?.primarySessionId,
|
||||
primarySessionState: existing?.primarySessionState,
|
||||
};
|
||||
@@ -56,12 +86,20 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
|
||||
const existingMultiById = new Map(snapshot.tasks.map((t) => [t.id, t]));
|
||||
const multiTasks = tasks.map((t) => {
|
||||
const fallback = existingMultiById.get(t.id);
|
||||
const repositoryId =
|
||||
t.repositoryId === undefined ? fallback?.repositoryId : t.repositoryId;
|
||||
const repositories =
|
||||
t.repositories !== undefined || repositoryId !== fallback?.repositoryId
|
||||
? t.repositories
|
||||
: fallback?.repositories;
|
||||
// Fall back to the multi-snapshot's own value only when the main
|
||||
// kanban lookup returned `undefined` (task absent from kanban.tasks).
|
||||
// An explicit `null` means the primary was intentionally cleared
|
||||
// and must NOT be replaced by a stale snapshot value.
|
||||
return {
|
||||
...t,
|
||||
repositoryId,
|
||||
repositories,
|
||||
primarySessionId:
|
||||
t.primarySessionId === undefined ? fallback?.primarySessionId : t.primarySessionId,
|
||||
primarySessionState:
|
||||
|
||||
Reference in New Issue
Block a user