diff --git a/DESIGN.md b/DESIGN.md index 7f2bc38c..9b770920 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -274,8 +274,9 @@ such detail into a token. New layout spacing must use the scale. Keep the last header action's outer edge flush with the shell; never apply the diff scrollbar inset to header actions. Theme and snapshot deletion are absent from the header. - A local checkout process replaces Refresh with an icon-only “Sync all - checkouts” action (folder-sync icon) that never swaps the open review. + A local checkout process replaces Refresh with an icon-only sync action + (folder-sync icon), “Sync checkout” for one selected checkout and “Sync all + checkouts” for discovered checkouts, that never swaps the open review. - Header text uses cap-height trimming with token padding to center UI and monospace labels without clipping ascenders or descenders. - Header lead: brand and divider identify the app. The file-sidebar toggle @@ -292,6 +293,14 @@ such detail into a token. New layout spacing must use the scale. still mounted, while collapsed). Piped wording, icons and grouping appear only there; elsewhere a stored stdin snapshot reads like any review. Components read the policy, never the deployment mode. +- Review column visibility is part of that policy: a remote server always shows + the column; a local checkout process shows the column, rail and “Show + reviews” drawer toggle only once the column would list two current streams, + then keeps them for the page session so a stream leaving never reflows the + review. Until then the island holds only the file sidebar, exactly as a + capture's, with no placeholder or motion; search, Cmd/Ctrl+K, the header + comboboxes and sync remain. The column appears instantly in its persisted + list or rail state. - Review column: the leading pane of the navigation island, titled “Reviews”, on `--panel`, sharing the file tree and diff's top and bottom edges below the header. Switching streams changes everything to its right; the column stays. diff --git a/apps/web/.impeccable/surfaces/apps-web-src-diffworkspace-tsx.md b/apps/web/.impeccable/surfaces/src-diffworkspace-tsx.md similarity index 95% rename from apps/web/.impeccable/surfaces/apps-web-src-diffworkspace-tsx.md rename to apps/web/.impeccable/surfaces/src-diffworkspace-tsx.md index 9a4513e1..f3615f47 100644 --- a/apps/web/.impeccable/surfaces/apps-web-src-diffworkspace-tsx.md +++ b/apps/web/.impeccable/surfaces/src-diffworkspace-tsx.md @@ -1,7 +1,7 @@ --- version: 1 -slug: "apps-web-src-diffworkspace-tsx" -primary_target: "apps/web/src/DiffWorkspace.tsx" +slug: "src-diffworkspace-tsx" +primary_target: "src/DiffWorkspace.tsx" related_targets: [] --- diff --git a/apps/web/PRODUCT.md b/apps/web/PRODUCT.md index 63bcc0be..9565e702 100644 --- a/apps/web/PRODUCT.md +++ b/apps/web/PRODUCT.md @@ -20,7 +20,7 @@ A local review workspace that combines a browser-based Git diff with comments th ## Operating Context -- A developer runs `diffx` from a Git repository or workspace. The Go server opens the review UI in a browser, shows the checkouts and worktrees discovered under its paths, and collects them all again when the developer syncs. +- A developer runs `diffx` from a Git repository or workspace. The Go server opens the review UI in a browser and shows that checkout (plus its other worktrees with `--scan-worktrees`) or the checkouts and worktrees discovered under its paths, collecting the same scope again when the developer syncs. With one review to look at, the Reviews column stays out of the way; it appears once there are two to switch between. - Reviews can cover all, staged, or unstaged changes. The file tree, reviewed-file marks, and comments help track progress. - Comments can be copied as agent-ready XML. The same review comments are available to compatible coding agents through MCP and, in supported browsers, WebMCP. - The CLI can also display a piped Git patch as a fixed review. diff --git a/apps/web/src/ContextIdentity.tsx b/apps/web/src/ContextIdentity.tsx index a61b5d44..1ebd1824 100644 --- a/apps/web/src/ContextIdentity.tsx +++ b/apps/web/src/ContextIdentity.tsx @@ -34,7 +34,7 @@ import { } from "./project-nav.ts"; import { NewMark, StreamIcon, useNow } from "./ProjectsNav.tsx"; import { useCatalog } from "./session-context.tsx"; -import { useShellPolicy } from "./shell-policy.ts"; +import { useShellPolicy } from "./use-shell-policy.ts"; /** * The open review's repository, branch and worktree. With review navigation, diff --git a/apps/web/src/Navigation.tsx b/apps/web/src/Navigation.tsx index 7f96181e..91064f61 100644 --- a/apps/web/src/Navigation.tsx +++ b/apps/web/src/Navigation.tsx @@ -1,15 +1,16 @@ import { useRef, type ReactNode } from "react"; import { FileTreeSkeleton } from "./WorkspaceLoading.tsx"; import { ProjectsNav } from "./ProjectsNav.tsx"; -import { useShellPolicy } from "./shell-policy.ts"; +import { useCatalog } from "./session-context.tsx"; import { useSidebarState } from "./sidebar-context.tsx"; import { useStreamNavState } from "./stream-nav-context.tsx"; /** * One navigation island: the review column, then the open review's file - * sidebar. Each pane owns its width and resize boundary. Without review - * navigation (a capture) only the file sidebar remains; the island hides, - * still mounted, while that sidebar is collapsed. + * sidebar. Each pane owns its width and resize boundary. Without the review + * column (a capture, or a local process with one review to switch to) only + * the file sidebar remains; the island hides, still mounted, while that + * sidebar is collapsed. */ export function Navigation({ children, @@ -20,15 +21,15 @@ export function Navigation({ }) { const sidebar = useSidebarState(); const streams = useStreamNavState(); - const { reviewNavigation } = useShellPolicy(); + const { reviewColumn } = useCatalog(); const files = Boolean(children) || loading; const filesWidth = files && !sidebar.collapsed ? sidebar.width : 0; - const rail = !reviewNavigation || streams.state === "rail"; + const rail = !reviewColumn || streams.state === "rail"; const empty = - !reviewNavigation && !sidebar.mobile && (!files || sidebar.collapsed); + !reviewColumn && !sidebar.mobile && (!files || sidebar.collapsed); return (
- {reviewNavigation && ( + {reviewColumn && (
: null; } -/** Collects every local checkout; shared by the review and its loading shell. */ +/** + * Collects the local process's checkout, or every discovered checkout; shared + * by the review and its loading shell. + */ export function SyncButton({ busy, onClick, @@ -22,11 +25,12 @@ export function SyncButton({ /** Omitted while the loading shell collects: the action is unavailable. */ onClick?: () => void; }) { + const { collection } = useShellPolicy(); return ( - {mobile && reviewNavigation && } + {mobile && reviewColumn && } {lead} diff --git a/apps/web/src/project-nav.ts b/apps/web/src/project-nav.ts index d0be195f..fdbe1097 100644 --- a/apps/web/src/project-nav.ts +++ b/apps/web/src/project-nav.ts @@ -41,6 +41,14 @@ function currentWork(context: ApiContext) { return state !== "closed" && state !== "merged"; } +/** How many streams the review column lists across every repository. */ +export function streamCount(heads: ApiContext[]) { + return projectGroups(heads, null).reduce( + (total, group) => total + group.streams.length, + 0, + ); +} + export function projectGroups(heads: ApiContext[], seen: SeenStreams | null) { const groups = new Map(); for (const context of heads) { diff --git a/apps/web/src/session-context.tsx b/apps/web/src/session-context.tsx index dbb618f7..d9b662be 100644 --- a/apps/web/src/session-context.tsx +++ b/apps/web/src/session-context.tsx @@ -11,8 +11,10 @@ import { useState, } from "react"; import { useIdentity } from "./identity-context.tsx"; +import { showsReviewColumn } from "./shell-policy.ts"; +import { useShellPolicy } from "./use-shell-policy.ts"; import { latestChangedContext } from "./context-activity.ts"; -import { streamKey, type SeenStreams } from "./project-nav.ts"; +import { streamCount, streamKey, type SeenStreams } from "./project-nav.ts"; import { useSeenStreams } from "./use-seen-streams.ts"; import { useSidebarState } from "./sidebar-context.tsx"; import { @@ -74,6 +76,8 @@ interface Catalog { collect: (() => Promise) | null; collecting: boolean; collectError: string; + /** Whether the shell shows the Reviews column, rail and drawer toggle. */ + reviewColumn: boolean; } /** Loading, empty and failure state for the shell shown until a review is ready. */ export interface SessionShellState { @@ -397,6 +401,12 @@ export function SessionProvider({ children }: { children: ReactNode }) { heads.loaded, ready ? current : undefined, ); + const policy = useShellPolicy(); + const [reviewColumn, setReviewColumn] = useState(false); + // Once shown, the column stays for the page session, so a stream leaving the + // list never reflows the review beneath the reader. + if (!reviewColumn && showsReviewColumn(policy, streamCount(heads.contexts))) + setReviewColumn(true); const catalog = useMemo( () => ({ heads, @@ -411,6 +421,7 @@ export function SessionProvider({ children }: { children: ReactNode }) { collect, collecting, collectError, + reviewColumn, }), [ heads, @@ -425,6 +436,7 @@ export function SessionProvider({ children }: { children: ReactNode }) { collect, collecting, collectError, + reviewColumn, ], ); // The root URL keeps waiting for changed work while showing the empty state. diff --git a/apps/web/src/shell-policy.ts b/apps/web/src/shell-policy.ts index e7de50b9..77936ec0 100644 --- a/apps/web/src/shell-policy.ts +++ b/apps/web/src/shell-policy.ts @@ -1,5 +1,4 @@ import type { ApiBootstrap } from "@diffx/api"; -import { useIdentity } from "./identity-context.tsx"; /** * What the review shell offers, resolved once from the server's bootstrap. @@ -13,18 +12,30 @@ export type ShellPolicy = { * switch to, so it shows only that diff and its file sidebar. */ reviewNavigation: boolean; + /** + * When the Reviews column, rail and compact drawer toggle show. A remote + * server always shows them; a local checkout process waits until there are + * two current reviews to switch between. + */ + reviewColumn: "always" | "multiple"; + /** What Sync collects: one selected checkout or every discovered one. */ + collection: "checkout" | "checkouts" | null; }; export function shellPolicy(bootstrap: ApiBootstrap): ShellPolicy { + if (bootstrap.mode === "remote") + return { reviewNavigation: true, reviewColumn: "always", collection: null }; + const collection = bootstrap.input === "capture" ? null : bootstrap.input; return { - reviewNavigation: - bootstrap.mode === "remote" || bootstrap.input === "checkouts", + reviewNavigation: collection !== null, + reviewColumn: "multiple", + collection, }; } -const unresolved: ShellPolicy = { reviewNavigation: false }; - -export function useShellPolicy(): ShellPolicy { - const { state } = useIdentity(); - return state.status === "ready" ? shellPolicy(state.bootstrap) : unresolved; +/** Whether the Reviews column shows while it lists `streams` streams. */ +export function showsReviewColumn(policy: ShellPolicy, streams: number) { + return ( + policy.reviewNavigation && (policy.reviewColumn === "always" || streams > 1) + ); } diff --git a/apps/web/src/use-shell-policy.ts b/apps/web/src/use-shell-policy.ts new file mode 100644 index 00000000..4fb6f165 --- /dev/null +++ b/apps/web/src/use-shell-policy.ts @@ -0,0 +1,13 @@ +import { useIdentity } from "./identity-context.tsx"; +import { shellPolicy, type ShellPolicy } from "./shell-policy.ts"; + +const unresolved: ShellPolicy = { + reviewNavigation: false, + reviewColumn: "multiple", + collection: null, +}; + +export function useShellPolicy(): ShellPolicy { + const { state } = useIdentity(); + return state.status === "ready" ? shellPolicy(state.bootstrap) : unresolved; +} diff --git a/apps/web/test/browser/app.spec.ts b/apps/web/test/browser/app.spec.ts index 15656796..46ce055c 100644 --- a/apps/web/test/browser/app.spec.ts +++ b/apps/web/test/browser/app.spec.ts @@ -6,6 +6,7 @@ import { reviewedToggle, test, withReviewNavigation, + withSecondReview, } from "./fixtures.ts"; import { resetFixtureState } from "./fixture-state.ts"; @@ -1799,6 +1800,7 @@ test("header icons and sidebar tab indicator use design-system sizes", async ({ page, }) => { await withReviewNavigation(page); + await withSecondReview(page); await page.goto("/"); const brand = page.locator(".topbar .brand"); const filesToggle = page.getByRole("button", { name: "Hide file sidebar" }); @@ -2045,6 +2047,7 @@ test("review column and file sidebar resize independently and persist", async ({ page, }) => { await withReviewNavigation(page); + await withSecondReview(page); await page.goto("/"); const column = page.locator(".stream-column"); const files = page.locator(".navigation-panel"); @@ -2383,6 +2386,7 @@ test("drawer aligns its header and retains desktop navigation density", async ({ page, }) => { await withReviewNavigation(page); + await withSecondReview(page); await page.setViewportSize({ width: 1280, height: 844 }); await page.goto("/"); await showDiff(page); diff --git a/apps/web/test/browser/fixtures.ts b/apps/web/test/browser/fixtures.ts index d2032e29..ef775be9 100644 --- a/apps/web/test/browser/fixtures.ts +++ b/apps/web/test/browser/fixtures.ts @@ -75,10 +75,10 @@ export function fileToggle(scope: Page | Locator, state?: ToggleState) { /** * The fixture server is a capture (one fixed patch), so the shell hides review - * navigation. Serving a checkout-process bootstrap restores the Reviews - * column, header comboboxes and search for tests that mock several contexts. - * Later `page.route` registrations take precedence, so tests can still - * override it. + * navigation. Serving a checkout-process bootstrap restores the header + * comboboxes and search for tests that mock several contexts; the Reviews + * column also needs two current reviews (see withSecondReview). Later + * `page.route` registrations take precedence, so tests can still override it. */ export async function withReviewNavigation( page: Page, @@ -97,3 +97,38 @@ export async function withReviewNavigation( }), ); } + +/** + * A local process shows the Reviews column once it lists two current reviews. + * Adds a second repository's copy of the fixture's review to every catalog + * page, so layout tests over the single fixture review get the column. + */ +export async function withSecondReview(page: Page) { + await page.route("**/api/v2/contexts?*", async (route) => { + const response = await route.fetch(); + const body: unknown = await response.json(); + if ( + typeof body !== "object" || + body === null || + !("contexts" in body) || + !Array.isArray(body.contexts) + ) + return route.fulfill({ response }); + const contexts: unknown[] = body.contexts; + const first = contexts[0]; + if (typeof first !== "object" || first === null) + return route.fulfill({ response }); + const second = { + ...first, + id: "second-review", + streamId: "second-review", + latestContextId: "second-review", + name: "second-repo", + repositoryId: "second-repo", + }; + return route.fulfill({ + response, + json: { ...body, contexts: [...contexts, second] }, + }); + }); +} diff --git a/apps/web/test/browser/local-workspace.spec.ts b/apps/web/test/browser/local-workspace.spec.ts index 30a9249a..7bd9bc8a 100644 --- a/apps/web/test/browser/local-workspace.spec.ts +++ b/apps/web/test/browser/local-workspace.spec.ts @@ -1985,18 +1985,20 @@ test("local first launch waits for collection with an empty catalog", async ({ ), ).toBe(true); await page.setViewportSize({ width: 1280, height: 720 }); - // The review column shows its own placeholders beside the file tree's. + // Without two reviews to switch between, a local process shows no review + // column; only the file tree's placeholders load. const projects = page.getByRole("navigation", { name: "Reviews", exact: true, }); - await expect(projects).toContainText("Collecting reviews…"); - await expect(projects.locator(".projects-skeleton")).toBeVisible(); + await expect(projects).toHaveCount(0); await expect(page.locator(".loading-sidebar")).toBeVisible(); - for (const name of ["Resize reviews column", "Resize file sidebar"]) - await expect(page.getByRole("separator", { name })).toBeVisible(); - await expect(projects).not.toContainText("No current changes"); - await expect(projects).not.toContainText("Agent hooks and diffx sync"); + await expect( + page.getByRole("separator", { name: "Resize file sidebar" }), + ).toBeVisible(); + await expect( + page.getByRole("separator", { name: "Resize reviews column" }), + ).toHaveCount(0); await expect( page.getByRole("heading", { name: "No review contexts yet" }), ).toHaveCount(0); @@ -2011,6 +2013,10 @@ test("local first launch waits for collection with an empty catalog", async ({ await expect(page).toHaveURL(/\/streams\/local-stream$/); expect(collections).toBe(1); await expect(page.locator(".session-status")).toHaveCount(0); + await expect(projects).toHaveCount(0); + await expect( + page.getByRole("button", { name: "Search reviews" }), + ).toBeVisible(); const reviewBounds = await page.locator(".workspace").boundingBox(); if (!reviewBounds) throw new Error("Missing loaded workspace"); expect(reviewBounds.x).toBeCloseTo(loadingBounds.x, 0); @@ -2157,11 +2163,12 @@ test("local root waits for collection to open its default over earlier heads", a const status = page.locator("main.session-status"); await expect(status).toContainText("Collecting latest changes"); await expect(status).toHaveAttribute("aria-busy", "true"); - await expect( - page - .getByRole("navigation", { name: "Reviews", exact: true }) - .getByRole("link"), - ).toHaveCount(1); + // One review so far: nothing to switch to, so no review column. + const projects = page.getByRole("navigation", { + name: "Reviews", + exact: true, + }); + await expect(projects).toHaveCount(0); await expect(page).toHaveURL(/\/$/); expect(opened).toEqual([]); if (!pending) throw new Error("Missing initial collection"); @@ -2172,6 +2179,17 @@ test("local root waits for collection to open its default over earlier heads", a await ready(page); await expect(page).toHaveURL(/\/streams\/launched-stream$/); expect(opened).toEqual(["launched"]); + // The second review brings the column, which then stays. + await expect(projects.getByRole("link")).toHaveCount(2); + heads = [launched]; + await page.route("**/api/v2/collections*", (route) => + route.fulfill({ + json: { contexts: heads, failures: [], defaultContextId: launched.id }, + }), + ); + await page.getByRole("button", { name: "Sync all checkouts" }).click(); + await expect(projects.getByRole("link")).toHaveCount(1); + await expect(projects).toBeVisible(); }); test("unresolved navigation joins collection without reporting a missing stream", async ({ @@ -2618,6 +2636,113 @@ test("each remote account baselines its own seen heads", async ({ page }) => { ).toContain("web-head"); }); +test("a single local checkout shows no review column but keeps search, comboboxes and sync", async ({ + page, +}) => { + const state = await localWorkspace(page); + await page.route("**/api/v2/bootstrap", (route) => + route.fulfill({ + json: { + mode: "local", + googleEnabled: false, + user: null, + csrfToken: null, + collect: true, + input: "checkout", + }, + }), + ); + const snapshot: ApiContext = { + ...state.context, + streamId: "local-stream", + latestContextId: state.context.id, + }; + await page.route("**/api/v2/contexts?*", (route) => + route.fulfill({ json: { contexts: [snapshot], nextCursor: null } }), + ); + await page.route("**/api/v2/collections*", (route) => + route.fulfill({ + json: { + contexts: [snapshot], + failures: [], + defaultContextId: snapshot.id, + }, + }), + ); + await page.goto("/"); + await ready(page); + await expect( + page.getByRole("navigation", { name: "Reviews", exact: true }), + ).toHaveCount(0); + await expect(page.locator(".stream-column")).toHaveCount(0); + await expect( + page.getByRole("separator", { name: "Resize reviews column" }), + ).toHaveCount(0); + const banner = page.getByRole("banner"); + await expect( + banner.getByRole("button", { name: "Search reviews" }), + ).toBeVisible(); + await expect( + banner.getByRole("combobox", { name: /^Repository .+ Switch repository$/ }), + ).toBeVisible(); + await expect( + banner.getByRole("button", { name: "Sync checkout" }), + ).toBeVisible(); + await expect( + banner.getByRole("button", { name: "Sync all checkouts" }), + ).toHaveCount(0); + // The diff starts where the file sidebar ends. + const sidebar = await page.locator("#sidebar").boundingBox(); + const navigation = await page.locator(".navigation-island").boundingBox(); + if (!sidebar || !navigation) throw new Error("Missing navigation"); + expect(sidebar.x).toBeCloseTo(navigation.x, 0); + await page.setViewportSize({ width: 390, height: 844 }); + await expect( + banner.getByRole("button", { name: /^(?:Show|Hide) reviews$/ }), + ).toHaveCount(0); + await expect( + banner.getByRole("button", { name: "Search reviews" }), + ).toBeVisible(); +}); + +test("a remote server shows the review column for a single review", async ({ + page, +}) => { + const state = await localWorkspace(page); + await page.route("**/api/v2/bootstrap", (route) => + route.fulfill({ + json: { + mode: "remote", + googleEnabled: false, + user: { id: "account-a", username: "alice", email: null, admin: false }, + csrfToken: "csrf", + collect: false, + }, + }), + ); + const head: ApiContext = { + ...state.context, + id: "api-head", + repositoryId: "api", + name: "api", + streamId: "stream-api", + latestContextId: "api-head", + }; + await page.route("**/api/v2/contexts?*", (route) => + route.fulfill({ json: { contexts: [head], nextCursor: null } }), + ); + await page.route("**/api/v2/contexts/*", (route) => + route.fulfill({ json: head }), + ); + await page.goto("/contexts/api-head"); + await ready(page); + const nav = page.getByRole("navigation", { name: "Reviews", exact: true }); + await expect(nav).toBeVisible(); + await expect( + nav.getByRole("button", { name: /^Open latest in / }), + ).toHaveText([/^api/]); +}); + async function capturedWorkspace(page: Page) { const state = await localWorkspace(page); state.context.streamId = "captured-stream"; diff --git a/apps/web/test/browser/native-scrollbars.spec.ts b/apps/web/test/browser/native-scrollbars.spec.ts index 7d782b97..103b4cdb 100644 --- a/apps/web/test/browser/native-scrollbars.spec.ts +++ b/apps/web/test/browser/native-scrollbars.spec.ts @@ -1,4 +1,9 @@ -import { expect, test, withReviewNavigation } from "./fixtures.ts"; +import { + expect, + test, + withReviewNavigation, + withSecondReview, +} from "./fixtures.ts"; import { resetFixtureState } from "./fixture-state.ts"; test.use({ launchOptions: { ignoreDefaultArgs: ["--hide-scrollbars"] } }); @@ -9,6 +14,7 @@ test("diff tracks fill frames and headers align with their gutters", async ({ }) => { // Header geometry is measured against the compact "Show reviews" toggle. await withReviewNavigation(page); + await withSecondReview(page); const colorSchemes: ("light" | "dark")[] = ["light", "dark"]; for (const colorScheme of colorSchemes) { await page.emulateMedia({ colorScheme }); diff --git a/apps/web/test/identity.test.ts b/apps/web/test/identity.test.ts index dee7bb98..ba47b3c4 100644 --- a/apps/web/test/identity.test.ts +++ b/apps/web/test/identity.test.ts @@ -3,6 +3,27 @@ import { test } from "node:test"; import { readBootstrap, readAccountTokens, readCreatedToken } from "@diffx/api"; test("server mode requires explicit bootstrap and consistent session identity", () => { + for (const input of ["checkout", "checkouts", "capture"]) { + const bootstrap = { + mode: "local", + googleEnabled: false, + user: null, + csrfToken: null, + collect: input !== "capture", + input, + }; + assert.deepEqual(readBootstrap(bootstrap), bootstrap); + } + assert.throws(() => + readBootstrap({ + mode: "local", + googleEnabled: false, + user: null, + csrfToken: null, + collect: true, + input: "worktrees", + }), + ); assert.deepEqual( readBootstrap({ mode: "local", diff --git a/apps/web/test/project-nav.test.ts b/apps/web/test/project-nav.test.ts index 362fae05..587856ed 100644 --- a/apps/web/test/project-nav.test.ts +++ b/apps/web/test/project-nav.test.ts @@ -11,6 +11,7 @@ import { relativeAge, repoMonogram, streamMatches, + streamCount, streamMeta, streamTitle, } from "../src/project-nav.ts"; @@ -84,6 +85,22 @@ test("projects list current changed streams by recent activity", () => { assert.equal(groups[1]?.lastActivity, 20); }); +test("stream count matches the streams the review column lists", () => { + assert.equal(streamCount([]), 0); + assert.equal( + streamCount([ + head("api", "api", 10), + head("clean", "api", 40, { changedFileCount: 0 }), + head("stale", "api", 50, { stale: true }), + ]), + 1, + ); + assert.equal( + streamCount([head("main", "api", 10), head("other", "api", 20)]), + 2, + ); +}); + test("a repository opens to its most active stream", () => { const [api] = projectGroups( [ diff --git a/apps/web/test/shell-policy.test.ts b/apps/web/test/shell-policy.test.ts new file mode 100644 index 00000000..54f44920 --- /dev/null +++ b/apps/web/test/shell-policy.test.ts @@ -0,0 +1,59 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import type { ApiBootstrap } from "@diffx/api"; +import { shellPolicy, showsReviewColumn } from "../src/shell-policy.ts"; + +function local(input: "checkout" | "checkouts" | "capture"): ApiBootstrap { + return { + mode: "local", + googleEnabled: false, + user: null, + csrfToken: null, + collect: input !== "capture", + input, + }; +} + +const inputs: ("checkout" | "checkouts")[] = ["checkout", "checkouts"]; + +const remote: ApiBootstrap = { + mode: "remote", + googleEnabled: false, + user: null, + csrfToken: null, +}; + +test("shell policy follows what the server serves", () => { + assert.deepEqual(shellPolicy(local("checkout")), { + reviewNavigation: true, + reviewColumn: "multiple", + collection: "checkout", + }); + assert.deepEqual(shellPolicy(local("checkouts")), { + reviewNavigation: true, + reviewColumn: "multiple", + collection: "checkouts", + }); + assert.deepEqual(shellPolicy(local("capture")), { + reviewNavigation: false, + reviewColumn: "multiple", + collection: null, + }); + assert.deepEqual(shellPolicy(remote), { + reviewNavigation: true, + reviewColumn: "always", + collection: null, + }); +}); + +test("a local review column waits for a second review; remote always shows", () => { + for (const input of inputs) { + const policy = shellPolicy(local(input)); + assert.equal(showsReviewColumn(policy, 0), false); + assert.equal(showsReviewColumn(policy, 1), false); + assert.equal(showsReviewColumn(policy, 2), true); + } + assert.equal(showsReviewColumn(shellPolicy(remote), 0), true); + assert.equal(showsReviewColumn(shellPolicy(remote), 1), true); + assert.equal(showsReviewColumn(shellPolicy(local("capture")), 3), false); +}); diff --git a/cmd/diffx/ingestion_test.go b/cmd/diffx/ingestion_test.go index 05df4c20..93affda8 100644 --- a/cmd/diffx/ingestion_test.go +++ b/cmd/diffx/ingestion_test.go @@ -49,7 +49,7 @@ func TestNonRepositoryWorkspaceFailsCollectionWithoutConsumingStdin(t *testing.T } defer stdin.Close() values := options{directory: ".", sourceID: "test-source", trigger: "manual"} - input, err := acquireLocalInput(values, stdin) + input, err := acquireLocalInput(t.Context(), values, stdin) if err != nil { t.Fatal(err) } @@ -57,7 +57,7 @@ func TestNonRepositoryWorkspaceFailsCollectionWithoutConsumingStdin(t *testing.T t.Fatalf("workspace input: %#v", input) } publications := 0 - _, err = collectWorkspaces(t.Context(), values, input.Paths, io.Discard, func(ingestion.Request) error { + _, err = collectWorkspaces(t.Context(), values, input, io.Discard, func(ingestion.Request) error { publications++ return nil }) @@ -78,7 +78,7 @@ func TestPipedInputKeepsSubmissionDirectoryWithoutGitIdentity(t *testing.T) { t.Chdir(root) patch := []byte("diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-before\n+after\n") values := options{directory: ".", sourceID: "test-source", trigger: "manual"} - input, err := acquireLocalInput(values, testInputFile(t, patch)) + input, err := acquireLocalInput(t.Context(), values, testInputFile(t, patch)) if err != nil { t.Fatal(err) } @@ -128,12 +128,12 @@ func TestLocalInputRejectsMissingCheckoutPathBeforeStartup(t *testing.T) { } for _, path := range []string{filepath.Join(root, "missing"), file} { values := options{directory: path, sourceID: "test-source", trigger: "manual"} - if _, err := acquireLocalInput(values, nil); err == nil || !strings.Contains(err.Error(), path) { + if _, err := acquireLocalInput(t.Context(), values, nil); err == nil || !strings.Contains(err.Error(), path) { t.Fatalf("accepted checkout path %s: %v", path, err) } } values := options{directory: root, paths: []string{root, filepath.Join(root, "missing")}, sourceID: "test-source", trigger: "manual"} - if _, err := acquireLocalInput(values, nil); err == nil { + if _, err := acquireLocalInput(t.Context(), values, nil); err == nil { t.Fatal("accepted a missing repeated checkout path") } } diff --git a/cmd/diffx/input.go b/cmd/diffx/input.go index 0c74eee0..17ede329 100644 --- a/cmd/diffx/input.go +++ b/cmd/diffx/input.go @@ -15,6 +15,7 @@ import ( "github.com/flexdinesh/diffx/internal/diffsource" "github.com/flexdinesh/diffx/internal/hooks" "github.com/flexdinesh/diffx/internal/ingestion" + "github.com/flexdinesh/diffx/internal/localserver" "github.com/flexdinesh/diffx/internal/review" buildversion "github.com/flexdinesh/diffx/internal/version" ) @@ -172,6 +173,7 @@ type initialInput struct { Kind string Path string Paths []string + Selection localserver.Selection Raw []byte SubmittedFrom string } diff --git a/cmd/diffx/local.go b/cmd/diffx/local.go index b073a79a..7c65d28d 100644 --- a/cmd/diffx/local.go +++ b/cmd/diffx/local.go @@ -15,6 +15,7 @@ import ( "time" "github.com/flexdinesh/diffx/internal/collector" + "github.com/flexdinesh/diffx/internal/diffsource" "github.com/flexdinesh/diffx/internal/ingestion" "github.com/flexdinesh/diffx/internal/localserver" "github.com/flexdinesh/diffx/internal/reviewstore" @@ -31,7 +32,7 @@ func runLocal(ctx context.Context, arguments []string, stdin *os.File, stdout, s if values.serverSet || values.tokenSet || values.branch != "" || values.fixture != "" || values.print || values.retry { return errors.New("local mode accepts a checkout path or piped diff; use sync for remote collection or dev --fixture for fixtures") } - input, err := acquireLocalInput(values, stdin) + input, err := acquireLocalInput(ctx, values, stdin) if err != nil { return err } @@ -48,9 +49,9 @@ func runLocal(ctx context.Context, arguments []string, stdin *os.File, stdout, s if input.Kind == "worktree" { label = strings.Join(input.Paths, ", ") collect := func(ctx context.Context, publish func(ingestion.Request) error) ([]localserver.CollectionFailure, error) { - return collectWorkspaces(ctx, values, input.Paths, stderr, publish) + return collectWorkspaces(ctx, values, input, stderr, publish) } - selected = localserver.Input{Collect: collect} + selected = localserver.Input{Collect: collect, Selection: input.Selection} } else { request, err := collectInitialInput(ctx, values, input) if err != nil { @@ -142,14 +143,16 @@ func askYes(ctx context.Context, stdin *os.File, stderr io.Writer, question stri } } -// collectWorkspaces collects every checkout discovered under the selected -// paths. Discovery diagnostics go to stderr; failures stay in the collection. -func collectWorkspaces(ctx context.Context, values options, paths []string, stderr io.Writer, publish func(ingestion.Request) error) ([]localserver.CollectionFailure, error) { +// collectWorkspaces collects the input's selected checkout or every checkout +// discovered under its paths. Discovery diagnostics go to stderr; failures stay +// in the collection. +func collectWorkspaces(ctx context.Context, values options, input initialInput, stderr io.Writer, publish func(ingestion.Request) error) ([]localserver.CollectionFailure, error) { settings, err := collectionOptions(values) if err != nil { return nil, err } - batch, err := collector.CollectWorkspaces(ctx, paths, values.scanDepth, settings, publish) + workspaces := collector.Workspaces{Paths: input.Paths, Depth: values.scanDepth, LinkedWorktrees: input.Selection == localserver.DiscoveredCheckouts} + batch, err := collector.CollectWorkspaces(ctx, workspaces, settings, publish) for _, diagnostic := range batch.Diagnostics { fmt.Fprintf(stderr, " diffx: %s\n", diagnostic) } @@ -163,7 +166,7 @@ func collectWorkspaces(ctx context.Context, values options, paths []string, stde return failures, nil } -func acquireLocalInput(values options, stdin *os.File) (initialInput, error) { +func acquireLocalInput(ctx context.Context, values options, stdin *os.File) (initialInput, error) { piped := false if stdin != nil { var err error @@ -194,5 +197,30 @@ func acquireLocalInput(values options, stdin *os.File) (initialInput, error) { } paths = append(paths, absolute) } - return initialInput{Kind: "worktree", Paths: paths}, nil + selection, err := checkoutSelection(ctx, values, paths) + if err != nil { + return initialInput{}, err + } + return initialInput{Kind: "worktree", Paths: paths, Selection: selection}, nil +} + +// checkoutSelection scopes a bare or positional checkout path to that checkout +// unless --scan-worktrees adds its repository's other worktrees. --path, and a +// directory outside a repository, discover every checkout and worktree below. +func checkoutSelection(ctx context.Context, values options, paths []string) (localserver.Selection, error) { + if values.pathSet || len(paths) != 1 { + return localserver.DiscoveredCheckouts, nil + } + if _, err := diffsource.OpenRepository(ctx, paths[0]); errors.Is(err, diffsource.ErrNotRepository) { + return localserver.DiscoveredCheckouts, nil + } else if err != nil { + return "", err + } + if values.scanDepthSet { + return "", errors.New("--scan-depth bounds directory search; a checkout path has none. Use --scan-worktrees for its worktrees") + } + if values.scanWorktrees { + return localserver.DiscoveredCheckouts, nil + } + return localserver.SelectedCheckout, nil } diff --git a/cmd/diffx/local_test.go b/cmd/diffx/local_test.go index db060423..c8e9b7c7 100644 --- a/cmd/diffx/local_test.go +++ b/cmd/diffx/local_test.go @@ -10,16 +10,18 @@ import ( "testing" "github.com/flexdinesh/diffx/internal/diffsource" + "github.com/flexdinesh/diffx/internal/ingestion" + "github.com/flexdinesh/diffx/internal/localserver" ) func TestLocalInputDefaultsToCurrentDirectory(t *testing.T) { root := t.TempDir() t.Chdir(root) - input, err := acquireLocalInput(options{directory: root, repositorySet: true}, nil) + input, err := acquireLocalInput(t.Context(), options{directory: root, repositorySet: true}, nil) if err != nil || input.Kind != "worktree" || !slices.Equal(input.Paths, []string{root}) { t.Fatalf("path input: %+v %v", input, err) } - input, err = acquireLocalInput(options{directory: "."}, nil) + input, err = acquireLocalInput(t.Context(), options{directory: "."}, nil) if err != nil || input.Kind != "worktree" || !slices.Equal(input.Paths, []string{root}) { t.Fatalf("default input: %+v %v", input, err) } @@ -32,7 +34,7 @@ func TestLocalInputDefaultsToCurrentDirectory(t *testing.T) { if err != nil { t.Fatal(err) } - input, err = acquireLocalInput(values, nil) + input, err = acquireLocalInput(t.Context(), values, nil) if err != nil || !slices.Equal(input.Paths, []string{filepath.Join(root, "one"), filepath.Join(root, "two")}) { t.Fatalf("repeated paths: %+v %v", input, err) } @@ -51,7 +53,7 @@ func TestLocalInputReadsPipeAndRedirectedFile(t *testing.T) { } writer.Close() for _, stdin := range []*os.File{reader, testInputFile(t, raw)} { - input, err := acquireLocalInput(options{directory: "."}, stdin) + input, err := acquireLocalInput(t.Context(), options{directory: "."}, stdin) if err != nil || input.Kind != "capture" || !bytes.Equal(input.Raw, raw) || !filepath.IsAbs(input.SubmittedFrom) { t.Fatalf("patch input: %+v %v", input, err) } @@ -65,7 +67,7 @@ func TestLocalInputRejectsPathWithRedirectedStdin(t *testing.T) { t.Fatal(err) } stdin := testInputFile(t, []byte("patch")) - if _, err := acquireLocalInput(values, stdin); err == nil || !strings.Contains(err.Error(), "cannot be combined") { + if _, err := acquireLocalInput(t.Context(), values, stdin); err == nil || !strings.Contains(err.Error(), "cannot be combined") { t.Fatalf("accepted path with stdin: %v", err) } position, err := stdin.Seek(0, io.SeekCurrent) @@ -95,3 +97,62 @@ func TestLocalPatchValidationBeforeStartup(t *testing.T) { }) } } + +func TestLocalInputScopesCheckoutPathsToTheirCheckout(t *testing.T) { + root := cliRepository(t) + linked := filepath.Join(t.TempDir(), "linked") + producerGit(t, root, "worktree", "add", "-b", "other", linked) + workspace := t.TempDir() + t.Chdir(root) + for _, test := range []struct { + arguments []string + want localserver.Selection + err string + }{ + {nil, localserver.SelectedCheckout, ""}, + {[]string{"."}, localserver.SelectedCheckout, ""}, + {[]string{linked}, localserver.SelectedCheckout, ""}, + {[]string{"--scan-worktrees"}, localserver.DiscoveredCheckouts, ""}, + {[]string{".", "--scan-worktrees"}, localserver.DiscoveredCheckouts, ""}, + {[]string{workspace}, localserver.DiscoveredCheckouts, ""}, + {[]string{workspace, "--scan-depth", "4"}, localserver.DiscoveredCheckouts, ""}, + {[]string{"--path", root}, localserver.DiscoveredCheckouts, ""}, + {[]string{"--path", root, "--path", linked}, localserver.DiscoveredCheckouts, ""}, + {[]string{"--scan-depth", "4"}, "", "--scan-depth bounds directory search"}, + {[]string{".", "--scan-worktrees", "--scan-depth", "4"}, "", "--scan-depth bounds directory search"}, + } { + values, err := parseOptions(test.arguments, io.Discard) + if err != nil { + t.Fatal(err) + } + input, err := acquireLocalInput(t.Context(), values, nil) + if test.err != "" { + if err == nil || !strings.Contains(err.Error(), test.err) { + t.Fatalf("%v: error %v, want %q", test.arguments, err, test.err) + } + continue + } + if err != nil || input.Selection != test.want { + t.Fatalf("%v: selection %q %v, want %q", test.arguments, input.Selection, err, test.want) + } + } + for selection, want := range map[localserver.Selection][]string{ + localserver.SelectedCheckout: {root}, + localserver.DiscoveredCheckouts: {root, linked}, + } { + values := options{directory: ".", sourceID: "test-source", trigger: "manual"} + var roots []string + input := initialInput{Kind: "worktree", Paths: []string{root}, Selection: selection} + if _, err := collectWorkspaces(t.Context(), values, input, io.Discard, func(request ingestion.Request) error { + roots = append(roots, request.Metadata.Root) + return nil + }); err != nil { + t.Fatal(err) + } + slices.Sort(roots) + slices.Sort(want) + if !slices.Equal(roots, want) { + t.Fatalf("%s collected %v, want %v", selection, roots, want) + } + } +} diff --git a/cmd/diffx/options.go b/cmd/diffx/options.go index 4eb78573..2a3ef853 100644 --- a/cmd/diffx/options.go +++ b/cmd/diffx/options.go @@ -27,6 +27,7 @@ type options struct { paths []string scanDepth int scanDepthSet bool + scanWorktrees bool repositorySet bool fixture string state string @@ -71,8 +72,8 @@ const maxScanDepth = 32 // singleCheckout rejects workspace discovery outside local mode. func (values options) singleCheckout() error { - if len(values.paths) > 1 || values.scanDepthSet { - return errors.New("multiple --path values and --scan-depth are only supported by local mode") + if len(values.paths) > 1 || values.scanDepthSet || values.scanWorktrees { + return errors.New("multiple --path values, --scan-depth and --scan-worktrees are only supported by local mode") } return nil } @@ -85,8 +86,9 @@ func parseOptionsMode(arguments []string, stderr io.Writer, allowHost bool) (opt flags := flag.NewFlagSet("diffx", flag.ContinueOnError) flags.SetOutput(stderr) values := options{host: "127.0.0.1", directory: ".", trigger: "manual"} - flags.Var(pathList{&values.paths}, "path", "checkout path to collect; local mode accepts it repeatedly and discovers checkouts below each") + flags.Var(pathList{&values.paths}, "path", "checkout path to collect; local mode accepts it repeatedly and discovers checkouts and their worktrees below each") flags.IntVar(&values.scanDepth, "scan-depth", 8, "local mode: directory levels searched below each path for repositories") + flags.BoolVar(&values.scanWorktrees, "scan-worktrees", false, "local mode: also collect the other worktrees of a checkout path's repository") flags.StringVar(&values.server, "server", values.server, "remote server URL for sync; overrides remote.url") flags.StringVar(&values.token, "token", values.token, "remote bearer token for sync; prefer DIFFX_REMOTE_TOKEN") flags.StringVar(&values.trigger, "trigger", values.trigger, "collection trigger: manual or agent-hook") @@ -114,7 +116,7 @@ func parseOptionsMode(arguments []string, stderr io.Writer, allowHost bool) (opt flags.BoolVar(&values.noBrowser, "no-browser", false, "do not open a browser") flags.BoolVar(&values.version, "version", false, "print version and exit") flags.Usage = func() { - fmt.Fprintln(stderr, " Usage: diffx [PATH] [--host IP] [--replace] [--reset-state]") + fmt.Fprintln(stderr, " Usage: diffx [PATH] [--scan-worktrees] [--host IP] [--replace] [--reset-state]") fmt.Fprintln(stderr, " diffx --path DIR [--path DIR...] [--scan-depth N]") fmt.Fprintln(stderr, " git diff | diffx [options]") fmt.Fprintln(stderr, " diffx sync [--path DIRECTORY] [--print] [--debug] [--retry]") diff --git a/cmd/diffx/options_test.go b/cmd/diffx/options_test.go index 0f4cee56..b869459e 100644 --- a/cmd/diffx/options_test.go +++ b/cmd/diffx/options_test.go @@ -64,9 +64,13 @@ func TestWorkspaceOptionsParseAndStayLocal(t *testing.T) { t.Fatalf("workspace options: %#v %v", values, err) } values, err = parseOptions([]string{"repo"}, io.Discard) - if err != nil || len(values.paths) != 1 || values.paths[0] != "repo" || values.scanDepth != 8 { + if err != nil || len(values.paths) != 1 || values.paths[0] != "repo" || values.scanDepth != 8 || values.scanWorktrees { t.Fatalf("positional path: %#v %v", values, err) } + values, err = parseOptions([]string{"repo", "--scan-worktrees"}, io.Discard) + if err != nil || !values.scanWorktrees || values.paths[0] != "repo" { + t.Fatalf("scan worktrees: %#v %v", values, err) + } for _, arguments := range [][]string{{"--scan-depth", "-1"}, {"--scan-depth", "33"}, {"--path", "one", "two"}} { if _, err := parseOptions(arguments, io.Discard); err == nil { t.Fatalf("accepted %v", arguments) @@ -75,6 +79,8 @@ func TestWorkspaceOptionsParseAndStayLocal(t *testing.T) { for _, arguments := range [][]string{ {"sync", "--path", "one", "--path", "two"}, {"sync", "--scan-depth", "2"}, + {"sync", "--scan-worktrees"}, + {"dev", "--fixture", "patch.diff", "--scan-worktrees"}, {"dev", "--fixture", "patch.diff", "--scan-depth", "2"}, } { if err := run(t.Context(), arguments, nil, io.Discard, io.Discard); err == nil || !strings.Contains(err.Error(), "only supported by local mode") { diff --git a/docs-website/src/pages/reviewing-changes.md b/docs-website/src/pages/reviewing-changes.md index 47c38f65..82befd9d 100644 --- a/docs-website/src/pages/reviewing-changes.md +++ b/docs-website/src/pages/reviewing-changes.md @@ -6,7 +6,9 @@ description: Navigate projects, snapshots, comments and reviewed marks. # Reviewing changes The Reviews column lists repositories by recent activity, each with its current -streams: one per source, checkout, branch and comparison. Each stream shows its +streams: one per source, checkout, branch and comparison. A local diffx shows +the column once there are at least two current streams to switch between; a +remote server always shows it. Each stream shows its newest snapshot only; clean or merged streams and superseded snapshots are hidden. A dot marks streams with changes you have not opened in this browser. Click a repository's name to open its most active stream; its chevron expands or @@ -15,7 +17,8 @@ collapsed across visits, and each mark opens that repository's latest stream. The header names the open review's repository, branch and worktree; the repository and branch are menus you can type into to filter, then press Enter to switch. In a local checkout -process, **Sync all checkouts** (Alt+R) collects every checkout again; like a +process, **Sync checkout** or **Sync all checkouts** (Alt+R) collects the +process's checkouts again; like a remote arrival, a newer snapshot of the open stream is announced, not swapped in. Collapsed files and folders are remembered across reloads in the same tab. When a newer snapshot arrives for the stream you are reviewing, a notice offers **View latest** without diff --git a/docs-website/src/pages/usage.md b/docs-website/src/pages/usage.md index b478e195..576ed036 100644 --- a/docs-website/src/pages/usage.md +++ b/docs-website/src/pages/usage.md @@ -10,22 +10,27 @@ Capture the checkouts under the selected paths in one foreground process: ```sh diffx diffx . +diffx . --scan-worktrees diffx /path/to/repo --host 0.0.0.0 +diffx ~/work --scan-depth 4 diffx --path ~/work --path ~/oss --scan-depth 4 diffx . --replace diffx . --base HEAD --no-browser ``` Collection and ingestion run in-process. The same process serves the web UI, -REST and MCP until Ctrl-C. The path defaults to the current directory; repeat -`--path` for several. A path inside a repository resolves to its checkout; -otherwise diffx searches up to `--scan-depth` levels (default 8) below it, -stopping at each repository and skipping hidden, `node_modules` and `vendor` -directories. Every registered worktree of each repository found is collected, -including worktrees outside the paths. Once collection finishes, the browser +REST and MCP until Ctrl-C. The path defaults to the current directory. When +that path, or one positional path, is inside a repository, diffx collects only +that checkout; add `--scan-worktrees` to also collect the repository's other +worktrees (`--scan-depth` does not apply there). With `--path` (repeat it for +several) or a directory outside a repository, a path inside a repository +resolves to its checkout; otherwise diffx searches up to `--scan-depth` levels +(default 8) below it, stopping at each repository and skipping hidden, +`node_modules` and `vendor` directories. Every registered worktree of each +repository found is collected, including worktrees outside the paths. Once collection finishes, the browser opens the changed checkout containing the path, otherwise the most recently active changed snapshot; later edits and branch switches do not change it until you sync, which -discovers the paths again and captures a new snapshot of every checkout. A +collects the same scope again and captures a new snapshot of every checkout in it. A checkout that fails to collect is reported without blocking the others and never substitutes an empty diff. Observations and comments persist across restarts in the shared local database; each process shows only its discovered checkouts or diff --git a/docs/architecture.md b/docs/architecture.md index f3fc1b6d..135299b6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -9,11 +9,11 @@ canonical record of package boundaries and architectural decisions. | Concern | Local `diffx [PATH]` | Remote `diffx-server` | | ---------- | ------------------------------------------------------------------- | ---------------------------------------- | -| Collection | Discovered checkouts at start, live page loads and sync; stdin once | Separate finite sync or hook producer | +| Collection | Selected checkout(s) at start, live page loads and sync; stdin once | Separate finite sync or hook producer | | Admission | Direct application call; startup runs after serving | Authenticated durable queue and worker | | Lifetime | Foreground; authenticated singleton replacement | Foreground; graceful worker shutdown | | Storage | One owner of persistent SQLite | One owner of persistent SQLite | -| Queries | Discovered checkouts or capture via REST/MCP | Same application services via REST/MCP | +| Queries | Selected checkout(s) or capture via REST/MCP | Same application services via REST/MCP | | Access | Public listener; private lifecycle credential | Account credentials wrap the application | | Identity | Automatic stable local owner; no account UI | Account sessions and scoped API tokens | @@ -26,10 +26,14 @@ validation and legacy interpretation; context queries expose stored scope summaries. Selecting a comparison reads the same immutable observation and never triggers collection or resolves the latest stream head. Manual sync and hook producers collect only the selected checkout. A local -checkout process discovers every repository under its paths (bounded depth, -stopping at repositories, skipping hidden directories) and collects all of their -registered worktrees, because one review process serves a developer's whole -workspace. Stored observations cannot refresh from Git. A local checkout process +process started on one checkout path does the same unless `--scan-worktrees` +adds that repository's registered worktrees. A local process given `--path` or +a directory outside a repository discovers every repository under its paths +(bounded depth, stopping at repositories, skipping hidden directories) and +collects all of their registered worktrees, because it serves a developer's +whole workspace. The entry point resolves this scope once at startup +(`collector.Workspaces`) and reports it in the bootstrap as `input: "checkout"` +or `"checkouts"`. Stored observations cannot refresh from Git. A local checkout process discovers and collects again only on user request (`POST /api/v2/collections`): opening a live URL or pressing Sync. Discovery feeds checkouts directly to four collection workers through a bounded pipeline. Each finished checkout publishes diff --git a/docs/development.md b/docs/development.md index ce2c498a..bb5fc509 100644 --- a/docs/development.md +++ b/docs/development.md @@ -319,12 +319,16 @@ latest-stream and branch-filter rules. Selecting a stream keeps the column and file sidebar open. `use-stream-nav.ts`, provided by `stream-nav-context.tsx`, owns the column's rail/list state, width and grouping. `shell-policy.ts` resolves review navigation once from the bootstrap (`input: "capture"` hides -all of it); components read that policy, never the deployment mode. +all of it) and `use-shell-policy.ts` reads it; components read that policy, +never the deployment mode. The policy's `reviewColumn` is `always` remotely and +`multiple` locally: the session owner shows the column once `streamCount` +reaches two and keeps it for the page session. Its `collection` names the Sync +scope. `use-sidebar.ts` owns file sidebar visibility and width and which mobile drawer is open, with its focus. `Navigation.tsx` owns the island and both resize handles. The selected context scopes every request. The session owner exposes local checkout sync when bootstrap reports `collect`; loading `/` or a live -`/streams/{streamId}` URL, the header's “Sync all checkouts” icon and Alt+R call +`/streams/{streamId}` URL, the header's “Sync checkout” or “Sync all checkouts” icon and Alt+R call `POST /api/v2/collections`. Load-time sync runs in the background while a live URL opens its stream's first available stored head; `/` waits for the collection's `defaultContextId`. Collection completion never replaces an diff --git a/docs/system.md b/docs/system.md index de49792b..86d90e23 100644 --- a/docs/system.md +++ b/docs/system.md @@ -36,16 +36,21 @@ The application core has two compositions. See [architecture.md](architecture.md for contracts, durability rules and migration details. `diffx [PATH]` is one foreground process: serve UI/REST/MCP immediately, then -discover and collect the checkouts under its paths and ingest directly in a -background task. Discovery feeds four workers as checkouts are found; a serial +collect its selected checkout, or discover and collect the checkouts under its +paths, and ingest directly in a background task. Discovery feeds four workers as checkouts are found; a serial publisher ingests each completed checkout immediately, so its snapshot becomes queryable while slower checkouts continue. Publication commits and emits the existing ingestion event for each observation. The UI opens available snapshots and shows loading feedback only for missing data; failures remain visible with Retry. -Omitted paths mean the current directory; `--path` repeats and `--scan-depth` (default 8) bounds the -search. Every path must be an existing directory; missing paths fail before serving. Captured snapshots remain fixed after edits and branch switches until the +Omitted paths mean the current directory. A bare or positional path inside a +checkout selects only that checkout; `--scan-worktrees` adds its repository's +other registered worktrees, and `--scan-depth` is rejected because a checkout +has no directory search. `--path` (repeatable) and a positional directory +outside a repository discover checkouts and all their worktrees, with +`--scan-depth` (default 8) bounding the search. The scope is decided at startup +and every later sync reuses it. Every path must be an existing directory; missing paths fail before serving. Captured snapshots remain fixed after edits and branch switches until the user requests collection by loading a live URL or pressing Sync, which discovers -the paths again and ingests a new snapshot of every checkout; nothing watches Git. +the paths again and ingests a new snapshot of every checkout in scope; nothing watches Git. Piped input is validated and collected once before serving. The home-directory database survives process termination, but each process serves only the checkouts it discovered (every branch and earlier run of them) @@ -79,7 +84,9 @@ and review operations. Their lifecycle and submission delivery differ. Collection resolves the baseline and all supported comparison scopes together with full-content identity and bounded immutable previews. It never fetches refs. Sync and hook collection select one checkout, resolving nested paths to its -root; their other registered worktrees are not collected. Local collection +root; their other registered worktrees are not collected. A local process +started on one checkout path collects that checkout the same way unless +`--scan-worktrees` is given. Otherwise local collection resolves each path inside a repository to its root, otherwise walks up to the scan depth, stopping at repositories and skipping hidden, `node_modules` and `vendor` directories, then collects every registered worktree of each repository @@ -227,7 +234,11 @@ head, the UI waits for collection; failures remain visible and offer Retry. Otherwise `/` opens the most recently active changed head's live URL. Startup selection and the local default MCP context use the same collection default. The Reviews column leads the navigation island, beside the open review's file -sidebar, aligned with the diff below the header. Switching streams replaces +sidebar, aligned with the diff below the header. A remote server always shows +it. A local checkout process shows it, its rail and the compact drawer toggle +only once the column would list two current streams, then keeps it for the +page session; until then the island holds only the file sidebar, while search, +the header comboboxes and sync remain. Switching streams replaces everything to its right and keeps the column. It collapses to a rail of repository marks. A repository (its rail mark or its name in the list) opens its most active stream; the list's chevron alone collapses or expands it. The diff --git a/internal/collector/discovery.go b/internal/collector/discovery.go index a46906f1..b04f93b8 100644 --- a/internal/collector/discovery.go +++ b/internal/collector/discovery.go @@ -79,17 +79,27 @@ func DiscoverCheckout(ctx context.Context, input, sourceID, base string) (Discov return result, nil } +// Workspaces selects the checkouts a local process collects. +type Workspaces struct { + Paths []string + // Depth bounds the directory levels walked below a path outside a repository. + Depth int + // LinkedWorktrees adds every registered worktree of each repository found; + // otherwise a repository contributes only the checkout containing its path. + LinkedWorktrees bool +} + // DiscoverWorkspaces finds every live checkout under the supplied paths. A path // inside a repository resolves to its root; otherwise directories are walked up // to depth levels, stopping at repositories and skipping hidden, node_modules -// and vendor directories. Each repository contributes all registered worktrees, -// including those outside the paths. It never fetches, switches branches, -// creates worktrees or follows symbolic links. -func DiscoverWorkspaces(ctx context.Context, inputs []string, depth int, sourceID, base string) (Discovery, error) { - return discoverWorkspaces(ctx, inputs, depth, sourceID, base, nil) +// and vendor directories. With LinkedWorktrees, each repository contributes all +// registered worktrees, including those outside the paths. It never fetches, +// switches branches, creates worktrees or follows symbolic links. +func DiscoverWorkspaces(ctx context.Context, workspaces Workspaces, sourceID, base string) (Discovery, error) { + return discoverWorkspaces(ctx, workspaces, sourceID, base, nil) } -func discoverWorkspaces(ctx context.Context, inputs []string, depth int, sourceID, base string, visit func(DiscoverySource) error) (Discovery, error) { +func discoverWorkspaces(ctx context.Context, workspaces Workspaces, sourceID, base string, visit func(DiscoverySource) error) (Discovery, error) { var result Discovery if base == "" { base = "auto" @@ -118,16 +128,19 @@ func discoverWorkspaces(ctx context.Context, inputs []string, depth int, sourceI result.Diagnostics = append(result.Diagnostics, fmt.Sprintf("identify repository %s: %v", root, err)) return nil } - // Enumeration deduplicates physical Git stores only. Independent clones - // can share a repository key while retaining distinct checkout identities. - if seenRepositories[facts.CommonDir] { - return nil - } - seenRepositories[facts.CommonDir] = true - _, worktrees, err := diffsource.RepositoryWorktrees(ctx, root) - if err != nil { - result.Diagnostics = append(result.Diagnostics, fmt.Sprintf("list worktrees %s: %v", root, err)) - return nil + worktrees := []diffsource.Worktree{{Root: root}} + if workspaces.LinkedWorktrees { + // Enumeration deduplicates physical Git stores only. Independent clones + // can share a repository key while retaining distinct checkout identities. + if seenRepositories[facts.CommonDir] { + return nil + } + seenRepositories[facts.CommonDir] = true + _, worktrees, err = diffsource.RepositoryWorktrees(ctx, root) + if err != nil { + result.Diagnostics = append(result.Diagnostics, fmt.Sprintf("list worktrees %s: %v", root, err)) + return nil + } } for _, worktree := range worktrees { if limitReached() { @@ -207,7 +220,7 @@ func discoverWorkspaces(ctx context.Context, inputs []string, depth int, sourceI } return nil } - for _, input := range inputs { + for _, input := range workspaces.Paths { input, err := filepath.Abs(input) if err != nil { return result, err @@ -232,7 +245,7 @@ func discoverWorkspaces(ctx context.Context, inputs []string, depth int, sourceI result.Diagnostics = append(result.Diagnostics, fmt.Sprintf("inspect %s: %v", input, err)) continue } - if err := scan(input, input, depth); err != nil { + if err := scan(input, input, workspaces.Depth); err != nil { return result, err } } diff --git a/internal/collector/discovery_test.go b/internal/collector/discovery_test.go index eda603a5..ace90845 100644 --- a/internal/collector/discovery_test.go +++ b/internal/collector/discovery_test.go @@ -51,7 +51,7 @@ func TestDiscoveryFindsRepositoriesAndAllTheirWorktrees(t *testing.T) { // Linked worktrees outside the scanned workspace still belong to the repository. linked := filepath.Join(t.TempDir(), "linked") git(t, root, "worktree", "add", "-b", "feature", linked) - result, err := DiscoverWorkspaces(t.Context(), []string{workspace}, 8, "machine", "auto") + result, err := DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || !slices.Equal(discoveredPaths(result), []string{root, linked}) { t.Fatalf("sources %v diagnostics %v err %v", discoveredPaths(result), result.Diagnostics, err) } @@ -60,12 +60,28 @@ func TestDiscoveryFindsRepositoriesAndAllTheirWorktrees(t *testing.T) { } identity := result.Sources[1].Identity git(t, root, "branch", "-m", "main", "master") - result, err = DiscoverWorkspaces(t.Context(), []string{workspace}, 8, "machine", "auto") + result, err = DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || len(result.Sources) != 2 || result.Sources[1].Identity != identity { t.Fatalf("worktree identity changed: %#v %v %v", result.Sources, result.Diagnostics, err) } } +func TestDiscoveryWithoutLinkedWorktreesSelectsOnlyTheContainingCheckout(t *testing.T) { + root := initialized(t, filepath.Join(t.TempDir(), "project")) + linked := filepath.Join(t.TempDir(), "linked") + git(t, root, "worktree", "add", "-b", "feature", linked) + nested := filepath.Join(linked, "src") + if err := os.Mkdir(nested, 0o700); err != nil { + t.Fatal(err) + } + for input, want := range map[string]string{root: root, nested: linked} { + result, err := DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{input}, Depth: 8}, "machine", "auto") + if err != nil || !slices.Equal(discoveredPaths(result), []string{want}) || result.Sources[0].InputPath != input { + t.Fatalf("input %s: sources %v diagnostics %v err %v", input, discoveredPaths(result), result.Diagnostics, err) + } + } +} + func TestDiscoveryStopsAtRepositoriesAndSkipsHiddenDirectories(t *testing.T) { workspace := t.TempDir() outer := initialized(t, filepath.Join(workspace, "outer")) @@ -76,12 +92,12 @@ func TestDiscoveryStopsAtRepositoriesAndSkipsHiddenDirectories(t *testing.T) { if err := os.Symlink(filepath.Join(workspace, ".hidden"), filepath.Join(workspace, "linked")); err != nil { t.Fatal(err) } - result, err := DiscoverWorkspaces(t.Context(), []string{workspace}, 8, "machine", "auto") + result, err := DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || !slices.Equal(discoveredPaths(result), []string{outer, visible}) { t.Fatalf("sources %v diagnostics %v err %v", discoveredPaths(result), result.Diagnostics, err) } hidden := filepath.Join(workspace, ".hidden") - result, err = DiscoverWorkspaces(t.Context(), []string{hidden}, 8, "machine", "auto") + result, err = DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{hidden}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || !slices.Equal(discoveredPaths(result), []string{filepath.Join(hidden, "project")}) { t.Fatalf("explicit hidden root skipped: %v %v %v", discoveredPaths(result), result.Diagnostics, err) } @@ -94,7 +110,7 @@ func TestDiscoveryResolvesNestedPathAndDeduplicatesOverlappingInputs(t *testing. if err := os.Mkdir(nested, 0o700); err != nil { t.Fatal(err) } - result, err := DiscoverWorkspaces(t.Context(), []string{nested, workspace, root}, 8, "machine", "auto") + result, err := DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{nested, workspace, root}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || !slices.Equal(discoveredPaths(result), []string{root}) || result.Sources[0].InputPath != nested { t.Fatalf("overlapping inputs: %#v %v %v", result.Sources, result.Diagnostics, err) } @@ -106,7 +122,7 @@ func TestDiscoveryIncludesIndependentClonesWithSameRemote(t *testing.T) { first, second := filepath.Join(workspace, "first"), filepath.Join(workspace, "second") git(t, workspace, "clone", original, first) git(t, workspace, "clone", original, second) - result, err := DiscoverWorkspaces(context.Background(), []string{workspace}, 8, "machine", "auto") + result, err := DiscoverWorkspaces(context.Background(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || !slices.Equal(discoveredPaths(result), []string{first, second}) || result.Sources[0].Identity == result.Sources[1].Identity { t.Fatalf("clones lost distinct checkout identities: %#v %v %v", result.Sources, result.Diagnostics, err) } @@ -125,7 +141,7 @@ func TestDiscoveryReportsCopiedGitIdentity(t *testing.T) { if err := os.CopyFS(second, os.DirFS(original)); err != nil { t.Fatal(err) } - result, err := DiscoverWorkspaces(t.Context(), []string{workspace}, 8, "machine", "auto") + result, err := DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || len(result.Sources) != 1 { t.Fatalf("copied identity discovery: %#v %v %v", result.Sources, result.Diagnostics, err) } @@ -144,7 +160,7 @@ func TestDiscoveryReportsMissingWorktree(t *testing.T) { if err := os.RemoveAll(linked); err != nil { t.Fatal(err) } - result, err := DiscoverWorkspaces(t.Context(), []string{root}, 8, "machine", "auto") + result, err := DiscoverWorkspaces(t.Context(), Workspaces{Paths: []string{root}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || !slices.Equal(discoveredPaths(result), []string{root}) || len(result.Diagnostics) != 1 || !strings.Contains(result.Diagnostics[0], "worktree unavailable") { t.Fatalf("missing worktree: %v %v %v", discoveredPaths(result), result.Diagnostics, err) } @@ -153,21 +169,21 @@ func TestDiscoveryReportsMissingWorktree(t *testing.T) { func TestDiscoveryBoundsDepthAndReportsMissingInput(t *testing.T) { workspace := t.TempDir() deep := initialized(t, filepath.Join(workspace, "one", "two", "three")) - result, err := DiscoverWorkspaces(context.Background(), []string{workspace}, 2, "machine", "auto") + result, err := DiscoverWorkspaces(context.Background(), Workspaces{Paths: []string{workspace}, Depth: 2, LinkedWorktrees: true}, "machine", "auto") if err != nil || len(result.Sources) != 0 { t.Fatalf("deep checkout escaped discovery bound: %#v %v", result, err) } - result, err = DiscoverWorkspaces(context.Background(), []string{workspace}, 3, "machine", "auto") + result, err = DiscoverWorkspaces(context.Background(), Workspaces{Paths: []string{workspace}, Depth: 3, LinkedWorktrees: true}, "machine", "auto") if err != nil || !slices.Equal(discoveredPaths(result), []string{deep}) { t.Fatalf("checkout within depth missed: %#v %v", result, err) } - result, err = DiscoverWorkspaces(context.Background(), []string{filepath.Join(workspace, "missing")}, 8, "machine", "auto") + result, err = DiscoverWorkspaces(context.Background(), Workspaces{Paths: []string{filepath.Join(workspace, "missing")}, Depth: 8, LinkedWorktrees: true}, "machine", "auto") if err != nil || len(result.Sources) != 0 || len(result.Diagnostics) == 0 { t.Fatalf("missing path lacks diagnostics: %#v %v", result, err) } ctx, cancel := context.WithCancel(context.Background()) cancel() - if _, err := DiscoverWorkspaces(ctx, []string{workspace}, 8, "machine", "auto"); err == nil { + if _, err := DiscoverWorkspaces(ctx, Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, "machine", "auto"); err == nil { t.Fatal("canceled discovery succeeded") } } @@ -180,7 +196,7 @@ func TestCollectWorkspacesCollectsEachCheckoutIndependently(t *testing.T) { write(t, second, "file.txt", "after\n") git(t, second, "add", ".") var requests []ingestion.Request - batch, err := CollectWorkspaces(t.Context(), []string{workspace}, 8, Options{SourceID: "machine", Hostname: "host", SubmissionID: "shared"}, func(input ingestion.Request) error { requests = append(requests, input); return nil }) + batch, err := CollectWorkspaces(t.Context(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, Options{SourceID: "machine", Hostname: "host", SubmissionID: "shared"}, func(input ingestion.Request) error { requests = append(requests, input); return nil }) slices.SortFunc(requests, func(a, b ingestion.Request) int { return strings.Compare(a.Metadata.Root, b.Metadata.Root) }) if err != nil || len(requests) != 2 || len(batch.Failures) != 0 { t.Fatalf("batch %#v err %v", batch, err) @@ -194,7 +210,7 @@ func TestCollectWorkspacesCollectsEachCheckoutIndependently(t *testing.T) { if requests[0].Metadata.TriggerRoot != workspace { t.Fatalf("trigger root %q", requests[0].Metadata.TriggerRoot) } - if _, err := CollectWorkspaces(t.Context(), []string{t.TempDir()}, 8, Options{SourceID: "machine", Hostname: "host"}, func(ingestion.Request) error { return nil }); err == nil || !strings.Contains(err.Error(), "no Git checkouts found") { + if _, err := CollectWorkspaces(t.Context(), Workspaces{Paths: []string{t.TempDir()}, Depth: 8, LinkedWorktrees: true}, Options{SourceID: "machine", Hostname: "host"}, func(ingestion.Request) error { return nil }); err == nil || !strings.Contains(err.Error(), "no Git checkouts found") { t.Fatalf("empty workspace: %v", err) } } @@ -208,7 +224,7 @@ func TestCollectWorkspacesCanonicalizesInputWithinItsCheckout(t *testing.T) { t.Fatal(err) } var requests []ingestion.Request - if _, err := CollectWorkspaces(t.Context(), []string{alias}, 8, Options{SourceID: "machine", Hostname: "host"}, func(input ingestion.Request) error { requests = append(requests, input); return nil }); err != nil || len(requests) != 2 { + if _, err := CollectWorkspaces(t.Context(), Workspaces{Paths: []string{alias}, Depth: 8, LinkedWorktrees: true}, Options{SourceID: "machine", Hostname: "host"}, func(input ingestion.Request) error { requests = append(requests, input); return nil }); err != nil || len(requests) != 2 { t.Fatalf("requests %d err %v", len(requests), err) } // Canonical checkout roots must contain the input to identify the launch checkout. @@ -227,7 +243,7 @@ func TestCollectWorkspacesReportsFailedCheckoutWithoutBlockingOthers(t *testing. // A base that cannot resolve fails only the checkout without that ref. git(t, healthy, "branch", "baseline") var requests []ingestion.Request - batch, err := CollectWorkspaces(t.Context(), []string{workspace}, 8, Options{SourceID: "machine", Hostname: "host", Base: "baseline"}, func(input ingestion.Request) error { requests = append(requests, input); return nil }) + batch, err := CollectWorkspaces(t.Context(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, Options{SourceID: "machine", Hostname: "host", Base: "baseline"}, func(input ingestion.Request) error { requests = append(requests, input); return nil }) if err != nil || len(requests) != 1 || requests[0].Metadata.Root != healthy { t.Fatalf("batch %#v err %v", batch, err) } @@ -256,7 +272,7 @@ func TestCollectWorkspacesPublishesBeforeOtherCheckoutFinishes(t *testing.T) { published := make(chan ingestion.Request, 2) done := make(chan error, 1) go func() { - _, err := CollectWorkspaces(t.Context(), []string{workspace}, 8, options, func(input ingestion.Request) error { + _, err := CollectWorkspaces(t.Context(), Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, options, func(input ingestion.Request) error { published <- input return nil }) @@ -318,7 +334,7 @@ func TestCollectWorkspacesJoinsWorkersAfterCancellationAndPublishFailure(t *test published := make(chan struct{}, 1) done := make(chan error, 1) go func() { - _, err := CollectWorkspaces(ctx, []string{workspace}, 8, options, func(ingestion.Request) error { + _, err := CollectWorkspaces(ctx, Workspaces{Paths: []string{workspace}, Depth: 8, LinkedWorktrees: true}, options, func(ingestion.Request) error { <-blocked published <- struct{}{} if failPublish { @@ -410,7 +426,7 @@ exec "$DIFFX_TEST_REAL_GIT" "$@" stopped := make(chan struct{}) go func() { defer close(stopped) - _, err := CollectWorkspaces(ctx, []string{first, second}, 8, Options{SourceID: "machine"}, func(input ingestion.Request) error { + _, err := CollectWorkspaces(ctx, Workspaces{Paths: []string{first, second}, Depth: 8, LinkedWorktrees: true}, Options{SourceID: "machine"}, func(input ingestion.Request) error { published <- input return nil }) diff --git a/internal/collector/workspaces.go b/internal/collector/workspaces.go index 2df75536..83d4f9df 100644 --- a/internal/collector/workspaces.go +++ b/internal/collector/workspaces.go @@ -31,7 +31,7 @@ type Batch struct { // publish runs serially as requests finish. Its failure cancels and joins all // remaining collection before returning. Four workers and a bounded result // buffer limit completed requests waiting to publish. -func CollectWorkspaces(ctx context.Context, paths []string, depth int, options Options, publish func(ingestion.Request) error) (Batch, error) { +func CollectWorkspaces(ctx context.Context, workspaces Workspaces, options Options, publish func(ingestion.Request) error) (Batch, error) { options, err := defaults(options) if err != nil { return Batch{}, err @@ -60,7 +60,7 @@ func CollectWorkspaces(ctx context.Context, paths []string, depth int, options O } discoveryDone := make(chan discovered, 1) go func() { - value, err := discoverWorkspaces(ctx, paths, depth, options.SourceID, options.Base, func(source DiscoverySource) error { + value, err := discoverWorkspaces(ctx, workspaces, options.SourceID, options.Base, func(source DiscoverySource) error { select { case jobs <- source: return nil @@ -122,7 +122,7 @@ func CollectWorkspaces(ctx context.Context, paths []string, depth int, options O return batch, discovery.err } if len(discovery.value.Sources) == 0 { - detail := "no Git checkouts found in " + strings.Join(paths, ", ") + detail := "no Git checkouts found in " + strings.Join(workspaces.Paths, ", ") if len(batch.Diagnostics) > 0 { detail += ": " + strings.Join(batch.Diagnostics, "; ") } diff --git a/internal/localserver/foreground.go b/internal/localserver/foreground.go index 909c7dc2..1e956264 100644 --- a/internal/localserver/foreground.go +++ b/internal/localserver/foreground.go @@ -19,13 +19,21 @@ type ( CollectionFailure = serverapp.CollectionFailure Collect = serverapp.Collect Published = serverapp.Published + Selection = serverapp.Selection +) + +const ( + SelectedCheckout = serverapp.SelectedCheckout + DiscoveredCheckouts = serverapp.DiscoveredCheckouts ) // Input supplies a fixed capture or checkout collection to run in the -// background at startup and again on explicit user request. +// background at startup and again on explicit user request. Selection names +// the scope Collect gathers. type Input struct { Collection Collection Collect Collect + Selection Selection } // RunForeground serializes discovery, replacement and publication. Shutdown @@ -65,7 +73,7 @@ func RunForeground(ctx context.Context, settings Settings, input Input, confirm defer RemoveDescriptor(client.RuntimeDirectory, id) ctx, cancel := context.WithCancel(ctx) defer cancel() - return runServer(ctx, cancel, settings, client.RuntimeDirectory, id, input.Collection, input.Collect, reset, func(status Status, published Published) { + return runServer(ctx, cancel, settings, client.RuntimeDirectory, id, input, reset, func(status Status, published Published) { _ = lifecycle.Close() if ready != nil { ready(status, published) diff --git a/internal/localserver/server.go b/internal/localserver/server.go index d4902b47..003b2733 100644 --- a/internal/localserver/server.go +++ b/internal/localserver/server.go @@ -61,9 +61,9 @@ func Run(ctx context.Context, settings Settings, runtimeDir string, initial *ing } defer output.Close() logger := log.New(output, "diffx: ", log.LstdFlags) - return runServer(ctx, cancel, settings, runtimeDir, instanceID, collection, nil, reset, published, logger) + return runServer(ctx, cancel, settings, runtimeDir, instanceID, Input{Collection: collection}, reset, published, logger) } - return runServer(ctx, cancel, settings, "", instanceID, collection, nil, reset, published, log.New(os.Stderr, "diffx: ", log.LstdFlags)) + return runServer(ctx, cancel, settings, "", instanceID, Input{Collection: collection}, reset, published, log.New(os.Stderr, "diffx: ", log.LstdFlags)) } // StateReset is an entry point's decision to replace local state. Remote @@ -115,7 +115,8 @@ func resetState(path string, reset StateReset) error { // runServer scopes reads to the capture or discovered checkouts. Checkout // discovery and collection start after serving, with an initially empty scope. -func runServer(ctx context.Context, cancel context.CancelFunc, settings Settings, runtimeDir, instanceID string, initial Collection, collect Collect, reset StateReset, ready func(Status, Published), logger *log.Logger) error { +func runServer(ctx context.Context, cancel context.CancelFunc, settings Settings, runtimeDir, instanceID string, input Input, reset StateReset, ready func(Status, Published), logger *log.Logger) error { + initial, collect := input.Collection, input.Collect settings = normalizeSettings(settings) if settings.RetentionDays < 1 || settings.RetentionDays > 106751 { return errors.New("retention-days must be between 1 and 106751") @@ -204,7 +205,7 @@ func runServer(ctx context.Context, cancel context.CancelFunc, settings Settings configuration := serverapp.Options{} tasks := []serverapp.Task{serverapp.Prune(store.PruneExpired)} if collect != nil { - collections := serverapp.NewCollections(service, collect, defaultContextID) + collections := serverapp.NewCollections(service, input.Selection, collect, defaultContextID) configuration.Collections = collections tasks = append(tasks, collections.Initial(func(published Published, err error) { if err != nil { diff --git a/internal/localserver/server_test.go b/internal/localserver/server_test.go index 5b440203..e048408e 100644 --- a/internal/localserver/server_test.go +++ b/internal/localserver/server_test.go @@ -58,18 +58,18 @@ func TestCheckoutServerServesBeforeCollectionAndWaitsForCollectorShutdown(t *tes stopped := make(chan struct{}) go func() { defer close(stopped) - done <- runServer(ctx, cancel, Settings{Host: "127.0.0.1", Port: 0, State: state}, "", "async-test", Collection{}, - func(ctx context.Context, _ func(ingestion.Request) error) ([]CollectionFailure, error) { + done <- runServer(ctx, cancel, Settings{Host: "127.0.0.1", Port: 0, State: state}, "", "async-test", Input{Selection: DiscoveredCheckouts, + Collect: func(ctx context.Context, _ func(ingestion.Request) error) ([]CollectionFailure, error) { <-ctx.Done() close(collectionCanceled) <-release return nil, ctx.Err() - }, StateReset{}, func(status Status, published Published) { - if len(published.Publications) != 0 { - t.Error("ready must not wait for checkout collection") - } - ready <- status - }, log.New(io.Discard, "", 0)) + }}, StateReset{}, func(status Status, published Published) { + if len(published.Publications) != 0 { + t.Error("ready must not wait for checkout collection") + } + ready <- status + }, log.New(io.Discard, "", 0)) }() t.Cleanup(func() { cancel() diff --git a/internal/serverapp/capabilities_test.go b/internal/serverapp/capabilities_test.go index fa26be7a..445482eb 100644 --- a/internal/serverapp/capabilities_test.go +++ b/internal/serverapp/capabilities_test.go @@ -31,14 +31,16 @@ import ( func TestAdmissionCapabilitiesMatchHealthAndRoutes(t *testing.T) { for _, test := range []struct { - name string - queue bool - collect bool - want serverapp.Capabilities + name string + queue bool + selection serverapp.Selection + want serverapp.Capabilities + input string }{ - {"foreground capture", false, false, serverapp.Capabilities{}}, - {"foreground checkout", false, true, serverapp.Capabilities{Collection: true}}, - {"remote queued", true, false, serverapp.Capabilities{IngestionEnabled: true, QueuedIngestion: true}}, + {"foreground capture", false, "", serverapp.Capabilities{}, "capture"}, + {"foreground checkout", false, serverapp.SelectedCheckout, serverapp.Capabilities{Collection: true, Selection: serverapp.SelectedCheckout}, "checkout"}, + {"foreground checkouts", false, serverapp.DiscoveredCheckouts, serverapp.Capabilities{Collection: true, Selection: serverapp.DiscoveredCheckouts}, "checkouts"}, + {"remote queued", true, "", serverapp.Capabilities{IngestionEnabled: true, QueuedIngestion: true}, "capture"}, } { t.Run(test.name, func(t *testing.T) { store, err := reviewstore.Open(":memory:") @@ -56,8 +58,8 @@ func TestAdmissionCapabilitiesMatchHealthAndRoutes(t *testing.T) { if test.queue { options.Queue = store.Queue() } - if test.collect { - options.Collections = serverapp.NewCollections(service, func(context.Context, func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { + if test.selection != "" { + options.Collections = serverapp.NewCollections(service, test.selection, func(context.Context, func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { return nil, errors.New("unused") }, "") } @@ -78,8 +80,8 @@ func TestAdmissionCapabilitiesMatchHealthAndRoutes(t *testing.T) { if bootstrap.Collect != test.want.Collection { t.Fatalf("bootstrap collect: %v, want %v", bootstrap.Collect, test.want.Collection) } - if want := map[bool]string{false: "capture", true: "checkouts"}[test.want.Collection]; bootstrap.Input != want { - t.Fatalf("bootstrap input: %q, want %q", bootstrap.Input, want) + if bootstrap.Input != test.input { + t.Fatalf("bootstrap input: %q, want %q", bootstrap.Input, test.input) } collections := request(t, d, "GET", "/api/v2/collections", nil) collections.Body.Close() @@ -268,7 +270,7 @@ func TestCollectionIngestsEveryCheckoutAndMovesMCPDefault(t *testing.T) { } return collection.Failures, nil } - server := httptest.NewServer(serverapp.Handler(t.Context(), service, store, fstest.MapFS{}, first.Context.ID, serverapp.Options{Collections: serverapp.NewCollections(service, collect, first.Context.ID)})) + server := httptest.NewServer(serverapp.Handler(t.Context(), service, store, fstest.MapFS{}, first.Context.ID, serverapp.Options{Collections: serverapp.NewCollections(service, serverapp.DiscoveredCheckouts, collect, first.Context.ID)})) defer server.Close() d := deployment{server: server} mcpDiff := func() review.RepositoryDiff { diff --git a/internal/serverapp/collection.go b/internal/serverapp/collection.go index 6aff80f3..5c33658f 100644 --- a/internal/serverapp/collection.go +++ b/internal/serverapp/collection.go @@ -150,17 +150,28 @@ func FailureSummary(failures []CollectionFailure) string { return strings.Join(details, "; ") } +// Selection names which checkouts a local process collects. +type Selection string + +const ( + // SelectedCheckout collects only the checkout the process was started for. + SelectedCheckout Selection = "checkout" + // DiscoveredCheckouts collects every checkout discovered under the paths. + DiscoveredCheckouts Selection = "checkouts" +) + // Collections serializes checkout collection and publication. The first UI // load request joins startup, including when startup finished before the browser opened. // Later requests collect fresh snapshots. A disconnected browser does not cancel // startup, whose lifetime belongs to the foreground process. type Collections struct { - mu sync.Mutex - pending *collectionResult - gate chan struct{} - service *contextservice.Service - collect Collect - current *defaultContext + mu sync.Mutex + pending *collectionResult + gate chan struct{} + service *contextservice.Service + selection Selection + collect Collect + current *defaultContext } type collectionResult struct { @@ -169,8 +180,8 @@ type collectionResult struct { err error } -func NewCollections(service *contextservice.Service, collect Collect, defaultID string) *Collections { - return &Collections{service: service, collect: collect, current: &defaultContext{id: defaultID}, gate: make(chan struct{}, 1)} +func NewCollections(service *contextservice.Service, selection Selection, collect Collect, defaultID string) *Collections { + return &Collections{service: service, selection: selection, collect: collect, current: &defaultContext{id: defaultID}, gate: make(chan struct{}, 1)} } // Initial reserves the startup result before serving requests and returns the diff --git a/internal/serverapp/collection_test.go b/internal/serverapp/collection_test.go index 3add1638..9c0f9026 100644 --- a/internal/serverapp/collection_test.go +++ b/internal/serverapp/collection_test.go @@ -123,7 +123,7 @@ func TestOlderCollectionRemainsSuccessfulWithoutRegressingStreamHead(t *testing. t.Fatalf("successful older collection lost its default: %+v, %v", fallback, ok) } var handler *Collections - handler = NewCollections(service, func(_ context.Context, publish func(ingestion.Request) error) ([]CollectionFailure, error) { + handler = NewCollections(service, DiscoveredCheckouts, func(_ context.Context, publish func(ingestion.Request) error) ([]CollectionFailure, error) { request.SubmissionID = "earlier-clock-again" if err := publish(request); err != nil { return nil, err diff --git a/internal/serverapp/handler.go b/internal/serverapp/handler.go index c9741083..e949b4f2 100644 --- a/internal/serverapp/handler.go +++ b/internal/serverapp/handler.go @@ -35,24 +35,31 @@ type Capabilities struct { IngestionEnabled bool QueuedIngestion bool Collection bool + // Selection is the collected checkouts' scope; empty without collection. + Selection Selection } -// Input names what a local process serves: discovered checkouts, which it can -// collect again, or one fixed capture (piped stdin or a fixture) with nothing -// to switch to. Only checkout processes own collections. +// Input names what a local process serves: its selected checkout or the +// discovered checkouts, which it can collect again, or one fixed capture +// (piped stdin or a fixture) with nothing to switch to. Only checkout +// processes own collections. func (capabilities Capabilities) Input() string { if capabilities.Collection { - return "checkouts" + return string(capabilities.Selection) } return "capture" } func (options Options) Capabilities() Capabilities { - return Capabilities{ + capabilities := Capabilities{ IngestionEnabled: options.Queue != nil, QueuedIngestion: options.Queue != nil, Collection: options.Collections != nil, } + if options.Collections != nil { + capabilities.Selection = options.Collections.selection + } + return capabilities } func Handler(ctx context.Context, service *contextservice.Service, store reviewservice.MutationStore, assets fs.FS, defaultID string, options ...Options) http.Handler { diff --git a/internal/serverapp/startup_collection_test.go b/internal/serverapp/startup_collection_test.go index 8ccbe4df..fee72005 100644 --- a/internal/serverapp/startup_collection_test.go +++ b/internal/serverapp/startup_collection_test.go @@ -100,7 +100,7 @@ func TestInitialCollectionJoinsFirstRequestAndLaterRequestsCollectAgain(t *testi service, _ := collectionService(t) started, release := make(chan struct{}), make(chan struct{}) var calls atomic.Int32 - collections := serverapp.NewCollections(service, func(ctx context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { + collections := serverapp.NewCollections(service, serverapp.DiscoveredCheckouts, func(ctx context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { call := calls.Add(1) if call == 1 { close(started) @@ -167,7 +167,7 @@ func TestInitialCollectionFailureIsReportedThenRetryCollectsAgain(t *testing.T) input := collectGit(t) service, _ := collectionService(t) var calls atomic.Int32 - collections := serverapp.NewCollections(service, func(_ context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { + collections := serverapp.NewCollections(service, serverapp.DiscoveredCheckouts, func(_ context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { if calls.Add(1) == 1 { return nil, errors.New("checkout unavailable") } @@ -197,7 +197,7 @@ func TestExplicitSyncCollectsFreshBeforeAnyBrowserInitialRequest(t *testing.T) { input := collectGit(t) service, _ := collectionService(t) var calls atomic.Int32 - collections := serverapp.NewCollections(service, func(_ context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { + collections := serverapp.NewCollections(service, serverapp.DiscoveredCheckouts, func(_ context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { fresh := input if calls.Add(1) > 1 { fresh.SubmissionID = "explicit-sync" @@ -222,7 +222,7 @@ func TestBrowserCancellationDoesNotCancelInitialCollection(t *testing.T) { service, _ := collectionService(t) started, release := make(chan struct{}), make(chan struct{}) var calls atomic.Int32 - collections := serverapp.NewCollections(service, func(ctx context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { + collections := serverapp.NewCollections(service, serverapp.DiscoveredCheckouts, func(ctx context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { if calls.Add(1) > 1 { return nil, publish(input) } @@ -267,7 +267,7 @@ func TestCollectionScopeStartsEmptyAndPublishesOnlySelectedCheckouts(t *testing. if err != nil { t.Fatal(err) } - collections := serverapp.NewCollections(service, func(_ context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { + collections := serverapp.NewCollections(service, serverapp.DiscoveredCheckouts, func(_ context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { return nil, publish(input) }, "") assets := fstest.MapFS{"index.html": {Data: []byte("review UI")}} @@ -324,7 +324,7 @@ func TestInitialCollectionMakesReadyCheckoutReadableWhileAnotherIsCollecting(t * var releaseOnce sync.Once t.Cleanup(func() { releaseOnce.Do(func() { close(release) }) }) var observed int - collections := serverapp.NewCollections(service, func(ctx context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { + collections := serverapp.NewCollections(service, serverapp.DiscoveredCheckouts, func(ctx context.Context, publish func(ingestion.Request) error) ([]serverapp.CollectionFailure, error) { options := collector.Options{SourceID: "streaming-startup", OnSource: func(ingestion.Metadata, diffsource.Comparison) { observed++ if observed == 2 { @@ -335,7 +335,7 @@ func TestInitialCollectionMakesReadyCheckoutReadableWhileAnotherIsCollecting(t * } } }} - batch, err := collector.CollectWorkspaces(ctx, []string{first, second}, 8, options, publish) + batch, err := collector.CollectWorkspaces(ctx, collector.Workspaces{Paths: []string{first, second}, Depth: 8, LinkedWorktrees: true}, options, publish) failures := make([]serverapp.CollectionFailure, 0, len(batch.Failures)) for _, failure := range batch.Failures { failures = append(failures, serverapp.CollectionFailure{Path: failure.Path, Detail: failure.Detail}) diff --git a/packages/api/openapi.yaml b/packages/api/openapi.yaml index 7af85778..80b12832 100644 --- a/packages/api/openapi.yaml +++ b/packages/api/openapi.yaml @@ -948,7 +948,7 @@ paths: /api/v2/collections: post: operationId: collectCheckouts - description: Local checkout mode only. Discovers the process's selected paths again and collects every checkout and linked worktree found, ingesting each as a fresh observation. Absent when the process serves piped input, a fixture or remote storage. + description: Local checkout mode only. Collects the process's selected checkout again, or discovers its selected paths again and collects every checkout and linked worktree found, ingesting each as a fresh observation. Absent when the process serves piped input, a fixture or remote storage. parameters: - name: initial in: query @@ -1541,8 +1541,8 @@ components: description: Whether this process can collect its selected checkouts again through POST /api/v2/collections. input: type: string - enum: [checkouts, capture] - description: What this process serves. `checkouts` reviews discovered checkouts; `capture` reviews one fixed patch (piped stdin or a fixture) and has nothing to switch to. + enum: [checkout, checkouts, capture] + description: What this process serves. `checkout` reviews one selected checkout; `checkouts` reviews discovered checkouts and their worktrees; `capture` reviews one fixed patch (piped stdin or a fixture) and has nothing to switch to. - type: object additionalProperties: false required: [mode, googleEnabled, user, csrfToken] diff --git a/packages/api/src/client.ts b/packages/api/src/client.ts index 10888593..8972ecc1 100644 --- a/packages/api/src/client.ts +++ b/packages/api/src/client.ts @@ -53,7 +53,9 @@ export function readBootstrap(value: unknown): ApiBootstrap { value.user === null && value.csrfToken === null && typeof value.collect === "boolean" && - (value.input === "checkouts" || value.input === "capture") + (value.input === "checkout" || + value.input === "checkouts" || + value.input === "capture") ) return { mode: "local", diff --git a/packages/api/src/schema.d.ts b/packages/api/src/schema.d.ts index 6cad1643..c78a1589 100644 --- a/packages/api/src/schema.d.ts +++ b/packages/api/src/schema.d.ts @@ -523,7 +523,7 @@ export interface paths { }; get?: never; put?: never; - /** @description Local checkout mode only. Discovers the process's selected paths again and collects every checkout and linked worktree found, ingesting each as a fresh observation. Absent when the process serves piped input, a fixture or remote storage. */ + /** @description Local checkout mode only. Collects the process's selected checkout again, or discovers its selected paths again and collects every checkout and linked worktree found, ingesting each as a fresh observation. Absent when the process serves piped input, a fixture or remote storage. */ post: operations["collectCheckouts"]; delete?: never; options?: never; @@ -849,10 +849,10 @@ export interface components { /** @description Whether this process can collect its selected checkouts again through POST /api/v2/collections. */ collect: boolean; /** - * @description What this process serves. `checkouts` reviews discovered checkouts; `capture` reviews one fixed patch (piped stdin or a fixture) and has nothing to switch to. + * @description What this process serves. `checkout` reviews one selected checkout; `checkouts` reviews discovered checkouts and their worktrees; `capture` reviews one fixed patch (piped stdin or a fixture) and has nothing to switch to. * @enum {string} */ - input: "checkouts" | "capture"; + input: "checkout" | "checkouts" | "capture"; } | { /** @enum {string} */ diff --git a/test/conformance/foreground_test.go b/test/conformance/foreground_test.go index aae3491d..22bdfd22 100644 --- a/test/conformance/foreground_test.go +++ b/test/conformance/foreground_test.go @@ -230,7 +230,7 @@ func TestForegroundSnapshotReplacementAndPersistentHistory(t *testing.T) { t.Fatal("foreground instance unavailable") return localserver.Status{} } - original, done := start("") + original, done := start("", "--scan-worktrees") status := await("") if status.PID != original.Process.Pid { t.Fatalf("wrong foreground owner: %+v", status) @@ -247,7 +247,7 @@ func TestForegroundSnapshotReplacementAndPersistentHistory(t *testing.T) { t.Fatalf("no context for %s: %+v", root, page) return contextservice.Context{} } - // Local collection includes every registered worktree of the repository. + // --scan-worktrees includes every registered worktree of the repository. if len(initial.Contexts) != 2 { t.Fatalf("local collection missed sibling worktree: %+v", initial) } @@ -371,7 +371,8 @@ func TestForegroundSnapshotReplacementAndPersistentHistory(t *testing.T) { t.Fatalf("other checkout context %s visible: %d", id, code) } } - // Reopening the first checkout shows its repository's persistent history again. + // Reopening the first checkout alone shows its persistent history again, + // without its sibling worktree. _, reopenedDone := start("", "--replace") reopened := await(replacement.InstanceID) select { @@ -386,14 +387,18 @@ func TestForegroundSnapshotReplacementAndPersistentHistory(t *testing.T) { requestJSON(t, reopened.BrowserURL+"/api/v2/contexts", &history) found := map[string]bool{} for _, item := range history.Contexts { - if item.Root == nil || (*item.Root != first && *item.Root != second) { + if item.Root == nil || *item.Root != first { t.Fatalf("reopened checkout showed another checkout: %+v", history) } found[item.ID] = true } - if !found[entry.ID] || !found[refreshed.ID] || !found[linkedEntry.ID] { + if !found[entry.ID] || !found[refreshed.ID] { t.Fatalf("reopened checkout lost history: %+v", history) } + requestJSON(t, reopened.BrowserURL+"/api/v2/bootstrap", &bootstrap) + if bootstrap.Input != "checkout" { + t.Fatalf("single checkout process input: %+v", bootstrap) + } // The server may use its five-second shutdown grace before forcing a // connection closed; leave time for descriptor and ownership cleanup too. ctx, cancel := context.WithTimeout(t.Context(), 8*time.Second)