diff --git a/apps/web/src/session-context.tsx b/apps/web/src/session-context.tsx index 7f05d340..4acce3f0 100644 --- a/apps/web/src/session-context.tsx +++ b/apps/web/src/session-context.tsx @@ -314,12 +314,14 @@ export function SessionProvider({ children }: { children: ReactNode }) { window.addEventListener("popstate", handleNavigation); return () => window.removeEventListener("popstate", handleNavigation); }, []); - // Open available catalog data while local collection continues. Once selected, - // a snapshot stays open; collection only announces newer heads. + // Live URLs open available catalog data while local collection continues; the + // root waits for the collection's default. Once selected, a snapshot stays + // open; collection only announces newer heads. const collected = useRef(null); const [collectionResult, setCollectionResult] = useState<{ route: Route; contexts: ApiContext[]; + defaultContextId: string; } | null>(null); const resolving = route.kind !== "pinned" && !selectedId && !missing; const collectionFinished = collectionResult?.route === route; @@ -334,7 +336,11 @@ export function SessionProvider({ children }: { children: ReactNode }) { setCollectError( `Unable to sync ${result.failures.length} ${result.failures.length === 1 ? "checkout" : "checkouts"}: ${result.failures.map((failure) => `${failure.path} (${failure.detail})`).join(", ")}`, ); - setCollectionResult({ route, contexts: result?.contexts ?? [] }); + setCollectionResult({ + route, + contexts: result?.contexts ?? [], + defaultContextId: result?.defaultContextId ?? "", + }); }); }, [collectOnLoad, route, collectLatest]); useEffect(() => { @@ -343,10 +349,23 @@ export function SessionProvider({ children }: { children: ReactNode }) { route.kind === "live" ? contexts.find((context) => context.streamId === route.stream) : latestChangedContext(contexts); + const collectedHead = + collectionResult?.route !== route + ? undefined + : route.kind === "live" + ? findHead(collectionResult.contexts) + : findHead( + collectionResult.contexts.filter( + (context) => context.id === collectionResult.defaultContextId, + ), + ); + // Only the finished collection names the root's default, such as the + // launched checkout; earlier heads are whichever checkouts finished first. const head = - (collectionResult?.route === route - ? findHead(collectionResult.contexts) - : undefined) ?? findHead(heads.contexts); + collectedHead ?? + (route.kind === "root" && collectOnLoad + ? undefined + : findHead(heads.contexts)); if (head) openStream(head, { replace: true, diff --git a/apps/web/test/browser/local-workspace.spec.ts b/apps/web/test/browser/local-workspace.spec.ts index 7dbeafb4..776a9cdc 100644 --- a/apps/web/test/browser/local-workspace.spec.ts +++ b/apps/web/test/browser/local-workspace.spec.ts @@ -1845,7 +1845,9 @@ test("local first launch waits for collection with an empty catalog", async ({ await expect.poll(() => collections).toBe(1); if (!pending) throw new Error("Missing startup collection"); heads = [snapshot]; - await pending.fulfill({ json: { contexts: heads, failures: [] } }); + await pending.fulfill({ + json: { contexts: heads, failures: [], defaultContextId: snapshot.id }, + }); await ready(page); await expect(page).toHaveURL(/\/streams\/local-stream$/); expect(collections).toBe(1); @@ -1945,6 +1947,66 @@ for (const initialHeads of ["stored", "arriving"]) { }); } +test("local root waits for collection to open its default over earlier heads", async ({ + page, +}) => { + const state = await localWorkspace(page); + await page.route("**/api/v2/bootstrap", (route) => + route.fulfill({ + json: { + mode: "local", + signupEnabled: false, + user: null, + csrfToken: null, + collect: true, + }, + }), + ); + const snapshot = (id: string, activity: number): ApiContext => ({ + ...state.context, + id, + streamId: `${id}-stream`, + latestContextId: id, + lastChangedAt: activity, + }); + // The first checkouts to finish are visible before the launched checkout. + const active = snapshot("active", 900); + const launched = snapshot("launched", 100); + let heads = [active]; + await page.route("**/api/v2/contexts?*", (route) => + route.fulfill({ json: { contexts: heads, nextCursor: null } }), + ); + const opened: string[] = []; + await page.route("**/api/v2/contexts/*", (route) => { + const id = new URL(route.request().url()).pathname.split("/").at(-1) ?? ""; + opened.push(id); + return route.fulfill({ json: id === "active" ? active : launched }); + }); + let pending: Route | undefined; + await page.route("**/api/v2/collections*", (route) => { + pending = route; + }); + + await page.goto("/"); + await expect.poll(() => pending !== undefined).toBe(true); + 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: "Repos" }).locator(".stream-row"), + ).toHaveCount(1); + await expect(page).toHaveURL(/\/$/); + expect(opened).toEqual([]); + if (!pending) throw new Error("Missing initial collection"); + heads = [active, launched]; + await pending.fulfill({ + json: { contexts: heads, failures: [], defaultContextId: launched.id }, + }); + await ready(page); + await expect(page).toHaveURL(/\/streams\/launched-stream$/); + expect(opened).toEqual(["launched"]); +}); + test("unresolved navigation joins collection without reporting a missing stream", async ({ page, }) => { @@ -2088,7 +2150,13 @@ for (const path of ["/", "/streams/local-stream"]) { await expect.poll(() => collections).toBe(2); if (!pending) throw new Error("Missing retried collection"); heads = [{ ...state.context, streamId: "local-stream" }]; - await pending.fulfill({ json: { contexts: heads, failures: [] } }); + await pending.fulfill({ + json: { + contexts: heads, + failures: [], + defaultContextId: state.context.id, + }, + }); await ready(page); await expect(page).toHaveURL(/\/streams\/local-stream$/); await expect(page.getByRole("alert")).toHaveCount(0); @@ -2119,6 +2187,7 @@ test("local startup reports failed checkouts while opening collected changes", a json: { contexts: [{ ...state.context, streamId: "local-stream" }], failures: [{ path: "/workspace/broken", detail: "git failed" }], + defaultContextId: state.context.id, }, }), ); @@ -2173,7 +2242,7 @@ test("local live URLs sync the checkouts on every load", async ({ page }) => { status: 422, json: { detail: "Unable to collect checkouts: git failed" }, }); - // The root opens the newest changed head; live URLs open their own stream. + // Live URLs open their own stream, not the most active one. const other = { ...snapshot(`other-${collected}`), streamId: "other", @@ -2181,13 +2250,15 @@ test("local live URLs sync the checkouts on every load", async ({ page }) => { lastSubmittedAt: 900, }; heads = [ - { ...snapshot(`collected-${collected}`), lastChangedAt: 200 }, + { ...snapshot(`collected-${collected}`), lastChangedAt: 50 }, other, ]; - return route.fulfill({ json: { contexts: heads, failures: [] } }); + return route.fulfill({ + json: { contexts: heads, failures: [], defaultContextId: other.id }, + }); }); - await page.goto("/"); + await page.goto("/streams/local-stream"); await ready(page); await expect(page.locator("#sync")).toHaveAttribute("aria-busy", "true"); await expect(page.locator(".session-status")).toHaveCount(0); diff --git a/docs-website/src/pages/usage.md b/docs-website/src/pages/usage.md index d6fe1618..cc1479d6 100644 --- a/docs-website/src/pages/usage.md +++ b/docs-website/src/pages/usage.md @@ -22,8 +22,9 @@ REST and MCP until Ctrl-C. The path defaults to the current directory; repeat 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. The browser opens the newest changed -snapshot; later edits and branch switches do not change it until you sync, which +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 checkout that fails to collect is reported without blocking the others and never substitutes an empty diff. Observations and comments persist across restarts in diff --git a/docs/development.md b/docs/development.md index 3c3cc89b..20d64bd8 100644 --- a/docs/development.md +++ b/docs/development.md @@ -274,9 +274,9 @@ controls remain available when the rail is hidden; the header shows the open review's identity. 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 -`POST /api/v2/collections`. Load-time sync runs in the background while the -first available stored head opens: the requested stream's head, or the most -recently active changed one for `/`. Collection completion never replaces an +`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 open review; the newer-head notice offers “View latest”. A workspace skeleton with “Collecting latest changes” appears only while no review is available. Skeletons retain shell geometry and display preferences, hide decorative placeholders from assistive diff --git a/docs/system.md b/docs/system.md index 506a0690..77390cbe 100644 --- a/docs/system.md +++ b/docs/system.md @@ -124,14 +124,16 @@ live URLs; the picker opens pinned ones; piped snapshots have no stream and stay pinned. A live URL resolves once, so later arrivals raise the notice instead of swapping the open review. With local collection enabled, loading `/` or a live URL syncs the checkouts in the background, joining startup on the first load -through `POST /api/v2/collections?initial=true`. The first available catalog -head opens immediately: the requested stream's head or, for `/`, the most -recently active changed one. The open snapshot stays selected when collection -finishes; a newer head raises the “Newer snapshot” notice with “View latest”. -Without an available 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 -activity ordering. +through `POST /api/v2/collections?initial=true`. A live URL opens the requested +stream's first available head immediately. `/` waits for collection, because +heads appear as each checkout finishes, then opens the collection's default: +current changed heads first, preferring the checkout containing the process's +path, so `diffx .` in a worktree opens that worktree, then the most recently +active. The open snapshot stays selected when collection finishes; a newer head +raises the “Newer snapshot” notice with “View latest”. Without an available +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. Repos and Diff share one navigation panel beside the activity rail, aligned with the diff below the header. Selecting a stream loads its diff and opens the Diff file tree; clicking the active rail icon collapses its panel. Diff appears diff --git a/internal/collector/discovery.go b/internal/collector/discovery.go index eaff3e75..a46906f1 100644 --- a/internal/collector/discovery.go +++ b/internal/collector/discovery.go @@ -216,6 +216,10 @@ func discoverWorkspaces(ctx context.Context, inputs []string, depth int, sourceI result.Diagnostics = append(result.Diagnostics, fmt.Sprintf("workspace unavailable %s: not a readable directory", input)) continue } + // Checkout roots are canonical; a canonical input locates its checkout. + if canonical, err := filepath.EvalSymlinks(input); err == nil { + input = canonical + } // An explicit path may sit anywhere inside a repository. source, err := diffsource.OpenRepository(ctx, input) if err == nil { diff --git a/internal/collector/discovery_test.go b/internal/collector/discovery_test.go index 097313d4..eda603a5 100644 --- a/internal/collector/discovery_test.go +++ b/internal/collector/discovery_test.go @@ -199,6 +199,26 @@ func TestCollectWorkspacesCollectsEachCheckoutIndependently(t *testing.T) { } } +func TestCollectWorkspacesCanonicalizesInputWithinItsCheckout(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) + alias := filepath.Join(t.TempDir(), "alias") + if err := os.Symlink(linked, alias); err != nil { + 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 { + t.Fatalf("requests %d err %v", len(requests), err) + } + // Canonical checkout roots must contain the input to identify the launch checkout. + for _, request := range requests { + if request.Metadata.TriggerRoot != linked { + t.Fatalf("symlinked input %q not canonical", request.Metadata.TriggerRoot) + } + } +} + func TestCollectWorkspacesReportsFailedCheckoutWithoutBlockingOthers(t *testing.T) { workspace := t.TempDir() healthy := initialized(t, filepath.Join(workspace, "healthy")) diff --git a/internal/serverapp/collection.go b/internal/serverapp/collection.go index ac11e65a..3226f923 100644 --- a/internal/serverapp/collection.go +++ b/internal/serverapp/collection.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "net/http" + "path/filepath" "slices" "strings" "sync" @@ -14,8 +15,8 @@ import ( "github.com/flexdinesh/diffx/internal/review" ) -// defaultContext is the review bare /mcp opens. It follows the newest changed -// context of the latest explicit collection so agents see what the user synced. +// defaultContext is the review bare /mcp opens. It follows the default of the +// latest explicit collection so agents see what the user synced. type defaultContext struct { mu sync.Mutex id string @@ -54,9 +55,15 @@ type Collect func(context.Context, func(ingestion.Request) error) ([]CollectionF type Published struct { Submissions []contextservice.Submission Failures []CollectionFailure + // launched is the context of the deepest checkout containing its request's + // input path. Reused observations keep their first run's metadata. + launched string + launchedDepth int } // Newest prefers current contexts with changes, then current clean contexts. +// Among equals, the checkout containing the collection's input path wins, so +// `diffx PATH` inside a worktree opens that worktree; then recent activity. // A successful collection of older observations still has an available default. func (published Published) Newest() (contextservice.Submission, bool) { var newest, fallback contextservice.Submission @@ -73,7 +80,7 @@ func (published Published) Newest() (contextservice.Submission, bool) { continue } hasChanges := submitted.Context.ChangedFileCount != nil && *submitted.Context.ChangedFileCount > 0 - if !found || hasChanges && !changed || hasChanges == changed && contextservice.CompareActivity(submitted.Context, newest.Context) < 0 { + if !found || hasChanges && !changed || hasChanges == changed && published.precedes(submitted.Context, newest.Context) { newest, found, changed = submitted, true, hasChanges } } @@ -83,6 +90,26 @@ func (published Published) Newest() (contextservice.Submission, bool) { return fallback, available } +func (published Published) precedes(left, right contextservice.Context) bool { + if launched := published.launched; launched != "" && (left.ID == launched) != (right.ID == launched) { + return left.ID == launched + } + return contextservice.CompareActivity(left, right) < 0 +} + +// launchDepth ranks a checkout whose root contains its trigger root, the input +// path of local collection. Nested worktrees prefer the deepest root. +func launchDepth(metadata ingestion.Metadata) int { + if metadata.Root == "" || metadata.TriggerRoot == "" { + return 0 + } + relative, err := filepath.Rel(metadata.Root, metadata.TriggerRoot) + if err != nil || !filepath.IsLocal(relative) { + return 0 + } + return len(metadata.Root) +} + // Publish selects each collected checkout into the composition's scope, then // ingests it serially. Each observation commits atomically on its own; a // rejected request is reported with its checkout and never blocks the others. @@ -108,6 +135,9 @@ func (published *Published) ingest(ctx context.Context, service *contextservice. return err } published.Submissions = append(published.Submissions, submitted) + if depth := launchDepth(input.Metadata); depth > published.launchedDepth { + published.launched, published.launchedDepth = submitted.Context.ID, depth + } return nil } @@ -233,10 +263,12 @@ func (handler *Collections) ServeHTTP(w http.ResponseWriter, r *http.Request) { applicationError(w, err) return } + newest, _ := published.Newest() result := struct { - Contexts []contextservice.Context `json:"contexts"` - Failures []CollectionFailure `json:"failures"` - }{make([]contextservice.Context, 0, len(published.Submissions)), make([]CollectionFailure, 0, len(published.Failures))} + Contexts []contextservice.Context `json:"contexts"` + Failures []CollectionFailure `json:"failures"` + DefaultContextID string `json:"defaultContextId"` + }{make([]contextservice.Context, 0, len(published.Submissions)), make([]CollectionFailure, 0, len(published.Failures)), newest.Context.ID} for _, submitted := range published.Submissions { result.Contexts = append(result.Contexts, submitted.Context) } diff --git a/internal/serverapp/collection_test.go b/internal/serverapp/collection_test.go index c3c88c2b..6908014f 100644 --- a/internal/serverapp/collection_test.go +++ b/internal/serverapp/collection_test.go @@ -40,6 +40,57 @@ func TestPublishedNewestUsesActivityThenStableStreamIdentity(t *testing.T) { } } +func TestPublishedNewestPrefersCheckoutContainingInputPath(t *testing.T) { + store, err := reviewstore.Open(":memory:") + if err != nil { + t.Fatal(err) + } + defer store.Close() + owner, err := store.User("local", "Local") + if err != nil { + t.Fatal(err) + } + service := contextservice.New(store, owner) + defer service.Close() + request := func(checkout, hash, root, input string, activity int64) ingestion.Request { + return ingestion.Request{ + ProtocolVersion: ingestion.ProtocolVersion, SubmissionID: checkout + input, ContentHash: strings.Repeat(hash, 64), + Metadata: ingestion.Metadata{SourceID: "source", RepositoryKey: "repository", CheckoutKey: checkout, Branch: checkout, Root: root, TriggerRoot: input, Trigger: "manual", CollectedAt: activity, ActivityAt: activity}, + Scopes: []ingestion.Scope{{Snapshot: review.RepositoryDiff{Source: "local", Root: root, Branch: checkout, Mode: review.DiffAll, Revision: hash, Files: []review.ChangedFile{}}, Patches: map[string]review.FilePatch{}}}, + } + } + collect := func(input string) (contextservice.Submission, Published) { + t.Helper() + published, err := Publish(t.Context(), service, Collection{Requests: []ingestion.Request{ + request("other", "a", "/work/other", input, 900), + request("prefix", "b", "/work/repo/.worktrees/feature-old", input, 800), + request("main", "c", "/work/repo", input, 700), + request("feature", "d", "/work/repo/.worktrees/feature", input, 100), + }}) + newest, ok := published.Newest() + if err != nil || !ok { + t.Fatalf("publish %s: %+v, %v", input, published, err) + } + return newest, published + } + // The first run's workspace input stays in reused observations. + if newest, _ := collect("/work"); newest.Context.Root == nil || *newest.Context.Root != "/work/other" { + t.Fatalf("workspace input did not follow activity: %+v", newest.Context) + } + newest, published := collect("/work/repo/.worktrees/feature/src") + if newest.Context.ID != published.Submissions[3].Context.ID { + t.Fatalf("launched checkout lost to activity or an enclosing checkout: %+v", newest.Context) + } + changed, clean := 1, 0 + published = Published{launched: "launched", launchedDepth: 1, Submissions: []contextservice.Submission{ + {Context: contextservice.Context{ID: "launched", LastChangedAt: 900, ChangedFileCount: &clean, Availability: "available"}}, + {Context: contextservice.Context{ID: "changed", LastChangedAt: 100, ChangedFileCount: &changed, Availability: "available"}}, + }} + if newest, ok := published.Newest(); !ok || newest.Context.ID != "changed" { + t.Fatalf("clean launched checkout displaced a changed one: %+v, %v", newest, ok) + } +} + func TestOlderCollectionRemainsSuccessfulWithoutRegressingStreamHead(t *testing.T) { store, err := reviewstore.Open(":memory:") if err != nil { diff --git a/packages/api/openapi.yaml b/packages/api/openapi.yaml index 22433e35..36801276 100644 --- a/packages/api/openapi.yaml +++ b/packages/api/openapi.yaml @@ -1273,7 +1273,7 @@ components: CollectionResult: type: object additionalProperties: false - required: [contexts, failures] + required: [contexts, failures, defaultContextId] properties: contexts: type: array @@ -1284,6 +1284,9 @@ components: description: Discovered checkouts that could not be collected or ingested; their earlier snapshots remain unchanged. items: $ref: "#/components/schemas/CollectionFailure" + defaultContextId: + type: string + description: The collected context that `/` and bare `/mcp` open. Changed current contexts come first; among them, the checkout containing the process's checkout path, then the most recently active. CollectionFailure: type: object additionalProperties: false diff --git a/packages/api/src/schema.d.ts b/packages/api/src/schema.d.ts index 6da78d98..12c98cf9 100644 --- a/packages/api/src/schema.d.ts +++ b/packages/api/src/schema.d.ts @@ -701,6 +701,8 @@ export interface components { contexts: components["schemas"]["Context"][]; /** @description Discovered checkouts that could not be collected or ingested; their earlier snapshots remain unchanged. */ failures: components["schemas"]["CollectionFailure"][]; + /** @description The collected context that `/` and bare `/mcp` open. Changed current contexts come first; among them, the checkout containing the process's checkout path, then the most recently active. */ + defaultContextId: string; }; CollectionFailure: { path: string;