From f278d7ac69cc6ec459b15cb0a491c66547dc8932 Mon Sep 17 00:00:00 2001 From: keliangliang Date: Sat, 17 Oct 2026 16:00:25 +0800 Subject: [PATCH] feat: add secure file and source control workflows --- CHANGELOG.md | 2 + docs/files-crud/intent.md | 23 + docs/files-crud/plan.md | 43 ++ docs/files-crud/spec.md | 60 ++ docs/orca-aligned-source-control/intent.md | 20 + docs/orca-aligned-source-control/plan.md | 25 + docs/orca-aligned-source-control/spec.md | 71 ++ .../project-files-surface-alignment/intent.md | 15 + docs/project-files-surface-alignment/plan.md | 6 + docs/project-files-surface-alignment/spec.md | 18 + electron/appState.ts | 13 +- electron/contracts.test.ts | 13 + electron/contracts.ts | 11 + electron/core/gitProcess.ts | 27 +- electron/core/keyedLock.test.ts | 22 + electron/core/keyedLock.ts | 18 + .../domains/filesystem/filesystem.test.ts | 151 ++++ electron/domains/filesystem/pathGuard.ts | 2 +- electron/domains/filesystem/service.ts | 505 +++++++++++- electron/domains/git/git.test.ts | 255 ++++++ electron/domains/git/service.ts | 414 ++++++++-- electron/ipc.test.ts | 74 ++ electron/ipc.ts | 78 +- src/bindings/generated.ts | 9 + src/bindings/index.ts | 52 ++ src/features/changes/ChangesPanel.test.tsx | 130 +++- src/features/changes/ChangesPanel.tsx | 729 +++++++++++++----- src/features/changes/CommitArea.tsx | 56 ++ src/features/changes/DiscardChangesDialog.tsx | 99 +++ src/features/changes/changesActions.test.ts | 111 +++ src/features/changes/changesActions.ts | 107 +++ .../changes/changesProjection.test.ts | 85 ++ src/features/changes/changesProjection.ts | 86 +++ src/features/changes/changesStore.test.ts | 25 + src/features/changes/changesStore.ts | 79 +- src/features/editor/EditorPane.tsx | 121 +-- .../editor/editorDocumentOperations.test.ts | 40 + .../editor/editorDocumentOperations.ts | 50 ++ src/features/editor/editorStore.test.ts | 62 ++ src/features/editor/editorStore.ts | 141 ++++ src/features/files/FileTree.test.tsx | 63 +- src/features/files/FileTree.tsx | 727 +++++++++++++++-- src/features/files/fileTreeStore.ts | 39 + src/features/files/filesystemEvents.test.ts | 27 +- src/features/files/filesystemEvents.ts | 20 +- src/i18n/en.ts | 60 +- src/i18n/zh-CN.ts | 57 +- src/lib/resourceCache.ts | 10 + src/lib/useTreeKeyboard.ts | 7 +- src/styles/index.css | 494 ++++++++++-- src/styles/themeTokens.test.ts | 41 + 51 files changed, 4949 insertions(+), 444 deletions(-) create mode 100644 docs/files-crud/intent.md create mode 100644 docs/files-crud/plan.md create mode 100644 docs/files-crud/spec.md create mode 100644 docs/orca-aligned-source-control/intent.md create mode 100644 docs/orca-aligned-source-control/plan.md create mode 100644 docs/orca-aligned-source-control/spec.md create mode 100644 docs/project-files-surface-alignment/intent.md create mode 100644 docs/project-files-surface-alignment/plan.md create mode 100644 docs/project-files-surface-alignment/spec.md create mode 100644 electron/core/keyedLock.test.ts create mode 100644 electron/core/keyedLock.ts create mode 100644 src/features/changes/CommitArea.tsx create mode 100644 src/features/changes/DiscardChangesDialog.tsx create mode 100644 src/features/changes/changesActions.test.ts create mode 100644 src/features/changes/changesActions.ts create mode 100644 src/features/changes/changesProjection.test.ts create mode 100644 src/features/changes/changesProjection.ts create mode 100644 src/features/editor/editorDocumentOperations.test.ts create mode 100644 src/features/editor/editorDocumentOperations.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d52bbf..1eba4a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,12 +8,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Added +- Complete Files CRUD with inline file/folder creation and rename, duplication, cut/copy/paste moves, system Trash deletion, file-manager reveal, active-file reveal, and secure worktree-relative Main-process operations. - Public contribution, security, support, conduct, issue, and pull request guidance. - Cross-platform development packaging for macOS arm64, Windows x64, and Linux x64. - Projects, managed worktrees, file editing, Git changes, terminals, multi-tab pi Agent chat, settings, and support for user-installed standard Pi resources. ### Changed +- Upgraded Git Changes into an Orca-inspired Source Control panel with filtering, collapsible conflict-aware groups, rich status metadata, safe stage/unstage/discard actions, and per-worktree commit drafts. - Refined the workbench with quieter neutral chrome, a narrower Chat reading column, accessible icon tooltips, and a System appearance option that follows the operating system theme. ### Removed diff --git a/docs/files-crud/intent.md b/docs/files-crud/intent.md new file mode 100644 index 0000000..5af3603 --- /dev/null +++ b/docs/files-crud/intent.md @@ -0,0 +1,23 @@ +# Files CRUD Intent + +## Problem + +SpireCode's Files panel can browse and open files but cannot create, rename, move, copy, or delete project content. Users must leave the app for routine project organization, and external mutations can leave editor tabs and the tree out of sync. + +## Outcome + +Make Files a complete, accessible project file manager while preserving SpireCode's Electron security boundary: Renderer operations identify a worktree and relative paths only; Main resolves canonical roots, rejects traversal and .git access, and owns every filesystem mutation. + +Users can create files and folders, rename or move entries, duplicate them, and move them to the operating system Trash. The tree refreshes without losing context, active files can be revealed, and open editor resources follow successful path mutations. + +## Non-goals + +- Exposing Node, Electron, absolute project paths, generic filesystem APIs, or shell command strings to Renderer. +- Permanent deletion as the default or a silent fallback when Trash fails. +- Following symlinks during recursive copy or allowing operations through symlinked parents. +- Cross-worktree moves, multi-selection, file-operation undo/redo, or tree virtualization in this delivery. +- Overwrite-on-conflict; mutations fail closed when a destination exists. + +## Risk + +High risk: this changes filesystem/path-isolation behavior and the Renderer/Main IPC contract, and includes destructive user-visible operations. The user explicitly confirmed the design and requested implementation after these risks and behaviors were described. diff --git a/docs/files-crud/plan.md b/docs/files-crud/plan.md new file mode 100644 index 0000000..11312dc --- /dev/null +++ b/docs/files-crud/plan.md @@ -0,0 +1,43 @@ +# Files CRUD Plan + +## 1. Contract and domain tests + +- Extend generated-facing TypeScript types, command allowlists, argument validation, bindings, and IPC dispatch. +- Add failing filesystem-domain tests for create, rename, move, recursive copy, Trash injection, conflicts, root rejection, .git/traversal, symlink-parent escape, and directory-into-descendant rejection. +- Implement operations in `FilesystemService` using exclusive creation and existing path guards; inject Trash from `AppState`. + +Proof: filesystem and IPC contract tests pass, including platform-conditional symlink cases. + +## 2. Store consistency tests + +- Add failing tests for directory-targeted invalidation, collapse-all/expansion helpers, editor path remapping, affected dirty-tab inspection, and tab removal after Trash. +- Implement focused FileTree and editor-store mutation APIs, including persisted tab IDs and draft remapping support. + +Proof: file-tree store, filesystem event, and editor store tests pass. + +## 3. Files interactions + +- Add toolbar, menu, inline input, operation error, clipboard state, and Trash confirmation to Files. +- Wire commands so state changes occur only after Main success. +- Add active-file ancestor expansion and scroll-to-row behavior. +- Add localized strings and neutral semantic styling consistent with the current workbench. + +Proof: component tests exercise keyboard create/rename cancellation and submission, CRUD command arguments, conflict errors, confirmation, clipboard move/copy, and editor synchronization. + +## 4. Refresh behavior and integration + +- Refresh only affected cached directories from non-truncated watcher payloads; preserve full invalidation for truncated payloads. +- Reuse existing Git invalidation and editor reload behavior without broadening Renderer authority. +- Update Changelog. + +Proof: watcher event tests cover exact, ancestor, and truncated invalidation. + +## 5. Verification and review + +- Run targeted red-green suites during each step. +- Run formatting, localization/brand/security guards, lint, both TypeScript projects, all Renderer/Electron tests, release tests, and production build through `pnpm check` and `pnpm build`. +- Inspect the final diff for raw paths, shell strings, unsafe overwrite, permanent-delete fallback, unlocalized copy, and contract drift. + +## Delivery note + +This branch is stacked on the current Orca-inspired P0 branch because it reuses the shared Tooltip and visual tokens. It should be merged only after its base change or rebased onto main. diff --git a/docs/files-crud/spec.md b/docs/files-crud/spec.md new file mode 100644 index 0000000..99a9322 --- /dev/null +++ b/docs/files-crud/spec.md @@ -0,0 +1,60 @@ +# Files CRUD Specification + +## Main contract + +Add narrow allowlisted commands: + +- `fs_create_file(worktreeId, relativePath)` +- `fs_create_directory(worktreeId, relativePath)` +- `fs_rename(worktreeId, relativePath, newName)` +- `fs_move(worktreeId, sourceRelativePath, destinationRelativePath)` +- `fs_copy(worktreeId, sourceRelativePath, destinationRelativePath)` +- `fs_trash(worktreeId, relativePath)` +- `fs_reveal(worktreeId, relativePath)` + +Mutations return a typed result containing operation kind, source path when applicable, destination path when applicable, and entry kind. No command accepts an absolute path. `fs_rename` accepts only a leaf name; arbitrary relocation uses `fs_move`. + +## Path and mutation safety + +- Resolve worktree IDs to canonical roots in Main. +- Reject absolute paths, empty mutation paths, `.`, `..`, NUL, and any `.git` component. +- Require canonical existing parents and reject symlink-parent traversal or escape. +- Never overwrite an existing destination. +- Reject moving/copying a directory into itself or a descendant. +- Copy symlinks as symlinks without traversing them; reject copied symlinks whose target would escape the worktree. +- Create files with exclusive creation and create directories without recursive missing-parent creation. +- Trash uses Electron `shell.trashItem`; failure is surfaced and never falls back to permanent deletion. +- The worktree root cannot be renamed, moved, copied, or trashed. + +## Files UI + +- A compact toolbar exposes only New File and New Folder with accessible labels and tooltips; filesystem watching keeps the tree current without manual Refresh, and users collapse directories directly in the tree. +- Context menus expose context-appropriate New File/Folder, Rename, Duplicate, Cut, Copy, Paste, Copy Relative Path, Reveal in Finder, and Move to Trash. +- Creation and rename use an inline input. Enter submits, Escape cancels, and validation or command errors remain visible without losing the input. +- Copy stores a source entry and Paste invokes copy. Cut stores a source entry and Paste invokes move. Clipboard state is scoped to one worktree and clears after a successful cut/move. +- Trash requires confirmation. If affected file tabs contain unsaved changes, the confirmation explicitly warns that local drafts will be lost. +- Filesystem failures are localized and announced. Existing tree content remains visible during refresh. + +## Editor and tree consistency + +- Successful rename/move remaps all open file and diff tabs at the exact source path or below a moved directory, preserving active tab, preview/pinned state, and dirty metadata. +- Draft buffers are keyed by tab ID and are remapped with tabs so unsaved content survives rename/move. +- Successful Trash closes affected tabs only after confirmation; unrelated tabs stay open. +- Copy/create refresh affected directories; rename/move/trash refresh source and destination parents. +- The active file is automatically revealed by expanding ancestors and scrolling it into view without stealing focus. +- Watcher events use their changed paths for directory-level invalidation when not truncated; truncated events retain full-worktree invalidation. + +## Accessibility and localization + +- Existing ARIA tree semantics and keyboard navigation remain intact. +- Menu and dialog controls are keyboard accessible; inline inputs have localized labels and errors. +- Every user-visible string exists in English and Simplified Chinese catalogs. + +## Acceptance criteria + +1. Every Main mutation rejects traversal, .git, root operations, symlink escape, descendant moves/copies, and destination conflicts in deterministic tests. +2. Create file/folder, rename, duplicate, cut/paste move, copy/paste, Trash, and Reveal are available from Files UI. +3. Rename/move preserves dirty open editor tabs and updates resource IDs; Trash warns and closes affected tabs only after success. +4. Failed operations do not optimistically corrupt tree or editor state. +5. Existing preview/pinned opening, keyboard tree navigation, ignore filtering, file saves, and watcher behavior remain covered. +6. `pnpm check` and `pnpm build` pass. diff --git a/docs/orca-aligned-source-control/intent.md b/docs/orca-aligned-source-control/intent.md new file mode 100644 index 0000000..8da95bc --- /dev/null +++ b/docs/orca-aligned-source-control/intent.md @@ -0,0 +1,20 @@ +# Intent: Orca-aligned Source Control + +## Problem + +SpireCode Changes is a safe, automatically refreshed Git status and diff viewer, but it does not yet support the source-control interactions users expect beside an editor. Status detail already returned by Main is underused, group headers imply collapse without supporting it, and common stage, unstage, discard, and commit workflows require leaving the panel for a terminal. + +## Outcome + +Turn Changes into a compact Source Control panel inspired by Orca while preserving SpireCode's Project -> Worktree -> Editor model, keyboard-accessible tree, automatic refresh, and strict Renderer/Main boundary. + +Success means users can filter and inspect accurate change metadata, stage or unstage individual/groups of files, safely discard eligible worktree changes, and commit staged changes without exposing paths, shell commands, or generic Git execution to Renderer. + +## Non-goals + +- Push, pull, sync, publish branch, or force push. +- Pull request or hosted review workflows. +- AI-generated commit messages or conflict repair. +- Git history or branch comparison. +- Multi-selection, virtualized rows, submodule expansion, or remote runtime routing. +- A manual refresh control; watcher-driven refresh remains authoritative. diff --git a/docs/orca-aligned-source-control/plan.md b/docs/orca-aligned-source-control/plan.md new file mode 100644 index 0000000..47b85a7 --- /dev/null +++ b/docs/orca-aligned-source-control/plan.md @@ -0,0 +1,25 @@ +# Plan: Orca-aligned Source Control + +## Phase 1: Rich read-only interaction + +1. Add a projection layer for conflicts, staged, unstaged, and untracked sections. +2. Add filtering, collapsible section headers, and an overflow view-mode menu without reintroducing manual refresh. +3. Render status-specific decoration, parent/original paths, numstat, active Diff state, and correct ARIA structure. +4. Add focused tests for projection, filtering, grouping, collapse, keyboard navigation, and status display. + +This phase is independently usable as a richer diff browser. + +## Phase 2: Safe Git mutations + +1. Add narrow command contracts, IPC argument validation, bindings, and GitService methods for stage, unstage, discard, and commit. +2. Extend backend tests first for normal operation, rename, path attacks, symlink escape, untracked deletion, Git failures, and per-worktree serialization. +3. Add row and section actions with mutation state and post-success refresh. +4. Add a destructive discard dialog and coordinate dirty/in-flight editor saves before mutation. +5. Add per-worktree commit drafts, eligibility, keyboard submit, single-flight execution, and post-commit refresh. +6. Verify focused tests, the full quality gate, production build, and final diff. + +This phase is independently usable without future remote, PR, history, AI, multi-select, or virtualization work. + +## Rollback + +The feature stores no migration state. Reverting the narrow commands, panel interactions, and styles restores the prior read-only viewer. Git mutations themselves are explicit user actions and are not automatically reversible; discard therefore remains confirmation-gated. diff --git a/docs/orca-aligned-source-control/spec.md b/docs/orca-aligned-source-control/spec.md new file mode 100644 index 0000000..95dc9f8 --- /dev/null +++ b/docs/orca-aligned-source-control/spec.md @@ -0,0 +1,71 @@ +# Spec: Orca-aligned Source Control + +## Panel structure + +- The header shows the unique changed-file count, a compact path filter, and an overflow menu containing List and Tree view choices. +- There is no manual Refresh action. +- Entries are projected into Conflicts, Staged Changes, Changes, and Untracked Files. Empty sections are omitted. +- Sections are collapsible buttons with `aria-expanded`, visible counts, and eligible section actions. +- Filtering matches the full relative path case-insensitively. Section-wide actions are hidden while filtering so visible results cannot be mistaken for the mutation scope. + +## Change rows + +- Rows retain roving tree focus, Arrow/Home/End navigation, Enter/Space preview opening, single-click preview, and double-click pinning. +- Rows show filename, muted parent path in list mode, accurate status labels and colors, rename origin when present, and available addition/deletion counts. +- Conflicts are shown before normal groups and are not presented as untracked changes. +- Active Diff rows remain visibly selected. +- Eligible hover/focus actions are Stage, Unstage, and Discard. + +## Git mutations + +Renderer receives only narrow commands: + +- `git_stage(worktreeId, relativePath)` +- `git_unstage(worktreeId, relativePath)`; Main derives any rename origin from current status +- `git_discard(worktreeId, relativePath, untracked)` +- `git_commit(worktreeId, message)` + +Main resolves worktree IDs to registered roots and validates every relative path. It rejects empty/absolute/traversal/NUL/`.git` paths and symlink escape. Git commands use argument arrays through the hardened runner, clear repository/index redirect environment variables, disable system Git config, and execute serially per worktree. Repositories selecting executable Git filters fail closed and must use the terminal for those operations. + +- Stage uses literal pathspec separation. +- Unstage restores the index from HEAD and includes both rename paths when required. +- Discard tracked restores only the selected worktree path. +- Discard untracked moves only the validated selected file or symlink entry to system Trash; it never invokes repository-wide clean, never follows the leaf symlink, and rejects directories. +- Commit requires a non-empty message within the IPC limit and commits staged content only. +- Mutation success and subsequent refresh failure are reported separately; successful mutations are never presented as failed solely because refresh failed. + +## Discard safety + +- Discard always requires an explicit destructive confirmation describing the target. +- Unresolved conflicts cannot be discarded from the panel. +- A dirty file cannot be discarded until its in-flight editor save has settled; the confirmation warns that unsaved content will be lost. +- After successful discard, file and diff resource caches are invalidated and visible editors reload from disk. For discarded untracked files, affected tabs are removed only after Main succeeds. +- Failure leaves editor state and caches intact. + +## Commit + +- Commit drafts are session-scoped and keyed by worktree ID. +- Commit is enabled only when staged entries exist, the trimmed message is non-empty, there is no unresolved conflict, and no mutation is active. +- Command/Ctrl+Enter submits only when Commit is eligible and ignores IME composition. +- Submission is single-flight. Success clears the draft only if the user has not edited it during the request, then refreshes status and open diffs. + +## Refresh and concurrency + +- Existing `git://changed` watcher refresh remains active. +- Renderer mutation state prevents duplicate operations. +- Changes and Diff generation guards continue to reject stale results. +- Main serializes status, diff, save, and Git mutations per worktree through the existing worktree operation queue boundary. + +## Accessibility + +- Header controls expose toolbar/group semantics and labels. +- Section headers expose expanded state and action labels. +- Row actions are keyboard-focusable and do not steal row preview activation. +- Dialogs trap focus, close on Escape when idle, and expose descriptions/errors through ARIA. + +## Acceptance + +- Happy paths: stage, unstage, tracked discard, untracked discard, and commit update the repository and panel correctly. +- Error paths: Git failure, path rejection, stale refresh failure, save conflict, and duplicate invocation preserve consistent state. +- Edge paths: rename, staged+unstaged file, deleted file, untracked file, unresolved conflict, detached HEAD, empty repository, and worktree switch are covered. +- `pnpm check`, `pnpm build`, and `git diff --check` pass. diff --git a/docs/project-files-surface-alignment/intent.md b/docs/project-files-surface-alignment/intent.md new file mode 100644 index 0000000..1f58247 --- /dev/null +++ b/docs/project-files-surface-alignment/intent.md @@ -0,0 +1,15 @@ +# Intent: Align Projects overlays with Files + +## Problem + +Projects currently uses older overlay styling for worktree dialogs, project/worktree action menus, and branch options. Their borders, radii, spacing, typography, hover states, and widths differ from the newer Files dialog and context-menu surfaces, making adjacent navigation modules feel unrelated. + +## Outcome + +Projects overlays use the same visual language as Files while retaining their existing behavior, accessibility semantics, and project/worktree workflows. + +## Non-goals + +- Changing project or worktree operations. +- Replacing existing dialog focus management. +- Changing Files behavior or introducing new dependencies. diff --git a/docs/project-files-surface-alignment/plan.md b/docs/project-files-surface-alignment/plan.md new file mode 100644 index 0000000..0f5c0e6 --- /dev/null +++ b/docs/project-files-surface-alignment/plan.md @@ -0,0 +1,6 @@ +# Plan: Align Projects overlays with Files + +1. Consolidate Projects menu selectors with the Files context-menu dimensions and semantic tokens. +2. Align worktree dialog geometry, typography, controls, and actions with the Files confirmation dialog. +3. Align branch option rows with Files menu rows while preserving listbox selection behavior. +4. Run focused Projects tests and the full repository quality gate. diff --git a/docs/project-files-surface-alignment/spec.md b/docs/project-files-surface-alignment/spec.md new file mode 100644 index 0000000..cbc1810 --- /dev/null +++ b/docs/project-files-surface-alignment/spec.md @@ -0,0 +1,18 @@ +# Spec: Align Projects overlays with Files + +## Dialogs + +- Worktree create, rename, and delete dialogs match the Files confirmation dialog's width, border, radius, background, shadow, title typography, and compact spacing. +- Form controls and action buttons continue to expose clear focus, primary, destructive, disabled, and pending states. + +## Menus + +- Project actions, worktree actions, and the origin-branch option list match the Files context menu's popover surface, border, radius, shadow, minimum row height, typography, and neutral hover state. +- Destructive actions remain visually distinct. +- Existing menu roles, listbox roles, keyboard behavior, and action semantics remain unchanged. + +## Acceptance + +- Projects and Files overlays use the same semantic surface and interaction tokens. +- Existing Projects and Files tests pass. +- Formatting, lint, type checking, and all repository tests pass. diff --git a/electron/appState.ts b/electron/appState.ts index 179e487..5a7bfd0 100644 --- a/electron/appState.ts +++ b/electron/appState.ts @@ -19,6 +19,7 @@ import { import { TerminalService } from "./domains/terminal/service.js"; import { WorktreeService } from "./domains/worktrees/index.js"; import { gitText } from "./core/gitProcess.js"; +import { KeyedLock } from "./core/keyedLock.js"; import { WindowCloseGuard } from "./windowCloseGuard.js"; import { DiagnosticsService } from "./domains/diagnostics/service.js"; import { resolveSpireResources } from "./domains/chat/spireSettings.js"; @@ -48,8 +49,16 @@ export class AppState { ) { this.diagnostics = new DiagnosticsService(dataDirectory, "0.1.0"); const root = (id: string) => projects.root(id); - this.filesystem = new FilesystemService(root); - this.git = new GitService(root, (id) => projects.assertGitWorktree(id)); + const worktreeOperationLock = new KeyedLock(); + this.filesystem = new FilesystemService(root, { + trashItem: (targetPath) => shell.trashItem(targetPath), + revealItem: async (targetPath) => shell.showItemInFolder(targetPath), + operationLock: worktreeOperationLock, + }); + this.git = new GitService(root, (id) => projects.assertGitWorktree(id), { + trashItem: (targetPath) => shell.trashItem(targetPath), + operationLock: worktreeOperationLock, + }); this.memory = new MemoryService(); this.models = new ModelCatalogService(); this.terminals = new TerminalService(root, (subscriptionId, payload) => { diff --git a/electron/contracts.test.ts b/electron/contracts.test.ts index ea1091b..26675b3 100644 --- a/electron/contracts.test.ts +++ b/electron/contracts.test.ts @@ -6,6 +6,17 @@ describe("Electron host allowlists", () => { for (const command of COMMANDS) expect(isCommand(command)).toBe(true); expect(COMMANDS).toEqual( expect.arrayContaining([ + "fs_create_file", + "fs_create_directory", + "fs_rename", + "fs_move", + "fs_copy", + "fs_trash", + "fs_reveal", + "git_stage", + "git_unstage", + "git_discard", + "git_commit", "chat_session_config", "chat_session_set_model", "chat_session_set_thinking_level", @@ -23,6 +34,8 @@ describe("Electron host allowlists", () => { expect(isCommand("chat_session_invoke")).toBe(false); expect(isCommand("pi_command")).toBe(false); expect(isCommand("fs_read_absolute")).toBe(false); + expect(isCommand("git_exec")).toBe(false); + expect(isCommand("git_reset_hard")).toBe(false); expect(isCommand("child_process.exec")).toBe(false); expect(isCommand(1)).toBe(false); }); diff --git a/electron/contracts.ts b/electron/contracts.ts index 11cc8b3..705bcf7 100644 --- a/electron/contracts.ts +++ b/electron/contracts.ts @@ -7,8 +7,19 @@ export const COMMANDS = [ "fs_read_dir", "fs_read_file", "fs_write_file", + "fs_create_file", + "fs_create_directory", + "fs_rename", + "fs_move", + "fs_copy", + "fs_trash", + "fs_reveal", "git_status", "git_diff_file", + "git_stage", + "git_unstage", + "git_discard", + "git_commit", "git_list_origin_branches", "worktree_create", "worktree_select", diff --git a/electron/core/gitProcess.ts b/electron/core/gitProcess.ts index 4ec1a93..b4a879c 100644 --- a/electron/core/gitProcess.ts +++ b/electron/core/gitProcess.ts @@ -18,8 +18,23 @@ const SAFE_CONFIG = [ "protocol.ext.allow=never", "-c", "core.fsmonitor=false", + "-c", + "commit.gpgSign=false", ]; +function safeGitEnvironment(): NodeJS.ProcessEnv { + const environment = Object.fromEntries( + Object.entries(process.env).filter( + ([key, value]) => !key.startsWith("GIT_") && value !== undefined, + ), + ); + return { + ...environment, + GIT_CONFIG_NOSYSTEM: "1", + GIT_TERMINAL_PROMPT: "0", + }; +} + export interface ProcessOutput { stdout: Buffer; stderr: Buffer; @@ -31,6 +46,7 @@ export interface GitRunOptions { maxOutput?: number; diffSafe?: boolean; allowFailure?: boolean; + input?: string | Buffer; } export async function runGit( @@ -47,8 +63,9 @@ export async function runGit( return new Promise((resolve, reject) => { const child = spawn("git", [...config, ...args], { cwd, + env: safeGitEnvironment(), shell: false, - stdio: ["ignore", "pipe", "pipe"], + stdio: [options.input === undefined ? "ignore" : "pipe", "pipe", "pipe"], }); const stdout: Buffer[] = []; const stderr: Buffer[] = []; @@ -83,8 +100,12 @@ export async function runGit( chunks.push(chunk); }; - child.stdout.on("data", collect(stdout, "stdout")); - child.stderr.on("data", collect(stderr, "stderr")); + child.stdin?.on("error", (error: NodeJS.ErrnoException) => { + if (error.code !== "EPIPE") fail(error); + }); + if (options.input !== undefined) child.stdin?.end(options.input); + child.stdout?.on("data", collect(stdout, "stdout")); + child.stderr?.on("data", collect(stderr, "stderr")); child.on("error", fail); child.on("close", (code, signal) => { if (settled) return; diff --git a/electron/core/keyedLock.test.ts b/electron/core/keyedLock.test.ts new file mode 100644 index 0000000..a71a265 --- /dev/null +++ b/electron/core/keyedLock.test.ts @@ -0,0 +1,22 @@ +// @vitest-environment node +import { describe, expect, it, vi } from "vitest"; +import { KeyedLock } from "./keyedLock.js"; + +describe("KeyedLock", () => { + it("serializes the same key while allowing different keys", async () => { + const lock = new KeyedLock(); + const releaseFirst = await lock.acquire("w1"); + const secondEntered = vi.fn(); + const second = lock.acquire("w1").then((release) => { + secondEntered(); + release(); + }); + const otherRelease = await lock.acquire("w2"); + otherRelease(); + await Promise.resolve(); + expect(secondEntered).not.toHaveBeenCalled(); + releaseFirst(); + await second; + expect(secondEntered).toHaveBeenCalledOnce(); + }); +}); diff --git a/electron/core/keyedLock.ts b/electron/core/keyedLock.ts new file mode 100644 index 0000000..15e25aa --- /dev/null +++ b/electron/core/keyedLock.ts @@ -0,0 +1,18 @@ +export class KeyedLock { + private readonly tails = new Map>(); + + async acquire(key: string): Promise<() => void> { + const previous = this.tails.get(key) ?? Promise.resolve(); + let release!: () => void; + const current = new Promise((resolve) => { + release = resolve; + }); + const tail = previous.then(() => current); + this.tails.set(key, tail); + await previous; + return () => { + release(); + if (this.tails.get(key) === tail) this.tails.delete(key); + }; + } +} diff --git a/electron/domains/filesystem/filesystem.test.ts b/electron/domains/filesystem/filesystem.test.ts index e8431e4..0471f4f 100644 --- a/electron/domains/filesystem/filesystem.test.ts +++ b/electron/domains/filesystem/filesystem.test.ts @@ -3,6 +3,7 @@ import { mkdtemp, mkdir, readFile, + readlink, realpath, rm, symlink, @@ -64,6 +65,10 @@ describe("resolveProjectPath", () => { resolveProjectPath(root, ".git/config", true), "OUTSIDE_PROJECT", ); + await expectCode( + resolveProjectPath(root, ".GIT/config", false), + "OUTSIDE_PROJECT", + ); await expectCode( resolveProjectPath(root, `folder${path.sep}.${path.sep}file`, false), "OUTSIDE_PROJECT", @@ -251,6 +256,152 @@ describe("FilesystemService", () => { ]); }); + it("creates files and directories exclusively inside existing parents", async () => { + const root = await temporaryDirectory(); + await mkdir(path.join(root, "src")); + const service = new FilesystemService(() => root); + + await expect( + service.createFile("worktree-1", "src/new.ts"), + ).resolves.toMatchObject({ + operation: "create", + destinationRelativePath: "src/new.ts", + kind: "file", + }); + await expect(readFile(path.join(root, "src/new.ts"), "utf8")).resolves.toBe( + "", + ); + await expect( + service.createDirectory("worktree-1", "src/components"), + ).resolves.toMatchObject({ + operation: "create", + destinationRelativePath: "src/components", + kind: "directory", + }); + await expectCode(service.createFile("worktree-1", "src/new.ts"), "EEXIST"); + await expectCode( + service.createFile("worktree-1", "missing/new.ts"), + "NOT_FOUND", + ); + }); + + it("renames and moves entries without overwriting or moving a directory into itself", async () => { + const root = await temporaryDirectory(); + await mkdir(path.join(root, "src/nested"), { recursive: true }); + await mkdir(path.join(root, "dest")); + await writeFile(path.join(root, "src/file.ts"), "content"); + await writeFile(path.join(root, "dest/existing.ts"), "existing"); + const service = new FilesystemService(() => root); + + await expect( + service.renameEntry("worktree-1", "src/file.ts", "renamed.ts"), + ).resolves.toMatchObject({ + operation: "move", + sourceRelativePath: "src/file.ts", + destinationRelativePath: "src/renamed.ts", + kind: "file", + }); + await expect( + service.moveEntry("worktree-1", "src/renamed.ts", "dest/renamed.ts"), + ).resolves.toMatchObject({ destinationRelativePath: "dest/renamed.ts" }); + await expectCode( + service.moveEntry("worktree-1", "dest/renamed.ts", "dest/existing.ts"), + "EEXIST", + ); + await expectCode( + service.moveEntry("worktree-1", "src", "src/nested/moved"), + "INVALID_ARGUMENT", + ); + }); + + it("recursively copies directories and preserves safe symlinks without following them", async () => { + const root = await temporaryDirectory(); + await mkdir(path.join(root, "src/nested"), { recursive: true }); + await writeFile(path.join(root, "src/nested/file.ts"), "content"); + if (process.platform !== "win32") { + await symlink("nested/file.ts", path.join(root, "src/link.ts")); + } + const service = new FilesystemService(() => root); + + await expect( + service.copyEntry("worktree-1", "src", "copy"), + ).resolves.toMatchObject({ + operation: "copy", + sourceRelativePath: "src", + destinationRelativePath: "copy", + kind: "directory", + }); + await expect( + readFile(path.join(root, "copy/nested/file.ts"), "utf8"), + ).resolves.toBe("content"); + if (process.platform !== "win32") { + await expect(readlink(path.join(root, "copy/link.ts"))).resolves.toBe( + "nested/file.ts", + ); + } + await expectCode( + service.copyEntry("worktree-1", "src", "src/nested/copy"), + "INVALID_ARGUMENT", + ); + }); + + it("moves entries to Trash only through the injected operation", async () => { + const root = await temporaryDirectory(); + await writeFile(path.join(root, "trash.txt"), "trash"); + const trashItem = vi.fn(async (target: string) => rm(target)); + const service = new FilesystemService(() => root, { trashItem }); + + await expect( + service.trashEntry("worktree-1", "trash.txt"), + ).resolves.toMatchObject({ + operation: "trash", + sourceRelativePath: "trash.txt", + kind: "file", + }); + expect(trashItem).toHaveBeenCalledWith( + path.join(await realpath(root), "trash.txt"), + ); + await expectCode(service.trashEntry("worktree-1", ""), "OUTSIDE_PROJECT"); + }); + + it("rejects mutation traversal, .git access, unsafe names, and symlink-parent escape", async () => { + const root = await temporaryDirectory(); + await mkdir(path.join(root, ".git")); + await writeFile(path.join(root, "file.ts"), "content"); + const service = new FilesystemService(() => root, { + trashItem: async () => undefined, + }); + + await expectCode( + service.createFile("worktree-1", "../escape"), + "OUTSIDE_PROJECT", + ); + await expectCode( + service.createFile("worktree-1", ".git/config"), + "OUTSIDE_PROJECT", + ); + await expectCode( + service.copyEntry("worktree-1", ".git", "git-copy"), + "OUTSIDE_PROJECT", + ); + await expectCode( + service.trashEntry("worktree-1", ".git"), + "OUTSIDE_PROJECT", + ); + await expectCode( + service.renameEntry("worktree-1", "file.ts", "../escape"), + "INVALID_ARGUMENT", + ); + if (process.platform !== "win32") { + const outside = await temporaryDirectory("spirecode-outside-"); + await symlink(outside, path.join(root, "escape")); + await expectCode( + service.createFile("worktree-1", "escape/new.ts"), + "OUTSIDE_PROJECT", + ); + } + }); + it("rejects directories, binary data, invalid UTF-8, and oversized files", async () => { const root = await temporaryDirectory(); await mkdir(path.join(root, "directory")); diff --git a/electron/domains/filesystem/pathGuard.ts b/electron/domains/filesystem/pathGuard.ts index 04807c2..6255edb 100644 --- a/electron/domains/filesystem/pathGuard.ts +++ b/electron/domains/filesystem/pathGuard.ts @@ -36,7 +36,7 @@ function validateRelativePath(relativePath: string): void { "path must contain only normal relative components", ); } - if (components.includes(".git")) { + if (components.some((component) => component.toLowerCase() === ".git")) { throw new CommandError("OUTSIDE_PROJECT", ".git is not accessible"); } } diff --git a/electron/domains/filesystem/service.ts b/electron/domains/filesystem/service.ts index d20dc80..be02541 100644 --- a/electron/domains/filesystem/service.ts +++ b/electron/domains/filesystem/service.ts @@ -1,19 +1,23 @@ import { chmod, lstat, + mkdir, open, readFile as fsReadFile, readdir, + readlink, realpath, rename, rm, stat, + symlink, } from "node:fs/promises"; import { constants as fsConstants, type Stats } from "node:fs"; import { createHash, randomUUID } from "node:crypto"; import path from "node:path"; import ignore, { type Ignore } from "ignore"; import { CommandError, toCommandError } from "../../core/errors.js"; +import { KeyedLock } from "../../core/keyedLock.js"; import { resolveProjectPath } from "./pathGuard.js"; export const MAX_TEXT_BYTES = 5 * 1024 * 1024; @@ -34,6 +38,19 @@ export interface FileContent { version: string; } +export interface FilesystemMutationResult { + operation: "create" | "move" | "copy" | "trash"; + kind: EntryKind; + sourceRelativePath?: string; + destinationRelativePath?: string; +} + +export interface FilesystemServiceOptions { + trashItem?: (targetPath: string) => Promise; + revealItem?: (targetPath: string) => Promise; + operationLock?: KeyedLock; +} + export type RootResolver = (worktreeId: string) => string | Promise; const naturalCollator = new Intl.Collator("en", { @@ -208,8 +225,280 @@ function isIgnored( return ignored; } +function entryKind(metadata: Stats): EntryKind { + return metadata.isSymbolicLink() + ? "symlink" + : metadata.isDirectory() + ? "directory" + : "file"; +} + +function assertMutationPath(relativePath: string): void { + const components = relativePath.split(/[\\/]/); + if ( + !relativePath || + relativePath.includes("\0") || + path.isAbsolute(relativePath) || + path.posix.isAbsolute(relativePath) || + path.win32.isAbsolute(relativePath) || + components.some( + (component) => + component === "." || + component === ".." || + component.toLowerCase() === ".git", + ) + ) { + throw new CommandError("OUTSIDE_PROJECT", "path is outside the project"); + } +} + +function sameEntry(current: Stats, expected: Stats): boolean { + return ( + current.dev === expected.dev && + current.ino === expected.ino && + current.size === expected.size && + current.mtimeMs === expected.mtimeMs + ); +} + +async function assertSameEntry( + targetPath: string, + expected: Stats, +): Promise { + const current = await lstat(targetPath); + if (!sameEntry(current, expected)) { + throw new CommandError( + "FILE_CONFLICT", + "filesystem entry changed during operation", + ); + } +} + +function assertLeafName(name: string): void { + const windowsStem = name.split(".")[0]?.toUpperCase(); + const windowsReserved = + windowsStem === "CON" || + windowsStem === "PRN" || + windowsStem === "AUX" || + windowsStem === "NUL" || + /^COM[1-9]$/.test(windowsStem ?? "") || + /^LPT[1-9]$/.test(windowsStem ?? ""); + if ( + !name || + name === "." || + name === ".." || + name.toLowerCase() === ".git" || + name.includes("\0") || + name.includes("/") || + name.includes("\\") || + /[<>:"|?*]/.test(name) || + [...name].some((character) => character.charCodeAt(0) < 32) || + /[. ]$/.test(name) || + windowsReserved + ) { + throw new CommandError( + "INVALID_ARGUMENT", + "name must be a single portable path component", + ); + } +} + +function isWithinRoot(root: string, candidate: string): boolean { + const relative = path.relative(root, candidate); + return ( + relative === "" || + (!relative.startsWith(`..${path.sep}`) && + relative !== ".." && + !path.isAbsolute(relative)) + ); +} + +async function expectMissing(targetPath: string): Promise { + try { + await lstat(targetPath); + throw new CommandError("EEXIST", "destination already exists"); + } catch (error) { + if (error instanceof CommandError) throw error; + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } +} + +async function resolveExistingEntry( + rootPath: string, + relativePath: string, +): Promise<{ root: string; path: string; metadata: Stats; kind: EntryKind }> { + assertMutationPath(relativePath); + const root = await realpath(rootPath); + const parentRelative = path.dirname(relativePath); + const parent = await resolveProjectPath( + root, + parentRelative === "." ? "" : parentRelative, + true, + ); + const targetPath = path.join(parent.path, path.basename(relativePath)); + const metadata = await lstat(targetPath); + if (metadata.isSymbolicLink()) { + await resolveProjectPath(root, relativePath, true); + } + return { root, path: targetPath, metadata, kind: entryKind(metadata) }; +} + +async function resolveMissingDestination( + rootPath: string, + relativePath: string, +): Promise<{ root: string; path: string }> { + assertMutationPath(relativePath); + const components = relativePath.split(/[\\/]/); + assertLeafName(components[components.length - 1] ?? ""); + const root = await realpath(rootPath); + const parentRelative = path.dirname(relativePath); + const parent = await resolveProjectPath( + root, + parentRelative === "." ? "" : parentRelative, + true, + ); + const targetPath = path.join(parent.path, path.basename(relativePath)); + const guarded = await resolveProjectPath(root, relativePath, false); + if (guarded.path !== targetPath) { + throw new CommandError( + "OUTSIDE_PROJECT", + "destination parent is outside project", + ); + } + await expectMissing(targetPath); + return { root, path: targetPath }; +} + +async function copyRegularFileNoFollow( + sourcePath: string, + destinationPath: string, + expected: Stats, +): Promise { + const source = await openNoFollow(sourcePath, fsConstants.O_RDONLY); + let destination: Awaited> | undefined; + try { + const current = await source.stat(); + if (!current.isFile() || !sameEntry(current, expected)) { + throw new CommandError("FILE_CONFLICT", "source changed during copy"); + } + destination = await openNoFollow( + destinationPath, + fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL, + expected.mode, + ); + const buffer = Buffer.allocUnsafe(64 * 1024); + let position = 0; + for (;;) { + const { bytesRead } = await source.read( + buffer, + 0, + buffer.length, + position, + ); + if (bytesRead === 0) break; + let written = 0; + while (written < bytesRead) { + const result = await destination.write( + buffer, + written, + bytesRead - written, + position + written, + ); + written += result.bytesWritten; + } + position += bytesRead; + } + const completed = await source.stat(); + if (!sameEntry(completed, expected)) { + throw new CommandError("FILE_CONFLICT", "source changed during copy"); + } + await destination.sync(); + } catch (error) { + await destination?.close().catch(() => {}); + destination = undefined; + await rm(destinationPath, { force: true }).catch(() => {}); + throw error; + } finally { + await destination?.close().catch(() => {}); + await source.close(); + } + await chmod(destinationPath, expected.mode); +} + +async function copyEntryNoFollow( + root: string, + sourcePath: string, + destinationPath: string, + metadata: Stats, +): Promise { + if (metadata.isSymbolicLink()) { + const target = await readlink(sourcePath); + const resolvedTarget = path.isAbsolute(target) + ? path.normalize(target) + : path.resolve(path.dirname(destinationPath), target); + if (!isWithinRoot(root, resolvedTarget)) { + throw new CommandError( + "OUTSIDE_PROJECT", + "copied symlink would escape project", + ); + } + const relativeTarget = path.relative(root, resolvedTarget); + await resolveProjectPath(root, relativeTarget, false); + await symlink(target, destinationPath); + return; + } + if (metadata.isDirectory()) { + await assertSameEntry(sourcePath, metadata); + if ((await realpath(sourcePath)) !== sourcePath) { + throw new CommandError( + "OUTSIDE_PROJECT", + "source directory changed during copy", + ); + } + await mkdir(destinationPath); + // On failure, leave a partial directory rather than recursively deleting + // paths that another process may have concurrently populated or replaced. + for (const child of await readdir(sourcePath, { withFileTypes: true })) { + await assertSameEntry(sourcePath, metadata); + if (child.name === ".git") continue; + const childSource = path.join(sourcePath, child.name); + const childDestination = path.join(destinationPath, child.name); + const childMetadata = await lstat(childSource); + if (childMetadata.isSymbolicLink()) { + const relativeSource = path.relative(root, childSource); + await resolveProjectPath(root, relativeSource, true); + } + await copyEntryNoFollow( + root, + childSource, + childDestination, + childMetadata, + ); + } + return; + } + if (!metadata.isFile()) { + throw new CommandError( + "UNSUPPORTED_FILE", + "special files cannot be copied", + ); + } + await copyRegularFileNoFollow(sourcePath, destinationPath, metadata); +} + export class FilesystemService { - constructor(private readonly rootResolver: RootResolver) {} + private readonly operationLock: KeyedLock; + + constructor( + private readonly rootResolver: RootResolver, + private readonly options: FilesystemServiceOptions = {}, + ) { + this.operationLock = options.operationLock ?? new KeyedLock(); + } + + private acquire(worktreeId: string): Promise<() => void> { + return this.operationLock.acquire(worktreeId); + } async readDir( worktreeId: string, @@ -274,6 +563,217 @@ export class FilesystemService { } } + async createFile( + worktreeId: string, + relativePath: string, + ): Promise { + const release = await this.acquire(worktreeId); + let handle: Awaited> | undefined; + try { + const root = await this.rootResolver(worktreeId); + const destination = await resolveMissingDestination(root, relativePath); + handle = await openNoFollow( + destination.path, + fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL, + 0o644, + ); + await handle.sync(); + await handle.close(); + handle = undefined; + await syncDirectory(path.dirname(destination.path)); + return { + operation: "create", + kind: "file", + destinationRelativePath: relativePath, + }; + } catch (error) { + await handle?.close().catch(() => {}); + throw toCommandError(error); + } finally { + release(); + } + } + + async createDirectory( + worktreeId: string, + relativePath: string, + ): Promise { + const release = await this.acquire(worktreeId); + try { + const root = await this.rootResolver(worktreeId); + const destination = await resolveMissingDestination(root, relativePath); + await mkdir(destination.path); + await syncDirectory(path.dirname(destination.path)); + return { + operation: "create", + kind: "directory", + destinationRelativePath: relativePath, + }; + } catch (error) { + throw toCommandError(error); + } finally { + release(); + } + } + + async renameEntry( + worktreeId: string, + relativePath: string, + newName: string, + ): Promise { + assertLeafName(newName); + const parent = path.dirname(relativePath); + const destination = path.join(parent === "." ? "" : parent, newName); + return this.moveEntry(worktreeId, relativePath, destination); + } + + async moveEntry( + worktreeId: string, + sourceRelativePath: string, + destinationRelativePath: string, + ): Promise { + const release = await this.acquire(worktreeId); + let copied = false; + let destinationPath: string | undefined; + let destinationIdentity: Stats | undefined; + let sourceKind: EntryKind | undefined; + try { + const rootPath = await this.rootResolver(worktreeId); + const source = await resolveExistingEntry(rootPath, sourceRelativePath); + const destination = await resolveMissingDestination( + rootPath, + destinationRelativePath, + ); + destinationPath = destination.path; + sourceKind = source.kind; + if ( + source.metadata.isDirectory() && + isWithinRoot(source.path, destination.path) + ) { + throw new CommandError( + "INVALID_ARGUMENT", + "directory cannot be moved into itself", + ); + } + await copyEntryNoFollow( + source.root, + source.path, + destination.path, + source.metadata, + ); + copied = true; + destinationIdentity = await lstat(destination.path); + await assertSameEntry(source.path, source.metadata); + await rm(source.path, { + recursive: source.metadata.isDirectory(), + force: false, + }); + await syncDirectory(path.dirname(source.path)); + await syncDirectory(path.dirname(destination.path)); + return { + operation: "move", + kind: source.kind, + sourceRelativePath, + destinationRelativePath, + }; + } catch (error) { + if ( + copied && + destinationPath && + destinationIdentity && + sourceKind !== "directory" + ) { + const cleanupPath = destinationPath; + await assertSameEntry(cleanupPath, destinationIdentity) + .then(() => rm(cleanupPath, { force: true })) + .catch(() => {}); + } + throw toCommandError(error); + } finally { + release(); + } + } + + async copyEntry( + worktreeId: string, + sourceRelativePath: string, + destinationRelativePath: string, + ): Promise { + const release = await this.acquire(worktreeId); + try { + const rootPath = await this.rootResolver(worktreeId); + const source = await resolveExistingEntry(rootPath, sourceRelativePath); + const destination = await resolveMissingDestination( + rootPath, + destinationRelativePath, + ); + if ( + source.metadata.isDirectory() && + isWithinRoot(source.path, destination.path) + ) { + throw new CommandError( + "INVALID_ARGUMENT", + "directory cannot be copied into itself", + ); + } + await copyEntryNoFollow( + source.root, + source.path, + destination.path, + source.metadata, + ); + await syncDirectory(path.dirname(destination.path)); + return { + operation: "copy", + kind: source.kind, + sourceRelativePath, + destinationRelativePath, + }; + } catch (error) { + throw toCommandError(error); + } finally { + release(); + } + } + + async trashEntry( + worktreeId: string, + relativePath: string, + ): Promise { + const release = await this.acquire(worktreeId); + try { + if (!this.options.trashItem) { + throw new CommandError("INVALID_ARGUMENT", "Trash is unavailable"); + } + const root = await this.rootResolver(worktreeId); + const source = await resolveExistingEntry(root, relativePath); + await assertSameEntry(source.path, source.metadata); + await this.options.trashItem(source.path); + return { + operation: "trash", + kind: source.kind, + sourceRelativePath: relativePath, + }; + } catch (error) { + throw toCommandError(error); + } finally { + release(); + } + } + + async revealEntry(worktreeId: string, relativePath: string): Promise { + try { + if (!this.options.revealItem) { + throw new CommandError("INVALID_ARGUMENT", "Reveal is unavailable"); + } + const root = await this.rootResolver(worktreeId); + const source = await resolveExistingEntry(root, relativePath); + await this.options.revealItem(source.path); + } catch (error) { + throw toCommandError(error); + } + } + async readFile( worktreeId: string, relativePath: string, @@ -311,6 +811,7 @@ export class FilesystemService { content: string, expectedVersion: string, ): Promise { + const release = await this.acquire(worktreeId); let temporaryPath: string | undefined; try { const root = await this.rootResolver(worktreeId); @@ -382,6 +883,8 @@ export class FilesystemService { if (temporaryPath) await rm(temporaryPath, { force: true }).catch(() => {}); throw toCommandError(error); + } finally { + release(); } } } diff --git a/electron/domains/git/git.test.ts b/electron/domains/git/git.test.ts index ada4ae6..6ac470d 100644 --- a/electron/domains/git/git.test.ts +++ b/electron/domains/git/git.test.ts @@ -5,6 +5,7 @@ import { mkdtemp, mkdir, readFile, + realpath, rm, symlink, unlink, @@ -37,6 +38,8 @@ async function temporaryRepository(): Promise { "-qm", "initial", ]); + await git(root, ["config", "user.name", "SpireCode"]); + await git(root, ["config", "user.email", "test@example.invalid"]); return root; } @@ -164,6 +167,68 @@ describe("safe Git runner", () => { }, ); + it("ignores inherited Git repository and index redirects", async () => { + const root = await temporaryRepository(); + const other = await temporaryRepository(); + const previous = { + gitDir: process.env.GIT_DIR, + workTree: process.env.GIT_WORK_TREE, + indexFile: process.env.GIT_INDEX_FILE, + }; + process.env.GIT_DIR = path.join(other, ".git"); + process.env.GIT_WORK_TREE = other; + process.env.GIT_INDEX_FILE = path.join(other, ".git", "index"); + try { + const topLevel = await safeGitText(root, [ + "rev-parse", + "--show-toplevel", + ]); + expect(await realpath(topLevel.trim())).toBe(await realpath(root)); + } finally { + if (previous.gitDir === undefined) delete process.env.GIT_DIR; + else process.env.GIT_DIR = previous.gitDir; + if (previous.workTree === undefined) delete process.env.GIT_WORK_TREE; + else process.env.GIT_WORK_TREE = previous.workTree; + if (previous.indexFile === undefined) delete process.env.GIT_INDEX_FILE; + else process.env.GIT_INDEX_FILE = previous.indexFile; + } + }); + + it.runIf(process.platform !== "win32")( + "does not execute filters from inherited global Git config", + async () => { + const root = await temporaryRepository(); + const marker = path.join(root, "global-filter-ran"); + const script = path.join(root, "global-filter.sh"); + const globalConfig = path.join(root, "global.gitconfig"); + await writeFile(script, `#!/bin/sh\ntouch '${marker}'\ncat\n`); + await chmod(script, 0o755); + await writeFile( + globalConfig, + `[filter "evil"]\n\tclean = ${script}\n\tsmudge = ${script}\n`, + ); + await writeFile( + path.join(root, ".gitattributes"), + "tracked.txt filter=evil\n", + ); + await writeFile(path.join(root, "tracked.txt"), "changed\n"); + const previous = process.env.GIT_CONFIG_GLOBAL; + process.env.GIT_CONFIG_GLOBAL = globalConfig; + try { + await expectCode( + new GitService(() => root).status("worktree-1"), + "GIT_UNSAFE_CONFIG", + ); + await expect(readFile(marker, "utf8")).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + if (previous === undefined) delete process.env.GIT_CONFIG_GLOBAL; + else process.env.GIT_CONFIG_GLOBAL = previous; + } + }, + ); + it("applies the non-interactive security configuration", async () => { const root = await temporaryRepository(); @@ -232,6 +297,22 @@ describe("GitService", () => { service.diffFile("directory-1", "file.txt", "unstaged"), "NOT_A_GIT_REPOSITORY", ); + await expectCode( + service.stage("directory-1", "file.txt"), + "NOT_A_GIT_REPOSITORY", + ); + await expectCode( + service.unstage("directory-1", "file.txt"), + "NOT_A_GIT_REPOSITORY", + ); + await expectCode( + service.discard("directory-1", "file.txt", false), + "NOT_A_GIT_REPOSITORY", + ); + await expectCode( + service.commit("directory-1", "message"), + "NOT_A_GIT_REPOSITORY", + ); expect(resolved).toBe(false); }); @@ -308,6 +389,180 @@ describe("GitService", () => { }); }); + it("stages and unstages tracked, deleted, and untracked changes", async () => { + const root = await temporaryRepository(); + const service = new GitService(() => root); + await writeFile(path.join(root, "tracked.txt"), "changed\n"); + await writeFile(path.join(root, "new.txt"), "new\n"); + + await service.stage("worktree-1", "tracked.txt"); + await service.stage("worktree-1", "new.txt"); + expect((await service.status("worktree-1")).changes).toEqual([ + expect.objectContaining({ path: "new.txt", staged: true }), + expect.objectContaining({ path: "tracked.txt", staged: true }), + ]); + + await service.unstage("worktree-1", "tracked.txt"); + await service.unstage("worktree-1", "new.txt"); + expect((await service.status("worktree-1")).changes).toEqual([ + expect.objectContaining({ path: "new.txt", untracked: true }), + expect.objectContaining({ path: "tracked.txt", unstaged: true }), + ]); + + await unlink(path.join(root, "tracked.txt")); + await service.stage("worktree-1", "tracked.txt"); + expect((await service.status("worktree-1")).changes).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: "tracked.txt", staged: true }), + ]), + ); + }); + + it("unstages both sides of a staged rename", async () => { + const root = await temporaryRepository(); + await git(root, ["mv", "tracked.txt", "renamed.txt"]); + const service = new GitService(() => root); + + await service.unstage("worktree-1", "renamed.txt"); + + const status = await service.status("worktree-1"); + expect(status.changes).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: "tracked.txt", unstaged: true }), + expect.objectContaining({ path: "renamed.txt", untracked: true }), + ]), + ); + expect(status.changes.some((change) => change.staged)).toBe(false); + }); + + it("unstages a new file in a repository without HEAD", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "spirecode-unborn-")); + temporaryDirectories.push(root); + await git(root, ["init", "-q"]); + await writeFile(path.join(root, "new.txt"), "new\n"); + const service = new GitService(() => root); + await service.stage("worktree-1", "new.txt"); + + await service.unstage("worktree-1", "new.txt"); + + expect((await service.status("worktree-1")).changes).toEqual([ + expect.objectContaining({ path: "new.txt", untracked: true }), + ]); + }); + + it("discards tracked changes without changing the staged snapshot", async () => { + const root = await temporaryRepository(); + const service = new GitService(() => root); + await writeFile(path.join(root, "tracked.txt"), "index\n"); + await service.stage("worktree-1", "tracked.txt"); + await writeFile(path.join(root, "tracked.txt"), "worktree\n"); + + await service.discard("worktree-1", "tracked.txt", false); + + expect(await readFile(path.join(root, "tracked.txt"), "utf8")).toBe( + "index\n", + ); + expect((await service.status("worktree-1")).changes).toEqual([ + expect.objectContaining({ staged: true, unstaged: false }), + ]); + }); + + it("moves only a currently untracked file to Trash", async () => { + const root = await temporaryRepository(); + const trashed: string[] = []; + const service = new GitService(() => root, undefined, { + trashItem: async (target) => { + trashed.push(target); + await unlink(target); + }, + }); + await writeFile(path.join(root, "new.txt"), "new\n"); + + await service.discard("worktree-1", "new.txt", true); + + expect(trashed).toEqual([path.join(await realpath(root), "new.txt")]); + expect((await service.status("worktree-1")).changes).toEqual([]); + await expectCode( + service.discard("worktree-1", "tracked.txt", true), + "GIT_SCOPE_CHANGED", + ); + + if (process.platform !== "win32") { + const outside = await mkdtemp(path.join(os.tmpdir(), "spirecode-link-")); + temporaryDirectories.push(outside); + const outsideFile = path.join(outside, "outside.txt"); + await writeFile(outsideFile, "keep\n"); + await symlink(outsideFile, path.join(root, "link.txt")); + await service.discard("worktree-1", "link.txt", true); + expect(await readFile(outsideFile, "utf8")).toBe("keep\n"); + expect(trashed.at(-1)).toBe(path.join(await realpath(root), "link.txt")); + } + }); + + it("commits staged changes only and rejects empty commits", async () => { + const root = await temporaryRepository(); + const service = new GitService(() => root); + await writeFile(path.join(root, "tracked.txt"), "index\n"); + await service.stage("worktree-1", "tracked.txt"); + await writeFile(path.join(root, "tracked.txt"), "worktree\n"); + + await service.commit("worktree-1", "source control commit"); + + expect( + (await execute("git", ["show", "-s", "--format=%s"], { cwd: root })) + .stdout, + ).toBe("source control commit\n"); + expect((await service.status("worktree-1")).changes).toEqual([ + expect.objectContaining({ staged: false, unstaged: true }), + ]); + await expectCode( + service.commit("worktree-1", "again"), + "NOTHING_TO_COMMIT", + ); + await expectCode(service.commit("worktree-1", " "), "INVALID_ARGUMENT"); + }); + + it("rejects stale scopes, unsafe paths, and paths with executable filters", async () => { + const root = await temporaryRepository(); + const marker = path.join(root, "filter-ran"); + const script = path.join(root, "filter.sh"); + await writeFile(script, `#!/bin/sh\ntouch '${marker}'\ncat\n`); + await chmod(script, 0o755); + await git(root, ["config", "filter.evil.clean", script]); + await writeFile( + path.join(root, ".gitattributes"), + "tracked.txt filter=evil\n", + ); + await writeFile(path.join(root, "tracked.txt"), "changed\n"); + const service = new GitService(() => root); + + await expectCode( + service.stage("worktree-1", "tracked.txt"), + "GIT_UNSAFE_CONFIG", + ); + await expect(readFile(marker, "utf8")).rejects.toMatchObject({ + code: "ENOENT", + }); + await expectCode( + service.unstage("worktree-1", "tracked.txt"), + "GIT_UNSAFE_CONFIG", + ); + for (const unsafePath of [ + "", + "../outside", + "/tmp/outside", + "C:\\outside", + "a\0b", + ".git/config", + ".GIT/config", + "a//b", + ]) + await expectCode( + service.stage("worktree-1", unsafePath), + "OUTSIDE_PROJECT", + ); + }); + it("rejects invalid scopes and paths outside the worktree", async () => { const root = await temporaryRepository(); const service = new GitService(() => root); diff --git a/electron/domains/git/service.ts b/electron/domains/git/service.ts index 1641012..5ea490f 100644 --- a/electron/domains/git/service.ts +++ b/electron/domains/git/service.ts @@ -1,6 +1,7 @@ -import { readFile, realpath, stat } from "node:fs/promises"; +import { lstat, readFile, realpath, stat } from "node:fs/promises"; import path from "node:path"; import { CommandError, toCommandError } from "../../core/errors.js"; +import { KeyedLock } from "../../core/keyedLock.js"; import { applyNumstat, parseStatus } from "./parser.js"; import { runSafeGit, safeGitObjectExists, safeGitText } from "./safeRunner.js"; import type { DiffScope, GitDiff, GitStatus } from "./types.js"; @@ -12,44 +13,181 @@ export type GitCapabilityResolver = ( worktreeId: string, ) => void | Promise; +export interface GitServiceOptions { + trashItem?: (targetPath: string) => Promise; + operationLock?: KeyedLock; +} + +const defaultTrashItem = async () => { + throw new CommandError("UNSUPPORTED_OPERATION", "Trash is unavailable"); +}; + export class GitService { - private readonly queues = new Map>(); + private readonly operationLock: KeyedLock; constructor( private readonly rootResolver: RootResolver, private readonly assertGit: GitCapabilityResolver = () => undefined, - ) {} + private readonly options: GitServiceOptions = {}, + ) { + this.operationLock = options.operationLock ?? new KeyedLock(); + } status(worktreeId: string): Promise { return this.serial(worktreeId, async () => { await this.assertGit(worktreeId); - const root = await this.rootResolver(worktreeId); - const output = await safeGitText(root, [ - "status", - "--porcelain=v2", - "--branch", - "-z", - "--untracked-files=all", + return this.statusAtRoot(await this.rootResolver(worktreeId)); + }); + } + + stage(worktreeId: string, relativePath: string): Promise { + return this.serial(worktreeId, async () => { + await this.assertGit(worktreeId); + const root = await realpath(await this.rootResolver(worktreeId)); + await resolveSafePath(root, relativePath, false); + const change = findChange(await this.statusAtRoot(root), relativePath); + if (!change || (!change.unstaged && !change.untracked)) + throw scopeChanged(relativePath); + const paths = change.originalPath + ? [change.originalPath, change.path] + : [change.path]; + for (const candidate of paths) { + await resolveSafePath(root, candidate, false); + await assertNoGitFilter(root, candidate); + } + await runSafeGit(root, [ + "--literal-pathspecs", + "add", + "--all", + "--", + ...paths, ]); - const result = parseStatus(output); - const staged = await safeGitText(root, [ - "diff", - "--cached", - "--numstat", - "-z", - "--no-ext-diff", - "--no-textconv", + }); + } + + unstage(worktreeId: string, relativePath: string): Promise { + return this.serial(worktreeId, async () => { + await this.assertGit(worktreeId); + const root = await realpath(await this.rootResolver(worktreeId)); + await resolveSafePath(root, relativePath, false); + const change = findChange(await this.statusAtRoot(root), relativePath); + if (!change?.staged) throw scopeChanged(relativePath); + const paths = change.originalPath + ? [change.originalPath, change.path] + : [change.path]; + for (const candidate of paths) + await resolveSafePath(root, candidate, false); + if (await safeGitObjectExists(root, "HEAD")) { + await runSafeGit(root, [ + "--literal-pathspecs", + "restore", + "--source=HEAD", + "--staged", + "--", + ...paths, + ]); + } else { + await runSafeGit(root, [ + "--literal-pathspecs", + "rm", + "--cached", + "--ignore-unmatch", + "--", + ...paths, + ]); + } + }); + } + + discard( + worktreeId: string, + relativePath: string, + untracked: boolean, + ): Promise { + return this.serial(worktreeId, async () => { + await this.assertGit(worktreeId); + const root = await realpath(await this.rootResolver(worktreeId)); + const target: { + path: string; + metadata?: Awaited>; + parentPath?: string; + parentMetadata?: Awaited>; + } = untracked + ? await resolveSafeEntryNoFollow(root, relativePath) + : { path: await resolveSafePath(root, relativePath, false) }; + const beforeStatus = await fingerprint(target.path); + const change = findChange(await this.statusAtRoot(root), relativePath); + if (!change || change.untracked !== untracked) + throw scopeChanged(relativePath); + if (untracked) { + const metadata = await lstat(target.path); + const parentMetadata = target.parentPath + ? await lstat(target.parentPath) + : undefined; + if (!metadata.isFile() && !metadata.isSymbolicLink()) { + throw new CommandError( + "UNSUPPORTED_FILE", + "only untracked files can be moved to Trash", + ); + } + if ( + (target.metadata && !sameMetadata(metadata, target.metadata)) || + (target.parentMetadata && + parentMetadata && + !sameMetadata(parentMetadata, target.parentMetadata)) || + !sameFingerprint(await fingerprint(target.path), beforeStatus) + ) + throw new CommandError( + "FILE_CONFLICT", + "untracked file changed before it could be moved to Trash", + ); + await (this.options.trashItem ?? defaultTrashItem)(target.path); + return; + } + if (!change.unstaged || isConflict(change.status)) + throw scopeChanged(relativePath); + await assertNoGitFilter(root, relativePath); + if (!sameFingerprint(await fingerprint(target.path), beforeStatus)) + throw new CommandError( + "FILE_CONFLICT", + "file changed before its changes could be discarded", + ); + await runSafeGit(root, [ + "--literal-pathspecs", + "restore", + "--worktree", + "--", + relativePath, ]); - const unstaged = await safeGitText(root, [ - "diff", - "--numstat", - "-z", - "--no-ext-diff", - "--no-textconv", + }); + } + + commit(worktreeId: string, message: string): Promise { + return this.serial(worktreeId, async () => { + await this.assertGit(worktreeId); + const normalized = message.trim(); + if (!normalized) + throw new CommandError("INVALID_ARGUMENT", "commit message is empty"); + const root = await realpath(await this.rootResolver(worktreeId)); + const snapshot = await this.statusAtRoot(root); + if (!snapshot.changes.some((change) => change.staged)) + throw new CommandError( + "NOTHING_TO_COMMIT", + "there are no staged changes", + ); + if (snapshot.changes.some((change) => isConflict(change.status))) + throw new CommandError( + "GIT_CONFLICT", + "resolve conflicts before committing", + ); + await runSafeGit(root, [ + "commit", + "--no-verify", + "--no-gpg-sign", + "--cleanup=strip", + "-m", + normalized, ]); - applyNumstat(result.changes, staged); - applyNumstat(result.changes, unstaged); - return result; }); } @@ -68,6 +206,7 @@ export class GitService { } const root = await realpath(await this.rootResolver(worktreeId)); await resolveSafePath(root, relativePath, false); + await assertNoGitFilter(root, relativePath); const indexSpec = `:./${relativePath}`; const headSpec = `HEAD:./${relativePath}`; @@ -114,29 +253,54 @@ export class GitService { }); } + private async statusAtRoot(root: string): Promise { + await assertNoSelectedFilters(root); + const output = await safeGitText(root, [ + "status", + "--porcelain=v2", + "--branch", + "-z", + "--untracked-files=all", + ]); + const result = parseStatus(output); + const staged = await safeGitText(root, [ + "diff", + "--cached", + "--numstat", + "-z", + "--no-ext-diff", + "--no-textconv", + ]); + const unstaged = await safeGitText(root, [ + "diff", + "--numstat", + "-z", + "--no-ext-diff", + "--no-textconv", + ]); + applyNumstat(result.changes, staged); + applyNumstat(result.changes, unstaged); + return result; + } + closeWorktree(worktreeId: string): void { - this.queues.delete(worktreeId); + void worktreeId; } closeProject(worktreeId: string): void { this.closeWorktree(worktreeId); } - private serial( + private async serial( worktreeId: string, operation: () => Promise, ): Promise { - const previous = this.queues.get(worktreeId) ?? Promise.resolve(); - const result = previous.catch(() => undefined).then(operation); - const tail = result.then( - () => undefined, - () => undefined, - ); - this.queues.set(worktreeId, tail); - void tail.finally(() => { - if (this.queues.get(worktreeId) === tail) this.queues.delete(worktreeId); - }); - return result; + const release = await this.operationLock.acquire(worktreeId); + try { + return await operation(); + } finally { + release(); + } } } @@ -187,26 +351,34 @@ async function readWorktreeFile( } } -async function resolveSafePath( - root: string, - relativePath: string, - mustExist: boolean, -): Promise { +function assertSafeRelativePath(relativePath: string): void { if ( relativePath === "" || + relativePath.includes("\0") || path.isAbsolute(relativePath) || + path.win32.isAbsolute(relativePath) || relativePath .split(/[\\/]/u) .some( (component) => - component === "." || component === ".." || component === ".git", + component === "" || + component === "." || + component === ".." || + component.toLocaleLowerCase() === ".git", ) - ) { + ) throw new CommandError( "OUTSIDE_PROJECT", "path must contain only normal relative components", ); - } +} + +async function resolveSafePath( + root: string, + relativePath: string, + mustExist: boolean, +): Promise { + assertSafeRelativePath(relativePath); const target = path.join(root, relativePath); try { const resolved = mustExist @@ -229,6 +401,91 @@ async function resolveSafePath( } } +async function resolveSafeEntryNoFollow( + root: string, + relativePath: string, +): Promise<{ + path: string; + metadata: Awaited>; + parentPath: string; + parentMetadata: Awaited>; +}> { + assertSafeRelativePath(relativePath); + const parent = await realpath(path.join(root, path.dirname(relativePath))); + const fromRoot = path.relative(root, parent); + if ( + fromRoot === ".." || + fromRoot.startsWith(`..${path.sep}`) || + path.isAbsolute(fromRoot) + ) + throw new CommandError( + "OUTSIDE_PROJECT", + "resolved path is outside project", + ); + const target = path.join(parent, path.basename(relativePath)); + return { + path: target, + metadata: await lstat(target), + parentPath: parent, + parentMetadata: await lstat(parent), + }; +} + +type FileFingerprint = { + dev: number; + ino: number; + mode: number; + size: number; + mtimeMs: number; + ctimeMs: number; +} | null; + +async function fingerprint(target: string): Promise { + try { + const metadata = await lstat(target); + return { + dev: metadata.dev, + ino: metadata.ino, + mode: metadata.mode, + size: metadata.size, + mtimeMs: metadata.mtimeMs, + ctimeMs: metadata.ctimeMs, + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return null; + throw error; + } +} + +function sameFingerprint( + current: FileFingerprint, + expected: FileFingerprint, +): boolean { + if (!current || !expected) return current === expected; + return ( + current.dev === expected.dev && + current.ino === expected.ino && + current.mode === expected.mode && + current.size === expected.size && + current.mtimeMs === expected.mtimeMs && + current.ctimeMs === expected.ctimeMs + ); +} + +function sameMetadata( + current: Awaited>, + expected: Awaited>, +): boolean { + return ( + current.dev === expected.dev && + current.ino === expected.ino && + current.mode === expected.mode && + current.size === expected.size && + current.mtimeMs === expected.mtimeMs && + current.ctimeMs === expected.ctimeMs + ); +} + async function realpathWithMissingTail(target: string): Promise { let existing = target; const missing: string[] = []; @@ -246,6 +503,67 @@ async function realpathWithMissingTail(target: string): Promise { } } +function findChange(status: GitStatus, relativePath: string) { + return status.changes.find( + (change) => + change.path === relativePath || change.originalPath === relativePath, + ); +} + +function scopeChanged(relativePath: string) { + return new CommandError( + "GIT_SCOPE_CHANGED", + `Git status changed for ${relativePath}; refresh and retry`, + ); +} + +function isConflict(status: string): boolean { + return /^(?:DD|AU|UD|UA|DU|AA|UU)$/u.test(status); +} + +async function assertNoSelectedFilters(root: string): Promise { + const paths = ( + await runSafeGit(root, ["ls-files", "-co", "--exclude-standard", "-z"]) + ).stdout; + if (paths.length === 0) return; + const attributes = ( + await runSafeGit(root, ["check-attr", "-z", "--stdin", "filter"], { + input: paths, + }) + ).stdout.toString("utf8"); + const fields = attributes.split("\0"); + for (let index = 2; index < fields.length; index += 3) { + const value = fields[index] ?? "unspecified"; + if (value !== "unspecified" && value !== "unset") { + throw new CommandError( + "GIT_UNSAFE_CONFIG", + "repository selects a Git filter; use the terminal for this operation", + ); + } + } +} + +async function assertNoGitFilter( + root: string, + relativePath: string, +): Promise { + const output = await safeGitText(root, [ + "--literal-pathspecs", + "check-attr", + "-z", + "filter", + "--", + relativePath, + ]); + const value = output.split("\0")[2] ?? "unspecified"; + if (value !== "unspecified" && value !== "unset") { + throw new CommandError( + "GIT_UNSAFE_CONFIG", + "selected path uses a Git filter; use the terminal for this operation", + ); + } +} + function untrackedPatch(relativePath: string, content: string): string { return `--- /dev/null\n+++ b/${relativePath}\n@@ -0,0 +1,${content.split(/\r?\n/u).filter((_, index, lines) => index < lines.length - 1 || lines[index] !== "").length} @@\n${content .split(/\r?\n/u) diff --git a/electron/ipc.test.ts b/electron/ipc.test.ts index 93eb6cb..5e02417 100644 --- a/electron/ipc.test.ts +++ b/electron/ipc.test.ts @@ -26,6 +26,46 @@ describe("IPC command argument validation", () => { ).toThrow("relativePath is empty or too large"); }); + it("validates narrow Git mutation arguments", () => { + expect( + validateCommandArgs("git_discard", { + worktreeId: "w1", + relativePath: "src/file.ts", + untracked: false, + }), + ).toEqual({ + worktreeId: "w1", + relativePath: "src/file.ts", + untracked: false, + }); + expect( + validateCommandArgs("git_commit", { + worktreeId: "w1", + message: "subject\n\nbody", + }), + ).toEqual({ worktreeId: "w1", message: "subject\n\nbody" }); + expect(() => + validateCommandArgs("git_commit", { + worktreeId: "w1", + message: " ", + }), + ).toThrow("message is empty"); + expect(() => + validateCommandArgs("git_stage", { + worktreeId: "w1", + relativePath: "file.ts", + args: ["reset", "--hard"], + }), + ).toThrow("Unexpected argument: args"); + expect(() => + validateCommandArgs("git_discard", { + worktreeId: "w1", + relativePath: "file.ts", + untracked: "yes", + }), + ).toThrow("untracked must be a boolean"); + }); + it("allows chat deletion IDs without accepting renderer paths", () => { expect( validateCommandArgs("chat_session_delete", { @@ -231,6 +271,40 @@ describe("IPC command argument validation", () => { ).toEqual({ worktreeId: "w1" }); }); + it("accepts only the narrow filesystem mutation fields", () => { + expect( + validateCommandArgs("fs_create_file", { + worktreeId: "w1", + relativePath: "src/new.ts", + }), + ).toEqual({ worktreeId: "w1", relativePath: "src/new.ts" }); + expect( + validateCommandArgs("fs_rename", { + worktreeId: "w1", + relativePath: "src/old.ts", + newName: "new.ts", + }), + ).toEqual({ + worktreeId: "w1", + relativePath: "src/old.ts", + newName: "new.ts", + }); + expect(() => + validateCommandArgs("fs_trash", { + worktreeId: "w1", + relativePath: "src/file.ts", + force: true, + }), + ).toThrow("Unexpected argument: force"); + expect(() => + validateCommandArgs("fs_move", { + worktreeId: "w1", + sourceRelativePath: "src/file.ts", + destinationRelativePath: "", + }), + ).toThrow("destinationRelativePath is empty or too large"); + }); + it("retains path length limits and rejects unexpected fields", () => { expect(() => validateCommandArgs("fs_read_dir", { diff --git a/electron/ipc.ts b/electron/ipc.ts index 11c6845..7c4c8fa 100644 --- a/electron/ipc.ts +++ b/electron/ipc.ts @@ -24,6 +24,7 @@ const text = (args: Args, key: string, allowEmpty = false): string => { if (typeof value !== "string") throw new TypeError(`${key} must be a string`); const limits: Record = { text: 64 * 1024, + message: 64 * 1024, data: 64 * 1024, relativePath: 4 * 1024, name: 48, @@ -159,6 +160,44 @@ async function invoke( fileContent(args), text(args, "expectedVersion"), ); + case "fs_create_file": + return state.filesystem.createFile( + text(args, "worktreeId"), + text(args, "relativePath"), + ); + case "fs_create_directory": + return state.filesystem.createDirectory( + text(args, "worktreeId"), + text(args, "relativePath"), + ); + case "fs_rename": + return state.filesystem.renameEntry( + text(args, "worktreeId"), + text(args, "relativePath"), + text(args, "newName"), + ); + case "fs_move": + return state.filesystem.moveEntry( + text(args, "worktreeId"), + text(args, "sourceRelativePath"), + text(args, "destinationRelativePath"), + ); + case "fs_copy": + return state.filesystem.copyEntry( + text(args, "worktreeId"), + text(args, "sourceRelativePath"), + text(args, "destinationRelativePath"), + ); + case "fs_trash": + return state.filesystem.trashEntry( + text(args, "worktreeId"), + text(args, "relativePath"), + ); + case "fs_reveal": + return state.filesystem.revealEntry( + text(args, "worktreeId"), + text(args, "relativePath"), + ); case "git_status": return state.git.status(text(args, "worktreeId")); case "git_diff_file": @@ -167,6 +206,24 @@ async function invoke( text(args, "relativePath"), text(args, "scope"), ); + case "git_stage": + return state.git.stage( + text(args, "worktreeId"), + text(args, "relativePath"), + ); + case "git_unstage": + return state.git.unstage( + text(args, "worktreeId"), + text(args, "relativePath"), + ); + case "git_discard": + return state.git.discard( + text(args, "worktreeId"), + text(args, "relativePath"), + boolean(args, "untracked"), + ); + case "git_commit": + return state.git.commit(text(args, "worktreeId"), text(args, "message")); case "git_list_origin_branches": return state.worktrees.listOriginBranches(text(args, "projectId")); case "worktree_create": @@ -385,8 +442,19 @@ const ALLOWED_FIELDS: Record = { fs_read_dir: ["worktreeId", "relativePath"], fs_read_file: ["worktreeId", "relativePath"], fs_write_file: ["worktreeId", "relativePath", "content", "expectedVersion"], + fs_create_file: ["worktreeId", "relativePath"], + fs_create_directory: ["worktreeId", "relativePath"], + fs_rename: ["worktreeId", "relativePath", "newName"], + fs_move: ["worktreeId", "sourceRelativePath", "destinationRelativePath"], + fs_copy: ["worktreeId", "sourceRelativePath", "destinationRelativePath"], + fs_trash: ["worktreeId", "relativePath"], + fs_reveal: ["worktreeId", "relativePath"], git_status: ["worktreeId"], git_diff_file: ["worktreeId", "relativePath", "scope"], + git_stage: ["worktreeId", "relativePath"], + git_unstage: ["worktreeId", "relativePath"], + git_discard: ["worktreeId", "relativePath", "untracked"], + git_commit: ["worktreeId", "message"], git_list_origin_branches: ["projectId"], worktree_create: ["projectId", "name", "baseRef"], worktree_select: ["worktreeId"], @@ -489,12 +557,18 @@ export function validateCommandArgs(command: CommandName, args: Args): Args { ) continue; if (key === "cols" || key === "rows") number(args, key); - else if (key === "force" || key === "enabled") boolean(args, key); + else if (key === "force" || key === "enabled" || key === "untracked") + boolean(args, key); else if (key === "thinkingLevel") thinkingLevel(args); else if (key === "language") appLanguage(args); else if (key === "document") memoryDocument(args); else if (key === "content") fileContent(args); - else if (key === "phase1ReasoningEffort" || key === "phase2ReasoningEffort") + else if (key === "message") { + if (!text(args, key).trim()) throw new TypeError("message is empty"); + } else if ( + key === "phase1ReasoningEffort" || + key === "phase2ReasoningEffort" + ) memoryReasoningEffort(args, key); else text( diff --git a/src/bindings/generated.ts b/src/bindings/generated.ts index 1283b59..30a7847 100644 --- a/src/bindings/generated.ts +++ b/src/bindings/generated.ts @@ -82,12 +82,21 @@ export interface FileEntry { name: string; relativePath: string; kind: "file" | "directory" | "symlink"; + size?: number; +} + +export interface FilesystemMutationResult { + operation: "create" | "move" | "copy" | "trash"; + kind: "file" | "directory" | "symlink"; + sourceRelativePath?: string; + destinationRelativePath?: string; } export interface FileContent { relativePath: string; content: string; language?: string | null; + size?: number; version: string; } diff --git a/src/bindings/index.ts b/src/bindings/index.ts index 407c244..58256e8 100644 --- a/src/bindings/index.ts +++ b/src/bindings/index.ts @@ -12,6 +12,7 @@ import type { ExtensionSetting, FileContent, FileEntry, + FilesystemMutationResult, GitDiff, GitStatus, MemoryConfig, @@ -147,11 +148,62 @@ export const commands = { content, expectedVersion, }), + fsCreateFile: (worktreeId: string, relativePath: string) => + command("fs_create_file", { + worktreeId, + relativePath, + }), + fsCreateDirectory: (worktreeId: string, relativePath: string) => + command("fs_create_directory", { + worktreeId, + relativePath, + }), + fsRename: (worktreeId: string, relativePath: string, newName: string) => + command("fs_rename", { + worktreeId, + relativePath, + newName, + }), + fsMove: ( + worktreeId: string, + sourceRelativePath: string, + destinationRelativePath: string, + ) => + command("fs_move", { + worktreeId, + sourceRelativePath, + destinationRelativePath, + }), + fsCopy: ( + worktreeId: string, + sourceRelativePath: string, + destinationRelativePath: string, + ) => + command("fs_copy", { + worktreeId, + sourceRelativePath, + destinationRelativePath, + }), + fsTrash: (worktreeId: string, relativePath: string) => + command("fs_trash", { + worktreeId, + relativePath, + }), + fsReveal: (worktreeId: string, relativePath: string) => + command("fs_reveal", { worktreeId, relativePath }), gitStatus: (worktreeId: string) => command("git_status", { worktreeId }), gitDiffFile: (worktreeId: string, relativePath: string, scope: string) => command("git_diff_file", { worktreeId, relativePath, scope }), + gitStage: (worktreeId: string, relativePath: string) => + command("git_stage", { worktreeId, relativePath }), + gitUnstage: (worktreeId: string, relativePath: string) => + command("git_unstage", { worktreeId, relativePath }), + gitDiscard: (worktreeId: string, relativePath: string, untracked: boolean) => + command("git_discard", { worktreeId, relativePath, untracked }), + gitCommit: (worktreeId: string, message: string) => + command("git_commit", { worktreeId, message }), terminalCreate: (worktreeId: string) => command("terminal_create", { diff --git a/src/features/changes/ChangesPanel.test.tsx b/src/features/changes/ChangesPanel.test.tsx index 253ac77..598f9c4 100644 --- a/src/features/changes/ChangesPanel.test.tsx +++ b/src/features/changes/ChangesPanel.test.tsx @@ -3,12 +3,19 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { GitStatus } from "../../bindings"; import { useEditorStore } from "../editor/editorStore"; import { ChangesPanel } from "./ChangesPanel"; +import { commitChanges, discardChanges, stageChanges } from "./changesActions"; import { refreshChanges } from "./changesRefresh"; import { useChangesStore } from "./changesStore"; vi.mock("./changesRefresh", () => ({ refreshChanges: vi.fn(), })); +vi.mock("./changesActions", () => ({ + stageChanges: vi.fn(() => Promise.resolve(true)), + unstageChanges: vi.fn(() => Promise.resolve(true)), + discardChanges: vi.fn(() => Promise.resolve(true)), + commitChanges: vi.fn(() => Promise.resolve(true)), +})); const snapshot: GitStatus = { branch: "main", @@ -42,6 +49,9 @@ const snapshot: GitStatus = { beforeEach(() => { vi.mocked(refreshChanges).mockReset(); + vi.mocked(stageChanges).mockClear(); + vi.mocked(discardChanges).mockClear(); + vi.mocked(commitChanges).mockClear(); useChangesStore.setState({ byWorktree: { w1: { @@ -53,10 +63,20 @@ beforeEach(() => { }, mode: "list", diffMode: "unified", + commitDraftByWorktree: {}, + operationByWorktree: {}, + operationErrorByWorktree: {}, }); useEditorStore.setState({ views: {}, navigationGeneration: 0 }); }); +const openViewOptions = () => + fireEvent.click(screen.getByRole("button", { name: "View options" })); +const selectTreeView = () => { + openViewOptions(); + fireEvent.click(screen.getByRole("menuitemradio", { name: "Tree view" })); +}; + describe("ChangesPanel view modes", () => { it("defaults to the flat list mode without a manual refresh control", () => { render(); @@ -65,14 +85,13 @@ describe("ChangesPanel view modes", () => { expect( screen.queryByRole("button", { name: "Refresh changes" }), ).toBeNull(); - expect(screen.getByRole("button", { name: "List view" })).toHaveAttribute( - "aria-pressed", - "true", - ); - expect(screen.getByRole("button", { name: "Tree view" })).toHaveAttribute( - "aria-pressed", - "false", - ); + openViewOptions(); + expect( + screen.getByRole("menuitemradio", { name: "List view" }), + ).toHaveAttribute("aria-checked", "true"); + expect( + screen.getByRole("menuitemradio", { name: "Tree view" }), + ).toHaveAttribute("aria-checked", "false"); expect( screen.getByRole("treeitem", { name: "src/features/changes/ChangesPanel.tsx M", @@ -83,7 +102,7 @@ describe("ChangesPanel view modes", () => { it("shows grouped paths as an expanded, collapsible tree", () => { render(); - fireEvent.click(screen.getByRole("button", { name: "Tree view" })); + selectTreeView(); const src = screen.getByRole("treeitem", { name: "src" }); expect(src).toHaveAttribute("aria-expanded", "true"); @@ -112,7 +131,7 @@ describe("ChangesPanel view modes", () => { it("adds tree semantics and supports the complete keyboard matrix", () => { render(); - fireEvent.click(screen.getByRole("button", { name: "Tree view" })); + selectTreeView(); expect(screen.getByRole("tree")).toBeInTheDocument(); const src = screen.getByRole("treeitem", { name: "src" }); @@ -159,7 +178,7 @@ describe("ChangesPanel view modes", () => { it("opens the original path with the group scope from tree mode", () => { render(); - fireEvent.click(screen.getByRole("button", { name: "Tree view" })); + selectTreeView(); fireEvent.click( screen.getByRole("treeitem", { name: "ChangesPanel.tsx M" }), ); @@ -175,12 +194,97 @@ describe("ChangesPanel view modes", () => { }); }); + it("filters by path and hides section-wide actions while filtering", () => { + render(); + fireEvent.click(screen.getByRole("button", { name: "Filter changes" })); + fireEvent.change(screen.getByRole("textbox", { name: "Filter changes" }), { + target: { value: "README" }, + }); + + expect( + screen.queryByRole("treeitem", { + name: "src/features/changes/ChangesPanel.tsx M", + }), + ).toBeNull(); + expect(screen.getByRole("treeitem", { name: "README.md U" })).toBeVisible(); + expect(screen.queryByRole("button", { name: "Stage all" })).toBeNull(); + }); + + it("runs a row action without opening its diff", () => { + render(); + const row = screen.getByRole("treeitem", { name: "README.md U" }); + fireEvent.click(within(row).getByRole("button", { name: "Stage" })); + + expect(stageChanges).toHaveBeenCalledWith( + "w1", + expect.arrayContaining([ + expect.objectContaining({ + change: expect.objectContaining({ path: "README.md" }), + }), + ]), + ); + expect(useEditorStore.getState().views.w1).toBeUndefined(); + }); + + it("collapses sections and exposes destructive confirmation", () => { + render(); + const heading = screen.getByRole("button", { name: "CHANGES2" }); + fireEvent.click(heading); + expect(heading).toHaveAttribute("aria-expanded", "false"); + expect( + screen.queryByRole("treeitem", { name: "src/main.tsx M" }), + ).toBeNull(); + fireEvent.click(heading); + const row = screen.getByRole("treeitem", { name: "src/main.tsx M" }); + fireEvent.click(within(row).getByRole("button", { name: "Discard" })); + const dialog = screen.getByRole("dialog", { name: "Discard changes?" }); + expect(dialog).toBeVisible(); + fireEvent.click(within(dialog).getByRole("button", { name: "Discard" })); + expect(discardChanges).toHaveBeenCalledWith( + "w1", + expect.arrayContaining([ + expect.objectContaining({ + change: expect.objectContaining({ path: "src/main.tsx" }), + }), + ]), + ); + }); + + it("commits a staged change with the platform keyboard shortcut", () => { + useChangesStore.setState({ + byWorktree: { + w1: { + snapshot: { + ...snapshot, + changes: [ + { + path: "staged.ts", + status: "M.", + staged: true, + unstaged: false, + untracked: false, + }, + ], + }, + loading: false, + staleError: null, + generation: 1, + }, + }, + }); + render(); + const message = screen.getByRole("textbox", { name: "Commit message" }); + fireEvent.change(message, { target: { value: "ship it" } }); + fireEvent.keyDown(message, { key: "Enter", metaKey: true }); + expect(commitChanges).toHaveBeenCalledWith("w1", "ship it"); + }); + it("keeps status groups separate in tree mode", () => { render(); - fireEvent.click(screen.getByRole("button", { name: "Tree view" })); + selectTreeView(); const groups = screen.getAllByRole("region"); expect(within(groups[0]).getByText("CHANGES")).toBeInTheDocument(); - expect(within(groups[1]).getByText("UNTRACKED")).toBeInTheDocument(); + expect(within(groups[1]).getByText("UNTRACKED FILES")).toBeInTheDocument(); }); }); diff --git a/src/features/changes/ChangesPanel.tsx b/src/features/changes/ChangesPanel.tsx index 324b702..3f0db60 100644 --- a/src/features/changes/ChangesPanel.tsx +++ b/src/features/changes/ChangesPanel.tsx @@ -2,22 +2,45 @@ import { type FocusEvent, type KeyboardEvent, useEffect, + useMemo, + useRef, useState, + type ReactNode, } from "react"; import { + RiAddLine, RiArrowRightSLine, + RiCloseLine, + RiDeleteBinLine, RiFileAddLine, RiFileListLine, + RiFilterLine, RiFolder3Line, RiFolderOpenLine, RiGitCommitLine, + RiMore2Fill, + RiSubtractLine, RiTreeLine, } from "@remixicon/react"; -import { type DiffScope, type GitChange } from "../../bindings"; +import type { GitChange } from "../../bindings"; import { Tooltip } from "../../components/Tooltip"; import { formatNumber, useTranslation } from "../../i18n"; import { useTreeKeyboard } from "../../lib/useTreeKeyboard"; import { diffResourceId, useEditorStore } from "../editor/editorStore"; +import { CommitArea } from "./CommitArea"; +import { DiscardChangesDialog } from "./DiscardChangesDialog"; +import { + commitChanges, + discardChanges, + stageChanges, + unstageChanges, +} from "./changesActions"; +import { + isConflictStatus, + projectChanges, + type ChangeSectionId, + type ProjectedChange, +} from "./changesProjection"; import { refreshChanges } from "./changesRefresh"; import { useChangesStore } from "./changesStore"; @@ -32,7 +55,7 @@ type ChangeTreeNode = kind: "file"; name: string; path: string; - change: GitChange; + row: ProjectedChange; }; interface MutableDirectory { @@ -44,20 +67,21 @@ interface MutableDirectory { const compareNames = (left: string, right: string) => left.localeCompare(right, undefined, { numeric: true, sensitivity: "base" }); - const treeRowPaddingLeft = (depth: number) => 12 + depth * 8; - -const buildChangeTree = (changes: GitChange[]): ChangeTreeNode[] => { +const parentPath = (relativePath: string) => { + const index = relativePath.lastIndexOf("/"); + return index < 0 ? "" : relativePath.slice(0, index); +}; +const buildChangeTree = (rows: ProjectedChange[]): ChangeTreeNode[] => { const root: MutableDirectory = { name: "", path: "", directories: new Map(), files: [], }; - - for (const change of changes) { - const parts = change.path.split("/").filter(Boolean); - const fileName = parts.pop() ?? change.path; + for (const projected of rows) { + const parts = projected.change.path.split("/").filter(Boolean); + const name = parts.pop() ?? projected.change.path; let directory = root; for (const part of parts) { const path = directory.path ? `${directory.path}/${part}` : part; @@ -70,12 +94,11 @@ const buildChangeTree = (changes: GitChange[]): ChangeTreeNode[] => { } directory.files.push({ kind: "file", - name: fileName, - path: change.path, - change, + name, + path: projected.change.path, + row: projected, }); } - const children = (directory: MutableDirectory): ChangeTreeNode[] => [ ...[...directory.directories.values()] .sort((left, right) => compareNames(left.name, right.name)) @@ -89,42 +112,109 @@ const buildChangeTree = (changes: GitChange[]): ChangeTreeNode[] => { compareNames(left.name, right.name), ), ]; - return children(root); }; -const statusLabel = (change: GitChange) => - change.untracked - ? "U" - : change.status.replace(/\./g, "").slice(0, 1).toUpperCase() || "M"; +function ChangeStats({ change }: { change: GitChange }) { + if (change.additions == null && change.deletions == null) return null; + return ( + + ); +} + +function RowActions({ + row, + disabled, + onStage, + onUnstage, + onDiscard, +}: { + row: ProjectedChange; + disabled: boolean; + onStage: (rows: ProjectedChange[]) => void; + onUnstage: (rows: ProjectedChange[]) => void; + onDiscard: (rows: ProjectedChange[]) => void; +}) { + const { t } = useTranslation(); + const action = ( + label: string, + icon: ReactNode, + run: () => void, + danger = false, + ) => ( + + + + ); + return ( + + {row.canDiscard && + action( + t(row.section === "untracked" ? "changes.delete" : "changes.discard"), + , + () => onDiscard([row]), + true, + )} + {row.canStage && + action(t("changes.stage"), , () => + onStage([row]), + )} + {row.canUnstage && + action(t("changes.unstage"), , () => + onUnstage([row]), + )} + + ); +} function ChangeFileRow({ worktreeId, - change, - scope, + row, activeTabId, label, depth, parentId, focusedId, + operationPending, onItemFocus, onKeyDown, + onStage, + onUnstage, + onDiscard, }: { worktreeId: string; - change: GitChange; - scope: DiffScope; + row: ProjectedChange; activeTabId: string | null; label: string; depth?: number; parentId?: string; focusedId: string | null; + operationPending: boolean; onItemFocus: (event: FocusEvent) => void; onKeyDown: (event: KeyboardEvent) => void; + onStage: (rows: ProjectedChange[]) => void; + onUnstage: (rows: ProjectedChange[]) => void; + onDiscard: (rows: ProjectedChange[]) => void; }) { - const resourceId = diffResourceId(worktreeId, scope, change.path); + const resourceId = diffResourceId(worktreeId, row.scope, row.change.path); const active = activeTabId === resourceId; - const status = statusLabel(change); - const itemId = `${scope}:${depth === undefined ? "list:" : ""}${change.path}`; + const itemId = + depth === undefined ? `${row.section}:list:${row.change.path}` : row.key; const open = (keep: boolean) => { useEditorStore.getState().beginNavigation(); useEditorStore.getState().open( @@ -132,21 +222,21 @@ function ChangeFileRow({ id: resourceId, worktreeId, type: "diff", - scope, - relativePath: change.path, + scope: row.scope, + relativePath: row.change.path, preview: !keep, }, keep, ); }; - + const path = parentPath(row.change.path); return ( - + + ); } function ChangeTree({ worktreeId, nodes, - scope, activeTabId, collapsed, toggleDirectory, focusedId, + operationPending, onItemFocus, onKeyDown, + onStage, + onUnstage, + onDiscard, + sectionId, parentId, depth = 0, }: { worktreeId: string; nodes: ChangeTreeNode[]; - scope: DiffScope; activeTabId: string | null; collapsed: Set; toggleDirectory: (key: string) => void; focusedId: string | null; + operationPending: boolean; onItemFocus: (event: FocusEvent) => void; onKeyDown: (event: KeyboardEvent) => void; + onStage: (rows: ProjectedChange[]) => void; + onUnstage: (rows: ProjectedChange[]) => void; + onDiscard: (rows: ProjectedChange[]) => void; + sectionId: ChangeSectionId; parentId?: string; depth?: number; }) { return nodes.map((node) => { - if (node.kind === "file") { + if (node.kind === "file") return ( ); - } - - const key = `${scope}:${node.path}`; + const key = `${sectionId}:dir:${node.path}`; const isOpen = !collapsed.has(key); return ( -
- +
{isOpen && ( - +
+ +
)} ); }); } -function ChangeGroup({ - worktreeId, - title, - changes, - scope, - activeTabId, - mode, - collapsed, - toggleDirectory, - focusedId, - onItemFocus, - onKeyDown, -}: { - worktreeId: string; - title: string; - changes: GitChange[]; - scope: DiffScope; - activeTabId: string | null; - mode: "list" | "tree"; - collapsed: Set; - toggleDirectory: (key: string) => void; - focusedId: string | null; - onItemFocus: (event: FocusEvent) => void; - onKeyDown: (event: KeyboardEvent) => void; -}) { - if (!changes.length) return null; - return ( -
-
- - {title} - {changes.length} -
- {mode === "tree" ? ( - - ) : ( - changes.map((change) => ( - - )) - )} -
- ); -} - export function ChangesPanel({ worktreeId }: { worktreeId: string }) { const { t } = useTranslation(); const state = useChangesStore((store) => store.byWorktree[worktreeId]); const mode = useChangesStore((store) => store.mode); const setMode = useChangesStore((store) => store.setMode); - const [collapsed, setCollapsed] = useState>(() => new Set()); + const draft = useChangesStore( + (store) => store.commitDraftByWorktree[worktreeId] ?? "", + ); + const operation = useChangesStore( + (store) => store.operationByWorktree[worktreeId] ?? null, + ); + const operationError = useChangesStore( + (store) => store.operationErrorByWorktree[worktreeId] ?? null, + ); + const [collapsedDirectories, setCollapsedDirectories] = useState>( + () => new Set(), + ); + const [collapsedSections, setCollapsedSections] = useState< + Set + >(() => new Set()); + const [filterOpen, setFilterOpen] = useState(false); + const [filterQuery, setFilterQuery] = useState(""); + const [viewMenuOpen, setViewMenuOpen] = useState(false); + const [discardRequest, setDiscardRequest] = useState< + ProjectedChange[] | null + >(null); + const menuRef = useRef(null); + const filterRef = useRef(null); const { treeRef, focusedId, onItemFocus, onKeyDown } = useTreeKeyboard(); const activeTabId = useEditorStore( (editor) => editor.views[worktreeId]?.activeTabId ?? null, ); + useEffect(() => { void refreshChanges(worktreeId); + setCollapsedDirectories(new Set()); + setCollapsedSections(new Set()); + setFilterQuery(""); + setFilterOpen(false); + setViewMenuOpen(false); + setDiscardRequest(null); }, [worktreeId]); + useEffect(() => { + if (filterOpen) filterRef.current?.focus(); + }, [filterOpen]); + useEffect(() => { + if (!viewMenuOpen) return; + const close = (event: PointerEvent) => { + if ( + event.target instanceof Node && + !menuRef.current?.contains(event.target) + ) + setViewMenuOpen(false); + }; + document.addEventListener("pointerdown", close); + return () => document.removeEventListener("pointerdown", close); + }, [viewMenuOpen]); + + const snapshot = state?.snapshot; + const sections = useMemo( + () => (snapshot ? projectChanges(snapshot, filterQuery) : []), + [filterQuery, snapshot], + ); + const count = snapshot?.changes.length ?? 0; + const stagedCount = + snapshot?.changes.filter((change) => change.staged).length ?? 0; + const conflictCount = + snapshot?.changes.filter((change) => isConflictStatus(change.status)) + .length ?? 0; + const operationPending = operation !== null; + const filterActive = filterQuery.trim().length > 0; + const sectionTitle = (id: ChangeSectionId) => + t( + id === "conflicts" + ? "changes.conflicts" + : id === "staged" + ? "changes.staged" + : id === "unstaged" + ? "changes.changes" + : "changes.untracked", + ); const toggleDirectory = (key: string) => - setCollapsed((current) => { + setCollapsedDirectories((current) => { const next = new Set(current); if (next.has(key)) next.delete(key); else next.add(key); return next; }); - const snapshot = state?.snapshot; - const staged = snapshot?.changes.filter((change) => change.staged) ?? []; - const unstaged = - snapshot?.changes.filter( - (change) => change.unstaged && !change.untracked, - ) ?? []; - const untracked = - snapshot?.changes.filter((change) => change.untracked) ?? []; - const count = snapshot?.changes.length ?? 0; + const toggleSection = (id: ChangeSectionId) => + setCollapsedSections((current) => { + const next = new Set(current); + if (next.has(id)) next.delete(id); + else next.add(id); + return next; + }); + const requestDiscard = (rows: ProjectedChange[]) => setDiscardRequest(rows); + const runDiscard = async () => { + if (!discardRequest) return; + if (await discardChanges(worktreeId, discardRequest)) { + setDiscardRequest(null); + return; + } + const latest = useChangesStore.getState().byWorktree[worktreeId]?.snapshot; + if (!latest) { + setDiscardRequest(null); + return; + } + const currentRows = projectChanges(latest, "").flatMap( + (section) => section.rows, + ); + const requested = new Set( + discardRequest.map(({ section, change }) => `${section}:${change.path}`), + ); + const remaining = currentRows.filter(({ section, change }) => + requested.has(`${section}:${change.path}`), + ); + setDiscardRequest(remaining.length ? remaining : null); + }; + const commit = () => void commitChanges(worktreeId, draft); + return (
-
+
{t(count === 1 ? "changes.count.one" : "changes.count.other", { count: formatNumber(count), })}
-
- + {filterOpen ? ( +
+ + setFilterQuery(event.currentTarget.value)} + onKeyDown={(event) => { + if (event.key === "Escape") setFilterOpen(false); + }} + /> + +
+ ) : ( + - + )} +
+ + {viewMenuOpen && ( +
+ + +
+ )}
@@ -397,6 +602,18 @@ export function ChangesPanel({ worktreeId }: { worktreeId: string }) { {t("changes.showingLast", { error: state.staleError.message })}
)} + {operationError && !discardRequest && ( +
+ {operationError.message} + +
+ )} {!snapshot && state?.loading && (
{t("changes.readingStatus")}
)} @@ -409,49 +626,171 @@ export function ChangesPanel({ worktreeId }: { worktreeId: string }) { {t("changes.clean")}
)} - {snapshot && ( -
- - - + {snapshot && count > 0 && ( +
+ {filterActive && sections.every(({ rows }) => rows.length === 0) && ( +
{t("changes.noMatches")}
+ )} +
+ {sections.map((section) => { + if (!section.rows.length) return null; + const collapsed = collapsedSections.has(section.id); + const title = sectionTitle(section.id); + return ( +
+
+ + {!filterActive && section.id !== "conflicts" && ( + + {section.id === "staged" ? ( + + + + ) : ( + <> + + + + + + + + )} + + )} +
+ {!collapsed && ( +
+ {mode === "tree" ? ( + + void stageChanges(worktreeId, rows) + } + onUnstage={(rows) => + void unstageChanges(worktreeId, rows) + } + onDiscard={requestDiscard} + sectionId={section.id} + /> + ) : ( + section.rows.map((row) => ( + + void stageChanges(worktreeId, rows) + } + onUnstage={(rows) => + void unstageChanges(worktreeId, rows) + } + onDiscard={requestDiscard} + /> + )) + )} +
+ )} +
+ ); + })} +
)} + {snapshot && count > 0 && ( + 0 || + !draft.trim() + } + pending={operation?.kind === "commit"} + error={operationError?.message ?? null} + onChange={(value) => + useChangesStore.getState().setCommitDraft(worktreeId, value) + } + onCommit={commit} + /> + )} + {discardRequest && ( + { + useChangesStore.getState().clearOperationError(worktreeId); + setDiscardRequest(null); + }} + onConfirm={() => void runDiscard()} + /> + )}
); } diff --git a/src/features/changes/CommitArea.tsx b/src/features/changes/CommitArea.tsx new file mode 100644 index 0000000..a7cfc02 --- /dev/null +++ b/src/features/changes/CommitArea.tsx @@ -0,0 +1,56 @@ +import { useTranslation } from "../../i18n"; + +export function CommitArea({ + draft, + disabled, + pending, + error, + onChange, + onCommit, +}: { + draft: string; + disabled: boolean; + pending: boolean; + error: string | null; + onChange: (value: string) => void; + onCommit: () => void; +}) { + const { t } = useTranslation(); + return ( +
+