Explorar el Código

feat(app): persist review state per session (#35488)

Luke Parker hace 1 mes
padre
commit
aa52d30d7f

+ 152 - 0
packages/app/e2e/regression/review-state-persistence.spec.ts

@@ -0,0 +1,152 @@
+import { base64Encode } from "@opencode-ai/core/util/encode"
+import { expect, test, type Page } from "@playwright/test"
+import { mockOpenCodeServer } from "../utils/mock-server"
+import { expectSessionTitle } from "../utils/waits"
+
+const directory = "C:/OpenCode/ReviewStatePersistence"
+const projectID = "proj_review_state_persistence"
+const sessionA = "ses_review_state_a"
+const sessionB = "ses_review_state_b"
+const titleA = "Alpha review state"
+const titleB = "Beta review state"
+const server = `http://${process.env.PLAYWRIGHT_SERVER_HOST ?? "127.0.0.1"}:${process.env.PLAYWRIGHT_SERVER_PORT ?? "4096"}`
+
+test.use({ viewport: { width: 1440, height: 900 } })
+
+test("restores review mode and selected file per session", async ({ page }) => {
+  await setup(page)
+  await page.goto(sessionHref(sessionA))
+  await expectSessionTitle(page, titleA)
+  await page.getByRole("button", { name: "Toggle review" }).click()
+
+  await selectMode(page, "Git changes", "Branch changes")
+  await selectFile(page, "beta.ts")
+
+  await switchSession(page, titleB)
+  await expect(page.getByRole("button", { name: "Git changes" })).toBeVisible()
+  await selectFile(page, "gamma.ts")
+
+  await switchSession(page, titleA)
+  await expect(page.getByRole("button", { name: "Branch changes" })).toBeVisible()
+  await expectSelectedFile(page, "beta.ts")
+  await selectMode(page, "Branch changes", "Git changes")
+  await expectSelectedFile(page, "alpha.ts")
+  await selectMode(page, "Git changes", "Branch changes")
+  await expectSelectedFile(page, "beta.ts")
+
+  await page.reload()
+  await expectSessionTitle(page, titleA)
+  await expect(page.getByRole("button", { name: "Branch changes" })).toBeVisible()
+  await expectSelectedFile(page, "beta.ts")
+
+  await switchSession(page, titleB)
+  await expect(page.getByRole("button", { name: "Git changes" })).toBeVisible()
+  await expectSelectedFile(page, "gamma.ts")
+})
+
+async function selectMode(page: Page, current: string, next: string) {
+  await page.getByRole("button", { name: current }).click()
+  await page.getByRole("option", { name: next }).click()
+}
+
+async function selectFile(page: Page, file: string) {
+  await page.getByRole("button", { name: file }).click()
+  await expectSelectedFile(page, file)
+}
+
+async function expectSelectedFile(page: Page, file: string) {
+  await expect(page.locator('[data-slot="session-review-v2-file-name"]')).toHaveText(file)
+}
+
+async function switchSession(page: Page, title: string) {
+  await page.locator("[data-titlebar-tab-slot]", { hasText: title }).click()
+  await expectSessionTitle(page, title)
+}
+
+async function setup(page: Page) {
+  await mockOpenCodeServer(page, {
+    directory,
+    project: {
+      id: projectID,
+      worktree: directory,
+      vcs: "git",
+      name: "review-state-persistence",
+      time: { created: 1700000000000, updated: 1700000000000 },
+      sandboxes: [],
+    },
+    provider: {
+      all: [
+        {
+          id: "opencode",
+          name: "OpenCode",
+          models: { test: { id: "test", name: "Test", limit: { context: 200_000 } } },
+        },
+      ],
+      connected: ["opencode"],
+      default: { providerID: "opencode", modelID: "test" },
+    },
+    sessions: [session(sessionA, titleA, 1700000000000), session(sessionB, titleB, 1700000001000)],
+    pageMessages: () => ({ items: [] }),
+  })
+  await page.route(/\/vcs(?:\?.*)?$/, (route) =>
+    route.fulfill({
+      status: 200,
+      contentType: "application/json",
+      body: JSON.stringify({ branch: "feature", default_branch: "dev" }),
+    }),
+  )
+  await page.route("**/vcs/diff**", (route) =>
+    route.fulfill({
+      status: 200,
+      contentType: "application/json",
+      body: JSON.stringify(
+        new URL(route.request().url()).searchParams.get("mode") === "branch"
+          ? [diff("src/alpha.ts"), diff("src/beta.ts")]
+          : [diff("src/alpha.ts"), diff("src/gamma.ts")],
+      ),
+    }),
+  )
+  await page.addInitScript(
+    ({ directory, server, sessions }) => {
+      localStorage.setItem("settings.v3", JSON.stringify({ general: { newLayoutDesigns: true } }))
+      localStorage.setItem(
+        "opencode.global.dat:server",
+        JSON.stringify({
+          projects: { local: [{ worktree: directory, expanded: true }] },
+          lastProject: { local: directory },
+        }),
+      )
+      localStorage.setItem(
+        "opencode.window.browser.dat:tabs",
+        JSON.stringify(sessions.map((sessionId: string) => ({ type: "session", server, sessionId }))),
+      )
+    },
+    { directory, server, sessions: [sessionA, sessionB] },
+  )
+}
+
+function session(id: string, title: string, created: number) {
+  return {
+    id,
+    slug: id,
+    projectID,
+    directory,
+    title,
+    version: "dev",
+    time: { created, updated: created },
+  }
+}
+
+function diff(file: string) {
+  return {
+    file,
+    additions: 1,
+    deletions: 1,
+    status: "modified",
+    patch: `diff --git a/${file} b/${file}\n--- a/${file}\n+++ b/${file}\n@@ -1 +1 @@\n-export const value = 'before'\n+export const value = 'after'\n`,
+  }
+}
+
+function sessionHref(sessionID: string) {
+  return `/server/${base64Encode(server)}/session/${sessionID}`
+}

+ 37 - 0
packages/app/src/context/layout.tsx

@@ -59,6 +59,8 @@ export function getProjectAvatarVariant(key?: string): ProjectAvatarVariant {
 type SessionView = {
   scroll: Record<string, SessionScroll>
   reviewOpen?: string[]
+  reviewMode?: ReviewChangeMode
+  reviewFile?: string
   pendingMessage?: string
   pendingMessageAt?: number
   todoCollapsed?: boolean
@@ -75,6 +77,7 @@ export type LocalProject = Partial<Project> & { worktree: string; expanded: bool
 export type HomeProjectSelection = { server: ServerConnection.Key; directory?: string }
 
 export type ReviewDiffStyle = "unified" | "split"
+export type ReviewChangeMode = "git" | "branch" | "turn"
 export type ReviewPanelSource = "context-button" | "other"
 
 export type LayoutRoute =
@@ -786,6 +789,14 @@ export const { use: useLayout, provider: LayoutProvider } = createSimpleContext(
       view(sessionKey: string | Accessor<string>) {
         const key = createSessionKeyReader(sessionKey, ensureKey)
         const s = createMemo(() => store.sessionView[key()] ?? { scroll: {} })
+        const reviewMode = createMemo(() => {
+          const mode = s().reviewMode
+          if (mode === "git" || mode === "branch" || mode === "turn") return mode
+        })
+        const reviewFile = createMemo(() => {
+          const file = s().reviewFile
+          if (typeof file === "string") return file
+        })
         const terminalOpened = createMemo(() => store.terminal?.opened ?? false)
         const reviewPanelOpened = createMemo(() => store.review?.panelOpened ?? DEFAULT_REVIEW_PANEL_OPENED)
         const reviewPanelSource = createMemo(() => (reviewPanelOpened() ? ephemeral.reviewPanelSource : "other"))
@@ -869,6 +880,32 @@ export const { use: useLayout, provider: LayoutProvider } = createSimpleContext(
             },
           },
           review: {
+            mode: reviewMode,
+            setMode(mode: ReviewChangeMode) {
+              const session = key()
+              const current = store.sessionView[session]
+              if (!current) {
+                setStore("sessionView", session, { scroll: {}, reviewMode: mode })
+                prune(session)
+                return
+              }
+              if (current.reviewMode === mode) return
+              setStore("sessionView", session, "reviewMode", mode)
+              prune(session)
+            },
+            file: reviewFile,
+            setFile(file: string) {
+              const session = key()
+              const current = store.sessionView[session]
+              if (!current) {
+                setStore("sessionView", session, { scroll: {}, reviewFile: file })
+                prune(session)
+                return
+              }
+              if (current.reviewFile === file) return
+              setStore("sessionView", session, "reviewFile", file)
+              prune(session)
+            },
             open: createMemo(() => s().reviewOpen ?? []),
             setOpen(open: string[]) {
               const session = key()

+ 33 - 23
packages/app/src/pages/session.tsx

@@ -101,7 +101,6 @@ type VcsMode = "git" | "branch"
 const sessionViewState = () => ({
   messageId: undefined as string | undefined,
   mobileTab: "session" as "session" | "changes",
-  changes: "git" as ChangeMode,
 })
 
 function isCurrentSessionNotFoundError(error: unknown, sessionID: string | undefined) {
@@ -348,6 +347,8 @@ export default function Page() {
   const location = useLocation()
   const navigate = useNavigate()
   const { params, sessionKey, workspaceKey, tabs, view } = useSessionLayout()
+  const reviewMode = () => view().review.mode() ?? "git"
+  const reviewFile = () => view().review.file()
   const sessionOwnership = createSessionOwnership(sessionKey)
   const newSessionDesign = createMemo(() => settings.general.newLayoutDesigns())
 
@@ -607,7 +608,8 @@ export default function Page() {
       : store.mobileTab === "changes",
   )
   const vcsMode = createMemo<VcsMode | undefined>(() => {
-    if (store.changes === "git" || store.changes === "branch") return store.changes
+    const mode = reviewMode()
+    if (mode === "git" || mode === "branch") return mode
   })
   const vcsKey = createMemo(
     () =>
@@ -634,15 +636,21 @@ export default function Page() {
   })
   const refreshVcs = debounce(() => void queryClient.invalidateQueries({ queryKey: vcsKey() }), 100)
   const reviewDiffs = () => {
-    if (store.changes === "git" || store.changes === "branch")
+    if (reviewMode() === "git" || reviewMode() === "branch")
       // avoids suspense
       return vcsQuery.isFetched ? (vcsQuery.data ?? []) : []
     return turnDiffs()
   }
+  const activeReviewFile = () => {
+    const diffs = reviewDiffs()
+    const selected = reviewFile()
+    if (selected && diffs.some((diff) => diff.file === selected)) return selected
+    return diffs[0]?.file
+  }
   const reviewCount = () => reviewDiffs().length
   const hasReview = () => reviewCount() > 0
   const reviewReady = () => {
-    if (store.changes === "git" || store.changes === "branch") return !vcsQuery.isPending
+    if (reviewMode() === "git" || reviewMode() === "branch") return !vcsQuery.isPending
     return true
   }
   const loadReviewDiff = async (file: string, version?: number): Promise<VcsFileDiff | undefined> => {
@@ -1002,12 +1010,15 @@ export default function Page() {
   }
 
   createEffect(() => {
+    if (!layout.ready()) return
+    if (sync().status !== "complete") return
     if (!sync().project) return
     const list = changesOptions()
-    if (list.includes(store.changes)) return
+    const mode = reviewMode()
+    if (list.includes(mode)) return
     const next = list[0]
     if (!next) return
-    setStore("changes", next)
+    view().review.setMode(next)
   })
 
   createEffect(
@@ -1027,7 +1038,6 @@ export default function Page() {
   const [tree, setTree] = createStore({
     reviewScroll: undefined as HTMLDivElement | undefined,
     pendingDiff: undefined as string | undefined,
-    activeDiff: undefined as string | undefined,
   })
 
   createEffect(
@@ -1037,7 +1047,6 @@ export default function Page() {
         setTree({
           reviewScroll: undefined,
           pendingDiff: undefined,
-          activeDiff: undefined,
         })
       },
       { defer: true },
@@ -1085,9 +1094,9 @@ export default function Page() {
     return (
       <Select
         options={changesOptions()}
-        current={store.changes}
+        current={reviewMode()}
         label={changesLabel}
-        onSelect={(option) => option && setStore("changes", option)}
+        onSelect={(option) => option && view().review.setMode(option)}
         variant="ghost"
         size="small"
         valueClass="text-14-medium"
@@ -1104,11 +1113,11 @@ export default function Page() {
       <SelectV2
         appearance="inline"
         options={changesOptions()}
-        current={store.changes}
+        current={reviewMode()}
         label={changesLabel}
         placement="bottom-start"
         gutter={6}
-        onSelect={(option) => option && setStore("changes", option)}
+        onSelect={(option) => option && view().review.setMode(option)}
       />
     )
   }
@@ -1136,18 +1145,18 @@ export default function Page() {
   )
 
   const reviewEmptyText = createMemo(() => {
-    if (store.changes === "git") return language.t("session.review.noUncommittedChanges")
-    if (store.changes === "branch") return language.t("session.review.noBranchChanges")
+    if (reviewMode() === "git") return language.t("session.review.noUncommittedChanges")
+    if (reviewMode() === "branch") return language.t("session.review.noBranchChanges")
     return language.t("session.review.noChanges")
   })
 
   const reviewEmpty = (input: { loadingClass: string; emptyClass: string }) => {
-    if (store.changes === "git" || store.changes === "branch") {
+    if (reviewMode() === "git" || reviewMode() === "branch") {
       if (!reviewReady()) return <div class={input.loadingClass}>{language.t("session.review.loadingChanges")}</div>
       return empty(reviewEmptyText())
     }
 
-    if (store.changes === "turn") {
+    if (reviewMode() === "turn") {
       if (nogit()) return createGit(input)
       return empty(reviewEmptyText())
     }
@@ -1160,10 +1169,10 @@ export default function Page() {
   }
 
   const reviewEmptyV2 = () => {
-    if ((store.changes === "git" || store.changes === "branch") && !reviewReady()) {
+    if ((reviewMode() === "git" || reviewMode() === "branch") && !reviewReady()) {
       return <div class="px-6 py-4 text-text-weak">{language.t("session.review.loadingChanges")}</div>
     }
-    if (store.changes === "turn" && nogit()) {
+    if (reviewMode() === "turn" && nogit()) {
       return <SessionReviewEmptyNoGitV2 pending={gitMutation.isPending} onInitGit={initGit} />
     }
     return <SessionReviewEmptyChangesV2 />
@@ -1185,7 +1194,7 @@ export default function Page() {
         diffStyle={input.diffStyle}
         onDiffStyleChange={input.onDiffStyleChange}
         onScrollRef={(el) => setTree("reviewScroll", el)}
-        focusedFile={tree.activeDiff}
+        focusedFile={activeReviewFile()}
         onLineComment={(comment) => addCommentToContext({ ...comment, origin: "review" })}
         onLineCommentUpdate={updateCommentInContext}
         onLineCommentDelete={removeCommentFromContext}
@@ -1221,7 +1230,7 @@ export default function Page() {
     },
     loadDiff: loadReviewDiff,
     get activeFile() {
-      return tree.activeDiff
+      return activeReviewFile()
     },
     onSelectFile: focusReviewDiff,
     get diffStyle() {
@@ -1334,7 +1343,8 @@ export default function Page() {
   const focusReviewDiff = (path: string) => {
     openReviewPanel()
     view().review.openPath(path)
-    setTree({ activeDiff: path, pendingDiff: path })
+    view().review.setFile(path)
+    setTree("pendingDiff", path)
   }
 
   createEffect(() => {
@@ -2198,7 +2208,7 @@ export default function Page() {
             reviewHasFocusableContent={hasReview}
             reviewCount={reviewCount}
             reviewPanel={reviewPanel}
-            activeDiff={tree.activeDiff}
+            activeDiff={activeReviewFile()}
             focusReviewDiff={focusReviewDiff}
             reviewSnap={ui.reviewSnap}
             size={size}
@@ -2224,7 +2234,7 @@ export default function Page() {
                     reviewCount={reviewCount}
                     reviewPanel={reviewPanelV2}
                     fileBrowserState={reviewV2State}
-                    activeDiff={tree.activeDiff}
+                    activeDiff={activeReviewFile()}
                     focusReviewDiff={focusReviewDiff}
                     reviewSnap={ui.reviewSnap}
                     size={size}