From d580be66d5f04a3b2fe17e134a4dbf5b106fbe5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Sat, 1 Aug 2026 09:45:06 +0100 Subject: [PATCH] fix(web): remove mobile repository switcher (#2101) --- .../internal/office/testharness/routes.go | 2 + .../office/testharness/routes_test.go | 31 +++ .../task/mobile/mobile-repo-pill.tsx | 92 -------- .../task/mobile/mobile-repos-section.test.tsx | 74 ------- .../task/mobile/mobile-repos-section.tsx | 209 ------------------ .../mobile/mobile-sessions-section.test.tsx | 68 +++++- .../task/mobile/mobile-sessions-section.tsx | 52 ++++- .../task/mobile/session-mobile-layout.tsx | 2 +- .../task/mobile/session-mobile-top-bar.tsx | 2 - apps/web/e2e/helpers/api-client.ts | 2 + .../layout/mobile-spa-resilience.spec.ts | 19 +- ...le-multi-repository-session-picker.spec.ts | 78 +++++++ apps/web/hooks/use-task-sessions.test.ts | 67 +++++- apps/web/hooks/use-task-sessions.ts | 53 ++++- .../lib/state/slices/session/session-slice.ts | 6 + .../session/session-slice.upsert.test.ts | 24 ++ apps/web/lib/ws/handlers/agent-session.ts | 6 +- .../plan.md | 80 +++++++ ...sk-01-remove-mobile-repository-switcher.md | 98 ++++++++ docs/specs/ui/mobile-task-navigation.md | 8 +- 20 files changed, 561 insertions(+), 412 deletions(-) delete mode 100644 apps/web/components/task/mobile/mobile-repo-pill.tsx delete mode 100644 apps/web/components/task/mobile/mobile-repos-section.test.tsx delete mode 100644 apps/web/components/task/mobile/mobile-repos-section.tsx create mode 100644 apps/web/e2e/tests/session/mobile-multi-repository-session-picker.spec.ts create mode 100644 docs/plans/mobile-repository-switcher-removal/plan.md create mode 100644 docs/plans/mobile-repository-switcher-removal/task-01-remove-mobile-repository-switcher.md diff --git a/apps/backend/internal/office/testharness/routes.go b/apps/backend/internal/office/testharness/routes.go index e57296012a..62b4907792 100644 --- a/apps/backend/internal/office/testharness/routes.go +++ b/apps/backend/internal/office/testharness/routes.go @@ -285,6 +285,7 @@ func seedTaskHandler(repo *sqliterepo.Repository, log *logger.Logger) gin.Handle type seedTaskSessionRequest struct { TaskID string `json:"task_id"` SessionID string `json:"session_id,omitempty"` + RepositoryID string `json:"repository_id,omitempty"` State string `json:"state"` AgentProfileID string `json:"agent_profile_id,omitempty"` StartedAt *string `json:"started_at,omitempty"` @@ -491,6 +492,7 @@ func buildSeededSession(req *seedTaskSessionRequest) (*models.TaskSession, error return &models.TaskSession{ ID: sessionID, TaskID: req.TaskID, + RepositoryID: req.RepositoryID, AgentProfileID: req.AgentProfileID, State: models.TaskSessionState(req.State), Metadata: metadata, diff --git a/apps/backend/internal/office/testharness/routes_test.go b/apps/backend/internal/office/testharness/routes_test.go index cfd9be57c8..9b3098c55b 100644 --- a/apps/backend/internal/office/testharness/routes_test.go +++ b/apps/backend/internal/office/testharness/routes_test.go @@ -257,6 +257,37 @@ func TestSeedTaskSessionUsesRequestedSessionID(t *testing.T) { } } +func TestSeedTaskSessionUsesRequestedRepositoryID(t *testing.T) { + repo, sqlxDB := newTestRepo(t) + taskID := uuid.New().String() + seedTask(t, sqlxDB, taskID) + r := newRouter(t, repo, nil) + + body := mustJSON(t, map[string]interface{}{ + "task_id": taskID, + "state": "WAITING_FOR_INPUT", + "repository_id": "repository-secondary", + }) + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/v1/_test/task-sessions", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String()) + } + var resp seedTaskSessionResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode response: %v", err) + } + session, err := repo.GetTaskSession(context.Background(), resp.SessionID) + if err != nil { + t.Fatalf("get session: %v", err) + } + if session.RepositoryID != "repository-secondary" { + t.Fatalf("repository_id = %q, want repository-secondary", session.RepositoryID) + } +} + func TestSeedTaskSessionTerminalRequiresCompletedAt(t *testing.T) { repo, sqlxDB := newTestRepo(t) taskID := uuid.New().String() diff --git a/apps/web/components/task/mobile/mobile-repo-pill.tsx b/apps/web/components/task/mobile/mobile-repo-pill.tsx deleted file mode 100644 index 02ed8a753c..0000000000 --- a/apps/web/components/task/mobile/mobile-repo-pill.tsx +++ /dev/null @@ -1,92 +0,0 @@ -"use client"; - -import { memo, useEffect, useMemo, useState } from "react"; -import { IconFolder } from "@tabler/icons-react"; -import { useAppStore } from "@/components/state-provider"; -import { MobilePillButton } from "./mobile-pill-button"; -import { MobilePickerSheet } from "./mobile-picker-sheet"; -import { MobileReposSection, useTaskRepoCount } from "./mobile-repos-section"; - -const COMPACT_VIEWPORT_PX = 360; -const EMPTY_REPOSITORIES = Object.freeze([]); - -/** Returns true once the viewport is narrower than the compact threshold. */ -function useIsCompactViewport(): boolean { - const [isCompact, setIsCompact] = useState(false); - useEffect(() => { - const mql = window.matchMedia(`(max-width: ${COMPACT_VIEWPORT_PX - 1}px)`); - const update = () => setIsCompact(mql.matches); - update(); - mql.addEventListener("change", update); - return () => mql.removeEventListener("change", update); - }, []); - return isCompact; -} - -function useTaskActiveRepoName(taskId: string | null, workspaceId: string | null): string | null { - const workspaceRepos = useAppStore((s) => - workspaceId - ? (s.repositories.itemsByWorkspaceId[workspaceId] ?? EMPTY_REPOSITORIES) - : EMPTY_REPOSITORIES, - ); - const activeSessionId = useAppStore((s) => s.tasks.activeSessionId); - const activeRepoId = useAppStore((s) => - activeSessionId ? (s.taskSessions.items[activeSessionId]?.repository_id ?? null) : null, - ); - const taskRepos = useAppStore((s) => { - if (!taskId) return undefined; - const task = s.kanban.tasks.find((t: { id: string }) => t.id === taskId); - return task?.repositories; - }); - return useMemo(() => { - if (!activeRepoId) { - // Fallback to the position-primary task repo when no session is active - // yet. Match the picker's ordering (mobile-repos-section sorts by - // position) so the pill label and the first sheet row agree. - const sorted = taskRepos ? [...taskRepos].sort((a, b) => a.position - b.position) : []; - const fallback = sorted[0]?.repository_id; - if (!fallback) return null; - const repo = workspaceRepos.find((r) => r.id === fallback); - return repo?.name ?? repo?.local_path ?? null; - } - const repo = workspaceRepos.find((r) => r.id === activeRepoId); - return repo?.name ?? repo?.local_path ?? null; - }, [activeRepoId, taskRepos, workspaceRepos]); -} - -export const MobileRepoPill = memo(function MobileRepoPill({ - taskId, - workspaceId, -}: { - taskId: string | null; - workspaceId: string | null; -}) { - const repoCount = useTaskRepoCount(taskId); - const activeName = useTaskActiveRepoName(taskId, workspaceId); - const isCompact = useIsCompactViewport(); - const [open, setOpen] = useState(false); - - if (repoCount <= 1) return null; - - const label = activeName ?? "Repo"; - return ( - <> - } - label={label} - compact={isCompact} - isOpen={open} - onClick={() => setOpen(true)} - data-testid="mobile-repo-pill" - ariaLabel={`Active repository: ${label}. Tap to switch.`} - /> - - setOpen(false)} - /> - - - ); -}); diff --git a/apps/web/components/task/mobile/mobile-repos-section.test.tsx b/apps/web/components/task/mobile/mobile-repos-section.test.tsx deleted file mode 100644 index 9020f92710..0000000000 --- a/apps/web/components/task/mobile/mobile-repos-section.test.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import { cleanup, render, screen } from "@testing-library/react"; -import { afterEach, describe, expect, it, vi } from "vitest"; -import { StateProvider } from "@/components/state-provider"; -import { ToastProvider } from "@/components/toast-provider"; -import { defaultState } from "@/lib/state/default-state"; -import { MobileReposSection } from "./mobile-repos-section"; - -afterEach(() => { - cleanup(); - vi.restoreAllMocks(); -}); - -describe("MobileReposSection", () => { - it("renders task repositories before repository details and sessions hydrate", () => { - const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); - - expect(() => - render( - - - undefined} - /> - - , - ), - ).not.toThrow(); - - expect(consoleError).not.toHaveBeenCalledWith( - expect.stringContaining("The result of getSnapshot should be cached"), - ); - expect(screen.getByTestId("mobile-repo-row-repository-1")).toBeTruthy(); - expect(screen.getByTestId("mobile-repo-row-repository-2")).toBeTruthy(); - }); -}); diff --git a/apps/web/components/task/mobile/mobile-repos-section.tsx b/apps/web/components/task/mobile/mobile-repos-section.tsx deleted file mode 100644 index abc652b608..0000000000 --- a/apps/web/components/task/mobile/mobile-repos-section.tsx +++ /dev/null @@ -1,209 +0,0 @@ -"use client"; - -import { memo, useCallback, useMemo } from "react"; -import { IconCheck, IconFolder, IconGitBranch } from "@tabler/icons-react"; -import { useAppStore } from "@/components/state-provider"; -import { useToast } from "@/components/toast-provider"; -import type { Repository, TaskSession } from "@/lib/types/http"; -import type { KanbanState } from "@/lib/state/slices"; - -const EMPTY_REPOSITORIES: Repository[] = []; -const EMPTY_TASK_SESSIONS: TaskSession[] = []; - -type RepoRow = { - taskRepositoryId: string; - repositoryId: string; - name: string; - baseBranch: string; - checkoutBranch?: string; - sessionCount: number; - /** A representative session for this repo, if any (preferring primary). */ - switchToSessionId: string | null; -}; - -function pickSwitchToSession( - sessions: TaskSession[], - repositoryId: string, - primarySessionId: string | null | undefined, -): string | null { - const onRepo = sessions.filter((s) => s.repository_id === repositoryId); - if (onRepo.length === 0) return null; - if (primarySessionId && onRepo.some((s) => s.id === primarySessionId)) return primarySessionId; - // Most recently started session on this repo as fallback. - const sorted = [...onRepo].sort( - (a, b) => new Date(b.started_at).getTime() - new Date(a.started_at).getTime(), - ); - return sorted[0].id; -} - -function buildRepoRows( - taskRepositories: NonNullable, - workspaceRepos: Repository[], - taskSessions: TaskSession[], - primarySessionId: string | null | undefined, -): RepoRow[] { - return [...taskRepositories] - .sort((a, b) => a.position - b.position) - .map((tr) => { - const repo = workspaceRepos.find((r) => r.id === tr.repository_id); - const onRepoSessions = taskSessions.filter((s) => s.repository_id === tr.repository_id); - return { - taskRepositoryId: tr.id, - repositoryId: tr.repository_id, - name: repo?.name ?? repo?.local_path ?? "Repository", - baseBranch: tr.base_branch, - checkoutBranch: tr.checkout_branch, - sessionCount: onRepoSessions.length, - switchToSessionId: pickSwitchToSession(taskSessions, tr.repository_id, primarySessionId), - }; - }); -} - -function useTaskRepoRows(taskId: string | null, workspaceId: string | null): RepoRow[] { - const taskRepositories = useAppStore((s) => { - if (!taskId) return undefined; - const task = s.kanban.tasks.find((t: { id: string }) => t.id === taskId); - return task?.repositories; - }); - const workspaceRepos = useAppStore((s) => - workspaceId - ? (s.repositories.itemsByWorkspaceId[workspaceId] ?? EMPTY_REPOSITORIES) - : EMPTY_REPOSITORIES, - ); - const taskSessions = useAppStore((s) => - taskId - ? (s.taskSessionsByTask.itemsByTaskId[taskId] ?? EMPTY_TASK_SESSIONS) - : EMPTY_TASK_SESSIONS, - ); - const primarySessionId = useAppStore((s) => { - if (!taskId) return null; - const task = s.kanban.tasks.find((t: { id: string }) => t.id === taskId); - return task?.primarySessionId ?? null; - }); - return useMemo( - () => - taskRepositories - ? buildRepoRows(taskRepositories, workspaceRepos, taskSessions, primarySessionId) - : [], - [taskRepositories, workspaceRepos, taskSessions, primarySessionId], - ); -} - -function RepoRowItem({ - row, - isActive, - onSelect, -}: { - row: RepoRow; - isActive: boolean; - onSelect: (row: RepoRow) => void; -}) { - return ( -
onSelect(row)} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - onSelect(row); - } - }} - data-testid={`mobile-repo-row-${row.repositoryId}`} - className={`flex items-center gap-2 border px-2 py-2 rounded-md cursor-pointer select-none ${ - isActive ? "border-primary/50 bg-card" : "border-transparent hover:bg-muted" - }`} - > - -
- {row.name} -
- - - {row.checkoutBranch || row.baseBranch} - - {row.sessionCount > 0 && ( - - {row.sessionCount} session{row.sessionCount === 1 ? "" : "s"} - - )} -
-
- {isActive && } -
- ); -} - -export const MobileReposSection = memo(function MobileReposSection({ - taskId, - workspaceId, - onClose, -}: { - taskId: string | null; - workspaceId: string | null; - onClose: () => void; -}) { - const setActiveSession = useAppStore((s) => s.setActiveSession); - const activeSessionId = useAppStore((s) => s.tasks.activeSessionId); - const activeRepoId = useAppStore((s) => - activeSessionId ? (s.taskSessions.items[activeSessionId]?.repository_id ?? null) : null, - ); - const rows = useTaskRepoRows(taskId, workspaceId); - const { toast } = useToast(); - - const handleSelect = useCallback( - (row: RepoRow) => { - if (!taskId) return; - if (row.switchToSessionId) { - setActiveSession(taskId, row.switchToSessionId); - onClose(); - return; - } - // We don't auto-launch from a repo tap — that would create a side effect - // for what looks like navigation. Surface a hint so the tap doesn't - // appear silently broken. - toast({ - title: "No session on this repo yet", - description: "Open the session picker to launch one.", - }); - }, - [taskId, setActiveSession, onClose, toast], - ); - - if (!taskId) { - return ( -
No active task
- ); - } - if (rows.length === 0) { - return ( -
- This task has no repositories. -
- ); - } - - return ( -
- {rows.map((row) => ( - - ))} -
- ); -}); - -/** Returns the count of repositories attached to the active task. */ -export function useTaskRepoCount(taskId: string | null): number { - return ( - useAppStore((s) => { - if (!taskId) return 0; - const task = s.kanban.tasks.find((t: { id: string }) => t.id === taskId); - return task?.repositories?.length ?? 0; - }) ?? 0 - ); -} diff --git a/apps/web/components/task/mobile/mobile-sessions-section.test.tsx b/apps/web/components/task/mobile/mobile-sessions-section.test.tsx index c18b3759c4..56ea789e57 100644 --- a/apps/web/components/task/mobile/mobile-sessions-section.test.tsx +++ b/apps/web/components/task/mobile/mobile-sessions-section.test.tsx @@ -3,13 +3,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { MobileSessionsPicker } from "./mobile-sessions-section"; import type { AgentProfileOption } from "@/lib/state/slices"; -import type { TaskSession } from "@/lib/types/http"; +import { repositoryId, type Repository, type TaskSession } from "@/lib/types/http"; const mocks = vi.hoisted(() => ({ activeSessionId: "session-a" as string | null, sessions: [] as TaskSession[], agentProfiles: [] as AgentProfileOption[], + repositoriesByWorkspaceId: {} as Record, messagesBySession: {} as Record, + setActiveSession: vi.fn(), })); vi.mock("@/hooks/use-task-sessions", () => ({ @@ -22,8 +24,9 @@ vi.mock("@/components/state-provider", () => ({ tasks: { activeSessionId: mocks.activeSessionId }, agentProfiles: { items: mocks.agentProfiles }, kanban: { tasks: [{ id: "task-1", primarySessionId: "session-a" }] }, + repositories: { itemsByWorkspaceId: mocks.repositoriesByWorkspaceId }, messages: { bySession: mocks.messagesBySession }, - setActiveSession: vi.fn(), + setActiveSession: mocks.setActiveSession, }), })); @@ -80,6 +83,29 @@ function profile(id: string, label: string, agentName: string): AgentProfileOpti }; } +function repository(id: string, name: string): Repository { + return { + id, + workspace_id: "workspace-1", + name, + source_type: "local", + local_path: `/tmp/${name}`, + provider: "", + provider_repo_id: "", + provider_owner: "", + provider_name: "", + default_branch: "main", + worktree_branch_prefix: "kandev/", + pull_before_worktree: false, + setup_script: "", + cleanup_script: "", + dev_script: "", + copy_files: "", + created_at: START_TIME, + updated_at: START_TIME, + } as Repository; +} + afterEach(cleanup); beforeEach(() => { @@ -92,6 +118,9 @@ beforeEach(() => { profile("profile-a", "Alpha", "claude"), profile("profile-b", "Beta", "codex"), ]; + mocks.repositoriesByWorkspaceId = {}; + mocks.messagesBySession = {}; + mocks.setActiveSession.mockReset(); }); describe("MobileSessionsPicker selection", () => { @@ -119,6 +148,41 @@ describe("MobileSessionsPicker selection", () => { expect(within(pill).getByTestId("mobile-session-agent-icon")).toBeTruthy(); expect(within(pill).getByTestId("agent-logo-codex")).toBeTruthy(); }); + + it("disambiguates repository-bound sessions without a workflow task snapshot", () => { + mocks.sessions = [ + session(SESSION_A, "profile-a", START_TIME, { + repository_id: repositoryId("repository-a"), + }), + session("session-b", "profile-a", SECOND_TIME, { + repository_id: repositoryId("repository-b"), + }), + ]; + mocks.agentProfiles = [profile("profile-a", "Alpha", "claude")]; + mocks.repositoriesByWorkspaceId = { + "workspace-1": [repository("repository-a", "Frontend"), repository("repository-b", "API")], + }; + + render(); + + expect( + screen.getByRole("button", { + name: "Active session: Alpha. Repository: Frontend. Tap to switch.", + }), + ).toBeTruthy(); + expect(screen.getByTestId(PILL_TESTID).textContent).toContain("Alpha · Frontend"); + + fireEvent.click(screen.getByTestId(PILL_TESTID)); + expect( + within(screen.getByTestId("mobile-session-row-session-a")).getByText("Frontend"), + ).toBeTruthy(); + expect( + within(screen.getByTestId("mobile-session-row-session-b")).getByText("API"), + ).toBeTruthy(); + + fireEvent.click(screen.getByTestId("mobile-session-row-session-b")); + expect(mocks.setActiveSession).toHaveBeenCalledWith(TASK_ID, "session-b"); + }); }); describe("MobileSessionsPicker activity precedence", () => { diff --git a/apps/web/components/task/mobile/mobile-sessions-section.tsx b/apps/web/components/task/mobile/mobile-sessions-section.tsx index e8e572bdf3..64a5691347 100644 --- a/apps/web/components/task/mobile/mobile-sessions-section.tsx +++ b/apps/web/components/task/mobile/mobile-sessions-section.tsx @@ -35,6 +35,7 @@ import { MobilePillButton } from "./mobile-pill-button"; import { MobilePickerSheet } from "./mobile-picker-sheet"; import { formatTaskSessionStateLabel } from "@/lib/ui/state-labels"; import { getSessionStateIcon } from "@/lib/ui/state-icons"; +import { repositorySlug } from "@/lib/repository-slug"; import { useSessionPendingInput, type PendingInput } from "@/hooks/use-task-pending-input"; import type { ForegroundActivity, TaskSession, TaskSessionState } from "@/lib/types/http"; import type { AgentProfileOption } from "@/lib/state/slices"; @@ -43,6 +44,7 @@ type SessionRow = { id: string; agentName: string | null; agentLabel: string; + repositoryLabel: string | null; state: TaskSessionState | null; foregroundActivity: ForegroundActivity | null; isPrimary: boolean; @@ -54,6 +56,7 @@ function buildSessionRows( sessions: TaskSession[], agentProfiles: AgentProfileOption[], primarySessionId: string | null | undefined, + repositoryLabelsById: ReadonlyMap, ): SessionRow[] { const sorted = [...sessions].sort( (a, b) => new Date(a.started_at).getTime() - new Date(b.started_at).getTime(), @@ -67,6 +70,7 @@ function buildSessionRows( // User-supplied session name wins over the derived profile label, // matching the desktop session tab title precedence. agentLabel: s.name || labelParts[1] || labelParts[0] || profile?.label || "Agent", + repositoryLabel: s.repository_id ? (repositoryLabelsById.get(s.repository_id) ?? null) : null, state: (s.state as TaskSessionState | undefined) ?? null, foregroundActivity: s.foreground_activity ?? null, isPrimary: primarySessionId ? s.id === primarySessionId : !!s.is_primary, @@ -251,6 +255,22 @@ function DeleteSessionConfirmDialog({ ); } +function SessionIdentity({ row }: { row: SessionRow }) { + return ( +
+ {row.agentLabel} + {row.repositoryLabel && ( + + {row.repositoryLabel} + + )} +
+ ); +} + function SessionRowItem({ row, taskId, @@ -305,7 +325,7 @@ function SessionRowItem({ )} {row.agentName && } - {row.agentLabel} + s.agentProfiles.items); + const repositoriesByWorkspace = useAppStore((s) => s.repositories.itemsByWorkspaceId); const primarySessionId = useAppStore((s) => { if (!taskId) return null; const task = s.kanban.tasks.find((t: { id: string }) => t.id === taskId); return task?.primarySessionId ?? null; }); const { sessions, isLoading } = useTaskSessions(taskId); + const repositoryLabelsById = useMemo(() => { + const sessionRepositoryIds = new Set( + sessions.flatMap((session) => (session.repository_id ? [session.repository_id] : [])), + ); + if (sessionRepositoryIds.size <= 1) return new Map(); + const labels = new Map(); + for (const repository of Object.values(repositoriesByWorkspace).flat()) { + if (sessionRepositoryIds.has(repository.id)) { + labels.set(repository.id, repositorySlug(repository)); + } + } + return labels; + }, [repositoriesByWorkspace, sessions]); const rows = useMemo( - () => buildSessionRows(sessions, agentProfiles, primarySessionId), - [sessions, agentProfiles, primarySessionId], + () => buildSessionRows(sessions, agentProfiles, primarySessionId, repositoryLabelsById), + [sessions, agentProfiles, primarySessionId, repositoryLabelsById], ); return { rows, isLoading, primarySessionId }; } @@ -442,6 +476,7 @@ function useActiveSessionPillLabel( count: string | undefined; agentName: string | null; effectiveSessionId: string | null; + ariaLabel: string; } { const storedActiveSessionId = useAppStore((s) => s.tasks.activeSessionId); const { rows } = useSessionRows(taskId); @@ -452,11 +487,16 @@ function useActiveSessionPillLabel( let count: string | undefined; if (total > 1 && idx) count = `${idx}/${total}`; else if (total > 1) count = `${total}`; + const agentLabel = activeRow?.agentLabel ?? "Session"; + const repositoryLabel = activeRow?.repositoryLabel; return { - label: activeRow?.agentLabel ?? "Session", + label: repositoryLabel ? `${agentLabel} · ${repositoryLabel}` : agentLabel, count, agentName: activeRow?.agentName ?? null, effectiveSessionId, + ariaLabel: repositoryLabel + ? `Active session: ${agentLabel}. Repository: ${repositoryLabel}. Tap to switch.` + : `Active session: ${agentLabel}. Tap to switch.`, }; } @@ -472,7 +512,7 @@ export const MobileSessionsPicker = memo(function MobileSessionsPicker({ fullWidth?: boolean; }) { const [open, setOpen] = useState(false); - const { label, count, agentName, effectiveSessionId } = useActiveSessionPillLabel( + const { label, count, agentName, effectiveSessionId, ariaLabel } = useActiveSessionPillLabel( taskId, sessionId, ); @@ -498,7 +538,7 @@ export const MobileSessionsPicker = memo(function MobileSessionsPicker({ isOpen={open} onClick={() => setOpen(true)} data-testid="mobile-sessions-pill" - ariaLabel={`Active session: ${label}. Tap to switch.`} + ariaLabel={ariaLabel} /> +
- {!isArchived && ( { expectNoUnexpectedIssues(issues, isExpectedPersistentChunkConsole); }); - test("opens a multi-repository picker when optional task hydration fails", async ({ + test("keeps a multi-repository task usable without a mobile repository switcher when optional hydration fails", async ({ testPage, apiClient, seedData, @@ -233,16 +233,15 @@ test.describe("Mobile SPA resilience", () => { releaseFailures.resolve(); await Promise.all([repositorySettled.promise, sessionSettled.promise]); - const repoPill = testPage.getByTestId("mobile-repo-pill"); - await expect(repoPill).toBeVisible(); - await repoPill.tap(); - - const picker = testPage.getByRole("dialog"); - await expect(picker.getByText("Repositories", { exact: true })).toBeVisible(); - await expect(testPage.getByTestId(`mobile-repo-row-${seedData.repositoryId}`)).toBeVisible(); - await expect(testPage.getByTestId(`mobile-repo-row-${secondaryRepo.id}`)).toBeVisible(); + const activeMobileTaskLayout = testPage.locator("[data-testid='mobile-task-layout']:visible"); + await expect(activeMobileTaskLayout).toHaveCount(1); + await expect(activeMobileTaskLayout.getByTestId("mobile-sessions-pill")).toBeVisible(); + await expect(activeMobileTaskLayout.getByTestId("mobile-repo-pill")).toHaveCount(0); await expect(testPage.locator("#root")).not.toBeEmpty(); - await assertNoDocumentHorizontalOverflow(testPage, "mobile repository picker recovery"); + await assertNoDocumentHorizontalOverflow( + testPage, + "mobile task recovery without repository switcher", + ); expect(repositoryFailures).toBe(1); expect(sessionFailures).toBe(1); diff --git a/apps/web/e2e/tests/session/mobile-multi-repository-session-picker.spec.ts b/apps/web/e2e/tests/session/mobile-multi-repository-session-picker.spec.ts new file mode 100644 index 0000000000..71b2decf2d --- /dev/null +++ b/apps/web/e2e/tests/session/mobile-multi-repository-session-picker.spec.ts @@ -0,0 +1,78 @@ +// Filename starts with "mobile-" so this runs on the mobile-chrome project. +import { execSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; + +import { test, expect } from "../../fixtures/test-base"; +import { makeGitEnv } from "../../helpers/git-helper"; +import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions"; + +test.describe("mobile: multi-repository session picker", () => { + test("shows repository context and switches to a session on another repository", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const secondaryRepoDir = path.join(backend.tmpDir, "repos", "mobile-session-secondary"); + fs.mkdirSync(secondaryRepoDir, { recursive: true }); + const gitEnv = makeGitEnv(backend.tmpDir); + execSync("git init -b main", { cwd: secondaryRepoDir, env: gitEnv }); + execSync('git commit --allow-empty -m "init"', { cwd: secondaryRepoDir, env: gitEnv }); + const secondaryRepo = await apiClient.createRepository( + seedData.workspaceId, + secondaryRepoDir, + "main", + { name: "Mobile secondary" }, + ); + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Mobile repository sessions", + seedData.agentProfileId, + { + description: 'e2e:message("primary repository session")', + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId, secondaryRepo.id], + executor_profile_id: seedData.worktreeExecutorProfileId, + }, + ); + if (!task.session_id) throw new Error("multi-repository task has no primary session"); + + const websocketConnected = testPage.waitForEvent("websocket"); + await testPage.goto(`/t/${task.id}`); + await websocketConnected; + + const layout = testPage.locator("[data-testid='mobile-task-layout']:visible"); + const pill = layout.getByTestId("mobile-sessions-pill"); + await expect(layout).toHaveCount(1); + await expect(pill).toHaveAccessibleName(/^Active session: .+\. Tap to switch\.$/); + await expect(pill).not.toHaveAccessibleName(/Repository:/); + + const secondarySession = await apiClient.seedTaskSession(task.id, { + state: "WAITING_FOR_INPUT", + sessionId: `mobile-secondary-${task.id}`, + repositoryId: secondaryRepo.id, + startedAt: "2026-01-01T00:01:00Z", + }); + + await expect(pill).toHaveAccessibleName(/Repository: E2E Repo/); + await expect(pill).toContainText("E2E Repo"); + + await pill.tap(); + const primaryRow = testPage.getByTestId(`mobile-session-row-${task.session_id}`); + const secondaryRow = testPage.getByTestId(`mobile-session-row-${secondarySession.session_id}`); + await expect(primaryRow.getByText("E2E Repo", { exact: true })).toBeVisible(); + await expect(secondaryRow.getByText("Mobile secondary", { exact: true })).toBeVisible(); + + await secondaryRow.tap(); + await expect(pill).toHaveAccessibleName( + "Active session: Agent. Repository: Mobile secondary. Tap to switch.", + ); + await expect(pill).toContainText("Agent · Mobile secondary"); + + await pill.tap(); + await expect(secondaryRow).toHaveAttribute("aria-current", "true"); + await assertNoDocumentHorizontalOverflow(testPage, "multi-repository session picker"); + }); +}); diff --git a/apps/web/hooks/use-task-sessions.test.ts b/apps/web/hooks/use-task-sessions.test.ts index 4958bf2ed2..d9455897b0 100644 --- a/apps/web/hooks/use-task-sessions.test.ts +++ b/apps/web/hooks/use-task-sessions.test.ts @@ -20,9 +20,13 @@ type MockTaskSessionsState = { let mockState: MockTaskSessionsState; -vi.mock("@/components/state-provider", () => ({ - useAppStore: (selector: (state: MockTaskSessionsState) => unknown) => selector(mockState), -})); +vi.mock("@/components/state-provider", () => { + const getState = () => mockState; + return { + useAppStore: (selector: (state: MockTaskSessionsState) => unknown) => selector(mockState), + useAppStoreApi: () => ({ getState }), + }; +}); vi.mock("@/lib/api", () => apiMock); @@ -104,6 +108,63 @@ describe("useTaskSessions", () => { ); expect(mockState.setTaskSessionsForTask).toHaveBeenCalledWith(TASK_ID, [session("sess-1")]); }); + + it("preserves live session rows when stale-list hydration fails", async () => { + const error = new Error("service unavailable"); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined); + const liveSessions = [session("existing"), session("live-upsert")]; + mockState.taskSessionsByTask.itemsByTaskId[TASK_ID] = liveSessions; + mockState.taskSessionsByTask.loadedByTaskId[TASK_ID] = false; + apiMock.listTaskSessions.mockRejectedValueOnce(error); + + renderHook(() => useTaskSessions(TASK_ID)); + + await waitFor(() => + expect(apiMock.listTaskSessions).toHaveBeenCalledWith(TASK_ID, { + cache: "no-store", + }), + ); + expect(mockState.setTaskSessionsForTask).toHaveBeenCalledWith(TASK_ID, liveSessions); + expect(consoleError).toHaveBeenCalledWith("Failed to load task sessions:", error); + }); + + it("preserves and rehydrates a live session added during an older request", async () => { + const existing = session("existing"); + const livePartial = session("live-upsert"); + const liveHydrated = { ...livePartial, name: "Hydrated live session" }; + const firstResponse = deferred<{ sessions: TaskSession[] }>(); + mockState.taskSessionsByTask.itemsByTaskId[TASK_ID] = [existing]; + mockState.setTaskSessionsLoading.mockImplementation((id: string, loading: boolean) => { + mockState.taskSessionsByTask.loadingByTaskId[id] = loading; + }); + mockState.setTaskSessionsForTask.mockImplementation((id: string, sessions: TaskSession[]) => { + mockState.taskSessionsByTask.itemsByTaskId[id] = sessions; + mockState.taskSessionsByTask.loadedByTaskId[id] = true; + }); + apiMock.listTaskSessions + .mockReturnValueOnce(firstResponse.promise) + .mockResolvedValueOnce({ sessions: [existing, liveHydrated] }); + + const { rerender } = renderHook(() => useTaskSessions(TASK_ID)); + await waitFor(() => expect(apiMock.listTaskSessions).toHaveBeenCalledTimes(1)); + + mockState.taskSessionsByTask.itemsByTaskId[TASK_ID] = [existing, livePartial]; + firstResponse.resolve({ sessions: [existing] }); + + await waitFor(() => + expect(mockState.setTaskSessionsForTask).toHaveBeenCalledWith(TASK_ID, [ + existing, + livePartial, + ]), + ); + rerender(); + + await waitFor(() => expect(apiMock.listTaskSessions).toHaveBeenCalledTimes(2)); + expect(mockState.setTaskSessionsForTask).toHaveBeenLastCalledWith(TASK_ID, [ + existing, + liveHydrated, + ]); + }); }); describe("useTaskSessions refreshes", () => { diff --git a/apps/web/hooks/use-task-sessions.ts b/apps/web/hooks/use-task-sessions.ts index cd03405d1b..4c1d9bb1ce 100644 --- a/apps/web/hooks/use-task-sessions.ts +++ b/apps/web/hooks/use-task-sessions.ts @@ -1,12 +1,48 @@ import { useCallback, useEffect, useRef } from "react"; -import { useAppStore } from "@/components/state-provider"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { listTaskSessions } from "@/lib/api"; +import type { AppState } from "@/lib/state/store"; import type { TaskSession } from "@/lib/types/http"; import { useForegroundRefresh } from "@/hooks/use-foreground-refresh"; const EMPTY_SESSIONS: TaskSession[] = []; +function storedTaskSessions(getStoreState: () => AppState, taskId: string) { + return getStoreState().taskSessionsByTask.itemsByTaskId[taskId] ?? EMPTY_SESSIONS; +} + +async function hydrateTaskSessions({ + taskId, + force, + getStoreState, + setTaskSessionsForTask, +}: { + taskId: string; + force: boolean; + getStoreState: () => AppState; + setTaskSessionsForTask: AppState["setTaskSessionsForTask"]; +}): Promise { + const sessionIdsAtRequestStart = new Set( + storedTaskSessions(getStoreState, taskId).map((session) => session.id), + ); + try { + const response = await listTaskSessions(taskId, { cache: "no-store" }); + const fetchedSessions = response.sessions ?? []; + const fetchedSessionIds = new Set(fetchedSessions.map((session) => session.id)); + const sessionsAddedDuringLoad = storedTaskSessions(getStoreState, taskId).filter( + (session) => !sessionIdsAtRequestStart.has(session.id) && !fetchedSessionIds.has(session.id), + ); + setTaskSessionsForTask(taskId, [...fetchedSessions, ...sessionsAddedDuringLoad]); + return sessionsAddedDuringLoad.length > 0; + } catch (error) { + console.error("Failed to load task sessions:", error); + if (!force) setTaskSessionsForTask(taskId, storedTaskSessions(getStoreState, taskId)); + return false; + } +} + export function useTaskSessions(taskId: string | null) { + const getStoreState = useAppStoreApi().getState; const sessions = useAppStore((state) => taskId ? (state.taskSessionsByTask.itemsByTaskId[taskId] ?? EMPTY_SESSIONS) : EMPTY_SESSIONS, ); @@ -37,12 +73,13 @@ export function useTaskSessions(taskId: string | null) { if (!force && isLoaded) return; setTaskSessionsLoading(taskId, true); try { - const response = await listTaskSessions(taskId, { cache: "no-store" }); - const sessions = response.sessions ?? []; - setTaskSessionsForTask(taskId, sessions); - } catch (error) { - console.error("Failed to load task sessions:", error); - if (!force) setTaskSessionsForTask(taskId, []); + const needsFollowUp = await hydrateTaskSessions({ + taskId, + force, + getStoreState, + setTaskSessionsForTask, + }); + if (needsFollowUp) pendingForcedReloadRef.current = true; } finally { setTaskSessionsLoading(taskId, false); if (force && !pendingForcedReloadRef.current) { @@ -51,7 +88,7 @@ export function useTaskSessions(taskId: string | null) { } } }, - [isLoaded, isLoading, setTaskSessionsForTask, setTaskSessionsLoading, taskId], + [getStoreState, isLoaded, isLoading, setTaskSessionsForTask, setTaskSessionsLoading, taskId], ); useEffect(() => { diff --git a/apps/web/lib/state/slices/session/session-slice.ts b/apps/web/lib/state/slices/session/session-slice.ts index 2062fc5b71..67783621e7 100644 --- a/apps/web/lib/state/slices/session/session-slice.ts +++ b/apps/web/lib/state/slices/session/session-slice.ts @@ -537,6 +537,12 @@ function buildTaskSessionActions(set: ImmerSet) { ) => set((draft) => { const existing = draft.taskSessions.items[session.id]; + if (!existing && draft.taskSessionsByTask.loadedByTaskId[taskId]) { + // State events intentionally carry partial session rows. When one + // introduces a new session, let useTaskSessions hydrate fields such + // as repository_id instead of treating the old list as authoritative. + draft.taskSessionsByTask.loadedByTaskId[taskId] = false; + } const merged = existing ? mergeTaskSession(existing, session) : session; draft.taskSessions.items[session.id] = merged; const list = draft.taskSessionsByTask.itemsByTaskId[taskId]; diff --git a/apps/web/lib/state/slices/session/session-slice.upsert.test.ts b/apps/web/lib/state/slices/session/session-slice.upsert.test.ts index d3caa0b04d..0b9467678e 100644 --- a/apps/web/lib/state/slices/session/session-slice.upsert.test.ts +++ b/apps/web/lib/state/slices/session/session-slice.upsert.test.ts @@ -107,6 +107,30 @@ describe("upsertTaskSessionFromEvent", () => { const list = store.getState().taskSessionsByTask.itemsByTaskId[TASK_ID]; expect(list.map((s) => s.id)).toEqual(["session-other", SESSION_ID]); }); + + it("invalidates a loaded list when an event introduces a partial session", () => { + const store = makeStore(); + const existing = makeSession({ id: "session-existing", repository_id: "repo-1" }); + store.getState().setTaskSessionsForTask(TASK_ID, [existing]); + + store.getState().upsertTaskSessionFromEvent(TASK_ID, makeSession()); + + expect(store.getState().taskSessionsByTask.loadedByTaskId[TASK_ID]).toBe(false); + expect(store.getState().taskSessionsByTask.itemsByTaskId[TASK_ID].map((s) => s.id)).toEqual([ + "session-existing", + SESSION_ID, + ]); + }); + + it("keeps a loaded list authoritative when an event updates a known session", () => { + const store = makeStore(); + store.getState().setTaskSessionsForTask(TASK_ID, [makeSession({ repository_id: "repo-1" })]); + + store.getState().upsertTaskSessionFromEvent(TASK_ID, makeSession({ state: "COMPLETED" })); + + expect(store.getState().taskSessionsByTask.loadedByTaskId[TASK_ID]).toBe(true); + expect(store.getState().taskSessions.items[SESSION_ID].repository_id).toBe("repo-1"); + }); }); describe("setTaskSessionsForTask preserves WS-seeded fields", () => { diff --git a/apps/web/lib/ws/handlers/agent-session.ts b/apps/web/lib/ws/handlers/agent-session.ts index a53bf1b968..c7973efa13 100644 --- a/apps/web/lib/ws/handlers/agent-session.ts +++ b/apps/web/lib/ws/handlers/agent-session.ts @@ -219,9 +219,9 @@ function buildSessionUpdate(payload: any): Record { } /** Upsert the session in the per-task sessions list from a WS event. - * Uses `upsertTaskSessionFromEvent` so the per-task list is not marked as - * fully loaded — partial event payloads must not gate the API hydration that - * fills in fields like agent_profile_id / repository_id / worktree_path. */ + * Uses `upsertTaskSessionFromEvent` so a new partial row invalidates any + * previously loaded list, allowing API hydration to fill fields like + * agent_profile_id / repository_id / worktree_path. */ function upsertTaskSessionList( store: StoreApi, taskId: TaskId, diff --git a/docs/plans/mobile-repository-switcher-removal/plan.md b/docs/plans/mobile-repository-switcher-removal/plan.md new file mode 100644 index 0000000000..1676b625f7 --- /dev/null +++ b/docs/plans/mobile-repository-switcher-removal/plan.md @@ -0,0 +1,80 @@ +--- +spec: docs/specs/ui/mobile-task-navigation.md +created: 2026-07-31 +status: complete +--- + +# Implementation Plan: Mobile Repository Switcher Removal + +## Overview + +Remove the repository pill and picker from the phone task workbench because the control does not select repository state directly: it changes the active session. Keep the existing session picker as the single mobile control for active runtime context, identify repository-bound sessions inside that control for multi-repository tasks, preserve desktop and tablet behavior, and retain the optional-hydration resilience regression. + +## Frontend + +### Mobile task top bar + +- `apps/web/components/task/mobile/session-mobile-top-bar.tsx`: stop rendering `MobileRepoPill` in `MobileTopBarActions`. Keep task, plugin, remote-executor, Git, approval, and task-switcher actions unchanged. +- Delete `apps/web/components/task/mobile/mobile-repo-pill.tsx`, `apps/web/components/task/mobile/mobile-repos-section.tsx`, and `apps/web/components/task/mobile/mobile-repos-section.test.tsx` after confirming they have no remaining consumers. +- Keep `MobileSessionsPicker` in `apps/web/components/task/mobile/mobile-sessions-section.tsx` as the mobile entry point for changing sessions and therefore repository context. When loaded sessions span repositories, include the bound repository slug in each session row and in the active-session pill; leave single-repository presentation unchanged. Derive that distinction from required session data rather than the optional workflow snapshot. + +No production backend, API, persistence, desktop Dockview, or tablet changes are required. The E2E-only session seed route accepts `repository_id` so Playwright can model sessions bound to different repositories. + +## Mobile Design Contract + +- **Desktop outcome and mobile entry:** desktop and tablet repository/session interactions remain unchanged. Phone users enter the task workbench normally and change runtime context through the existing session pill above Chat. +- **Nearest shipped exemplar:** `MobileSessionsPicker` in `mobile-sessions-section.tsx` already uses `MobilePillButton` plus `MobilePickerSheet` for the actionable session hierarchy. +- **Hierarchy and primary action:** task identity and task-level actions stay in the fixed top bar; active-session selection stays with Chat. Repository identity is not promoted as a separate action. +- **Presentation and rationale:** no replacement drawer or navigation is added. Repository choice is not an independent mobile action in this surface, and the former picker only redirected to a representative session. Multi-repository context is secondary text within the retained session hierarchy. +- **Scroll, viewport, safe area, and touch:** existing `h-dvh` workbench, fixed top bar, internal panel scrolling, bottom navigation, and safe-area behavior remain unchanged. Removing the pill reduces top-bar crowding. +- **Shared logic:** session state, repository bindings, and all non-mobile repository flows remain unchanged; only phone presentation code is removed. +- **Mobile proof:** production Playwright scenarios cover optional repository/session hydration failure and selecting a session bound to another repository. They assert the repository pill is absent, the session pill remains usable and repository-aware, selection updates active context, and document horizontal overflow is absent. + +## Tests + +- Delete the component test dedicated to `MobileReposSection` with the removed component. +- Extend the existing `MobileSessionsPicker` component coverage for two same-agent sessions bound to different repositories, including the accessible pill label, visible row metadata, and selected-session action. + +## E2E Tests + +- **Scenario:** **GIVEN** a multi-repository task whose optional repository or session hydration fails, **WHEN** the phone task view opens, **THEN** no repository switcher is rendered, the session picker remains visible, and the task view stays usable without horizontal overflow. +- **File:** `apps/web/e2e/tests/layout/mobile-spa-resilience.spec.ts`. +- **What to verify:** replace repository-picker interaction assertions with absence of `mobile-repo-pill`, visibility of `mobile-sessions-pill`, a non-empty root, no unexpected browser errors, and no document horizontal overflow. +- **Repository-aware session scenario:** open a task with its primary session, seed a secondary-repository session while the page and WebSocket are live, select it through `MobileSessionsPicker`, and verify API hydration, row labels, active pill, selected row, and the horizontal-overflow contract in `apps/web/e2e/tests/session/mobile-multi-repository-session-picker.spec.ts`. + +## Implementation + +- [x] [Task 01 — Remove mobile repository switcher](task-01-remove-mobile-repository-switcher.md) + +## Risks + +- The existing SPA-resilience scenario currently relies on opening the repository picker. Update it without weakening its delayed/failed hydration, blank-root, browser-error, or overflow coverage. +- Repository controls outside the phone task top bar are out of scope and must remain intact. + +## Verification + +```bash +cd apps/web && pnpm e2e:run --project mobile-chrome -- tests/layout/mobile-spa-resilience.spec.ts --grep "keeps a multi-repository task usable without a mobile repository switcher when optional hydration fails" --workers=1 +cd apps/web && pnpm e2e:run tests/session/mobile-multi-repository-session-picker.spec.ts -- --project=mobile-chrome +cd apps && pnpm --filter @kandev/web test -- --run components/task/mobile/mobile-sessions-section.test.tsx +cd apps/backend && go test ./internal/office/testharness +cd apps/web && pnpm run typecheck +cd apps/web && pnpm lint +``` + +## Results + +- RED: after waiting for the retained session picker, the baseline production build failed as expected because `mobile-repo-pill` had count `1`, not `0`. +- GREEN: removing the top-bar render made the focused `mobile-chrome` scenario pass. +- REFACTOR: deleted the orphaned repository picker components and component test; web typecheck passed, then the final production rebuild and focused Playwright scenario passed (`1 passed`). +- REVIEW RED: Codex identified that same-label sessions on different repositories were indistinguishable; the focused component test and production mobile E2E both failed on the missing repository-aware pill label. +- REVIEW GREEN: multi-repository session rows and the active pill now include the canonical repository slug, the E2E-only seed route preserves `repository_id`, and the focused component suite (`8 passed`) plus cross-repository mobile Playwright scenario (`1 passed`) pass. +- REVIEW 2 RED: removing the workflow-task repository snapshot from the focused component fixture reproduced the missing repository labels. +- REVIEW 2 GREEN: repository labels now derive from required loaded session bindings plus the repository store, independent of optional workflow hydration. The focused component suite (`8 passed`), web typecheck, web lint, and production mobile Playwright scenario (`1 passed`) pass. +- REVIEW VERIFICATION: `go test ./internal/office/testharness` and full `pnpm lint` both pass. +- REVIEW 3 RED: a focused session-slice test showed that a partial live-session upsert left an already-loaded task session list marked authoritative (`true`, expected `false`). +- REVIEW 3 GREEN: an unknown live-session upsert now invalidates the task list for API hydration, while updates to known sessions remain cached. Focused slice/hook/picker tests (`39 passed`), typecheck, full lint, production build, and the live-seeded mobile Playwright scenario (`1 passed`) pass. +- REVIEW 4 RED: a focused hook test showed a failed stale-list hydration replaced the existing and partial live session rows with an empty list. +- REVIEW 4 GREEN: non-forced hydration failures now preserve the current store snapshot and mark it loaded, avoiding immediate retry loops while reconnect/foreground refresh retains a recovery path. Focused slice/hook/picker tests (`40 passed`), typecheck, full lint, production build, and the live-seeded mobile Playwright scenario (`1 passed`) pass. +- REVIEW 5 RED: a deferred hook test showed that an older successful list response dropped a live session added while the request was in flight and did not schedule another fetch. +- REVIEW 5 GREEN: list loads now retain sessions introduced after the request began and queue one forced authoritative reload to hydrate them. Request reconciliation was extracted to keep the hook within lint limits; focused slice/hook/picker tests (`41 passed`), typecheck, full lint, production build, and the live-seeded mobile Playwright scenario (`1 passed`) pass. diff --git a/docs/plans/mobile-repository-switcher-removal/task-01-remove-mobile-repository-switcher.md b/docs/plans/mobile-repository-switcher-removal/task-01-remove-mobile-repository-switcher.md new file mode 100644 index 0000000000..6e7a2ad7e5 --- /dev/null +++ b/docs/plans/mobile-repository-switcher-removal/task-01-remove-mobile-repository-switcher.md @@ -0,0 +1,98 @@ +--- +id: "01-remove-mobile-repository-switcher" +title: "Remove mobile repository switcher" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/ui/mobile-task-navigation.md" +--- + +# Task 01: Remove Mobile Repository Switcher + +## Acceptance + +- A multi-repository task never renders the repository pill or repository picker in the phone task workbench. +- The existing mobile session pill remains visible and continues to own active-session and repository-context changes. +- When loaded sessions span repositories, the session rows and active-session pill identify the bound repository so same-label sessions are distinguishable; selecting a row updates the active repository context without depending on optional workflow hydration. +- Optional repository/session hydration failures leave the phone task view non-empty, free of unexpected browser errors, and without document horizontal overflow; desktop, tablet, and other repository flows remain unchanged. + +## TDD Sequence + +1. **RED:** update the focused mobile SPA-resilience scenario to expect no `mobile-repo-pill` and a visible `mobile-sessions-pill`; run it and confirm it fails because the repository pill still renders. +2. **GREEN:** remove the top-bar `MobileRepoPill` render and delete its now-unreferenced picker components and component test. +3. **REFACTOR:** remove dangling imports/references only, then rerun the focused browser scenario and typecheck. +4. **REVIEW RED:** add component and mobile E2E coverage for two repository-bound sessions and confirm both fail because the retained picker omits repository identity. +5. **REVIEW GREEN:** add repository slugs to the retained picker when sessions span repositories and extend the E2E seed route to persist a session repository binding. +6. **REVIEW REFACTOR:** rerun focused component, backend harness, typecheck, and production mobile E2E checks. +7. **REVIEW 2 RED:** remove the workflow-task repository snapshot from the focused component fixture and confirm repository labels disappear. +8. **REVIEW 2 GREEN:** derive repository labels from loaded session bindings and the repository store, then rerun focused component, typecheck, lint, and production mobile E2E checks. +9. **REVIEW 3 RED:** introduce a partial session event into an already-loaded task session list and confirm the list incorrectly remains authoritative. +10. **REVIEW 3 GREEN:** invalidate the loaded list only for unknown live sessions so `useTaskSessions` hydrates their full rows, then verify known-session events do not trigger reloads. +11. **REVIEW 4 RED:** fail the API hydration after a live partial upsert and confirm the non-forced error path clears all session rows. +12. **REVIEW 4 GREEN:** preserve the current store snapshot on non-forced hydration failure so live rows remain visible and later refresh triggers can recover. +13. **REVIEW 5 RED:** resolve an older in-flight list request after a live session arrives and confirm its response drops the new row without another fetch. +14. **REVIEW 5 GREEN:** preserve sessions added during a list request and queue one forced follow-up load for authoritative hydration. + +## Verification + +```bash +cd apps/web && pnpm e2e:run --project mobile-chrome -- tests/layout/mobile-spa-resilience.spec.ts --grep "keeps a multi-repository task usable without a mobile repository switcher when optional hydration fails" --workers=1 +cd apps/web && pnpm e2e:run tests/session/mobile-multi-repository-session-picker.spec.ts -- --project=mobile-chrome +cd apps && pnpm --filter @kandev/web test -- --run components/task/mobile/mobile-sessions-section.test.tsx +cd apps/backend && go test ./internal/office/testharness +cd apps/web && pnpm run typecheck +cd apps/web && pnpm lint +``` + +## Files Likely Touched + +- `apps/web/components/task/mobile/session-mobile-top-bar.tsx` +- `apps/web/components/task/mobile/mobile-repo-pill.tsx` (delete) +- `apps/web/components/task/mobile/mobile-repos-section.tsx` (delete) +- `apps/web/components/task/mobile/mobile-repos-section.test.tsx` (delete) +- `apps/web/e2e/tests/layout/mobile-spa-resilience.spec.ts` +- `apps/web/components/task/mobile/mobile-sessions-section.tsx` +- `apps/web/components/task/mobile/mobile-sessions-section.test.tsx` +- `apps/web/e2e/tests/session/mobile-multi-repository-session-picker.spec.ts` +- `apps/web/e2e/helpers/api-client.ts` +- `apps/backend/internal/office/testharness/routes.go` +- `apps/backend/internal/office/testharness/routes_test.go` + +## Dependencies + +None. + +## Parallelism + +Sequential. Production removal and its browser regression own the same interaction contract. + +## Inputs + +- `docs/specs/ui/mobile-task-navigation.md` — mobile Dockview behavior and resilience scenario. +- `plan.md` — scoped frontend removal and mobile design contract. +- `apps/web/components/task/mobile/mobile-sessions-section.tsx` — retained session-picker behavior. +- `apps/web/e2e/tests/layout/mobile-spa-resilience.spec.ts` — existing multi-repository hydration-failure setup. + +## Risks + +- Do not remove repository selection from task creation, source attachment, Files, desktop, or tablet surfaces. +- Preserve all failure interception and browser-issue assertions in the existing resilience scenario. + +## Output Contract + +Report RED and GREEN evidence, files deleted/changed, exact command results, remaining risks, and update this task plus `plan.md` status in the same conversation. + +## Results + +- **RED:** baseline production build, after the retained session pill became visible, failed with `mobile-repo-pill` expected count `0`, received `1`. +- **GREEN:** removing `MobileRepoPill` from `session-mobile-top-bar.tsx` made the focused phone scenario pass. +- **REFACTOR:** deleted `mobile-repo-pill.tsx`, `mobile-repos-section.tsx`, and `mobile-repos-section.test.tsx`; no remaining source or mobile E2E interaction references exist. +- **Verification:** `cd apps/web && pnpm run typecheck` passed. Final change-aware `pnpm e2e:run --project mobile-chrome -- tests/layout/mobile-spa-resilience.spec.ts --grep "keeps a multi-repository task usable without a mobile repository switcher when optional hydration fails" --workers=1` rebuilt production assets and passed (`1 passed`). +- **Review remediation:** repository-aware picker tests failed first on the missing label. The retained picker now shows canonical repository slugs when loaded sessions span repositories, and selecting the secondary-repository row updates the active pill. The focused component suite passed (`8 passed`), the E2E seed-route repository test passed, and the production cross-repository mobile scenario passed (`1 passed`). +- **Review remediation 2:** removing the optional workflow-task repository snapshot reproduced the missing labels. Label derivation now uses required loaded session bindings plus the repository store; the focused component suite (`8 passed`), web typecheck, web lint, and production cross-repository mobile scenario (`1 passed`) passed. +- **Recorded commands:** `cd apps/backend && go test ./internal/office/testharness` passed; `cd apps/web && pnpm lint` passed. +- **Review remediation 3:** a partial live-session upsert now marks an already-loaded per-task list stale so the existing hook fetches authoritative repository fields; events for known sessions keep the list loaded. Focused slice/hook/picker tests (`39 passed`), typecheck, full lint, production build, and a page-live secondary-session mobile E2E (`1 passed`) passed. +- **Review remediation 4:** failed stale-list hydration now re-commits the current live store rows instead of replacing them with `[]`; the list remains available and avoids a retry loop until reconnect or foreground refresh. Focused slice/hook/picker tests (`40 passed`), typecheck, full lint, production build, and the live-seeded mobile E2E (`1 passed`) passed. +- **Review remediation 5:** an older successful response now merges live sessions introduced after its request began, then schedules one forced list reload so their complete repository data replaces the partial row. Request reconciliation was extracted to keep the hook within lint limits; focused slice/hook/picker tests (`41 passed`), typecheck, full lint, production build, and the live-seeded mobile E2E (`1 passed`) passed. +- **Remaining risks:** none known within scoped phone task workbench; repository selection elsewhere is unchanged. diff --git a/docs/specs/ui/mobile-task-navigation.md b/docs/specs/ui/mobile-task-navigation.md index 5b15252946..3d927851e8 100644 --- a/docs/specs/ui/mobile-task-navigation.md +++ b/docs/specs/ui/mobile-task-navigation.md @@ -26,7 +26,7 @@ Mobile users need the same task controls as desktop without relying on long pres - Editing a task from a mobile context menu exposes its title even after work has started. The existing lock on a started task's prompt remains unchanged. - The mobile Home menu and Dockview task switcher open as inset, card-style bottom surfaces with internal vertical scrolling and safe-area spacing, rather than edge-to-edge side sheets. - Switching tasks from the mobile task switcher keeps the workbench usable while destination details hydrate. Transient placeholder data must not crash mobile chrome, and a remembered panel that is unavailable for the destination falls back to Chat. -- Opening the repository picker for a multi-repository task remains usable while repository details and sessions are still loading. Delayed data cannot crash mobile chrome. +- Mobile Dockview does not expose a repository switcher. The existing session picker owns active-session changes; choosing a session also changes repository context when that session belongs to another repository. When loaded sessions span repositories, the active-session pill and session rows identify each session's repository so otherwise-identical sessions remain distinguishable, even if optional workflow hydration is unavailable. - The active-session control at the top of mobile Dockview shows the active agent's icon beside its session label. - Desktop and tablet Kanban, context menus, drag/drop, and workflow filtering retain their existing behavior. @@ -46,7 +46,9 @@ Mobile users need the same task controls as desktop without relying on long pres - **GIVEN** a started task on mobile Home, **WHEN** the user chooses Edit from its context menu, **THEN** the title can be changed while the prompt remains locked. - **GIVEN** the mobile Home menu or Dockview task switcher is opened, **WHEN** its content exceeds the viewport, **THEN** an inset bottom card remains within the safe area and scrolls internally. - **GIVEN** two tasks with active sessions, **WHEN** the user selects the other task from the mobile task switcher, **THEN** the destination URL, title, and chat render without a blank screen or mobile-chrome crash. -- **GIVEN** a multi-repository task whose repository details or sessions are delayed, **WHEN** the user opens the mobile repository picker, **THEN** the picker remains usable and the task view does not become blank. +- **GIVEN** a multi-repository task whose repository details or sessions are delayed or fail to load, **WHEN** the user opens the mobile task view, **THEN** the task remains usable, the session picker remains available, and no repository switcher is rendered. +- **GIVEN** a multi-repository task with sessions on different repositories, **WHEN** the user opens the mobile session picker and chooses another session, **THEN** each bound repository is identified and the active-session pill updates to the selected repository context. +- **GIVEN** a task page is already open, **WHEN** a live session event introduces a session bound to another repository, **THEN** the session picker hydrates its repository identity without a page refresh; if that hydration request fails, the existing and partial live rows remain available for a later refresh. - **GIVEN** Review is remembered for a mobile session but its selected merge request is no longer available, **WHEN** the session becomes active, **THEN** Chat is rendered and selected instead of an empty center pane. - **GIVEN** a mobile Dockview task with an active session, **WHEN** its chat panel is visible, **THEN** the active-session control shows the session agent's icon and label. - **GIVEN** a desktop viewport, **WHEN** the same task menus and Kanban open, **THEN** their existing desktop interaction and layout remain unchanged. @@ -57,8 +59,10 @@ Mobile users need the same task controls as desktop without relying on long pres - Persisting the transient focused mobile step across reloads. - Changing backend task-move contracts, workflow ordering, or task permissions. - Unlocking or changing a started task's prompt. +- Changing repository selection during task creation, workspace-source attachment, file browsing, or desktop and tablet task interaction. ## Implementation plans - [Mobile task navigation refinement](../../plans/mobile-task-navigation-refinement/plan.md) - [SPA blank-screen resilience](../../plans/spa-blank-screen-resilience/plan.md) +- [Mobile repository switcher removal](../../plans/mobile-repository-switcher-removal/plan.md)