feat(git): add graph view + branch switcher dropdown#962
Open
feldsys wants to merge 3 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds commit-graph rendering and interactive branch switching: new dependency, main IPC handlers, preload and renderer APIs/types, GitGraphView component, BranchSwitcherDropdown UI, GitLogViewer list/graph toggle, MainPanelHeader click/keyboard wiring, modal priority constant, and tests. ChangesGit Graph Visualization & Branch Switching Feature
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant Preload
participant Main
participant GitCLI
Renderer->>Preload: window.maestro.git.graph(cwd, options)
Preload->>Main: ipc invoke 'git:graph'
Main->>GitCLI: git log --pretty=GRAPH_START...
GitCLI-->>Main: stdout (nodes)
Main-->>Preload: { nodes, error }
Preload-->>Renderer: { nodes, error }
Renderer->>Preload: window.maestro.git.switchBranch(cwd, branch)
Preload->>Main: ipc invoke 'git:switch'
Main->>GitCLI: git switch <branch>
GitCLI-->>Main: stdout/stderr (exit code)
Main-->>Preload: { success, stdout, stderr }
Preload-->>Renderer: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
patches/node-pty+1.1.0.patch (1)
213-214:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix OOB/uninitialized FD close in cleanup loop
At Line 213,
i <= countcan accesslow_fds[3]whencount == 3, and may also close entries never initialized on early-exit paths. This is a runtime safety bug.Suggested fix
- int low_fds[3]; + int low_fds[3] = { -1, -1, -1 }; size_t count = 0; @@ - for (size_t i = 0; i <= count; i++) { - close(low_fds[i]); + size_t opened = (count < 3) ? (count + 1) : 3; + for (size_t i = 0; i < opened; i++) { + if (low_fds[i] != -1) { + close(low_fds[i]); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/node-pty`+1.1.0.patch around lines 213 - 214, The cleanup loop is using `for (size_t i = 0; i <= count; i++) { close(low_fds[i]); }`, which can index one past the array and close uninitialized entries; change the loop to only iterate over actual initialized file descriptors (e.g. `for (size_t i = 0; i < count; ++i)`) or introduce and use an `initialized_count` that is incremented as entries in `low_fds` are populated and then loop `i < initialized_count` to ensure you never access `low_fds` out of bounds or close uninitialized FDs; apply this change where `low_fds` and `count` are used in the cleanup routine.src/renderer/components/MainPanel/MainPanelHeader.tsx (1)
166-202:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProvide a keyboard-accessible path to open the branch switcher.
Right now the new branch-switch flow is only exposed via
onDoubleClickon the chip. A focused button still triggers the single-click path on Enter/Space, so keyboard-only users can't reach branch switching at all. Add a dedicated key path or separate switch button.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/MainPanel/MainPanelHeader.tsx` around lines 166 - 202, The branch-switch flow is only reachable via onDoubleClick on the branch chip in MainPanelHeader, blocking keyboard users; update the branch chip button (the element using handleBranchChipClick and handleBranchChipDoubleClick) to provide a keyboard-accessible path by either adding an onKeyDown that maps a specific key combo (e.g., Enter+Shift or Enter while holding Alt) to call handleBranchChipDoubleClick, or add a separate, focusable small button (with an accessible aria-label like "Switch branch") next to the chip that invokes handleBranchChipDoubleClick; ensure the new control is reachable by keyboard, has an appropriate title/aria-label, and preserves existing single-click/Enter behavior for handleBranchChipClick.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/components/GitGraphView.tsx`:
- Around line 87-160: The Gitgraph instance is created only on mount so the
per-commit styling in commitOptions (the selectedHash-based style built inside
the Gitgraph children function) won't update when selectedHash changes; to force
a remount, add key={selectedHash ?? ''} to the <Gitgraph> element in
GitGraphView (so the Gitgraph component re-initializes and re-renders children
when selectedHash changes). Optionally consider the alternative mentioned in the
comment—external DOM/CSS highlighting—if remounting large graphs proves too
expensive.
In `@src/renderer/components/GitLogViewer.tsx`:
- Around line 114-120: The graph view fetches only 100 commits while the list
view fetches 200, causing view desync when a selected commit is outside the
graph range; update the graph fetch to use the same limit as the list (use a
shared constant rather than hardcoding 100) so both gitService.getGraph(cwd, {
limit: ... }, sshRemoteId) and the list fetch use the same limit value (extract
a COMMIT_FETCH_LIMIT constant and reference it where getGraph and the list fetch
are called, and ensure any related logic that assumes 200 is updated
accordingly).
- Around line 47-50: The initial localStorage read in the GitLogViewer
component's useState initializer (where
localStorage.getItem(VIEW_MODE_STORAGE_KEY) is used) can throw and crash render;
wrap the getItem call in a try/catch so any exceptions are caught and the
initializer returns the safe default 'list', and within the catch log the error
(e.g., console.error or the app logger) instead of silently swallowing it;
ensure the initializer still returns 'graph' only when stored === 'graph' and
otherwise falls back to 'list'.
In `@src/renderer/components/MainPanel/BranchSwitcherDropdown.tsx`:
- Around line 110-133: doSwitch currently sets switching before awaiting
gitService.switchBranch but only clears it on the success path, which leaves
switching stuck if switchBranch throws; wrap the await in a try/catch/finally
inside doSwitch: in try await gitService.switchBranch(cwd, branch, sshRemoteId),
in catch call notifyToast (or propagate) with the error details (do not silently
swallow), and in finally always call setSwitching(null) so the UI is restored;
reference doSwitch, gitService.switchBranch, setSwitching, notifyToast,
onSwitched, and onClose when implementing the change.
In `@src/renderer/services/git.ts`:
- Around line 188-195: The wrapper currently swallows main-process errors from
window.maestro.git.graph by returning an empty array; change the call inside
createIpcMethod so that after awaiting window.maestro.git.graph(cwd, options,
sshRemoteId) you check result.error and if present surface it (e.g., throw a new
Error(result.error) or return the full result object) instead of returning
[]—otherwise only return result.nodes (or []) when there is no result.error;
update the call in the createIpcMethod block that references
window.maestro.git.graph and result.nodes accordingly.
---
Outside diff comments:
In `@patches/node-pty`+1.1.0.patch:
- Around line 213-214: The cleanup loop is using `for (size_t i = 0; i <= count;
i++) { close(low_fds[i]); }`, which can index one past the array and close
uninitialized entries; change the loop to only iterate over actual initialized
file descriptors (e.g. `for (size_t i = 0; i < count; ++i)`) or introduce and
use an `initialized_count` that is incremented as entries in `low_fds` are
populated and then loop `i < initialized_count` to ensure you never access
`low_fds` out of bounds or close uninitialized FDs; apply this change where
`low_fds` and `count` are used in the cleanup routine.
In `@src/renderer/components/MainPanel/MainPanelHeader.tsx`:
- Around line 166-202: The branch-switch flow is only reachable via
onDoubleClick on the branch chip in MainPanelHeader, blocking keyboard users;
update the branch chip button (the element using handleBranchChipClick and
handleBranchChipDoubleClick) to provide a keyboard-accessible path by either
adding an onKeyDown that maps a specific key combo (e.g., Enter+Shift or Enter
while holding Alt) to call handleBranchChipDoubleClick, or add a separate,
focusable small button (with an accessible aria-label like "Switch branch") next
to the chip that invokes handleBranchChipDoubleClick; ensure the new control is
reachable by keyboard, has an appropriate title/aria-label, and preserves
existing single-click/Enter behavior for handleBranchChipClick.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2147a55e-08cb-4b8c-884b-eb92aeba45cf
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
package.jsonpatches/node-pty+1.1.0.patchsrc/__tests__/renderer/components/MainPanel.test.tsxsrc/main/ipc/handlers/git.tssrc/main/preload/git.tssrc/renderer/components/GitGraphView.tsxsrc/renderer/components/GitLogViewer.tsxsrc/renderer/components/MainPanel/BranchSwitcherDropdown.tsxsrc/renderer/components/MainPanel/MainPanelHeader.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/global.d.tssrc/renderer/services/git.ts
Greptile SummaryThis PR adds two interactive git features to the Maestro header: a toggleable list/graph commit view in
Confidence Score: 4/5Safe to merge; the new features work correctly for the common case and failures surface gracefully. The IPC plumbing, branch-switcher, and single/double-click debounce are solid. The graph view has two edge cases worth tracking: date-based sort may render incorrect topology for repos with backdated or rebased commits, and full GitgraphCore reconstruction on every commit click could cause jank at the 200-commit limit. Neither blocks shipping, but the graph topology issue is a silent correctness gap for non-trivial histories. src/renderer/components/GitGraphView.tsx — commit ordering and per-click rebuild strategy. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant MH as MainPanelHeader
participant BSD as BranchSwitcherDropdown
participant GLV as GitLogViewer
participant GGV as GitGraphView
participant SVC as gitService
participant IPC as git IPC handlers
U->>MH: single click branch chip (220ms)
MH->>GLV: setGitLogOpen(true)
GLV->>IPC: git:log + git:commitCount
U->>MH: double click branch chip
MH->>BSD: open BranchSwitcherDropdown
BSD->>SVC: getBranches(cwd, sshRemoteId)
SVC->>IPC: git:branches
IPC-->>BSD: string[]
U->>BSD: select branch
BSD->>SVC: switchBranch(cwd, branch, sshRemoteId)
SVC->>IPC: git:switch
IPC-->>SVC: "{ success, stdout, stderr }"
alt success
BSD->>MH: onSwitched → refreshGitStatus()
BSD->>BSD: onClose()
else failure
BSD-->>U: notifyToast(stderr)
end
U->>GLV: click Graph toggle
GLV->>SVC: "getGraph(cwd, {limit:200}, sshRemoteId)"
SVC->>IPC: git:graph (--all)
IPC-->>GGV: GitGraphNode[]
GGV-->>U: SVG commit graph
U->>GGV: click commit node
GGV->>GLV: onCommitClick(hash)
GLV->>IPC: git:show (load diff)
Reviews (4): Last reviewed commit: "fix(git): handle refreshGitStatus + getB..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Collaborator
|
@feldsys — thanks a lot for this contribution! Nice piece of work threading both new affordances cleanly through the existing SSH-remote pathway, and the issue→PR mapping in the description is super helpful. A few things to address before this can land. CodeRabbit caught most of these and I agree with them; flagging here so they don't get lost: Bugs / correctness
Defense-in-depth nit
Scope
Minor / discussion
Once the items above are addressed, happy to take another pass. |
feldsys
added a commit
to feldsys/Maestro
that referenced
this pull request
May 7, 2026
- BranchSwitcherDropdown.doSwitch wrapped in try/finally so a rejected
switchBranch IPC no longer leaves the dropdown permanently spinning
and the other branch buttons disabled.
- GitLogViewer list and graph views now share a COMMIT_FETCH_LIMIT
constant (200) so toggling between them after selecting a deep commit
no longer desyncs the detail panel.
- gitService.getGraph rethrows main-process errors instead of
collapsing them into an empty array; GitLogViewer surfaces the
message via a new graphError state.
- Guard the initial localStorage read in the view-mode initialiser so a
throwing storage backend can't crash the modal mid-render.
- graphLoading initialised to true so the first frame after toggling
to Graph shows the spinner instead of flashing "No commits found".
- <Gitgraph key={selectedHash}> forces a remount so the
selected-commit dot styling takes effect (@gitgraph/react@1.6.0
doesn't re-run the children function on prop changes).
- gitService.getGraph and gitService.switchBranch accept and forward
remoteCwd so SSH sessions with a divergent remote working directory
hit the correct path.
- git:switch IPC handler rejects branch names starting with "-" to
block flag injection through execFile.
- Branch chip: Shift+Enter opens the branch switcher (keyboard a11y
parity with double-click); the 220ms single-click debounce timer is
cleared on unmount so it can't fire against a stale parent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feldsys
force-pushed
the
feature/git-graph-and-branch-switch
branch
from
6336308 to
d9c107b
Compare
3 tasks
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/components/GitLogViewer.tsx (1)
123-144: ⚡ Quick winGraph data reloads on every re-toggle to graph mode, despite the stated "first time" intent.
Every time
viewModechanges to'graph', the effect fires unconditionally — showing a loading spinner and re-fetching even whengraphNodesis already populated for the currentcwd/sshRemoteId. On SSH remotes this adds noticeable latency each time the user switches back to graph view.A
useRefloaded-key guard would fix this cleanly:♻️ Suggested refactor
+ const graphLoadedKeyRef = useRef<string | null>(null); + // Lazy-load graph data the first time the user switches to the Graph view (and on cwd change). useEffect(() => { if (viewMode !== 'graph') return; + const loadKey = `${cwd}::${sshRemoteId ?? ''}`; + if (graphLoadedKeyRef.current === loadKey) return; // already loaded for this cwd/remote let cancelled = false; setGraphLoading(true); setGraphError(null); (async () => { try { const nodes = await gitService.getGraph(cwd, { limit: COMMIT_FETCH_LIMIT }, sshRemoteId); if (!cancelled) { setGraphNodes(nodes); + graphLoadedKeyRef.current = loadKey; // mark as loaded only on success } } catch (err) { if (!cancelled) { setGraphError(err instanceof Error ? err.message : String(err)); setGraphNodes([]); + // Don't update loadKey — keeps retry working on next toggle } } finally { if (!cancelled) setGraphLoading(false); } })(); return () => { cancelled = true; }; }, [viewMode, cwd, sshRemoteId]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/GitLogViewer.tsx` around lines 123 - 144, The effect that fetches graph data (useEffect watching viewMode, cwd, sshRemoteId) unconditionally reloads on every toggle to 'graph'; add a useRef guard (e.g., loadedGraphRef keyed by cwd+sshRemoteId) to skip the fetch when graphNodes for the current cwd/sshRemoteId are already loaded: before setting setGraphLoading(true) and calling gitService.getGraph(..., COMMIT_FETCH_LIMIT, sshRemoteId) check the ref and return early if it indicates data is present; on successful load (where setGraphNodes is called) mark the ref as loaded for that key, and clear the ref entry if a fetch fails or cwd/sshRemoteId change so future loads still occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/renderer/components/GitLogViewer.tsx`:
- Around line 123-144: The effect that fetches graph data (useEffect watching
viewMode, cwd, sshRemoteId) unconditionally reloads on every toggle to 'graph';
add a useRef guard (e.g., loadedGraphRef keyed by cwd+sshRemoteId) to skip the
fetch when graphNodes for the current cwd/sshRemoteId are already loaded: before
setting setGraphLoading(true) and calling gitService.getGraph(...,
COMMIT_FETCH_LIMIT, sshRemoteId) check the ref and return early if it indicates
data is present; on successful load (where setGraphNodes is called) mark the ref
as loaded for that key, and clear the ref entry if a fetch fails or
cwd/sshRemoteId change so future loads still occur.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d7bbe45-772c-4d76-8da4-1ef2574ba194
📒 Files selected for processing (7)
src/__tests__/renderer/components/MainPanel.test.tsxsrc/main/ipc/handlers/git.tssrc/renderer/components/GitGraphView.tsxsrc/renderer/components/GitLogViewer.tsxsrc/renderer/components/MainPanel/BranchSwitcherDropdown.tsxsrc/renderer/components/MainPanel/MainPanelHeader.tsxsrc/renderer/services/git.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/tests/renderer/components/MainPanel.test.tsx
- src/renderer/components/MainPanel/BranchSwitcherDropdown.tsx
- src/main/ipc/handlers/git.ts
feldsys
added a commit
to feldsys/Maestro
that referenced
this pull request
May 7, 2026
- BranchSwitcherDropdown.doSwitch wrapped in try/finally so a rejected
switchBranch IPC no longer leaves the dropdown permanently spinning
and the other branch buttons disabled.
- GitLogViewer list and graph views now share a COMMIT_FETCH_LIMIT
constant (200) so toggling between them after selecting a deep commit
no longer desyncs the detail panel.
- gitService.getGraph rethrows main-process errors instead of
collapsing them into an empty array; GitLogViewer surfaces the
message via a new graphError state.
- Guard the initial localStorage read in the view-mode initialiser so a
throwing storage backend can't crash the modal mid-render.
- graphLoading initialised to true so the first frame after toggling
to Graph shows the spinner instead of flashing "No commits found".
- GitGraphView refactored to build a GitgraphCore instance via useMemo
and pass it through @gitgraph/react's `<Gitgraph graph={...}>` prop
instead of the children-callback API. The callback API populates the
graph during componentDidMount, which under React.StrictMode runs
twice and ends up corrupting the SVG (blanks it out on busy `--all`
ranges; production was unaffected because StrictMode is dev-only).
Owning the GitgraphCore ourselves keeps the data stable across the
dev mount→unmount→remount cycle, so dev now matches production.
Full commit hashes are passed as commit ids so React keys are unique
even when 7-char shortHashes collide. The originally-suggested
`key={selectedHash}` is kept on top of graph-prop mode so the
selected-commit dot styling actually updates on click; this is now
StrictMode-safe because both mounts share the useMemo'd GitgraphCore.
- @gitgraph/core declared explicitly in package.json since we now
import GitgraphCore directly (was previously a transitive dep).
- gitService.getGraph and gitService.switchBranch accept and forward
remoteCwd so SSH sessions with a divergent remote working directory
hit the correct path.
- git:switch IPC handler rejects branch names starting with "-" to
block flag injection through execFile.
- Branch chip: Shift+Enter opens the branch switcher (keyboard a11y
parity with double-click); the 220ms single-click debounce timer is
cleared on unmount so it can't fire against a stale parent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feldsys
force-pushed
the
feature/git-graph-and-branch-switch
branch
from
d9c107b to
4e2f72d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/components/MainPanel/BranchSwitcherDropdown.tsx (1)
47-62: ⚡ Quick winSurface
getBranchesfailures to the user.The promise chain has
.thenand.finallybut no.catch. IfgitService.getBranchesrejects (IPC failure, transport error, etc.),branchesstays[],loadingflips tofalse, and the user sees "No matching branches" — indistinguishable from a repo with zero branches. The rejection also surfaces as an unhandled promise rejection.Add an explicit error path so the user knows what happened, e.g. via a local error state or a toast.
🛡️ Proposed fix (toast-based, mirroring `doSwitch`'s pattern)
useEffect(() => { let cancelled = false; setLoading(true); gitService .getBranches(cwd, sshRemoteId) .then((b) => { if (cancelled) return; setBranches(b); }) + .catch((err) => { + if (cancelled) return; + notifyToast({ + color: 'red', + title: 'Failed to load branches', + message: err instanceof Error ? err.message : String(err), + dismissible: true, + }); + }) .finally(() => { if (!cancelled) setLoading(false); }); return () => { cancelled = true; }; }, [cwd, sshRemoteId]);As per coding guidelines: "Do not silently swallow errors. Handle expected/recoverable errors explicitly."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/MainPanel/BranchSwitcherDropdown.tsx` around lines 47 - 62, The effect in BranchSwitcherDropdown calls gitService.getBranches but lacks error handling; add a .catch handler to the promise returned by gitService.getBranches(cwd, sshRemoteId) that records/shows the error (either set a local error state and render an error message or call the same toast/error helper used by doSwitch) and ensure setLoading(false) still happens; update the effect to call setBranches only on success, set an error (or toast) on failure, and keep the existing cancelled-flag checks so you don't update state after unmount.src/renderer/components/GitLogViewer.tsx (1)
452-466: ⚡ Quick winMemoize
onCommitClickto avoid rebuilding the graph on every render.The arrow function passed as
onCommitClickis recreated on each render ofGitLogViewer.GitGraphViewincludesonCommitClickin the dep array of its mainuseMemo(the one that imperatively builds theGitgraphCoreinstance over all ordered nodes), so every parent render — e.g. typing in unrelated state, scroll-driven re-renders, diff-loading toggles — invalidates the memo and rebuilds the full graph for ~200 commits, plus a remount via thekey={selectedHash}on the next selection change.Wrap the handler in
useCallback:♻️ Proposed fix
+ const handleGraphCommitClick = useCallback( + (hash: string) => { + const idx = entries.findIndex((e) => e.hash === hash); + if (idx >= 0) { + setGraphSelected(null); + setSelectedIndex(idx); + } else { + const node = graphNodes.find((n) => n.hash === hash); + if (node) setGraphSelected(node); + } + }, + [entries, graphNodes, setSelectedIndex] + ); @@ <GitGraphView nodes={graphNodes} theme={theme} selectedHash={displayedCommit?.hash} - onCommitClick={(hash) => { - const idx = entries.findIndex((e) => e.hash === hash); - if (idx >= 0) { - setGraphSelected(null); - setSelectedIndex(idx); - } else { - const node = graphNodes.find((n) => n.hash === hash); - if (node) setGraphSelected(node); - } - }} + onCommitClick={handleGraphCommitClick} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/GitLogViewer.tsx` around lines 452 - 466, The onCommitClick handler passed into GitGraphView inside GitLogViewer is recreated every render causing the graph to rebuild; wrap that inline arrow in a useCallback within GitLogViewer (referencing entries, graphNodes, setGraphSelected, setSelectedIndex, and displayedCommit?.hash as needed) so the function identity is stable and only changes when its dependencies change; then pass the memoized callback to GitGraphView to prevent unnecessary recomputations and remounts of the GitgraphCore instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/renderer/components/GitLogViewer.tsx`:
- Around line 452-466: The onCommitClick handler passed into GitGraphView inside
GitLogViewer is recreated every render causing the graph to rebuild; wrap that
inline arrow in a useCallback within GitLogViewer (referencing entries,
graphNodes, setGraphSelected, setSelectedIndex, and displayedCommit?.hash as
needed) so the function identity is stable and only changes when its
dependencies change; then pass the memoized callback to GitGraphView to prevent
unnecessary recomputations and remounts of the GitgraphCore instance.
In `@src/renderer/components/MainPanel/BranchSwitcherDropdown.tsx`:
- Around line 47-62: The effect in BranchSwitcherDropdown calls
gitService.getBranches but lacks error handling; add a .catch handler to the
promise returned by gitService.getBranches(cwd, sshRemoteId) that records/shows
the error (either set a local error state and render an error message or call
the same toast/error helper used by doSwitch) and ensure setLoading(false) still
happens; update the effect to call setBranches only on success, set an error (or
toast) on failure, and keep the existing cancelled-flag checks so you don't
update state after unmount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 438a5982-5bb6-4e05-928c-2f9e3273a2af
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.jsonsrc/__tests__/renderer/components/MainPanel.test.tsxsrc/main/ipc/handlers/git.tssrc/renderer/components/GitGraphView.tsxsrc/renderer/components/GitLogViewer.tsxsrc/renderer/components/MainPanel/BranchSwitcherDropdown.tsxsrc/renderer/components/MainPanel/MainPanelHeader.tsxsrc/renderer/services/git.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/tests/renderer/components/MainPanel.test.tsx
- src/renderer/components/MainPanel/MainPanelHeader.tsx
- src/renderer/services/git.ts
feldsys
added a commit
to feldsys/Maestro
that referenced
this pull request
May 7, 2026
- BranchSwitcherDropdown.doSwitch wrapped in try/finally so a rejected
switchBranch IPC no longer leaves the dropdown permanently spinning
and the other branch buttons disabled.
- GitLogViewer list and graph views now share a COMMIT_FETCH_LIMIT
constant (200) so toggling between them after selecting a deep commit
no longer desyncs the detail panel.
- gitService.getGraph rethrows main-process errors instead of
collapsing them into an empty array; GitLogViewer surfaces the
message via a new graphError state.
- Guard the initial localStorage read in the view-mode initialiser so a
throwing storage backend can't crash the modal mid-render.
- graphLoading initialised to true so the first frame after toggling
to Graph shows the spinner instead of flashing "No commits found".
- GitGraphView refactored to build a GitgraphCore instance via useMemo
and pass it through @gitgraph/react's `<Gitgraph graph={...}>` prop
instead of the children-callback API. The callback API populates the
graph during componentDidMount, which under React.StrictMode runs
twice and ends up corrupting the SVG (blanks it out on busy `--all`
ranges; production was unaffected because StrictMode is dev-only).
Owning the GitgraphCore ourselves keeps the data stable across the
dev mount→unmount→remount cycle, so dev now matches production.
Full commit hashes are passed as commit ids so React keys are unique
even when 7-char shortHashes collide. The originally-suggested
`key={selectedHash}` is kept on top of graph-prop mode so the
selected-commit dot styling actually updates on click; this is now
StrictMode-safe because both mounts share the useMemo'd GitgraphCore.
- @gitgraph/core declared explicitly in package.json since we now
import GitgraphCore directly (was previously a transitive dep).
- gitService.getGraph and gitService.switchBranch accept and forward
remoteCwd so SSH sessions with a divergent remote working directory
hit the correct path.
- git:switch IPC handler rejects branch names starting with "-" to
block flag injection through execFile.
- git:graph IPC handler now uses ASCII Unit Separator (%x1f) instead
of "|" between fields. Author names and commit subjects can legally
contain "|", which silently shifts every subsequent field by one
position and produces wrong dates / refs / subjects in the rendered
graph. US is a non-printing control character that doesn't appear
in real text.
- handleGraphCommitClick wrapped in useCallback so a stable identity
is passed to GitGraphView; previously the inline arrow rebuilt the
entire GitgraphCore in useMemo on every parent render.
- Branch chip: Shift+Enter opens the branch switcher (keyboard a11y
parity with double-click); the 220ms single-click debounce timer is
cleared on unmount so it can't fire against a stale parent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feldsys
force-pushed
the
feature/git-graph-and-branch-switch
branch
from
4e2f72d to
54b823b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/components/MainPanel/MainPanelHeader.tsx`:
- Around line 111-117: refreshGitStatus() is being called fire-and-forget in the
MainPanelHeader click handlers (the block using gitTooltip, branchClickTimerRef
and setGitLogOpen) and must explicitly handle rejections; wrap the call in a
try/catch (or use .catch) and report failures using the Sentry utilities
(captureException/captureMessage from src/utils/sentry.ts) with contextual info
(e.g., "refreshGitStatus failed in branch click handler"), and apply the same
pattern to the other handler where refreshGitStatus() is invoked so no rejection
goes unreported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 27b2c2d0-3156-429c-97f7-f56d929796f5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.jsonsrc/__tests__/renderer/components/MainPanel.test.tsxsrc/main/ipc/handlers/git.tssrc/renderer/components/GitGraphView.tsxsrc/renderer/components/GitLogViewer.tsxsrc/renderer/components/MainPanel/BranchSwitcherDropdown.tsxsrc/renderer/components/MainPanel/MainPanelHeader.tsxsrc/renderer/services/git.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- package.json
- src/renderer/services/git.ts
- src/main/ipc/handlers/git.ts
- src/tests/renderer/components/MainPanel.test.tsx
- src/renderer/components/MainPanel/BranchSwitcherDropdown.tsx
- src/renderer/components/GitLogViewer.tsx
- src/renderer/components/GitGraphView.tsx
Contributor
|
@greptile re-review this pr please |
Contributor
|
@feldsys please address the orphan issue in the latest greptile feedback |
feldsys
added a commit
to feldsys/Maestro
that referenced
this pull request
May 8, 2026
Wrap fire-and-forget refreshGitStatus() in MainPanelHeader so unhandled rejections surface to Sentry instead of becoming console noise, and add a .catch on the BranchSwitcherDropdown branch fetch so IPC failures show a toast instead of silently looking like a repo with zero branches. Addresses CodeRabbit findings on PR RunMaestro#962.
Author
|
@jSydorowicz21 i did :) |
feldsys
added a commit
to feldsys/Maestro
that referenced
this pull request
Jun 1, 2026
Adds git graph view + branch switcher dropdown, with PR RunMaestro#962 review feedback and rejection handling for refreshGitStatus + getBranches.
chr1syy
reviewed
Jun 5, 2026
chr1syy
left a comment
chr1syy
left a comment
Contributor
There was a problem hiding this comment.
Took a fresh pass on this — the substantive items from the earlier round are all addressed, so flagging as comment rather than request-changes.
What's holding it up
-
Stale rebase. Last update 2026-05-20, ~50 commits behind
rc. Trial-merge into currentrcproduces exactly one conflict —src/__tests__/renderer/components/MainPanel.test.tsx.rcnow stubsuseLayerStackdirectly with the fullregisterLayer/unregisterLayer/... shape; this PR stubsuseModalLayer. PR's approach is the canonically correct one per CLAUDE.md (useModalLayer()is listed as the canonical mock, "do NOT use manualregisterLayer()boilerplate"), so the resolution is to keep PR's stub and layer theuseLayerStackstub on top, or refactorMainPanelContentto also go throughuseModalLayer. Everything else auto-merges (the IPC handler additions land in a different region thanrc's newgit:inithandler). -
CI has never run. All six workflow runs sit at
action_required— first-time-contributor gate. We have no lint/test signal on the latest commit, only CodeRabbit (✅) and Greptile rounds. Worth a maintainer click to start the workflow once the conflict is resolved.
Earlier review checklist — all addressed
| # | Item from 2026-05-07 review | Status |
|---|---|---|
| 1 | switching could stick on IPC reject |
✅ try/catch/finally in doSwitch |
| 2 | Graph fetched 100, list 200 (desync) | ✅ shared COMMIT_FETCH_LIMIT |
| 3 | getGraph swallowed result.error |
✅ rethrows; graphError state surfaces it |
| 4 | localStorage.getItem initializer unguarded |
✅ try/catch fallback to 'list' |
| 5 | @gitgraph/react children-callback doesn't react to prop changes |
✅ refactored to <Gitgraph graph={GitgraphCore}> via useMemo — better than the suggested key= remount, and StrictMode-safe |
| 6 | Validate branchName against flag injection |
✅ rejects empty / leading - |
| 7 | node-pty patch out of scope | ✅ no longer in diff (12 files, +1035/-34) |
Bonus fixes the author shipped on top: ASCII Unit Separator (%x1f) instead of | as the git log field delimiter (real fix — author names and subjects can legally contain |), full hashes as commit IDs (avoids React-key collisions on short-hash clashes), useCallback around handleGraphCommitClick so GitGraphView's useMemo doesn't rebuild the whole graph every parent render, debounce timer cleared on unmount, Shift+Enter keyboard a11y for the branch switcher, captureException around the fire-and-forget refreshGitStatus().
Minor leftovers — optional, non-blocking
- Graph data refetches on every toggle to Graph mode (
GitLogViewer.tsx~line 123–144). The lazy-load effect fires unconditionally onviewMode === 'graph'; nouseRef-keyed "already loaded" guard. CodeRabbit flagged this as a quick-win nitpick. Noticeable latency on SSH remotes for every Graph re-toggle. useEffect(loadLog, [cwd])missingsshRemoteIdinGitLogViewer.tsx. Pre-existing pattern, but worth folding in if you're already touching the file.BranchSwitcherDropdowndoesn't forwardremoteCwdtogetBranches/switchBranch, even though the newswitchBranchservice accepts it. This matches the existinggitService.getBranches(cwd, sshRemoteId)signature inrc, so the regression isn't introduced here — but SSH sessions with a divergentremoteCwd(set by terminalcdtracking) will hit the wrong directory when switching branches. Worth a follow-up issue.
On the 220 ms single-click delay
Author kept the 220 ms and added Shift+Enter for keyboard parity. The mouse path still pays the delay on every git-log open. Reasonable tradeoff if double-click is the only way to disambiguate; if anyone wants single-click for git-log + a dedicated ⌄ chevron next to the branch chip for the switcher, that's a small follow-up.
Bottom line
Solid work, responsive author, both new affordances threaded cleanly through the SSH-remote pathway. Just needs a rebase + maintainer-kicked CI to land.
feldsys
added a commit
to feldsys/Maestro
that referenced
this pull request
Jun 8, 2026
- BranchSwitcherDropdown.doSwitch wrapped in try/finally so a rejected
switchBranch IPC no longer leaves the dropdown permanently spinning
and the other branch buttons disabled.
- GitLogViewer list and graph views now share a COMMIT_FETCH_LIMIT
constant (200) so toggling between them after selecting a deep commit
no longer desyncs the detail panel.
- gitService.getGraph rethrows main-process errors instead of
collapsing them into an empty array; GitLogViewer surfaces the
message via a new graphError state.
- Guard the initial localStorage read in the view-mode initialiser so a
throwing storage backend can't crash the modal mid-render.
- graphLoading initialised to true so the first frame after toggling
to Graph shows the spinner instead of flashing "No commits found".
- GitGraphView refactored to build a GitgraphCore instance via useMemo
and pass it through @gitgraph/react's `<Gitgraph graph={...}>` prop
instead of the children-callback API. The callback API populates the
graph during componentDidMount, which under React.StrictMode runs
twice and ends up corrupting the SVG (blanks it out on busy `--all`
ranges; production was unaffected because StrictMode is dev-only).
Owning the GitgraphCore ourselves keeps the data stable across the
dev mount→unmount→remount cycle, so dev now matches production.
Full commit hashes are passed as commit ids so React keys are unique
even when 7-char shortHashes collide. The originally-suggested
`key={selectedHash}` is kept on top of graph-prop mode so the
selected-commit dot styling actually updates on click; this is now
StrictMode-safe because both mounts share the useMemo'd GitgraphCore.
- @gitgraph/core declared explicitly in package.json since we now
import GitgraphCore directly (was previously a transitive dep).
- gitService.getGraph and gitService.switchBranch accept and forward
remoteCwd so SSH sessions with a divergent remote working directory
hit the correct path.
- git:switch IPC handler rejects branch names starting with "-" to
block flag injection through execFile.
- git:graph IPC handler now uses ASCII Unit Separator (%x1f) instead
of "|" between fields. Author names and commit subjects can legally
contain "|", which silently shifts every subsequent field by one
position and produces wrong dates / refs / subjects in the rendered
graph. US is a non-printing control character that doesn't appear
in real text.
- handleGraphCommitClick wrapped in useCallback so a stable identity
is passed to GitGraphView; previously the inline arrow rebuilt the
entire GitgraphCore in useMemo on every parent render.
- Branch chip: Shift+Enter opens the branch switcher (keyboard a11y
parity with double-click); the 220ms single-click debounce timer is
cleared on unmount so it can't fire against a stale parent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feldsys
force-pushed
the
feature/git-graph-and-branch-switch
branch
from
da194a9 to
7870daa
Compare
feldsys
added a commit
to feldsys/Maestro
that referenced
this pull request
Jun 8, 2026
Wrap fire-and-forget refreshGitStatus() in MainPanelHeader so unhandled rejections surface to Sentry instead of becoming console noise, and add a .catch on the BranchSwitcherDropdown branch fetch so IPC failures show a toast instead of silently looking like a repo with zero branches. Addresses CodeRabbit findings on PR RunMaestro#962.
Author
|
@chr1syy sorry for the late response, i did the rebase now :) thank you, for your review |
Contributor
|
@greptile @CodeRabbit new review pass please |
Contributor
|
While those are going we do have a short freeze for merging new features in so we can cut a new mainline release but if this ones in good shape we'll get it merged into the next rc! |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Read all key changed files
echo "=== BranchSwitcherDropdown.tsx ==="
cat src/renderer/components/MainPanel/BranchSwitcherDropdown.tsx
echo ""
echo "=== GitLogViewer.tsx ==="
cat src/renderer/components/GitLogViewer.tsx
echo ""
echo "=== GitGraphView.tsx ==="
cat src/renderer/components/GitGraphView.tsx
echo ""
echo "=== renderer/services/git.ts ==="
cat src/renderer/services/git.ts
echo ""
echo "=== main/ipc/handlers/git.ts ==="
cat src/main/ipc/handlers/git.ts
echo ""
echo "=== MainPanelHeader.tsx ==="
cat src/renderer/components/MainPanel/MainPanelHeader.tsx
echo ""
echo "=== MainPanel.test.tsx ==="
cat src/__tests__/renderer/components/MainPanel.test.tsxLength of output: 252691
Review Pass — PR
|
| # | Issue | Status |
|---|---|---|
| 1 | switching state can stick forever |
Fixed — doSwitch has a finally { setSwitching(null) } block |
| 2 | Graph fetches 100, list fetches 200 | Fixed — shared const COMMIT_FETCH_LIMIT = 200 used for both |
| 3 | getGraph swallows main-process error |
Fixed — rethrow: true + throws on result.error, caller has try/catch |
| 4 | localStorage.getItem unguarded |
Fixed — useState initializer wrapped in try/catch, falls back to 'list' |
| 5 | @gitgraph/react children not re-run on prop change |
Fixed — <Gitgraph key={selectedHash ?? 'none'}> forces remount on selection change |
| 6 | branchName flag-injection risk in git:switch |
Fixed — validates typeof === 'string', non-empty, and !startsWith('-') |
🐛 New finding — stale sshRemoteId closure in GitLogViewer.tsx
File: src/renderer/components/GitLogViewer.tsx
loadCommitDiff closes over the sshRemoteId prop but only lists cwd in its dependency array. If sshRemoteId ever changes (e.g., a session is re-configured), diffs will silently be fetched via the stale remote.
// Current — sshRemoteId is stale after remount if it changes
const loadCommitDiff = useCallback(
async (hash: string) => {
const result = await window.maestro.git.show(cwd, hash, sshRemoteId);
// ...
},
[cwd] // ← sshRemoteId missing
);Fix:
const loadCommitDiff = useCallback(
async (hash: string) => {
const result = await window.maestro.git.show(cwd, hash, sshRemoteId);
// ...
},
[cwd, sshRemoteId] // ← add sshRemoteId
);ℹ️ Minor inconsistency — git:log IPC default limit
git:log handler defaults to 100 while git:graph defaults to 200. There is no user-visible impact today because the renderer always passes COMMIT_FETCH_LIMIT, but it's a silent footgun if the handler is ever called without options:
// src/main/ipc/handlers/git.ts
const limit = options?.limit || 100; // git:log
const limit = options?.limit || 200; // git:graph ← inconsistentWorth aligning these to the same value (200).
🔖 Process item (still outstanding)
Issue #7 (separate node-pty Windows fix commit) cannot be confirmed from code alone — please verify with @feldsys whether this commit has been moved to its own PR before merging.
Summary
The PR is in good shape. The one functional bug (stale sshRemoteId in loadCommitDiff) should be fixed before merge. The limit inconsistency and process item are lower priority but worth addressing.
✏️ Learnings added
Learnt from: jSydorowicz21
URL: https://github.com/RunMaestro/Maestro/pull/962
Timestamp: 2026-06-08T14:04:41.752Z
Learning: In RunMaestro/Maestro PR `#962`, `gitService.getGraph` in `src/renderer/services/git.ts` intentionally uses `rethrow: true` (unlike most other gitService methods) so the graph viewer can distinguish a git command failure from a legitimately empty commit list. This is a deliberate exception to the codebase's general pattern of swallowing IPC errors via `defaultValue` without `rethrow`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ksylvan
Repo: RunMaestro/Maestro PR: 925
File: src/renderer/hooks/remote/useAppRemoteEventListeners.ts:518-520
Timestamp: 2026-04-29T04:26:49.067Z
Learning: In RunMaestro/Maestro, `gitService` methods (`isRepo`, `getBranches`, `getTags`, `getStatus`, `getDiff`, etc.) in `src/renderer/services/git.ts` all use `createIpcMethod` with a `defaultValue` and no `rethrow: true`. This means they NEVER throw — IPC failures are swallowed internally. `createIpcMethod` in `src/renderer/services/ipcWrapper.ts` already calls `captureException` on the swallow path. Therefore, wrapping calls to these gitService methods in try/catch and adding `captureException` is unnecessary (dead code) and would duplicate Sentry reports. The correct pattern (as in `useSessionCrud.ts`) is to call gitService methods directly without a surrounding try/catch.
Adds two new affordances to the git surface: - **Graph view in GitLogViewer** — toggleable List/Graph view via header buttons. Graph mode renders commits with parent-hash topology using @gitgraph/react, supports clicking commits to drive the existing diff panel, and persists view preference via localStorage. Side-branch commits only visible via `git log --all` are tracked separately so the right panel stays in sync. - **Branch switcher dropdown on header branch chip** — single click still opens the git log (debounced ~220ms); double click opens a filterable branch picker that switches the working tree via `git switch <branch>`. Failures (e.g. dirty working tree) surface the git stderr in the dropdown. New IPC: `git:graph` (returns commits with parent hashes for lane rendering) and `git:switch` (returns success + stderr instead of throwing). Both honor the existing SSH remote pathway. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- BranchSwitcherDropdown.doSwitch wrapped in try/finally so a rejected
switchBranch IPC no longer leaves the dropdown permanently spinning
and the other branch buttons disabled.
- GitLogViewer list and graph views now share a COMMIT_FETCH_LIMIT
constant (200) so toggling between them after selecting a deep commit
no longer desyncs the detail panel.
- gitService.getGraph rethrows main-process errors instead of
collapsing them into an empty array; GitLogViewer surfaces the
message via a new graphError state.
- Guard the initial localStorage read in the view-mode initialiser so a
throwing storage backend can't crash the modal mid-render.
- graphLoading initialised to true so the first frame after toggling
to Graph shows the spinner instead of flashing "No commits found".
- GitGraphView refactored to build a GitgraphCore instance via useMemo
and pass it through @gitgraph/react's `<Gitgraph graph={...}>` prop
instead of the children-callback API. The callback API populates the
graph during componentDidMount, which under React.StrictMode runs
twice and ends up corrupting the SVG (blanks it out on busy `--all`
ranges; production was unaffected because StrictMode is dev-only).
Owning the GitgraphCore ourselves keeps the data stable across the
dev mount→unmount→remount cycle, so dev now matches production.
Full commit hashes are passed as commit ids so React keys are unique
even when 7-char shortHashes collide. The originally-suggested
`key={selectedHash}` is kept on top of graph-prop mode so the
selected-commit dot styling actually updates on click; this is now
StrictMode-safe because both mounts share the useMemo'd GitgraphCore.
- @gitgraph/core declared explicitly in package.json since we now
import GitgraphCore directly (was previously a transitive dep).
- gitService.getGraph and gitService.switchBranch accept and forward
remoteCwd so SSH sessions with a divergent remote working directory
hit the correct path.
- git:switch IPC handler rejects branch names starting with "-" to
block flag injection through execFile.
- git:graph IPC handler now uses ASCII Unit Separator (%x1f) instead
of "|" between fields. Author names and commit subjects can legally
contain "|", which silently shifts every subsequent field by one
position and produces wrong dates / refs / subjects in the rendered
graph. US is a non-printing control character that doesn't appear
in real text.
- handleGraphCommitClick wrapped in useCallback so a stable identity
is passed to GitGraphView; previously the inline arrow rebuilt the
entire GitgraphCore in useMemo on every parent render.
- Branch chip: Shift+Enter opens the branch switcher (keyboard a11y
parity with double-click); the 220ms single-click debounce timer is
cleared on unmount so it can't fire against a stale parent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrap fire-and-forget refreshGitStatus() in MainPanelHeader so unhandled rejections surface to Sentry instead of becoming console noise, and add a .catch on the BranchSwitcherDropdown branch fetch so IPC failures show a toast instead of silently looking like a repo with zero branches. Addresses CodeRabbit findings on PR RunMaestro#962.
feldsys
force-pushed
the
feature/git-graph-and-branch-switch
branch
from
7870daa to
4773005
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #958
Summary
Two new affordances on the git surface, both plumbed through the existing SSH-remote pathway:
GitLogViewer— toggleable List/Graph buttons in the modal header. Graph mode renders commits with parent-hash topology via@gitgraph/react, supports clicking commits to drive the existing diff panel, and persists the user's view preference vialocalStorage. Side-branch commits only visible viagit log --allare tracked separately so the right-hand commit detail panel stays in sync regardless of which view is active.git switch <branch>. Failures (e.g. dirty working tree) surface git's stderr inside the dropdown instead of throwing.Mapping to issue #958
git switchIPC surface
Two new handlers, both honoring the existing
sshRemoteId/remoteCwdSSH wrapping:git:graph— returns commits with parent hashes ({ nodes, error }) for lane rendering. Default limit 200, configurable viaoptions.limit.git:switch— returns{ success, stdout, stderr }instead of throwing, so the UI can render git's failure message in-place.Files
src/main/ipc/handlers/git.ts,src/main/preload/git.ts,src/renderer/services/git.ts,src/renderer/global.d.ts— IPC + service plumbingsrc/renderer/components/GitGraphView.tsx(new) —@gitgraph/reactwrapper themed viatheme.colorssrc/renderer/components/GitLogViewer.tsx— list/graph toggle + selection syncsrc/renderer/components/MainPanel/BranchSwitcherDropdown.tsx(new) — filterable picker that callsgit:switchsrc/renderer/components/MainPanel/MainPanelHeader.tsx— single/double-click handling on branch chipsrc/renderer/constants/modalPriorities.ts— addsBRANCH_SWITCHER: 195src/__tests__/renderer/components/MainPanel.test.tsx— updated SSH-remote click test, new double-click testpackage.json/package-lock.json— adds@gitgraph/react ^1.6.0The branch also includes a separate commit (
fix(node-pty): patch winpty.gyp paths and harden POSIX spawn errors) that fixes a Windows build issue — happy to split it into its own PR if preferred.Why
Per #958, currently the only way to inspect history is the linear list, which makes branch topology impossible to read at a glance, and the only way to switch branches inside Maestro is to open a terminal. Both moves keep the user inside the AI surface.
Test plan
npm run lint— TypeScript across all three configsnpm run lint:eslint— ESLintnpm run test -- --run src/__tests__/renderer/components/MainPanel.test.tsx— 137 tests including the new double-click + branch switcher coveragegetSshRemoteByIdSummary by CodeRabbit
New Features
Dependency
Tests