fix(web): reset stale topbar branch when navigating between tasks (#546)

This commit is contained in:
Carlos Florêncio
2026-04-06 12:52:42 +01:00
committed by GitHub
parent 221e7fea46
commit 6d81734c66
5 changed files with 223 additions and 28 deletions
@@ -174,7 +174,9 @@ function BranchPillTrigger({
return (
<div className="group flex items-center gap-1.5 rounded-md px-2 h-7 bg-muted/40 hover:bg-muted/60 cursor-pointer transition-colors min-w-0 max-w-full">
<IconGitBranch className="h-3.5 w-3.5 text-muted-foreground shrink-0" />
<span className="text-xs text-muted-foreground truncate">{displayBranch}</span>
<span data-testid="topbar-branch-name" className="text-xs text-muted-foreground truncate">
{displayBranch}
</span>
{canRename && (
<button
type="button"
+1 -1
View File
@@ -190,7 +190,7 @@ export const backendFixture = base.extend<object, { backend: BackendContext }>({
KANDEV_MOCK_AGENT: "only",
KANDEV_MOCK_GITHUB: "true",
KANDEV_DOCKER_ENABLED: "false",
KANDEV_WORKTREE_ENABLED: "false",
KANDEV_WORKTREE_ENABLED: "true",
KANDEV_WORKTREE_BASEPATH: worktreeBase,
KANDEV_REPOCLONE_BASEPATH: repoCloneBase,
KANDEV_LOG_LEVEL: process.env.KANDEV_LOG_LEVEL ?? "warn",
+11
View File
@@ -14,6 +14,8 @@ export type SeedData = {
steps: WorkflowStep[];
repositoryId: string;
agentProfileId: string;
/** Executor profile ID for the worktree executor — use to create tasks with git worktree isolation. */
worktreeExecutorProfileId: string;
};
export const test = backendFixture.extend<
@@ -64,6 +66,14 @@ export const test = backendFixture.extend<
throw new Error("E2E seed failed: no agent profile available");
}
// Find the worktree executor's profile so tests can opt in to worktree-based sessions.
const { executors } = await apiClient.listExecutors();
const worktreeExec = executors.find((e) => e.type === "worktree");
const worktreeExecutorProfileId = worktreeExec?.profiles?.[0]?.id;
if (!worktreeExecutorProfileId) {
throw new Error("E2E seed failed: no worktree executor profile available");
}
await use({
workspaceId: workspace.id,
workflowId: workflow.id,
@@ -71,6 +81,7 @@ export const test = backendFixture.extend<
steps: sorted,
repositoryId: repo.id,
agentProfileId,
worktreeExecutorProfileId,
});
},
{ scope: "worker" },
@@ -0,0 +1,112 @@
import { test, expect } from "../../fixtures/test-base";
import { KanbanPage } from "../../pages/kanban-page";
import { SessionPage } from "../../pages/session-page";
const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"];
/**
* Regression test: navigating from a task with a worktree branch to a new
* sessionless task must not display the previous task's branch in the topbar.
*
* Root cause: useSessionResumption kept stale local state (worktreeBranch)
* from the previous session. On navigation, a race between async
* checkAndResume calls for the old and new sessions could leave the old
* branch value persisted in the topbar.
*/
test.describe("Stale topbar branch", () => {
test("navigating from a task with a branch to a sessionless task does not show stale branch", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(120_000);
// The branch pill is hidden below 1352px container width (@max-[1352px]/topbar:hidden).
await testPage.setViewportSize({ width: 1600, height: 900 });
// 1. Create Task A with worktree executor so it gets a real worktree branch
const taskA = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Task With Branch",
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
executor_profile_id: seedData.worktreeExecutorProfileId,
},
);
// 2. Wait for Task A's session to finish and capture its worktree branch
let taskABranch: string | null = null;
await expect
.poll(
async () => {
const { sessions } = await apiClient.listTaskSessions(taskA.id);
const s = sessions[0];
if (s?.worktree_branch) taskABranch = s.worktree_branch;
return DONE_STATES.includes(s?.state);
},
{ timeout: 30_000, message: "Waiting for Task A session to finish" },
)
.toBe(true);
expect(taskABranch).toBeTruthy();
// 3. Create Task B (no session, no repository)
await apiClient.createTask(seedData.workspaceId, "Sessionless Task", {
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
});
// 4. Navigate to Task A to populate the store with its session/branch data
const kanban = new KanbanPage(testPage);
await kanban.goto();
const cardA = kanban.taskCardByTitle("Task With Branch");
await expect(cardA).toBeVisible({ timeout: 10_000 });
await cardA.click();
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
const session = new SessionPage(testPage);
await session.waitForLoad();
await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible({
timeout: 15_000,
});
// Confirm topbar shows Task A's worktree branch
const branchPill = testPage.getByTestId("topbar-branch-name");
await expect(branchPill).toBeVisible({ timeout: 15_000 });
await expect(branchPill).toHaveText(taskABranch!, { timeout: 5_000 });
// 5. Navigate to Task B via the topbar home breadcrumb (SPA link) then clicking Task B.
// This preserves Zustand store state (activeSessionId from Task A persists).
const homeLink = testPage.locator("header a[href='/']").first();
await homeLink.click();
await expect(kanban.board).toBeVisible({ timeout: 10_000 });
const cardB = kanban.taskCardByTitle("Sessionless Task");
await expect(cardB).toBeVisible({ timeout: 10_000 });
await cardB.click();
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
// 6. Verify Task B's page shows the correct title (breadcrumb in topbar)
await expect(testPage.getByRole("link", { name: "Sessionless Task" })).toBeVisible({
timeout: 10_000,
});
// 7. Task A's worktree branch must NOT appear on Task B's topbar.
// Wait for any async state updates to settle.
await testPage.waitForTimeout(3_000);
const branchAfterNav = testPage.getByTestId("topbar-branch-name");
const isBranchVisible = await branchAfterNav.isVisible().catch(() => false);
if (isBranchVisible) {
const currentBranch = await branchAfterNav.textContent();
expect(currentBranch).not.toBe(taskABranch);
}
// If no branch is visible, that's correct for a sessionless task
});
});
@@ -204,45 +204,56 @@ interface UseSessionResumptionReturn {
* When a sessionId is provided (from URL), it checks the session status
* and automatically resumes if needed.
*/
export function useSessionResumption(
type SessionResetAndCheckResult = {
sessionStatus: SessionStatus | null;
};
/** Extracted effects: reset state on session/task change, auto-check/resume, and remote retry. */
function useSessionResetAndCheck(
taskId: string | null,
sessionId: string | null,
): UseSessionResumptionReturn {
const [resumptionState, setResumptionState] = useState<ResumptionState>("idle");
const [sessionStatus, setSessionStatus] = useState<SessionStatus | null>(null);
const [error, setError] = useState<string | null>(null);
const [worktreePath, setWorktreePath] = useState<string | null>(null);
const [worktreeBranch, setWorktreeBranch] = useState<string | null>(null);
const connectionStatus = useAppStore((state) => state.connection.status);
const session = useAppStore((state) =>
sessionId ? (state.taskSessions.items[sessionId] ?? null) : null,
);
const setTaskSession = useAppStore((state) => state.setTaskSession);
const setSessionAgentctlStatus = useAppStore((state) => state.setSessionAgentctlStatus);
connectionStatus: string,
session: { started_at?: string; updated_at?: string } | null,
setters: ResumeStateSetter,
): SessionResetAndCheckResult {
const [sessionStatusState, setSessionStatus] = useState<{
id: string | null;
status: SessionStatus | null;
}>({ id: sessionId, status: null });
// Reset sessionStatus when sessionId changes (derived, not in effect)
const sessionStatus = sessionStatusState.id === sessionId ? sessionStatusState.status : null;
const hasAttemptedResume = useRef(false);
const remoteStatusRetryCount = useRef(0);
const activeSessionRef = useRef(sessionId);
const setters: ResumeStateSetter = {
setResumptionState,
setError,
setWorktreePath,
setWorktreeBranch,
setTaskSession,
setAgentctlReady: (sid: string) => setSessionAgentctlStatus(sid, { status: "ready" }),
};
// Reset all local state when session or task changes to prevent stale data
// from a previous session leaking into the new one (e.g. topbar branch).
useEffect(() => {
activeSessionRef.current = sessionId;
hasAttemptedResume.current = false;
remoteStatusRetryCount.current = 0;
}, [sessionId, taskId]);
setters.setResumptionState("idle");
setters.setError(null);
setters.setWorktreePath(null);
setters.setWorktreeBranch(null);
}, [sessionId, taskId]); // eslint-disable-line react-hooks/exhaustive-deps -- intentional reset on dep change
// Check session status and auto-resume if needed
useEffect(() => {
if (!taskId || !sessionId || connectionStatus !== "connected" || hasAttemptedResume.current)
return;
hasAttemptedResume.current = true;
checkAndResume({ taskId, sessionId, session, setSessionStatus, setters });
}, [taskId, sessionId, connectionStatus, setTaskSession, session]); // eslint-disable-line react-hooks/exhaustive-deps
const guardedSetters = buildGuardedSetters(activeSessionRef, sessionId, setters);
checkAndResume({
taskId,
sessionId,
session,
setSessionStatus: (s) => {
if (activeSessionRef.current === sessionId) setSessionStatus({ id: sessionId, status: s });
},
setters: guardedSetters,
});
}, [taskId, sessionId, connectionStatus, session]); // eslint-disable-line react-hooks/exhaustive-deps
// Freshly created remote sessions may return status before runtime metadata is available.
// Retry a few times so topbar/tooltips can show remote details without manual refresh.
@@ -261,7 +272,7 @@ export function useSessionResumption(
task_id: taskId,
session_id: sessionId,
});
setSessionStatus(nextStatus);
setSessionStatus({ id: sessionId, status: nextStatus });
} catch {
// Best-effort refresh only.
}
@@ -270,6 +281,65 @@ export function useSessionResumption(
return () => window.clearTimeout(timer);
}, [taskId, sessionId, connectionStatus, sessionStatus]);
return { sessionStatus };
}
/** Wrap state setters with a guard that prevents stale async callbacks from updating state. */
function buildGuardedSetters(
activeSessionRef: React.RefObject<string | null>,
capturedSessionId: string,
setters: ResumeStateSetter,
): ResumeStateSetter {
const guard = () => activeSessionRef.current === capturedSessionId;
return {
...setters,
setResumptionState: (s) => {
if (guard()) setters.setResumptionState(s);
},
setError: (e) => {
if (guard()) setters.setError(e);
},
setWorktreePath: (p) => {
if (guard()) setters.setWorktreePath(p);
},
setWorktreeBranch: (b) => {
if (guard()) setters.setWorktreeBranch(b);
},
};
}
export function useSessionResumption(
taskId: string | null,
sessionId: string | null,
): UseSessionResumptionReturn {
const [resumptionState, setResumptionState] = useState<ResumptionState>("idle");
const [error, setError] = useState<string | null>(null);
const [worktreePath, setWorktreePath] = useState<string | null>(null);
const [worktreeBranch, setWorktreeBranch] = useState<string | null>(null);
const connectionStatus = useAppStore((state) => state.connection.status);
const session = useAppStore((state) =>
sessionId ? (state.taskSessions.items[sessionId] ?? null) : null,
);
const setTaskSession = useAppStore((state) => state.setTaskSession);
const setSessionAgentctlStatus = useAppStore((state) => state.setSessionAgentctlStatus);
const setters: ResumeStateSetter = {
setResumptionState,
setError,
setWorktreePath,
setWorktreeBranch,
setTaskSession,
setAgentctlReady: (sid: string) => setSessionAgentctlStatus(sid, { status: "ready" }),
};
const { sessionStatus } = useSessionResetAndCheck(
taskId,
sessionId,
connectionStatus,
session,
setters,
);
// Manual resume function
const resumeSession = useCallback(async (): Promise<boolean> => {
if (!taskId || !sessionId) return false;