diff --git a/src/renderer/state/gitReviewActionStore.test.ts b/src/renderer/state/gitReviewActionStore.test.ts new file mode 100644 index 00000000..d47dc344 --- /dev/null +++ b/src/renderer/state/gitReviewActionStore.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { useGitReviewActionStore } from "./gitReviewActionStore"; + +function resetStore() { + useGitReviewActionStore.setState({ panels: {} }); +} + +const { patch } = useGitReviewActionStore.getState(); +const panels = () => useGitReviewActionStore.getState().panels; + +describe("gitReviewActionStore", () => { + beforeEach(resetStore); + afterEach(resetStore); + + it("keeps panel state keyed and isolated across keys", () => { + patch("projA", { commitMessage: "feat: a", prTargetBranch: "main" }); + patch("projB", { commitMessage: "fix: b" }); + + expect(panels()["projA"]?.commitMessage).toBe("feat: a"); + expect(panels()["projA"]?.prTargetBranch).toBe("main"); + expect(panels()["projB"]?.commitMessage).toBe("fix: b"); + // Writing one panel must not bleed into another. + patch("projA", { commitMessage: "feat: a2" }); + expect(panels()["projB"]?.commitMessage).toBe("fix: b"); + }); + + // The bug this store fixes: the git panel unmounts on project switch, so an + // async action (which keeps running in the supervisor) resolved into a dead + // component and its result/pending state was dropped. Routed through the + // store, both are captured against the panel's key and are there on return. + it("captures a generation result that resolves after the panel 'unmounts'", () => { + // Panel A kicks off generation, then the user switches away (no reads). + patch("projA", { isGenerating: true }); + expect(panels()["projA"]?.isGenerating).toBe(true); + + // While away, the user works in panel B — independent state. + patch("projB", { commitMessage: "wip" }); + + // Generation completes after the unmount and writes through the captured, + // key-bound setter (this is what used to hit a dead component). + patch("projA", { commitMessage: "feat: captured", isGenerating: false }); + + // Back on panel A: the spinner is cleared and the message is waiting. + expect(panels()["projA"]).toMatchObject({ + commitMessage: "feat: captured", + isGenerating: false, + }); + // Panel B was never disturbed. + expect(panels()["projB"]?.commitMessage).toBe("wip"); + }); + + it("preserves an in-flight commit/push spinner across a switch", () => { + // Commit & Push uses both flags; both must survive the remount. + patch("p", { isCommitting: true, isSyncing: true }); + // ...user switches away and back (re-read) — still pending. + expect(panels()["p"]).toMatchObject({ isCommitting: true, isSyncing: true }); + // Operation finishes and clears them. + patch("p", { isCommitting: false, isSyncing: false }); + expect(panels()["p"]).toMatchObject({ isCommitting: false, isSyncing: false }); + }); + + it("tracks each in-flight action flag independently", () => { + patch("p", { isMerging: true, isPullingFromSource: true, isCreatingPr: true }); + patch("p", { isMerging: false }); + expect(panels()["p"]).toMatchObject({ + isMerging: false, + isPullingFromSource: true, + isCreatingPr: true, + }); + }); + + it("skips no-op writes so subscribers don't re-render needlessly", () => { + patch("p", { commitMessage: "x" }); + const before = panels(); + patch("p", { commitMessage: "x" }); // same value + expect(panels()).toBe(before); // identical reference — no update + }); + + it("leaves untouched keys referentially stable when another key changes", () => { + patch("a", { commitMessage: "a" }); + patch("b", { commitMessage: "b" }); + const panelB = panels()["b"]; + patch("a", { commitMessage: "a2" }); + // 'b' object identity is preserved, so a selector on key 'b' won't re-render. + expect(panels()["b"]).toBe(panelB); + }); +}); diff --git a/src/renderer/state/gitReviewActionStore.ts b/src/renderer/state/gitReviewActionStore.ts new file mode 100644 index 00000000..41d4c3eb --- /dev/null +++ b/src/renderer/state/gitReviewActionStore.ts @@ -0,0 +1,96 @@ +import { create } from "zustand"; + +/** + * Per-(project, worktree) working state for the git review panel: in-progress + * drafts (commit message, PR title/body/target) and the in-flight flags for + * every async action the panel can run (generate, commit, sync, merge, pull, + * create PR). + * + * Lives in a module-level store — NOT component `useState` — so it survives + * the panel unmounting and remounting when the user switches projects. The + * panel is keyed by `${projectId}:${worktreePath}` (see AppOverlays / + * GitReviewPanelContent), so switching away and back fully remounts the + * subtree; local state there would reset to empty. Because every action + * (commit, push, merge, and especially the long-running generate/PR-summary + * supervisor calls) keeps running across that remount, its pending flag and + * any result must live somewhere that outlives the component. Routing all of + * it through this store — keyed by the same `storeKey` the panel uses + * (`statusKey ?? project.id`) — means spinners reappear on return and async + * results land against the right panel even if it unmounted mid-flight, + * instead of resolving into a dead component instance. + * + * Intentionally in-memory only (no localStorage): an in-flight flag must not + * survive an app restart, or an action killed with the process would leave its + * spinner stuck on forever. + */ +export interface GitReviewActionState { + /** Draft commit message — typed by the user or filled in by generation. */ + commitMessage: string; + /** Draft PR title. */ + prTitle: string; + /** Draft PR body. */ + prBody: string; + /** Draft PR target branch (null = use the resolved source branch). */ + prTargetBranch: string | null; + /** A commit-message generation is in flight (supervisor one-shot LLM call). */ + isGenerating: boolean; + /** A PR-summary generation is in flight. */ + isGeneratingPr: boolean; + /** A commit (and optional push) is in flight. */ + isCommitting: boolean; + /** A push/sync/pull is in flight. */ + isSyncing: boolean; + /** A worktree merge is in flight. */ + isMerging: boolean; + /** A pull-from-source is in flight. */ + isPullingFromSource: boolean; + /** A merge abort is in flight. */ + isAbortingMerge: boolean; + /** A merge finish (commit) is in flight. */ + isFinishingMerge: boolean; + /** A PR creation is in flight. */ + isCreatingPr: boolean; +} + +/** Stable default returned for panels with no state yet — never mutate. */ +const EMPTY_STATE: GitReviewActionState = Object.freeze({ + commitMessage: "", + prTitle: "", + prBody: "", + prTargetBranch: null, + isGenerating: false, + isGeneratingPr: false, + isCommitting: false, + isSyncing: false, + isMerging: false, + isPullingFromSource: false, + isAbortingMerge: false, + isFinishingMerge: false, + isCreatingPr: false, +}); + +interface GitReviewActionStore { + panels: Record; + /** Merge `patch` into the state for `key`; no-op writes are skipped. */ + patch: (key: string, patch: Partial) => void; +} + +export const useGitReviewActionStore = create((set, get) => ({ + panels: {}, + patch: (key, patch) => { + const current = get().panels[key] ?? EMPTY_STATE; + const keys = Object.keys(patch) as (keyof GitReviewActionState)[]; + if (keys.every((k) => current[k] === patch[k])) return; + set((state) => ({ + panels: { ...state.panels, [key]: { ...current, ...patch } }, + })); + }, +})); + +/** + * Reactive read of a single panel's state. Returns a stable empty default when + * the panel has no state yet, so an absent key never triggers re-render churn. + */ +export function useGitReviewActionState(key: string): GitReviewActionState { + return useGitReviewActionStore((s) => s.panels[key] ?? EMPTY_STATE); +} diff --git a/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/parts/useGitReviewActions.ts b/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/parts/useGitReviewActions.ts index ced50414..788b7234 100644 --- a/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/parts/useGitReviewActions.ts +++ b/src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/parts/useGitReviewActions.ts @@ -1,4 +1,3 @@ -import { useState } from "react"; import { toast } from "@heroui/react"; import type { GitBranchInfo, GitStatusResult, Project, ProjectLocation } from "@/shared/contracts"; import { getProjectAgentStatuses } from "@/shared/agentStatus"; @@ -7,6 +6,10 @@ import { msg, friendlyError, friendlyErrorWithDetail } from "@/shared/messages"; import { readBridge } from "@/renderer/bridge"; import { captureProductEvent } from "@/renderer/analytics/posthog"; import { useAgentStatusesStore } from "@/renderer/state/agentStatusesStore"; +import { + useGitReviewActionState, + useGitReviewActionStore, +} from "@/renderer/state/gitReviewActionStore"; import { useGitStore } from "@/renderer/state/gitStore"; import { startPostPushPrStatusRefresh } from "@/renderer/state/gitRefresh"; import { usePullFromSourceDialogStore } from "@/renderer/state/pullFromSourceDialogStore"; @@ -99,20 +102,44 @@ export function useGitReviewActions(args: UseGitReviewActionsArgs) { wslAgentStatuses, ); - const [commitMessage, setCommitMessage] = useState(""); - const [isCommitting, setIsCommitting] = useState(false); - const [isGenerating, setIsGenerating] = useState(false); - const [isSyncing, setIsSyncing] = useState(false); - const [isMerging, setIsMerging] = useState(false); - const [isPullingFromSource, setIsPullingFromSource] = useState(false); - const [isAbortingMerge, setIsAbortingMerge] = useState(false); - const [isFinishingMerge, setIsFinishingMerge] = useState(false); - - const [prTitle, setPrTitle] = useState(""); - const [prBody, setPrBody] = useState(""); - const [prTargetBranch, setPrTargetBranch] = useState(null); - const [isCreatingPr, setIsCreatingPr] = useState(false); - const [isGeneratingPr, setIsGeneratingPr] = useState(false); + // The git review panel is keyed by `${projectId}:${worktreePath}` and fully + // remounts when the user switches projects, so any useState here would reset + // on switch — dropping draft text, the generation spinner, and any in-flight + // operation's pending state while the work keeps running in the supervisor. + // Holding it all in a store keyed by the same `storeKey` makes it survive the + // remount: spinners reappear, async results land against the right panel even + // if it unmounted mid-flight, and drafts are preserved. See + // gitReviewActionStore. + const { + commitMessage, + prTitle, + prBody, + prTargetBranch, + isGenerating, + isGeneratingPr, + isCommitting, + isSyncing, + isMerging, + isPullingFromSource, + isAbortingMerge, + isFinishingMerge, + isCreatingPr, + } = useGitReviewActionState(storeKey); + const patch = useGitReviewActionStore((s) => s.patch); + const setCommitMessage = (value: string) => patch(storeKey, { commitMessage: value }); + const setPrTitle = (value: string) => patch(storeKey, { prTitle: value }); + const setPrBody = (value: string) => patch(storeKey, { prBody: value }); + const setPrTargetBranch = (value: string | null) => patch(storeKey, { prTargetBranch: value }); + const setIsGenerating = (value: boolean) => patch(storeKey, { isGenerating: value }); + const setIsGeneratingPr = (value: boolean) => patch(storeKey, { isGeneratingPr: value }); + const setIsCommitting = (value: boolean) => patch(storeKey, { isCommitting: value }); + const setIsSyncing = (value: boolean) => patch(storeKey, { isSyncing: value }); + const setIsMerging = (value: boolean) => patch(storeKey, { isMerging: value }); + const setIsPullingFromSource = (value: boolean) => + patch(storeKey, { isPullingFromSource: value }); + const setIsAbortingMerge = (value: boolean) => patch(storeKey, { isAbortingMerge: value }); + const setIsFinishingMerge = (value: boolean) => patch(storeKey, { isFinishingMerge: value }); + const setIsCreatingPr = (value: boolean) => patch(storeKey, { isCreatingPr: value }); const writeActions = usePrWriteActions({ projectLocation: project.location, @@ -240,6 +267,9 @@ export function useGitReviewActions(args: UseGitReviewActionsArgs) { } async function handleGenerateMessage(): Promise { + // A generation may still be running from before a panel remount — don't + // start a second one; the first one's result will land in the store. + if (isGenerating) return; setIsGenerating(true); try { const message = await generateMessage(); @@ -467,6 +497,9 @@ export function useGitReviewActions(args: UseGitReviewActionsArgs) { return; } + // Don't start a second PR-summary generation if one is still in flight + // from before a panel remount — its result will land in the store. + if (isGeneratingPr) return; setIsGeneratingPr(true); for (const candidate of candidates) { const resolved = resolveCommitGenConfig(candidate, commitGenModel, commitGenEffort);