diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..89e4ea1352 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,21 @@ +# Source is always text, and must always diff as text. +# +# git classifies a blob as binary if it finds a NUL byte in the first 8k — and a +# stray NUL in a source file is easy to introduce (a NUL used as a string +# delimiter, pasted verbatim instead of escaped) and almost impossible to notice. +# The cost is severe and silent: `git diff` and GitHub render the whole file as +# "Bin N -> M bytes", so every change to it sails through review unseen, and it +# gets no three-way merge. +# +# This happened: MachineKeepAliveHost.tsx carried a literal NUL and was binary to +# git for its whole history. Forcing `diff` on source extensions makes that class +# of mistake cosmetic instead of review-defeating. +*.ts diff +*.tsx diff +*.js diff +*.jsx diff +*.mjs diff +*.cjs diff +*.json diff +*.css diff +*.md diff diff --git a/apps/web/src/app/api/machines/__tests__/route.test.ts b/apps/web/src/app/api/machines/__tests__/route.test.ts new file mode 100644 index 0000000000..769824bf4c --- /dev/null +++ b/apps/web/src/app/api/machines/__tests__/route.test.ts @@ -0,0 +1,97 @@ +/** + * Contract tests for GET /api/machines — the Development surface's + * list-machines-in-a-drive query. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const { mockAuthenticateRequest, mockIsAuthError, mockListDriveMachines, mockAuditRequest } = vi.hoisted(() => ({ + mockAuthenticateRequest: vi.fn(), + mockIsAuthError: vi.fn((result: unknown) => result != null && typeof result === 'object' && 'error' in result), + mockListDriveMachines: vi.fn(), + mockAuditRequest: vi.fn(), +})); + +vi.mock('@/lib/auth', () => ({ + authenticateRequestWithOptions: (...args: unknown[]) => mockAuthenticateRequest(...args), + isAuthError: (result: unknown) => mockIsAuthError(result), +})); + +vi.mock('@pagespace/lib/audit/audit-log', () => ({ + auditRequest: (...args: unknown[]) => mockAuditRequest(...args), +})); + +vi.mock('@/lib/machines/machine-list-runtime', () => ({ + listDriveMachines: (...args: unknown[]) => mockListDriveMachines(...args), +})); + +import { GET } from '../route'; + +const AUTH_ADMIN = { userId: 'user-1', role: 'admin' }; +const AUTH_NON_ADMIN = { userId: 'user-2', role: 'user' }; +const AUTH_DENIED = { error: new Response(null, { status: 401 }) }; + +const MACHINE = { id: 'machine-1', title: 'Dev box', updatedAt: '2026-07-11T00:00:00.000Z' }; + +beforeEach(() => { + vi.clearAllMocks(); + mockAuthenticateRequest.mockResolvedValue(AUTH_ADMIN); + mockListDriveMachines.mockResolvedValue([MACHINE]); +}); + +describe('GET /api/machines', () => { + it('refuses a non-admin, and never enumerates the drive for them', async () => { + // Machines are an app-admin feature: a non-admin who can merely VIEW a + // Machine page must not be able to enumerate the drive's machines (and, + // through the tree, their projects/branches/sessions). + mockAuthenticateRequest.mockResolvedValue(AUTH_NON_ADMIN); + + const response = await GET(new Request('http://localhost/api/machines?driveId=drive-1')); + + expect(response.status).toBe(403); + expect(mockListDriveMachines).not.toHaveBeenCalled(); + }); + + it('audits the non-admin denial', async () => { + mockAuthenticateRequest.mockResolvedValue(AUTH_NON_ADMIN); + + await GET(new Request('http://localhost/api/machines?driveId=drive-1')); + + expect(mockAuditRequest).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ eventType: 'authz.access.denied', userId: 'user-2' }), + ); + }); + + it('returns the drive\'s machines for an admin', async () => { + const response = await GET(new Request('http://localhost/api/machines?driveId=drive-1')); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ machines: [MACHINE] }); + expect(mockListDriveMachines).toHaveBeenCalledWith('user-1', 'drive-1'); + }); + + it('400s without a driveId', async () => { + const response = await GET(new Request('http://localhost/api/machines')); + + expect(response.status).toBe(400); + expect(mockListDriveMachines).not.toHaveBeenCalled(); + }); + + it('propagates the auth error and never touches the drive', async () => { + mockAuthenticateRequest.mockResolvedValue(AUTH_DENIED); + + const response = await GET(new Request('http://localhost/api/machines?driveId=drive-1')); + + expect(response.status).toBe(401); + expect(mockListDriveMachines).not.toHaveBeenCalled(); + }); + + it('serves an empty list rather than 404 when the drive has no machines', async () => { + mockListDriveMachines.mockResolvedValue([]); + + const response = await GET(new Request('http://localhost/api/machines?driveId=drive-1')); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ machines: [] }); + }); +}); diff --git a/apps/web/src/app/api/machines/route.ts b/apps/web/src/app/api/machines/route.ts new file mode 100644 index 0000000000..87c7baaa12 --- /dev/null +++ b/apps/web/src/app/api/machines/route.ts @@ -0,0 +1,63 @@ +/** + * Machines API — the Development surface's aggregated tree needs the one thing + * no other machine route serves: every Machine in a drive, not one Machine by + * id. + * + * GET ?driveId= → { machines: [{ id, title, updatedAt }] } + * + * Session-only (no MCP/agent tokens) — a human/UI surface, like the rest of + * `/api/machines/*`. + * + * App-admin only, matching the rest of the Machine feature: creating a MACHINE + * page requires `admin` (see POST /api/pages) and `MachineView` refuses to mount + * its tabs for anyone else. Without this, a non-admin drive member who can VIEW a + * Machine page could enumerate the drive's machines from the Development surface. + * + * Note this route is STRICTER than its siblings, not a system-wide guarantee: + * /api/machines/{projects,branches,agent-terminals} gate on `canViewMachine`, not + * on admin, so a non-admin with view access on a Machine page can still call them + * directly. This surface simply declines to be the thing that hands them the list + * of machines to call them with. + * + * Admin is necessary but not sufficient: the list is still filtered per page + * through `canUserViewPage`, so a Machine withheld from this admin by a + * page-level grant never appears. + */ + +import { NextResponse } from 'next/server'; +import { authenticateRequestWithOptions, isAuthError } from '@/lib/auth'; +import { auditRequest } from '@pagespace/lib/audit/audit-log'; +import { loggers } from '@pagespace/lib/logging/logger-config'; +import { listDriveMachines } from '@/lib/machines/machine-list-runtime'; + +const AUTH_OPTIONS_READ = { allow: ['session'] as const, requireCSRF: false }; + +export async function GET(request: Request) { + const auth = await authenticateRequestWithOptions(request, AUTH_OPTIONS_READ); + if (isAuthError(auth)) return auth.error; + + const driveId = new URL(request.url).searchParams.get('driveId'); + if (!driveId) { + return NextResponse.json({ error: 'driveId is required' }, { status: 400 }); + } + + if (auth.role !== 'admin') { + auditRequest(request, { + eventType: 'authz.access.denied', + userId: auth.userId, + resourceType: 'drive', + resourceId: driveId, + details: { reason: 'app_admin_required', method: 'GET', route: 'machines' }, + riskScore: 0.5, + }); + return NextResponse.json({ error: 'Machines require administrator privileges' }, { status: 403 }); + } + + try { + const machines = await listDriveMachines(auth.userId, driveId); + return NextResponse.json({ machines }); + } catch (error) { + loggers.api.error('Error listing machines:', error as Error); + return NextResponse.json({ error: 'Failed to list machines' }, { status: 500 }); + } +} diff --git a/apps/web/src/app/dashboard/DashboardLayoutClient.tsx b/apps/web/src/app/dashboard/DashboardLayoutClient.tsx index d2d9e2f41d..fc8cfb88ba 100644 --- a/apps/web/src/app/dashboard/DashboardLayoutClient.tsx +++ b/apps/web/src/app/dashboard/DashboardLayoutClient.tsx @@ -15,6 +15,7 @@ const FULL_PAGE_ROUTES = [ '/dashboard/calendar', '/dashboard/channels', '/dashboard/connections', + '/dashboard/development', '/dashboard/dms', '/dashboard/drives', '/dashboard/storage', @@ -32,7 +33,7 @@ export default function DashboardLayoutClient({ children, nonce }: { children: R // Also match /dashboard/[driveId]/activity pattern const isFullPageRoute = FULL_PAGE_ROUTES.some(route => pathname === route || pathname?.startsWith(route + '/') - ) || pathname?.match(/^\/dashboard\/[^/]+\/(activity|calendar|channels|files|tasks|trash|settings|members|workflows)/); + ) || pathname?.match(/^\/dashboard\/[^/]+\/(activity|calendar|channels|development|files|tasks|trash|settings|members|workflows)/); return ( diff --git a/apps/web/src/app/dashboard/[driveId]/development/[machineId]/page.tsx b/apps/web/src/app/dashboard/[driveId]/development/[machineId]/page.tsx new file mode 100644 index 0000000000..bf56bb3b3b --- /dev/null +++ b/apps/web/src/app/dashboard/[driveId]/development/[machineId]/page.tsx @@ -0,0 +1,16 @@ +/** + * The Development surface's detail route. + * + * Renders NOTHING on purpose. The machine is drawn by `MachineKeepAliveHost` in + * this segment's layout, which keeps recently-visited machines mounted across + * navigation (CSS-hiding the inactive ones) so their terminals survive. Mounting + * a `MachineView` here as well would create a second, competing terminal subtree + * for the same machine — the same reason `CenterPanel` renders nothing for + * MACHINE pages in the drive view. + * + * The route still exists to make a machine bookmarkable: the URL is what the + * layout reads to decide which machine is active. + */ +export default function DevelopmentMachinePage() { + return null; +} diff --git a/apps/web/src/app/dashboard/[driveId]/development/__tests__/layout.test.tsx b/apps/web/src/app/dashboard/[driveId]/development/__tests__/layout.test.tsx new file mode 100644 index 0000000000..2ffbf03dbb --- /dev/null +++ b/apps/web/src/app/dashboard/[driveId]/development/__tests__/layout.test.tsx @@ -0,0 +1,195 @@ +/** + * The Development surface's detail region — the composition, not its parts. + * + * Every piece this layout uses is unit-tested; what wasn't was how they fit + * together, and that's where six review passes kept finding bugs. So these tests + * pin the three properties that were actually broken at some point: + * - the sticky machine set converges instead of re-rendering forever, and + * keeps its identity so the keep-alive host doesn't churn its LRU; + * - a failed fetch says "failed", not "your machine was deleted"; + * - a machine that vanishes stops being DISPLAYED but is not evicted (which + * would disconnect its terminal). + */ +import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; + +const mockUseAuth = vi.fn(); +const mockUseDriveMachines = vi.fn(); +/** Every render of the stubbed keep-alive host, so we can assert on what it was handed. */ +const hostRenders: { activePageId: string | null; machineIds: readonly string[] }[] = []; + +vi.mock('next/navigation', () => ({ + useParams: () => ({ driveId: 'drive-1' }), + usePathname: () => '/dashboard/drive-1/development/machine-1', +})); + +vi.mock('@/hooks/useAuth', () => ({ useAuth: () => mockUseAuth() })); +vi.mock('@/hooks/useDriveMachines', () => ({ useDriveMachines: () => mockUseDriveMachines() })); + +vi.mock('@/components/layout/middle-content/MachineKeepAliveHost', () => ({ + default: (props: { activePageId: string | null; machineIds: readonly string[] }) => { + hostRenders.push({ activePageId: props.activePageId, machineIds: props.machineIds }); + return
; + }, +})); + +import DevelopmentLayout from '../layout'; +import { usePendingSessionStore } from '@/stores/development/usePendingSessionStore'; +import { useMachineWorkspaceStore } from '@/stores/machine-workspace/useMachineWorkspaceStore'; + +const machine = (id: string) => ({ id, title: id, updatedAt: '2026-07-12T00:00:00.000Z' }); + +const driveMachines = (over: Partial<{ machines: ReturnType[]; isLoading: boolean; error: Error | undefined }> = {}) => ({ + machines: over.machines ?? [machine('machine-1')], + isLoading: over.isLoading ?? false, + error: over.error, + mutate: vi.fn(), +}); + +beforeEach(() => { + vi.clearAllMocks(); + hostRenders.length = 0; + usePendingSessionStore.setState({ pending: null }); + useMachineWorkspaceStore.setState({ workspaces: {} }); + mockUseAuth.mockReturnValue({ user: { role: 'admin' }, isLoading: false }); + mockUseDriveMachines.mockReturnValue(driveMachines()); +}); + +describe('DevelopmentLayout', () => { + test('hands the selected machine and its drive\'s machines to the keep-alive host', () => { + render({null}); + + expect(hostRenders.at(-1)).toEqual({ activePageId: 'machine-1', machineIds: ['machine-1'] }); + }); + + test('settles instead of re-rendering forever', () => { + // useStickyMachineIds sets state DURING render. If its key were derived from + // the array's identity rather than its contents it would loop, because SWR + // hands back a fresh array every render. + render({null}); + + expect(hostRenders.length).toBeLessThanOrEqual(3); + }); + + test('keeps the machine-id list stable across a revalidation that changes nothing', () => { + // A new array identity each render would look to the host like a changed + // machine set, which is what drives LRU eviction — i.e. terminal teardown. + const { rerender } = render({null}); + const first = hostRenders.at(-1)!.machineIds; + + mockUseDriveMachines.mockReturnValue(driveMachines({ machines: [machine('machine-1')] })); + rerender({null}); + + expect(hostRenders.at(-1)!.machineIds).toBe(first); + }); + + test('a failed fetch reports the failure — it does not claim the machine is gone', () => { + // SWR reports isLoading:false with no data on the error path, which is + // indistinguishable from "no such machine" unless error is checked first. + mockUseDriveMachines.mockReturnValue(driveMachines({ machines: [], error: new Error('boom') })); + + render({null}); + + expect(screen.getByText(/failed to load machines/i)).toBeDefined(); + expect(screen.queryByText(/machine not found/i)).toBeNull(); + }); + + test('a failed POLL does not blank out a machine we can still show', () => { + // The list polls, and SWR keeps the last good data while setting `error` on a + // failed revalidation. Reporting the error ahead of the data would let one + // blip of a background poll replace a working machine with an error notice. + mockUseDriveMachines.mockReturnValue(driveMachines({ machines: [machine('machine-1')], error: new Error('blip') })); + + render({null}); + + expect(screen.queryByText(/failed to load machines/i)).toBeNull(); + expect(hostRenders.at(-1)!.activePageId).toBe('machine-1'); + }); + + test('a machine that vanishes stops being shown, but is NOT evicted', () => { + // It may have been deleted — or the per-page permission check may have + // swallowed a DB error and reported "cannot view". Stop DISPLAYING it either + // way, but keep it in the mountable set: evicting it would unmount MachineView + // and disconnect a terminal that might be perfectly alive. + // + // Must be a rerender, NOT cleanup() + render: a fresh mount rebuilds the + // sticky set from the (now empty) fetch, which would evict the machine and + // make this test pass while proving the opposite of its name. + const { rerender } = render({null}); + expect(hostRenders.at(-1)!.machineIds).toContain('machine-1'); + + mockUseDriveMachines.mockReturnValue(driveMachines({ machines: [] })); + rerender({null}); + + expect(screen.getByText(/machine not found/i)).toBeDefined(); + // Not displayed… + expect(hostRenders.at(-1)!.activePageId).toBeNull(); + // …but still mountable, so its terminal is not torn down. + expect(hostRenders.at(-1)!.machineIds).toContain('machine-1'); + }); + + test('says nothing about admin rights until auth resolves', () => { + mockUseAuth.mockReturnValue({ user: undefined, isLoading: true }); + + render({null}); + + expect(screen.queryByText(/administrator privileges/i)).toBeNull(); + }); + + test('refuses a non-admin', () => { + mockUseAuth.mockReturnValue({ user: { role: 'user' }, isLoading: false }); + + render({null}); + + expect(screen.getByText(/administrator privileges/i)).toBeDefined(); + }); + + test('does not open a session into a machine the host is keeping HIDDEN', () => { + // The drain must gate on what is DISPLAYED, not on what the URL selects. If a + // machine is transiently missing from the list the host hides every pane, and + // opening a session then mounts an xterm inside a `display:none` container — + // fit() measures a zero-sized box and the PTY is created at a bogus geometry, + // wrapping its output for the life of the session. + mockUseDriveMachines.mockReturnValue(driveMachines({ machines: [] })); + usePendingSessionStore.setState({ pending: { machineId: 'machine-1', scope: { name: 'agent-1' } } }); + + render({null}); + + expect(hostRenders.at(-1)!.activePageId).toBeNull(); + // Held, not applied — it converges once the machine is displayed again. + expect(useMachineWorkspaceStore.getState().workspaces['machine-1']).toBeUndefined(); + }); + + test('holds a session intent while the list loads, then applies it once the machine is displayed', () => { + // The other half of gating the drain on what's DISPLAYED: holding must not + // become dropping. An intent for a machine that isn't in the list *yet* waits, + // and converges as soon as the machine appears — otherwise the fix for the + // hidden-machine case would have silently broken the ordinary one. + mockUseDriveMachines.mockReturnValue(driveMachines({ machines: [], isLoading: true })); + usePendingSessionStore.setState({ pending: { machineId: 'machine-1', scope: { name: 'agent-1' } } }); + const { rerender } = render({null}); + + // Held, not dropped. + expect(usePendingSessionStore.getState().pending).not.toBeNull(); + + // The list arrives, and the machine's pane region has ensured a workspace. + mockUseDriveMachines.mockReturnValue(driveMachines({ machines: [machine('machine-1')] })); + useMachineWorkspaceStore.getState().ensureWorkspace('machine-1'); + rerender({null}); + + const workspace = useMachineWorkspaceStore.getState().workspaces['machine-1']; + const activePane = workspace.columns.flatMap((c) => c.panes).find((p) => p.id === workspace.activePaneId); + expect(activePane?.scope).toMatchObject({ name: 'agent-1' }); + }); + + test('drops an unconverged session intent when the surface is left', () => { + // The store is a module singleton: an intent left behind would still be + // sitting there on the next visit, ready to fire into whatever pane is active. + usePendingSessionStore.setState({ pending: { machineId: 'machine-9', scope: { name: 'agent-1' } } }); + const { unmount } = render({null}); + + unmount(); + + expect(usePendingSessionStore.getState().pending).toBeNull(); + }); +}); diff --git a/apps/web/src/app/dashboard/[driveId]/development/layout.tsx b/apps/web/src/app/dashboard/[driveId]/development/layout.tsx new file mode 100644 index 0000000000..42cee93378 --- /dev/null +++ b/apps/web/src/app/dashboard/[driveId]/development/layout.tsx @@ -0,0 +1,224 @@ +'use client'; + +import { useEffect, useState } from 'react'; +import { useParams, usePathname } from 'next/navigation'; +import { Cpu } from 'lucide-react'; +import MachineKeepAliveHost from '@/components/layout/middle-content/MachineKeepAliveHost'; +import { useAuth } from '@/hooks/useAuth'; +import { useDriveMachines } from '@/hooks/useDriveMachines'; +import { useMachineWorkspaceStore } from '@/stores/machine-workspace/useMachineWorkspaceStore'; +import { usePendingSessionStore } from '@/stores/development/usePendingSessionStore'; +import { parseSelectedMachineId } from '@/lib/development/development-route'; +import { resolvePendingSession } from '@/lib/development/pending-session'; + +/** + * The Development surface's detail region. + * + * Machines are rendered by {@link MachineKeepAliveHost}, NOT by the + * `[machineId]` route — exactly as the drive view does it (see `CenterPanel`, + * which likewise refuses to render `MachineView` inline). The route segment + * remounts on every machine-to-machine navigation, so a `MachineView` rendered + * from it would tear its xterm buffer, its socket, and its workspace down each + * time you clicked another machine — on the one surface whose entire purpose is + * keeping terminals alive. The host instead keeps a bounded LRU of machines + * mounted and CSS-hides the inactive ones, so switching machines is instant and + * the sessions you left running are still running. + * + * This layout sits ABOVE the `[machineId]` segment, so it (and the host, and + * every warm machine) survives that navigation. + */ +export default function DevelopmentLayout({ children }: { children: React.ReactNode }) { + const params = useParams(); + const pathname = usePathname() ?? ''; + const driveIdParams = params.driveId; + const driveId = Array.isArray(driveIdParams) ? driveIdParams[0] : driveIdParams; + const selectedMachineId = parseSelectedMachineId(pathname, driveId); + + const { user, isLoading: authLoading } = useAuth(); + const isAdmin = user?.role === 'admin'; + + // Same SWR key (and same admin gate) as the sidebar, so this is a cache read + // rather than a second request — and a non-admin still fires none. It is also + // the host's source of truth for what counts as a machine (its `machineIds` + // prop): the surface must not disagree with itself about which machines exist. + const { machines, isLoading, error } = useDriveMachines(isAdmin ? driveId ?? null : null); + const stickyMachineIds = useStickyMachineIds(machines, driveId); + + // Two different questions, two different answers — conflating them is what made + // an earlier version both kill live terminals AND never report a deleted one. + // + // "Does this machine still exist?" is answered by the LATEST fetch: a machine + // that's gone must stop being shown, or "Machine not found" is unreachable. + // "Which machines may stay mounted?" is answered by the STICKY set, because a + // machine can drop out of a fetch without being deleted (see below) and + // unmounting it would disconnect a running terminal. + // + // So a machine that vanishes stops being *displayed* immediately, while its + // terminal stays warm (hidden) until the LRU ages it out. A fetch blip + // therefore costs the user a notice, never a dead session — and the notice IS + // transient, because `useDriveMachines` polls (without that, a single blip + // would hide a live machine for the rest of the session, and both ways out — + // reload, or leave and return — unmount this host and kill every warm + // terminal). + const isKnownMachine = selectedMachineId !== null && machines.some((m) => m.id === selectedMachineId); + // What the host actually DISPLAYS — not merely what the URL selects. The drain + // must gate on this same value: opening a session into a machine the host is + // keeping hidden would mount an xterm inside a `display:none` container, where + // `fit()` measures a zero-sized box and the PTY is created at a bogus geometry + // — wrapping its output for the life of the session. + const displayedMachineId = isKnownMachine ? selectedMachineId : null; + + useDrainPendingSession(displayedMachineId); + + return ( +
+ {children} + + {selectedMachineId && ( + + )} + + +
+ ); +} + +/** + * Every machine id seen in this drive — the set the host is allowed to keep + * MOUNTED. Add-only, and only within a drive. + * + * The host unmounts (and so DISCONNECTS) anything missing from this set. But a + * machine can drop out of `/api/machines` without having been deleted: the + * per-page permission check swallows DB errors and reports "cannot view", so a + * transient hiccup silently omits a live machine. Evicting on that would kill a + * running terminal — the exact failure this list was introduced to prevent. + * + * Being add-only doesn't strand a deleted machine on screen: what is DISPLAYED + * is decided by the latest fetch (see `isKnownMachine`), so a deleted machine + * stops being shown at once and merely lingers, hidden, until the bounded LRU + * ages it out. Changing drive resets the set — that eviction is deliberate, since + * a PTY stream must not outlive its drive context. + */ +function useStickyMachineIds(machines: { id: string }[], driveId: string | undefined): string[] { + // Keyed on the fetched ids (not the array identity — SWR hands back a fresh + // array on every revalidation) and the drive. Derived with the same + // "adjust state during render" pattern MachineKeepAliveHost uses for its LRU: + // the key guards the re-render, and state (unlike a ref) is discarded if a + // concurrent render is abandoned, so an interrupted navigation can't leave a + // machine set behind that was never committed. + const key = `${driveId ?? ''}\u0000${machines.map((machine) => machine.id).join('|')}`; + const [sticky, setSticky] = useState<{ key: string; driveId: string | undefined; ids: string[] }>({ + key: '', + driveId, + ids: [], + }); + + if (sticky.key !== key) { + const ids = sticky.driveId === driveId ? [...sticky.ids] : []; + for (const machine of machines) { + if (!ids.includes(machine.id)) ids.push(machine.id); + } + setSticky({ key, driveId, ids }); + return ids; + } + + return sticky.ids; +} + +/** + * What the detail pane shows when the machine itself can't be. Rendered UNDER + * the keep-alive host (which is `absolute inset-0 z-10` and opaque), so a state + * here is covered the moment the machine actually mounts. Without it, every one + * of these cases is an unexplained blank region — the route renders null and the + * host declines to mount. + */ +function DetailState({ + authLoading, + isAdmin, + isLoading, + error, + isKnownMachine, +}: { + authLoading: boolean; + isAdmin: boolean; + isLoading: boolean; + error: Error | undefined; + isKnownMachine: boolean; +}) { + // `role` isn't persisted across a reload, so on every cold load it is briefly + // unknown. Refusing the user in that window would flash "you're not an admin" + // at an admin refreshing the page — the same gate the sidebar applies. + if (authLoading) return ; + if (!isAdmin) return ; + // Ahead of "not found", because a failed fetch leaves `machines` empty with + // isLoading false — indistinguishable from "this machine doesn't exist" unless + // the error is checked first. But only when the machine ISN'T known: the list + // polls, and SWR keeps the last good data while setting `error` on a failed + // revalidation, so a blip must not blank out a machine we can still show. + if (error && !isKnownMachine) { + return ( + + ); + } + if (isLoading) return ; + if (!isKnownMachine) { + return ( + + ); + } + // The machine exists and the host is mounting it — it will paint over this. + return ; +} + +function DetailNotice({ title, description }: { title: string; description?: string }) { + return ( +
+ +
+

{title}

+ {description &&

{description}

} +
+
+ ); +} + +/** + * Honours a session the user clicked in the sidebar, once the machine it belongs + * to actually has a workspace to open it into. + * + * The decision is the pure `resolvePendingSession`; this is the plumbing. It + * re-evaluates whenever the workspace changes, so an intent applied against a + * workspace that is then torn down and rebuilt (a remount — StrictMode's + * double-invoke does this on first mount) re-applies to the new one instead of + * being silently lost. + * + * Leaving the surface drops any unconverged intent: the store is a module + * singleton, so an intent left behind here would otherwise still be sitting + * there on the user's next visit, ready to fire into whatever pane was active. + */ +function useDrainPendingSession(displayedMachineId: string | null) { + const pending = usePendingSessionStore((state) => state.pending); + const clearPending = usePendingSessionStore((state) => state.clearPending); + const openTerminal = useMachineWorkspaceStore((state) => state.openTerminal); + const workspace = useMachineWorkspaceStore((state) => + pending ? state.workspaces[pending.machineId] : undefined, + ); + + useEffect(() => { + const action = resolvePendingSession(pending, displayedMachineId, workspace); + if (action.type === 'open') openTerminal(action.machineId, action.scope); + // A 'clear' with no pending intent is a no-op, so this cannot loop. + else if (action.type === 'clear') clearPending(); + }, [pending, displayedMachineId, workspace, openTerminal, clearPending]); + + useEffect(() => () => clearPending(), [clearPending]); +} diff --git a/apps/web/src/app/dashboard/[driveId]/development/page.tsx b/apps/web/src/app/dashboard/[driveId]/development/page.tsx new file mode 100644 index 0000000000..015a1d1b7f --- /dev/null +++ b/apps/web/src/app/dashboard/[driveId]/development/page.tsx @@ -0,0 +1,20 @@ +import { SquareTerminal } from 'lucide-react'; + +/** + * The Development surface with no machine selected. The sidebar (the surface's + * actual content — the aggregated machine tree) lives above this route in + * `MemoizedSidebar`, so this is only the detail pane's resting state. + */ +export default function DevelopmentPage() { + return ( +
+ +
+

Select a machine

+

+ Pick a machine from the sidebar to open its terminals, code, and diffs. +

+
+
+ ); +} diff --git a/apps/web/src/app/dashboard/development/page.tsx b/apps/web/src/app/dashboard/development/page.tsx new file mode 100644 index 0000000000..6df351965c --- /dev/null +++ b/apps/web/src/app/dashboard/development/page.tsx @@ -0,0 +1,60 @@ +'use client'; + +import { useEffect, useState } from 'react'; +import { useRouter } from 'next/navigation'; +import { useDriveStore } from '@/hooks/useDrive'; +import { resolveActiveDriveId } from '@/lib/development/resolve-active-drive'; + +/** + * The driveless entry to the Development surface. NOT a second implementation of + * the surface — it resolves the active drive and forwards to the one real route + * tree at `/dashboard/[driveId]/development`, so there is exactly one page + * component per view (deliberately unlike Channels/Tasks/Calendar, which each + * ship a driveless `?driveId=` twin of their drive-scoped page). + * + * A client redirect rather than the server's `redirect()`: "the drive you were + * last in" is `currentDriveId` in the persisted (localStorage) drive store, so + * the server has nothing to resolve it from. + * + * `currentDriveId` is read ONCE, at first render. DriveSwitcher clears it on + * mount whenever the URL names no drive — which this route, by definition, does + * not — so reading it in an effect would race that clear and lose the very + * answer we came for. + */ +export default function DevelopmentRedirectPage() { + const router = useRouter(); + const drives = useDriveStore((state) => state.drives); + const fetchDrives = useDriveStore((state) => state.fetchDrives); + const [lastVisitedDriveId] = useState(() => useDriveStore.getState().currentDriveId); + const [drivesSettled, setDrivesSettled] = useState(false); + + // Gate the redirect on the fetch SETTLING, not on `isLoading` being false: + // with a cold store, `isLoading` is still false on the first render (the fetch + // hasn't started), and `drives` is still [] — so redirecting on that render + // would send anyone with an empty or expired cache to the drive picker even + // though they have drives. `fetchDrives` no-ops on a warm cache, so this costs + // a fresh session nothing. + useEffect(() => { + let cancelled = false; + void fetchDrives().finally(() => { + if (!cancelled) setDrivesSettled(true); + }); + return () => { + cancelled = true; + }; + }, [fetchDrives]); + + useEffect(() => { + if (!drivesSettled) return; + const driveId = resolveActiveDriveId(drives, lastVisitedDriveId); + // No drive to develop in — the drive picker is the only useful destination. + router.replace(driveId ? `/dashboard/${driveId}/development` : '/dashboard/drives'); + }, [drives, drivesSettled, lastVisitedDriveId, router]); + + return ( +
+
+ Opening Development… +
+ ); +} diff --git a/apps/web/src/components/layout/left-sidebar/DevelopmentSidebar.tsx b/apps/web/src/components/layout/left-sidebar/DevelopmentSidebar.tsx new file mode 100644 index 0000000000..b5e318d1cd --- /dev/null +++ b/apps/web/src/components/layout/left-sidebar/DevelopmentSidebar.tsx @@ -0,0 +1,250 @@ +'use client'; + +import { useCallback, useEffect, useState } from 'react'; +import { useParams, usePathname, useRouter } from 'next/navigation'; + +import { ScrollArea } from '@/components/ui/scroll-area'; +import { cn, isElectron } from '@/lib/utils'; +import type { SidebarProps } from './index'; +import DriveSwitcher from '@/components/layout/navbar/DriveSwitcher'; +import DashboardFooter from './DashboardFooter'; +import DriveFooter from './DriveFooter'; +import PrimaryNavigation from './PrimaryNavigation'; +import { useAuth } from '@/hooks/useAuth'; +import { useBreakpoint } from '@/hooks/useBreakpoint'; +import { useLayoutStore } from '@/stores/useLayoutStore'; +import { useDriveStore } from '@/hooks/useDrive'; +import { canManageDrive } from '@/hooks/usePermissions'; +import { useDriveMachines, type DriveMachine } from '@/hooks/useDriveMachines'; +import { usePendingSessionStore } from '@/stores/development/usePendingSessionStore'; +import { useMachineTabStore } from '@/stores/machine-workspace/useMachineTabStore'; +import { parseSelectedMachineId } from '@/lib/development/development-route'; +import type { OpenTerminalScope } from '@/stores/machine-workspace/useMachineWorkspaceStore'; +import MachineTree, { type MachineTreeNode } from '@/components/layout/middle-content/page-views/machine/workspace/MachineTree'; +import SessionLeaves from '@/components/layout/middle-content/page-views/machine/workspace/SessionLeaves'; + +/** + * The Development surface's left sidebar: every Machine in the drive, each + * expanding into the SAME `MachineTree` the Machine page's Terminal tab uses, + * with the SAME session leaves hanging off its nodes. The aggregation is the + * only new part — the tree below each machine is the existing component. + * + * It sits above the routed detail pane, so clicking through machines swaps only + * the pane and the tree keeps its expansion state. The terminals themselves + * survive because the detail region renders machines through + * `MachineKeepAliveHost` (see this surface's layout) rather than from the route + * segment — which remounts, and would otherwise tear down the xterm buffer and + * socket on every machine switch. + */ +export default function DevelopmentSidebar({ className }: SidebarProps) { + const params = useParams(); + const [isElectronMac, setIsElectronMac] = useState(false); + const isSheetBreakpoint = useBreakpoint('(max-width: 1023px)'); + + const driveIdParams = params.driveId; + const driveId = Array.isArray(driveIdParams) ? driveIdParams[0] : driveIdParams; + + const drives = useDriveStore((state) => state.drives); + const drive = drives.find((d) => d.id === driveId); + const canManage = canManageDrive(drive); + + const { user, isLoading: authLoading } = useAuth(); + const isAdmin = user?.role === 'admin'; + + // The same gate MachineView applies, moved one level earlier: a non-admin who + // can VIEW a Machine page must not be able to enumerate the drive's machines — + // nor have this tree fetch their projects/branches/sessions on their behalf, + // which is structure the Machine page itself withholds from them. Passing a + // null driveId is what keeps those requests from ever being made. + const { machines, isLoading, error } = useDriveMachines(isAdmin ? driveId ?? null : null); + const pathname = usePathname() ?? ''; + const selectedMachineId = parseSelectedMachineId(pathname, driveId); + + useEffect(() => { + setIsElectronMac(isElectron() && /Mac/.test(navigator.platform)); + }, []); + + return ( + + ); +} + +/** A resting state of the machine list: one line of muted text, no tree. */ +function ListNotice({ children }: { children: string }) { + return
{children}
; +} + +/** + * The list body. Its states are mutually exclusive, so they're early returns + * rather than a stack of `&&` guards each having to re-state every earlier + * condition's negation. + */ +function MachineList({ + authLoading, + isAdmin, + driveId, + machines, + isLoading, + error, + selectedMachineId, + isSheetBreakpoint, +}: { + authLoading: boolean; + isAdmin: boolean; + driveId: string | undefined; + machines: DriveMachine[]; + isLoading: boolean; + error: Error | undefined; + selectedMachineId: string | null; + isSheetBreakpoint: boolean; +}) { + // Until auth resolves, `role` is simply unknown — saying "you're not an admin" + // then would flash the refusal at an admin on every cold load. + if (authLoading) return Loading…; + // Same wording MachineView uses, so the surface and the page refuse a + // non-admin identically. + if (!isAdmin) return Machine access requires administrator privileges; + // The driveless entry redirects, so a missing driveId is the redirect in + // flight — not a state the user can sit in. + if (!driveId) return Opening Development…; + // Only when the failure left us with NOTHING to show. The list polls, and SWR + // keeps the last good data while setting `error` on a failed revalidation — so + // reporting the error ahead of the data would let one blip of a background poll + // tear down the whole tree (losing every expansion and its session leaves) while + // the app still holds a perfectly good list. + if (error && machines.length === 0) return Failed to load machines; + if (isLoading) return Loading…; + if (machines.length === 0) return No machines in this drive yet; + + return ( + <> + {machines.map((machine) => ( + + ))} + + ); +} + +const MACHINE_NODE: MachineTreeNode = { level: 'machine' }; + +/** Only the machine row addresses a URL, so only it is selectable. */ +const isMachineNode = (node: MachineTreeNode) => node.level === 'machine'; + +/** + * One machine in the aggregated tree. Selecting the machine row routes to its + * detail pane; the projects/branches below it are the shared `MachineTree`, and + * are NOT selectable — only the machine row addresses a URL, so making the other + * rows "selectable" would hand them a click action that goes nowhere (and would + * cost them their expand-on-label-click affordance). + */ +function MachineTreeSection({ + driveId, + machineId, + title, + selected, + isSheetBreakpoint, +}: { + driveId: string; + machineId: string; + title: string; + selected: boolean; + /** Passed down rather than re-derived: one matchMedia listener for the sidebar, not one per machine. */ + isSheetBreakpoint: boolean; +}) { + const router = useRouter(); + const setLeftSheetOpen = useLayoutStore((state) => state.setLeftSheetOpen); + const requestSession = usePendingSessionStore((state) => state.requestSession); + const clearPending = usePendingSessionStore((state) => state.clearPending); + const focusTerminal = useMachineTabStore((state) => state.focusTerminal); + + const navigateToMachine = useCallback(() => { + router.push(`/dashboard/${driveId}/development/${machineId}`); + if (isSheetBreakpoint) setLeftSheetOpen(false); + }, [router, driveId, machineId, isSheetBreakpoint, setLeftSheetOpen]); + + const openMachine = useCallback(() => { + // Picking the machine itself (not one of its sessions) says the user wants + // this machine as it is — so an older, still-unconverged session intent must + // not follow them here and take over the pane. + clearPending(); + navigateToMachine(); + }, [clearPending, navigateToMachine]); + + const onOpenTerminal = useCallback( + (scope: OpenTerminalScope) => { + // Bring the machine's Terminal tab forward FIRST. Only that tab mounts the + // machine's workspace, so on a machine parked on Code/Diff/Settings the + // session would otherwise have nowhere to land — the click would do + // nothing at all. + focusTerminal(machineId); + // Then record the intent and navigate; the surface's layout opens the + // session once that machine's pane region exists. Writing the pane straight + // into the workspace store from here would not survive — MachineWorkspace + // disposes its workspace on unmount and rebuilds it on mount, destroying + // anything authored ahead of it. + requestSession(machineId, scope); + navigateToMachine(); + }, + [focusTerminal, requestSession, machineId, navigateToMachine], + ); + + const renderNodeChildren = useCallback( + (node: MachineTreeNode) => ( + + ), + [machineId, onOpenTerminal], + ); + + return ( + + ); +} diff --git a/apps/web/src/components/layout/left-sidebar/MemoizedSidebar.tsx b/apps/web/src/components/layout/left-sidebar/MemoizedSidebar.tsx index 1bb84238ad..2243f7e351 100644 --- a/apps/web/src/components/layout/left-sidebar/MemoizedSidebar.tsx +++ b/apps/web/src/components/layout/left-sidebar/MemoizedSidebar.tsx @@ -5,28 +5,30 @@ import { usePathname } from 'next/navigation'; import Sidebar, { type SidebarProps } from './index'; import DMSidebar from './DMSidebar'; import ChannelsSidebar from './ChannelsSidebar'; - -const DMS_PATH = /^\/dashboard\/dms(\/|$)/; -const CHANNELS_PATH = /^\/dashboard\/channels(\/|$)/; -const DRIVE_CHANNELS_PATH = /^\/dashboard\/[^/]+\/channels(\/|$)/; +import DevelopmentSidebar from './DevelopmentSidebar'; +import { resolveSidebarVariant } from './sidebar-routes'; /** * Memoized version of Sidebar to prevent unnecessary re-renders. * Each top-level nav item gets its own sidebar feed so the list is * always visible (mirrors PageTree always-on behavior in drive view). + * + * Which feed a pathname gets is decided by `resolveSidebarVariant` — this + * component only maps that answer to a component. */ const MemoizedSidebar = memo((props: SidebarProps) => { const pathname = usePathname() ?? ''; - if (DMS_PATH.test(pathname)) { - return ; - } - - if (CHANNELS_PATH.test(pathname) || DRIVE_CHANNELS_PATH.test(pathname)) { - return ; + switch (resolveSidebarVariant(pathname)) { + case 'dms': + return ; + case 'channels': + return ; + case 'development': + return ; + case 'default': + return ; } - - return ; }); MemoizedSidebar.displayName = 'MemoizedSidebar'; diff --git a/apps/web/src/components/layout/left-sidebar/PrimaryNavigation.tsx b/apps/web/src/components/layout/left-sidebar/PrimaryNavigation.tsx index 0a55cbbd1f..0e1483a3f8 100644 --- a/apps/web/src/components/layout/left-sidebar/PrimaryNavigation.tsx +++ b/apps/web/src/components/layout/left-sidebar/PrimaryNavigation.tsx @@ -2,9 +2,10 @@ import Link from "next/link"; import { usePathname } from "next/navigation"; -import { Calendar, CheckSquare, Folder, Hash, Home, MessageSquare } from "lucide-react"; +import { Calendar, CheckSquare, Folder, Hash, Home, MessageSquare, SquareTerminal } from "lucide-react"; import { cn } from "@/lib/utils"; +import { useAuth } from "@/hooks/useAuth"; import { useLayoutStore } from "@/stores/useLayoutStore"; import { useBreakpoint } from "@/hooks/useBreakpoint"; import { useSidebarBadges } from "@/hooks/useSidebarBadges"; @@ -18,6 +19,11 @@ export default function PrimaryNavigation({ driveId }: PrimaryNavigationProps) { const isSheetBreakpoint = useBreakpoint("(max-width: 1023px)"); const setLeftSheetOpen = useLayoutStore((state) => state.setLeftSheetOpen); const badges = useSidebarBadges(); + const { user } = useAuth(); + // Machines are an app-admin feature end to end (only an admin can create a + // MACHINE page, and MachineView mounts no tabs for anyone else), so a + // non-admin gets no nav entry rather than a destination that refuses them. + const isAdmin = user?.role === "admin"; const navigation = [ { @@ -63,6 +69,17 @@ export default function PrimaryNavigation({ driveId }: PrimaryNavigationProps) { exact: false, badge: badges.calendar, }, + // Driveless href hits a redirect, not a second implementation of the + // surface — the drive always ends up in the path. + ...(isAdmin + ? [{ + name: "Development", + href: driveId ? `/dashboard/${driveId}/development` : "/dashboard/development", + icon: SquareTerminal, + exact: false, + badge: 0, + }] + : []), ]; const handleLinkClick = () => { diff --git a/apps/web/src/components/layout/left-sidebar/__tests__/DevelopmentSidebar.test.tsx b/apps/web/src/components/layout/left-sidebar/__tests__/DevelopmentSidebar.test.tsx new file mode 100644 index 0000000000..b0cac1e618 --- /dev/null +++ b/apps/web/src/components/layout/left-sidebar/__tests__/DevelopmentSidebar.test.tsx @@ -0,0 +1,173 @@ +/** + * The Development sidebar's wiring: who may see the machine list, and what a + * session click actually does. + * + * The session-click path is the one worth pinning down — it was broken twice in + * review. It has to do three things in concert: bring the machine's Terminal tab + * forward (only that tab mounts a workspace, so otherwise the session has + * nowhere to land), record the intent (the pane region isn't mounted yet, so it + * cannot be applied now), and navigate. + */ +import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +const mockPush = vi.fn(); +const mockUseAuth = vi.fn(); + +vi.mock('next/navigation', () => ({ + useParams: () => ({ driveId: 'drive-1' }), + usePathname: () => '/dashboard/drive-1/development', + useRouter: () => ({ push: mockPush }), +})); + +vi.mock('@/hooks/useAuth', () => ({ useAuth: () => mockUseAuth() })); + +// Spied, not just stubbed: a null driveId is HOW the sidebar refuses to fetch for +// a non-admin, so the argument is the security property — asserting only that no +// machine renders would still pass if the gate were dropped, since the refusal +// notice short-circuits the list anyway. +interface DriveMachinesResult { + machines: { id: string; title: string; updatedAt: string }[]; + isLoading: boolean; + error: Error | undefined; + mutate: () => void; +} + +const mockUseDriveMachines = vi.fn( + (driveId: string | null): DriveMachinesResult => ({ + machines: driveId ? [{ id: 'machine-1', title: 'Dev box', updatedAt: '2026-07-12T00:00:00.000Z' }] : [], + isLoading: false, + error: undefined, + mutate: vi.fn(), + }), +); + +vi.mock('@/hooks/useDriveMachines', () => ({ + useDriveMachines: (driveId: string | null) => mockUseDriveMachines(driveId), +})); + +vi.mock('@/hooks/useMachineProjects', () => ({ + useMachineProjects: () => ({ projects: [], isLoading: false, addProject: vi.fn(), removeProject: vi.fn() }), +})); +vi.mock('@/hooks/useMachineBranches', () => ({ + useMachineBranches: () => ({ branches: [], isLoading: false, addBranch: vi.fn(), removeBranch: vi.fn() }), +})); +vi.mock('@/hooks/useAgentTerminals', () => ({ + useAgentTerminals: (machineId: string | null) => ({ + agentTerminals: machineId ? [{ name: 'agent-1', agentType: 'claude', createdAt: '2026-07-12' }] : [], + isLoading: false, + addAgentTerminal: vi.fn(), + removeAgentTerminal: vi.fn(), + }), +})); +vi.mock('@/hooks/useGithubRepos', () => ({ + useGithubRepos: () => ({ repos: [], connected: true, isLoading: false, error: undefined, mutate: vi.fn() }), +})); +vi.mock('@/hooks/useIntegrations', () => ({ useProviders: () => ({ providers: [] }) })); + +// Sidebar chrome that isn't under test. +vi.mock('@/components/layout/navbar/DriveSwitcher', () => ({ default: () =>
})); +vi.mock('../PrimaryNavigation', () => ({ default: () =>
})); +vi.mock('../DriveFooter', () => ({ default: () =>
})); +vi.mock('../DashboardFooter', () => ({ default: () =>
})); + +import DevelopmentSidebar from '../DevelopmentSidebar'; +import { usePendingSessionStore } from '@/stores/development/usePendingSessionStore'; +import { useMachineTabStore } from '@/stores/machine-workspace/useMachineTabStore'; + +beforeEach(() => { + vi.clearAllMocks(); + usePendingSessionStore.setState({ pending: null }); + useMachineTabStore.setState({ tabs: {} }); + mockUseAuth.mockReturnValue({ user: { role: 'admin' }, isLoading: false }); +}); + +describe('DevelopmentSidebar', () => { + test('lists the drive\'s machines for an admin', async () => { + render(); + + expect(await screen.findByText('Dev box')).toBeDefined(); + }); + + test('refuses a non-admin, and asks the API for no machines on their behalf', () => { + mockUseAuth.mockReturnValue({ user: { role: 'user' }, isLoading: false }); + + render(); + + expect(screen.getByText(/administrator privileges/i)).toBeDefined(); + expect(screen.queryByText('Dev box')).toBeNull(); + // The load-bearing half: the request is never made, rather than made and + // discarded. (The server rejects a non-admin too — this is the client half.) + expect(mockUseDriveMachines).toHaveBeenCalledWith(null); + expect(mockUseDriveMachines).not.toHaveBeenCalledWith('drive-1'); + }); + + test('an admin does fetch the drive\'s machines', () => { + render(); + + expect(mockUseDriveMachines).toHaveBeenCalledWith('drive-1'); + }); + + test('a failed POLL keeps the tree — it does not replace it with an error', () => { + // The list polls, and SWR keeps the last good data while setting `error` on a + // failed revalidation. Reporting the error ahead of the data would let one + // blip tear down every MachineTree, losing its expansion state and the + // session leaves under it, while the app still holds a good list. + mockUseDriveMachines.mockReturnValueOnce({ + machines: [{ id: 'machine-1', title: 'Dev box', updatedAt: '2026-07-12T00:00:00.000Z' }], + isLoading: false, + error: new Error('blip'), + mutate: vi.fn(), + }); + + render(); + + expect(screen.getByText('Dev box')).toBeDefined(); + expect(screen.queryByText(/failed to load machines/i)).toBeNull(); + }); + + test('says nothing about admin rights until auth has resolved', () => { + // `role` isn't persisted across a reload, so an early refusal would flash at + // a real admin on every cold load. + mockUseAuth.mockReturnValue({ user: undefined, isLoading: true }); + + render(); + + expect(screen.queryByText(/administrator privileges/i)).toBeNull(); + }); + + test('clicking a session focuses the machine\'s Terminal tab, records the intent, and navigates', async () => { + const user = userEvent.setup(); + // The machine is parked on another tab — the case where the click used to do + // nothing at all, because only the Terminal tab mounts a workspace. + useMachineTabStore.getState().setTab('machine-1', 'code'); + render(); + + // Expand the machine to reveal its session leaves. + await user.click(await screen.findByRole('button', { name: 'Expand' })); + await user.click(await screen.findByText('agent-1')); + + expect(useMachineTabStore.getState().tabs['machine-1']).toBe('terminal'); + expect(usePendingSessionStore.getState().pending).toMatchObject({ + machineId: 'machine-1', + scope: { name: 'agent-1' }, + }); + expect(mockPush).toHaveBeenCalledWith('/dashboard/drive-1/development/machine-1'); + }); + + test('clicking the machine itself drops a stale session intent', async () => { + // Picking the machine (not one of its sessions) says "this machine as it is" + // — an older intent must not follow the user here and take over the pane. + const user = userEvent.setup(); + usePendingSessionStore.setState({ + pending: { machineId: 'machine-1', scope: { name: 'old-session' } }, + }); + render(); + + await user.click(await screen.findByText('Dev box')); + + expect(usePendingSessionStore.getState().pending).toBeNull(); + expect(mockPush).toHaveBeenCalledWith('/dashboard/drive-1/development/machine-1'); + }); +}); diff --git a/apps/web/src/components/layout/left-sidebar/__tests__/sidebar-routes.test.ts b/apps/web/src/components/layout/left-sidebar/__tests__/sidebar-routes.test.ts new file mode 100644 index 0000000000..3bce5ad353 --- /dev/null +++ b/apps/web/src/components/layout/left-sidebar/__tests__/sidebar-routes.test.ts @@ -0,0 +1,35 @@ +import { describe, test, expect } from 'vitest'; +import { resolveSidebarVariant } from '../sidebar-routes'; + +describe('resolveSidebarVariant', () => { + test('routes the drive-scoped Development tree to the Development sidebar', () => { + expect(resolveSidebarVariant('/dashboard/drive-1/development')).toBe('development'); + }); + + test('keeps the Development sidebar when a machine is selected', () => { + // The whole point of the sidebar living above the routed page: picking a + // machine swaps the detail pane, not the sidebar. + expect(resolveSidebarVariant('/dashboard/drive-1/development/machine-1')).toBe('development'); + }); + + test('the driveless Development entry resolves from the same matcher', () => { + expect(resolveSidebarVariant('/dashboard/development')).toBe('development'); + }); + + test("does not swallow a drive's ordinary page route", () => { + expect(resolveSidebarVariant('/dashboard/drive-1/page-1')).toBe('default'); + }); + + test('does not match a path that merely starts with the segment', () => { + expect(resolveSidebarVariant('/dashboard/drive-1/development-notes')).toBe('default'); + expect(resolveSidebarVariant('/dashboard/developments')).toBe('default'); + }); + + test('leaves the existing swaps alone', () => { + expect(resolveSidebarVariant('/dashboard/dms')).toBe('dms'); + expect(resolveSidebarVariant('/dashboard/dms/thread-1')).toBe('dms'); + expect(resolveSidebarVariant('/dashboard/channels')).toBe('channels'); + expect(resolveSidebarVariant('/dashboard/drive-1/channels')).toBe('channels'); + expect(resolveSidebarVariant('/dashboard')).toBe('default'); + }); +}); diff --git a/apps/web/src/components/layout/left-sidebar/sidebar-routes.ts b/apps/web/src/components/layout/left-sidebar/sidebar-routes.ts new file mode 100644 index 0000000000..e23beb3235 --- /dev/null +++ b/apps/web/src/components/layout/left-sidebar/sidebar-routes.ts @@ -0,0 +1,32 @@ +/** + * Which sidebar a pathname gets. Each top-level nav destination that swaps the + * left sidebar owns a matcher here; everything else falls through to the drive's + * page tree. + * + * A pure function rather than inline `if`s in `MemoizedSidebar` so the matchers + * — the part with the actual edge cases — are testable without rendering a + * sidebar. + */ + +const DMS_PATH = /^\/dashboard\/dms(\/|$)/; +const CHANNELS_PATH = /^\/dashboard\/channels(\/|$)/; +const DRIVE_CHANNELS_PATH = /^\/dashboard\/[^/]+\/channels(\/|$)/; +/** + * ONE matcher for both Development shapes: the driveless entry + * (`/dashboard/development`, which redirects) and the real drive-scoped tree + * (`/dashboard/{driveId}/development[/{machineId}]`). The surface keeps the + * drive in the path, so unlike Channels it needs no driveless twin. + * + * The optional drive segment is why this is anchored and segment-bounded: a + * drive's ordinary page route (`/dashboard/{driveId}/{pageId}`) must not match. + */ +const DEVELOPMENT_PATH = /^\/dashboard\/(?:[^/]+\/)?development(\/|$)/; + +export type SidebarVariant = 'dms' | 'channels' | 'development' | 'default'; + +export function resolveSidebarVariant(pathname: string): SidebarVariant { + if (DMS_PATH.test(pathname)) return 'dms'; + if (CHANNELS_PATH.test(pathname) || DRIVE_CHANNELS_PATH.test(pathname)) return 'channels'; + if (DEVELOPMENT_PATH.test(pathname)) return 'development'; + return 'default'; +} diff --git a/apps/web/src/components/layout/middle-content/MachineKeepAliveHost.tsx b/apps/web/src/components/layout/middle-content/MachineKeepAliveHost.tsx index 40a2b906dc..f6aae71a11 100644 Binary files a/apps/web/src/components/layout/middle-content/MachineKeepAliveHost.tsx and b/apps/web/src/components/layout/middle-content/MachineKeepAliveHost.tsx differ diff --git a/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.test.tsx b/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.test.tsx index a486a358d5..df7525cd00 100644 --- a/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.test.tsx +++ b/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.test.tsx @@ -48,6 +48,7 @@ vi.mock('motion/react', () => ({ })); import MachineView from './MachineView'; +import { useMachineTabStore } from '@/stores/machine-workspace/useMachineTabStore'; const asAdmin = () => mockUseAuth.mockReturnValue({ user: { role: 'admin' } }); @@ -55,6 +56,9 @@ describe('MachineView (Machine 4-tab shell)', () => { beforeEach(() => { vi.clearAllMocks(); lifecycle.length = 0; + // The active tab now lives in a module-singleton store, so a test that + // switches tabs would otherwise leave the next one parked on that tab. + useMachineTabStore.setState({ tabs: {} }); }); test('mounts only the Terminal tab body on load — not all four', () => { diff --git a/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.tsx b/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.tsx index 53764410bf..9c463954e8 100644 --- a/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.tsx +++ b/apps/web/src/components/layout/middle-content/page-views/machine/MachineView.tsx @@ -6,6 +6,11 @@ import { motion } from 'motion/react'; import { Code2, GitCompare, Settings, TerminalSquare } from 'lucide-react'; import { cn } from '@/lib/utils'; import { useAuth } from '@/hooks/useAuth'; +import { + useMachineTabStore, + DEFAULT_MACHINE_TAB, + type MachineTabValue, +} from '@/stores/machine-workspace/useMachineTabStore'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; import TerminalTab from './tabs/TerminalTab'; import CodeTab from './tabs/CodeTab'; @@ -16,8 +21,6 @@ interface MachineViewProps { pageId: string; } -type MachineTabValue = 'terminal' | 'code' | 'diff' | 'settings'; - const TAB_TRIGGERS: { value: MachineTabValue; label: string; icon: React.ElementType }[] = [ { value: 'terminal', label: 'Terminal', icon: TerminalSquare }, { value: 'code', label: 'Code', icon: Code2 }, @@ -35,10 +38,19 @@ const TAB_TRIGGERS: { value: MachineTabValue; label: string; icon: React.Element * initializes. Terminal is the default. The export name (`MachineView`) and * `{ pageId }` prop shape are preserved so `CenterPanel.tsx` / * `MachineKeepAliveHost.tsx` need no change. + * + * The active tab is held in `useMachineTabStore` rather than by Radix, so that + * "show me this machine's terminal" is something another surface can ask for. + * The Development sidebar needs it: only the Terminal tab mounts a machine's + * workspace, so a session clicked on a machine parked on Code/Diff/Settings had + * nowhere to land. Behaviour is otherwise unchanged — a machine with no stored + * tab shows Terminal, as before. */ const MachineView = ({ pageId }: MachineViewProps) => { const { user } = useAuth(); const isAdmin = user?.role === 'admin'; + const activeTab = useMachineTabStore((state) => state.tabs[pageId] ?? DEFAULT_MACHINE_TAB); + const setTab = useMachineTabStore((state) => state.setTab); return ( { )} {isAdmin && ( - + setTab(pageId, value as MachineTabValue)} + className="flex min-h-0 flex-1 flex-col gap-0" + >
{TAB_TRIGGERS.map(({ value, label, icon: Icon }) => ( diff --git a/apps/web/src/components/layout/middle-content/page-views/machine/tabs/TerminalTab.tsx b/apps/web/src/components/layout/middle-content/page-views/machine/tabs/TerminalTab.tsx index 589b98660e..2ee0e13a80 100644 --- a/apps/web/src/components/layout/middle-content/page-views/machine/tabs/TerminalTab.tsx +++ b/apps/web/src/components/layout/middle-content/page-views/machine/tabs/TerminalTab.tsx @@ -1,37 +1,11 @@ "use client"; -import { useCallback, useState } from 'react'; +import { useCallback } from 'react'; import dynamic from 'next/dynamic'; -import { toast } from 'sonner'; -import { Plus, TerminalSquare } from 'lucide-react'; -import { Button } from '@/components/ui/button'; -import { Input } from '@/components/ui/input'; -import { - Dialog, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle, - DialogTrigger, -} from '@/components/ui/dialog'; -import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, -} from '@/components/ui/select'; -import { useAgentTerminals, type AgentTerminal } from '@/hooks/useAgentTerminals'; -import { AGENT_LAUNCH_SPECS, type AgentRuntimeType } from '@pagespace/lib/services/machines/agent-terminal-types'; import { useMachineWorkspaceStore, type OpenTerminalScope } from '@/stores/machine-workspace/useMachineWorkspaceStore'; import MachineTree, { type MachineTreeNode } from '../workspace/MachineTree'; -import ConfirmRemoveDialog from '../workspace/ConfirmRemoveDialog'; -import RemoveButton from '../workspace/RemoveButton'; +import SessionLeaves from '../workspace/SessionLeaves'; import TabSidebar from './TabSidebar'; -import { SidebarLoading, SidebarNotice } from './tab-states'; - -const AGENT_TYPES = Object.keys(AGENT_LAUNCH_SPECS) as AgentRuntimeType[]; // MachineWorkspace owns the xterm subtree + socket; it must never SSR. const MachineWorkspace = dynamic(() => import('../workspace/MachineWorkspace'), { ssr: false }); @@ -47,7 +21,7 @@ interface TerminalTabProps { * {@link MachineTree} with session-terminal leaves injected under every * Machine/Project/Branch node, beside the pane workspace. This is the new home of * the session navigation that used to live in the right-sidebar Navigator tab — - * clicking a session opens it in the workspace via the shared terminal-workspace + * clicking a session opens it in the workspace via the shared machine-workspace * store, exactly as before. * * Opening a session also `close()`s the sidebar, which on a narrow viewport @@ -90,160 +64,3 @@ function SessionTree({ machineId, onOpened }: { machineId: string; onOpened: () return ; } - -/** Resolves a tree node to the `useAgentTerminals` scope it addresses, and the - * `OpenTerminalScope` a session under it opens with. */ -function useNodeTerminals(machineId: string, node: MachineTreeNode) { - const projectName = node.level === 'machine' ? null : node.projectName; - const branchName = node.level === 'branch' ? node.branchName : null; - const scopeFor = useCallback( - (name: string): OpenTerminalScope => ({ - projectName: projectName ?? undefined, - branchName: branchName ?? undefined, - name, - }), - [projectName, branchName], - ); - return { terminals: useAgentTerminals(machineId, projectName, branchName), scopeFor }; -} - -/** Session-terminal leaves injected by {@link MachineTree}'s `renderNodeChildren` - * under each expanded node — mounts (and thus fetches) only while its node is - * open. */ -function SessionLeaves({ - machineId, - node, - onOpenTerminal, -}: { - machineId: string; - node: MachineTreeNode; - onOpenTerminal(scope: OpenTerminalScope): void; -}) { - const { terminals, scopeFor } = useNodeTerminals(machineId, node); - const { agentTerminals, isLoading, addAgentTerminal, removeAgentTerminal } = terminals; - - return ( - onOpenTerminal(scopeFor(name))} - /> - ); -} - -function TerminalList({ - terminals, - isLoading, - onAdd, - onRemove, - onOpen, -}: { - terminals: AgentTerminal[]; - isLoading: boolean; - onAdd(name: string, agentType: AgentRuntimeType): Promise; - onRemove(name: string): Promise; - onOpen(name: string): void; -}) { - const [pendingRemove, setPendingRemove] = useState(null); - - return ( -
-
- Terminals - -
- {isLoading && } - {!isLoading && terminals.length === 0 && ( - - )} - {terminals.map((terminal) => ( -
- - setPendingRemove(terminal.name)} label={`Remove terminal ${terminal.name}`} /> -
- ))} - !open && setPendingRemove(null)} - title="Remove terminal?" - description={pendingRemove ? `Remove terminal "${pendingRemove}"?` : ''} - onConfirm={() => { - if (pendingRemove === null) return Promise.resolve(); - return onRemove(pendingRemove); - }} - /> -
- ); -} - -function AddAgentTerminalDialog({ onAdd }: { onAdd(name: string, agentType: AgentRuntimeType): Promise }) { - const [open, setOpen] = useState(false); - const [name, setName] = useState(''); - const [agentType, setAgentType] = useState(AGENT_TYPES[0]); - const [submitting, setSubmitting] = useState(false); - - const handleSubmit = async () => { - setSubmitting(true); - try { - await onAdd(name.trim(), agentType); - setOpen(false); - setName(''); - } catch (err) { - toast.error(err instanceof Error ? err.message : 'Failed to add terminal'); - } finally { - setSubmitting(false); - } - }; - - return ( - - - - - - - Add terminal - Named PTY session running a pluggable agent type at this node's scope. - -
- setName(e.target.value)} /> - -
- - - -
-
- ); -} - diff --git a/apps/web/src/components/layout/middle-content/page-views/machine/workspace/MachineTree.tsx b/apps/web/src/components/layout/middle-content/page-views/machine/workspace/MachineTree.tsx index d7418ca92c..8404c17e8f 100644 --- a/apps/web/src/components/layout/middle-content/page-views/machine/workspace/MachineTree.tsx +++ b/apps/web/src/components/layout/middle-content/page-views/machine/workspace/MachineTree.tsx @@ -53,6 +53,20 @@ export type MachineTreeNode = interface MachineTreeProps { machineId: string; + /** + * The machine row's label. Defaults to "Machine" — right on the Machine page, + * where the page's own title is already in the header and there is only one + * machine. The Development surface stacks a tree per machine, so it passes + * each Machine page's title to tell them apart. + */ + machineLabel?: string; + /** + * Whether the machine row starts expanded. Default `true` (the Machine page: + * one tree, and its projects are the point). The Development surface passes + * `false` — its machine rows are collapsed until asked for, so listing N + * machines doesn't fire N project fetches on mount. + */ + defaultExpanded?: boolean; /** Called when a Machine/Project/Branch row is clicked. Omit if the tree itself isn't selectable (e.g. selection lives on injected leaf content instead). */ onSelectNode?: (node: MachineTreeNode) => void; /** @@ -97,11 +111,13 @@ export function isSameMachineTreeNode(a: MachineTreeNode | null | undefined, b: } /** Presentation-only Machine → Project → Branch tree, reusable across any tab that needs this navigation shape (Terminal, Diff, …). Has no opinion on what a row click does — callers own that via `onSelectNode`. */ -export default function MachineTree({ machineId, onSelectNode, isNodeSelectable, selectedNode, renderNodeChildren }: MachineTreeProps) { +export default function MachineTree({ machineId, machineLabel, defaultExpanded, onSelectNode, isNodeSelectable, selectedNode, renderNodeChildren }: MachineTreeProps) { return (
ReactNode; } -function MachineNode({ machineId, onSelectNode, isNodeSelectable, selectedNode, renderNodeChildren }: TreeLevelProps & { machineId: string }) { - const [expanded, setExpanded] = useState(true); +function MachineNode({ + machineId, + machineLabel = 'Machine', + defaultExpanded = true, + onSelectNode, + isNodeSelectable, + selectedNode, + renderNodeChildren, +}: TreeLevelProps & { machineId: string; machineLabel?: string; defaultExpanded?: boolean }) { + const [expanded, setExpanded] = useState(defaultExpanded); const node: MachineTreeNode = { level: 'machine' }; const { projects, isLoading: projectsLoading, addProject, removeProject } = useMachineProjects(expanded ? machineId : null); @@ -210,7 +234,7 @@ function MachineNode({ machineId, onSelectNode, isNodeSelectable, selectedNode, onSelect={selectHandlerFor(node, onSelectNode, isNodeSelectable)} selected={isSameMachineTreeNode(node, selectedNode)} icon={} - label="Machine" + label={machineLabel} labelClassName="font-medium" /> {expanded && ( diff --git a/apps/web/src/components/layout/middle-content/page-views/machine/workspace/SessionLeaves.tsx b/apps/web/src/components/layout/middle-content/page-views/machine/workspace/SessionLeaves.tsx new file mode 100644 index 0000000000..7155c1840f --- /dev/null +++ b/apps/web/src/components/layout/middle-content/page-views/machine/workspace/SessionLeaves.tsx @@ -0,0 +1,195 @@ +"use client"; + +import { useCallback, useState } from 'react'; +import { toast } from 'sonner'; +import { Plus, TerminalSquare } from 'lucide-react'; +import { Button } from '@/components/ui/button'; +import { Input } from '@/components/ui/input'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, + DialogTrigger, +} from '@/components/ui/dialog'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select'; +import { useAgentTerminals, type AgentTerminal } from '@/hooks/useAgentTerminals'; +import { AGENT_LAUNCH_SPECS, type AgentRuntimeType } from '@pagespace/lib/services/machines/agent-terminal-types'; +import type { OpenTerminalScope } from '@/stores/machine-workspace/useMachineWorkspaceStore'; +import type { MachineTreeNode } from './MachineTree'; +import ConfirmRemoveDialog from './ConfirmRemoveDialog'; +import RemoveButton from './RemoveButton'; +import { SidebarLoading, SidebarNotice } from '../tabs/tab-states'; + +const AGENT_TYPES = Object.keys(AGENT_LAUNCH_SPECS) as AgentRuntimeType[]; + +/** Resolves a tree node to the `useAgentTerminals` scope it addresses, and the + * `OpenTerminalScope` a session under it opens with. */ +function useNodeTerminals(machineId: string, node: MachineTreeNode) { + const projectName = node.level === 'machine' ? null : node.projectName; + const branchName = node.level === 'branch' ? node.branchName : null; + const scopeFor = useCallback( + (name: string): OpenTerminalScope => ({ + projectName: projectName ?? undefined, + branchName: branchName ?? undefined, + name, + }), + [projectName, branchName], + ); + return { terminals: useAgentTerminals(machineId, projectName, branchName), scopeFor }; +} + +/** + * Session-terminal leaves injected by {@link MachineTree}'s `renderNodeChildren` + * under each expanded node — mounts (and thus fetches) only while its node is + * open. + * + * Shared by the Machine page's Terminal tab and the Development surface's + * sidebar: both hang the same sessions off the same tree, and differ only in + * what `onOpenTerminal` does (open a pane here; route to the machine first, + * there). + */ +export default function SessionLeaves({ + machineId, + node, + onOpenTerminal, +}: { + machineId: string; + node: MachineTreeNode; + onOpenTerminal(scope: OpenTerminalScope): void; +}) { + const { terminals, scopeFor } = useNodeTerminals(machineId, node); + const { agentTerminals, isLoading, addAgentTerminal, removeAgentTerminal } = terminals; + + return ( + onOpenTerminal(scopeFor(name))} + /> + ); +} + +function TerminalList({ + terminals, + isLoading, + onAdd, + onRemove, + onOpen, +}: { + terminals: AgentTerminal[]; + isLoading: boolean; + onAdd(name: string, agentType: AgentRuntimeType): Promise; + onRemove(name: string): Promise; + onOpen(name: string): void; +}) { + const [pendingRemove, setPendingRemove] = useState(null); + + return ( +
+
+ Terminals + +
+ {isLoading && } + {!isLoading && terminals.length === 0 && ( + + )} + {terminals.map((terminal) => ( +
+ + setPendingRemove(terminal.name)} label={`Remove terminal ${terminal.name}`} /> +
+ ))} + !open && setPendingRemove(null)} + title="Remove terminal?" + description={pendingRemove ? `Remove terminal "${pendingRemove}"?` : ''} + onConfirm={() => { + if (pendingRemove === null) return Promise.resolve(); + return onRemove(pendingRemove); + }} + /> +
+ ); +} + +function AddAgentTerminalDialog({ onAdd }: { onAdd(name: string, agentType: AgentRuntimeType): Promise }) { + const [open, setOpen] = useState(false); + const [name, setName] = useState(''); + const [agentType, setAgentType] = useState(AGENT_TYPES[0]); + const [submitting, setSubmitting] = useState(false); + + const handleSubmit = async () => { + setSubmitting(true); + try { + await onAdd(name.trim(), agentType); + setOpen(false); + setName(''); + } catch (err) { + toast.error(err instanceof Error ? err.message : 'Failed to add terminal'); + } finally { + setSubmitting(false); + } + }; + + return ( + + + + + + + Add terminal + Named PTY session running a pluggable agent type at this node's scope. + +
+ setName(e.target.value)} /> + +
+ + + +
+
+ ); +} diff --git a/apps/web/src/hooks/useDriveMachines.ts b/apps/web/src/hooks/useDriveMachines.ts new file mode 100644 index 0000000000..0fa8a438c1 --- /dev/null +++ b/apps/web/src/hooks/useDriveMachines.ts @@ -0,0 +1,58 @@ +'use client'; + +import useSWR from 'swr'; +import { fetchWithAuth } from '@/lib/auth/auth-fetch'; + +export interface DriveMachine { + id: string; + title: string; + updatedAt: string; +} + +const fetcher = (url: string) => + fetchWithAuth(url).then(async (res) => { + if (!res.ok) { + const body = await res.json().catch(() => null); + throw new Error(body?.error ?? 'Failed to fetch machines'); + } + return res.json() as Promise<{ machines: DriveMachine[] }>; + }); + +/** + * Every Machine page in a drive — the root tier of the Development surface's + * aggregated Machine → Project → Branch → session tree. Pass a `null` `driveId` + * (the driveless `/dashboard/development` route) to disable fetching. + */ +export function useDriveMachines(driveId: string | null) { + const key = driveId ? `/api/machines?driveId=${encodeURIComponent(driveId)}` : null; + + // This list must be able to RECOVER, which is why it revalidates at all (the + // sibling machine hooks don't). A machine can drop out of it without having + // been deleted — the per-page permission check swallows DB errors and reports + // "cannot view" — and the surface responds by hiding that machine. Fetched + // once and never again, a single blip would hide a live machine for the rest + // of the session, and the only ways out (reload, or leave and return) both + // unmount the keep-alive host and disconnect every warm terminal. + // + // It also means a machine created elsewhere shows up without a reload, and + // (by dropping the previous `revalidateOnFocus: false`) that coming back to the + // tab recovers immediately rather than waiting out the interval. + // + // Cheap: one indexed query on an admin-only surface, and SWR suppresses the + // poll entirely while the tab is hidden. A poll cannot churn the keep-alive LRU + // — note that is NOT because SWR preserves the array's identity (it only does + // that when the whole payload is deep-equal, and `updatedAt` moves whenever a + // Machine page is touched). It's because the consumers key on the IDS alone: + // `useStickyMachineIds` and the host's `validKey` both collapse the list to its + // ids, so a payload that carries new timestamps but the same machines is inert. + const { data, error, isLoading, mutate } = useSWR(key, fetcher, { + refreshInterval: 30_000, + }); + + return { + machines: data?.machines ?? [], + isLoading, + error: error as Error | undefined, + mutate, + }; +} diff --git a/apps/web/src/lib/development/__tests__/development-route.test.ts b/apps/web/src/lib/development/__tests__/development-route.test.ts new file mode 100644 index 0000000000..a4809794b0 --- /dev/null +++ b/apps/web/src/lib/development/__tests__/development-route.test.ts @@ -0,0 +1,25 @@ +import { describe, test, expect } from 'vitest'; +import { parseSelectedMachineId } from '../development-route'; + +describe('parseSelectedMachineId', () => { + test('reads the machine id out of the detail URL', () => { + expect(parseSelectedMachineId('/dashboard/drive-1/development/machine-1', 'drive-1')).toBe('machine-1'); + }); + + test('no machine is selected at the surface root', () => { + expect(parseSelectedMachineId('/dashboard/drive-1/development', 'drive-1')).toBeNull(); + expect(parseSelectedMachineId('/dashboard/drive-1/development/', 'drive-1')).toBeNull(); + }); + + test('ignores a path belonging to a different drive', () => { + expect(parseSelectedMachineId('/dashboard/drive-2/development/machine-1', 'drive-1')).toBeNull(); + }); + + test('takes only the machine segment, not what follows it', () => { + expect(parseSelectedMachineId('/dashboard/drive-1/development/machine-1/extra', 'drive-1')).toBe('machine-1'); + }); + + test('without a drive there is no machine', () => { + expect(parseSelectedMachineId('/dashboard/development', undefined)).toBeNull(); + }); +}); diff --git a/apps/web/src/lib/development/__tests__/pending-session.test.ts b/apps/web/src/lib/development/__tests__/pending-session.test.ts new file mode 100644 index 0000000000..cbba3522fc --- /dev/null +++ b/apps/web/src/lib/development/__tests__/pending-session.test.ts @@ -0,0 +1,65 @@ +import { describe, test, expect } from 'vitest'; +import type { WorkspaceState } from '@/stores/machine-workspace/useMachineWorkspaceStore'; +import { resolvePendingSession, type PendingSession } from '../pending-session'; + +const SCOPE = { projectName: 'repo', branchName: 'main', name: 'agent-1' }; +const PENDING: PendingSession = { machineId: 'machine-1', scope: SCOPE }; + +/** A workspace whose active pane holds `scope` (null = a fresh, empty pane). */ +const workspaceWith = (scope: WorkspaceState['columns'][number]['panes'][number]['scope']): WorkspaceState => ({ + columns: [{ id: 'col-1', panes: [{ id: 'pane-1', scope }] }], + activePaneId: 'pane-1', +}); + +describe('resolvePendingSession', () => { + test('opens the session once the user is on the machine and it has a workspace', () => { + // Also the rebuild case: MachineWorkspace disposes on unmount and re-creates on + // mount (StrictMode double-invokes exactly this on the first visit), so the + // intent must still resolve to `open` against a fresh, empty workspace rather + // than being a one-shot that the rebuild destroys. + expect(resolvePendingSession(PENDING, 'machine-1', workspaceWith(null))).toEqual({ + type: 'open', + machineId: 'machine-1', + scope: SCOPE, + }); + }); + + test('holds while the click\'s own navigation is still in flight', () => { + // The bug this guards, and the reason the surface's headline flow was + // silently dead: the click (a store write) lands in React's SYNC lane, while + // router.push dispatches inside a TRANSITION. React commits the sync update + // first, so there is an intermediate commit holding the new intent and the + // OLD pathname. Reading that as "the user navigated away" threw the intent + // away before the navigation it was waiting for ever arrived. + expect(resolvePendingSession(PENDING, 'machine-9', undefined)).toEqual({ type: 'wait' }); + }); + + test('holds until the machine\'s pane region has mounted', () => { + expect(resolvePendingSession(PENDING, 'machine-1', undefined)).toEqual({ type: 'wait' }); + }); + + test('clears once the session is actually in the active pane', () => { + expect(resolvePendingSession(PENDING, 'machine-1', workspaceWith(SCOPE))).toEqual({ type: 'clear' }); + }); + + test('a satisfied intent is dropped, so it cannot clobber the user\'s next pane change', () => { + // With no intent left, nothing is ever re-applied over the user's own choice. + expect( + resolvePendingSession(null, 'machine-1', workspaceWith({ ...SCOPE, name: 'agent-2' })), + ).toEqual({ type: 'clear' }); + }); + + test('distinguishes same-named sessions at different scopes', () => { + // A machine-scope "agent-1" is not the branch-scope "agent-1"; treating them + // as the same would report the intent satisfied by the wrong session. + expect(resolvePendingSession(PENDING, 'machine-1', workspaceWith({ name: 'agent-1' }))).toEqual({ + type: 'open', + machineId: 'machine-1', + scope: SCOPE, + }); + }); + + test('no intent is a no-op', () => { + expect(resolvePendingSession(null, 'machine-1', undefined)).toEqual({ type: 'clear' }); + }); +}); diff --git a/apps/web/src/lib/development/__tests__/resolve-active-drive.test.ts b/apps/web/src/lib/development/__tests__/resolve-active-drive.test.ts new file mode 100644 index 0000000000..20b9f6145b --- /dev/null +++ b/apps/web/src/lib/development/__tests__/resolve-active-drive.test.ts @@ -0,0 +1,53 @@ +import { describe, test, expect } from 'vitest'; +import type { Drive } from '@pagespace/lib/types'; +import { resolveActiveDriveId } from '../resolve-active-drive'; + +const drive = (id: string, overrides: Partial = {}): Drive => ({ + id, + name: id, + slug: id, + ownerId: 'user-1', + isTrashed: false, + trashedAt: null, + createdAt: '2026-07-11T00:00:00.000Z', + updatedAt: '2026-07-11T00:00:00.000Z', + isOwned: true, + ...overrides, +}); + +describe('resolveActiveDriveId', () => { + test('prefers the drive the user was last in', () => { + const drives = [drive('drive-1'), drive('drive-2')]; + + expect(resolveActiveDriveId(drives, 'drive-2')).toBe('drive-2'); + }); + + test('falls back to the first drive when there is no last-visited one', () => { + const drives = [drive('drive-1'), drive('drive-2')]; + + expect(resolveActiveDriveId(drives, null)).toBe('drive-1'); + }); + + test('falls back to the first drive when the last-visited one is gone', () => { + const drives = [drive('drive-1')]; + + expect(resolveActiveDriveId(drives, 'drive-deleted')).toBe('drive-1'); + }); + + test('never forwards into a trashed drive, even the last-visited one', () => { + const drives = [drive('drive-trashed', { isTrashed: true }), drive('drive-2')]; + + expect(resolveActiveDriveId(drives, 'drive-trashed')).toBe('drive-2'); + }); + + test('skips trashed drives when falling back', () => { + const drives = [drive('drive-trashed', { isTrashed: true }), drive('drive-2')]; + + expect(resolveActiveDriveId(drives, null)).toBe('drive-2'); + }); + + test('resolves to null when there is no drive to go to', () => { + expect(resolveActiveDriveId([], null)).toBeNull(); + expect(resolveActiveDriveId([drive('drive-trashed', { isTrashed: true })], 'drive-trashed')).toBeNull(); + }); +}); diff --git a/apps/web/src/lib/development/development-route.ts b/apps/web/src/lib/development/development-route.ts new file mode 100644 index 0000000000..443b113469 --- /dev/null +++ b/apps/web/src/lib/development/development-route.ts @@ -0,0 +1,13 @@ +/** + * The Development surface's URL shape: `/dashboard/{driveId}/development[/{machineId}]`. + * + * Both the sidebar (to highlight the selected machine) and the surface's layout + * (to tell the keep-alive host which machine is active) need the selected + * machine id, so the parse lives here rather than being written twice. + */ +export function parseSelectedMachineId(pathname: string, driveId: string | undefined): string | null { + if (!driveId) return null; + const prefix = `/dashboard/${driveId}/development/`; + if (!pathname.startsWith(prefix)) return null; + return pathname.slice(prefix.length).split('/')[0] || null; +} diff --git a/apps/web/src/lib/development/pending-session.ts b/apps/web/src/lib/development/pending-session.ts new file mode 100644 index 0000000000..a8de391a0c --- /dev/null +++ b/apps/web/src/lib/development/pending-session.ts @@ -0,0 +1,87 @@ +import type { OpenTerminalScope, WorkspaceState } from '@/stores/machine-workspace/useMachineWorkspaceStore'; + +/** A session the user clicked in the Development sidebar, to be opened on the machine it belongs to. */ +export interface PendingSession { + machineId: string; + scope: OpenTerminalScope; +} + +export type PendingSessionAction = + /** The user isn't on that machine yet, or its pane region isn't there — hold. */ + | { type: 'wait' } + /** Apply the intent (idempotent: `openTerminal` sets the active pane's scope). */ + | { type: 'open'; machineId: string; scope: OpenTerminalScope } + /** Done, superseded, or expired — drop the intent. */ + | { type: 'clear' }; + +function sameScope(a: OpenTerminalScope | null, b: OpenTerminalScope): boolean { + return ( + a !== null && + a.name === b.name && + (a.projectName ?? null) === (b.projectName ?? null) && + (a.branchName ?? null) === (b.branchName ?? null) + ); +} + +function activePaneScope(workspace: WorkspaceState): OpenTerminalScope | null { + for (const column of workspace.columns) { + for (const pane of column.panes) { + if (pane.id === workspace.activePaneId) return pane.scope; + } + } + return null; +} + +/** + * What to do with a sidebar session-click intent, given where the user now is + * and whether the target machine's workspace exists yet. + * + * Two things make this trickier than it looks, and both were live bugs: + * + * 1. The intent CANNOT be applied when it is made. The clicked session may + * belong to a machine whose pane region isn't mounted, and writing a pane + * into the store ahead of that mount does not survive: `MachineWorkspace` + * disposes its workspace on unmount and rebuilds it on mount, destroying + * anything authored early. So the intent is held and re-evaluated as state + * arrives, and it CONVERGES rather than firing once — it keeps asking until + * the workspace reports the session in its active pane. That is what makes it + * survive the workspace being torn down and rebuilt underneath it (React + * StrictMode does exactly this on first mount). Re-applying is harmless + * because `openTerminal` is idempotent. + * + * 2. "The user isn't on that machine yet" and "the user went somewhere else" + * are INDISTINGUISHABLE from a single commit — both are just + * `selectedMachineId !== pending.machineId`. They can't be told apart because + * the click (a store write) lands in React's sync lane while `router.push` + * dispatches in a transition, so there is a commit holding the new intent and + * the old pathname. An earlier version treated that commit as "navigated + * away" and dropped the intent before its own navigation landed — which + * silently broke every session click on a machine the user wasn't already + * viewing. So a mismatch WAITS. + * + * An intent that never converges is therefore held — but it cannot outlive its + * usefulness, because the two things that would make it stale both clear it + * outright: leaving the surface (the layout clears on unmount) and picking a + * machine ROW rather than one of its sessions (the sidebar clears, since that + * says "this machine as it is"). Once satisfied it clears too, so the user's own + * later pane changes are never clobbered. + */ +export function resolvePendingSession( + pending: PendingSession | null, + selectedMachineId: string | null, + workspace: WorkspaceState | undefined, +): PendingSessionAction { + if (!pending) return { type: 'clear' }; + + // Not there yet: either the click's own navigation hasn't committed, or the + // user is en route. Holding is safe — see the note above on what clears it. + if (pending.machineId !== selectedMachineId) return { type: 'wait' }; + + // On the machine, but its pane region hasn't mounted (and ensured a workspace). + if (!workspace) return { type: 'wait' }; + + // Satisfied: the session is in the active pane. + if (sameScope(activePaneScope(workspace), pending.scope)) return { type: 'clear' }; + + return { type: 'open', machineId: pending.machineId, scope: pending.scope }; +} diff --git a/apps/web/src/lib/development/resolve-active-drive.ts b/apps/web/src/lib/development/resolve-active-drive.ts new file mode 100644 index 0000000000..8fe3619ddf --- /dev/null +++ b/apps/web/src/lib/development/resolve-active-drive.ts @@ -0,0 +1,24 @@ +import type { Drive } from '@pagespace/lib/types'; + +/** + * Which drive the driveless `/dashboard/development` entry should forward to. + * + * The Development surface keeps the drive in the path (one route tree), so its + * driveless entry is a redirect rather than a second implementation. This is the + * decision that redirect makes, extracted as a pure function so it's testable + * without a router. + * + * The preference order is the app's existing one — the same + * `find(currentDriveId) ?? first` fallback the backups settings page already + * uses to pick a drive when the URL doesn't name one. `currentDriveId` is the + * drive store's persisted "drive you were last in"; a trashed drive is never a + * redirect target, including when it IS the last-visited one. + * + * Returns null when the user has no drive to go to — the caller sends them to + * the drive picker instead. + */ +export function resolveActiveDriveId(drives: Drive[], currentDriveId: string | null): string | null { + const candidates = drives.filter((drive) => !drive.isTrashed); + const preferred = candidates.find((drive) => drive.id === currentDriveId); + return (preferred ?? candidates[0])?.id ?? null; +} diff --git a/apps/web/src/lib/machines/machine-list-runtime.ts b/apps/web/src/lib/machines/machine-list-runtime.ts new file mode 100644 index 0000000000..07eb26eecf --- /dev/null +++ b/apps/web/src/lib/machines/machine-list-runtime.ts @@ -0,0 +1,48 @@ +/** + * Production wiring for the shared "list a drive's Machines" service + * (`@pagespace/lib/services/machines/machine-list`) — binds the drive scan to + * the real `pages` table and the visibility filter to the real permission + * function, the same way `machine-access-runtime.ts` binds the per-machine + * view/edit checks. + */ + +import { and, asc, eq } from '@pagespace/db/operators'; +import { db } from '@pagespace/db/db'; +import { pages } from '@pagespace/db/schema/core'; +import { canUserViewPage } from '@pagespace/lib/permissions/permissions'; +import { PageType } from '@pagespace/lib/utils/enums'; +import { + listMachinesInDrive as listMachinesInDriveCore, + type MachineListDeps, + type MachinePageSummary, +} from '@pagespace/lib/services/machines/machine-list'; + +export type { MachinePageSummary }; + +function buildMachineListDeps(): MachineListDeps { + return { + findMachinePagesInDrive: async (driveId) => { + const rows = await db.query.pages.findMany({ + // Covered by the pages_drive_id_is_trashed_type_idx index. + where: and( + eq(pages.driveId, driveId), + eq(pages.isTrashed, false), + eq(pages.type, PageType.MACHINE), + ), + columns: { id: true, title: true, updatedAt: true }, + orderBy: [asc(pages.title)], + }); + return rows.map((row) => ({ + id: row.id, + title: row.title, + updatedAt: row.updatedAt.toISOString(), + })); + }, + canUserViewPage, + }; +} + +/** The Machine pages in `driveId` that the actor may view, ordered by title. */ +export async function listDriveMachines(actorUserId: string, driveId: string): Promise { + return listMachinesInDriveCore(buildMachineListDeps(), actorUserId, driveId); +} diff --git a/apps/web/src/stores/development/usePendingSessionStore.ts b/apps/web/src/stores/development/usePendingSessionStore.ts new file mode 100644 index 0000000000..5eb97ffd61 --- /dev/null +++ b/apps/web/src/stores/development/usePendingSessionStore.ts @@ -0,0 +1,29 @@ +import { create } from 'zustand'; +import type { PendingSession } from '@/lib/development/pending-session'; +import type { OpenTerminalScope } from '@/stores/machine-workspace/useMachineWorkspaceStore'; + +/** + * The one session-open intent in flight from the Development sidebar. + * + * The sidebar and the machine's pane region are siblings in the layout (the + * sidebar lives above the routed page), so the click and the component that can + * honour it have no common parent to hold this — hence a store, in the same way + * the machine workspace itself is shared by composition. + * + * Single-slot on purpose: a second click supersedes the first, because the user + * only ever ends up on one machine. All the decision-making lives in the pure + * `resolvePendingSession`; this only holds the value. + */ +interface PendingSessionStoreState { + pending: PendingSession | null; + requestSession: (machineId: string, scope: OpenTerminalScope) => void; + clearPending: () => void; +} + +export const usePendingSessionStore = create((set) => ({ + pending: null, + requestSession: (machineId, scope) => set({ pending: { machineId, scope } }), + // Identity-stable when there's nothing to clear, so a no-op clear can't + // re-render (and so the drain effect can call it unconditionally). + clearPending: () => set((state) => (state.pending === null ? state : { pending: null })), +})); diff --git a/apps/web/src/stores/machine-workspace/__tests__/useMachineTabStore.test.ts b/apps/web/src/stores/machine-workspace/__tests__/useMachineTabStore.test.ts new file mode 100644 index 0000000000..94f60b2c90 --- /dev/null +++ b/apps/web/src/stores/machine-workspace/__tests__/useMachineTabStore.test.ts @@ -0,0 +1,50 @@ +import { describe, test, expect, beforeEach } from 'vitest'; +import { useMachineTabStore, DEFAULT_MACHINE_TAB } from '../useMachineTabStore'; + +beforeEach(() => { + useMachineTabStore.setState({ tabs: {} }); +}); + +const tabOf = (machineId: string) => useMachineTabStore.getState().tabs[machineId] ?? DEFAULT_MACHINE_TAB; + +describe('useMachineTabStore', () => { + test('a machine with no stored tab shows Terminal', () => { + // MachineView's previous uncontrolled default, preserved. + expect(tabOf('machine-1')).toBe('terminal'); + }); + + test('remembers each machine\'s tab independently', () => { + useMachineTabStore.getState().setTab('machine-1', 'diff'); + + expect(tabOf('machine-1')).toBe('diff'); + expect(tabOf('machine-2')).toBe('terminal'); + }); + + test('focusTerminal brings a machine parked on another tab back to Terminal', () => { + // The reason this store exists: only the Terminal tab mounts a machine's + // workspace, so a session clicked on a machine sitting on Code/Diff/Settings + // had nowhere to land and the click did nothing at all. + useMachineTabStore.getState().setTab('machine-1', 'code'); + + useMachineTabStore.getState().focusTerminal('machine-1'); + + expect(tabOf('machine-1')).toBe('terminal'); + }); + + test('no-op writes keep state identity, so they cannot re-render subscribers', () => { + useMachineTabStore.getState().setTab('machine-1', 'code'); + const before = useMachineTabStore.getState().tabs; + + useMachineTabStore.getState().setTab('machine-1', 'code'); + + expect(useMachineTabStore.getState().tabs).toBe(before); + }); + + test('focusTerminal on a machine already showing Terminal is a no-op', () => { + const before = useMachineTabStore.getState().tabs; + + useMachineTabStore.getState().focusTerminal('machine-1'); + + expect(useMachineTabStore.getState().tabs).toBe(before); + }); +}); diff --git a/apps/web/src/stores/machine-workspace/useMachineTabStore.ts b/apps/web/src/stores/machine-workspace/useMachineTabStore.ts new file mode 100644 index 0000000000..d1eddfb1dc --- /dev/null +++ b/apps/web/src/stores/machine-workspace/useMachineTabStore.ts @@ -0,0 +1,41 @@ +import { create } from 'zustand'; + +export type MachineTabValue = 'terminal' | 'code' | 'diff' | 'settings'; + +export const DEFAULT_MACHINE_TAB: MachineTabValue = 'terminal'; + +/** + * Which tab each Machine is showing, keyed by machine id. + * + * The Machine page's tabs used to be uncontrolled (`defaultValue="terminal"`), + * which made the active tab unreachable from anywhere else — and that quietly + * broke the Development surface: `MachineWorkspace` (the thing that creates a + * machine's workspace) only mounts inside the Terminal tab, so clicking a + * session for a machine parked on Code/Diff/Settings had nowhere to land. The + * click did nothing at all. + * + * Hoisting the value here makes "show me this machine's terminal" something a + * caller can actually ask for, without any other change to how the tabs behave: + * a machine with no entry shows Terminal, exactly as before. + */ +interface MachineTabStoreState { + tabs: Record; + setTab: (machineId: string, tab: MachineTabValue) => void; + /** Bring a machine's Terminal tab to the front — used when opening a session on it. */ + focusTerminal: (machineId: string) => void; +} + +export const useMachineTabStore = create((set) => ({ + tabs: {}, + setTab: (machineId, tab) => + set((state) => (state.tabs[machineId] === tab ? state : { tabs: { ...state.tabs, [machineId]: tab } })), + focusTerminal: (machineId) => + set((state) => + // An unset machine is ALREADY showing Terminal (that's the default), so + // this must not write an entry for it — that would be a state change, and + // a re-render of every MachineView, for no visible difference. + (state.tabs[machineId] ?? DEFAULT_MACHINE_TAB) === DEFAULT_MACHINE_TAB + ? state + : { tabs: { ...state.tabs, [machineId]: DEFAULT_MACHINE_TAB } }, + ), +})); diff --git a/packages/lib/package.json b/packages/lib/package.json index 2eb4c53597..7964afcb18 100644 --- a/packages/lib/package.json +++ b/packages/lib/package.json @@ -642,6 +642,11 @@ "import": "./dist/services/machines/machine-access.js", "require": "./dist/services/machines/machine-access.js" }, + "./services/machines/machine-list": { + "types": "./dist/services/machines/machine-list.d.ts", + "import": "./dist/services/machines/machine-list.js", + "require": "./dist/services/machines/machine-list.js" + }, "./services/sandbox/tool-gate": { "types": "./dist/services/sandbox/tool-gate.d.ts", "import": "./dist/services/sandbox/tool-gate.js", @@ -1749,6 +1754,9 @@ "services/machines/machine-access": [ "./dist/services/machines/machine-access.d.ts" ], + "services/machines/machine-list": [ + "./dist/services/machines/machine-list.d.ts" + ], "services/sandbox/sandbox-client/types": [ "./dist/services/sandbox/sandbox-client/types.d.ts" ], diff --git a/packages/lib/src/services/machines/__tests__/machine-list.test.ts b/packages/lib/src/services/machines/__tests__/machine-list.test.ts new file mode 100644 index 0000000000..14bc648984 --- /dev/null +++ b/packages/lib/src/services/machines/__tests__/machine-list.test.ts @@ -0,0 +1,60 @@ +import { describe, test, expect, vi } from 'vitest'; +import { listMachinesInDrive, type MachineListDeps, type MachinePageSummary } from '../machine-list'; + +const machine = (id: string, title = id): MachinePageSummary => ({ + id, + title, + updatedAt: '2026-07-11T00:00:00.000Z', +}); + +function buildDeps( + pagesInDrive: MachinePageSummary[], + viewable: (pageId: string) => boolean, +): MachineListDeps { + return { + findMachinePagesInDrive: vi.fn(async () => pagesInDrive), + canUserViewPage: vi.fn(async (_userId: string, pageId: string) => viewable(pageId)), + }; +} + +describe('listMachinesInDrive', () => { + test('returns the drive\'s machines in scan order', async () => { + const deps = buildDeps([machine('m-1', 'alpha'), machine('m-2', 'beta')], () => true); + + const result = await listMachinesInDrive(deps, 'user-1', 'drive-1'); + + expect(result.map((m) => m.id)).toEqual(['m-1', 'm-2']); + expect(deps.findMachinePagesInDrive).toHaveBeenCalledWith('drive-1'); + }); + + test('withholds a machine the actor cannot view', async () => { + const deps = buildDeps( + [machine('m-1'), machine('m-secret'), machine('m-3')], + (pageId) => pageId !== 'm-secret', + ); + + const result = await listMachinesInDrive(deps, 'user-1', 'drive-1'); + + expect(result.map((m) => m.id)).toEqual(['m-1', 'm-3']); + }); + + test('checks visibility against the acting user', async () => { + const deps = buildDeps([machine('m-1')], () => true); + + await listMachinesInDrive(deps, 'user-42', 'drive-1'); + + expect(deps.canUserViewPage).toHaveBeenCalledWith('user-42', 'm-1'); + }); + + test('a drive with no machines is an empty list, not an error', async () => { + const deps = buildDeps([], () => true); + + expect(await listMachinesInDrive(deps, 'user-1', 'drive-1')).toEqual([]); + }); + + test('a drive whose every machine is withheld is an empty list', async () => { + const deps = buildDeps([machine('m-1'), machine('m-2')], () => false); + + expect(await listMachinesInDrive(deps, 'user-1', 'drive-1')).toEqual([]); + }); +}); diff --git a/packages/lib/src/services/machines/machine-list.ts b/packages/lib/src/services/machines/machine-list.ts new file mode 100644 index 0000000000..11ab6151e4 --- /dev/null +++ b/packages/lib/src/services/machines/machine-list.ts @@ -0,0 +1,51 @@ +/** + * "Every Machine in this drive" — the Development surface's aggregated tree is + * a list of Machine pages, and nothing else listed one. The existing machine + * services all address ONE machine by id, and the global-machine-config + * repository finds only THE global machine, so this is the one net-new query + * the surface needs. + * + * Pure + DI'd like the rest of `services/machines`: the drive scan and the + * permission check are both injected, so the interesting part (a page the actor + * cannot view must not leak out of the drive scan) is testable without a DB. + */ + +export interface MachinePageSummary { + id: string; + title: string; + /** ISO-8601. Served for callers that want recency (the tree itself orders by title). */ + updatedAt: string; +} + +export interface MachineListDeps { + /** Every non-trashed MACHINE-type page in the drive, in the order it should be presented. */ + findMachinePagesInDrive: (driveId: string) => Promise; + canUserViewPage: (userId: string, pageId: string) => Promise; +} + +/** + * The Machine pages in `driveId` that `actorUserId` may view, in scan order. + * + * The drive scan is a raw `type = MACHINE` query, so it is NOT permission-aware + * on its own — a page-level grant can withhold an individual Machine from a + * drive member. Every candidate is therefore re-checked against + * `canUserViewPage` here, which is the same view-level gate every other machine + * route applies before serving a machine's projects/branches/sessions. + * + * That check is per page, so this fans out N permission lookups for N machines. + * Fine at this scale — the surface is app-admin-only, machines are heavyweight + * things a drive has a handful of, and an owner/admin short-circuits early. If a + * drive ever holds enough machines for it to matter, resolve drive membership + * once and fall back to the per-page check only for the non-owner case. + */ +export async function listMachinesInDrive( + deps: MachineListDeps, + actorUserId: string, + driveId: string, +): Promise { + const candidates = await deps.findMachinePagesInDrive(driveId); + const visibility = await Promise.all( + candidates.map((machine) => deps.canUserViewPage(actorUserId, machine.id)), + ); + return candidates.filter((_, index) => visibility[index]); +}