Skip to content

Overhaul History tab with GitKraken-style commit graph and unified activity#332

Merged
arul28 merged 12 commits into
mainfrom
cursor/history-tab-gitcraken-overhaul-9c6f
May 22, 2026
Merged

Overhaul History tab with GitKraken-style commit graph and unified activity#332
arul28 merged 12 commits into
mainfrom
cursor/history-tab-gitcraken-overhaul-9c6f

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented May 22, 2026

Summary

History tab overhaul plus CLI parity so agents and terminals can read the same data as the History pane.

Desktop (History tab)

  • Commits surface: DAG commit graph, virtualized list, branch ref labels, search, git context menu, commit detail with files + related ADE operations
  • Activity surface: existing operations timeline with cross-links to commits, PRs, missions, Work
  • Export button, lane header, author column, refresh, URL deep links (surface, laneId, commitSha, eventId)

CLI (ade history)

  • ade history list — operations timeline (same store as the pane)
  • ade history show --id <op> — single operation
  • ade history commits --lane <lane> — recent commits with parents
  • ade history export — JSON to stdout or --out (headless)

Demo (Linux cloud VM)

history-tab-demo.mp4

Recorded on the cloud VM: Vite renderer at localhost:5173/#/history (browser mock + live CLI). Full Electron shell is limited on this VM (no stable CDP); local macOS dev remains the reference for full desktop parity.

Example CLI

ade history list --text
ade history commits --lane <lane-id> --limit 30 --text
ade history show --id <operation-id> --text
ade history export --out ./history.json

Follow-ups

  • Lanes deep-link to auto-select commit in git pane
  • Richer multi-branch swimlanes for complex merges

To show artifacts inline, enable in settings.

Open in Web Open in Cursor 

ADE   Open in ADE  ·  cursor/history-tab-gitcraken-overhaul-9c6f branch  ·  PR #332

Summary by CodeRabbit

  • New Features
    • Interactive commit graph and detailed commit panel; commit search, pagination, and export with status filtering
    • Expanded Git actions: pull modes (ff-only, rebase, merge), create tag, reset to commit, undo/redo last head change, stash and remote sync helpers
    • New lane deep-linking to commits and explicit lane creation from a start commit
  • UI
    • Timeline columns configurable; duration shown by default; activity vs commits surfaces
  • Tests
    • Added broad test coverage for history UI, git operations, CLI, and IPC behaviors

Greptile Summary

This PR adds a GitKraken-style commit graph to the History tab, unifies activity and commit surfaces under a single timeline store, and adds ade history CLI subcommands for querying the operation timeline and recent commits.

  • Commits surface: virtualised DAG graph (commitGraphLayout.ts), branch ref labels, search, context-menu actions (create tag, reset, cherry-pick, undo/redo HEAD), and a commit detail panel with related ADE operations.
  • Activity surface: supplemental records (CTO sessions, missions, worker runs, chats) merged with DB operations, tight-polling kept separate from supplemental fetches.
  • CLI: ade history list/show/commits/export routed through the action registry; new git pull --mode, git undo, git redo CLI flags; operation.get direct-lookup added.

Confidence Score: 4/5

The new commit graph, CLI commands, and git action set are largely well-constructed, but several behavioural gaps from earlier review rounds remain open in the changed code.

The undo/redo HEAD-change chain, supplemental-record merging, and direct operation.get lookup are solid additions. Several concerns from prior threads remain open in the diff. Newly identified issues in historyGitActions.ts and HistoryPage.tsx erode correctness at the feature boundary but do not crash the app.

apps/desktop/src/renderer/components/history/historyGitActions.ts (create_lane baseBranch and compare_parent/view_files navigation), apps/desktop/src/renderer/components/history/HistoryPage.tsx (refreshToken coupling)

Important Files Changed

Filename Overview
apps/desktop/src/renderer/components/history/historyGitActions.ts New file: commit-level context menu actions (create tag, reset, cherry-pick, create lane). Two issues: compare_parent/view_files/open_lane_git share the same navigation URL as checkout (no distinct sub-view); create_lane uses the source lane's tracking branch as baseBranch for the new lane.
apps/desktop/src/renderer/components/history/HistoryPage.tsx Major rework: adds surface switching (commits/activity), URL deep-link sync, commit selection, auto-refresh split between tight polling and supplemental. refreshToken uses rawEvents.length which triggers unnecessary commit reloads on non-git events.
apps/desktop/src/renderer/components/history/historyLaneActions.ts New file: defines and executes 28 lane-scoped actions (fetch/pull/push, undo/redo HEAD, stash, conflict resolution, lane CRUD). Logic is straightforward; confirmOrCancel/promptConfirmOrCancel guards destructive actions. No critical issues found.
apps/desktop/src/renderer/components/history/useTimelineStore.ts Adds surface, focusLaneId, selectedCommit state; splits fetchEvents into main + supplemental paths with skipSupplemental fast-path for polling. The passesFilters lane-ID guard still drops null-laneId supplemental records when a lane filter is active (flagged in prior thread).
apps/desktop/src/renderer/components/history/CommitHistoryView.tsx New file: virtualised commit graph with SVG edge rendering, search, scroll-triggered pagination (120 to 500). Request sequencing via loadRequestSeq ref is correct. No critical issues found.
apps/desktop/src/renderer/components/history/historyActivitySources.ts New file: maps chat sessions, missions, CTO snapshots, and worker runs into OperationRecord-shaped objects for the unified timeline. Data transformation is defensive with null guards throughout.
apps/desktop/src/main/services/git/gitOperationsService.ts Adds createTag, resetToCommit, getCommit, pull modes, undoLastHeadChange, redoLastHeadChange. New functions validate inputs via array args (no injection risk). The listHeadChanges limit-100 window issue on busy lanes was flagged in a prior thread and is still present.
apps/desktop/src/main/services/history/operationService.ts Adds get (direct ID lookup), listHeadChanges (SQL-filtered by kind prefix and differing pre/post SHA), and status filter to list. SQL is correct; substr(o.kind,1,4) properly matches git_ prefix in SQLite's 1-indexed substr.
apps/desktop/src/main/services/lanes/laneService.ts Adds startPoint parameter to lane creation: when set, the start point is resolved via rev-parse --verify before creating the worktree, skipping the primary-lane fetch. Injection-safe (array args). No critical issues found.
apps/ade-cli/src/cli.ts Adds ade history list/show/commits/export and git pull --mode/--rebase/--merge, git undo/redo. history show now uses direct operation.get lookup (no 1000-record cap). Double-consume of --limit in history export flagged in prior thread.

Sequence Diagram

sequenceDiagram
    participant UI as HistoryPage (renderer)
    participant Store as useTimelineStore
    participant IPC as preload / IPC
    participant Git as gitOperationsService
    participant Op as operationService

    UI->>Store: "fetchEvents({ skipSupplemental })"
    Store->>IPC: "history.listOperations({ laneId, status, limit })"
    IPC->>Op: operationService.list(args)
    Op-->>IPC: OperationRecord[]
    IPC-->>Store: raw[]
    Store->>Store: merge supplemental + sort
    Store-->>UI: rawEvents / events

    UI->>IPC: "git.listRecentCommits({ laneId, limit })"
    IPC->>Git: gitService.listRecentCommits(args)
    Git-->>IPC: GitCommitSummary[]
    IPC-->>UI: commits (CommitHistoryView)

    UI->>IPC: "git.undoLastHeadChange({ laneId })"
    IPC->>Git: undoLastHeadChange(args)
    Git->>Op: "listHeadChanges({ laneId, limit:100 })"
    Op-->>Git: HeadChangeOperationRecord[]
    Git->>Git: "verify currentHead === operation.postHeadSha"
    Git->>Git: git reset --hard preHeadSha
    Git-->>IPC: GitActionResult
    IPC-->>UI: result
Loading

Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
apps/desktop/src/renderer/components/history/HistoryPage.tsx:385
The `refreshToken` prop is computed as `rawEvents.length + commitRefreshToken`. Every time any activity event arrives (including non-git supplemental records like chat sessions or CTO snapshots), this sum changes and `CommitHistoryView` re-runs `load()`, issuing a fresh `listRecentCommits` IPC call. During a polling cycle with running operations this happens every 4 s for all unrelated activity. Deriving the token only from `commitRefreshToken` (incremented on explicit git actions) keeps commit reloads intentional.

```suggestion
        refreshToken={commitRefreshToken}
```

### Issue 2 of 3
apps/desktop/src/renderer/components/history/historyGitActions.ts:405-409
**`compare_parent` and `view_files` navigate to the same URL as `checkout`**

All four cases — `checkout`, `open_lane_git`, `compare_parent`, and `view_files` — call `navigate(laneCommitDeepLink(laneId, commit.sha))`, which resolves to `/lanes?laneId=X&focus=single&commitSha=Y`. Because the deep-link carries no hint about which sub-view to open, clicking "Compare with parent" or "View changed files" lands the user in exactly the same place as "Inspect on lane", with no automatic diff or file-list panel opened.

### Issue 3 of 3
apps/desktop/src/renderer/components/history/historyGitActions.ts:339-361
**`create_lane` sets `baseBranch` to the current lane's own tracking branch**

`remote.branch` returned by `getOriginRemote({ laneId })` is the local branch name of the source lane (e.g. `feature/foo`). Passing it as `baseBranch` writes `feature/foo` into the new lane's `base_ref` column, so the new lane's "sync / rebase onto base" operations would target `feature/foo` rather than the project's integration branch (e.g. `main`). If the intended behaviour is a sub-branch fork, this is correct; if the intent is "create an independent lane at this commit", the `baseBranch` should be omitted or set to the parent lane's own `base_ref`.

Reviews (7): Last reviewed commit: "ship: address Copilot and Cursor review ..." | Re-trigger Greptile

@vercel
Copy link
Copy Markdown

vercel Bot commented May 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 22, 2026 8:34pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

📝 Walkthrough

Walkthrough

Adds mode-aware git operations (pull modes, createTag, resetToCommit), head-change undo/redo, IPC/preload typing and handlers, CLI history commands and export, lane startPoint support, and a new history UI (commit graph, commit/detail panels, actions, timeline store). Comprehensive tests across services, IPC, CLI, and renderer.

Changes

Unified Git + History feature

Layer / File(s) Summary
Feature surface and types / IPC / preload
apps/desktop/src/shared/types/git.ts, apps/desktop/src/shared/ipc.ts, apps/desktop/src/preload/*, apps/desktop/src/preload/global.d.ts
Adds GitPullMode, GitPullArgs, GitCreateTagArgs, GitResetCommitArgs, GitHeadChangeActionArgs; new IPC channels and preload/renderer bindings for createTag, resetToCommit, undoLastHeadChange, redoLastHeadChange; updates pull signatures.
Core git operations service
apps/desktop/src/main/services/git/gitOperationsService.ts, gitOperationsService.test.ts
Adds createTag, resetToCommit, mode-aware pull, undoLastHeadChange, redoLastHeadChange; tag name validation; head-change history helpers; listRecentCommits limit increased to 500. Tests for modes, undo/redo guards, and tag/reset behaviors.
IPC handler wiring (main)
apps/desktop/src/main/services/ipc/registerIpc.ts
Registers IPC handlers for new git actions and updates IPC.gitPull payload typing; history export list now conditionally includes status filter.
CLI & ADE RPC
apps/ade-cli/src/cli.ts, apps/ade-cli/src/adeRpcServer.ts, apps/ade-cli/src/services/sync/syncRemoteCommandService.ts, apps/ade-cli/src/cli.test.ts, adeRpcServer.test.ts
CLI: ade git pull supports mutually-exclusive modes, adds git undo/git redo; adds ade history subcommands (list/show/commits/export) with export persistence. ADE RPC: tool schemas accept pull mode; new RPC tools for undo/redo; sync service parses/validates remote command args. Tests updated/added.
Preload / renderer bridges & mocks
apps/desktop/src/preload/preload.ts, apps/desktop/src/renderer/browserMock.ts
Preload exposes new ade.git methods and normalizes argument shapes; browser mock stubs added methods for UI testing.
ADE actions & CTO tools
apps/desktop/src/main/services/adeActions/registry.ts, apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts, apps/desktop/src/main/services/cto/ctoStateService.ts
Expose new git actions in allowlist and operator tool contracts; update capability manifest.
History timeline, store, and search
apps/desktop/src/renderer/components/history/*
New commit graph layout and CommitHistoryView, CommitDetailPanel, HistoryGitContextMenu, history actions (commit- and lane-scoped), history activity sources aggregation, search/filtering utilities, timeline store surface/selection and supplemental record integration, timeline list/compact column configurability, TimelineToolbar export and lane actions. Many tests added.
Lane creation and deep-linking
apps/desktop/src/main/services/lanes/laneService.ts, laneService.test.ts, apps/desktop/src/renderer/components/lanes/LanesPage.tsx
Lane create accepts optional startPoint (validated via rev-parse) and uses it for worktree creation; LanesPage adds commitSha deep-link handling to select and highlight a specific commit. Tests added for startPoint.
Operation service history queries
apps/desktop/src/main/services/history/operationService.ts, operationService.test.ts
Adds get and listHeadChanges methods; list supports optional status filtering. Tests validate head-change listing.
ADE automation schemas
apps/desktop/src/renderer/components/automations/adeActionSchemas.ts
Adds schemas for createTag, resetToCommit, extends pull with mode, and adds undoLastHeadChange/redoLastHeadChange entries.

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • arul28/ADE#205: Deep-link handling in LanesPage; relates to this PR’s commitSha deep-link work.

Suggested labels

desktop

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/history-tab-gitcraken-overhaul-9c6f

@arul28 arul28 force-pushed the cursor/history-tab-gitcraken-overhaul-9c6f branch from 41e6fcc to 86cc82a Compare May 22, 2026 10:40
@arul28 arul28 marked this pull request as ready for review May 22, 2026 10:40
@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 22, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

PR Review

Scope: 46 file(s), +5886 / −212
Verdict: Needs changes

This PR overhauls the History tab with a commit graph, unified activity feed, lane/git context menus, and supporting main-process + CLI plumbing. The main blocker is URL/state handling when switching lanes on the Commits surface: a prior lane's commitSha can stick in the query string and get re-applied to the wrong lane.


🐛 Functionality

[High] Lane switch keeps stale commitSha in the URL

File: apps/desktop/src/renderer/components/history/HistoryPage.tsx:L133-L143, apps/desktop/src/renderer/components/history/HistoryPage.tsx:L208-L235, apps/desktop/src/renderer/components/history/useTimelineStore.ts:L294-L299
Issue: Changing the lane on the Commits surface clears selectedCommitSha in the store, but the URL writer only removes commitSha when surface === "activity". The hydrator then reads the old commitSha from the URL and re-selects it while laneId already points at the new lane, so the detail pane and selection can show a commit from the previous lane (or nothing useful if that SHA is absent locally).
Repro: Open Commits with ?surface=commits&laneId=<lane-A>&commitSha=<sha-on-A>. Use the lane dropdown to switch to lane B. Observe commitSha remains in the address bar and selection/detail try to use SHA from lane A on lane B.
Fix: When focusLaneId changes on the commits surface, delete commitSha from URL params (and clear selectedCommitSha / selectedCommit) in the same transaction—e.g. extend the URL-write effect to next.delete("commitSha") when the lane param changes, or clear commit selection inside setFocusLaneId.

[Medium] Git actions disabled after any load error

File: apps/desktop/src/renderer/components/history/CommitHistoryView.tsx:L316
Issue: hasWorktree is computed as Boolean(laneId) && !error, so any failed listRecentCommits / listBranches load (timeout, transient git error, etc.) marks the worktree missing and disables every context-menu git action with the "Lane worktree is missing" reason—even when the lane worktree is fine.
Repro: Provoke a transient git IPC failure (or temporarily break git log), reload Commits; after the error banner appears, right-click a row—the destructive/apply actions stay disabled with the missing-worktree message.
Fix: Pass a dedicated hasWorktree flag (e.g. from lane metadata or a narrow "worktree missing" error classifier) instead of coupling it to the generic error string.

[Medium] Deep-linked commit not found outside first 200 SHAs

File: apps/desktop/src/renderer/components/history/HistoryPage.tsx:L195-L201, apps/desktop/src/renderer/components/history/CommitDetailPanel.tsx:L68-L74
Issue: Resolving ?commitSha= only scans listRecentCommits({ limit: 200 }). Commits older than that window never hydrate into selectedCommit / resolvedCommit, so shared URLs and detail actions silently show an empty or partial commit even though the SHA is valid.
Repro: Copy a commit link for a SHA beyond the 200th commit on a long branch, paste into History, open Commits—the graph may not select it and the detail panel stays empty unless the user scrolls to load more and click manually.
Fix: On URL/commit selection, call a single-commit lookup (e.g. git show / existing getCommitMessage + metadata) when find on the recent list misses, or raise the initial fetch limit when commitSha is present.


⚡ Performance

[Medium] Commits surface still loads full activity feed

File: apps/desktop/src/renderer/components/history/HistoryPage.tsx:L168-L171, apps/desktop/src/renderer/components/history/useTimelineStore.ts:L374-L388
Issue: fetchEvents runs whenever History is active, including on surface === "commits" (silent mode). That still pulls up to 500 operations plus supplemental chats/missions/CTO/worker runs on every tab visit and lane/surface change, even though the Commits UI does not render them.
Impact: Several IPC round-trips and JSON enrichment on each History open while the user only wanted git log; scales with project activity volume.
Fix: Skip fetchEvents (and fetchSupplementalTimelineRecords) when surface === "commits", or gate supplemental fetch behind the Activity surface only.

[Medium] Copy patch fans out unbounded per-file diff IPC

File: apps/desktop/src/renderer/components/history/historyGitActions.ts:L114-L126
Issue: copyCommitPatch loads every changed file for the commit and issues one getFilePatch per path in parallel via Promise.allSettled, with no cap on file count.
Impact: Large commits (hundreds/thousands of files) can spawn hundreds of concurrent IPC/git processes and freeze the renderer for seconds.
Fix: Cap files (e.g. first 50), batch sequentially, or stream a single git show patch from main instead of N per-file calls.


🎨 UI/UX

[Low] Icon-only refresh control lacks accessible name

File: apps/desktop/src/renderer/components/history/CommitHistoryView.tsx:L191-L202
Issue: The refresh control is a 28×28px icon button with title only; screen readers get no accessible name and the control fails the icon-button guidance in the review rubric.
Fix: Add aria-label="Refresh commits" (keep title if desired).

[Low] Activity view-mode toggles lack accessible names

File: apps/desktop/src/renderer/components/history/TimelineToolbar.tsx:L237-L251
Issue: Graph/List/Compact switches are icon-only buttons sized 28×28px with title tooltips but no aria-label, so assistive tech users cannot discern the active view mode.
Fix: Add aria-label per mode (e.g. aria-label="Graph view") and aria-pressed={viewMode === mode}.

[Low] Commits lane <select> has no associated label

File: apps/desktop/src/renderer/components/history/TimelineToolbar.tsx:L211-L221
Issue: The lane picker is a bare <select> on the Commits toolbar row without <label htmlFor=…> or aria-label, so it is unnamed in accessibility trees.
Fix: Add a visually hidden <label htmlFor="history-lane-select">Lane</label> and matching id, or set aria-label="Lane" on the select.


Notes

  • Undo/redo head changes are guarded by recorded preHeadSha/postHeadSha checks before git reset --hard, which limits accidental rewinds when local HEAD diverged.
  • Commit graph virtualization and lane read caching (readLaneCached) are sensible performance choices.
  • New sync RPC entries for undo/redo follow the existing viewerAllowed: true git-command pattern used elsewhere in syncRemoteCommandService.ts; worth a separate hardening pass if mobile viewers should be read-only for destructive git ops.
Open in Web View Automation 

Sent by Cursor Automation: BUGBOT in Versic

Comment thread apps/desktop/src/renderer/components/history/commitGraphLayout.ts
Comment thread apps/desktop/src/renderer/components/history/commitGraphLayout.ts
Comment thread apps/desktop/src/renderer/components/history/historySearch.ts Outdated
Comment thread apps/desktop/src/renderer/components/history/useTimelineStore.ts
Comment thread apps/ade-cli/src/cli.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/history/TimelineListView.tsx (1)

140-145: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add an explicit accessible name to timeline row buttons.

With column toggles, a row can become effectively unlabeled (or label-hidden on smaller breakpoints) for screen readers. Add a stable aria-label so selection remains accessible regardless of visible columns.

Suggested fix
               <button
                 key={ev.id}
                 type="button"
                 data-tour="history.entry"
                 onClick={() => onSelectEvent(ev.id)}
+                aria-label={`${ev.label} (${ev.kind})`}
                 className={cn(

Also applies to: 174-191, 226-230

🤖 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 `@apps/desktop/src/renderer/components/history/TimelineListView.tsx` around
lines 140 - 145, The timeline row buttons (the button with key={ev.id} that
calls onSelectEvent(ev.id) and the similar buttons around the other entries)
lack an explicit accessible name; add an aria-label to each row button so screen
readers always have a stable name (e.g., aria-label derived from the event title
with a fallback to the id such as "Select timeline event {ev.title || ev.id}").
Update every similar button instance in TimelineListView.tsx (the button using
onSelectEvent(ev.id) and the other row buttons referenced in the review) to
include this aria-label.
🧹 Nitpick comments (5)
apps/desktop/src/renderer/components/history/commitGraphLayout.test.ts (1)

38-45: ⚡ Quick win

Add a regression assertion for branch lane separation.

This test only checks edge count. It should also assert that merge parents are in distinct columns, otherwise a collapsed graph can still pass.

🤖 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 `@apps/desktop/src/renderer/components/history/commitGraphLayout.test.ts`
around lines 38 - 45, The test currently only checks edge count for the merge
commit but must also assert that the merge parents occupy distinct lanes/columns
to prevent collapsed layouts; after calling buildCommitGraphLayout([merge, main,
feature]) inspect layout.nodes to locate the nodes for "main" and "feature" (by
their sha) and add an assertion that their column/lane property (e.g.,
node.column or node.lane) are not equal, ensuring the parents are placed in
separate columns; keep the existing mergeEdges check and add this new inequality
assertion referencing commit("m"), buildCommitGraphLayout, layout.edges, and
layout.nodes.
apps/desktop/src/renderer/components/history/historyLaneActions.ts (1)

50-95: ⚡ Quick win

Rename LANE_ACTION_GROUPS to camelCase.

This constant is in changed TypeScript code, and the repo rule for TS/JS files asks for camelCase variable names.

As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript files.

🤖 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 `@apps/desktop/src/renderer/components/history/historyLaneActions.ts` around
lines 50 - 95, Rename the constant LANE_ACTION_GROUPS to camelCase (e.g.,
laneActionGroups) and update all references in this file to use the new
identifier; ensure the exported name (if any) and any imports/uses (components,
functions, or tests that reference LANE_ACTION_GROUPS) are updated too so there
are no unresolved identifiers, and keep the value and type annotation unchanged
(the array shape and HistoryLaneActionId references stay the same).
apps/desktop/src/renderer/components/history/historyGitActions.ts (1)

36-61: ⚡ Quick win

Rename COMMIT_ACTION_GROUPS to camelCase.

This constant is in changed TypeScript code, and the repo rule for TS/JS files asks for camelCase variable names.

As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript files.

🤖 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 `@apps/desktop/src/renderer/components/history/historyGitActions.ts` around
lines 36 - 61, Rename the top-level constant COMMIT_ACTION_GROUPS to camelCase
(commitActionGroups) and update every reference to it across the codebase
(including imports/exports and usages in historyGitActions.ts and any consumers)
so identifiers match; preserve the const declaration shape and its type
annotation Array<{ id: string; label: string; actionIds: HistoryGitActionId[]
}>, and run/typecheck to catch missed references (search for
COMMIT_ACTION_GROUPS and replace with commitActionGroups, and update any named
exports/imports accordingly).
apps/desktop/src/renderer/components/automations/adeActionSchemas.ts (1)

344-354: 💤 Low value

Consider using enum type for mode parameters.

Both resetToCommit.mode and pull.mode use type: "string" with descriptions listing valid values. For consistency with other enum-like parameters in this file (e.g., conflicts.rebaseLane.provider, pr.submitReview.event) and to enable better form validation in the automation rule editor, consider using type: "enum" with enumValues.

♻️ Suggested improvement
   {
     domain: "git",
     action: "resetToCommit",
     label: "Reset to commit",
     description: "Move the lane branch to a commit using soft, mixed, or hard reset.",
     params: [
       LANE_ID_PARAM,
       COMMIT_SHA_PARAM,
-      { name: "mode", type: "string", required: true, description: "Reset mode: soft, mixed, or hard." },
+      { name: "mode", type: "enum", required: true, enumValues: ["soft", "mixed", "hard"], description: "Reset mode." },
     ],
   },
   {
     domain: "git",
     action: "pull",
     label: "Pull from remote",
     description: "Pull the lane's branch from its upstream tracking branch.",
     params: [
       LANE_ID_PARAM,
-      { name: "mode", type: "string", description: "Pull mode: ff-only, rebase, or merge." },
+      { name: "mode", type: "enum", enumValues: ["ff-only", "rebase", "merge"], description: "Pull mode." },
     ],
   },

Also applies to: 367-371

🤖 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 `@apps/desktop/src/renderer/components/automations/adeActionSchemas.ts` around
lines 344 - 354, The mode params for the git actions should be changed from
type: "string" to type: "enum" and supply an enumValues array matching the
documented valid options; specifically update the param object in the git action
with domain:"git", action:"resetToCommit" (the "mode" param alongside
LANE_ID_PARAM and COMMIT_SHA_PARAM) to type: "enum" and add enumValues:
["soft","mixed","hard"], and likewise update the git action with domain:"git",
action:"pull" to use type: "enum" with enumValues that match its description
(e.g., ["rebase","merge","ff-only"] or the exact valid values used elsewhere in
the codebase) so the automation editor can validate choices.
apps/desktop/src/renderer/components/history/HistoryPage.tsx (1)

310-314: ⚡ Quick win

Memoize laneData to prevent unnecessary re-renders.

This creates a new array with new objects on every render. Since laneData is passed to TimelineGraph, it will cause the child component to re-render even when the underlying lanes data hasn't changed.

♻️ Suggested fix
-  const laneData = lanes.map((l) => ({
-    id: l.id,
-    name: l.name,
-    color: l.color ?? null,
-  }));
+  const laneData = useMemo(
+    () =>
+      lanes.map((l) => ({
+        id: l.id,
+        name: l.name,
+        color: l.color ?? null,
+      })),
+    [lanes],
+  );
🤖 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 `@apps/desktop/src/renderer/components/history/HistoryPage.tsx` around lines
310 - 314, The laneData array is recreated on every render causing TimelineGraph
to re-render; wrap the laneData computation in React.useMemo inside the
HistoryPage component so it only rebuilds when the lanes input changes
(useMemo(() => lanes.map(...), [lanes])), keeping the same id/name/color shape
and using lanes as the dependency to prevent unnecessary child re-renders.
🤖 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 `@apps/ade-cli/src/cli.ts`:
- Around line 5147-5149: The history show flow calls
actionStep("result","operation","list",{limit: readIntOption(args, ["--limit"],
1000)}) and only scans the most recent 1000 operations so older operation IDs
can return “not found”; change this to perform a proper id-based lookup or a
paginated search: either call a dedicated lookup endpoint if available (e.g.,
operation.getById or similar) or iterate operation.list with a reasonable page
size using the returned cursor/next token until the requested id is found or the
pages are exhausted, and remove the hard-coded 1000 window (also surface a
user-facing error if exhaustive search fails). Ensure you update the code paths
in the history show logic that resolve IDs so they use this paginated/search
approach instead of the fixed limit.
- Around line 13395-13425: The history status filter is being applied after the
fetch/limit step so `history list/export` uses an already-capped window (rows)
and drops matching items outside that window; change the logic so the status
predicate participates in pagination: either pass plan.historyStatusFilter down
into the server query that produces the raw rows (so the server applies the
status before limiting), or if server-side change isn’t possible, apply the
status filter immediately after asOperationRows(raw) and before any
limit/pagination, i.e. call filterHistoryOperations(asOperationRows(raw),
plan.historyStatusFilter) earlier (or re-run pagination on the filtered array)
so variables like rows, rowCount and the exported rows reflect the
status-filtered set for both "history list" and "history export".
- Around line 2884-2895: The code currently allows mixing --mode with flag forms
and silently prefers flags (see explicitMode, flagModes, rawMode, mode), so add
a validation that if explicitMode is set (non-empty) and flagModes.length > 0
you throw a CliUsageError refusing mixed inputs; keep the existing single-flag
check for flagModes and the ff_only -> ff-only mapping, but perform the new
explicitMode+flagModes conflict check before deriving rawMode/mode and before
normalizing/validating the final mode.

In `@apps/desktop/src/main/services/sync/syncRemoteCommandService.test.ts`:
- Around line 1219-1227: Add the missing negative assertion for redo lane
validation: after the existing undo rejection check, call
service.execute(makePayload("git.redoLastHeadChange", {})) and assert it rejects
with the same error message ("git.redoLastHeadChange requires laneId."); this
mirrors the undo test and ensures git.redoLastHeadChange's laneId validation is
covered for service.execute and makePayload interactions with
git.redoLastHeadChange.

In `@apps/desktop/src/renderer/components/history/CommitDetailPanel.tsx`:
- Around line 100-127: The effect that fetches commit data (useEffect watching
resolvedCommit/laneId) can flash the previous commit's UI; to fix, immediately
clear commit-scoped state at the start of that effect by calling
setFullMessage(null), setFiles([]) and setLoadingFiles(true) (or appropriate
initial values) before invoking window.ade.git.getCommitMessage and
window.ade.git.listCommitFiles, keep the existing cancelled flag and cleanup
logic around the async .then/.catch/.finally handlers on getCommitMessage and
listCommitFiles to avoid updates after unmount or new selection; ensure you
still setLoadingFiles(false) in the finally only when not cancelled.

In `@apps/desktop/src/renderer/components/history/commitGraphLayout.ts`:
- Around line 76-79: Single-parent commits currently always inherit their
parent's column (shaToCol/parentCol), which collapses forks; change the logic in
commitGraphLayout.ts so that when parents.length === 1 you check whether that
parent has multiple children (i.e., out-degree > 1) and only reuse parentCol
when the parent has a single child; if the parent is forked, call allocCol() to
allocate a new lane for this sibling instead. Locate the block using shaToCol,
parents, parentCol and ensure you use the existing commit graph/children map (or
build a temporary children count) to detect multi-child parents before deciding
between parentCol and allocCol.

In `@apps/desktop/src/renderer/components/history/CommitHistoryView.tsx`:
- Around line 111-114: The current search only filters the locally loaded
commits array (commits) via filterCommitsForSearch, so when filtered becomes
empty the UI shows EmptyState and stops pagination, making matches beyond the
current page undiscoverable; fix by making the search operate against the full
bounded history (or trigger loading more pages while search is active) before
deciding there are no matches — update the CommitHistoryView logic around the
useMemo that computes filtered (and the EmptyState check) to either (a) call a
variant like filterCommitsForSearchOnFullHistory that uses the complete
backend/boundedHistory source instead of just commits, or (b) kick off loading
additional pages when search is non-empty and filtered.length === 0, then only
show EmptyState after the full-bounded-history search completes; ensure you
reference and update the same symbols (filterCommitsForSearch, commits,
refsBySha, search, filtered, EmptyState) so pagination is not aborted
prematurely.
- Around line 167-176: The current early return when error is truthy in
CommitHistoryView hides the header/toolbar (removing the refresh affordance);
instead of returning early, keep rendering the normal layout (header/toolbar)
and render the error banner inline inside that layout (e.g., place the existing
error JSX block below the header/toolbar and above the commit list) so the
toolbar remains visible and users can retry; modify the conditional from an
early return to a conditional insertion of the banner using the existing error
variable inside the CommitHistoryView render.

In `@apps/desktop/src/renderer/components/history/historyActivitySources.ts`:
- Around line 316-326: The current checks dereference window.ade before optional
chaining and can throw when window.ade is undefined; update each capability
check (e.g., window.ade.agentChat?.list, window.ade.missions?.list,
window.ade.cto?.getState, window.ade.cto?.listAgentRuns) to first guard the
parent object (either use window.ade && typeof window.ade.agentChat?.list ===
"function" or typeof window.ade?.agentChat?.list === "function") so you never
access properties on undefined, then call settle(...) or Promise.resolve(null)
as before, preserving the safeLimit usage for the missions/cto calls.

In `@apps/desktop/src/renderer/components/history/historyLaneActions.ts`:
- Around line 607-625: The four IPC git calls in the dispatcher call the bridge
with a raw laneId string; update the calls to pass an object payload { laneId }
instead: change window.ade.git.rebaseContinue(laneId), rebaseAbort(laneId),
mergeContinue(laneId), and mergeAbort(laneId) to use
window.ade.git.rebaseContinue({ laneId }), window.ade.git.rebaseAbort({ laneId
}), window.ade.git.mergeContinue({ laneId }), and window.ade.git.mergeAbort({
laneId }) respectively so they match the rest of the file's IPC contract.

In `@apps/desktop/src/renderer/components/history/TimelineToolbar.tsx`:
- Around line 528-549: The run callback currently awaits runHistoryLaneAction
and then calls setRunningAction(null) only on success; wrap the await in a
try/catch/finally inside the run function (the function declared as run =
useCallback(...)) so that any thrown error is caught (call setError with the
error message or a fallback) and setRunningAction(null) is always called in the
finally block; keep existing onError/onComplete/onNotice props when calling
runHistoryLaneAction and do not remove the navigate handler.

---

Outside diff comments:
In `@apps/desktop/src/renderer/components/history/TimelineListView.tsx`:
- Around line 140-145: The timeline row buttons (the button with key={ev.id}
that calls onSelectEvent(ev.id) and the similar buttons around the other
entries) lack an explicit accessible name; add an aria-label to each row button
so screen readers always have a stable name (e.g., aria-label derived from the
event title with a fallback to the id such as "Select timeline event {ev.title
|| ev.id}"). Update every similar button instance in TimelineListView.tsx (the
button using onSelectEvent(ev.id) and the other row buttons referenced in the
review) to include this aria-label.

---

Nitpick comments:
In `@apps/desktop/src/renderer/components/automations/adeActionSchemas.ts`:
- Around line 344-354: The mode params for the git actions should be changed
from type: "string" to type: "enum" and supply an enumValues array matching the
documented valid options; specifically update the param object in the git action
with domain:"git", action:"resetToCommit" (the "mode" param alongside
LANE_ID_PARAM and COMMIT_SHA_PARAM) to type: "enum" and add enumValues:
["soft","mixed","hard"], and likewise update the git action with domain:"git",
action:"pull" to use type: "enum" with enumValues that match its description
(e.g., ["rebase","merge","ff-only"] or the exact valid values used elsewhere in
the codebase) so the automation editor can validate choices.

In `@apps/desktop/src/renderer/components/history/commitGraphLayout.test.ts`:
- Around line 38-45: The test currently only checks edge count for the merge
commit but must also assert that the merge parents occupy distinct lanes/columns
to prevent collapsed layouts; after calling buildCommitGraphLayout([merge, main,
feature]) inspect layout.nodes to locate the nodes for "main" and "feature" (by
their sha) and add an assertion that their column/lane property (e.g.,
node.column or node.lane) are not equal, ensuring the parents are placed in
separate columns; keep the existing mergeEdges check and add this new inequality
assertion referencing commit("m"), buildCommitGraphLayout, layout.edges, and
layout.nodes.

In `@apps/desktop/src/renderer/components/history/historyGitActions.ts`:
- Around line 36-61: Rename the top-level constant COMMIT_ACTION_GROUPS to
camelCase (commitActionGroups) and update every reference to it across the
codebase (including imports/exports and usages in historyGitActions.ts and any
consumers) so identifiers match; preserve the const declaration shape and its
type annotation Array<{ id: string; label: string; actionIds:
HistoryGitActionId[] }>, and run/typecheck to catch missed references (search
for COMMIT_ACTION_GROUPS and replace with commitActionGroups, and update any
named exports/imports accordingly).

In `@apps/desktop/src/renderer/components/history/historyLaneActions.ts`:
- Around line 50-95: Rename the constant LANE_ACTION_GROUPS to camelCase (e.g.,
laneActionGroups) and update all references in this file to use the new
identifier; ensure the exported name (if any) and any imports/uses (components,
functions, or tests that reference LANE_ACTION_GROUPS) are updated too so there
are no unresolved identifiers, and keep the value and type annotation unchanged
(the array shape and HistoryLaneActionId references stay the same).

In `@apps/desktop/src/renderer/components/history/HistoryPage.tsx`:
- Around line 310-314: The laneData array is recreated on every render causing
TimelineGraph to re-render; wrap the laneData computation in React.useMemo
inside the HistoryPage component so it only rebuilds when the lanes input
changes (useMemo(() => lanes.map(...), [lanes])), keeping the same id/name/color
shape and using lanes as the dependency to prevent unnecessary child re-renders.
🪄 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: 2eb41010-8289-4289-9c1a-412354d41d6d

📥 Commits

Reviewing files that changed from the base of the PR and between c08e86e and 86cc82a.

📒 Files selected for processing (46)
  • apps/ade-cli/src/adeRpcServer.test.ts
  • apps/ade-cli/src/adeRpcServer.ts
  • apps/ade-cli/src/cli.test.ts
  • apps/ade-cli/src/cli.ts
  • apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
  • apps/desktop/src/main/services/adeActions/registry.ts
  • apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts
  • apps/desktop/src/main/services/cto/ctoStateService.ts
  • apps/desktop/src/main/services/git/gitOperationsService.test.ts
  • apps/desktop/src/main/services/git/gitOperationsService.ts
  • apps/desktop/src/main/services/ipc/registerIpc.ts
  • apps/desktop/src/main/services/lanes/laneService.test.ts
  • apps/desktop/src/main/services/lanes/laneService.ts
  • apps/desktop/src/main/services/sync/syncRemoteCommandService.test.ts
  • apps/desktop/src/preload/global.d.ts
  • apps/desktop/src/preload/preload.ts
  • apps/desktop/src/renderer/browserMock.ts
  • apps/desktop/src/renderer/components/automations/adeActionSchemas.ts
  • apps/desktop/src/renderer/components/history/CommitDetailPanel.tsx
  • apps/desktop/src/renderer/components/history/CommitHistoryView.tsx
  • apps/desktop/src/renderer/components/history/EventDetailPanel.tsx
  • apps/desktop/src/renderer/components/history/HistoryGitContextMenu.tsx
  • apps/desktop/src/renderer/components/history/HistoryPage.tsx
  • apps/desktop/src/renderer/components/history/TimelineCompactView.tsx
  • apps/desktop/src/renderer/components/history/TimelineListView.tsx
  • apps/desktop/src/renderer/components/history/TimelineToolbar.tsx
  • apps/desktop/src/renderer/components/history/commitGraphLayout.test.ts
  • apps/desktop/src/renderer/components/history/commitGraphLayout.ts
  • apps/desktop/src/renderer/components/history/eventTaxonomy.test.ts
  • apps/desktop/src/renderer/components/history/eventTaxonomy.ts
  • apps/desktop/src/renderer/components/history/historyActivitySources.test.ts
  • apps/desktop/src/renderer/components/history/historyActivitySources.ts
  • apps/desktop/src/renderer/components/history/historyGitActions.test.ts
  • apps/desktop/src/renderer/components/history/historyGitActions.ts
  • apps/desktop/src/renderer/components/history/historyLaneActions.test.ts
  • apps/desktop/src/renderer/components/history/historyLaneActions.ts
  • apps/desktop/src/renderer/components/history/historySearch.test.ts
  • apps/desktop/src/renderer/components/history/historySearch.ts
  • apps/desktop/src/renderer/components/history/timelineTypes.ts
  • apps/desktop/src/renderer/components/history/useTimelineStore.test.ts
  • apps/desktop/src/renderer/components/history/useTimelineStore.ts
  • apps/desktop/src/renderer/components/lanes/LanesPage.tsx
  • apps/desktop/src/shared/ipc.ts
  • apps/desktop/src/shared/types/git.ts
  • apps/desktop/src/shared/types/lanes.ts
  • apps/desktop/src/shared/types/sync.ts

Comment thread apps/ade-cli/src/cli.ts
Comment thread apps/ade-cli/src/cli.ts Outdated
Comment thread apps/ade-cli/src/cli.ts Outdated
Comment thread apps/desktop/src/renderer/components/history/CommitDetailPanel.tsx
Comment thread apps/desktop/src/renderer/components/history/CommitHistoryView.tsx Outdated
Comment thread apps/desktop/src/renderer/components/history/historyActivitySources.ts Outdated
Comment thread apps/desktop/src/renderer/components/history/historyLaneActions.ts Outdated
Comment thread apps/desktop/src/renderer/components/history/historySearch.ts
Comment thread apps/desktop/src/renderer/components/history/TimelineToolbar.tsx
Comment thread apps/desktop/src/main/services/git/gitOperationsService.ts Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
apps/desktop/src/main/services/git/gitOperationsService.ts (1)

1298-1306: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard undo/redo against dirty worktrees.

Both paths call git reset --hard after only checking HEAD, so any staged or unstaged edits on the current HEAD get discarded. Reject the action when the lane has uncommitted changes before resetting.

Proposed fix
         fn: async (lane) => {
+          if (await isWorktreeDirty(lane.worktreePath)) {
+            throw new Error("Cannot undo while the lane has uncommitted changes. Commit or stash them first.");
+          }
           const currentHead = await getHeadSha(lane.worktreePath);
           if (currentHead !== operation.postHeadSha) {
             throw new Error("Cannot undo because the lane head has changed since that operation.");
           }
           await runGitOrThrow(["reset", "--hard", operation.preHeadSha], {
@@
         fn: async (lane) => {
+          if (await isWorktreeDirty(lane.worktreePath)) {
+            throw new Error("Cannot redo while the lane has uncommitted changes. Commit or stash them first.");
+          }
           const currentHead = await getHeadSha(lane.worktreePath);
           if (currentHead !== operation.postHeadSha) {
             throw new Error("Cannot redo because the lane head has changed since the undo.");
           }
           await runGitOrThrow(["reset", "--hard", redoHeadSha], {

Also applies to: 1322-1330

apps/desktop/src/renderer/components/history/HistoryPage.tsx (2)

255-298: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep selectedCommitSha in sync with commit detail state.

These handlers clear selectedCommit, but they never clear the backing selectedCommitSha and they do not set it eagerly on commit selection. On the commits surface, the URL sync effect can write the old commitSha straight back, and the resolver effect can hydrate the previous commit again after close or event selection.

Proposed fix
   const handleSelectEvent = useCallback(
     (id: string) => {
       setSelectedEventId(id);
       setSelectedCommit(null);
+      setSelectedCommitSha(null);
       setSearchParams((prev) => {
         const next = new URLSearchParams(prev);
         next.set("eventId", id);
         next.delete("commitSha");
         next.set("surface", "activity");
         lastWrittenUrlRef.current = next.toString();
         return next;
       });
     },
-    [setSelectedEventId, setSelectedCommit, setSearchParams],
+    [setSelectedEventId, setSelectedCommit, setSelectedCommitSha, setSearchParams],
   );

   const handleSelectCommit = useCallback(
     (commit: GitCommitSummary) => {
       setSelectedCommit(commit);
+      setSelectedCommitSha(commit.sha);
       setSelectedEventId(null);
       setSearchParams((prev) => {
         const next = new URLSearchParams(prev);
         next.set("commitSha", commit.sha);
         next.delete("eventId");
         next.set("surface", "commits");
         if (focusLaneId) next.set("laneId", focusLaneId);
         lastWrittenUrlRef.current = next.toString();
         return next;
       });
     },
-    [setSelectedCommit, setSelectedEventId, setSearchParams, focusLaneId],
+    [setSelectedCommit, setSelectedCommitSha, setSelectedEventId, setSearchParams, focusLaneId],
   );

   const handleCloseDetail = useCallback(() => {
     setSelectedEventId(null);
     setSelectedCommit(null);
+    setSelectedCommitSha(null);
     setSearchParams((prev) => {
       const next = new URLSearchParams(prev);
       next.delete("eventId");
       next.delete("commitSha");
       lastWrittenUrlRef.current = next.toString();
       return next;
     });
-  }, [setSelectedEventId, setSelectedCommit, setSearchParams]);
+  }, [setSelectedEventId, setSelectedCommit, setSelectedCommitSha, setSearchParams]);
🤖 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 `@apps/desktop/src/renderer/components/history/HistoryPage.tsx` around lines
255 - 298, The handlers manipulate selectedCommit and URL but neglect the
backing selectedCommitSha; update handleSelectCommit to call
setSelectedCommitSha(commit.sha) when selecting a commit, and update
handleSelectEvent and handleCloseDetail to call setSelectedCommitSha(null) when
clearing selection; also add setSelectedCommitSha to the useCallback dependency
arrays for handleSelectEvent, handleSelectCommit, and handleCloseDetail so the
closures stay correct.

190-201: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear stale commit details when the SHA lookup misses.

If a deep link points to a commit outside the 500-row window, or just an invalid SHA, found stays undefined and the previous selectedCommit remains mounted. That leaves the detail panel and commit actions targeting the wrong commit.

Proposed fix
     void window.ade.git
       .listRecentCommits({ laneId: focusLaneId, limit: 500 })
       .then((rows) => {
         if (cancelled) return;
-        const found = rows.find((r) => r.sha === selectedCommitSha);
-        if (found) setSelectedCommit(found);
+        const found = rows.find((r) => r.sha === selectedCommitSha) ?? null;
+        setSelectedCommit(found);
       })
-      .catch(() => {});
+      .catch(() => {
+        if (!cancelled) setSelectedCommit(null);
+      });
🤖 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 `@apps/desktop/src/renderer/components/history/HistoryPage.tsx` around lines
190 - 201, The effect that looks up a commit by selectedCommitSha via
window.ade.git.listRecentCommits may leave a stale selectedCommit when the SHA
is not found; update the handler in the useEffect (the block using active,
focusLaneId, selectedCommitSha, selectedCommit, setSelectedCommit and
listRecentCommits) so that when rows.find(...) returns undefined you explicitly
clear the selection (e.g., setSelectedCommit(null) or undefined) instead of
leaving the previous commit, and keep the cancelled check; also ensure any
promise rejection is handled to avoid leaving stale state on error.
apps/desktop/src/renderer/components/history/useTimelineStore.ts (1)

382-392: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Apply the lane filter before limiting supplemental history.

Line 388 caps supplemental records globally, and Line 391 filters by opts.laneId afterward. If supplemental history exceeds limit, valid records for the selected lane can be dropped before the lane filter runs, so the lane view becomes incomplete. Please push lane scoping into fetchSupplementalTimelineRecords (or over-fetch before truncating) so each lane gets its own top-limit slice.

🤖 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 `@apps/desktop/src/renderer/components/history/useTimelineStore.ts` around
lines 382 - 392, The supplemental records are being globally limited before
lane-scoping, which can drop valid per-lane items; update the call to
fetchSupplementalTimelineRecords so lane scoping happens inside that helper
(e.g., add a laneId parameter or options object:
fetchSupplementalTimelineRecords(limit, opts?.laneId) or
fetchSupplementalTimelineRecords({limit, laneId: opts?.laneId})), remove the
later per-lane filter on scopedSupplemental, and ensure
fetchSupplementalTimelineRecords returns the top `limit` records already
filtered by laneId; alternatively, if you prefer not to change the helper
signature, over-fetch (e.g., fetch more than `limit`) and then apply the lane
filter before truncating to `limit` in this code path.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/components/history/historySearch.ts (1)

57-60: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Support quoted shorthand filters in the tokenizer.

searchTokenFromRaw() now handles @, #, and =, but the regex still splits @"Jane Doe" / ="fix parser" into multiple tokens. Those searches miss the keyed path and regress back to loose free-text matching.

Proposed fix
 function tokenizeSearch(query: string): SearchToken[] {
-  const matches = query.match(/[^\s"]+:"[^"]+"|"[^"]+"|\S+/g) ?? [];
+  const matches = query.match(/[^\s"]+:"[^"]+"|[@#=]"[^"]+"|"[^"]+"|\S+/g) ?? [];
   return matches
     .map(searchTokenFromRaw)
     .filter((token): token is SearchToken => token != null);
 }
🤖 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 `@apps/desktop/src/renderer/components/history/historySearch.ts` around lines
57 - 60, The tokenizer regex in tokenizeSearch still splits shorthand quoted
filters like @"Jane Doe" or ="fix parser" into multiple tokens; update the regex
so it treats an optional shorthand symbol (@, #, =) directly attached to a
quoted phrase or to a key:value pair as a single token. Specifically, change the
match pattern used in tokenizeSearch to include alternatives such as
[@#=]?[^\s"]+:"[^"]+" and [@#=]"[^"]+" (in addition to the existing quoted and
non-space alternatives) so searchTokenFromRaw receives @"...", ="..." or
key:"..." as one token.
🤖 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 `@apps/desktop/src/preload/preload.ts`:
- Around line 6830-6831: The handler rebaseContinue (and other IPC handlers that
extract laneId) currently assumes args is non-null and can be string or object,
which will crash when args === null; add a runtime guard that first checks args
!== null/undefined and then if typeof args === "string" use it, else if typeof
args === "object" && typeof args.laneId === "string" use args.laneId, otherwise
return a safe failure GitActionResult (or throw a controlled IPC error) so
malformed inputs don't cause runtime exceptions when extracting laneId.

---

Outside diff comments:
In `@apps/desktop/src/renderer/components/history/HistoryPage.tsx`:
- Around line 255-298: The handlers manipulate selectedCommit and URL but
neglect the backing selectedCommitSha; update handleSelectCommit to call
setSelectedCommitSha(commit.sha) when selecting a commit, and update
handleSelectEvent and handleCloseDetail to call setSelectedCommitSha(null) when
clearing selection; also add setSelectedCommitSha to the useCallback dependency
arrays for handleSelectEvent, handleSelectCommit, and handleCloseDetail so the
closures stay correct.
- Around line 190-201: The effect that looks up a commit by selectedCommitSha
via window.ade.git.listRecentCommits may leave a stale selectedCommit when the
SHA is not found; update the handler in the useEffect (the block using active,
focusLaneId, selectedCommitSha, selectedCommit, setSelectedCommit and
listRecentCommits) so that when rows.find(...) returns undefined you explicitly
clear the selection (e.g., setSelectedCommit(null) or undefined) instead of
leaving the previous commit, and keep the cancelled check; also ensure any
promise rejection is handled to avoid leaving stale state on error.

In `@apps/desktop/src/renderer/components/history/useTimelineStore.ts`:
- Around line 382-392: The supplemental records are being globally limited
before lane-scoping, which can drop valid per-lane items; update the call to
fetchSupplementalTimelineRecords so lane scoping happens inside that helper
(e.g., add a laneId parameter or options object:
fetchSupplementalTimelineRecords(limit, opts?.laneId) or
fetchSupplementalTimelineRecords({limit, laneId: opts?.laneId})), remove the
later per-lane filter on scopedSupplemental, and ensure
fetchSupplementalTimelineRecords returns the top `limit` records already
filtered by laneId; alternatively, if you prefer not to change the helper
signature, over-fetch (e.g., fetch more than `limit`) and then apply the lane
filter before truncating to `limit` in this code path.

---

Duplicate comments:
In `@apps/desktop/src/renderer/components/history/historySearch.ts`:
- Around line 57-60: The tokenizer regex in tokenizeSearch still splits
shorthand quoted filters like @"Jane Doe" or ="fix parser" into multiple tokens;
update the regex so it treats an optional shorthand symbol (@, #, =) directly
attached to a quoted phrase or to a key:value pair as a single token.
Specifically, change the match pattern used in tokenizeSearch to include
alternatives such as [@#=]?[^\s"]+:"[^"]+" and [@#=]"[^"]+" (in addition to the
existing quoted and non-space alternatives) so searchTokenFromRaw receives
@"...", ="..." or key:"..." as one token.
🪄 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: bada1424-dc39-44a9-87eb-bd1da1986544

📥 Commits

Reviewing files that changed from the base of the PR and between 86cc82a and 6aba4bf.

📒 Files selected for processing (30)
  • apps/ade-cli/src/cli.test.ts
  • apps/ade-cli/src/cli.ts
  • apps/desktop/src/main/services/adeActions/registry.ts
  • apps/desktop/src/main/services/git/gitOperationsService.test.ts
  • apps/desktop/src/main/services/git/gitOperationsService.ts
  • apps/desktop/src/main/services/history/operationService.test.ts
  • apps/desktop/src/main/services/history/operationService.ts
  • apps/desktop/src/main/services/ipc/registerIpc.ts
  • apps/desktop/src/main/services/sync/syncRemoteCommandService.test.ts
  • apps/desktop/src/preload/global.d.ts
  • apps/desktop/src/preload/preload.test.ts
  • apps/desktop/src/preload/preload.ts
  • apps/desktop/src/renderer/components/automations/adeActionSchemas.ts
  • apps/desktop/src/renderer/components/history/CommitDetailPanel.tsx
  • apps/desktop/src/renderer/components/history/CommitHistoryView.tsx
  • apps/desktop/src/renderer/components/history/HistoryPage.tsx
  • apps/desktop/src/renderer/components/history/TimelineListView.tsx
  • apps/desktop/src/renderer/components/history/TimelineToolbar.tsx
  • apps/desktop/src/renderer/components/history/commitGraphLayout.test.ts
  • apps/desktop/src/renderer/components/history/commitGraphLayout.ts
  • apps/desktop/src/renderer/components/history/historyActivitySources.ts
  • apps/desktop/src/renderer/components/history/historyGitActions.test.ts
  • apps/desktop/src/renderer/components/history/historyGitActions.ts
  • apps/desktop/src/renderer/components/history/historyLaneActions.test.ts
  • apps/desktop/src/renderer/components/history/historyLaneActions.ts
  • apps/desktop/src/renderer/components/history/historySearch.test.ts
  • apps/desktop/src/renderer/components/history/historySearch.ts
  • apps/desktop/src/renderer/components/history/useTimelineStore.test.ts
  • apps/desktop/src/renderer/components/history/useTimelineStore.ts
  • apps/desktop/src/shared/types/git.ts

Comment thread apps/desktop/src/preload/preload.ts Outdated
Comment thread apps/ade-cli/src/cli.ts
@arul28 arul28 closed this May 22, 2026
@arul28 arul28 reopened this May 22, 2026
cursoragent and others added 11 commits May 22, 2026 15:59
Add dual-surface History: Commits (DAG graph + virtualized list) and
Activity (existing operations timeline). Commit view includes branch ref
labels, search, right-click git actions, and a detail panel with file list
and related ADE operations. Activity detail links to commits, PRs, missions,
and work. Raise listRecentCommits cap to 500 for deeper history.

Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
Hydrate commit selection from URL, clear selection on lane change,
fetch operations on commits surface, virtualize SVG edges/nodes,
fix HEAD badge and bottom-scroll load-more, isolate git action errors,
sync surface/lane to URL, and confirm destructive git ops.

Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
- TimelineToolbar: Export button calling history.exportOperations with
  headless fallback notice; gear kept for column settings
- EventDetailPanel: show missionId and sessionId from parsed metadata
- CommitHistoryView: refresh control and author on commit rows
- useTimelineStore: default listOperations limit 500
- HistoryPage: lane header strip on commits surface; consolidate URL
  hydration and remove duplicate activity refresh listeners

Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
Implement list, show, commits, and headless export subcommands wired to
operation.list and git.listRecentCommits with text/json formatters and
CLI plan tests.

Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
Phase 2 simplifier polish across 14 files (history feature, git/lanes,
glue/IPC) plus /automate test consolidation: drop single-test
eventTaxonomy file into historyActivitySources, extend operationService
contract tests, refresh docs/features/history + lanes + sync remote-
commands + PRD for new head-change undo/redo, tag/reset/pull-mode
surface.

Tests were cancelled mid-run on a clean tree; CI 8-way shard verifies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@arul28 arul28 force-pushed the cursor/history-tab-gitcraken-overhaul-9c6f branch from 9a496cc to 2bdfdf4 Compare May 22, 2026 20:02
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 22, 2026

@copilot review but do not make fixes

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 22, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Copy link
Copy Markdown
Contributor

Copilot AI commented May 22, 2026

@copilot review but do not make fixes

Review completed (no fixes applied). I still see these issues in the current branch: (1) create_branch only trims input before checkout, no pre-validation in historyGitActions.ts; (2) ensureGitTagName still allows whitespace in gitOperationsService.ts; (3) confirmOrCancel defaults to true when window.confirm is unavailable in historyLaneActions.ts; (4) invalid surface query values are not stripped from URL in HistoryPage.tsx; (5) desktop export is still hard-capped to 500 rows in TimelineToolbar.tsx with no user override.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Scope: 59 file(s), +7004 / −360
Verdict: Minor issues

This PR overhauls the History tab with a GitKraken-style commit graph, unified activity feed (operations + chats/missions/CTO/worker runs), and extensive git/lane actions wired through IPC and remote commands. The main risks are deep-link/commit-window limits and a few UX/perf edges on large histories—not trust-boundary regressions.


🐛 Functionality

[Medium] Deep-linked commits outside the 500-commit window never hydrate

File: apps/desktop/src/renderer/components/history/HistoryPage.tsx:L190-L206
Issue: When commitSha is present in the URL (or store) but that commit is not in the newest 500 rows returned by listRecentCommits, the code only scans that capped list and silently leaves selectedCommit null.
Repro: Open /history?surface=commits&laneId=<lane>&commitSha=<sha> for a commit older than the 500th entry (or after loading only 120 until scroll). The detail pane stays empty while selectedCommitSha remains set.
Fix: Resolve by SHA with a targeted git call (e.g. git rev-parse --verify + one-off log -1 <sha> / existing commit lookup IPC) instead of scanning listRecentCommits; mirror the same pattern in CommitDetailPanel.tsx:L65-L79.

[Medium] Stale laneId in the URL is not reconciled after lane deletion

File: apps/desktop/src/renderer/components/history/HistoryPage.tsx:L111-L122
Issue: URL hydration applies laneId only when it still exists in lanes. If the param references a deleted/unknown lane, neither branch runs, so focusLaneId and the URL can stay out of sync with the lane picker (empty selection + broken git calls).
Repro: Bookmark a commits URL with laneId for a lane you later delete; reload History. The toolbar shows “Select lane…” while the query string still carries the dead id.
Fix: Add an else branch for laneFromUrl && !lanes.some(...) that clears laneId/commitSha from the URL and applies the same fallback used when laneId is absent.


⚡ Performance

[Medium] “Copy patch” fans out up to 50 concurrent file-patch IPC calls

File: apps/desktop/src/renderer/components/history/historyGitActions.ts:L106-L118
Issue: copyCommitPatch loads up to 50 changed files and issues Promise.allSettled over diff.getFilePatch for each path in one user gesture.
Impact: Large commits can spike main-process git/diff work and freeze the renderer briefly on a laptop.
Fix: Batch patches sequentially or with a small concurrency cap (e.g. 4–8), or add a single git.show/aggregate patch IPC for whole-commit export.

[Medium] Activity load merges full operations list plus four supplemental sources every fetch

File: apps/desktop/src/renderer/components/history/useTimelineStore.ts:L381-L396, apps/desktop/src/renderer/components/history/historyActivitySources.ts:L315-L328
Issue: Each fetchEvents pulls up to 500 history.listOperations rows and, in parallel, agent chats (up to 500), missions, CTO snapshot, and worker runs, then sorts/dedupes in the renderer. The 4s poll while any event is running repeats this work.
Impact: Noticeable delay switching to Activity on busy projects; extra CPU/IPC during long-running operations.
Fix: Cache supplemental records with a TTL, pass laneId into supplemental fetchers when scoped, or raise the poll interval unless the visible surface is Activity.


🎨 UI/UX

[Low] Commit history refetch hides in-progress loading when rows already exist

File: apps/desktop/src/renderer/components/history/CommitHistoryView.tsx:L140-L150, L162-L169
Issue: loading is only surfaced via EmptyState when commits.length === 0. Scroll/search bumps limit (120→500) and triggers load() without a visible busy state, so the graph can look frozen during refetch.
Fix: Show a slim top progress indicator or dim the list whenever loading && commits.length > 0.

[Low] Export control is icon-only without an accessible name

File: apps/desktop/src/renderer/components/history/TimelineToolbar.tsx:L313-L326
Issue: The Export button relies on title text only; screen readers get no aria-label, unlike the adjacent refresh control in the commit search bar.
Fix: Add aria-label="Export activity history" (and keep title for tooltip parity).


Notes

Positive patterns worth keeping: request sequencing in CommitHistoryView (loadRequestSeq), virtualized rows + edge culling in the SVG graph, destructive git/lane actions gated with confirm/prompt text checks, and server-side head guards on undo/redo (gitOperationsService.ts:L1292-L1318). No new package.json dependencies in this diff. I could not pull the GitHub PR body via gh (401); review is based on origin/main...origin/cursor/history-tab-gitcraken-overhaul-9c6f at 9a496cc0.

Open in Web View Automation 

Sent by Cursor Automation: BUGBOT in Versic

Copilot finished work on behalf of arul28 May 22, 2026 20:06
- ensureGitTagName: reject whitespace in tag names
- create_branch/create_lane: pre-validate branch names against git ref rules
- confirmOrCancel: default to false when window.confirm is unavailable
- HistoryPage: strip unknown surface URL params, reconcile stale laneId,
  and hydrate deep-linked commits outside the 500-row window via new
  git.getCommit IPC; mirror commit-fallback in CommitDetailPanel
- copyCommitPatch: cap concurrent file-patch IPC fanout to 6
- useTimelineStore: tighten polling — skip supplemental sources on
  in-flight polls, refresh fully on focus
- CommitHistoryView: show slim top loading stripe during refetch
- TimelineToolbar: add aria-label and prompt for custom export limit

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 22, 2026

@copilot review but do not make fixes

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 22, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants