Skip to content

Commit 95cc821

Browse files
SDSLeonSDSLeon
authored
fix(git-review): persist panel action state across remounts (#150)
- move draft text and in-flight flags into a keyed Zustand store - update the review sidebar hook to read and write shared panel state - add tests for key isolation and remount-survival behavior
1 parent 720c9c0 commit 95cc821

3 files changed

Lines changed: 231 additions & 15 deletions

File tree

‎src/renderer/state/gitReviewActionStore.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
2+
import { useGitReviewActionStore } from "./gitReviewActionStore";
3+
4+
function resetStore() {
5+
useGitReviewActionStore.setState({ panels: {} });
6+
}
7+
8+
const { patch } = useGitReviewActionStore.getState();
9+
const panels = () => useGitReviewActionStore.getState().panels;
10+
11+
describe("gitReviewActionStore", () => {
12+
beforeEach(resetStore);
13+
afterEach(resetStore);
14+
15+
it("keeps panel state keyed and isolated across keys", () => {
16+
patch("projA", { commitMessage: "feat: a", prTargetBranch: "main" });
17+
patch("projB", { commitMessage: "fix: b" });
18+
19+
expect(panels()["projA"]?.commitMessage).toBe("feat: a");
20+
expect(panels()["projA"]?.prTargetBranch).toBe("main");
21+
expect(panels()["projB"]?.commitMessage).toBe("fix: b");
22+
// Writing one panel must not bleed into another.
23+
patch("projA", { commitMessage: "feat: a2" });
24+
expect(panels()["projB"]?.commitMessage).toBe("fix: b");
25+
});
26+
27+
// The bug this store fixes: the git panel unmounts on project switch, so an
28+
// async action (which keeps running in the supervisor) resolved into a dead
29+
// component and its result/pending state was dropped. Routed through the
30+
// store, both are captured against the panel's key and are there on return.
31+
it("captures a generation result that resolves after the panel 'unmounts'", () => {
32+
// Panel A kicks off generation, then the user switches away (no reads).
33+
patch("projA", { isGenerating: true });
34+
expect(panels()["projA"]?.isGenerating).toBe(true);
35+
36+
// While away, the user works in panel B — independent state.
37+
patch("projB", { commitMessage: "wip" });
38+
39+
// Generation completes after the unmount and writes through the captured,
40+
// key-bound setter (this is what used to hit a dead component).
41+
patch("projA", { commitMessage: "feat: captured", isGenerating: false });
42+
43+
// Back on panel A: the spinner is cleared and the message is waiting.
44+
expect(panels()["projA"]).toMatchObject({
45+
commitMessage: "feat: captured",
46+
isGenerating: false,
47+
});
48+
// Panel B was never disturbed.
49+
expect(panels()["projB"]?.commitMessage).toBe("wip");
50+
});
51+
52+
it("preserves an in-flight commit/push spinner across a switch", () => {
53+
// Commit & Push uses both flags; both must survive the remount.
54+
patch("p", { isCommitting: true, isSyncing: true });
55+
// ...user switches away and back (re-read) — still pending.
56+
expect(panels()["p"]).toMatchObject({ isCommitting: true, isSyncing: true });
57+
// Operation finishes and clears them.
58+
patch("p", { isCommitting: false, isSyncing: false });
59+
expect(panels()["p"]).toMatchObject({ isCommitting: false, isSyncing: false });
60+
});
61+
62+
it("tracks each in-flight action flag independently", () => {
63+
patch("p", { isMerging: true, isPullingFromSource: true, isCreatingPr: true });
64+
patch("p", { isMerging: false });
65+
expect(panels()["p"]).toMatchObject({
66+
isMerging: false,
67+
isPullingFromSource: true,
68+
isCreatingPr: true,
69+
});
70+
});
71+
72+
it("skips no-op writes so subscribers don't re-render needlessly", () => {
73+
patch("p", { commitMessage: "x" });
74+
const before = panels();
75+
patch("p", { commitMessage: "x" }); // same value
76+
expect(panels()).toBe(before); // identical reference — no update
77+
});
78+
79+
it("leaves untouched keys referentially stable when another key changes", () => {
80+
patch("a", { commitMessage: "a" });
81+
patch("b", { commitMessage: "b" });
82+
const panelB = panels()["b"];
83+
patch("a", { commitMessage: "a2" });
84+
// 'b' object identity is preserved, so a selector on key 'b' won't re-render.
85+
expect(panels()["b"]).toBe(panelB);
86+
});
87+
});

‎src/renderer/state/gitReviewActionStore.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { create } from "zustand";
2+
3+
/**
4+
* Per-(project, worktree) working state for the git review panel: in-progress
5+
* drafts (commit message, PR title/body/target) and the in-flight flags for
6+
* every async action the panel can run (generate, commit, sync, merge, pull,
7+
* create PR).
8+
*
9+
* Lives in a module-level store — NOT component `useState` — so it survives
10+
* the panel unmounting and remounting when the user switches projects. The
11+
* panel is keyed by `${projectId}:${worktreePath}` (see AppOverlays /
12+
* GitReviewPanelContent), so switching away and back fully remounts the
13+
* subtree; local state there would reset to empty. Because every action
14+
* (commit, push, merge, and especially the long-running generate/PR-summary
15+
* supervisor calls) keeps running across that remount, its pending flag and
16+
* any result must live somewhere that outlives the component. Routing all of
17+
* it through this store — keyed by the same `storeKey` the panel uses
18+
* (`statusKey ?? project.id`) — means spinners reappear on return and async
19+
* results land against the right panel even if it unmounted mid-flight,
20+
* instead of resolving into a dead component instance.
21+
*
22+
* Intentionally in-memory only (no localStorage): an in-flight flag must not
23+
* survive an app restart, or an action killed with the process would leave its
24+
* spinner stuck on forever.
25+
*/
26+
export interface GitReviewActionState {
27+
/** Draft commit message — typed by the user or filled in by generation. */
28+
commitMessage: string;
29+
/** Draft PR title. */
30+
prTitle: string;
31+
/** Draft PR body. */
32+
prBody: string;
33+
/** Draft PR target branch (null = use the resolved source branch). */
34+
prTargetBranch: string | null;
35+
/** A commit-message generation is in flight (supervisor one-shot LLM call). */
36+
isGenerating: boolean;
37+
/** A PR-summary generation is in flight. */
38+
isGeneratingPr: boolean;
39+
/** A commit (and optional push) is in flight. */
40+
isCommitting: boolean;
41+
/** A push/sync/pull is in flight. */
42+
isSyncing: boolean;
43+
/** A worktree merge is in flight. */
44+
isMerging: boolean;
45+
/** A pull-from-source is in flight. */
46+
isPullingFromSource: boolean;
47+
/** A merge abort is in flight. */
48+
isAbortingMerge: boolean;
49+
/** A merge finish (commit) is in flight. */
50+
isFinishingMerge: boolean;
51+
/** A PR creation is in flight. */
52+
isCreatingPr: boolean;
53+
}
54+
55+
/** Stable default returned for panels with no state yet — never mutate. */
56+
const EMPTY_STATE: GitReviewActionState = Object.freeze({
57+
commitMessage: "",
58+
prTitle: "",
59+
prBody: "",
60+
prTargetBranch: null,
61+
isGenerating: false,
62+
isGeneratingPr: false,
63+
isCommitting: false,
64+
isSyncing: false,
65+
isMerging: false,
66+
isPullingFromSource: false,
67+
isAbortingMerge: false,
68+
isFinishingMerge: false,
69+
isCreatingPr: false,
70+
});
71+
72+
interface GitReviewActionStore {
73+
panels: Record<string, GitReviewActionState>;
74+
/** Merge `patch` into the state for `key`; no-op writes are skipped. */
75+
patch: (key: string, patch: Partial<GitReviewActionState>) => void;
76+
}
77+
78+
export const useGitReviewActionStore = create<GitReviewActionStore>((set, get) => ({
79+
panels: {},
80+
patch: (key, patch) => {
81+
const current = get().panels[key] ?? EMPTY_STATE;
82+
const keys = Object.keys(patch) as (keyof GitReviewActionState)[];
83+
if (keys.every((k) => current[k] === patch[k])) return;
84+
set((state) => ({
85+
panels: { ...state.panels, [key]: { ...current, ...patch } },
86+
}));
87+
},
88+
}));
89+
90+
/**
91+
* Reactive read of a single panel's state. Returns a stable empty default when
92+
* the panel has no state yet, so an absent key never triggers re-render churn.
93+
*/
94+
export function useGitReviewActionState(key: string): GitReviewActionState {
95+
return useGitReviewActionStore((s) => s.panels[key] ?? EMPTY_STATE);
96+
}

‎src/renderer/views/GitReviewOverlay/parts/GitReviewSidebar/parts/useGitReviewActions.ts‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { useState } from "react";
21
import { toast } from "@heroui/react";
32
import type { GitBranchInfo, GitStatusResult, Project, ProjectLocation } from "@/shared/contracts";
43
import { getProjectAgentStatuses } from "@/shared/agentStatus";
@@ -7,6 +6,10 @@ import { msg, friendlyError, friendlyErrorWithDetail } from "@/shared/messages";
76
import { readBridge } from "@/renderer/bridge";
87
import { captureProductEvent } from "@/renderer/analytics/posthog";
98
import { useAgentStatusesStore } from "@/renderer/state/agentStatusesStore";
9+
import {
10+
useGitReviewActionState,
11+
useGitReviewActionStore,
12+
} from "@/renderer/state/gitReviewActionStore";
1013
import { useGitStore } from "@/renderer/state/gitStore";
1114
import { startPostPushPrStatusRefresh } from "@/renderer/state/gitRefresh";
1215
import { usePullFromSourceDialogStore } from "@/renderer/state/pullFromSourceDialogStore";
@@ -99,20 +102,44 @@ export function useGitReviewActions(args: UseGitReviewActionsArgs) {
99102
wslAgentStatuses,
100103
);
101104

102-
const [commitMessage, setCommitMessage] = useState("");
103-
const [isCommitting, setIsCommitting] = useState(false);
104-
const [isGenerating, setIsGenerating] = useState(false);
105-
const [isSyncing, setIsSyncing] = useState(false);
106-
const [isMerging, setIsMerging] = useState(false);
107-
const [isPullingFromSource, setIsPullingFromSource] = useState(false);
108-
const [isAbortingMerge, setIsAbortingMerge] = useState(false);
109-
const [isFinishingMerge, setIsFinishingMerge] = useState(false);
110-
111-
const [prTitle, setPrTitle] = useState("");
112-
const [prBody, setPrBody] = useState("");
113-
const [prTargetBranch, setPrTargetBranch] = useState<string | null>(null);
114-
const [isCreatingPr, setIsCreatingPr] = useState(false);
115-
const [isGeneratingPr, setIsGeneratingPr] = useState(false);
105+
// The git review panel is keyed by `${projectId}:${worktreePath}` and fully
106+
// remounts when the user switches projects, so any useState here would reset
107+
// on switch — dropping draft text, the generation spinner, and any in-flight
108+
// operation's pending state while the work keeps running in the supervisor.
109+
// Holding it all in a store keyed by the same `storeKey` makes it survive the
110+
// remount: spinners reappear, async results land against the right panel even
111+
// if it unmounted mid-flight, and drafts are preserved. See
112+
// gitReviewActionStore.
113+
const {
114+
commitMessage,
115+
prTitle,
116+
prBody,
117+
prTargetBranch,
118+
isGenerating,
119+
isGeneratingPr,
120+
isCommitting,
121+
isSyncing,
122+
isMerging,
123+
isPullingFromSource,
124+
isAbortingMerge,
125+
isFinishingMerge,
126+
isCreatingPr,
127+
} = useGitReviewActionState(storeKey);
128+
const patch = useGitReviewActionStore((s) => s.patch);
129+
const setCommitMessage = (value: string) => patch(storeKey, { commitMessage: value });
130+
const setPrTitle = (value: string) => patch(storeKey, { prTitle: value });
131+
const setPrBody = (value: string) => patch(storeKey, { prBody: value });
132+
const setPrTargetBranch = (value: string | null) => patch(storeKey, { prTargetBranch: value });
133+
const setIsGenerating = (value: boolean) => patch(storeKey, { isGenerating: value });
134+
const setIsGeneratingPr = (value: boolean) => patch(storeKey, { isGeneratingPr: value });
135+
const setIsCommitting = (value: boolean) => patch(storeKey, { isCommitting: value });
136+
const setIsSyncing = (value: boolean) => patch(storeKey, { isSyncing: value });
137+
const setIsMerging = (value: boolean) => patch(storeKey, { isMerging: value });
138+
const setIsPullingFromSource = (value: boolean) =>
139+
patch(storeKey, { isPullingFromSource: value });
140+
const setIsAbortingMerge = (value: boolean) => patch(storeKey, { isAbortingMerge: value });
141+
const setIsFinishingMerge = (value: boolean) => patch(storeKey, { isFinishingMerge: value });
142+
const setIsCreatingPr = (value: boolean) => patch(storeKey, { isCreatingPr: value });
116143

117144
const writeActions = usePrWriteActions({
118145
projectLocation: project.location,
@@ -240,6 +267,9 @@ export function useGitReviewActions(args: UseGitReviewActionsArgs) {
240267
}
241268

242269
async function handleGenerateMessage(): Promise<void> {
270+
// A generation may still be running from before a panel remount — don't
271+
// start a second one; the first one's result will land in the store.
272+
if (isGenerating) return;
243273
setIsGenerating(true);
244274
try {
245275
const message = await generateMessage();
@@ -467,6 +497,9 @@ export function useGitReviewActions(args: UseGitReviewActionsArgs) {
467497
return;
468498
}
469499

500+
// Don't start a second PR-summary generation if one is still in flight
501+
// from before a panel remount — its result will land in the store.
502+
if (isGeneratingPr) return;
470503
setIsGeneratingPr(true);
471504
for (const candidate of candidates) {
472505
const resolved = resolveCommitGenConfig(candidate, commitGenModel, commitGenEffort);

0 commit comments

Comments
 (0)