diff --git a/README.md b/README.md index 9d3b68e1..1ed1631c 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,18 @@

- Turn wait time into parallel progress. + Ten agents.
+ Ten branches.
+ One afternoon.
+

+ +

+ Dispatch AI coding agents in parallel, each in its own worktree.
+ Review the diffs, merge the wins, toss the rest. +

+ +

+ Works with Claude Code, Codex, and Gemini · Every change isolated in its own git worktree · Free, open source, no extra platform fee

@@ -24,8 +35,6 @@ Parallel Code demo

-**Parallel Code** is a desktop app that gives every AI coding agent its own git branch and worktree — automatically. - ## Screenshots | Multiple agents in parallel | Focused view on a single task | @@ -36,7 +45,7 @@ ## Why Parallel Code? -- **Use the AI coding tools you already trust** — [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), [Gemini CLI](https://github.com/google-gemini/gemini-cli) — all from one interface. +- **Use the AI coding tools you already trust** — [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), [Gemini CLI](https://github.com/google-gemini/gemini-cli), and [Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli) — all from one interface. - **Free and open source** — no extra subscription required. MIT licensed. - **Keep every change isolated and reviewable** — each task gets its own git branch and worktree automatically. - **Run agents in parallel, not in sequence** — five agents on five features at the same time, zero conflicts. @@ -71,10 +80,18 @@ When you're happy with the result, merge the branch back to main from the sideba More features - Tiled panel layout with drag-to-reorder -- Built-in diff viewer and changed files list per task +- **Focus mode** — single-task layout with a clean two-column view on wide screens (`Ctrl+Shift+F`) +- Built-in diff viewer with inline review comments and per-commit navigation +- **Steps tracking panel** — engineering-manager-style timeline of agent progress (writes to `.claude/steps.json`) +- **Notes panel per task** — jot ideas, then send the notes straight to the agent as a prompt +- **PR CI status watcher** — desktop notification when GitHub checks settle - Shell terminals per task, scoped to the worktree -- Direct mode for working on the main branch without isolation -- Six themes — Minimal, Graphite, Classic, Indigo, Ember, Glacier +- **Direct mode** for working on the main branch without isolation, plus support for **folders without a git repo** +- **Existing worktree import** — bring already-created worktrees into Parallel Code +- **Sandboxing with project-specific Dockerfiles** — drop a `.parallel-code/Dockerfile` into the project and tasks run inside it +- **Coverage radar** — per-file test-coverage badges in the Changed Files panel +- **Configurable keyboard shortcuts** with per-agent presets +- 10 themes — Islands Dark, Minimal, Graphite, Midnight, Classic, Indigo, Ember, Glacier, Zenburnesque, Workbench - State persists across restarts - macOS and Linux @@ -98,7 +115,7 @@ When you're happy with the result, merge the branch back to main from the sideba - **macOS** — `.dmg` (universal) - **Linux** — `.AppImage` or `.deb` -2. **Install at least one AI coding CLI:** [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), or [Gemini CLI](https://github.com/google-gemini/gemini-cli) +2. **Install at least one AI coding CLI:** [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Codex CLI](https://github.com/openai/codex), [Gemini CLI](https://github.com/google-gemini/gemini-cli), or [Copilot CLI](https://docs.github.com/en/copilot/concepts/agents/about-copilot-cli) 3. **Open Parallel Code**, point it at a git repo, and start dispatching tasks. @@ -135,6 +152,7 @@ Requires [Node.js](https://nodejs.org/) v18+. | `Alt+Arrows` | Navigate between panels | | `Ctrl+Alt+Left/Right` | Reorder active task | | `Ctrl+B` | Toggle sidebar | +| `Ctrl+Shift+F` | Toggle focus mode | | **Terminals** | | | `Ctrl+Shift+T` | New shell terminal | | `Ctrl+Shift+D` | New standalone terminal | diff --git a/electron/ipc/agents.ts b/electron/ipc/agents.ts index ea316e16..5ac0e7c8 100644 --- a/electron/ipc/agents.ts +++ b/electron/ipc/agents.ts @@ -31,7 +31,7 @@ const DEFAULT_AGENTS: AgentDef[] = [ command: 'codex', args: [], resume_args: ['resume', '--last'], - skip_permissions_args: ['--full-auto'], + skip_permissions_args: ['--dangerously-bypass-approvals-and-sandbox'], description: "OpenAI's Codex CLI agent", }, { diff --git a/electron/ipc/channels.ts b/electron/ipc/channels.ts index 0b3221d9..61c037cb 100644 --- a/electron/ipc/channels.ts +++ b/electron/ipc/channels.ts @@ -39,6 +39,7 @@ export enum IPC { GetBranchCommits = 'get_branch_commits', GetCommitChangedFiles = 'get_commit_changed_files', GetCommitDiffs = 'get_commit_diffs', + GetUncommittedFileDiffs = 'get_uncommitted_file_diffs', GetCoverageSummary = 'get_coverage_summary', // Persistence @@ -119,7 +120,8 @@ export enum IPC { ReadFileText = 'read_file_text', // Clipboard - SaveClipboardImage = 'save_clipboard_image', + ResolveClipboardPaste = 'resolve_clipboard_paste', + SaveDroppedImage = 'save_dropped_image', // Notifications ShowNotification = 'show_notification', diff --git a/electron/ipc/git.test.ts b/electron/ipc/git.test.ts index e9f57725..5c882638 100644 --- a/electron/ipc/git.test.ts +++ b/electron/ipc/git.test.ts @@ -55,6 +55,7 @@ import { getChangedFiles, getAllFileDiffs, getFileDiff, + checkMergeStatus, } from './git.js'; type ExecFileCallback = (err: Error | null, stdout: string, stderr: string) => void; @@ -168,7 +169,7 @@ describe('from-branch diff helpers (pickMergeBase wiring)', () => { expect(mergeBaseCall).toContain('main'); }); - it('feeds the picked merge-base SHA into the diff command', async () => { + it('feeds the picked base ref into the one-way diff command', async () => { const calls: string[][] = []; setupMock( calls, @@ -184,8 +185,7 @@ describe('from-branch diff helpers (pickMergeBase wiring)', () => { const diffCall = calls.find((a) => a[0] === 'diff'); expect(diffCall).toBeDefined(); - expect(diffCall).toContain(SHA_LOCAL); - expect(diffCall).toContain('feature'); + expect(diffCall).toContain('develop...feature'); }); }); @@ -209,7 +209,7 @@ describe('from-branch diff helpers (pickMergeBase wiring)', () => { expect(mergeBaseCall).toContain('main'); }); - it('feeds the picked merge-base SHA into the diff command', async () => { + it('feeds the picked base ref into the one-way diff command', async () => { const calls: string[][] = []; setupMock( calls, @@ -225,13 +225,12 @@ describe('from-branch diff helpers (pickMergeBase wiring)', () => { const diffCall = calls.find((a) => a[0] === 'diff'); expect(diffCall).toBeDefined(); - expect(diffCall).toContain(SHA_LOCAL); - expect(diffCall).toContain('feature'); + expect(diffCall).toContain('develop...feature'); }); }); describe('getFileDiffFromBranch', () => { - it('feeds the picked merge-base SHA into the diff and the show command', async () => { + it('feeds the picked base ref into the diff and the merge-base SHA into the show command', async () => { const calls: string[][] = []; setupMock( calls, @@ -247,8 +246,7 @@ describe('from-branch diff helpers (pickMergeBase wiring)', () => { const diffCall = calls.find((a) => a[0] === 'diff'); expect(diffCall).toBeDefined(); - expect(diffCall).toContain(SHA_LOCAL); - expect(diffCall).toContain('feature'); + expect(diffCall).toContain('develop...feature'); // The redundant inline `git merge-base baseRef branchName` is gone — the // only merge-base calls should be the picker's two probes plus optional @@ -284,8 +282,8 @@ describe('pickMergeBase (via getChangedFilesFromBranch)', () => { await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'main'); const diffCall = calls.find((a) => a[0] === 'diff'); - expect(diffCall).toContain(SHA_LOCAL); - expect(diffCall).not.toContain(SHA_ORIGIN); + expect(diffCall).toContain('main...feature'); + expect(diffCall).not.toContain('origin/main...feature'); }); it('picks origin merge-base when local merge-base is its ancestor', async () => { @@ -306,8 +304,8 @@ describe('pickMergeBase (via getChangedFilesFromBranch)', () => { await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'main'); const diffCall = calls.find((a) => a[0] === 'diff'); - expect(diffCall).toContain(SHA_ORIGIN); - expect(diffCall).not.toContain(SHA_LOCAL); + expect(diffCall).toContain('origin/main...feature'); + expect(diffCall).not.toContain('main...feature'); }); it('prefers local on divergence (neither merge-base is an ancestor of the other)', async () => { @@ -328,8 +326,8 @@ describe('pickMergeBase (via getChangedFilesFromBranch)', () => { await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'main'); const diffCall = calls.find((a) => a[0] === 'diff'); - expect(diffCall).toContain(SHA_LOCAL); - expect(diffCall).not.toContain(SHA_ORIGIN); + expect(diffCall).toContain('main...feature'); + expect(diffCall).not.toContain('origin/main...feature'); }); it('uses local merge-base when only the local ref exists', async () => { @@ -347,7 +345,7 @@ describe('pickMergeBase (via getChangedFilesFromBranch)', () => { await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'main'); const diffCall = calls.find((a) => a[0] === 'diff'); - expect(diffCall).toContain(SHA_LOCAL); + expect(diffCall).toContain('main...feature'); const ancestorCalls = calls.filter((a) => a[0] === 'merge-base' && a[1] === '--is-ancestor'); expect(ancestorCalls.length).toBe(0); @@ -368,7 +366,7 @@ describe('pickMergeBase (via getChangedFilesFromBranch)', () => { await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'main'); const diffCall = calls.find((a) => a[0] === 'diff'); - expect(diffCall).toContain(SHA_ORIGIN); + expect(diffCall).toContain('origin/main...feature'); }); it('returns no committed diff when neither ref resolves', async () => { @@ -384,8 +382,7 @@ describe('pickMergeBase (via getChangedFilesFromBranch)', () => { const result = await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'gone'); - // detectMergeBase falls back to headRef ('feature'), so the diff range - // becomes feature..feature → empty. + // detectOneWayDiffRange falls back to feature...feature → empty. expect(result).toEqual([]); const ancestorCalls = calls.filter((a) => a[0] === 'merge-base' && a[1] === '--is-ancestor'); @@ -408,7 +405,7 @@ describe('pickMergeBase (via getChangedFilesFromBranch)', () => { await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'main'); const diffCall = calls.find((a) => a[0] === 'diff'); - expect(diffCall).toContain(SHA_LOCAL); + expect(diffCall).toContain('main...feature'); // Identical SHAs short-circuit before any --is-ancestor probe. const ancestorCalls = calls.filter((a) => a[0] === 'merge-base' && a[1] === '--is-ancestor'); @@ -434,8 +431,9 @@ function uniqueWorktreePath(): string { /** * Build a mock handler for worktree-based functions. * - * No-remote scenario: detectMergeBase returns MERGE_BASE. All diff commands - * use the merge-base ref for one-way diffs (feature branch changes only). + * No-remote scenario: detectMergeBase returns MERGE_BASE. Committed snapshot + * diffs use the ordered local base ref range (`base...head`); dirty worktree + * diffs use the merge-base SHA directly so tracked local edits are included. */ function buildWorktreeMockHandler(opts: { mergeBase?: string; @@ -445,6 +443,15 @@ function buildWorktreeMockHandler(opts: { showOutputs?: Record; diffOutput?: string; statusPorcelain?: string; + /** + * Output of `git log --cherry-pick --right-only --reverse --pretty='%H %P'` — + * newline-separated ` ` pairs (one per unique commit, in + * chronological order; the first line's parent SHA is what refinement + * uses as the new base). + */ + cherryPickUnique?: string; + /** Output of `git rev-list --count` for the refined range. */ + refinedRangeCount?: string; }): MockHandler { const mergeBase = opts.mergeBase ?? MERGE_BASE; @@ -481,13 +488,33 @@ function buildWorktreeMockHandler(opts: { return; } - // diff --raw --numstat — committed changes (one-way) + // log --cherry-pick --right-only — refinement's unique-commit list. + // Default produces an interleaved-fallback so existing tests (which + // assert behavior against the picked merge-base) keep passing without + // having to opt in to refinement defaults. New tests opt in by setting + // `cherryPickUnique` (and matching `refinedRangeCount` for contiguity). + if (cmd === 'log' && args.includes('--cherry-pick')) { + const out = + opts.cherryPickUnique !== undefined + ? opts.cherryPickUnique + : '__test_unique__ __test_parent__'; + cb(null, out, ''); + return; + } + + // rev-list --count .. — refinement's contiguity check. + // Default returns 999 (non-matching) → triggers interleaved fallback. + if (cmd === 'rev-list' && args.includes('--count')) { + cb(null, (opts.refinedRangeCount ?? '999') + '\n', ''); + return; + } + + // diff --raw --numstat ... — committed changes (one-way) if ( cmd === 'diff' && args.includes('--raw') && args.includes('--numstat') && - args.includes(mergeBase) && - args.includes(HEAD_HASH) + args.some((arg) => arg.endsWith(`...${HEAD_HASH}`)) ) { cb(null, opts.committedRawNumstat ?? '', ''); return; @@ -505,17 +532,16 @@ function buildWorktreeMockHandler(opts: { return; } - // diff -U3 — getAllFileDiffs unified diff (one-way) + // diff -U3 — getAllFileDiffs unified diff including tracked local edits if (cmd === 'diff' && args.includes('-U3') && args.includes(mergeBase)) { cb(null, opts.diffOutput ?? '', ''); return; } - // diff -- — getFileDiff committed diff (one-way) + // diff ... -- — getFileDiff committed diff (one-way) if ( cmd === 'diff' && - args.includes(mergeBase) && - args.includes(HEAD_HASH) && + args.some((arg) => arg.endsWith(`...${HEAD_HASH}`)) && args.includes('--') ) { cb(null, opts.diffOutput ?? '', ''); @@ -680,7 +706,7 @@ describe('getChangedFiles (worktree-based, merge-base diff)', () => { expect(files).toEqual([]); }); - it('should diff against merge-base, not branch tip', async () => { + it('should use a base-to-head one-way range, not branch-tip-to-base', async () => { const calls: string[][] = []; setupMock( calls, @@ -691,16 +717,17 @@ describe('getChangedFiles (worktree-based, merge-base diff)', () => { await getChangedFiles(uniqueWorktreePath(), 'main'); - // The committed diff should use MERGE_BASE, not MAIN_TIP + // The committed diff must be ordered base...head, not head...base. const diffCall = calls.find( (a) => a[0] === 'diff' && a.includes('--raw') && a.includes('--numstat') && - a.includes(HEAD_HASH), + a.some((arg) => arg.endsWith(`...${HEAD_HASH}`)), ); expect(diffCall).toBeDefined(); - expect(diffCall).toContain(MERGE_BASE); + expect(diffCall).toContain(`main...${HEAD_HASH}`); + expect(diffCall).not.toContain(`${HEAD_HASH}...main`); }); it('probes both local and origin refs when both exist', async () => { @@ -797,7 +824,7 @@ describe('getAllFileDiffs (worktree-based, merge-base diff)', () => { vi.clearAllMocks(); }); - it('should diff against merge-base without file filtering', async () => { + it('should diff the working tree from the merge-base without file filtering', async () => { const calls: string[][] = []; setupMock( calls, @@ -812,6 +839,7 @@ describe('getAllFileDiffs (worktree-based, merge-base diff)', () => { const u3Call = calls.find((a) => a[0] === 'diff' && a.includes('-U3')); expect(u3Call).toBeDefined(); expect(u3Call).toContain(MERGE_BASE); + expect(u3Call).not.toContain('main...'); // No file filter (no -- separator) expect(u3Call).not.toContain('--'); }); @@ -922,7 +950,7 @@ describe('getFileDiff (worktree-based, merge-base diff)', () => { expect(result.oldContent).toBe(''); }); - it('should issue diff command against merge-base ref', async () => { + it('should issue diff command as a one-way base-to-head range', async () => { const calls: string[][] = []; setupMock( calls, @@ -939,7 +967,7 @@ describe('getFileDiff (worktree-based, merge-base diff)', () => { const diffCall = calls.find((a) => a[0] === 'diff' && a.includes('--')); expect(diffCall).toBeDefined(); - expect(diffCall).toContain(MERGE_BASE); + expect(diffCall).toContain(`main...${HEAD_HASH}`); }); it('should include HEAD hash in the diff command', async () => { @@ -958,7 +986,7 @@ describe('getFileDiff (worktree-based, merge-base diff)', () => { await getFileDiff(uniqueWorktreePath(), 'file.ts', 'main'); const diffCall = calls.find((a) => a[0] === 'diff' && a.includes('--')); - expect(diffCall).toContain(HEAD_HASH); + expect(diffCall?.some((arg) => arg.includes(HEAD_HASH))).toBe(true); }); it('should return the diff output from the merge-base-to-HEAD diff', async () => { @@ -1003,3 +1031,273 @@ describe('getFileDiff (worktree-based, merge-base diff)', () => { expect(result.newContent).toBe('disk content with local edits'); }); }); + +// --------------------------------------------------------------------------- +// Cherry-pick refinement: drops rebased patch-equivalent commits from the +// diff range when the unique commits are contiguous at the tip. +// --------------------------------------------------------------------------- + +describe('refineDiffBaseWithCherryPick (via getChangedFiles)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const UNIQUE_COMMIT = 'unique000abc'; + const UNIQUE_PARENT = 'parent00xyz'; + + it('refines diff base to oldest unique commit parent when commits are contiguous at tip', async () => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + committedRawNumstat: rawNumstatEntry('feat.ts', 5, 1), + cherryPickUnique: `${UNIQUE_COMMIT} ${UNIQUE_PARENT}`, + refinedRangeCount: '1', + }), + ); + + await getChangedFiles(uniqueWorktreePath(), 'main'); + + // The committed-diff range should now use the refined parent SHA, not main. + const committedDiffCall = calls.find( + (a) => + a[0] === 'diff' && + a.includes('--raw') && + a.includes('--numstat') && + a.some((arg) => arg.endsWith(`...${HEAD_HASH}`)), + ); + expect(committedDiffCall).toBeDefined(); + expect(committedDiffCall).toContain(`${UNIQUE_PARENT}...${HEAD_HASH}`); + }); + + it('refines correctly with multiple contiguous unique commits', async () => { + const calls: string[][] = []; + const sha2 = 'unique000def'; + const sha3 = 'unique000ghi'; + setupMock( + calls, + buildWorktreeMockHandler({ + committedRawNumstat: rawNumstatEntry('feat.ts', 5, 1), + // --reverse order: oldest first. Refinement keys off line[0]'s parent. + cherryPickUnique: [ + `${UNIQUE_COMMIT} ${UNIQUE_PARENT}`, + `${sha2} ${UNIQUE_COMMIT}`, + `${sha3} ${sha2}`, + ].join('\n'), + refinedRangeCount: '3', // 3 commits in (parent..HEAD], 3 unique → contiguous + }), + ); + + await getChangedFiles(uniqueWorktreePath(), 'main'); + + const committedDiffCall = calls.find( + (a) => + a[0] === 'diff' && + a.includes('--raw') && + a.includes('--numstat') && + a.some((arg) => arg.endsWith(`...${HEAD_HASH}`)), + ); + expect(committedDiffCall).toBeDefined(); + expect(committedDiffCall).toContain(`${UNIQUE_PARENT}...${HEAD_HASH}`); + }); + + it('falls back to picked base when unique commits are interleaved with patch-equivalent ones', async () => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + committedRawNumstat: rawNumstatEntry('feat.ts', 5, 1), + cherryPickUnique: `${UNIQUE_COMMIT} ${UNIQUE_PARENT}`, // 1 unique + refinedRangeCount: '5', // 5 commits in the range → 4 are patch-equivalent + }), + ); + + await getChangedFiles(uniqueWorktreePath(), 'main'); + + const committedDiffCall = calls.find( + (a) => + a[0] === 'diff' && + a.includes('--raw') && + a.includes('--numstat') && + a.some((arg) => arg.endsWith(`...${HEAD_HASH}`)), + ); + expect(committedDiffCall).toBeDefined(); + // Falls back to the original picked ref (main), not the refined SHA. + expect(committedDiffCall).toContain(`main...${HEAD_HASH}`); + }); + + it('collapses diff range to head...head when branch is fully merged upstream', async () => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + committedRawNumstat: '', + cherryPickUnique: '', // fully merged — no unique commits + }), + ); + + await getChangedFiles(uniqueWorktreePath(), 'main'); + + const committedDiffCall = calls.find( + (a) => + a[0] === 'diff' && + a.includes('--raw') && + a.includes('--numstat') && + a.some((arg) => arg.endsWith(`...${HEAD_HASH}`)), + ); + expect(committedDiffCall).toBeDefined(); + // Diff range collapses to head...head — no patch-equivalent noise. + expect(committedDiffCall).toContain(`${HEAD_HASH}...${HEAD_HASH}`); + expect(committedDiffCall).not.toContain(`main...${HEAD_HASH}`); + + // Refinement short-circuits: no rev-list count needed. + const revListCounts = calls.filter((a) => a[0] === 'rev-list' && a.includes('--count')); + expect(revListCounts).toHaveLength(0); + }); + + it('uses refined SHA for the working-tree single-point diff (getAllFileDiffs)', async () => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + diffOutput: 'diff --git a/file.ts b/file.ts\n', + cherryPickUnique: `${UNIQUE_COMMIT} ${UNIQUE_PARENT}`, + refinedRangeCount: '1', + }), + ); + + await getAllFileDiffs(uniqueWorktreePath(), 'main'); + + const u3Call = calls.find((a) => a[0] === 'diff' && a.includes('-U3')); + expect(u3Call).toBeDefined(); + // After refinement, single-point diff anchors on the refined parent SHA, + // not the original merge-base. + expect(u3Call).toContain(UNIQUE_PARENT); + expect(u3Call).not.toContain(MERGE_BASE); + }); +}); + +// --------------------------------------------------------------------------- +// Refinement applies on the from-branch path too (getChangedFilesFromBranch), +// not just the worktree-based path. +// --------------------------------------------------------------------------- + +describe('refineDiffBaseWithCherryPick (via getChangedFilesFromBranch)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('refines diff range when called with a branch name (not HEAD)', async () => { + const calls: string[][] = []; + const SHA = 'br000sha'; + const PARENT = 'br000par'; + + setupMock(calls, (args, cb) => { + const [cmd] = args; + if (cmd === 'rev-parse' && args[1] === '--verify') { + const ref = args[2]; + if (ref === 'refs/heads/main') return cb(null, 'exists\n', ''); + return cb(new Error('no remote'), '', ''); + } + if (cmd === 'symbolic-ref') return cb(new Error('no remote'), '', ''); + if (cmd === 'merge-base') return cb(null, MERGE_BASE + '\n', ''); + if (cmd === 'log' && args.includes('--cherry-pick')) { + return cb(null, `${SHA} ${PARENT}\n`, ''); + } + if (cmd === 'rev-list' && args.includes('--count')) { + return cb(null, '1\n', ''); + } + if (cmd === 'diff') return cb(null, '', ''); + return cb(null, '', ''); + }); + + await getChangedFilesFromBranch(uniqueRepoPath(), 'feature', 'main'); + + const diffCall = calls.find((a) => a[0] === 'diff'); + expect(diffCall).toBeDefined(); + // Range uses the refined parent SHA against the branch ref (not HEAD). + expect(diffCall).toContain(`${PARENT}...feature`); + }); +}); + +// --------------------------------------------------------------------------- +// checkMergeStatus — main-ahead count uses cherry-pick filtering so rebased +// patch-equivalent commits in main don't trigger a needless rebase prompt. +// --------------------------------------------------------------------------- + +describe('checkMergeStatus (cherry-pick filtered ahead count)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('asks rev-list to count with --cherry-pick --right-only against HEAD...mainBranch', async () => { + const calls: string[][] = []; + setupMock(calls, (args, cb) => { + const [cmd] = args; + if (cmd === 'rev-list' && args.includes('--count')) { + return cb(null, '0\n', ''); + } + return cb(null, '', ''); + }); + + await checkMergeStatus(uniqueWorktreePath(), 'main'); + + const revListCall = calls.find((a) => a[0] === 'rev-list' && a.includes('--count')); + expect(revListCall).toBeDefined(); + expect(revListCall).toContain('--cherry-pick'); + expect(revListCall).toContain('--right-only'); + expect(revListCall).toContain('HEAD...main'); + // --no-merges would silently drop merge commits with unique content + // (evil merges) and undercount truly-ahead main, so it must not be set. + expect(revListCall).not.toContain('--no-merges'); + }); + + it('reports zero ahead when every main commit is patch-equivalent to one in HEAD', async () => { + const calls: string[][] = []; + setupMock(calls, (args, cb) => { + const [cmd] = args; + if (cmd === 'rev-list' && args.includes('--count')) { + // --cherry-pick filters patch-equivalents → none unique on the right. + return cb(null, '0\n', ''); + } + return cb(null, '', ''); + }); + + const result = await checkMergeStatus(uniqueWorktreePath(), 'main'); + + expect(result.main_ahead_count).toBe(0); + expect(result.conflicting_files).toEqual([]); + // Conflict probe must be skipped when ahead count is zero. + const mergeTreeCall = calls.find((a) => a[0] === 'merge-tree'); + expect(mergeTreeCall).toBeUndefined(); + }); + + it('runs the conflict probe and surfaces conflicting files when main is genuinely ahead', async () => { + const calls: string[][] = []; + setupMock(calls, (args, cb) => { + const [cmd] = args; + if (cmd === 'rev-list' && args.includes('--count')) { + // 2 commits in main are unique after cherry-pick filtering. + return cb(null, '2\n', ''); + } + if (cmd === 'merge-tree') { + // git merge-tree exits non-zero and emits conflict info on stderr/stdout + // when the trial merge fails — checkMergeStatus parses the rejection. + const err = new Error( + 'CONFLICT (content): Merge conflict in src/foo.ts\nCONFLICT (content): Merge conflict in src/bar.ts', + ); + return cb(err, '', ''); + } + return cb(null, '', ''); + }); + + const result = await checkMergeStatus(uniqueWorktreePath(), 'main'); + + expect(result.main_ahead_count).toBe(2); + const mergeTreeCall = calls.find((a) => a[0] === 'merge-tree'); + expect(mergeTreeCall).toBeDefined(); + expect(mergeTreeCall).toContain('HEAD'); + expect(mergeTreeCall).toContain('main'); + expect(result.conflicting_files).toEqual(['src/foo.ts', 'src/bar.ts']); + }); +}); diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index afe284e4..3ad32d37 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -40,10 +40,20 @@ interface CacheEntry { expiresAt: number; } +interface PickedMergeBase { + sha: string; + ref: string; +} + +interface DiffBaseCacheEntry { + value: PickedMergeBase; + expiresAt: number; +} + const mainBranchCache = new Map(); -const mergeBaseCache = new Map(); +const diffBaseCache = new Map(); const MAIN_BRANCH_TTL = 60_000; // 60s -const MERGE_BASE_TTL = 30_000; // 30s +const DIFF_BASE_TTL = 30_000; // 30s const MAX_BUFFER = 10 * 1024 * 1024; // 10MB const STDERR_CAP = 4096; // cap for stderr buffers in spawned git processes @@ -55,8 +65,8 @@ setInterval(() => { for (const [k, v] of mainBranchCache) { if (v.expiresAt <= now) mainBranchCache.delete(k); } - for (const [k, v] of mergeBaseCache) { - if (v.expiresAt <= now) mergeBaseCache.delete(k); + for (const [k, v] of diffBaseCache) { + if (v.expiresAt <= now) diffBaseCache.delete(k); } }, CACHE_SWEEP_INTERVAL).unref(); @@ -77,8 +87,8 @@ async function isBinaryFile(filePath: string): Promise { } } -function invalidateMergeBaseCache(): void { - mergeBaseCache.clear(); +function invalidateDiffBaseCache(): void { + diffBaseCache.clear(); } function cacheKey(p: string): string { @@ -272,7 +282,7 @@ async function pickMergeBase( repoRoot: string, branch: string, head: string, -): Promise { +): Promise { const [hasLocal, hasOrigin] = await Promise.all([ localBranchExists(repoRoot, branch), remoteTrackingRefExists(repoRoot, branch), @@ -295,9 +305,10 @@ async function pickMergeBase( ]); if (!localMb && !originMb) return null; - if (!localMb) return originMb; - if (!originMb) return localMb; - if (localMb === originMb) return localMb; + if (!localMb && originMb) return { sha: originMb, ref: `origin/${branch}` }; + if (!originMb && localMb) return { sha: localMb, ref: branch }; + if (!localMb || !originMb) return null; + if (localMb === originMb) return { sha: localMb, ref: branch }; const isAncestor = async (anc: string, desc: string): Promise => { try { @@ -308,38 +319,174 @@ async function pickMergeBase( } }; - if (await isAncestor(originMb, localMb)) return localMb; - if (await isAncestor(localMb, originMb)) return originMb; - return localMb; + if (await isAncestor(originMb, localMb)) return { sha: localMb, ref: branch }; + if (await isAncestor(localMb, originMb)) return { sha: originMb, ref: `origin/${branch}` }; + return { sha: localMb, ref: branch }; } /** - * Resolve the diff base SHA for `head` against `baseBranch` (or the detected - * main branch). Delegates ref-picking to `pickMergeBase`; falls back to - * `headRef` when no candidate ref resolves so callers diff against themselves - * (empty diff) rather than against the branch tip. + * Refine a picked merge-base by dropping commits that are patch-equivalent + * to ones already on `base.ref` (rebased duplicates from a prior `git merge` + * of the upstream that was later rebased onto a new base). + * + * Uses git's built-in patch-id detection via `--cherry-pick --right-only`. + * The first call's `%H %P` format embeds the oldest unique commit's parent + * SHA so we don't need a separate `rev-parse` step. + * + * Three outcomes: + * - **No unique commits** (branch is fully merged upstream): collapse the + * diff range to `head...head` (empty) so the user sees zero changes + * instead of the noisy patch-equivalent set. + * - **Unique commits contiguous at the tip** (the common case for + * agent-driven branches): refine the base to the oldest unique commit's + * parent so the diff range only contains real branch work. + * - **Interleaved** (a patch-equivalent commit sits between unique ones): + * keep the picked base — accepting the noise is cheaper than stitching + * per-commit patches. Logged for diagnostics. + * + * Any git invocation failure falls back to the picked base unchanged. + * + * Dual-side counterpart: `checkMergeStatus` applies `--cherry-pick + * --right-only` to `HEAD...
` to count *main's* unique commits not in + * HEAD (i.e. how stale HEAD is relative to main), so the merge dialog's + * "Rebase first" prompt agrees with this filter. */ -async function detectMergeBase( +async function refineDiffBaseWithCherryPick( + repoRoot: string, + base: PickedMergeBase, + head: string, +): Promise { + let unique: string[]; + let oldestParent: string | null = null; + try { + const { stdout } = await exec( + 'git', + [ + 'log', + '--cherry-pick', + '--right-only', + '--no-merges', + '--reverse', + '--pretty=%H %P', + `${base.ref}...${head}`, + ], + { cwd: repoRoot, maxBuffer: MAX_BUFFER }, + ); + const lines = stdout + .split('\n') + .map((s) => s.trim()) + .filter(Boolean); + unique = lines.map((line) => line.split(' ', 1)[0]); + if (lines.length > 0) { + // First line is the oldest unique commit (--reverse). With --no-merges + // every commit has exactly one parent, so the first SHA after the + // commit hash is that parent. + const parts = lines[0].split(' '); + oldestParent = parts[1] ?? null; + } + } catch { + return base; + } + + if (unique.length === 0) { + // Fully merged upstream — every branch commit is patch-equivalent to + // one already on the base. Collapse the diff range to empty so the + // user sees no changes (instead of the noisy duplicated patch set + // that `base.ref...HEAD` would produce). + return { sha: head, ref: head }; + } + + if (!oldestParent) return base; + + let rangeCount: number; + try { + const { stdout } = await exec( + 'git', + ['rev-list', '--count', '--no-merges', `${oldestParent}..${head}`], + { cwd: repoRoot }, + ); + rangeCount = parseInt(stdout.trim(), 10); + if (isNaN(rangeCount)) return base; + } catch { + return base; + } + + if (rangeCount === unique.length) { + return { sha: oldestParent, ref: oldestParent }; + } + + logDebug( + 'git', + `cherry-pick refine: interleaved (unique=${unique.length}, range=${rangeCount}) — keeping ${base.ref}`, + ); + return base; +} + +/** + * Resolve both sides needed for one-way diffs. + * + * Git's three-dot diff is directional: `git diff base...head` means + * `git diff $(git merge-base base head) head`, while `git diff head...base` + * shows base-only commits. Keep the picked base ref around so committed-only + * diffs can use the correctly ordered three-dot range. + * + * Working-tree diffs are different: `git diff base...` still compares against + * HEAD, not the dirty working tree. For those callers, use `base.sha` as the + * single diff start point (`git diff `) so tracked local edits + * remain visible. + * + * Result is post-refinement: rebased patch-equivalent commits are dropped from + * the diff range when possible. See `refineDiffBaseWithCherryPick`. + */ +async function detectDiffBase( repoRoot: string, head?: string, baseBranch?: string, -): Promise { +): Promise { const branch = baseBranch ?? (await detectMainBranch(repoRoot)); const headRef = head ?? 'HEAD'; const key = `${cacheKey(repoRoot)}:${branch}:${headRef}`; - const cached = mergeBaseCache.get(key); + const cached = diffBaseCache.get(key); if (cached) { if (cached.expiresAt > Date.now()) return cached.value; - mergeBaseCache.delete(key); + diffBaseCache.delete(key); } const picked = await pickMergeBase(repoRoot, branch, headRef); - if (picked) { - mergeBaseCache.set(key, { value: picked, expiresAt: Date.now() + MERGE_BASE_TTL }); - return picked; - } + if (!picked) return { sha: headRef, ref: headRef }; - return headRef; + const refined = await refineDiffBaseWithCherryPick(repoRoot, picked, headRef); + diffBaseCache.set(key, { value: refined, expiresAt: Date.now() + DIFF_BASE_TTL }); + return refined; +} + +/** + * Resolve the diff base SHA for `head` against `baseBranch` (or the detected + * main branch). Falls back to `headRef` when no candidate ref resolves so + * callers diff against themselves (empty diff) rather than against the branch + * tip. + */ +async function detectMergeBase( + repoRoot: string, + head?: string, + baseBranch?: string, +): Promise { + const headRef = head ?? 'HEAD'; + const result = await detectDiffBase(repoRoot, headRef, baseBranch); + return result.sha; +} + +function oneWayDiffRange(base: PickedMergeBase, head: string): string { + return `${base.ref}...${head}`; +} + +async function detectOneWayDiffRange( + repoRoot: string, + head?: string, + baseBranch?: string, +): Promise { + const headRef = head ?? 'HEAD'; + return oneWayDiffRange(await detectDiffBase(repoRoot, headRef, baseBranch), headRef); } async function pinHead(worktreePath: string): Promise { @@ -491,8 +638,8 @@ async function computeBranchDiffStats( mainBranch: string, branchName: string, ): Promise<{ linesAdded: number; linesRemoved: number }> { - const base = await detectMergeBase(projectRoot, branchName, mainBranch); - const { stdout } = await exec('git', ['diff', '--numstat', base, branchName], { + const diffRange = await detectOneWayDiffRange(projectRoot, branchName, mainBranch); + const { stdout } = await exec('git', ['diff', '--numstat', diffRange], { cwd: projectRoot, maxBuffer: MAX_BUFFER, }); @@ -823,12 +970,18 @@ export async function getChangedFiles( ): Promise { const headHash = await pinHead(worktreePath); - // Diff merge-base → HEAD: one-way diff showing only what the feature branch changed. - const base = await detectMergeBase(worktreePath, headHash, baseBranch).catch(() => headHash); + // One-way diff from the base branch's merge-base to HEAD. The three-dot + // range is intentionally ordered as base...head so base-only commits do not + // show up as changed files on stale task branches. + const diffBase = await detectDiffBase(worktreePath, headHash, baseBranch).catch(() => ({ + sha: headHash, + ref: headHash, + })); + const diffRange = oneWayDiffRange(diffBase, headHash); let diffStr = ''; try { - const { stdout } = await exec('git', ['diff', '--raw', '--numstat', base, headHash], { + const { stdout } = await exec('git', ['diff', '--raw', '--numstat', diffRange], { cwd: worktreePath, maxBuffer: MAX_BUFFER, }); @@ -868,7 +1021,7 @@ export async function getChangedFiles( const files: ChangedFile[] = []; const seen = new Set(); - // Committed files from diff base..HEAD + // Committed files from the one-way base...HEAD diff for (const [p, [added, removed]] of committedNumstatMap) { const status = committedStatusMap.get(p) ?? 'M'; // If also in uncommitted diff, mark as uncommitted (has local changes on top) @@ -928,16 +1081,65 @@ export async function getChangedFiles( return files; } +async function buildUntrackedPseudoDiffs(worktreePath: string): Promise { + const parts: string[] = []; + let stdout = ''; + try { + const result = await exec('git', ['ls-files', '--others', '--exclude-standard'], { + cwd: worktreePath, + maxBuffer: MAX_BUFFER, + }); + stdout = result.stdout; + } catch { + return parts; + } + + for (const line of stdout.split('\n')) { + const filePath = normalizeStatusPath(line); + if (!filePath) continue; + const fullPath = path.join(worktreePath, filePath); + try { + const stat = await fs.promises.stat(fullPath); + if (!stat.isFile() || stat.size >= MAX_BUFFER) continue; + if (await isBinaryFile(fullPath)) { + parts.push( + `diff --git a/${filePath} b/${filePath}\nnew file mode 100644\nBinary files /dev/null and b/${filePath} differ\n`, + ); + continue; + } + const content = await fs.promises.readFile(fullPath, 'utf8'); + const lines = content.split('\n'); + const lineCount = content.endsWith('\n') ? lines.length - 1 : lines.length; + const pseudoLines: string[] = []; + pseudoLines.push(`diff --git a/${filePath} b/${filePath}`); + pseudoLines.push('new file mode 100644'); + pseudoLines.push('--- /dev/null'); + pseudoLines.push(`+++ b/${filePath}`); + pseudoLines.push(`@@ -0,0 +1,${lineCount} @@`); + for (let i = 0; i < lineCount; i++) { + pseudoLines.push(`+${lines[i]}`); + } + parts.push(pseudoLines.join('\n') + '\n'); + } catch { + /* skip unreadable files */ + } + } + return parts; +} + export async function getAllFileDiffs(worktreePath: string, baseBranch?: string): Promise { const headHash = await pinHead(worktreePath); - // Diff merge-base → working tree: one-way diff showing only feature branch changes - // (including uncommitted edits). - const base = await detectMergeBase(worktreePath, headHash, baseBranch).catch(() => headHash); + // For working-tree output, use the merge-base SHA as a single diff start + // point. `git diff base...` would stop at HEAD and hide tracked local edits. + const diffBase = await detectDiffBase(worktreePath, headHash, baseBranch).catch(() => ({ + sha: headHash, + ref: headHash, + })); let combinedDiff = ''; try { - const { stdout } = await exec('git', ['diff', '-U3', base], { + const { stdout } = await exec('git', ['diff', '-U3', diffBase.sha], { cwd: worktreePath, maxBuffer: MAX_BUFFER, }); @@ -946,47 +1148,29 @@ export async function getAllFileDiffs(worktreePath: string, baseBranch?: string) /* empty */ } - // Untracked files: build pseudo-diffs - const untrackedParts: string[] = []; + const untrackedParts = await buildUntrackedPseudoDiffs(worktreePath); + const parts = [combinedDiff, untrackedParts.join('')].filter((p) => p.length > 0); + return parts.join('\n'); +} + +export async function getUncommittedFileDiffs(worktreePath: string): Promise { + const headHash = await pinHead(worktreePath); + + // Diff against HEAD captures only tracked uncommitted changes (working tree + // vs HEAD), excluding committed work. Uses the HEAD SHA directly so it does + // not need the index lock and works while an agent holds it. + let combinedDiff = ''; try { - const { stdout } = await exec('git', ['ls-files', '--others', '--exclude-standard'], { + const { stdout } = await exec('git', ['diff', '-U3', headHash], { cwd: worktreePath, maxBuffer: MAX_BUFFER, }); - for (const line of stdout.split('\n')) { - const filePath = normalizeStatusPath(line); - if (!filePath) continue; - const fullPath = path.join(worktreePath, filePath); - try { - const stat = await fs.promises.stat(fullPath); - if (!stat.isFile() || stat.size >= MAX_BUFFER) continue; - if (await isBinaryFile(fullPath)) { - untrackedParts.push( - `diff --git a/${filePath} b/${filePath}\nnew file mode 100644\nBinary files /dev/null and b/${filePath} differ\n`, - ); - continue; - } - const content = await fs.promises.readFile(fullPath, 'utf8'); - const lines = content.split('\n'); - const lineCount = content.endsWith('\n') ? lines.length - 1 : lines.length; - const pseudoLines: string[] = []; - pseudoLines.push(`diff --git a/${filePath} b/${filePath}`); - pseudoLines.push('new file mode 100644'); - pseudoLines.push('--- /dev/null'); - pseudoLines.push(`+++ b/${filePath}`); - pseudoLines.push(`@@ -0,0 +1,${lineCount} @@`); - for (let i = 0; i < lineCount; i++) { - pseudoLines.push(`+${lines[i]}`); - } - untrackedParts.push(pseudoLines.join('\n') + '\n'); - } catch { - /* skip unreadable files */ - } - } + combinedDiff = stdout; } catch { /* empty */ } + const untrackedParts = await buildUntrackedPseudoDiffs(worktreePath); const parts = [combinedDiff, untrackedParts.join('')].filter((p) => p.length > 0); return parts.join('\n'); } @@ -996,10 +1180,9 @@ export async function getAllFileDiffsFromBranch( branchName: string, baseBranch?: string, ): Promise { - const mainBranch = baseBranch ?? (await detectMainBranch(projectRoot)); - const base = await detectMergeBase(projectRoot, branchName, mainBranch); + const diffRange = await detectOneWayDiffRange(projectRoot, branchName, baseBranch); try { - const { stdout } = await exec('git', ['diff', '-U3', base, branchName], { + const { stdout } = await exec('git', ['diff', '-U3', diffRange], { cwd: projectRoot, maxBuffer: MAX_BUFFER, }); @@ -1021,7 +1204,11 @@ export async function getFileDiff( baseBranch?: string, ): Promise { const headHash = await pinHead(worktreePath); - const base = await detectMergeBase(worktreePath, headHash, baseBranch).catch(() => headHash); + const diffBase = await detectDiffBase(worktreePath, headHash, baseBranch).catch(() => ({ + sha: headHash, + ref: headHash, + })); + const base = diffBase.sha; // Old content from merge-base (what existed when the branch was created) let oldContent = ''; @@ -1091,7 +1278,8 @@ export async function getFileDiff( // Generate diff between merge-base and HEAD for committed files let diff = ''; try { - const { stdout } = await exec('git', ['diff', base, headHash, '--', filePath], { + const diffRange = oneWayDiffRange(diffBase, headHash); + const { stdout } = await exec('git', ['diff', diffRange, '--', filePath], { cwd: worktreePath, maxBuffer: MAX_BUFFER, }); @@ -1238,11 +1426,22 @@ export async function checkMergeStatus( ): Promise<{ main_ahead_count: number; conflicting_files: string[]; base_branch: string }> { const mainBranch = baseBranch ?? (await detectMainBranch(worktreePath)); + // Count main commits not in HEAD, excluding patch-equivalents already + // applied via rebase/cherry-pick. Mirrors the `--cherry-pick --right-only` + // filter `refineDiffBaseWithCherryPick` uses on the diff side, so the + // dialog doesn't demand a needless rebase when HEAD's history already + // carries main's recent commits with different SHAs. Unlike the diff-side + // helper we *don't* pass `--no-merges`: it's load-bearing there for `%H %P` + // single-parent parsing, but here it would silently drop merge commits + // that brought genuinely-new content into main (so-called "evil merges"), + // turning a real "rebase first" warning into a quiet zero. let mainAheadCount = 0; try { - const { stdout } = await exec('git', ['rev-list', '--count', `HEAD..${mainBranch}`], { - cwd: worktreePath, - }); + const { stdout } = await exec( + 'git', + ['rev-list', '--count', '--cherry-pick', '--right-only', `HEAD...${mainBranch}`], + { cwd: worktreePath }, + ); mainAheadCount = parseInt(stdout.trim(), 10) || 0; } catch { /* ignore */ @@ -1370,7 +1569,7 @@ export async function mergeTask( } } - invalidateMergeBaseCache(); + invalidateDiffBaseCache(); if (cleanup) { await removeWorktree(projectRoot, branchName, true); @@ -1400,12 +1599,11 @@ export async function getChangedFilesFromBranch( branchName: string, baseBranch?: string, ): Promise { - const mainBranch = baseBranch ?? (await detectMainBranch(projectRoot)); - const base = await detectMergeBase(projectRoot, branchName, mainBranch); + const diffRange = await detectOneWayDiffRange(projectRoot, branchName, baseBranch); let diffStr = ''; try { - const { stdout } = await exec('git', ['diff', '--raw', '--numstat', base, branchName], { + const { stdout } = await exec('git', ['diff', '--raw', '--numstat', diffRange], { cwd: projectRoot, maxBuffer: MAX_BUFFER, }); @@ -1439,12 +1637,13 @@ export async function getFileDiffFromBranch( filePath: string, baseBranch?: string, ): Promise { - const mainBranch = baseBranch ?? (await detectMainBranch(projectRoot)); - const base = await detectMergeBase(projectRoot, branchName, mainBranch); + const diffBase = await detectDiffBase(projectRoot, branchName, baseBranch); + const base = diffBase.sha; + const diffRange = oneWayDiffRange(diffBase, branchName); let diff = ''; try { - const { stdout } = await exec('git', ['diff', base, branchName, '--', filePath], { + const { stdout } = await exec('git', ['diff', diffRange, '--', filePath], { cwd: projectRoot, maxBuffer: MAX_BUFFER, }); @@ -1548,7 +1747,7 @@ export async function rebaseTask(worktreePath: string, baseBranch?: string): Pro ); throw new Error(`Rebase failed: ${e}`); } - invalidateMergeBaseCache(); + invalidateDiffBaseCache(); }); } diff --git a/electron/ipc/pty.test.ts b/electron/ipc/pty.test.ts index afa4a15b..86238e23 100644 --- a/electron/ipc/pty.test.ts +++ b/electron/ipc/pty.test.ts @@ -4,63 +4,66 @@ import path from 'path'; import type { BrowserWindow } from 'electron'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { mockExecFileSync, mockExecFile, mockChildProcessSpawn, mockPtySpawn } = vi.hoisted(() => { - const mockExecFileSync = vi.fn((command: string, args?: string[]) => { - if (command === 'which' && args?.[0] === 'nonexistent-binary-xyz') { - throw new Error('not found'); - } - return ''; - }); - - const mockExecFile = vi.fn(); - const mockChildProcessSpawn = vi.fn(() => ({ - stdout: { on: vi.fn() }, - stderr: { on: vi.fn() }, - on: vi.fn(), - })); - - const mockPtySpawn = vi.fn( - (_command: string, _args: string[], options: { cols: number; rows: number }) => { - let onDataHandler: ((data: string) => void) | undefined; - let onExitHandler: - | ((event: { exitCode: number; signal: number | undefined }) => void) - | undefined; - - const proc = { - cols: options.cols, - rows: options.rows, - write: vi.fn(), - resize: vi.fn((cols: number, rows: number) => { - proc.cols = cols; - proc.rows = rows; - }), - pause: vi.fn(), - resume: vi.fn(), - kill: vi.fn(() => { - onExitHandler?.({ exitCode: 0, signal: 15 }); - }), - onData: vi.fn((handler: (data: string) => void) => { - onDataHandler = handler; - }), - onExit: vi.fn( - (handler: (event: { exitCode: number; signal: number | undefined }) => void) => { - onExitHandler = handler; +const { mockExecFileSync, mockExecFile, mockChildProcessSpawn, mockPtySpawn, mockLogDebug } = + vi.hoisted(() => { + const mockExecFileSync = vi.fn((command: string, args?: string[]) => { + if (command === 'which' && args?.[0] === 'nonexistent-binary-xyz') { + throw new Error('not found'); + } + return ''; + }); + + const mockExecFile = vi.fn(); + const mockChildProcessSpawn = vi.fn(() => ({ + stdout: { on: vi.fn() }, + stderr: { on: vi.fn() }, + on: vi.fn(), + })); + + const mockPtySpawn = vi.fn( + (_command: string, _args: string[], options: { cols: number; rows: number }) => { + let onDataHandler: ((data: string) => void) | undefined; + let onExitHandler: + | ((event: { exitCode: number; signal: number | undefined }) => void) + | undefined; + + const proc = { + cols: options.cols, + rows: options.rows, + write: vi.fn(), + resize: vi.fn((cols: number, rows: number) => { + proc.cols = cols; + proc.rows = rows; + }), + pause: vi.fn(), + resume: vi.fn(), + kill: vi.fn(() => { + onExitHandler?.({ exitCode: 0, signal: 15 }); + }), + onData: vi.fn((handler: (data: string) => void) => { + onDataHandler = handler; + }), + onExit: vi.fn( + (handler: (event: { exitCode: number; signal: number | undefined }) => void) => { + onExitHandler = handler; + }, + ), + emitData(data: string) { + onDataHandler?.(data); }, - ), - emitData(data: string) { - onDataHandler?.(data); - }, - emitExit(event: { exitCode: number; signal: number | undefined }) { - onExitHandler?.(event); - }, - }; + emitExit(event: { exitCode: number; signal: number | undefined }) { + onExitHandler?.(event); + }, + }; - return proc; - }, - ); + return proc; + }, + ); - return { mockExecFileSync, mockExecFile, mockChildProcessSpawn, mockPtySpawn }; -}); + const mockLogDebug = vi.fn(); + + return { mockExecFileSync, mockExecFile, mockChildProcessSpawn, mockPtySpawn, mockLogDebug }; + }); vi.mock('child_process', async () => { const actual = await vi.importActual('child_process'); @@ -76,6 +79,10 @@ vi.mock('node-pty', () => ({ spawn: mockPtySpawn, })); +vi.mock('../log.js', () => ({ + debug: mockLogDebug, +})); + import { buildDockerImage, DOCKER_CONTAINER_HOME, @@ -119,6 +126,7 @@ function buildSpawnArgs( rows: 40, dockerMode: true, dockerImage: 'parallel-code-agent:test', + shareDockerAgentAuth: false, onOutput: { __CHANNEL_ID__: 'channel-1' }, ...overrides, }; @@ -155,6 +163,14 @@ function getFlagValues(args: string[], flag: string): string[] { return values; } +function getSpawnCommandLogCtx(): { args: string[]; command: string } { + const call = mockLogDebug.mock.calls.find( + ([category, msg]) => category === 'pty' && String(msg).startsWith('spawn command '), + ); + expect(call).toBeTruthy(); + return call?.[2] as { args: string[]; command: string }; +} + function makeTempHome(entries: string[]): string { const home = fs.mkdtempSync(path.join(os.tmpdir(), 'pty-docker-home-')); tempPaths.push(home); @@ -229,6 +245,61 @@ describe('spawnAgent docker mode', () => { expect(envFlags).not.toContain(`HOME=${rendererHome}`); }); + it('redacts docker env values in spawn debug logs', () => { + spawnAgent( + createMockWindow(), + buildSpawnArgs({ + env: { + API_KEY: 'secret-api-key', + NO_VALUE: '', + }, + }), + ); + + const ctx = getSpawnCommandLogCtx(); + const logged = ctx.args.join(' '); + + expect(ctx.command).toBe('docker'); + expect(getFlagValues(ctx.args, '-e')).toContain('API_KEY='); + expect(getFlagValues(ctx.args, '-e')).toContain('NO_VALUE='); + expect(getFlagValues(ctx.args, '-e')).toContain(`HOME=`); + expect(logged).not.toContain('secret-api-key'); + expect(logged).not.toContain(`HOME=${DOCKER_CONTAINER_HOME}`); + expect(logged).toContain('parallel-code-agent:test'); + }); + + it('redacts inline docker env values in spawn debug logs', () => { + spawnAgent( + createMockWindow(), + buildSpawnArgs({ + args: ['--env=INLINE_TOKEN=inline-secret', '--env', 'SPLIT_TOKEN=split-secret'], + }), + ); + + const logged = getSpawnCommandLogCtx().args.join(' '); + + expect(logged).toContain('--env=INLINE_TOKEN='); + expect(logged).toContain('SPLIT_TOKEN='); + expect(logged).not.toContain('inline-secret'); + expect(logged).not.toContain('split-secret'); + }); + + it('redacts shell command strings in spawn debug logs', () => { + spawnAgent( + createMockWindow(), + buildSpawnArgs({ + command: '/bin/sh', + args: ['-c', 'codex exec "prompt containing private context"'], + dockerMode: false, + }), + ); + + const ctx = getSpawnCommandLogCtx(); + + expect(ctx.command).toBe('/bin/sh'); + expect(ctx.args).toEqual(['-c', '']); + }); + it('redirects credential mounts under /tmp inside the container', () => { const home = makeTempHome(['.ssh/', '.gitconfig', '.config/gh/']); vi.stubEnv('HOME', home); @@ -240,6 +311,82 @@ describe('spawnAgent docker mode', () => { expect(volumeFlags).toContain(`${home}/.gitconfig:${DOCKER_CONTAINER_HOME}/.gitconfig:ro`); expect(volumeFlags).toContain(`${home}/.config/gh:${DOCKER_CONTAINER_HOME}/.config/gh:ro`); }); + + describe('agent config dir mounts (shareDockerAgentAuth)', () => { + it.each([ + ['claude', '.claude'], + ['codex', '.codex'], + ['gemini', '.gemini'], + ['opencode', '.config/opencode'], + ['copilot', '.config/github-copilot'], + ])( + '%s bind-mounts a user-owned host directory when shareDockerAgentAuth is enabled', + (command, relDir) => { + const home = makeTempHome([]); + vi.stubEnv('HOME', home); + + spawnAgent(createMockWindow(), buildSpawnArgs({ command, shareDockerAgentAuth: true })); + + const volumeFlags = getFlagValues(getLastSpawnCall().args, '-v'); + const expectedHostDir = `${home}/.parallel-code/agent-auth/${command}/${relDir}`; + expect(volumeFlags).toContain(`${expectedHostDir}:${DOCKER_CONTAINER_HOME}/${relDir}`); + }, + ); + + it('creates the host auth directory so it is user-owned before mounting', () => { + const home = makeTempHome([]); + vi.stubEnv('HOME', home); + + spawnAgent( + createMockWindow(), + buildSpawnArgs({ command: 'claude', shareDockerAgentAuth: true }), + ); + + const hostDir = `${home}/.parallel-code/agent-auth/claude/.claude`; + expect(fs.existsSync(hostDir)).toBe(true); + }); + + it('bind-mounts .claude.json file for claude so auth persists across containers', () => { + const home = makeTempHome([]); + vi.stubEnv('HOME', home); + + spawnAgent( + createMockWindow(), + buildSpawnArgs({ command: 'claude', shareDockerAgentAuth: true }), + ); + + const volumeFlags = getFlagValues(getLastSpawnCall().args, '-v'); + const expectedHostFile = `${home}/.parallel-code/agent-auth/claude/.claude.json`; + expect(volumeFlags).toContain(`${expectedHostFile}:${DOCKER_CONTAINER_HOME}/.claude.json`); + expect(fs.readFileSync(expectedHostFile, 'utf8')).toBe('{}'); + }); + + it('does not mount agent auth directory when shareDockerAgentAuth is disabled', () => { + const home = makeTempHome([]); + vi.stubEnv('HOME', home); + + spawnAgent( + createMockWindow(), + buildSpawnArgs({ command: 'claude', shareDockerAgentAuth: false }), + ); + + const volumeFlags = getFlagValues(getLastSpawnCall().args, '-v'); + expect(volumeFlags.some((v) => v.includes('.parallel-code/agent-auth'))).toBe(false); + }); + + it('does not mount agent auth directory for an unknown agent command', () => { + const home = makeTempHome([]); + vi.stubEnv('HOME', home); + + spawnAgent( + createMockWindow(), + buildSpawnArgs({ command: 'unknown-agent', shareDockerAgentAuth: true }), + ); + + const volumeFlags = getFlagValues(getLastSpawnCall().args, '-v'); + expect(volumeFlags.some((v) => v.includes('.parallel-code/agent-auth'))).toBe(false); + }); + }); }); describe('validateCommand', () => { diff --git a/electron/ipc/pty.ts b/electron/ipc/pty.ts index 10194263..56bca6a9 100644 --- a/electron/ipc/pty.ts +++ b/electron/ipc/pty.ts @@ -64,6 +64,50 @@ const BATCH_INTERVAL = 8; // ms const TAIL_CAP = 8 * 1024; const MAX_LINES = 50; +function redactedSpawnArgs(command: string, args: string[]): string[] { + if ((command === '/bin/sh' || command.endsWith('/sh')) && args[0] === '-c') { + return ['-c', '']; + } + if (command === 'docker') { + return redactDockerArgs(args); + } + return args; +} + +function redactDockerArgs(args: string[]): string[] { + const redacted: string[] = []; + let redactNextEnv = false; + + for (const arg of args) { + if (redactNextEnv) { + redacted.push(redactEnvAssignment(arg)); + redactNextEnv = false; + continue; + } + + if (arg === '-e' || arg === '--env') { + redacted.push(arg); + redactNextEnv = true; + continue; + } + + if (arg.startsWith('--env=')) { + redacted.push(`--env=${redactEnvAssignment(arg.slice('--env='.length))}`); + continue; + } + + redacted.push(arg); + } + + return redacted; +} + +function redactEnvAssignment(value: string): string { + const eqIdx = value.indexOf('='); + if (eqIdx <= 0) return ''; + return `${value.slice(0, eqIdx)}=`; +} + /** Verify that a command exists in PATH. Throws a descriptive error if not found. */ export function validateCommand(command: string): void { if (!command || !command.trim()) { @@ -104,6 +148,7 @@ export function spawnAgent( isShell?: boolean; dockerMode?: boolean; dockerImage?: string; + shareDockerAgentAuth?: boolean; onOutput: { __CHANNEL_ID__: string }; }, ): void { @@ -220,7 +265,7 @@ export function spawnAgent( '-e', `HOME=${DOCKER_CONTAINER_HOME}`, // Mount SSH and git config read-only for git operations - ...buildDockerCredentialMounts(), + ...buildDockerCredentialMounts(args.command, args.shareDockerAgentAuth === true), image, command, ...args.args, @@ -230,6 +275,14 @@ export function spawnAgent( spawnArgs = args.args; } + logDebug('pty', `spawn command ${args.agentId}`, { + taskId: args.taskId, + command: spawnCommand, + args: redactedSpawnArgs(spawnCommand, spawnArgs), + cwd, + dockerMode: args.dockerMode === true, + }); + const proc = pty.spawn(spawnCommand, spawnArgs, { name: 'xterm-256color', cols: args.cols, @@ -564,7 +617,21 @@ function buildDockerEnvFlags(env: Record): string[] { return flags; } -function buildDockerCredentialMounts(): string[] { +// Config directories each agent CLI uses for auth/settings, relative to HOME. +const AGENT_CONFIG_DIRS: Record = { + claude: ['.claude'], + codex: ['.codex'], + gemini: ['.gemini'], + opencode: ['.config/opencode'], + copilot: ['.config/github-copilot'], +}; + +// Config files (not directories) each agent CLI uses for auth, relative to HOME. +const AGENT_CONFIG_FILES: Record = { + claude: ['.claude.json'], +}; + +function buildDockerCredentialMounts(agentCommand: string, shareAgentAuth: boolean): string[] { const mounts: string[] = []; const home = process.env.HOME; if (!home) return mounts; @@ -601,6 +668,38 @@ function buildDockerCredentialMounts(): string[] { mountIfExists(googleCredsFile, googleCredsFile); } + // When "Share agent auth across Linux containers" is enabled, bind-mount a + // host directory (created here, owned by the current user) into the agent's + // config location inside the container. Using a host directory avoids the + // root-ownership problem of Docker named volumes: the directory is created + // by this process (running as the user), so the containerised agent can + // write credentials on first login and read them on subsequent runs. + if (shareAgentAuth) { + const baseCommand = path.basename(agentCommand); + for (const relDir of AGENT_CONFIG_DIRS[baseCommand] ?? []) { + const hostDir = path.join(home, '.parallel-code', 'agent-auth', baseCommand, relDir); + try { + fs.mkdirSync(hostDir, { recursive: true, mode: 0o700 }); + mounts.push('-v', `${hostDir}:${DOCKER_CONTAINER_HOME}/${relDir}`); + } catch { + console.warn(`[docker-auth] Could not create host auth dir ${hostDir}, skipping mount`); + } + } + for (const relFile of AGENT_CONFIG_FILES[baseCommand] ?? []) { + const hostFile = path.join(home, '.parallel-code', 'agent-auth', baseCommand, relFile); + try { + const hostDir = path.dirname(hostFile); + fs.mkdirSync(hostDir, { recursive: true, mode: 0o700 }); + if (!fs.existsSync(hostFile) || fs.statSync(hostFile).size === 0) { + fs.writeFileSync(hostFile, '{}', { mode: 0o600 }); + } + mounts.push('-v', `${hostFile}:${DOCKER_CONTAINER_HOME}/${relFile}`); + } catch { + console.warn(`[docker-auth] Could not create host auth file ${hostFile}, skipping mount`); + } + } + } + return mounts; } diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index 1df97739..ddb17147 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -1,4 +1,5 @@ import { ipcMain, dialog, shell, app, clipboard, BrowserWindow, Notification } from 'electron'; +import crypto from 'crypto'; import fs from 'fs'; import os from 'os'; import { fileURLToPath } from 'url'; @@ -56,6 +57,7 @@ import { getBranchCommits, getCommitChangedFiles, getCommitDiffs, + getUncommittedFileDiffs, } from './git.js'; import { createTask, deleteTask } from './tasks.js'; import { listAgents } from './agents.js'; @@ -131,6 +133,65 @@ function getOptionalImageTag(value: unknown): string | undefined { return imageTag; } +/** First file URL on the clipboard, or null if none. + * macOS uses `public.file-url` (one URL per call). + * Linux file managers vary: + * - Files (Nautilus), Nemo, etc. publish `x-special/gnome-copied-files` + * as `\nfile:///path1\nfile:///path2`, where is `copy` + * or `cut`. This is the dominant Linux desktop format and MUST be + * checked before `text/uri-list` because some apps publish both + * flavours and the GNOME flavour is the authoritative one. + * - Falls back to `text/uri-list` (newline-separated) for KDE, Xfce, + * and any cross-desktop publisher that follows RFC 2483. */ +function readClipboardFileUrl(formats: string[]): string | null { + if (formats.includes('public.file-url')) { + const url = clipboard.read('public.file-url').trim(); + if (url) return url; + } + if (formats.includes('x-special/gnome-copied-files')) { + const payload = clipboard.read('x-special/gnome-copied-files'); + // First line is the verb (copy/cut); subsequent lines are file URLs. + const lines = payload.split('\n'); + for (let i = 1; i < lines.length; i++) { + const trimmed = lines[i].trim(); + if (trimmed.startsWith('file://')) return trimmed; + } + } + if (formats.includes('text/uri-list')) { + const list = clipboard.read('text/uri-list'); + for (const line of list.split('\n')) { + const trimmed = line.trim(); + if (trimmed && !trimmed.startsWith('#') && trimmed.startsWith('file://')) return trimmed; + } + } + return null; +} + +/** Convert a file:// URL to an absolute path, returning '' on failure. */ +function fileUrlToPath(url: string): string { + try { + return fileURLToPath(url); + } catch { + return ''; + } +} + +/** Strip path separators and clamp to a sane length so a renderer-supplied + * filename can't escape the temp dir. Falls back to a generic name when empty. + * Always appends a 6-char random suffix so two same-name drops landing in the + * same millisecond don't overwrite each other. */ +function sanitizeDroppedName(name: string): string { + const base = name + // eslint-disable-next-line no-control-regex -- intentional NUL strip for filesystem safety + .replace(/[\\/\x00]/g, '_') + .replace(/^\.+/, '') + .trim() + .slice(0, 200); + const stamp = `${Date.now()}-${crypto.randomBytes(3).toString('hex')}`; + if (base) return `parallel-code-drop-${stamp}-${base}`; + return `parallel-code-drop-${stamp}.png`; +} + /** * Create a leading+trailing throttled event forwarder. * Fires immediately, suppresses for `intervalMs`, then fires once more @@ -176,6 +237,7 @@ export function registerAllHandlers(win: BrowserWindow): void { assertInt(args.rows, 'rows'); assertOptionalBoolean(args.dockerMode, 'dockerMode'); assertOptionalString(args.dockerImage, 'dockerImage'); + assertOptionalBoolean(args.shareDockerAgentAuth, 'shareDockerAgentAuth'); assertOptionalBoolean(args.stepsEnabled, 'stepsEnabled'); if (args.cwd) validatePath(args.cwd, 'cwd'); if (!args.isShell && args.cwd) { @@ -411,6 +473,10 @@ export function registerAllHandlers(win: BrowserWindow): void { validateCommitHash(args.commitHash, 'commitHash'); return getCommitDiffs(args.worktreePath, args.commitHash); }); + ipcMain.handle(IPC.GetUncommittedFileDiffs, (_e, args) => { + validatePath(args.worktreePath, 'worktreePath'); + return getUncommittedFileDiffs(args.worktreePath); + }); ipcMain.handle(IPC.GetCoverageSummary, (_e, args) => { validatePath(args.repoRoot, 'repoRoot'); assertOptionalString(args.reportPath, 'reportPath'); @@ -644,19 +710,57 @@ export function registerAllHandlers(win: BrowserWindow): void { // --- Clipboard --- const clipboardImagePath = path.join(os.tmpdir(), 'parallel-code-clipboard.png'); - ipcMain.handle(IPC.SaveClipboardImage, async () => { + + // Resolve the most useful representation of the current clipboard contents + // for pasting into a terminal. Order of preference: + // 1. file references (Finder copy, Nautilus copy, etc.) → return absolute path + // 2. raster image (screenshot, image-app copy) → save PNG to tmp + return path + // 3. plain text → return as-is + // + // Without (1), copying an image file from Finder gives only the basename via + // navigator.clipboard.readText(), which is useless to a CLI agent that needs a + // path it can stat. macOS exposes file copies via the 'public.file-url' format, + // Linux via 'text/uri-list'. Windows is not a published target. + ipcMain.handle(IPC.ResolveClipboardPaste, async () => { try { + const formats = clipboard.availableFormats(); + const fileUrl = readClipboardFileUrl(formats); + if (fileUrl) { + const filePath = fileUrlToPath(fileUrl); + if (filePath) return { kind: 'file', path: filePath }; + } const img = clipboard.readImage(); - if (img.isEmpty()) return null; - const buf = img.toPNG(); - await fs.promises.writeFile(clipboardImagePath, buf); - return clipboardImagePath; + if (!img.isEmpty()) { + const buf = img.toPNG(); + await fs.promises.writeFile(clipboardImagePath, buf); + return { kind: 'image', path: clipboardImagePath }; + } + const text = clipboard.readText(); + if (text) return { kind: 'text', text }; + return { kind: 'empty' }; } catch (e) { - console.error('[clipboard] Failed to save clipboard image:', e); - return null; + console.error('[clipboard] resolveClipboardPaste failed:', e); + return { kind: 'empty' }; } }); + // Save image bytes that were dropped from a source without a filesystem path + // (e.g. dragged from a browser). The renderer reads the dropped File as + // an ArrayBuffer, base64-encodes it, and forwards it here so the CLI agent + // can read the result. We require base64 (not Uint8Array / ArrayBuffer) + // because the renderer's invoke() wrapper does a JSON.parse(JSON.stringify(args)) + // round-trip that destroys typed arrays. + ipcMain.handle(IPC.SaveDroppedImage, async (_e, args) => { + if (!args || typeof args !== 'object') throw new Error('invalid args'); + const { name, data } = args as { name?: unknown; data?: unknown }; + if (typeof data !== 'string') throw new Error('data must be a base64 string'); + const buf = Buffer.from(data, 'base64'); + const safeName = sanitizeDroppedName(typeof name === 'string' ? name : ''); + const filePath = path.join(os.tmpdir(), safeName); + await fs.promises.writeFile(filePath, buf); + return filePath; + }); + // --- System --- ipcMain.handle(IPC.GetSystemFonts, () => getSystemMonospaceFonts()); diff --git a/electron/ipc/trace.ts b/electron/ipc/trace.ts index b570a892..7a68b161 100644 --- a/electron/ipc/trace.ts +++ b/electron/ipc/trace.ts @@ -37,7 +37,8 @@ const NEVER_SAFE: ReadonlySet = new Set([ IPC.DialogOpen, IPC.OpenPath, IPC.ReadFileText, - IPC.SaveClipboardImage, + IPC.ResolveClipboardPaste, + IPC.SaveDroppedImage, IPC.CreateArenaWorktree, IPC.RemoveArenaWorktree, IPC.CheckPathExists, diff --git a/electron/log.ts b/electron/log.ts index 5f8fce67..d3e45ffe 100644 --- a/electron/log.ts +++ b/electron/log.ts @@ -37,6 +37,17 @@ let minLevel: LogLevel = isProd ? 'warn' : 'debug'; let inLogger = false; +// `console.*` writes to process.stdout/stderr asynchronously; if the parent +// pipe closes mid-shutdown (e.g. `concurrently` SIGTERMs us after vite dies), +// the resulting EPIPE surfaces as an *async* 'error' event the per-call +// try/catch in writeConsole can't catch. Swallow EPIPE here so a routine +// teardown doesn't become an Uncaught Exception. Re-throw anything else. +const swallowEpipe = (err: NodeJS.ErrnoException): void => { + if (err.code !== 'EPIPE') throw err; +}; +process.stdout.on('error', swallowEpipe); +process.stderr.on('error', swallowEpipe); + export function setMinLevel(level: LogLevel): void { minLevel = level; } diff --git a/electron/preload.cjs b/electron/preload.cjs index dfd905d6..3bef0c03 100644 --- a/electron/preload.cjs +++ b/electron/preload.cjs @@ -1,4 +1,4 @@ -const { contextBridge, ipcRenderer, webFrame } = require('electron'); +const { contextBridge, ipcRenderer, webFrame, webUtils } = require('electron'); // Allowlist of valid IPC channels. // IMPORTANT: This list MUST stay in sync with the IPC enum in electron/ipc/channels.ts. @@ -108,7 +108,8 @@ const ALLOWED_CHANNELS = new Set([ 'open_path', 'read_file_text', // Clipboard - 'save_clipboard_image', + 'resolve_clipboard_paste', + 'save_dropped_image', // Notifications 'show_notification', 'notification_clicked', @@ -142,4 +143,14 @@ contextBridge.exposeInMainWorld('electron', { }, }, setZoomFactor: (factor) => webFrame.setZoomFactor(factor), + // Returns the absolute filesystem path for a File obtained from a drop event + // (or any DataTransfer / input[type=file]). Returns '' for File objects that + // have no backing path (e.g. images dragged from a browser tab). + getPathForFile: (file) => { + try { + return webUtils.getPathForFile(file) || ''; + } catch { + return ''; + } + }, }); diff --git a/openspec/changes/archive/2026-04-28-support-paste-images/design.md b/openspec/changes/archive/2026-04-28-support-paste-images/design.md new file mode 100644 index 00000000..f722339f --- /dev/null +++ b/openspec/changes/archive/2026-04-28-support-paste-images/design.md @@ -0,0 +1,203 @@ +# Design — Support Paste-Image into Terminal + +## Why a design doc + +The user-facing behaviour ("dropping or pasting an image types its absolute +path into the terminal") is one sentence; the spec covers the visible +contract. Three things need a written design because they are non-obvious +choices that future contributors will be tempted to "fix" the wrong way: + +1. Why clipboard resolution lives in the main process instead of the + renderer. +2. Why the drop handler runs in DOM capture phase rather than bubble. +3. Why dropped binary payloads cross the IPC boundary as base64 strings + instead of `Uint8Array` / `ArrayBuffer`. + +Plus a fourth: why `escapePath` uses backslash escaping and not +single-quote wrapping or any per-agent convention. + +## 1. Clipboard resolution lives in the main process + +The renderer has access to `navigator.clipboard.readText()` and +`navigator.clipboard.read()`. They are insufficient for our use case: + +- `readText()` on macOS returns the basename when Finder copied a file + (Finder writes the basename to the `text/plain` flavour of the + clipboard alongside the file URL). The renderer cannot tell that the + basename belongs to a real file on disk without also walking + `clipboard.read()` for the `Files` flavour, which is gated by + permissions, only available in secure contexts, and does not return + paths. +- The Electron main process can read OS-specific clipboard flavours via + `clipboard.read('public.file-url')` (macOS) and + `clipboard.read('text/uri-list')` (Linux) and call `fileURLToPath` to + recover the absolute path. + +So the resolution must live in main. A single IPC, `ResolveClipboardPaste`, +returns a tagged-union response (`{ kind: 'file' | 'image' | 'text' | 'empty', … }`) +that encodes the priority order: + +``` +1. public.file-url (macOS Finder copy) + or text/uri-list (Linux file managers) + → return { kind: 'file', path } +2. clipboard.readImage() + → save PNG to temp + return { kind: 'image', path } +3. clipboard.readText() + → return { kind: 'text', text } +4. nothing + → return { kind: 'empty' } +``` + +The renderer's Cmd+V handler is a `switch` on `kind`. One round-trip, +no `readText`-then-`readImage` race, no opportunity to insert a +basename when a file URL was available the whole time. + +`SaveClipboardImage` (the legacy single-purpose handler) becomes dead +code as soon as the renderer migrates. It is removed in this change to +keep the IPC surface tight and to prevent future callers re-introducing +the basename trap by accident. + +## 2. Drop handler runs in DOM capture phase + +xterm's element subscribes to `drop` and `dragover` itself. Its default +handler reads `dataTransfer.getData('text/plain')`, which the OS +populates with the basename when files are dragged from a file manager. +If the application's drop handler runs in the bubble phase (the default +for `addEventListener` and JSX `on:drop`), xterm's handler runs first and +inserts the basename before our handler ever runs. + +The fix is to register the listener in the DOM capture phase: + +```ts +containerRef.addEventListener('dragover', handleDragOver, true); // capture +containerRef.addEventListener('drop', handleDrop, true); +``` + +Plus `e.preventDefault()` and `e.stopPropagation()` inside the handler. +`stopPropagation` short-circuits the bubble phase entirely; xterm's +listeners never fire. The capture-phase registration is what makes the +two `preventDefault()` calls sufficient — without capture, xterm +sometimes runs synchronously before the bubble phase even starts. + +The handler also calls `term.focus()` before `enqueueInput()` so the +terminal is the active element when the typed input arrives. Without it, +dropping onto a non-focused terminal types into the previously-focused +field (which can be a text input elsewhere in the UI). + +## 3. Binary payloads cross IPC as base64 + +The repo's renderer-side `invoke()` wrapper does this: + +```ts +const safeArgs = args ? (JSON.parse(JSON.stringify(args)) as …) : undefined; +return window.electron.ipcRenderer.invoke(cmd, safeArgs); +``` + +The JSON round-trip exists to make `Channel` tokens work +(`Channel.toJSON` produces a plain `{__CHANNEL_ID__}` object). It also +silently destroys typed arrays: `JSON.stringify(new Uint8Array([137,80,78]))` +produces `'{"0":137,"1":80,"2":78}'`. The receiving handler's +`instanceof Uint8Array` check then fails and throws `data must be +ArrayBuffer or Uint8Array`. Browser-image drops therefore look like +they "do nothing" with no surfaced error. + +Two ways out: + +- **A. Base64 the payload in the renderer**, decode in the main process + via `Buffer.from(data, 'base64')`. Stays inside the JSON envelope; no + invasive change to the existing IPC wrapper; works through the + contextBridge as a plain string. +- **B. Bypass the JSON round-trip** for known-binary channels. Smaller + wire payload (~33 % overhead saved) but requires either a parallel + `invokeBinary()` API or a per-channel allowlist inside `invoke()`, + and risks mis-handling `Channel` tokens that legitimately appear in + the payload. + +We pick **A**. The blast radius is one channel and one helper. The +`SaveDroppedImage` handler accepts `{ name: string, data: string /* +base64 */ }` and reconstructs a `Buffer`. The 50 MB renderer-side cap +on dropped item size makes the base64 overhead negligible in practice. + +## 4. `escapePath` uses backslash escaping + +The path is going to one of two places: + +- A POSIX shell (when the terminal hosts `bash` / `zsh` directly). +- An agent's prompt parser (when the terminal hosts `claude`, `codex`, + etc.). + +Backslash escaping works in both: + +- A real shell parses `\` as a literal char, so + `My\ File.png` becomes the single argv `My File.png`. +- An agent reads the prompt as text. The backslashes are visible but + every popular agent's path parser strips them. The output also matches + what the user would see if they dragged the same file into a native + macOS Terminal session — least surprise. + +Single-quote wrapping was the first cut and was rejected for two +reasons: it produces ugly `'…'\''…'` quote walls when paths contain +apostrophes, and it does not match any native terminal's drag-insert +convention. + +The metaset escaped is intentionally wide: + +``` +[whitespace ' " \ $ ` ! ( ) ; & | < > * ? [ ] { } ~ #] +``` + +Filenames legitimately contain `&` `(` `)` etc., and a too-narrow set +will break in real shells (`ls foo&bar.png` backgrounds `ls foo`). +Backslash before a normal letter is a no-op in bash, so over-escaping +is harmless; under-escaping silently corrupts. + +## 5. Resolved payloads go through `term.paste()`, not the PTY directly + +The first cut wrote the resolved path straight to the PTY via +`enqueueInput`. The path arrived at the agent — but agents like Claude +Code do not act on a path that arrives byte-by-byte the same way they +act on a path that arrives inside a paste. CC enables bracketed-paste +mode (`\x1b[?2004h`) on startup; the terminal is then expected to +wrap pasted content in `\x1b[200~ … \x1b[201~`. CC's input parser +inspects the wrapped payload, recognises image-file paths, and +attaches them as `[Image #N]`. Direct PTY writes carry no such +wrapper, so CC sees literal typing and skips the attachment. + +The fix is to deliver paste / drop payloads via xterm's +`term.paste(data)`. xterm reads the agent's current bracketed-paste +setting and either emits the markers or not, automatically. The +`onData` event still fires (the existing PTY forwarding pipeline +keeps working), and the spec keeps the rule observable: when +bracketed-paste mode is on, the PTY sees the marker bytes. + +A second-order benefit: multi-line text pastes (e.g. a snippet +copied from a doc) now arrive as a single bracketed paste instead +of N separate "typed line" events, which is what shells and agents +expect for proper history and indentation handling. + +## Why the legacy SaveClipboardImage handler is removed + +Keeping it as a "backward compatibility" shim invites future callers to +reach for the simple `→ image path` API and re-create the basename +trap. The handler has exactly one historical caller (the old paste +flow), and that caller migrates in this change. Removing it now is +cheaper than removing it later. + +## Why no UX feedback when a drop is over the size cap or fails + +`dataTransferToShellArgs` silently filters out files that fail to +resolve (over the 50 MB cap, `getPathForFile` returns `''` and there +are no bytes to fall back on, etc.). The user-visible result of +"dropped a 200 MB video and nothing happened" is acceptable for the +v1 of this feature; a toast or status-bar notice is intentionally out +of scope. If real users hit it, it earns its own change. + +## Capture-phase listener teardown + +The two `addEventListener('…', …, true)` registrations are paired with +`removeEventListener('…', …, true)` inside `onCleanup`. The third +argument MUST match between add and remove or the listener leaks. +This is the kind of thing that breaks silently and only shows up +under HMR / repeated mount-unmount; the spec calls it out explicitly +to keep it from regressing. diff --git a/openspec/changes/archive/2026-04-28-support-paste-images/proposal.md b/openspec/changes/archive/2026-04-28-support-paste-images/proposal.md new file mode 100644 index 00000000..da96ace9 --- /dev/null +++ b/openspec/changes/archive/2026-04-28-support-paste-images/proposal.md @@ -0,0 +1,84 @@ +# Support Paste-Image into Terminal + +## Why + +Pasting or dragging an image into a terminal session (typically driving an +agent like Claude Code) used to insert only the file's basename. Agents then +either errored out (`No such file: foo.png`) or fell back to literal text +about a filename, which was useless. The user reported the regression against +v1.2.1 with: "I expected the behavior that Claude Code supports." + +Two underlying causes: + +1. The terminal had no `drop` handler. xterm fell back to the dragged item's + `text/plain` payload, which the OS populates with the basename only. +2. The Cmd+V handler called `navigator.clipboard.readText()` first. On + macOS, copying an image file in Finder puts the basename on the + clipboard as `text/plain`, so the existing image-fallback never ran — + the user got the same useless basename. + +The user-visible promise we want to make is the same one the macOS Terminal +makes: dragging or pasting a file into the terminal types its absolute path, +and pasting raw image bytes (e.g. a screenshot) saves the bytes to a temp +file and types that path. The receiving agent then reads the file from disk +exactly as if the user had typed the path themselves. + +## What changes + +- New IPC `ResolveClipboardPaste`: a single main-process call that picks the + most useful clipboard representation in priority order (file URL → image + → plain text → empty), so the renderer no longer has to ask twice and + no longer falls into the basename trap. +- New IPC `SaveDroppedImage`: receives bytes for a dropped item that has no + filesystem path (e.g. an `` dragged from a browser tab), writes them + to a sanitized temp file, and returns the absolute path. +- `webUtils.getPathForFile` exposed as `window.electron.getPathForFile` + through the existing preload bridge — the contextBridge-safe replacement + for the `File.path` field that Electron 32+ no longer ships. +- `TerminalView` registers capture-phase `dragover` / `drop` listeners on + its container so xterm's own listeners cannot win and insert the + basename. Multiple dropped files are resolved to absolute paths and + inserted as a space-joined, backslash-escaped string. +- New helper `src/lib/terminalDrop.ts` exporting `escapePath(p)` (was + `shellQuote` in the first cut) and `dataTransferToShellArgs(dt)`. + Backslash-escape is the single quoting rule across all targets — it + matches macOS Terminal / iTerm2 / VS Code muscle memory and renders + cleanly inside agent prompts. +- Cmd+V binding in `TerminalView` switches from the two-call + (`readText` → `SaveClipboardImage`) flow to the single-call + `ResolveClipboardPaste` flow and quotes file paths with `escapePath`. +- Drop payloads cross the contextBridge IPC boundary as base64-encoded + strings. The naive `Uint8Array` payload is destroyed by the existing + `JSON.parse(JSON.stringify(args))` round-trip in `src/lib/ipc.ts`, + silently breaking browser-image drops; base64 keeps the envelope + JSON-clean and is decoded back to a `Buffer` in the main process. +- The legacy `SaveClipboardImage` IPC and its main-process handler are + removed once `ResolveClipboardPaste` is shipping; nothing else calls it. + +## Impact + +- **New capability:** `terminal-image-paste`. No prior spec — the whole + capability is `## ADDED Requirements`. +- **IPC surface:** +`ResolveClipboardPaste`, +`SaveDroppedImage`, + −`SaveClipboardImage`. Preload allowlist, IPC enum, and `trace.ts` + `NEVER_SAFE` all updated. +- **Preload bridge:** new `getPathForFile` function exposed through + `contextBridge`, backed by `webUtils.getPathForFile`. No `nodeIntegration` + change — preload remains the only privileged surface. +- **Renderer:** new `src/lib/terminalDrop.ts`; `TerminalView.tsx` gains + capture-phase drop listeners and uses the new IPCs. No changes to the + PTY / output pipeline. +- **Type surface:** `Window['electron']` gains `getPathForFile(file: File): string`. +- **Persistence / state:** none. Dropped/pasted bytes are written to + `os.tmpdir()` with a `parallel-code-drop--` + prefix; lifetime tracking is intentionally not specified — the OS + cleans the temp dir on its own schedule. +- **Platforms:** macOS and Linux only, per project scope. Clipboard + format detection covers `public.file-url` (macOS) and `text/uri-list` + (Linux); no Windows clipboard reader. +- **Out of scope:** Windows support (clipboard format `FileNameW`, + Windows-reserved character sanitization, double-quote vs backslash + trade-off); per-agent insertion conventions (e.g. `@path` for Claude + Code, markdown image links); paste/drop into UI surfaces other than + the terminal (e.g. the new-task dialog has its own GitHub-URL drop + handler covered elsewhere). diff --git a/openspec/changes/archive/2026-04-28-support-paste-images/specs/terminal-image-paste/spec.md b/openspec/changes/archive/2026-04-28-support-paste-images/specs/terminal-image-paste/spec.md new file mode 100644 index 00000000..7abe15d6 --- /dev/null +++ b/openspec/changes/archive/2026-04-28-support-paste-images/specs/terminal-image-paste/spec.md @@ -0,0 +1,309 @@ +# Terminal Image Paste Specification + +## ADDED Requirements + +### Requirement: Cmd+V resolves the clipboard to its most useful representation + +The system SHALL resolve the clipboard to a single representation chosen +by priority — a filesystem path the agent can read, an image saved to a +temp file, or plain text — and never insert a bare basename when the +underlying file URL was available. Resolution happens when the user +invokes the paste keybinding while the terminal has focus. + +#### Scenario: Finder-copied image file pastes its absolute path + +- **WHEN** the user copies an image file in macOS Finder and presses + Cmd+V in the terminal +- **THEN** the absolute path to that file is typed into the terminal +- **AND** the typed value is escaped per the path-escaping requirement +- **AND** the basename alone is never inserted + +#### Scenario: Linux GNOME file-manager copy pastes its absolute path + +- **WHEN** the user copies a file in a GNOME-family file manager + (Nautilus, Nemo, Caja) and presses the paste binding +- **THEN** the system reads the `x-special/gnome-copied-files` clipboard + flavour, skips the leading `copy` / `cut` verb line, takes the first + remaining `file://` URL, converts it to an absolute path, and types + the escaped path +- **AND** the GNOME flavour is checked before `text/uri-list` so it + wins when both flavours are present + +#### Scenario: Other Linux file managers fall back to text/uri-list + +- **WHEN** the user copies a file from a non-GNOME Linux file manager + (KDE Dolphin, Xfce Thunar, etc.) that publishes only the cross-desktop + RFC 2483 format +- **THEN** the system reads the `text/uri-list` clipboard flavour, takes + the first non-comment `file://` URL, converts it to an absolute path, + and types the escaped path + +#### Scenario: Raw image on the clipboard is saved and pasted as a path + +- **WHEN** the clipboard does not contain a file URL but does contain + raster image bytes (e.g. a screenshot) +- **THEN** the system writes the image as PNG to the OS temp directory +- **AND** types the escaped absolute path of that file + +#### Scenario: Plain text on the clipboard pastes as text + +- **WHEN** the clipboard contains neither a file URL nor a raster image + but does contain plain text +- **THEN** that text is typed verbatim, with no quoting and no escaping + +#### Scenario: Empty clipboard does nothing + +- **WHEN** the clipboard contains no file, no image, and no text +- **THEN** no input is sent to the terminal +- **AND** the keypress is consumed (the keybinding's `preventDefault` + still runs so xterm does not insert a fallback) + +#### Scenario: Resolution happens in the main process via a single IPC + +- **WHEN** the renderer needs to resolve a paste +- **THEN** it makes exactly one IPC call (`ResolveClipboardPaste`) and + switches on the returned `kind` +- **AND** it does NOT call `navigator.clipboard.readText()` first + +### Requirement: File drop on the terminal types absolute path(s) + +The system SHALL type each dropped file's absolute path into the +terminal — space-joined and escaped — instead of allowing the browser +default to insert the basename. This applies whenever the user drops +one or more files onto the terminal viewport. + +#### Scenario: Single file dropped from the OS file manager + +- **WHEN** the user drags one file from Finder / Nautilus and drops it + on the terminal +- **THEN** the file's absolute path is typed into the terminal, escaped + per the path-escaping requirement +- **AND** the basename alone is never inserted + +#### Scenario: Multiple files dropped at once + +- **WHEN** the user drops two or more files in a single drop +- **THEN** every file's escaped path is typed +- **AND** the paths are joined with a single ASCII space, in the order + the OS reported them +- **AND** no trailing space is appended + +#### Scenario: Drop target is the terminal even when not focused + +- **WHEN** the user drops a file on the terminal while focus is in a + different element (e.g. the prompt input or sidebar) +- **THEN** the terminal is focused before the path is typed +- **AND** the resulting input arrives at the terminal, not at the + previously-focused element + +#### Scenario: Drop handler runs in DOM capture phase + +- **WHEN** a `drop` event fires on the terminal container +- **THEN** the application's handler runs in the capture phase, calls + `preventDefault()` and `stopPropagation()`, and xterm's own bubble + phase listener does not run + +#### Scenario: Capture-phase listeners are torn down on unmount + +- **WHEN** a `TerminalView` unmounts +- **THEN** the application's `dragover` and `drop` listeners are removed + with the same `{ capture: true }` setting they were registered with +- **AND** subsequent drops on the unmounted node do nothing + +#### Scenario: Drop without files is ignored + +- **WHEN** a `drop` fires on the terminal but `dataTransfer.files` is + empty (e.g. text-only drag, GitHub URL drag) +- **THEN** the application's terminal drop handler returns without + preventing default and without typing anything +- **AND** the surrounding application's URL-drop handler may still + process the drop normally + +### Requirement: Browser-origin items without a path are persisted to a temp file + +The system SHALL read a dropped item's bytes and persist them to the OS +temp directory, then type that temp path, whenever the item has no +backing filesystem path (e.g. an `` dragged from a browser tab, an +item from a virtual file system). + +#### Scenario: Browser image drop produces a usable temp path + +- **WHEN** the user drags an `` element from a browser into the + terminal +- **AND** `webUtils.getPathForFile(file)` returns the empty string +- **THEN** the renderer reads the file as an `ArrayBuffer`, + base64-encodes it, and calls `SaveDroppedImage` with `{ name, data }` +- **AND** the main process decodes the base64 to a `Buffer` and writes + it to the OS temp directory +- **AND** the returned absolute path is typed into the terminal, + escaped per the path-escaping requirement + +#### Scenario: Filename for the temp file is sanitized + +- **WHEN** the renderer-supplied filename contains path separators (`/`, + `\`), a NUL byte, leading dots, or extra surrounding whitespace +- **THEN** the main process strips those characters / runs before + writing +- **AND** clamps the remaining basename to 200 characters +- **AND** prefixes the basename with + `parallel-code-drop--<6-hex>-` where `<6-hex>` is a fresh + random suffix per call so two same-name drops landing in the same + millisecond never collide on disk +- **AND** when the sanitized basename is empty, falls back to + `parallel-code-drop--<6-hex>.png` + +#### Scenario: Renderer caps oversized drops without surfacing an error + +- **WHEN** the dropped item is larger than 50 MB +- **THEN** the renderer skips that item without sending it across IPC +- **AND** continues to process other items in the same drop + +#### Scenario: One failing item does not cancel the rest of the drop + +- **WHEN** a mixed drop contains both items that resolve successfully + (path-backed Files or path-less items whose bytes save cleanly) + and items whose resolution throws (oversized, `arrayBuffer()` rejects, + `SaveDroppedImage` rejects, base64 encoding fails) +- **THEN** the failing items are silently filtered out +- **AND** the successfully-resolved items are still typed into the + terminal as a space-joined escaped string +- **AND** no error is surfaced through the drop handler's catch + block (the failure is local to the failing item) + +#### Scenario: Binary payload survives the IPC envelope + +- **WHEN** binary bytes are sent over `SaveDroppedImage` +- **THEN** they are encoded as base64 in the renderer and decoded with + `Buffer.from(data, 'base64')` in the main process +- **AND** the file written to disk has the exact byte length of the + source payload +- **AND** they are NOT sent as `Uint8Array` or `ArrayBuffer` (the + application's `invoke()` wrapper destroys typed arrays via its + `JSON.parse(JSON.stringify(args))` round-trip) + +### Requirement: Paths are escaped with backslash before insertion + +Paths typed into the terminal SHALL be backslash-escaped before +insertion so they round-trip correctly through both POSIX shells (bash, +zsh) and CLI agents that parse paths from prompt text. + +#### Scenario: Safe paths pass through unchanged + +- **WHEN** a path contains only characters from + `[A-Za-z0-9_./@:+,%=-]` +- **THEN** it is typed verbatim with no escape characters added + +#### Scenario: Whitespace is escaped + +- **WHEN** a path contains a space or any other whitespace character +- **THEN** each whitespace character is preceded by a single backslash + +#### Scenario: Shell metacharacters are escaped + +- **WHEN** a path contains any of the characters + `' " \ $ \` ! ( ) ; & | < > \* ? [ ] { } ~ #` +- **THEN** each such character is preceded by a single backslash + +#### Scenario: Empty path renders as empty bash literal + +- **WHEN** a path is the empty string +- **THEN** it renders as `""` (two ASCII double quotes) so a real shell + receives an explicit empty argument rather than dropping the position + +#### Scenario: Multiple paths are space-joined after escaping + +- **WHEN** more than one resolved path is being inserted in one drop +- **THEN** each path is escaped individually +- **AND** the escaped paths are joined with a single ASCII space + +### Requirement: Resolved paste/drop content flows through xterm's paste pipeline + +The system SHALL deliver resolved paste and drop payloads (file paths, +image temp paths, plain text) to the agent through `term.paste()`, +never via a direct PTY write. This causes xterm to wrap the payload +with bracketed-paste markers (`\x1b[200~` … `\x1b[201~`) when the agent +has bracketed-paste mode enabled, which CLI agents like Claude Code use +to distinguish "the user pasted this" from "the user typed this +character by character" — the former is what triggers automatic +image-file recognition and attachment. + +#### Scenario: Cmd+V file path uses term.paste + +- **WHEN** the paste handler resolves the clipboard to `kind: 'file'` + or `kind: 'image'` +- **THEN** the escaped path is delivered via `term.paste(path)` +- **AND** is NOT delivered via direct PTY write + +#### Scenario: Cmd+V plain text uses term.paste + +- **WHEN** the paste handler resolves the clipboard to `kind: 'text'` +- **THEN** the text is delivered via `term.paste(text)` so a paste of + multiple lines into a bracketed-paste-aware shell is treated as a + single paste rather than a sequence of typed lines + +#### Scenario: Drop payload uses term.paste + +- **WHEN** the drop handler has resolved the dropped files into a + space-joined escaped path string +- **THEN** the string is delivered via `term.paste(args)` after + `term.focus()` + +#### Scenario: Bracketed-paste markers appear when the agent enables them + +- **WHEN** the agent has previously sent the bracketed-paste-mode + enable sequence (`\x1b[?2004h`) +- **THEN** the bytes the PTY actually receives for a paste/drop + delivery start with `\x1b[200~` and end with `\x1b[201~` +- **AND** when bracketed-paste mode is disabled (or never enabled), + the bytes the PTY receives are the payload alone with no marker + bytes + +### Requirement: IPC contract for clipboard and drop + +The system SHALL communicate clipboard and drop intents through dedicated +IPC channels declared in `electron/ipc/channels.ts` and allowlisted in +the preload bridge. + +#### Scenario: ResolveClipboardPaste channel exists + +- **WHEN** the renderer invokes `IPC.ResolveClipboardPaste` +- **THEN** the main process returns a tagged-union object with one of + the shapes `{ kind: 'file', path: string }`, + `{ kind: 'image', path: string }`, `{ kind: 'text', text: string }`, + or `{ kind: 'empty' }` +- **AND** no other shape is ever returned + +#### Scenario: SaveDroppedImage channel exists + +- **WHEN** the renderer invokes `IPC.SaveDroppedImage` with + `{ name: string, data: string /* base64 */ }` +- **THEN** the main process returns the absolute path of the file it + wrote +- **AND** rejects payloads whose `data` is not a string + +#### Scenario: getPathForFile is exposed via the preload bridge + +- **WHEN** the renderer calls `window.electron.getPathForFile(file)` + with a `File` object obtained from a drop event +- **THEN** the function returns the absolute filesystem path for files + that have one +- **AND** returns the empty string for files that do not (browser-origin + items, virtual file system items, etc.) +- **AND** returns the empty string instead of throwing on any internal + error from `webUtils.getPathForFile` + +#### Scenario: Both new channels are flagged NEVER_SAFE for tracing + +- **WHEN** the IPC trace module initialises +- **THEN** both `ResolveClipboardPaste` and `SaveDroppedImage` are + members of `NEVER_SAFE` +- **AND** their argument and return payloads are never written to the + debug log + +#### Scenario: The legacy SaveClipboardImage channel is removed + +- **WHEN** this change is shipped +- **THEN** the IPC enum no longer contains `SaveClipboardImage` +- **AND** the preload allowlist no longer contains + `'save_clipboard_image'` +- **AND** no main-process handler for the legacy channel is registered diff --git a/openspec/changes/archive/2026-04-28-support-paste-images/tasks.md b/openspec/changes/archive/2026-04-28-support-paste-images/tasks.md new file mode 100644 index 00000000..5a261f5a --- /dev/null +++ b/openspec/changes/archive/2026-04-28-support-paste-images/tasks.md @@ -0,0 +1,83 @@ +# Tasks — Support Paste-Image into Terminal + +Most of the code is already on the `feature/support-paste-images` branch. +The unchecked items are the gaps the spec exposed during retro-review. + +## Already implemented + +- [x] Add `IPC.ResolveClipboardPaste` and `IPC.SaveDroppedImage` to + `electron/ipc/channels.ts`. +- [x] Add the same channels to the preload allowlist in + `electron/preload.cjs`. +- [x] Add both channels to `NEVER_SAFE` in `electron/ipc/trace.ts` so + paths and bytes are never logged. +- [x] Implement `ResolveClipboardPaste` in `electron/ipc/register.ts`: + priority order file URL → image → text → empty; macOS reads + `public.file-url`, Linux reads `text/uri-list`; image fallback + writes a PNG to `os.tmpdir()`. +- [x] Implement `SaveDroppedImage` in `electron/ipc/register.ts`: + sanitize the supplied filename, write to `os.tmpdir()` with the + `parallel-code-drop--` prefix, return the + absolute path. +- [x] Expose `webUtils.getPathForFile(file)` from `electron/preload.cjs` + as `window.electron.getPathForFile`; update the `Window['electron']` + type in `src/lib/ipc.ts`. +- [x] Create `src/lib/terminalDrop.ts` with `shellQuote` (initial cut + using single-quote wrap) and `dataTransferToShellArgs`. +- [x] Add capture-phase `dragover` and `drop` listeners on + `TerminalView`'s container; teardown in `onCleanup`. +- [x] Migrate the Cmd+V handler in `TerminalView` to use + `ResolveClipboardPaste` and quote file paths through `shellQuote`. +- [x] Unit tests for `shellQuote` in `src/lib/terminalDrop.test.ts`. +- [x] `npx tsc --noEmit` (renderer + electron tsconfigs) and + `npx vitest run` pass. + +## Remaining + +- [x] Rewrite `shellQuote` in `src/lib/terminalDrop.ts` to + `escapePath`: backslash-escape the metaset + `[whitespace ' " \ $ \` ! ( ) ; & | < > \* ? [ ] { } ~ #]`. The +empty string becomes `""` (literal empty argv). Update callers +(`TerminalView`paste handler,`dataTransferToShellArgs`). +- [x] Update `src/lib/terminalDrop.test.ts` to cover the new escape + rule: empty path → `""`; safe path passes through; spaces escape; + embedded apostrophes escape; embedded `$` `` ` `` `(` `)` `&` + escape; mixed-meta path escapes every metachar; backslash in path + itself escapes. +- [x] Fix the binary-IPC bug: change `dataTransferToShellArgs` to + base64-encode the dropped bytes before calling `SaveDroppedImage`, + and update `SaveDroppedImage` to accept `data: string` and decode + via `Buffer.from(data, 'base64')`. Strengthen the main-process + validation accordingly. +- [x] Add a renderer-side regression test (vitest, no DOM) that exercises + `dataTransferToShellArgs` end-to-end with a stubbed + `window.electron.invoke` / `getPathForFile`, asserting the IPC is + called with a base64 string for a path-less File and the resulting + command line is correctly escaped. +- [x] Delete the legacy `SaveClipboardImage` flow: remove the + `IPC.SaveClipboardImage` enum entry, the `'save_clipboard_image'` + preload allowlist line, the entry in `trace.ts NEVER_SAFE`, and + the handler implementation in `electron/ipc/register.ts`. The + `clipboardImagePath` constant moves into the + `ResolveClipboardPaste` handler closure. +- [x] Codex review follow-up: parse `x-special/gnome-copied-files` + ahead of `text/uri-list` in `readClipboardFileUrl` so GNOME-family + file managers (Files / Nautilus, Nemo, Caja) get the absolute + path treatment they advertise rather than the basename fallback. +- [x] Codex review follow-up: wrap each per-file resolution in + `pathForDroppedItem` in its own try/catch so one unreadable + browser/virtual-file in a mixed drop never cancels the whole + `Promise.all` and silently drops the resolvable siblings. +- [x] Codex review follow-up: append a 6-char `crypto.randomBytes(3)` + suffix to the `parallel-code-drop-…` temp filename so two + same-name drops landing inside the same millisecond don't + overwrite each other on disk. +- [x] Switch paste/drop delivery from `enqueueInput` (direct PTY write) to + `term.paste()` so xterm emits bracketed-paste markers + (`\x1b[200~ … \x1b[201~`) when the agent has bracketed-paste mode + on. Without this, CLI agents like Claude Code see a dropped path + as literal typed text and skip the file-attachment recognition + that turns the path into an `[Image #N]` reference. +- [x] Validate with `npm run typecheck`, `npm test`, + `npm run format:check`, `npm run lint`, and + `openspec validate --all --strict`. diff --git a/openspec/specs/terminal-image-paste/spec.md b/openspec/specs/terminal-image-paste/spec.md new file mode 100644 index 00000000..46e51844 --- /dev/null +++ b/openspec/specs/terminal-image-paste/spec.md @@ -0,0 +1,318 @@ +# Terminal Image Paste Specification + +## Purpose + +Allow users to paste or drop images and files into a terminal so that CLI agents +(Claude Code, etc.) receive a usable absolute filesystem path — not a basename, +not raw bytes, and not a browser-default text fallback. Resolution covers the +macOS and Linux clipboard flavours that file managers actually publish, and +both Cmd+V paste and drag-and-drop entry points, delivering the result through +xterm's bracketed-paste pipeline so agents recognize it as a paste. + +## Requirements + +### Requirement: Cmd+V resolves the clipboard to its most useful representation + +The system SHALL resolve the clipboard to a single representation chosen +by priority — a filesystem path the agent can read, an image saved to a +temp file, or plain text — and never insert a bare basename when the +underlying file URL was available. Resolution happens when the user +invokes the paste keybinding while the terminal has focus. + +#### Scenario: Finder-copied image file pastes its absolute path + +- **WHEN** the user copies an image file in macOS Finder and presses + Cmd+V in the terminal +- **THEN** the absolute path to that file is typed into the terminal +- **AND** the typed value is escaped per the path-escaping requirement +- **AND** the basename alone is never inserted + +#### Scenario: Linux GNOME file-manager copy pastes its absolute path + +- **WHEN** the user copies a file in a GNOME-family file manager + (Nautilus, Nemo, Caja) and presses the paste binding +- **THEN** the system reads the `x-special/gnome-copied-files` clipboard + flavour, skips the leading `copy` / `cut` verb line, takes the first + remaining `file://` URL, converts it to an absolute path, and types + the escaped path +- **AND** the GNOME flavour is checked before `text/uri-list` so it + wins when both flavours are present + +#### Scenario: Other Linux file managers fall back to text/uri-list + +- **WHEN** the user copies a file from a non-GNOME Linux file manager + (KDE Dolphin, Xfce Thunar, etc.) that publishes only the cross-desktop + RFC 2483 format +- **THEN** the system reads the `text/uri-list` clipboard flavour, takes + the first non-comment `file://` URL, converts it to an absolute path, + and types the escaped path + +#### Scenario: Raw image on the clipboard is saved and pasted as a path + +- **WHEN** the clipboard does not contain a file URL but does contain + raster image bytes (e.g. a screenshot) +- **THEN** the system writes the image as PNG to the OS temp directory +- **AND** types the escaped absolute path of that file + +#### Scenario: Plain text on the clipboard pastes as text + +- **WHEN** the clipboard contains neither a file URL nor a raster image + but does contain plain text +- **THEN** that text is typed verbatim, with no quoting and no escaping + +#### Scenario: Empty clipboard does nothing + +- **WHEN** the clipboard contains no file, no image, and no text +- **THEN** no input is sent to the terminal +- **AND** the keypress is consumed (the keybinding's `preventDefault` + still runs so xterm does not insert a fallback) + +#### Scenario: Resolution happens in the main process via a single IPC + +- **WHEN** the renderer needs to resolve a paste +- **THEN** it makes exactly one IPC call (`ResolveClipboardPaste`) and + switches on the returned `kind` +- **AND** it does NOT call `navigator.clipboard.readText()` first + +### Requirement: File drop on the terminal types absolute path(s) + +The system SHALL type each dropped file's absolute path into the +terminal — space-joined and escaped — instead of allowing the browser +default to insert the basename. This applies whenever the user drops +one or more files onto the terminal viewport. + +#### Scenario: Single file dropped from the OS file manager + +- **WHEN** the user drags one file from Finder / Nautilus and drops it + on the terminal +- **THEN** the file's absolute path is typed into the terminal, escaped + per the path-escaping requirement +- **AND** the basename alone is never inserted + +#### Scenario: Multiple files dropped at once + +- **WHEN** the user drops two or more files in a single drop +- **THEN** every file's escaped path is typed +- **AND** the paths are joined with a single ASCII space, in the order + the OS reported them +- **AND** no trailing space is appended + +#### Scenario: Drop target is the terminal even when not focused + +- **WHEN** the user drops a file on the terminal while focus is in a + different element (e.g. the prompt input or sidebar) +- **THEN** the terminal is focused before the path is typed +- **AND** the resulting input arrives at the terminal, not at the + previously-focused element + +#### Scenario: Drop handler runs in DOM capture phase + +- **WHEN** a `drop` event fires on the terminal container +- **THEN** the application's handler runs in the capture phase, calls + `preventDefault()` and `stopPropagation()`, and xterm's own bubble + phase listener does not run + +#### Scenario: Capture-phase listeners are torn down on unmount + +- **WHEN** a `TerminalView` unmounts +- **THEN** the application's `dragover` and `drop` listeners are removed + with the same `{ capture: true }` setting they were registered with +- **AND** subsequent drops on the unmounted node do nothing + +#### Scenario: Drop without files is ignored + +- **WHEN** a `drop` fires on the terminal but `dataTransfer.files` is + empty (e.g. text-only drag, GitHub URL drag) +- **THEN** the application's terminal drop handler returns without + preventing default and without typing anything +- **AND** the surrounding application's URL-drop handler may still + process the drop normally + +### Requirement: Browser-origin items without a path are persisted to a temp file + +The system SHALL read a dropped item's bytes and persist them to the OS +temp directory, then type that temp path, whenever the item has no +backing filesystem path (e.g. an `` dragged from a browser tab, an +item from a virtual file system). + +#### Scenario: Browser image drop produces a usable temp path + +- **WHEN** the user drags an `` element from a browser into the + terminal +- **AND** `webUtils.getPathForFile(file)` returns the empty string +- **THEN** the renderer reads the file as an `ArrayBuffer`, + base64-encodes it, and calls `SaveDroppedImage` with `{ name, data }` +- **AND** the main process decodes the base64 to a `Buffer` and writes + it to the OS temp directory +- **AND** the returned absolute path is typed into the terminal, + escaped per the path-escaping requirement + +#### Scenario: Filename for the temp file is sanitized + +- **WHEN** the renderer-supplied filename contains path separators (`/`, + `\`), a NUL byte, leading dots, or extra surrounding whitespace +- **THEN** the main process strips those characters / runs before + writing +- **AND** clamps the remaining basename to 200 characters +- **AND** prefixes the basename with + `parallel-code-drop--<6-hex>-` where `<6-hex>` is a fresh + random suffix per call so two same-name drops landing in the same + millisecond never collide on disk +- **AND** when the sanitized basename is empty, falls back to + `parallel-code-drop--<6-hex>.png` + +#### Scenario: Renderer caps oversized drops without surfacing an error + +- **WHEN** the dropped item is larger than 50 MB +- **THEN** the renderer skips that item without sending it across IPC +- **AND** continues to process other items in the same drop + +#### Scenario: One failing item does not cancel the rest of the drop + +- **WHEN** a mixed drop contains both items that resolve successfully + (path-backed Files or path-less items whose bytes save cleanly) + and items whose resolution throws (oversized, `arrayBuffer()` rejects, + `SaveDroppedImage` rejects, base64 encoding fails) +- **THEN** the failing items are silently filtered out +- **AND** the successfully-resolved items are still typed into the + terminal as a space-joined escaped string +- **AND** no error is surfaced through the drop handler's catch + block (the failure is local to the failing item) + +#### Scenario: Binary payload survives the IPC envelope + +- **WHEN** binary bytes are sent over `SaveDroppedImage` +- **THEN** they are encoded as base64 in the renderer and decoded with + `Buffer.from(data, 'base64')` in the main process +- **AND** the file written to disk has the exact byte length of the + source payload +- **AND** they are NOT sent as `Uint8Array` or `ArrayBuffer` (the + application's `invoke()` wrapper destroys typed arrays via its + `JSON.parse(JSON.stringify(args))` round-trip) + +### Requirement: Paths are escaped with backslash before insertion + +Paths typed into the terminal SHALL be backslash-escaped before +insertion so they round-trip correctly through both POSIX shells (bash, +zsh) and CLI agents that parse paths from prompt text. + +#### Scenario: Safe paths pass through unchanged + +- **WHEN** a path contains only characters from + `[A-Za-z0-9_./@:+,%=-]` +- **THEN** it is typed verbatim with no escape characters added + +#### Scenario: Whitespace is escaped + +- **WHEN** a path contains a space or any other whitespace character +- **THEN** each whitespace character is preceded by a single backslash + +#### Scenario: Shell metacharacters are escaped + +- **WHEN** a path contains any of the characters + `' " \ $ \` ! ( ) ; & | < > \* ? [ ] { } ~ #` +- **THEN** each such character is preceded by a single backslash + +#### Scenario: Empty path renders as empty bash literal + +- **WHEN** a path is the empty string +- **THEN** it renders as `""` (two ASCII double quotes) so a real shell + receives an explicit empty argument rather than dropping the position + +#### Scenario: Multiple paths are space-joined after escaping + +- **WHEN** more than one resolved path is being inserted in one drop +- **THEN** each path is escaped individually +- **AND** the escaped paths are joined with a single ASCII space + +### Requirement: Resolved paste/drop content flows through xterm's paste pipeline + +The system SHALL deliver resolved paste and drop payloads (file paths, +image temp paths, plain text) to the agent through `term.paste()`, +never via a direct PTY write. This causes xterm to wrap the payload +with bracketed-paste markers (`\x1b[200~` … `\x1b[201~`) when the agent +has bracketed-paste mode enabled, which CLI agents like Claude Code use +to distinguish "the user pasted this" from "the user typed this +character by character" — the former is what triggers automatic +image-file recognition and attachment. + +#### Scenario: Cmd+V file path uses term.paste + +- **WHEN** the paste handler resolves the clipboard to `kind: 'file'` + or `kind: 'image'` +- **THEN** the escaped path is delivered via `term.paste(path)` +- **AND** is NOT delivered via direct PTY write + +#### Scenario: Cmd+V plain text uses term.paste + +- **WHEN** the paste handler resolves the clipboard to `kind: 'text'` +- **THEN** the text is delivered via `term.paste(text)` so a paste of + multiple lines into a bracketed-paste-aware shell is treated as a + single paste rather than a sequence of typed lines + +#### Scenario: Drop payload uses term.paste + +- **WHEN** the drop handler has resolved the dropped files into a + space-joined escaped path string +- **THEN** the string is delivered via `term.paste(args)` after + `term.focus()` + +#### Scenario: Bracketed-paste markers appear when the agent enables them + +- **WHEN** the agent has previously sent the bracketed-paste-mode + enable sequence (`\x1b[?2004h`) +- **THEN** the bytes the PTY actually receives for a paste/drop + delivery start with `\x1b[200~` and end with `\x1b[201~` +- **AND** when bracketed-paste mode is disabled (or never enabled), + the bytes the PTY receives are the payload alone with no marker + bytes + +### Requirement: IPC contract for clipboard and drop + +The system SHALL communicate clipboard and drop intents through dedicated +IPC channels declared in `electron/ipc/channels.ts` and allowlisted in +the preload bridge. + +#### Scenario: ResolveClipboardPaste channel exists + +- **WHEN** the renderer invokes `IPC.ResolveClipboardPaste` +- **THEN** the main process returns a tagged-union object with one of + the shapes `{ kind: 'file', path: string }`, + `{ kind: 'image', path: string }`, `{ kind: 'text', text: string }`, + or `{ kind: 'empty' }` +- **AND** no other shape is ever returned + +#### Scenario: SaveDroppedImage channel exists + +- **WHEN** the renderer invokes `IPC.SaveDroppedImage` with + `{ name: string, data: string /* base64 */ }` +- **THEN** the main process returns the absolute path of the file it + wrote +- **AND** rejects payloads whose `data` is not a string + +#### Scenario: getPathForFile is exposed via the preload bridge + +- **WHEN** the renderer calls `window.electron.getPathForFile(file)` + with a `File` object obtained from a drop event +- **THEN** the function returns the absolute filesystem path for files + that have one +- **AND** returns the empty string for files that do not (browser-origin + items, virtual file system items, etc.) +- **AND** returns the empty string instead of throwing on any internal + error from `webUtils.getPathForFile` + +#### Scenario: Both new channels are flagged NEVER_SAFE for tracing + +- **WHEN** the IPC trace module initialises +- **THEN** both `ResolveClipboardPaste` and `SaveDroppedImage` are + members of `NEVER_SAFE` +- **AND** their argument and return payloads are never written to the + debug log + +#### Scenario: The legacy SaveClipboardImage channel is removed + +- **WHEN** this change is shipped +- **THEN** the IPC enum no longer contains `SaveClipboardImage` +- **AND** the preload allowlist no longer contains + `'save_clipboard_image'` +- **AND** no main-process handler for the legacy channel is registered diff --git a/package.json b/package.json index 8f385933..4a49d7a7 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "test:coverage": "vitest run --coverage", "check": "npm run typecheck && npm run lint && npm run format:check", "release": "npm run typecheck && npm version patch && git push --follow-tags", + "postinstall": "node scripts/fix-node-pty-spawn-helper.mjs", "prepare": "husky && git config core.hooksPath .husky" }, "license": "MIT", diff --git a/screens/islands-plan-review.png b/screens/islands-plan-review.png new file mode 100644 index 00000000..6f95200f Binary files /dev/null and b/screens/islands-plan-review.png differ diff --git a/scripts/fix-node-pty-spawn-helper.mjs b/scripts/fix-node-pty-spawn-helper.mjs new file mode 100644 index 00000000..f7c0b08e --- /dev/null +++ b/scripts/fix-node-pty-spawn-helper.mjs @@ -0,0 +1,18 @@ +import { chmodSync, existsSync } from 'fs'; +import { join } from 'path'; +import process from 'process'; + +if (process.platform === 'darwin') { + const helperPath = join( + process.cwd(), + 'node_modules', + 'node-pty', + 'prebuilds', + `darwin-${process.arch}`, + 'spawn-helper', + ); + + if (existsSync(helperPath)) { + chmodSync(helperPath, 0o755); + } +} diff --git a/src/arena/ConfigScreen.tsx b/src/arena/ConfigScreen.tsx index 07b5183a..22097aac 100644 --- a/src/arena/ConfigScreen.tsx +++ b/src/arena/ConfigScreen.tsx @@ -24,7 +24,7 @@ import type { BattleCompetitor } from './types'; /** Built-in tool presets — click to fill the next empty competitor slot */ const TOOL_PRESETS: Array<{ name: string; command: string }> = [ { name: 'Claude', command: 'claude -p "{prompt}" --dangerously-skip-permissions' }, - { name: 'Codex', command: 'codex exec --full-auto "{prompt}"' }, + { name: 'Codex', command: 'codex exec --dangerously-bypass-approvals-and-sandbox "{prompt}"' }, { name: 'Gemini', command: 'gemini -p "{prompt}" --yolo' }, { name: 'Copilot', command: 'copilot -p "{prompt}" --yolo' }, { name: 'Aider', command: 'aider -m "{prompt}" --yes' }, diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index e7071f3f..2a3e70b3 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -5,6 +5,11 @@ import { theme } from '../lib/theme'; import { sf } from '../lib/fontScale'; import { getStatusColor } from '../lib/status-colors'; import { buildFileTree, flattenVisibleTree } from '../lib/file-tree'; +import { + type CommitSelection, + isCommitHashSelection, + isUncommittedSelection, +} from './CommitNavBar'; import type { ChangedFile, CoverageFileSummary, CoverageSummary } from '../ipc/types'; interface ChangedFilesListProps { @@ -12,6 +17,8 @@ interface ChangedFilesListProps { isActive?: boolean; panelFocused?: boolean; onFileClick?: (file: ChangedFile) => void; + /** Optional path to visually mark as the active/open diff target. */ + activeFilePath?: string | null; ref?: (el: HTMLDivElement) => void; /** Optional coverage artifact path relative to the repo root. */ coverageReportPath?: string; @@ -21,8 +28,13 @@ interface ChangedFilesListProps { branchName?: string | null; /** Base branch for diff comparison (e.g. 'main', 'develop'). Undefined = auto-detect. */ baseBranch?: string; - /** When set to a commit hash, show files for that single commit. null/undefined = all changes. */ - selectedCommit?: string | null; + /** + * Selection mode for the file list: + * - undefined/null: all changes (committed + uncommitted) + * - UNCOMMITTED_SELECTION: only currently uncommitted changes + * - any commit hash: files for that single commit + */ + selectedCommit?: CommitSelection; } const SOURCE_FILE_RE = /\.(?:[cm]?[jt]sx?)$/i; @@ -87,11 +99,12 @@ function coverageBadgeTitle(summary: CoverageFileSummary): string { function FileCoverageBadge(props: { file: ChangedFile; - selectedCommit?: string | null; + selectedCommit?: CommitSelection; summary?: CoverageFileSummary; hasCoverageArtifact: boolean; }) { - const isEligible = () => !props.selectedCommit && isCoverageEligible(props.file); + const isEligible = () => + !isCommitHashSelection(props.selectedCommit) && isCoverageEligible(props.file); const summary = () => (isEligible() ? props.summary : undefined); return ( @@ -269,18 +282,22 @@ export function ChangedFilesList(props: ChangedFilesListProps) { } } - // Poll every 5s, matching the git status polling interval. // Falls back to branch-based diff when worktree path doesn't exist. - // When selectedCommit is set, fetches files for that single commit (no polling). + // When selectedCommit is a hash, fetches files for that single commit (no polling). + // When selectedCommit is the uncommitted sentinel, fetches all changes and filters. + // Always runs an initial fetch on mount/input change so non-active tasks have + // populated data — CommitNavBar buttons stopPropagation, so navigating there + // wouldn't otherwise activate the task and trigger a fetch. Polling at 5s + // (matching git status) is still gated on isActive to avoid running git + // pipelines for every off-screen task. createEffect(() => { const path = props.worktreePath; const projectRoot = props.projectRoot; const branchName = props.branchName; const baseBranch = props.baseBranch; - const commitHash = props.selectedCommit; - // In single-commit mode the user explicitly navigated — always fetch. - // In all-changes mode skip when inactive to avoid background polling. - if (!commitHash && !props.isActive) return; + const selection = props.selectedCommit; + const singleCommitHash = isCommitHashSelection(selection) ? selection : null; + const uncommittedOnly = isUncommittedSelection(selection); let cancelled = false; let inFlight = false; let usingBranchFallback = false; @@ -290,11 +307,11 @@ export function ChangedFilesList(props: ChangedFilesListProps) { inFlight = true; try { // Single-commit mode: fetch files for that commit only - if (commitHash && path) { + if (singleCommitHash && path) { try { const result = await invoke(IPC.GetCommitChangedFiles, { worktreePath: path, - commitHash, + commitHash: singleCommitHash, }); if (!cancelled) setFiles(result); } catch { @@ -310,7 +327,9 @@ export function ChangedFilesList(props: ChangedFilesListProps) { worktreePath: path, baseBranch, }); - if (!cancelled) setFiles(result); + if (!cancelled) { + setFiles(uncommittedOnly ? result.filter((f) => !f.committed) : result); + } return; } catch { // Worktree may not exist — try branch fallback below @@ -326,7 +345,9 @@ export function ChangedFilesList(props: ChangedFilesListProps) { branchName, baseBranch, }); - if (!cancelled) setFiles(result); + if (!cancelled) { + setFiles(uncommittedOnly ? result.filter((f) => !f.committed) : result); + } } catch { // Branch may no longer exist } @@ -337,12 +358,14 @@ export function ChangedFilesList(props: ChangedFilesListProps) { } void refresh(); - // No polling needed for single-commit view (committed data is immutable) - const timer = commitHash - ? undefined - : setInterval(() => { + // Polling: skip when inactive (off-screen tasks) and when viewing a single + // commit (committed data is immutable). + const shouldPoll = singleCommitHash === null && props.isActive; + const timer = shouldPoll + ? setInterval(() => { if (!usingBranchFallback) void refresh(); - }, 5000); + }, 5000) + : undefined; onCleanup(() => { cancelled = true; if (timer !== undefined) clearInterval(timer); @@ -351,8 +374,8 @@ export function ChangedFilesList(props: ChangedFilesListProps) { createEffect(() => { const repoRoot = props.worktreePath; - const commitHash = props.selectedCommit; - if (!repoRoot || commitHash) { + const selection = props.selectedCommit; + if (!repoRoot || isCommitHashSelection(selection)) { setCoverage(null); return; } @@ -421,10 +444,16 @@ export function ChangedFilesList(props: ChangedFilesListProps) { cursor: 'pointer', 'border-radius': '3px', opacity: - !props.selectedCommit && (row().isDir || row().node.file?.committed) + !isCommitHashSelection(props.selectedCommit) && + (row().isDir || row().node.file?.committed) ? '0.45' : '1', - background: selectedIndex() === i ? theme.bgHover : 'transparent', + background: + selectedIndex() === i + ? theme.bgHover + : row().node.file && row().node.path === props.activeFilePath + ? 'rgba(88, 166, 255, 0.16)' + : 'transparent', }} onClick={() => { setSelectedIndex(i); @@ -551,7 +580,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { 'flex-wrap': 'wrap', }} > - 0}> + 0}>
void; + selectedCommitHash: CommitSelection; + onNavigate: (selection: CommitSelection) => void; compact?: boolean; showMessage?: boolean; } @@ -14,13 +31,17 @@ interface CommitNavBarProps { export function CommitNavBar(props: CommitNavBarProps) { const currentIndex = createMemo(() => { const hash = props.selectedCommitHash; - if (hash === null) return -1; + if (hash === null || hash === UNCOMMITTED_SELECTION) return -1; return props.commits.findIndex((c) => c.hash === hash); }); const isAllChanges = () => props.selectedCommitHash === null; + const isUncommittedOnly = () => props.selectedCommitHash === UNCOMMITTED_SELECTION; const hasCommits = () => props.commits.length > 0; - const canGoLeft = () => hasCommits() && (isAllChanges() || currentIndex() > 0); + // All → Uncommitted (always); Uncommitted → latest commit (needs commits); + // commit N → commit N-1 (needs a previous commit). + const canGoLeft = () => + isAllChanges() || (isUncommittedOnly() && hasCommits()) || (hasCommits() && currentIndex() > 0); const canGoRight = () => !isAllChanges(); const selectedCommit = createMemo(() => { @@ -29,32 +50,61 @@ export function CommitNavBar(props: CommitNavBarProps) { }); function goLeft() { - const commits = props.commits; - if (commits.length === 0) return; if (isAllChanges()) { + props.onNavigate(UNCOMMITTED_SELECTION); + return; + } + if (isUncommittedOnly()) { + const commits = props.commits; + if (commits.length === 0) return; props.onNavigate(commits[commits.length - 1].hash); - } else { - const idx = currentIndex(); - if (idx > 0) { - props.onNavigate(commits[idx - 1].hash); - } + return; + } + const idx = currentIndex(); + if (idx > 0) { + props.onNavigate(props.commits[idx - 1].hash); } } function goRight() { + if (isAllChanges()) return; + if (isUncommittedOnly()) { + props.onNavigate(null); + return; + } const commits = props.commits; - if (commits.length === 0 || isAllChanges()) return; const idx = currentIndex(); if (idx < commits.length - 1) { props.onNavigate(commits[idx + 1].hash); } else { - props.onNavigate(null); + props.onNavigate(UNCOMMITTED_SELECTION); } } const compact = () => props.compact ?? false; const btnSize = () => (compact() ? '18px' : '22px'); const iconSize = () => (compact() ? 12 : 14); + const pillPadding = () => (compact() ? '1px 4px' : '2px 8px'); + const pillFontSize = () => sf(compact() ? 10 : 12); + + function pillStyle(active: boolean) { + return { + background: active ? `color-mix(in srgb, ${theme.accent} 15%, transparent)` : 'transparent', + border: `1px solid ${active ? theme.accent : theme.border}`, + color: active ? theme.accent : theme.fgMuted, + cursor: 'pointer', + 'border-radius': '4px', + padding: pillPadding(), + 'font-size': pillFontSize(), + 'font-family': "'JetBrains Mono', monospace", + 'font-weight': active ? '600' : '400', + 'line-height': '1', + 'flex-shrink': '0', + display: 'inline-flex', + 'align-items': 'center', + height: btnSize(), + } as const; + } return (
+ {/* Uncommitted-only button */} + + {/* All Changes button */} + + + Uncommitted changes only + + + {(commit) => ( void; + /** Optional coverage artifact path relative to the repo root. */ + coverageReportPath?: string; /** Project root for branch-based fallback when worktree doesn't exist */ projectRoot?: string; /** Branch name for branch-based fallback when worktree doesn't exist */ @@ -31,10 +42,10 @@ interface DiffViewerDialogProps { agentId?: string; /** List of commits on this branch (oldest first) for commit navigation */ commitList?: CommitInfo[]; - /** Currently selected commit hash, or null for "all changes" mode */ - selectedCommit?: string | null; - /** Callback to navigate to a different commit or null for all changes */ - onCommitNavigate?: (hash: string | null) => void; + /** Current selection: null = all changes, sentinel = uncommitted-only, hash = single commit */ + selectedCommit?: CommitSelection; + /** Callback to navigate to a different selection */ + onCommitNavigate?: (selection: CommitSelection) => void; /** Git isolation mode — CommitNavBar is only shown for worktree-isolated tasks */ gitIsolation?: GitIsolationMode; } @@ -59,18 +70,21 @@ export function DiffViewerDialog(props: DiffViewerDialogProps) {

- Diff viewer: {props.scrollToFile ?? 'all changes'} + Diff viewer for {props.taskName ?? 'task'}: {props.scrollToFile ?? 'all changes'}

([]); const [loading, setLoading] = createSignal(false); const [error, setError] = createSignal(''); const [searchQuery, setSearchQuery] = createSignal(''); + const [activeFilePath, setActiveFilePath] = createSignal(null); let fetchGeneration = 0; let searchInputRef: HTMLInputElement | undefined; @@ -131,11 +149,15 @@ function DiffViewerContent(props: DiffViewerDialogProps) { onCleanup(() => document.removeEventListener('keydown', handler)); }); + createEffect(() => { + setActiveFilePath(props.scrollToFile); + }); + createEffect(() => { const scrollTarget = props.scrollToFile; // Access selectedCommit before the early return so the effect tracks it // even when the dialog is closed — ensures we re-run on reopen. - const commitHash = props.selectedCommit; + const selection = props.selectedCommit; if (!scrollTarget) return; const worktreePath = props.worktreePath; @@ -151,9 +173,15 @@ function DiffViewerContent(props: DiffViewerDialogProps) { let diffPromise: Promise; - if (commitHash && worktreePath) { + if (isCommitHashSelection(selection) && worktreePath) { // Single-commit mode - diffPromise = invoke(IPC.GetCommitDiffs, { worktreePath, commitHash }); + diffPromise = invoke(IPC.GetCommitDiffs, { + worktreePath, + commitHash: selection, + }); + } else if (isUncommittedSelection(selection) && worktreePath) { + // Uncommitted-only mode (worktree only — branch fallback has no working tree) + diffPromise = invoke(IPC.GetUncommittedFileDiffs, { worktreePath }); } else { // All-changes mode (existing behavior) const worktreePromise = worktreePath @@ -230,11 +258,48 @@ function DiffViewerContent(props: DiffViewerDialogProps) { display: 'flex', 'align-items': 'center', gap: '10px', - padding: '12px 20px', + padding: `${headerPaddingTop} 20px 12px`, 'border-bottom': `1px solid ${theme.border}`, 'flex-shrink': '0', }} > +
+ + Changes + + + {props.taskName ?? 'Untitled task'} + +
+ + +
8 ? 'min(840px, calc(100vw - 48px))' : '560px'} labelledBy={titleId} - panelStyle={{ gap: '20px' }} + panelStyle={{ padding: '0', overflow: 'hidden', gap: '0' }} >
-
-

- New Task -

-
- - {/* Project selector */}
- - -
- - {/* Prompt input (optional) */} -
- -