feat(web): redesign review panels and supporting pages

apps/server/src/pullRequest/PullRequestService.ts

@@ -114,6 +114,15 @@ function supersedesReading(current: PullRequestReading | undefined, next: PullRe
   return (next.observedAt ?? -Infinity) >= (current.observedAt ?? -Infinity);
 }
 
+// Retry a shared flight canceled by its previous reader after that flight leaves the cache.
+const getShared = <Key, A, E, R>(cache: Cache.Cache<Key, A, E, R>, key: Key) =>
+  Cache.get(cache, key).pipe(
+    Effect.catchCauseIf(
+      (cause) => Cause.hasInterruptsOnly(cause),
+      (_cause) => Cache.get(cache, key),
+    ),
+  );
+
 // Providers request one extra row; 99 stays within GitHub/GitLab's 100-row API limit.
 const DEFAULT_REPOSITORY_LIST_LIMIT = 99;
 const REPOSITORY_CONCURRENCY = 12;
@@ -1169,10 +1178,10 @@ export const make = Effect.gen(function* () {
           const roots =
             viewerRoots.get(host) ?? forHost.map(({ project }) => project.workspaceRoot);
           const key = JSON.stringify([host, api.kind, [...new Set(roots)].sort()]);
-          if (options?.allowPaused === true) return Cache.get(viewerFlights, key);
+          if (options?.allowPaused === true) return getShared(viewerFlights, key);
           // Check pauses outside the shared lookup so background reads cannot spawn repeated failing CLIs and extend the pause.
           return rateLimits.check({ provider: api.kind, host }).pipe(
-            Effect.flatMap(() => Cache.get(viewerFlights, key)),
+            Effect.flatMap(() => getShared(viewerFlights, key)),
             Effect.catch((error) =>
               Effect.succeed<ResolvedViewer>({
                 host,
@@ -1868,6 +1877,9 @@ export const make = Effect.gen(function* () {
               reviewThreads: activity.reviewThreads,
               commits: activity.commits,
               ...(activity.reactions === undefined ? {} : { reactions: activity.reactions }),
+              ...(activity.reviewRequests === undefined
+                ? {}
+                : { reviewRequests: activity.reviewRequests }),
             })),
           ),
       ),
@@ -3020,13 +3032,7 @@ export const make = Effect.gen(function* () {
         ? null
         : Object.entries(input.cursors).toSorted(([left], [right]) => left.localeCompare(right)),
     ]);
-    // A replacement reader can join a lookup still finishing its previous reader's cancellation.
-    return Cache.get(listCache, key).pipe(
-      Effect.catchCauseIf(
-        (cause) => Cause.hasInterruptsOnly(cause),
-        (_cause) => Cache.get(listCache, key),
-      ),
-    );
+    return getShared(listCache, key);
   };
 
   const checksCache = yield* Cache.makeWith(
@@ -3232,7 +3238,7 @@ export const make = Effect.gen(function* () {
   const detail: PullRequestService["Service"]["detail"] = (input) => {
     const key = refCacheKey(input);
     // Skip reads older than strict summaries; caching them would preserve regressions without asking the host again.
-    const read = Cache.get(detailCache, key).pipe(
+    const read = getShared(detailCache, key).pipe(
       Effect.tap((value) => {
         if (value.state === "merged") noteMerged(input);
         const summary = summaryFromDetail(value, lastGoodSummary.peek(key));
@@ -3302,15 +3308,15 @@ export const make = Effect.gen(function* () {
     return Cache.getSuccess(detailCache, key).pipe(
       Effect.flatMap(
         Option.match({
-          onNone: () => Cache.get(previewCache, key),
+          onNone: () => getShared(previewCache, key),
           onSome: (detail) => Effect.succeed(previewFields(detail)),
         }),
       ),
     );
   };
   const activity: PullRequestService["Service"]["activity"] = (input) => {
     const key = refCacheKey(input);
-    return Cache.get(activityCache, key);
+    return getShared(activityCache, key);
   };
 
   const diffCache = yield* Cache.makeWith(
@@ -3381,7 +3387,7 @@ export const make = Effect.gen(function* () {
       ),
       Effect.uninterruptible,
     );
-    const read = Cache.get(diffCache, key).pipe(
+    const read = getShared(diffCache, key).pipe(
       Effect.ensuring(evictDiffs),
       Effect.ensuring(Effect.sync(armDiffSweep)),
     );
@@ -3406,7 +3412,7 @@ export const make = Effect.gen(function* () {
   const filesViewed: PullRequestService["Service"]["filesViewed"] = (input) =>
     canonicalRef(input).pipe(
       Effect.flatMap((ref) =>
-        Cache.get(filesViewedCache, JSON.stringify([refCacheKey(ref), filesViewedEpoch(ref)])),
+        getShared(filesViewedCache, JSON.stringify([refCacheKey(ref), filesViewedEpoch(ref)])),
       ),
     );
 
@@ -3453,12 +3459,7 @@ export const make = Effect.gen(function* () {
     }
     if (missing.size === 0) return { stats: held };
     const key = statsBatchKey(missing.values());
-    const { result, at } = yield* Cache.get(listStatsCache, key).pipe(
-      Effect.catchCauseIf(
-        (cause) => Cause.hasInterruptsOnly(cause),
-        (_cause) => Cache.get(listStatsCache, key),
-      ),
-    );
+    const { result, at } = yield* getShared(listStatsCache, key);
     for (const [key, ref] of missing) {
       const stat = result.stats.find(
         (stat) =>
@@ -3727,9 +3728,9 @@ export const make = Effect.gen(function* () {
     ),
     refreshAfterTurn,
     detail: credentialCached(detail),
-    checks: credentialCached((input) => Cache.get(checksCache, refCacheKey(input))),
+    checks: credentialCached((input) => getShared(checksCache, refCacheKey(input))),
     watchFingerprint: credentialCached((input) =>
-      Cache.get(watchFingerprintCache, refCacheKey(input)),
+      getShared(watchFingerprintCache, refCacheKey(input)),
     ),
     activity: credentialCached(activity),
     preview: credentialCached(preview),

apps/server/src/vcs/GitVcsDriverCore.test.ts

@@ -3313,6 +3313,7 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
           path: "HEAD",
           insertions: 1,
           deletions: 0,
+          status: "modified",
         });
       }),
     );
@@ -3333,9 +3334,9 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
         const status = yield* driver.statusDetails(cwd);
 
         assert.deepEqual(status.workingTree.files, [
-          { path: "café notes.md", insertions: 1, deletions: 0 },
-          { path: "src/new.ts", insertions: 1, deletions: 0 },
-          { path: "src/old.ts", insertions: 0, deletions: 0 },
+          { path: "café notes.md", insertions: 1, deletions: 0, status: "modified" },
+          { path: "src/new.ts", insertions: 1, deletions: 0, status: "renamed" },
+          { path: "src/old.ts", insertions: 0, deletions: 0, status: "deleted" },
         ]);
         yield* driver.commit(cwd, "Commit selection", "", {
           stage: { filePaths: status.workingTree.files.map((file) => file.path) },
@@ -3363,9 +3364,9 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
         const status = yield* driver.statusDetails(nestedCwd);
 
         assert.deepEqual(status.workingTree.files, [
-          { path: "nested/new.txt", insertions: 0, deletions: 0 },
-          { path: "nested/tracked.txt", insertions: 1, deletions: 0 },
-          { path: "README.md", insertions: 1, deletions: 0 },
+          { path: "nested/new.txt", insertions: 0, deletions: 0, status: "untracked" },
+          { path: "nested/tracked.txt", insertions: 1, deletions: 0, status: "modified" },
+          { path: "README.md", insertions: 1, deletions: 0, status: "modified" },
         ]);
         // The root is what the paths above are relative to, not the nested cwd, and it keeps
         // the trailing space so the resolved editor target is the file that exists.
@@ -3398,8 +3399,8 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
         const status = yield* driver.statusDetails(nestedCwd);
 
         assert.deepEqual(status.workingTree.files, [
-          { path: "nested/new.txt", insertions: 0, deletions: 0 },
-          { path: "nested/tracked.txt", insertions: 1, deletions: 0 },
+          { path: "nested/new.txt", insertions: 0, deletions: 0, status: "untracked" },
+          { path: "nested/tracked.txt", insertions: 1, deletions: 0, status: "modified" },
         ]);
         assert.equal(
           NodeFS.realpathSync.native(status.workingTree.rootPath!),
@@ -3408,6 +3409,54 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
       }),
     );
 
+    it.effect("reports how each working-tree file changed", () =>
+      Effect.gen(function* () {
+        const cwd = yield* makeTmpDir();
+        yield* initRepoWithCommit(cwd);
+        yield* writeTextFile(cwd, "keep.ts", "keep\n");
+        yield* writeTextFile(cwd, "remove.ts", "remove\n");
+        yield* writeTextFile(cwd, "old name.ts", "rename me\n");
+        yield* git(cwd, ["add", "."]);
+        yield* git(cwd, ["commit", "-m", "add files"]);
+        yield* writeTextFile(cwd, "keep.ts", "keep\nedited\n");
+        yield* git(cwd, ["rm", "-q", "remove.ts"]);
+        yield* git(cwd, ["mv", "old name.ts", "new name.ts"]);
+        yield* writeTextFile(cwd, "staged.ts", "staged\n");
+        yield* git(cwd, ["add", "staged.ts"]);
+        yield* writeTextFile(cwd, "untracked.ts", "untracked\n");
+        const blob = yield* git(cwd, ["rev-parse", "HEAD:keep.ts"]);
+        const conflicts = [
+          ["AU", [2]],
+          ["UD", [1, 2]],
+          ["UA", [3]],
+          ["DU", [1, 3]],
+          ["AA", [2, 3]],
+          ["UU", [1, 2, 3]],
+        ] as const;
+        for (const [xy, stages] of conflicts) {
+          yield* writeTextFile(cwd, `${xy}.ts`, "conflicted\n");
+          yield* (yield* GitVcsDriver.GitVcsDriver).execute({
+            operation: "test.unmergedIndex",
+            cwd,
+            args: ["update-index", "--index-info"],
+            stdin: stages.map((stage) => `100644 ${blob} ${stage}\t${xy}.ts\n`).join(""),
+          });
+        }
+
+        const status = yield* (yield* GitVcsDriver.GitVcsDriver).statusDetails(cwd);
+        const statuses = Object.fromEntries(
+          status.workingTree.files.map((file) => [file.path, file.status]),
+        );
+
+        assert.equal(statuses["keep.ts"], "modified");
+        assert.equal(statuses["remove.ts"], "deleted");
+        assert.equal(statuses["new name.ts"], "renamed");
+        assert.equal(statuses["staged.ts"], "added");
+        assert.equal(statuses["untracked.ts"], "untracked");
+        for (const [xy] of conflicts) assert.equal(statuses[`${xy}.ts`], "modified", xy);
+      }),
+    );
+
     it.effect("reports default-branch delta separately from upstream delta", () =>
       Effect.gen(function* () {
         const cwd = yield* makeTmpDir();

apps/server/src/vcs/GitVcsDriverCore.ts

@@ -32,6 +32,7 @@ import {
   type ReviewDiffFileStat,
   type ReviewDiffPreviewSource,
   type VcsRef,
+  type VcsWorkingTreeFileStatus,
 } from "@t3tools/contracts";
 import { dedupeRemoteBranchesWithLocalMatches } from "@t3tools/shared/git";
 import * as HostProcess from "@t3tools/shared/HostProcess";
@@ -221,6 +222,13 @@ function parseReviewNumstat(stdout: string): ReviewDiffFileStat[] {
   return files;
 }
 
+function withFileStatus<T extends { path: string }>(
+  file: T,
+  status: VcsWorkingTreeFileStatus | null | undefined,
+): T & { status?: VcsWorkingTreeFileStatus } {
+  return status ? { ...file, status } : file;
+}
+
 // Space-separated fields precede the path in each porcelain v2 record type. The path itself
 // may contain spaces, so it is everything after them.
 const PORCELAIN_V2_FIELDS_BEFORE_PATH: Record<string, number> = {
@@ -238,6 +246,30 @@ function parsePorcelainPath(record: string): string | null {
   return filePath.length > 0 ? filePath : null;
 }
 
+function parsePorcelainEntry(
+  record: string,
+): { path: string; status: VcsWorkingTreeFileStatus | null } | null {
+  const path = parsePorcelainPath(record);
+  if (path === null) return null;
+  const recordType = record[0];
+  const xy = record.split(" ")[1] ?? "";
+  const status: VcsWorkingTreeFileStatus | null =
+    recordType === "?"
+      ? "untracked"
+      : recordType === "!"
+        ? null
+        : recordType === "2"
+          ? "renamed"
+          : recordType === "u"
+            ? "modified"
+            : xy[1] === "D" || xy === "D."
+              ? "deleted"
+              : xy[0] === "A" || xy[1] === "A"
+                ? "added"
+                : "modified";
+  return { path, status };
+}
+
 function filterBranchesForListQuery(
   refs: ReadonlyArray<VcsRef>,
   query?: string,
@@ -2236,7 +2268,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
     let behindCount = 0;
     let aheadOfDefaultCount = 0;
     let hasWorkingTreeChanges = false;
-    const changedFilesWithoutNumstat = new Set<string>();
+    const changedFileStatuses = new Map<string, VcsWorkingTreeFileStatus | null>();
     const untrackedRoots: string[] = [];
 
     const statusRecords = statusStdout.split("\0");
@@ -2261,16 +2293,19 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
       }
       if (line.length > 0 && !line.startsWith("#")) {
         hasWorkingTreeChanges = true;
-        const pathValue = parsePorcelainPath(line);
-        if (pathValue) {
-          changedFilesWithoutNumstat.add(pathValue);
-          if (line.startsWith("? ")) untrackedRoots.push(pathValue);
+        const entry = parsePorcelainEntry(line);
+        if (entry) {
+          changedFileStatuses.set(entry.path, entry.status);
+          if (line.startsWith("? ")) untrackedRoots.push(entry.path);
         }
-        // A rename record is followed by its original path as a separate record. Listing it
-        // keeps the deletion selectable when committing only some files.
+        // Rename records carry a second NUL-delimited original path, selectable for partial commits.
         if (line.startsWith("2 ")) {
           const originalPath = statusRecords[++index];
-          if (originalPath) changedFilesWithoutNumstat.add(originalPath);
+          if (originalPath && !changedFileStatuses.has(originalPath))
+            changedFileStatuses.set(
+              originalPath,
+              line.split(" ")[1]?.includes("R") ? "deleted" : null,
+            );
         }
       }
     }
@@ -2319,12 +2354,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
     const files = Array.from(fileStatMap.entries()).map(([filePath, stat]) => {
       insertions += stat.insertions;
       deletions += stat.deletions;
-      return { path: filePath, insertions: stat.insertions, deletions: stat.deletions };
+      return withFileStatus(
+        { path: filePath, insertions: stat.insertions, deletions: stat.deletions },
+        changedFileStatuses.get(filePath),
+      );
     });
 
-    for (const filePath of changedFilesWithoutNumstat) {
+    for (const [filePath, status] of changedFileStatuses) {
       if (fileStatMap.has(filePath)) continue;
-      files.push({ path: filePath, insertions: 0, deletions: 0 });
+      files.push(withFileStatus({ path: filePath, insertions: 0, deletions: 0 }, status));
     }
     files.sort((a, b) => a.path.localeCompare(b.path));
 

apps/web/src/browserFaviconStore.ts

@@ -1,17 +1,15 @@
 import {
-  parseScopedProjectKey,
   scopedProjectKey,
   scopedThreadKey,
   scopeProjectRef,
 } from "@t3tools/client-runtime/environment";
 import type { DesktopPreviewFavicon, ScopedProjectRef, ScopedThreadRef } from "@t3tools/contracts";
-import * as Option from "effect/Option";
 import { create } from "zustand";
 import { createJSONStorage, persist } from "zustand/middleware";
 
 import { normalizeHostname } from "~/browser/browserTargetResolver";
-import { readThreadShell, useQuietThreadShell } from "~/state/entities";
-import { readPreparedConnection, usePreparedConnection } from "~/state/session";
+import { readThreadShell } from "~/state/entities";
+import { readPreparedConnection } from "~/state/session";
 
 import {
   BROWSER_FAVICON_MAX_ALIASES_PER_ENTRY,
@@ -171,17 +169,6 @@ function mergeBrowserFaviconState(
   };
 }
 
-/** Server threads name their project in the shell; drafts in the composer draft store. */
-function useFaviconProjectRefForThread(threadRef: ScopedThreadRef): ScopedProjectRef | null {
-  const shell = useQuietThreadShell(threadRef);
-  const draftProjectRef = useComposerDraftStore((state) => {
-    const draft = state.getDraftSessionByRef(threadRef);
-    return draft ? scopedProjectKey(scopeProjectRef(draft.environmentId, draft.projectId)) : null;
-  });
-  if (shell?.projectId) return scopeProjectRef(threadRef.environmentId, shell.projectId);
-  return draftProjectRef === null ? null : parseScopedProjectKey(draftProjectRef);
-}
-
 function readThreadProjectRef(threadRef: ScopedThreadRef): ScopedProjectRef | null {
   const projectId = readThreadShell(threadRef)?.projectId;
   if (projectId) return scopeProjectRef(threadRef.environmentId, projectId);
@@ -284,18 +271,6 @@ function flushPendingFaviconsForThread(
   return Object.keys(remaining).length === 0;
 }
 
-export function useFaviconForThreadUrl(threadRef: ScopedThreadRef, url: string): string | null {
-  const projectRef = useFaviconProjectRefForThread(threadRef);
-  const preparedConnection = usePreparedConnection(threadRef.environmentId);
-  const environmentHostname = Option.isSome(preparedConnection)
-    ? new URL(preparedConnection.value.httpBaseUrl).hostname
-    : null;
-  const threadKey = scopedThreadKey(threadRef);
-  return useBrowserFaviconStore((state) =>
-    lookupFaviconForThread(state, threadKey, projectRef, url, environmentHostname),
-  );
-}
-
 /**
  * A capture waiting for its thread's project still shows on that thread, and wins
  * over an older stored capture for the same origin.