Skip to content

Conversation

@andreasasprou
Copy link
Collaborator

Links

  • ExecPlan: apps/desktop/.agents/plans/20260104-1105-interactive-plan-viewer.md

Summary

  • Add plan-viewer pane type that displays AI agent plans with Tufte-styled markdown
  • Plans appear automatically without stealing focus (uses needsAttention indicator)
  • Integrates with both Claude Code (ExitPlanMode hook) and OpenCode (submit_plan tool)

Why / Context

When AI agents create implementation plans, users need to review them before execution. Currently plans are just text in the terminal. This feature displays plans in a dedicated viewer pane, making them easier to read and review.

This is Phase 1 of a Plannotator-inspired feature. Future phases will add approve/reject workflow and plan editing.

How It Works

Data Flow:

  1. Agent submits plan → writes to temp file → HTTP POST to /hook/plan
  2. Main process validates file (path security, size limit) → reads content → emits event
  3. tRPC subscription delivers event to renderer → store action creates plan-viewer pane

Key Design Decisions:

Decision Choice Rationale
Temp file transport File + HTTP POST Safe for large plans; avoids shell escaping issues
Plan persistence Ephemeral (not persisted) Plans are session-specific; prevents stale data
Focus behavior needsAttention flag Plans appear without interrupting user's current work
Pane reuse Reuse unlocked panes Reduces clutter; lock button for important plans

Agent Integrations:

  • Claude Code: ExitPlanMode hook intercepts plan submission, extracts content from stdin JSON, writes to temp file
  • OpenCode: Plugin v4 adds submit_plan tool that agents can call directly

Manual QA Checklist

Plan Viewer Pane

  • Plan pane appears when agent submits plan
  • Plan renders with markdown formatting
  • Timestamp badge shows relative time
  • Agent type badge shows (claude/opencode)
  • Plan doesn't steal focus from terminal

Lock Feature

  • Lock button toggles lock state
  • Locked pane shows closed lock icon
  • New plan reuses unlocked pane
  • New plan creates new pane if existing is locked

Close / Cleanup

  • Close button removes pane
  • Plan panes not persisted after app restart
  • Old temp files cleaned up on startup (24h max age)

Security

  • Path traversal attempts rejected (../../../etc/passwd)
  • Files outside PLANS_TMP_DIR rejected
  • Plan file size limit enforced (1MB)
  • Invalid plan ID patterns rejected

Testing

  • bun run typecheck
  • bun run lint
  • bun run lint:check-node-imports
  • Manual testing in dev mode ✅

New Files

Main Process

  • src/main/lib/plans/ - Plan file handling (paths, validation, cleanup)

Renderer

  • src/renderer/.../PlanViewerPane/ - Plan viewer component

Agent Setup

  • Updated agent-wrappers.ts - Claude ExitPlanMode hook + OpenCode submit_plan tool
  • Updated ensure-agent-hooks.ts - Plan hook setup

Follow-ups

  • Phase 2: Approve/reject workflow with response back to agent
  • Phase 3: Plan editing in viewer

@coderabbitai
Copy link

coderabbitai bot commented Jan 4, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@andreasasprou andreasasprou marked this pull request as draft January 4, 2026 13:14
andreasasprou and others added 28 commits January 4, 2026 22:12
Introduces a workspace-level view mode toggle allowing users to switch between:
- **Workbench mode**: Mosaic panes layout with terminals + file viewers for in-flow work
- **Review mode**: Dedicated Changes page for focused code review

Key changes:
- Add ViewModeToggle component in workspace header (prominent segmented control)
- Add FileViewerPane with Raw/Rendered/Diff modes, lock/unlock, and split support
- Add GroupStrip for group switching above Mosaic content
- Unify sidebar to use full ChangesView in both modes (with onFileOpen callback)
- Add workspace-view-mode store with per-workspace persistence
- Add readWorkingFile tRPC procedure for safe file reads (size/binary checks)
- Wire file clicks to open/reuse FileViewer panes (MRU unlocked policy)
- Cmd+T in Review mode switches to Workbench first, then creates terminal
- Add !!worktreePath checks to FileViewerPane query enabled conditions
- Add !!worktreePath checks to save handler guards (handleSaveRaw, handleSaveDiff)
- Fix stale state reference after addTab() in addFileViewerPane action

Addresses CodeRabbit review feedback.
… and UX improvements

- Add validatePathForWrite() to prevent path traversal attacks in saveFile
- Add aria-pressed attribute to ViewModeToggle buttons for accessibility
- Increase close button touch target in GroupStrip for better UX
- Add .mdx file support for rendered view mode
P0 Security:
- Add path validation to getUnstagedVersions before readFile (prevents traversal)
- Key Editor/DiffViewer by filePath to force remount (fixes stale Cmd+S closure)

P1 Fixes:
- Update originalContentRef when raw content loads (dirty tracking fix)
- Add .mdx to isMarkdown check for toolbar consistency
- Gate diff editable to staged/unstaged only (against-main/committed now read-only)

P2 Performance:
- Add safeGitShow helper with 2MB size limit on all git show calls
- Add size check before reading working tree files in getUnstagedVersions
- P0-1: Add file-viewer type and fileViewer object to paneSchema for tab persistence
- P0-2: Fix symlink escape vulnerability in validatePathForWrite by checking
  if target is symlink and resolving parent directory paths
- P1-1: Pass defaultBranch to FileViewerPane for against-main diffs
- P1-2: Switch staged diff to unstaged after save (matches Review mode behavior)
- P2: Use Buffer.byteLength instead of string.length in safeGitShow for
  accurate UTF-8 byte counting
P0 (critical):
- Remove duplicate saveFile from git-operations.ts that was overwriting
  the hardened version in file-contents.ts (security vulnerability)

P1 (must fix):
- Use basename()/dirname() from node:path instead of split('/') for
  cross-platform Windows compatibility in validatePathForWrite
- Add preflight size check with git cat-file -s in safeGitShow to
  prevent memory spikes from large blobs before materializing content

P2 (UX):
- Fix split pane to clone file-viewer state instead of creating terminal
  when splitting a file-viewer pane (locked by default)

Question fix:
- Show GroupStrip with add button even when tabs.length === 0 so users
  have visible UI to create new terminal (not just hotkey)
…ocalDb

P0 (CRITICAL SECURITY):
- Add validateWorktreePathInDb() that verifies worktreePath exists in
  localDb.worktrees before any filesystem operations
- Without this, a compromised renderer could read/write arbitrary files
  by passing worktreePath='/' and filePath='.ssh/id_rsa'
- Applied to getFileContents, saveFile, and readWorkingFile procedures

P1 (correctness):
- Replace startsWith('..') checks with segment-aware containsPathTraversal()
  and isPathOutsideBase() helpers that use path.sep
- Fixes false positives on valid paths like '..foo/bar' (directories
  starting with '..')
- Cross-platform compatible (handles both / and \ separators)

P2 (performance):
- Guard killTerminalForPane() calls on pane.type === 'terminal'
- Prevents unnecessary IPC and warning logs when closing file-viewer panes
…ve editor drafts

P0 (security):
- Extract assertWorktreePathInDb to shared security.ts
- Returns worktree record to avoid duplicate queries
- Apply validation to ALL routes: git-operations, status, staging, branches

P1 (data loss):
- Preserve unsaved editor content across view mode switches
- Store draft in ref before switching away from raw mode
- Restore draft when returning to raw mode
- Clear draft on save and file change
- Update dirty state on editor mount with draft content
…otection

P0 Security (BLOCK):
- Add validatePathInWorktree() check before rm() in deleteUntracked
- Prevents path traversal (../) and symlink escape attacks
- Consolidated path validation utilities in security.ts

P1 Data Loss:
- Track save source (Raw vs Diff) with savingFromRawRef
- Only clear draft when saving from Raw mode
- Disable Diff editing when Raw draft exists (forces user to save/discard)

P2:
- Use ['--', filePath] in git.add() to handle paths starting with -
…th validation

- Reduce security.ts from 213 to 65 lines
- Remove async symlink/realpath checking (users own their repos)
- Remove segment-aware path traversal (..foo edge case not worth complexity)
- Keep worktreePath DB validation (the real security boundary)
- Keep simple .. and absolute path checks (sufficient for 99.99% of cases)
P0 (blocking):
- Add path validation (rejects .. and absolute) to applyUntrackedLineCount
- Add 1MB size cap to prevent OOM on large untracked files during polling

P2:
- Add type guard in updateTabLayout - only call killTerminalForPane for terminal panes
- Use ['--', branch] in switchBranch to ensure branch treated as refname
…lidation

- Add path-validation.ts with industry-standard containment check (path.relative)
- Add secure-fs.ts with self-validating FS wrappers (symlink escape protection)
- Add git-commands.ts with semantic helpers (gitSwitchBranch vs gitCheckoutFile)
- Fix switchBranch: use 'git switch' instead of 'git checkout --' (was file checkout)
- Fix path traversal check: segment-aware (allows ..foo, rejects ..)
- Fix symlink bypass: check parent dirs for new files, use stat not lstat
- Fix worktree root deletion: explicit allowRoot check
- All FS operations now go through secureFs with validation built-in
Add a setting to control whether Cmd+clicking file paths in the terminal
opens them in an external editor (default) or in the in-app FileViewerPane.

- Add terminalLinkBehavior setting to local-db schema with migration
- Add getTerminalLinkBehavior/setTerminalLinkBehavior tRPC procedures
- Refactor createTerminalInstance to accept onFileLinkClick callback
- Wire up setting in Terminal.tsx with ref pattern (avoids terminal recreation)
- Add Select UI in BehaviorSettings for choosing link behavior
Remove async symlink escape detection from path validation since the
threat model doesn't justify it: a compromised renderer already has
terminal access for arbitrary command execution.

Changes:
- Replace resolveSecurePath (async) with resolvePathInWorktree (sync)
- Remove assertNoSymlinkEscape and checkSymlinks option
- Add validateRelativePath for simple path safety checks
- Update secure-fs.ts to use new sync functions
- Update threat model documentation in path-validation.ts
…s sidebar)

Add a setting to let users choose between displaying workspaces as
horizontal tabs in the TopBar (current behavior) or in a dedicated left
sidebar (new feature, similar to Linear/GitHub Desktop).

Key changes:
- Add navigationStyle column to settings table (migration 0005)
- Add navigation style dropdown in Behavior Settings
- Create WorkspaceSidebar component with collapsible project sections
- Create shared useWorkspaceShortcuts hook (⌘1-9 shortcuts, auto-create)
- Update TopBar to conditionally render based on navigation style
- Add ⌘⇧B hotkey to toggle workspace sidebar

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ition

P0: Add CreateWorkspaceButton to TopBar when in sidebar mode
- Button was previously only rendered via WorkspaceTabs
- Now renders in top-right section when navigationStyle is sidebar

P1: Fix race condition in createBranchWorkspace
- Add unique partial index on (projectId) WHERE type='branch'
- Use INSERT ON CONFLICT DO NOTHING to handle concurrent calls
- If conflict, fetch the existing workspace instead of failing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
WorkspaceListItem now has full feature parity:
- Close/delete button with confirmation dialog
- Context menu (Rename, Open in Finder)
- Hover card with PR details, checks, reviews
- Needs attention indicator (red pulse)
- Inline rename (double-click)
- Drag & drop reordering
- BranchSwitcher for branch workspaces

Other changes:
- Remove CreateWorkspaceButton from TopBar in sidebar mode
- Add per-project "Add workspace" dropdown (New Workspace, Quick Create)
- Add preSelectedProjectId to modal store for pre-selecting project
- Simplify GroupStrip to use consistent browser-tab style

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Extract useWorkspaceDeleteHandler hook for shared delete logic
- Use existing extractPaneIdsFromLayout from tabs/utils instead of inline collectPaneIds
- Add named constants for magic numbers (staleTime, delays, shortcut index)
- Reduces code duplication between WorkspaceItem and WorkspaceListItem

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ar mode

Move SidebarControl to shared location (screens/main/components/) and
conditionally render based on navigation style:
- top-bar mode: toggle in TopBar (unchanged)
- sidebar mode: toggle in WorkspaceActionBar (left side, before branch info)

This improves UX by placing the sidebar toggle closer to the content
it controls when in sidebar navigation mode.
Default to Mac layout (80px padding) while platform query is loading,
since undefined === 'darwin' evaluates to false, causing 16px padding
to be used initially on Mac before the query resolves.
…minal links

P0: Fix branch workspace support in assertRegisteredWorktree
- Extended validation to check both worktrees.path AND projects.mainRepoPath
- Branch workspaces use mainRepoPath which wasn't being validated

P1: Fix terminal file-viewer links for absolute paths
- Normalize absolute paths to worktree-relative before opening file viewer
- File viewer expects relative paths but terminal links can be absolute

P2: Fix misleading security comments
- Removed claims about symlink checks that aren't implemented
- Comments now accurately describe worktree registration + path traversal validation
- Add markWorkspaceAsUnread action to tabs store that sets needsAttention=true
  for all panes in a workspace
- Add context menu item with LuEyeOff icon to:
  - WorkspaceItemContextMenu (top bar tabs)
  - WorkspaceListItem (sidebar) for both branch and worktree workspaces
- Leverages existing needsAttention indicator system (red pulsing dot)
- Logs when no panes exist in workspace (empty workspace edge case)
Previously, clicking a workspace only set it as active but didn't clear
the needsAttention state on panes. This meant 'Mark as Unread' worked
but clicking the workspace didn't mark it as read.

Added clearWorkspaceAttention action that clears needsAttention for all
panes in a workspace, called when clicking workspace in both top bar
and sidebar.
P0: Make unique branch workspace migration resilient to existing duplicates
- Dedupe existing duplicate branch workspaces before creating unique index
- Update settings.last_active_workspace_id if it points to deleted workspace

P1: Fix isResizing persistence and store selector usage
- Exclude ephemeral isResizing from Zustand persistence via partialize
- Use selector pattern in MainScreen to avoid unnecessary re-renders

P1: Fix delete handler to show dialog when canDeleteData is undefined
- Safe default: show confirmation when query fails

P2: Fix path boundary check in terminal file links
- Use path.startsWith(cwd + '/') to avoid false prefix matches

P2: Add missing drizzle migration snapshot
- Created 0006_snapshot.json for consistency
P0: Add symlink escape protection for File Viewer writes
- Block writes if file's realpath escapes worktree boundary
- Add isSymlinkEscaping() method for UI to detect and warn users
- Update threat model docs to reflect new symlink protection

P1: Fix race condition in createBranchWorkspace ordering
- Insert workspace first, then shift others only on success
- Losers of the race no longer corrupt tabOrder
- Exclude newly inserted workspace from shift operation

P2: Fix migration to keep most recently used duplicate
- Select survivor by last_opened_at DESC (not arbitrary MIN(id))
- Use id ASC as tiebreaker when last_opened_at is equal
Consolidate two navigation bars into a single unified top bar:

- Create WorkspaceControls component group in TopBar/
  - BranchIndicator: shows current branch (keyboard-focusable)
  - OpenInMenuButton: compact Open button with dropdown
  - ViewModeToggleCompact: Workbench/Review toggle (24px height)

- Remove WorkspaceActionBar from WorkspaceView entirely
- Delete unused WorkspaceActionBar component folder

Improvements based on Oracle review:
- Optimize Zustand selector to minimize rerenders
- Add toast error handling for open/copy mutations
- Add path to Open button tooltip for discoverability
- Add focus-visible rings for keyboard accessibility
- Use text-xs for consistent typography

Also includes minor lint auto-fixes (import sorting, template literals)
- Simplify useWorkspaceDeleteHandler to always show dialog instead of
  auto-deleting clean workspaces
- Update tooltip from 'Delete workspace' to 'Close or delete' for clarity
- Add isUnread state for workspace unread tracking
- Add workspace setUnread mutation and schema migration
Replace intense bg-tertiary-active background with subtle border-b-2
border-border for active tab state - cleaner visual that's less heavy
Change from bottom border to top/left/right borders for a traditional
tab appearance that connects to content below
…n mode

Show SidebarControl in GroupStrip header and EmptyTabView when using
sidebar navigation style, allowing users to toggle the changes sidebar
…egression

The sidebar toggle was missing when switching to Review mode because it was
rendered inside TabsContent, which doesn't render in review mode.

Changes:
- Create ContentHeader component at ContentView level with leadingAction slot
- ContentView now owns SidebarControl placement for both workbench and review modes
- Remove SidebarControl from TabsContent and EmptyTabView (now handled by parent)
- Remove unused BranchIndicator component and HOTKEYS import (cleanup)
CMD+T creates a new Group (tab container), not a terminal pane.
This aligns the hotkey naming with the existing 'New Group' button tooltip.
In sidebar navigation mode, move the 'Open In' button and 'Workbench/Review'
toggle from TopBar down to ContentHeader alongside the group tabs. This keeps
the profile dropdown in TopBar while placing workspace controls closer to
the content they affect.

Changes:
- ContentHeader: Add trailingAction prop for right-side controls
- ContentView: Pass WorkspaceControls as trailingAction when isSidebarMode
- TopBar: Only render WorkspaceControls when not in sidebar mode

The controls are visible in both Workbench (with group tabs) and Review
(without group tabs) modes. Top-bar navigation mode is unchanged.
…andling

Security fixes (code review feedback):
- P0-1: Fix ENOENT handling in secure-fs to detect dangling symlinks and
  validate parent directory chains for write operations
- P0-2: Add centralized symlink-escape enforcement to secureFs.readFile
  and readFileBuffer - reads now blocked if symlink escapes worktree
- Add 'symlink-escape' error reason to distinguish from other validation errors
- Use path.relative() for safer boundary checks instead of string prefix matching
- P1: Handle undefined workspaceCwd gracefully in Terminal file-viewer mode
  by falling back to external editor when workspace is still initializing
- P2: Update FileViewerState schema comment to clarify intentional omission
  of transient fields (initialLine/initialColumn)
- P2: Document unique branch-workspace index limitation in schema.ts

Feature completion:
- Terminal file links now pass line/column to File Viewer for initial scroll
- FileViewerPane applies initial line/column navigation in raw mode
- Add initialLine/initialColumn to FileViewerState and store types
- P0: Add dangling symlink validation to prevent writes outside worktree
- P2: Fix isPathWithinWorktree to not incorrectly reject '..config' dirs
- P1: Add symlink-escape error message in FileViewerPane
- P1: Fix line/column navigation when clicking same file with new coords
P2 from review: The dangling symlink target check in assertParentInWorktree
was still using startsWith('..') which incorrectly rejects '..config' dirs.
Now uses the same pattern as isPathWithinWorktree.
P0: secureFs.delete symlink escape
- Check if target is symlink via lstat before deletion
- Symlinks: delete the link itself (safe - lives in worktree)
- Files/dirs: validate realpath is within worktree before rm
- Prevents attack: docs -> /victim, delete docs/file deletes /victim/file

P1: Block external images in markdown renderer
- Add SafeImage component that blocks http:// and https:// URLs
- Shows 'External image blocked' placeholder for external images
- Allows relative paths and data: URLs (safe for repo content)
- Prevents tracking pixels and privacy leaks from untrusted repos

Design decisions (reviewer questions):
- Markdown remote images: Blocked by default. No opt-in toggle yet.
- Deleting symlinks: Allow deleting the symlink file itself (it lives
  in worktree). Never follow the symlink during deletion.
P0: Previous fix only blocked http/https but missed:
- file:// URLs (arbitrary local file access)
- Absolute paths /... or \... (become file:// in Electron)
- Relative paths with .. (escape repo boundary)
- UNC paths //server/share (Windows NTLM credential leak)

Now uses strict allowlist: ONLY data: URLs are allowed.
All other sources show 'Image blocked' placeholder.

Future: Could add opt-in secure loader for repo-relative images
via secureFs validation + blob: URL serving.
- Remove questionable 'unknown switch' error check in gitSwitchBranch
  (not a valid Git error message for missing commands)
- Remove redundant 'relativePath === ""' check in isPathWithinWorktree
  (already handled by equality check above)
- Use 'mod+' instead of 'meta+' for cross-platform keyboard shortcuts
  (maps to Cmd on macOS and Ctrl on Windows/Linux)
- Add getActive query invalidation when toggling workspace unread state
  (ensures UI stays in sync across all queries)
These files diverged from the main branch structure and are no longer needed.
- Remove colored project dots (confusing overlap with working indicators)
- Use muted text color for workspace items and header
- Active workspace retains full foreground color with font-medium
…odes

- Add UnsavedChangesDialog with Cancel, Discard & Switch, Save & Switch options
- Fix DiffViewer read-only mode not updating when editable prop changes
- Use registerSaveAction with addAction for idempotent save command registration
- Add isEditorMounted state to ensure Monaco effects run after mount
- Track dirty state properly across Raw/Diff mode switches
- Handle async save race conditions with proper await and loading state
- Reset baseline refs when switching modes to prevent false dirty state
Add a new setting to control where terminal group tabs are displayed:
- "Sidebar" (default): Groups shown in left sidebar via ModeCarousel + TabsView
  with rename, reorder, and preset features
- "Content header": Groups shown as compact horizontal strip (GroupStrip)

This restores the original sidebar behavior as the default while making
the content header position available as an option.

Implementation:
- Add GroupTabsPosition type and column to local-db schema
- Add getGroupTabsPosition/setGroupTabsPosition tRPC procedures
- Update Sidebar to conditionally render ModeCarousel or ChangesView
- Update ContentView to conditionally show GroupStrip
- Add dropdown in Settings → Behavior

Review mode always shows ChangesView regardless of setting.
- Add runtime validation for stored groupTabsPosition (reset to default
  if invalid value found, following ringtone pattern)
- Centralize valid positions as const to avoid drift between schema,
  tRPC input validation, and UI
- Hoist SIDEBAR_MODES array to stable constant to avoid ModeCarousel
  effect churn on every render
- Add trailing newline to migration SQL file
Add a new pane type that displays implementation plans submitted by AI agents
(Claude Code and OpenCode) in a dedicated viewer with Tufte-styled markdown.

Key features:
- Plans appear automatically without stealing focus (uses needsAttention)
- Lock button to prevent plan replacement by newer submissions
- Plans are ephemeral (not persisted across app restarts)
- Security: path validation, size limits, pattern matching for plan files

Integration:
- Claude Code: ExitPlanMode hook captures plan content
- OpenCode: submit_plan tool in plugin v4
…iewer

Flesh out Phase 2 (approve/reject workflow) and Phase 3 (text annotations)
with architectural decisions informed by Oracle review:

- Response file polling protocol with atomic writes and .waiting sentinel
- Security hardening (path traversal, CSRF, input validation)
- React + web-highlighter compatibility spike requirements
- Comprehensive gotchas and edge cases section
Phase 2 improvements:
- Structured .waiting file with token, pid, createdAt for stale detection
- Exclusive-create writes (O_EXCL) to prevent response race conditions
- Per-install secret token for CSRF protection on localhost endpoints
- planId/planPath mismatch validation
- Race window fix: create .waiting BEFORE notifying Superset

Phase 3 improvements:
- Critical sanitization warning for uncontrolled containers
- Multi-source selection handling (not just sources[0])
- Context-menu conflict mitigation via exceptSelectors
- Toolbar positioning using selection range rect + scroll handling
- Explicit overlap rules with auto-remove behavior

Updated gotchas section with all Oracle findings.
andreasasprou added a commit to andreasasprou/superset that referenced this pull request Jan 4, 2026
Base automatically changed from workspace-sidebar to main January 5, 2026 19:18
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.

2 participants