diff --git a/AGENTS.md b/AGENTS.md index 29130ea6..7f9a1586 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,17 @@ # AGENTS.md +## Code Comments: Document the "Why" + +When writing or modifying code driven by a design doc or non-obvious constraint, you **must** add a comment explaining **why** the code behaves the way it does. "What" is visible in the code; "why" is not. Target these categories: + +- Safety constraints (suppressed actions, guarded entry points) +- Fallback/error-handling choices and their rationale +- Architectural boundaries (IPC separation, which surface owns a feature) +- Compatibility shims (fields that exist for downstream plumbing, not semantics) +- Intentional omissions (skipped data, unsupported edge cases) + +If the design doc has a gotcha, the code must have a comment. A maintainer who hasn't read the doc should still understand why the code must not be changed casually. + ## Worktree Safety Always use the primary working directory (the worktree) for all file reads and edits. Never follow absolute paths from subagent results that point to the main repo. diff --git a/CLAUDE.md b/CLAUDE.md index 048c03cd..656f0c42 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,6 +2,18 @@ See also: [AGENTS.md](./AGENTS.md) for agent-specific rules. +## Code Comments: Document the "Why" + +When implementing behavior driven by a design doc, spec, or non-obvious constraint, **add a comment explaining why** the code does what it does — not just what it does. This is especially important for: + +- **Safety constraints** — e.g., suppressing an action because it could silently erase a signal, re-create a conflict, or mislead the user. +- **Fallback/error-handling choices** — e.g., defaulting to `'modified'` on fs error because it's the least misleading option. +- **Architectural boundaries** — e.g., why state lives in the renderer and never crosses IPC, or why a feature belongs to Source Control and not Checks. +- **Compatibility shims** — e.g., when a field exists purely for downstream plumbing and does not carry semantic meaning. +- **Intentional omissions** — e.g., skipping submodule conflicts or not providing rename metadata because the data source doesn't support it. + +A future maintainer who hasn't read the design doc should be able to understand from the comment alone why the code must not be changed casually. + ## Package Manager This project uses **pnpm**. Never use `npm` or `yarn`. diff --git a/docs/right-sidebar-conflict-resolution-status.md b/docs/right-sidebar-conflict-resolution-status.md new file mode 100644 index 00000000..8500c3db --- /dev/null +++ b/docs/right-sidebar-conflict-resolution-status.md @@ -0,0 +1,822 @@ +# Right Sidebar Conflict Resolution Status + +## Goal + +Show active conflict state during merge, rebase, and cherry-pick operations directly inside the existing `Staged Changes` and `Changes` lists in the right sidebar, similar to VS Code and GitHub Desktop. + +All three operations produce the same porcelain v2 `u` records when conflicts arise. This design applies uniformly to all of them. + +The panel should answer four questions without opening a diff: + +1. Is this file in a merge conflict? +2. What kind of conflict is it? +3. Is this conflict still unresolved right now? +4. If it is no longer unresolved, what should I do next? + +It should not claim more than Git can prove in the current refresh. + +In v1, live unresolved conflict state must come from the current Git status output. +However, the UI may also carry a clearly-labeled local session state for already-open tabs and recently-resolved rows when that state is derived from a file the user opened from a live unresolved conflict in the current session. +That local state is UX scaffolding, not Git truth, and must never be labeled as if Git is still reporting an unmerged entry. + +V1 does not attempt to detect conflict markers in working-tree file contents. +If the user resolves a conflict outside the app (e.g., runs `git add ` in a terminal), the `u` record disappears from `git status` and the app treats the file as no longer conflicted. +This means a file can leave the conflict state while still containing unresolved conflict markers. +This is a known limitation; v1 trusts Git's index state as the sole authority for whether a file is currently in an unresolved merge conflict. + +## Current State + +Today the right sidebar groups files only by: + +- `staged` +- `unstaged` +- `untracked` + +Each row only knows (via `GitUncommittedEntry`, aliased as `GitStatusEntry`): + +- `path` +- `status` +- `area` +- `oldPath?` + +That is enough for normal file changes, but not for merge conflict UX: + +- unresolved conflicts are not represented explicitly +- the user cannot tell conflict type (`both modified`, `deleted by them`, etc.) +- the user cannot tell whether a file is currently unresolved +- conflict resolution progress becomes hard to follow once a file leaves the live `u` state +- the panel does not visually prioritize conflicted files + +Relevant code: + +- [src/main/git/status.ts](../src/main/git/status.ts) +- [src/shared/types.ts](../src/shared/types.ts) +- [src/renderer/src/components/right-sidebar/SourceControl.tsx](../src/renderer/src/components/right-sidebar/SourceControl.tsx) + +## Design Principles + +- Keep the current sidebar structure. Users already understand `Staged Changes` and `Changes`. +- Add conflict state to rows, but also provide a merge-resolution summary so users can scan progress quickly. +- Make unresolved conflicts impossible to miss. +- Show only conflict state that Git can prove in the current refresh. +- Use the same file in both sections when Git says both staged and unstaged states exist. +- When the UI shows session-local post-conflict state, label it explicitly as local UI state rather than current Git conflict truth. + +## Proposed UX + +### 0. Merge summary + +When at least one unresolved conflict exists, show a compact merge summary above the normal sections. + +Recommended presentation: + +- `Merge conflicts: 3 unresolved` (count reflects only live `u`-record conflicts, not `Resolved locally` rows) +- the label should reflect the active operation when detectable: + - `Merge conflicts: 3 unresolved` when `MERGE_HEAD` exists + - `Rebase conflicts: 3 unresolved` when `REBASE_HEAD` exists + - `Cherry-pick conflicts: 3 unresolved` when `CHERRY_PICK_HEAD` exists + - `Conflicts: 3 unresolved` as a fallback when no ref file is found or multiple exist +- operation detection uses `fs.existsSync` on `.git/MERGE_HEAD`, `.git/REBASE_HEAD`, and `.git/CHERRY_PICK_HEAD` in the main process, performed alongside the existing status poll (note: there is a race between the `git status` call and the `fs.existsSync` call — the HEAD file may not yet exist or may already be cleaned up — in that case the operation falls back to `'unknown'` for one poll cycle, which is acceptable) +- secondary action: `Review conflicts` +- optional tertiary hint: `Resolved files move back to normal changes after they leave the live conflict state` + +Why: + +- users in a merge flow think first in terms of “how many conflicts are left” +- this preserves the existing section structure without forcing conflict work to compete visually with every other file change +- it gives the user a stable place to orient before scanning individual rows + +V1 may keep `Changes` and `Staged Changes` as the main lists, but unresolved conflicts should also be discoverable through this merge summary entry point. + +`Review conflicts` must have a concrete v1 behavior: + +- it opens a lightweight conflict-review tab scoped to the current live unresolved conflict set +- the tab lists only unresolved conflict rows, not ordinary diffs +- each item opens the same conflict-safe single-file entry point described in section 5 +- if the unresolved set changes later, the already-open review tab may keep showing the snapshot it was opened with, as long as the UI labels it as a snapshot and offers a refresh or reopen action +- if all conflicts in the snapshot are resolved and the list becomes empty, the tab must show an explicit "all conflicts resolved" state with a dismiss action, not a blank list +- the "all conflicts resolved" state should also offer a link back to `Source Control` to continue the merge workflow + +V1 does not require a merge-aware three-way diff queue. A lightweight conflict list view is sufficient. + +Information architecture decision for v1: + +- `Review conflicts` belongs to `Source Control`, not `Checks` +- it is launched from `Source Control` into the editor area as a dedicated review tab, similar to `Open all diffs` +- do not create a new permanent right-sidebar top-level tab for conflicts in v1 + +Why: + +- merge-conflict review is source-control work, not CI/PR status +- the app already uses editor tabs as the place where review workflows expand beyond the sidebar +- this improves the conflict workflow without fragmenting navigation + +### 1. File row anatomy + +Each file row keeps the current filename and directory, and may also show a normal status letter when one exists for the current row model. +Conflict rows add explicit conflict UI instead of pretending they are ordinary file states. + +Row layout: + +`[icon] filename dir [status letter?] [conflict badge]` + +Conflict badge values for v1: + +- `Unresolved` +- `Resolved locally` + +Conflict subtype text: + +- `Both modified` +- `Both added` +- `Deleted by us` +- `Deleted by them` +- `Added by us` +- `Added by them` +- `Both deleted` + +Recommended presentation: + +- badge is compact and high-contrast +- subtype appears as muted secondary text under or beside the filename +- unresolved uses destructive color +- `Resolved locally` uses a quieter success or accent treatment and must include tooltip/help text that it is derived from the current session, not from live Git conflict output +- if a conflict row does not have a meaningful ordinary status letter, omit the letter instead of inventing one +- conflict rows may replace ordinary status letters entirely when showing a normal status letter would create contradictory meaning + +Action-oriented helper text: + +- `Both modified` -> `Open and edit the final contents` +- `Both added` -> `Choose which version to keep, or combine them` +- `Deleted by us` -> `Decide whether to restore the file` +- `Deleted by them` -> `Decide whether to keep the file or accept deletion` +- `Added by us` -> `Review whether to keep the added file` +- `Added by them` -> `Review the added file before keeping it` +- `Both deleted` -> `Resolve in Git or restore one side before editing` + +Non-goal for v1: + +- carrying conflict history forward after the `u` record disappears + +Clarification: + +- v1 may show `Resolved locally` only when the app can tie that row or tab to a file that was opened from a live unresolved conflict during the current session +- v1 should not invent historical conflict state for files the user never interacted with in the current session + +### 2. Section behavior + +Keep the existing sections, but add a merge-resolution summary above them and order conflicted files first within `Changes` and `Staged Changes`. + +#### `Changes` + +This section should contain: + +- normal unstaged files +- unresolved conflicts +- optionally, recently resolved files from the current session that still have unstaged changes and need review + +Expected labels: + +- unresolved conflict: `Unresolved` +- recently resolved in current session: `Resolved locally` + +This matches the user expectation: the working tree still needs attention. + +#### `Staged Changes` + +This section should contain: + +- normal staged files +- optionally, recently resolved files from the current session that were staged after conflict resolution + +`Resolved locally` badge lifecycle is governed by the state machine defined in the Representing resolved conflicts section. +This state is tied to the file's presence in the current sidebar workflow, not to whether its tab happens to remain open. + +### 3. Summary counts + +Section headers should surface unresolved conflict counts when present. + +Examples: + +- `Changes 5 · 2 conflicts` +- merge summary: `Merge conflicts: 2 unresolved` + +This should remain terse. The row badge carries the detailed meaning. + +### 4. Row actions + +Conflict-aware row actions: + +- unresolved in `Changes`: open editor, no discard shortcut, no stage shortcut +- resolved locally in `Changes`: open editor, stage is allowed, discard is hidden in v1 (discarding a just-resolved conflict file can silently re-create the conflict or lose the resolution — v1 does not have the UX to explain this clearly, so hiding it is the safe default) +- resolved locally in `Staged Changes`: open editor, unstage is allowed + +Reasoning: + +- unresolved conflicts are higher risk than normal edits +- a one-click discard on an unresolved conflict is too easy to misfire +- a one-click stage on an unresolved conflict can immediately erase the sidebar conflict signal because Git stops reporting the `u` record after `git add` +- users should resolve conflicts in the editor first, then stage from a state that still preserves continuity that this file just came out of conflict resolution + +### 5. Diff/editor entry point + +Opening a conflicted file should preserve the current navigation flow, but v1 must not assume the existing two-way diff backend can render unresolved conflicts correctly. + +For unresolved conflicts, the safe requirement is: + +- clicking the row opens the existing file entry point for the file +- the header reflects the conflict badge and subtype when that metadata exists +- unresolved conflicts must not be routed into a misleading two-way diff view +- if the app cannot render a merge-aware view in v1, open the editable file view with conflict metadata instead of the normal diff view +- any fallback state must be explicit and explain that merge-aware diff rendering is not available yet +- the opened view should include action-oriented guidance for the current conflict subtype rather than only Git terminology + +Do not claim VS Code style merge presentation in v1 unless the diff backend is updated to read conflict stages explicitly. +This applies to every entry point, including section-level `Open all diffs`. + +Required routing rule for v1: + +- if the `GitStatusEntry` for the row has `conflictStatus === 'unresolved'`, never call the normal uncommitted diff opener for that row +- instead, route to a conflict-safe entry point that opens either: + - the editable working-tree file view, when a working-tree file exists + - a conflict details panel or read-only placeholder view, when no working-tree file exists + +Required bulk-review behavior for v1: + +- the product must not present a bulk action that appears to review every file while silently excluding unresolved conflicts +- if a section-level `Open all diffs` action remains, its label or adjacent copy must make the scope explicit when unresolved conflicts are present +- preferred v1 behavior: + - keep `Open all diffs` for normal change review + - add `Review conflicts` from the merge summary when unresolved conflicts exist + - if both actions are shown together, the UI must explain the split clearly +- acceptable fallback behavior: + - `Open all diffs` opens normal diffs only + - unresolved conflicts are excluded from the combined diff set + - the trigger and resulting view both state that conflicted files are reviewed separately +- if rows are excluded, the combined-diff tab state must carry an explicit `skippedConflicts` payload so the notice is deterministic and does not depend on reconstructing the skipped set later from live status alone +- if every candidate row for a given combined-diff open is excluded, the tab must render a conflict-specific state rather than a generic `No changes to display` +- that excluded-only state must list the conflicted paths and provide a direct `Review conflicts` action + +Definition of `Review conflicts` in this context: + +- if invoked from the merge summary, open the unresolved-conflict review tab for the full live unresolved set +- if invoked from an excluded-only combined-diff state, open the unresolved-conflict review tab preloaded with that tab's stored skipped-conflict snapshot +- the action must never route unresolved conflicts into the ordinary two-way diff viewer + +This requirement is intentionally strict because the current diff pipeline reads normal index and worktree content, not merge stages, and because bulk actions must not undermine user trust in the review queue. + +Examples: + +- `app.tsx · Unresolved conflict · Both modified` + +This keeps the sidebar and editor consistent. + +### 6. Conflict kinds without a working-tree file + +Not every unresolved conflict can be opened as a normal editable file. + +Examples: + +- `both_deleted` +- some `deleted_by_us` / `deleted_by_them` states, depending on which side leaves a working-tree file behind + +For these cases, v1 must not attempt to open the normal editable file view and then show a generic file-read error. + +Instead, the entry point should show an explicit conflict state such as: + +- `This file is in an unresolved merge conflict. No working-tree file is available to edit.` +- subtype text such as `Both deleted` +- guidance to resolve via Git or restore one side before editing +- action-oriented next step when possible, such as `Restore one side, then reopen this file` + +This can be a lightweight placeholder screen in the editor area. +The important requirement is that the state is conflict-aware and not presented as a broken file open. + +Required state shape for v1: + +- opening a non-editable unresolved conflict must not reuse plain `mode: 'edit'` +- the opened tab state must distinguish `conflict-placeholder` from ordinary editable files and ordinary diffs +- the placeholder state must carry at least: + - `path` + - `conflictStatus` + - `conflictKind` + - `message` + - optional `guidance` + +## Proposed Data Model + +Extend `GitUncommittedEntry` (the base type behind `GitStatusEntry`) with conflict metadata, and extend `OpenFile` (the editor tab state in `src/renderer/src/store/slices/editor.ts`) with conflict-aware metadata instead of relying on sidebar rows as the only source of truth. + +```ts +export type GitConflictKind = + | 'both_modified' + | 'both_added' + | 'both_deleted' + | 'added_by_us' + | 'added_by_them' + | 'deleted_by_us' + | 'deleted_by_them' + +export type GitConflictResolutionStatus = 'unresolved' | 'resolved_locally' + +export type GitConflictStatusSource = 'git' | 'session' + +// Extend GitUncommittedEntry (src/shared/types.ts) +// Currently: { path, status, area, oldPath? } +// GitStatusEntry is an alias for GitUncommittedEntry; extending the base extends both. +// Note: conflictHint is NOT included here. The main process returns only +// Git-derived data. Hint text is derived in the renderer from conflictKind +// using a CONFLICT_HINT_MAP lookup (see Renderer hint derivation below). +// +// conflictStatusSource is NOT set by the main process. The main process +// returns only conflictKind and conflictStatus (always 'unresolved') for +// live u records. The renderer sets conflictStatusSource: 'git' when +// populating from IPC data, and 'session' when applying Resolved locally +// state from trackedConflictPaths. This keeps the main process free of +// session-awareness while letting the renderer distinguish the two sources. +export type GitUncommittedEntry = { + path: string + status: GitFileStatus + area: GitStagingArea + oldPath?: string + conflictKind?: GitConflictKind + conflictStatus?: GitConflictResolutionStatus + conflictStatusSource?: GitConflictStatusSource +} + +// Active operation detected by the main process alongside status polling. +// Used by the renderer to label the merge summary correctly. +export type GitConflictOperation = 'merge' | 'rebase' | 'cherry-pick' | 'unknown' + +// Renderer hint derivation: +// The renderer maps conflictKind to a user-facing hint string using a +// CONFLICT_HINT_MAP constant (e.g., both_modified -> 'Open and edit the +// final contents'). This keeps UI copy out of the main process parser. + +export type OpenConflictMetadata = { + conflictKind: GitConflictKind + conflictStatus: GitConflictResolutionStatus + conflictStatusSource: GitConflictStatusSource +} + +export type OpenConflictPlaceholder = OpenConflictMetadata & { + kind: 'conflict-placeholder' + message: string + guidance?: string +} + +export type OpenConflictEditable = OpenConflictMetadata & { + kind: 'conflict-editable' +} + +export type ConflictReviewState = { + kind: 'conflict-review' + source: 'live-summary' | 'combined-diff-exclusion' + /** Timestamp (ms since epoch) when the snapshot was taken. The renderer + * derives the display label at render time (e.g., '3 unresolved conflicts + * at 2:34 PM') from snapshotTimestamp + entries.length, keeping UI copy + * out of stored state — consistent with the CONFLICT_HINT_MAP approach. */ + snapshotTimestamp: number + entries: ConflictReviewEntry[] +} + +export type CombinedDiffSkippedConflict = { + path: string + conflictKind: GitConflictKind +} + +export type ConflictSummaryState = { + unresolvedCount: number + paths: string[] +} + +export type ConflictReviewEntry = { + path: string + conflictKind: GitConflictKind +} + +// Extend OpenFile (src/renderer/src/store/slices/editor.ts) +// Current shape: +// { id, filePath, relativePath, worktreeId, language, isDirty, +// mode: 'edit' | 'diff', diffSource?, branchCompare?, +// branchOldPath?, combinedAlternate?, combinedAreaFilter?, isPreview? } +// +// OpenFile uses a discriminated union on `mode` so that conflict-review tabs +// do not require a filePath (they are list views, not file views). +// Normal file tabs ('edit' | 'diff') keep filePath required. +// Add: + +type OpenFileBase = { + id: string + worktreeId: string + isPreview?: boolean +} + +type OpenFileTab = OpenFileBase & { + mode: 'edit' | 'diff' + filePath: string + relativePath: string + language: string + isDirty: boolean + diffSource?: DiffSource + branchCompare?: BranchCompareSnapshot + branchOldPath?: string + combinedAlternate?: CombinedDiffAlternate + combinedAreaFilter?: string + // conflict fields for single-file tabs + conflict?: OpenConflictEditable | OpenConflictPlaceholder + skippedConflicts?: CombinedDiffSkippedConflict[] +} + +type OpenConflictReviewTab = OpenFileBase & { + mode: 'conflict-review' + /** id for conflict-review tabs uses a deterministic scheme: + * `conflict-review-${worktreeId}` — at most one conflict-review tab + * per worktree. Opening a new review replaces the existing one. */ + conflictReview: ConflictReviewState +} + +export type OpenFile = OpenFileTab | OpenConflictReviewTab +``` + +Notes: + +- `conflictKind` describes the merge shape +- `conflictStatus` describes whether the UI is showing a live unresolved state or a session-local resolved state +- `conflictStatusSource` distinguishes live Git truth from session-local continuity state +- action-oriented hint text (e.g., `Open and edit the final contents` for `both_modified`) is derived at render time from `conflictKind` via the renderer-side `CONFLICT_HINT_MAP` constant — it is not a field on `GitUncommittedEntry` and is never returned by the main process +- treat a row as conflicted when `conflictStatus` is present +- the `status` value on unresolved conflicts is a rendering compatibility choice for existing icon/color plumbing, not a semantic claim — the conflict badge carries the real semantics +- opened editor tabs are a separate state domain from live git-status rows and need their own conflict metadata +- `Review conflicts` is also an editor-tab state, but it is not a diff tab and should not be forced into `mode: 'diff'` +- v1 therefore needs an explicit editor-tab representation for conflict review rather than overloading combined-diff state + +Compatibility rule for non-upgraded consumers: + +- any consumer of `GitStatusEntry` that has not been upgraded to read `conflictStatus` may still render `modified` styling, but must not offer file-existence-dependent affordances (diff loading, drag payloads, editable-file opening) for unresolved conflicts +- this affects file explorer decorations, tab badges, and any other surface outside `Source Control` +- any consumer of `OpenFile` that accesses `filePath` must narrow on `mode` first, because `OpenConflictReviewTab` does not carry `filePath` — code that assumes `openFile.filePath` exists without checking `mode` will break at compile time once the discriminated union is in place + +Known limitation: + +- passive file explorer and tab badge surfaces may show `modified` styling for unresolved conflicts in v1 + +Explicit v1 scope decision: + +- required to upgrade in v1: `Source Control`, editor tab state, single-file conflict opening, section-level bulk actions, and any shared open-file helper they depend on +- allowed to defer in v1: passive decorations in file explorer and tab-strip visuals +- not allowed to defer in v1: any entry point that can directly open an unresolved-conflict file into the ordinary diff or ordinary editable path without the conflict-aware routing rules +- not allowed to defer in v1: the dedicated editor-tab state and open action for `Review conflicts` + +## Git Parsing Plan + +### Source of truth + +Use `git status --porcelain=v2 --untracked-files=all` as the primary source, but start parsing `u` records in addition to `1`, `2`, and `?`. + +Today [status.ts](../src/main/git/status.ts) ignores unmerged records entirely. + +### Performance + +Parsing `u` records adds no meaningful overhead — they use the same line-by-line parsing loop as `1`/`2`/`?` records, and the number of unmerged entries during a typical merge is small (usually single digits). The filesystem existence check for ambiguous conflict kinds (`deleted_by_us`, `deleted_by_them`, etc.) adds one `fs.existsSync` call per ambiguous entry, which is negligible within the 3-second polling interval. + +If the filesystem existence check throws (permissions error, unmounted path, etc.), default to `status: 'modified'`. This is the safer fallback because it avoids suppressing the conflict row from the sidebar and avoids presenting a `deleted` status that could mislead the user into thinking the file is gone when the check simply failed. The conflict badge and subtype still carry the real semantics regardless of the `status` fallback. + +### Mapping unmerged records + +Porcelain v2 unmerged `XY` states should map like this: + +- `UU` -> `both_modified` +- `AA` -> `both_added` +- `DD` -> `both_deleted` +- `AU` -> `added_by_us` +- `UA` -> `added_by_them` +- `DU` -> `deleted_by_us` +- `UD` -> `deleted_by_them` + +V1 should also map every unresolved `u` record to: + +- `area: 'unstaged'` +- `status`: determined by whether a working-tree file exists for the conflict kind: + - `'modified'` for kinds that always leave a working-tree file: `both_modified`, `both_added` + - `'deleted'` for kinds that never leave a working-tree file: `both_deleted` + - for `deleted_by_us`, `deleted_by_them`, `added_by_us`, `added_by_them`: check whether the working-tree file exists at parse time and use `'modified'` if it does, `'deleted'` if it does not (for `added_by_us`/`added_by_them`, Git typically does leave a working-tree file, so the check is defensive — the primary ambiguity is in `deleted_by_*` variants where the merge strategy determines whether the surviving side's content is written to the worktree) + +Why: + +- the current sidebar, tab decorations, and file explorer already expect a `GitFileStatus` +- `modified` is the least misleading compatibility fallback for conflicts where a working-tree file exists +- `deleted` is a better fallback when no working-tree file exists because calling it `modified` would be contradictory +- `deleted_by_us` / `deleted_by_them` and the `added_by_*` variants do not have a guaranteed working-tree file — Git's behavior depends on the merge strategy and the specific conflict — so the parser must check the filesystem rather than hardcoding an assumption +- the conflict badge/subtype still carries the real semantics in all cases +- v1 should not invent new single-letter codes for unmerged rows without a broader status-system redesign + +### Representing unresolved conflicts in the existing sections + +Unresolved conflicts should be emitted as `area: 'unstaged'` entries with: + +- `status`: determined by working-tree file existence (see Mapping unmerged records above) +- `conflictStatus: 'unresolved'` +- `conflictKind: ...` + +Why: + +- the user still needs to act in the working tree +- this keeps the existing panel layout intact +- it matches the mental model of “this is still pending” + +Ordering requirement: + +- conflicted entries sort before ordinary entries within `Changes` +- `Staged Changes` has no conflict rows in v1 because unresolved `u` records are emitted only into `unstaged` + +### Representing resolved conflicts + +In v1, live Git status must stop representing a file as conflicted after Git stops reporting a `u` record. +However, the UI may carry forward a temporary `Resolved locally` state for files the user opened from a live unresolved conflict in the current session. + +Why: + +- `git status --porcelain=v2` no longer marks the file as unmerged after resolution is staged +- users still need continuity that the file they were just resolving is now in the review-or-stage step of the same workflow +- limiting this to files the user explicitly opened in the current session keeps the scope auditable and avoids broad historical inference + +Corollary: + +- unresolved rows must not expose a `Stage` shortcut, because that shortcut can remove the only live conflict signal before the user has actually completed review of the file +- `Resolved locally` rows may expose normal safe actions again because the user is now in the post-resolution workflow stage + +### `Resolved locally` state machine + +#### Store location + +`trackedConflictPaths` is a `Map>` keyed by `worktreeId`, stored in the renderer-side Zustand git store (the same store that holds `gitStatus`). It is not component-local state — it must survive across re-renders of `SourceControl` and be accessible to both the click handler that adds paths and the polling hook that checks for `u`-record disappearance. Keying by worktree ensures paths are scoped correctly when the app has multiple worktrees open. + +The map is populated by the `Source Control` click handler and read by the status-polling reconciliation logic. It is never sent to the main process. + +#### State transitions + +A file enters `Resolved locally` through a precise sequence: + +1. **Track**: when the user clicks an unresolved conflict row in `Source Control` to open or focus a tab, record that path in the `trackedConflictPaths` set. Opening the same file from the file explorer, terminal, or any non-conflict-row entry point does **not** add it to this set. +2. **Transition**: on the next `git status` poll, if a path in `trackedConflictPaths` no longer has a `u` record, mark that path as `Resolved locally` with `conflictStatusSource: 'session'`. +3. **Re-enter**: if a path currently in `Resolved locally` state reappears as a `u` record (e.g., the user ran `git checkout -m ` to re-create the conflict), replace the session-local resolved state with live Git conflict state (`conflictStatus: 'unresolved'`, `conflictStatusSource: 'git'`). The path remains in `trackedConflictPaths` so it can transition back to `Resolved locally` if the `u` record disappears again. +4. **Expire**: clear the `Resolved locally` state when any of these happens: + - the file leaves the sidebar entirely (no staged or unstaged entry) + - the app session resets (window reload, app restart) + - the file re-enters a live unresolved `u` state, which replaces the local resolved state with live Git conflict state again +5. **Abort**: when the merge/rebase/cherry-pick operation is aborted (`git merge --abort`, `git rebase --abort`, `git cherry-pick --abort`), all `u` records disappear simultaneously and the operation HEAD file (`.git/MERGE_HEAD`, etc.) is cleaned up. On the next poll, if the detected operation changes to `'unknown'` (no HEAD file found) and the unresolved count drops to zero in the same poll cycle, clear the entire `trackedConflictPaths` set for that worktree rather than transitioning each path to `Resolved locally`. Abort is not resolution — showing `Resolved locally` on every previously-conflicted file after an abort would be misleading. + +If a file's `u` record disappears but the path was never in `trackedConflictPaths`, the file simply reverts to its ordinary `GitFileStatus` with no `Resolved locally` badge. + +Important consequence: + +- closing a tab does not clear `Resolved locally` by itself +- if a file is still present in `Changes` or `Staged Changes`, the continuity badge should remain visible until the file leaves the sidebar, the session resets, or the file becomes live-unresolved again + +Guardrails for `Resolved locally`: + +- only show it when the path is in `trackedConflictPaths` and the `u` record has disappeared +- label it as local/session-derived, not as live Git conflict output +- never recreate it from polling history alone for files the user did not actively open from a conflict row + +## IPC Boundary + +Git status parsing runs in the main process (`src/main/git/status.ts`). The renderer receives status data via the `git:status` IPC channel (`src/main/ipc/filesystem.ts`), which returns `GitStatusEntry[]` directly. The polling hook (`src/renderer/src/components/right-sidebar/useGitStatusPolling.ts`) stores the result in the Zustand store without field filtering. + +Electron's structured clone serialization preserves all enumerable properties on returned objects. Adding new optional fields to `GitUncommittedEntry` (and therefore `GitStatusEntry`) requires no IPC layer changes -- the new fields will serialize and deserialize automatically. + +Session-local state (`Resolved locally` tracking) lives entirely in the renderer and does not cross the IPC boundary. The main process returns only what `git status` reports. + +The main process should also return the detected `GitConflictOperation` alongside `GitStatusEntry[]` so the renderer can label the merge summary correctly. This can be added to the existing `git:status` IPC response shape as an optional field. + +Required renderer-store change: + +- `setGitStatus` must treat conflict metadata changes as meaningful updates +- equality checks that compare only `path`, `status`, and `area` are insufficient for this design +- at minimum, renderer cache invalidation must also react to `conflictStatus`, `conflictKind`, and `conflictStatusSource` +- otherwise a row can remain visually stale when conflict state changes without changing its base `GitFileStatus` + +## Sorting + +Within each section, sort by: + +1. unresolved conflicts +2. resolved locally +3. normal file changes +4. path name + +This mirrors the urgency ordering used by editor UIs. + +## Visual Spec + +### Icons + +Keep the existing file-type/status icon, but add a conflict badge rather than replacing the file icon. + +Reason: + +- users still need file identity and normal file-type affordances +- unresolved conflict is more important than ordinary status letters when the two signals compete +- the UI should prefer the conflict badge over a potentially misleading ordinary status letter + +### Badge styles + +- `Unresolved`: red background, red text emphasis +- `Resolved locally`: success or accent styling with a tooltip that says the state came from this app session, not from current Git conflict output + +### Secondary text + +Show conflict subtype in a small muted label: + +- `Both modified` +- `Deleted by them` + +This is more useful than raw `UU` / `UD` codes. + +When space allows, add helper text focused on the next decision rather than only the Git term. + +## Interaction Details + +### Hover + +On hover, keep existing stage/unstage actions where they are safe, but do not hide conflict badges. + +For unresolved conflicts in `Changes`: + +- hide `Discard` +- hide `Stage` + +For `Resolved locally` rows: + +- restore safe actions appropriate to the row's current section +- keep the badge visible until the session-local continuity state expires + +### Click + +Clicking a conflicted row follows the routing rules defined in section 5 (Diff/editor entry point). Key constraints: + +- do not reuse `openDiff(...)` unchanged for unresolved conflict rows — use the conflict-aware open path +- the opened tab must carry `conflictStatus` and `conflictKind` so the header can render them without re-querying sidebar state +- use the same tab identity as ordinary editable tabs; attach `conflict.kind: 'conflict-editable'` metadata rather than inventing a second tab namespace +- if no working-tree file exists, open a conflict-placeholder tab instead of an ordinary file tab +- `Review conflicts` uses a separate editor-tab mode and open action; it is launched from `Source Control`, not from `Checks` + +Tab reconciliation on status refresh: + +- editable tabs: downgrade conflict metadata in place (same tab id, no duplicate) +- placeholder tabs: close or convert deterministically when the path is no longer unresolved +- conflict-review tabs: preserve their stored snapshot unless the user explicitly refreshes or reopens from the current merge summary +- tab closure alone must not clear sidebar `Resolved locally` state while the file still appears in `Changes` or `Staged Changes` + +### Section actions + +Section-level actions follow the bulk-review rules defined in section 5 (Required bulk-review behavior for v1). The key invariant: do not leave a hidden unsafe path where single-row clicks are safe but bulk-open actions are not. + +### Empty state + +If all remaining files are conflicted, do not show `No changes detected`. The normal section rendering already covers this once conflicts are included in status parsing. + +## Edge Cases + +### Rename plus conflict + +Do not solve this specially in v1. Prefer showing the destination path and conflict badge. + +Important constraint: + +- porcelain v2 `u` records do not provide rename-origin metadata like `2` records do +- assume `oldPath` is unavailable for unresolved conflicts unless a separate Git query is added later +- v1 should not promise rename ancestry in conflict rows + +### Submodule conflicts + +Porcelain v2 also emits `u` records for submodule conflicts. Submodule conflicts are out of scope for v1. The parser should skip `u` records where any of the `h1`/`h2`/`h3` mode fields indicates a submodule (mode `160000`) and leave them unhandled rather than presenting them with the same UX as normal file conflicts. + +### Binary conflicts + +Use the same row badge model even if the diff viewer cannot render a normal text diff. + +## Implementation Plan + +### Phase 1a: Parsing and types + +- extend shared types with conflict metadata and `GitConflictOperation` +- parse porcelain v2 `u` entries in [status.ts](../src/main/git/status.ts), including filesystem existence checks for ambiguous conflict kinds (with `'modified'` fallback on fs errors) +- detect active operation via `.git/MERGE_HEAD`, `.git/REBASE_HEAD`, `.git/CHERRY_PICK_HEAD` and return `GitConflictOperation` alongside status entries +- add renderer-side `CONFLICT_HINT_MAP` constant that maps `GitConflictKind` to user-facing hint strings +- update store equality checks so conflict metadata changes trigger UI updates +- add tests for parsing each porcelain v2 unmerged `XY` variant (`UU`, `UD`, `DU`, `AA`, `AU`, `UA`, `DD`) into the expected `conflictKind` +- add tests for filesystem existence check fallback behavior on error + +### Phase 1b: Sidebar UI + +- render conflict badges and subtype text in [SourceControl.tsx](../src/renderer/src/components/right-sidebar/SourceControl.tsx) +- add merge-summary state derived from live unresolved entries, with operation-aware label and the concrete `Review conflicts` list-view entry point defined above +- add conflict helper text and action-oriented next-step messaging using renderer-side `CONFLICT_HINT_MAP` +- suppress `Discard` and `Stage` for unresolved conflicts; suppress `Discard` for `Resolved locally` rows +- add `trackedConflictPaths` set to the renderer Zustand git store +- add accessibility attributes: `role="status"` on badges, `aria-live="polite"` on merge summary count +- sort conflicted rows to the top of their section +- add UI tests for badge rendering, ordering, merge summary, and suppressed actions + +### Phase 1c: Editor and tab integration + +- extend opened editor tab state with optional conflict metadata +- add a dedicated `openConflictReview(...)` store action that opens the lightweight review tab from `Source Control` +- add a dedicated conflict-aware open action for unresolved rows instead of routing them through `openDiff(...)` +- add a dedicated conflict-placeholder tab for unresolved conflicts without a working-tree file +- add reconciliation logic that downgrades or clears conflict metadata on already-open tabs when live status changes +- render `Resolved locally` only for files tracked via the state machine (see `Resolved locally` state machine section) +- add session-local continuity cleanup so `Resolved locally` expires deterministically based on sidebar presence, not tab closure +- add the lightweight conflict-review tab used by the merge summary and excluded-only bulk-review states + +### Phase 1d: Bulk actions and guardrails + +- make section-level `Open all diffs` explicit about its scope, backed by stored `skippedConflicts` tab state +- add the dedicated conflict-review handoff state for bulk actions where every candidate row was excluded +- ensure any open-capable consumer that assumes file existence branches on `conflictStatus` +- leave passive explorer/tab decorations as plain `modified` in v1 unless separately upgraded, but do not allow unsafe open routing from those surfaces +- add tests that bulk-open paths do not route unresolved conflicts into the normal two-way diff viewer + +### Phase 2 + +- decide whether to build a merge-aware diff view based on index stages +- decide whether to expand the v1 lightweight `Review conflicts` list into a fuller multi-file conflict queue or merge-aware review surface +- consider a dedicated conflict filter if repos with many changes become noisy + +Explicitly out of scope until a stronger design exists: + +- session-based reconstruction for files the user never opened from a live unresolved conflict +- broad resolved-state reconstruction for files the user never opened from a live unresolved conflict + +## Test Cases + +### Parsing + +- each porcelain v2 unmerged `XY` variant (`UU`, `AA`, `DD`, `AU`, `UA`, `DU`, `UD`) maps to the expected `conflictKind` +- unresolved conflict rows are emitted as `area: 'unstaged'` with `conflictStatus: 'unresolved'` +- `status` is `'modified'` when a working-tree file exists, `'deleted'` when it does not (including `both_deleted` and ambiguous `deleted_by_*` / `added_by_*` cases) + +### Sidebar rendering + +- unresolved `both modified` file appears in `Changes` with `Unresolved` badge and subtype +- unresolved `deleted by them` file appears in `Changes` with correct subtype +- merge summary shows unresolved count and a `Review conflicts` action when conflicts are present +- unresolved conflicts do not show the `Stage` or `Discard` actions +- conflict rows sort above normal modified files; `Resolved locally` rows sort between unresolved and ordinary rows +- changing only conflict metadata still triggers a re-render of the affected row + +### Editor / tab integration + +- clicking an unresolved conflict row opens the conflict-aware editable-file path, not the normal unstaged diff path +- the editor header repeats the unresolved badge and subtype for a conflicted file tab +- opening a `both deleted` conflict (or any conflict without a working-tree file) creates a conflict-placeholder tab, not a generic file-load failure +- placeholder and editable conflict tabs show action-oriented next-step guidance +- clicking `Review conflicts` from `Source Control` opens a dedicated conflict-review editor tab, not a `Checks` surface and not a right-sidebar top-level tab +- the conflict-review tab renders from stored snapshot entries rather than reconstructing its contents from live status on every paint + +### Tab reconciliation + +- an already-open conflicted tab downgrades to `Resolved locally` after the `u` record disappears on the next status poll +- downgrading preserves the same tab identity — no duplicate tab is created +- a stale conflict-placeholder tab is closed or converted deterministically once the path is no longer unresolved +- closing a tab does not clear `Resolved locally` while the file still appears in the sidebar + +### Bulk actions + +- `Open all diffs` does not send unresolved conflicts through the normal two-way diff viewer +- the exclusion of unresolved conflicts is explicit in both the trigger label and the resulting view +- the skipped-conflicts notice is stable for the lifetime of the combined-diff tab (sourced from stored `skippedConflicts`, not live status) +- when every candidate row is skipped, the dedicated conflict-review handoff state is shown instead of a generic empty state + +### Resolved locally lifecycle + +- `Resolved locally` appears only for files the user opened from an unresolved conflict row in the current session +- `Resolved locally` clears when the file leaves the sidebar, the session resets, or the file becomes live-unresolved again +- a file in `Resolved locally` state that reappears as a `u` record reverts to `Unresolved` with `conflictStatusSource: 'git'` +- after re-entering unresolved state, the file can transition back to `Resolved locally` if the `u` record disappears again (path remains in `trackedConflictPaths`) +- files that were never opened from a conflict row do not get `Resolved locally` after their `u` record disappears +- a file resolved externally (e.g., `git add` in terminal) drops the `Unresolved` badge on the next poll and reverts to ordinary `GitFileStatus` without showing `Resolved locally`, because the file was never opened from a conflict row +- aborting the operation (`git merge --abort`) clears the entire `trackedConflictPaths` set — no files show `Resolved locally` after an abort + +### Merge summary + +- merge summary label reflects the active operation (`Merge conflicts`, `Rebase conflicts`, `Cherry-pick conflicts`, or `Conflicts` as fallback) +- conflict-review tab empty state shows "all conflicts resolved" with dismiss action when snapshot entries are all resolved + +### Accessibility + +- conflict badges use `role="status"` and include `aria-label` text (e.g., "Unresolved conflict, both modified") so screen readers announce conflict state without requiring visual inspection +- the merge summary unresolved count is a live region (`aria-live="polite"`) so count changes are announced +- focus management (stretch goal for Phase 1b — implement badge and live-region accessibility first): `Review conflicts` moves focus to the opened conflict-review tab; closing the review tab returns focus to the merge summary action + +### Regression + +- normal non-conflict rows keep current behavior + +## Recommendation + +Implement the v1 design by enriching the existing row model and adding a compact merge summary above the existing sections. + +That gives the app the quick scan value users need for active merge conflicts while preserving the current right-sidebar layout, avoiding unsafe fake diff behavior, and keeping the workflow legible after a file leaves the live `u` state. diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index 7751e9c6..b4099a00 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -1,10 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import path from 'path' -const { execFileAsyncMock, readFileMock, rmMock } = vi.hoisted(() => ({ +const { execFileAsyncMock, readFileMock, rmMock, existsSyncMock } = vi.hoisted(() => ({ execFileAsyncMock: vi.fn(), readFileMock: vi.fn(), - rmMock: vi.fn() + rmMock: vi.fn(), + existsSyncMock: vi.fn() })) vi.mock('util', async () => { @@ -20,7 +21,11 @@ vi.mock('fs/promises', () => ({ rm: rmMock })) -import { discardChanges, getBranchCompare, getDiff, isWithinWorktree } from './status' +vi.mock('fs', () => ({ + existsSync: existsSyncMock +})) + +import { discardChanges, getBranchCompare, getDiff, getStatus, isWithinWorktree } from './status' describe('discardChanges', () => { beforeEach(() => { @@ -86,6 +91,7 @@ describe('getDiff', () => { beforeEach(() => { execFileAsyncMock.mockReset() readFileMock.mockReset() + existsSyncMock.mockReset() }) it('uses the index as the left side for unstaged diffs when present', async () => { @@ -147,6 +153,71 @@ describe('getDiff', () => { }) }) +describe('getStatus', () => { + beforeEach(() => { + execFileAsyncMock.mockReset() + readFileMock.mockReset() + existsSyncMock.mockReset() + }) + + it('parses unmerged porcelain v2 entries into unresolved conflict rows', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockImplementation((target: string) => target.endsWith('MERGE_HEAD')) + execFileAsyncMock.mockResolvedValueOnce({ + stdout: + 'u UU N... 100644 100644 100644 100644 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc\tsrc/app.ts\n' + }) + + const result = await getStatus('/repo') + + expect(result.conflictOperation).toBe('merge') + expect(result.entries).toEqual([ + { + path: 'src/app.ts', + area: 'unstaged', + status: 'modified', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + } + ]) + }) + + it('maps deleted conflicts to deleted when the working tree file is absent', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + execFileAsyncMock.mockResolvedValueOnce({ + stdout: + 'u UD N... 100644 100644 000000 100644 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc\tsrc/deleted.ts\n' + }) + + const result = await getStatus('/repo') + + expect(result.entries[0]).toEqual({ + path: 'src/deleted.ts', + area: 'unstaged', + status: 'deleted', + conflictKind: 'deleted_by_them', + conflictStatus: 'unresolved' + }) + }) + + it('falls back to modified when the filesystem existence check throws', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockImplementation(() => { + throw new Error('stat failed') + }) + execFileAsyncMock.mockResolvedValueOnce({ + stdout: + 'u AU N... 100644 100644 100644 100644 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc\tsrc/new.ts\n' + }) + + const result = await getStatus('/repo') + + expect(result.entries[0]?.status).toBe('modified') + expect(result.entries[0]?.conflictKind).toBe('added_by_us') + }) +}) + describe('getBranchCompare', () => { beforeEach(() => { execFileAsyncMock.mockReset() diff --git a/src/main/git/status.ts b/src/main/git/status.ts index e6a51e7a..2b2eba4e 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines */ import { execFile } from 'child_process' +import { existsSync } from 'fs' import { readFile, rm } from 'fs/promises' import { promisify } from 'util' import * as path from 'path' @@ -8,9 +9,12 @@ import type { GitBranchChangeStatus, GitBranchCompareResult, GitBranchCompareSummary, + GitConflictKind, + GitConflictOperation, GitDiffResult, GitFileStatus, - GitStatusEntry + GitStatusEntry, + GitStatusResult } from '../../shared/types' const execFileAsync = promisify(execFile) @@ -19,8 +23,9 @@ const MAX_GIT_SHOW_BYTES = 10 * 1024 * 1024 /** * Parse `git status --porcelain=v2` output into structured entries. */ -export async function getStatus(worktreePath: string): Promise { +export async function getStatus(worktreePath: string): Promise { const entries: GitStatusEntry[] = [] + const conflictOperation = await detectConflictOperation(worktreePath) try { const { stdout } = await execFileAsync( @@ -71,13 +76,18 @@ export async function getStatus(worktreePath: string): Promise // Untracked file const path = line.slice(2) entries.push({ path, status: 'untracked', area: 'untracked' }) + } else if (line.startsWith('u ')) { + const unmergedEntry = await parseUnmergedEntry(worktreePath, line) + if (unmergedEntry) { + entries.push(unmergedEntry) + } } } } catch { // Not a git repo or git not available } - return entries + return { entries, conflictOperation } } function parseStatusChar(char: string): GitFileStatus { @@ -114,6 +124,154 @@ function parseBranchStatusChar(char: string): GitBranchChangeStatus { } } +async function parseUnmergedEntry( + worktreePath: string, + line: string +): Promise { + const tabIndex = line.indexOf('\t') + if (tabIndex === -1) { + return null + } + + const metadata = line.slice(0, tabIndex) + const filePath = line.slice(tabIndex + 1) + const parts = metadata.split(' ') + const xy = parts[1] + const modeStage1 = parts[3] + const modeStage2 = parts[4] + const modeStage3 = parts[5] + + // Why: submodule conflicts (mode 160000) are out of scope for v1. + // Presenting them with normal file-conflict UX would be misleading because + // submodule resolution requires different Git commands and user mental model. + if ([modeStage1, modeStage2, modeStage3].some((mode) => mode === '160000')) { + return null + } + + const conflictKind = parseConflictKind(xy) + if (!conflictKind) { + return null + } + + // Why: porcelain v2 `u` records do not provide rename-origin metadata (unlike + // `2` records), so oldPath is intentionally omitted. v1 should not promise + // rename ancestry in conflict rows without a separate Git query. + return { + path: filePath, + area: 'unstaged', + status: await getConflictCompatibilityStatus(worktreePath, filePath, conflictKind), + conflictKind, + conflictStatus: 'unresolved' + } +} + +function parseConflictKind(xy: string): GitConflictKind | null { + switch (xy) { + case 'UU': + return 'both_modified' + case 'AA': + return 'both_added' + case 'DD': + return 'both_deleted' + case 'AU': + return 'added_by_us' + case 'UA': + return 'added_by_them' + case 'DU': + return 'deleted_by_us' + case 'UD': + return 'deleted_by_them' + default: + return null + } +} + +// Why: the `status` field on conflict entries is a *rendering compatibility* +// choice for existing icon/color plumbing, not a semantic claim about the file. +// The conflict badge and subtype carry the real meaning. We use 'modified' when +// a working-tree file exists and 'deleted' when it does not, so that downstream +// consumers (file explorer decorations, tab badges) get a reasonable fallback +// without needing conflict-aware upgrades in v1. +// +// For `deleted_by_us` / `deleted_by_them` and the `added_by_*` variants, Git's +// behavior depends on the merge strategy, so we check the filesystem rather +// than hardcoding an assumption. +async function getConflictCompatibilityStatus( + worktreePath: string, + filePath: string, + conflictKind: GitConflictKind +): Promise { + if (conflictKind === 'both_modified' || conflictKind === 'both_added') { + return 'modified' + } + + if (conflictKind === 'both_deleted') { + return 'deleted' + } + + try { + return existsSync(path.join(worktreePath, filePath)) ? 'modified' : 'deleted' + } catch { + // Why: if the filesystem check throws (permissions error, unmounted path, + // etc.), 'modified' is the safer fallback. It avoids suppressing the row + // from the sidebar and avoids a misleading 'deleted' when we simply could + // not check. The conflict badge still carries the real semantics. + return 'modified' + } +} + +// Why: there is an inherent race between the `git status` call and these +// fs.existsSync checks — the HEAD file may not yet exist or may already be +// cleaned up by the time we check. In that case we fall back to 'unknown' for +// one poll cycle, which is acceptable. The renderer uses this to label the +// merge summary ("Merge conflicts" vs "Rebase conflicts" vs generic "Conflicts"). +async function detectConflictOperation(worktreePath: string): Promise { + const gitDir = await resolveGitDir(worktreePath) + const mergeHead = path.join(gitDir, 'MERGE_HEAD') + const rebaseHead = path.join(gitDir, 'REBASE_HEAD') + const cherryPickHead = path.join(gitDir, 'CHERRY_PICK_HEAD') + + let hasMergeHead = false + let hasRebaseHead = false + let hasCherryPickHead = false + + try { + hasMergeHead = existsSync(mergeHead) + hasRebaseHead = existsSync(rebaseHead) + hasCherryPickHead = existsSync(cherryPickHead) + } catch { + return 'unknown' + } + + if (Number(hasMergeHead) + Number(hasRebaseHead) + Number(hasCherryPickHead) !== 1) { + return 'unknown' + } + + if (hasMergeHead) { + return 'merge' + } + if (hasRebaseHead) { + return 'rebase' + } + return 'cherry-pick' +} + +async function resolveGitDir(worktreePath: string): Promise { + const dotGitPath = path.join(worktreePath, '.git') + + try { + const dotGitContents = await readFile(dotGitPath, 'utf-8') + const match = dotGitContents.match(/^gitdir:\s*(.+)\s*$/m) + if (match) { + return path.resolve(worktreePath, match[1]) + } + } catch { + // `.git` is likely a directory in a non-worktree checkout. + } + + return dotGitPath +} + /** * Get original and modified content for diffing a file. */ diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index d2f16a0d..368a03ae 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -7,8 +7,8 @@ import type { Store } from '../persistence' import type { DirEntry, GitBranchCompareResult, - GitStatusEntry, GitDiffResult, + GitStatusResult, SearchOptions, SearchResult, SearchFileResult @@ -300,7 +300,7 @@ export function registerFilesystemHandlers(store: Store): void { // ─── Git operations ───────────────────────────────────── ipcMain.handle( 'git:status', - async (_event, args: { worktreePath: string }): Promise => { + async (_event, args: { worktreePath: string }): Promise => { const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) return getStatus(worktreePath) } diff --git a/src/preload/index.d.ts b/src/preload/index.d.ts index 290f5f39..195890f0 100644 --- a/src/preload/index.d.ts +++ b/src/preload/index.d.ts @@ -141,7 +141,7 @@ type FsApi = { } type GitApi = { - status: (args: { worktreePath: string }) => Promise + status: (args: { worktreePath: string }) => Promise diff: (args: { worktreePath: string filePath: string diff --git a/src/preload/index.ts b/src/preload/index.ts index 7d0cca1a..b22f42ff 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -248,7 +248,7 @@ const api = { }, git: { - status: (args: { worktreePath: string }): Promise => + status: (args: { worktreePath: string }): Promise => ipcRenderer.invoke('git:status', args), diff: (args: { worktreePath: string; filePath: string; staged: boolean }): Promise => ipcRenderer.invoke('git:diff', args), diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index 699306b6..df7d592d 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -1,15 +1,12 @@ import React, { useState, useEffect, useCallback, useRef } from 'react' -import { LazySection } from './LazySection' -import { ChevronDown, ChevronRight } from 'lucide-react' -import { DiffEditor, type DiffOnMount } from '@monaco-editor/react' import type { editor as monacoEditor } from 'monaco-editor' import { useAppStore } from '@/store' -import { basename, dirname, joinPath } from '@/lib/path' -import { detectLanguage } from '@/lib/language-detect' +import { joinPath } from '@/lib/path' import '@/lib/monaco-setup' -import { cn } from '@/lib/utils' +import { Button } from '@/components/ui/button' import type { OpenFile } from '@/store/slices/editor' import type { GitDiffResult, GitStatusEntry } from '../../../../shared/types' +import { DiffSectionItem } from './DiffSectionItem' type DiffSection = { key: string @@ -31,6 +28,7 @@ export default function CombinedDiffViewer({ file }: { file: OpenFile }): React. const gitBranchChangesByWorktree = useAppStore((s) => s.gitBranchChangesByWorktree) const gitBranchCompareSummaryByWorktree = useAppStore((s) => s.gitBranchCompareSummaryByWorktree) const openAllDiffs = useAppStore((s) => s.openAllDiffs) + const openConflictReview = useAppStore((s) => s.openConflictReview) const openBranchAllDiffs = useAppStore((s) => s.openBranchAllDiffs) const isDark = settings?.theme === 'dark' || @@ -49,6 +47,9 @@ export default function CombinedDiffViewer({ file }: { file: OpenFile }): React. const uncommittedEntries = React.useMemo( () => (gitStatusByWorktree[file.worktreeId] ?? []).filter((entry) => { + if (entry.conflictStatus === 'unresolved') { + return false + } if (file.combinedAreaFilter) { return entry.area === file.combinedAreaFilter } @@ -211,6 +212,45 @@ export default function CombinedDiffViewer({ file }: { file: OpenFile }): React. } }, [branchSummary, file, openAllDiffs, openBranchAllDiffs]) + if (sections.length === 0 && (file.skippedConflicts?.length ?? 0) > 0) { + return ( +
+
+
+ Conflicted files are reviewed separately +
+
+ This diff view excludes unresolved conflicts because the normal two-way diff pipeline is + not conflict-safe. +
+
+ {file.skippedConflicts!.map((entry) => entry.path).join(', ')} +
+
+ +
+
+
+ ) + } + if (sections.length === 0) { return (
@@ -219,6 +259,38 @@ export default function CombinedDiffViewer({ file }: { file: OpenFile }): React. ) } + const skippedConflictNotice = + (file.skippedConflicts?.length ?? 0) > 0 ? ( +
+
Conflicted files are reviewed separately
+
+ {file.skippedConflicts!.length} unresolved conflict + {file.skippedConflicts!.length === 1 ? '' : 's'} were excluded from this diff view. +
+
+ +
+
+ ) : null + return (
@@ -259,143 +331,25 @@ export default function CombinedDiffViewer({ file }: { file: OpenFile }): React.
- {sections.map((section, index) => { - const language = detectLanguage(section.path) - const fileName = basename(section.path) - const parentDir = dirname(section.path) - const dirPath = parentDir === '.' ? '' : parentDir - const isEditable = section.area === 'unstaged' - - const handleMount: DiffOnMount = (editor, monaco) => { - const modifiedEditor = editor.getModifiedEditor() - - // Track content size to dynamically resize the container - const updateHeight = (): void => { - const contentHeight = editor.getModifiedEditor().getContentHeight() - setSectionHeights((prev) => { - if (prev[index] === contentHeight) { - return prev - } - return { ...prev, [index]: contentHeight } - }) - } - modifiedEditor.onDidContentSizeChange(updateHeight) - updateHeight() - - if (!isEditable) { - return - } - - modifiedEditorsRef.current.set(index, modifiedEditor) - modifiedEditor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => - handleSectionSaveRef.current(index) - ) - modifiedEditor.onDidChangeModelContent(() => { - const current = modifiedEditor.getValue() - setSections((prev) => - prev.map((s, i) => - i === index ? { ...s, dirty: current !== s.modifiedContent } : s - ) - ) - }) - } - - return ( - - - - {!section.collapsed && ( -
- {section.loading ? ( -
- Loading... -
- ) : section.diffResult?.kind === 'binary' ? ( -
-
-
- Binary file changed -
-
- {isBranchMode - ? 'Text diff is unavailable for this file in branch compare.' - : 'Text diff is unavailable for this file.'} -
-
-
- ) : ( - - )} -
- )} - - ) - })} + {skippedConflictNotice} + {sections.map((section, index) => ( + + ))}
) diff --git a/src/renderer/src/components/editor/ConflictComponents.tsx b/src/renderer/src/components/editor/ConflictComponents.tsx new file mode 100644 index 00000000..8725f2e7 --- /dev/null +++ b/src/renderer/src/components/editor/ConflictComponents.tsx @@ -0,0 +1,200 @@ +import React from 'react' +import { CircleCheck, GitMerge, TriangleAlert, X } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { cn } from '@/lib/utils' +import type { OpenFile } from '@/store/slices/editor' +import type { GitConflictKind, GitStatusEntry } from '../../../../shared/types' + +export const CONFLICT_KIND_LABELS: Record = { + both_modified: 'Both modified', + both_added: 'Both added', + deleted_by_us: 'Deleted by us', + deleted_by_them: 'Deleted by them', + added_by_us: 'Added by us', + added_by_them: 'Added by them', + both_deleted: 'Both deleted' +} + +export const CONFLICT_HINT_MAP: Record = { + both_modified: 'Open and edit the final contents', + both_added: 'Choose which version to keep, or combine them', + deleted_by_us: 'Decide whether to restore the file', + deleted_by_them: 'Decide whether to keep the file or accept deletion', + added_by_us: 'Review whether to keep the added file', + added_by_them: 'Review the added file before keeping it', + both_deleted: 'Resolve in Git or restore one side before editing' +} + +export function ConflictBanner({ + file, + entry +}: { + file: OpenFile + entry: GitStatusEntry | null +}): React.JSX.Element | null { + const conflict = file.conflict + if (!conflict) { + return null + } + + const isUnresolved = conflict.conflictStatus === 'unresolved' + const label = isUnresolved ? 'Unresolved' : 'Resolved locally' + + return ( +
+
+ {isUnresolved ? ( + + ) : ( + + )} + + {label} conflict · {CONFLICT_KIND_LABELS[conflict.conflictKind]} + +
+
{CONFLICT_HINT_MAP[conflict.conflictKind]}
+ {!isUnresolved && ( +
+ Session-local continuity state. Git is no longer reporting this file as unmerged. +
+ )} + {entry?.oldPath && ( +
Renamed from {entry.oldPath}
+ )} +
+ ) +} + +export function ConflictPlaceholderView({ file }: { file: OpenFile }): React.JSX.Element | null { + const conflict = file.conflict + if (!conflict) { + return null + } + + return ( +
+
+
+ {CONFLICT_KIND_LABELS[conflict.conflictKind]} +
+
+ {conflict.message ?? 'No working-tree file is available to edit for this conflict.'} +
+
+ {conflict.guidance ?? CONFLICT_HINT_MAP[conflict.conflictKind]} +
+
+
+ ) +} + +export function ConflictReviewPanel({ + file, + liveEntries, + onOpenEntry, + onDismiss, + onRefreshSnapshot, + onReturnToSourceControl +}: { + file: OpenFile + liveEntries: GitStatusEntry[] + onOpenEntry: (entry: GitStatusEntry) => void + onDismiss: () => void + onRefreshSnapshot: () => void + onReturnToSourceControl: () => void +}): React.JSX.Element { + const snapshotEntries = file.conflictReview?.entries ?? [] + const liveEntriesByPath = new Map(liveEntries.map((entry) => [entry.path, entry])) + const unresolvedSnapshotEntries = snapshotEntries.filter( + (entry) => liveEntriesByPath.get(entry.path)?.conflictStatus === 'unresolved' + ) + + if (snapshotEntries.length > 0 && unresolvedSnapshotEntries.length === 0) { + return ( +
+
+
All conflicts resolved
+
+ This review snapshot no longer has any live unresolved conflicts. +
+
+ + +
+
+
+ ) + } + + return ( +
+
+
+ {snapshotEntries.length} unresolved conflict{snapshotEntries.length === 1 ? '' : 's'} +
+
+ Snapshot captured at{' '} + {new Date(file.conflictReview?.snapshotTimestamp ?? Date.now()).toLocaleTimeString()}. +
+
+ + +
+
+
+ {snapshotEntries.map((snapshotEntry) => { + const liveEntry = liveEntriesByPath.get(snapshotEntry.path) + const isStillUnresolved = liveEntry?.conflictStatus === 'unresolved' + return ( + + ) + })} +
+
+ ) +} diff --git a/src/renderer/src/components/editor/DiffSectionItem.tsx b/src/renderer/src/components/editor/DiffSectionItem.tsx new file mode 100644 index 00000000..b9b7b50e --- /dev/null +++ b/src/renderer/src/components/editor/DiffSectionItem.tsx @@ -0,0 +1,184 @@ +import React, { type MutableRefObject } from 'react' +import { LazySection } from './LazySection' +import { ChevronDown, ChevronRight } from 'lucide-react' +import { DiffEditor, type DiffOnMount } from '@monaco-editor/react' +import type { editor as monacoEditor } from 'monaco-editor' +import { basename, dirname } from '@/lib/path' +import { detectLanguage } from '@/lib/language-detect' +import { cn } from '@/lib/utils' +import type { GitDiffResult } from '../../../../shared/types' + +type DiffSection = { + key: string + path: string + status: string + area?: 'staged' | 'unstaged' | 'untracked' + oldPath?: string + originalContent: string + modifiedContent: string + collapsed: boolean + loading: boolean + dirty: boolean + diffResult: GitDiffResult | null +} + +export function DiffSectionItem({ + section, + index, + isBranchMode, + sideBySide, + isDark, + settings, + sectionHeight, + loadSection, + toggleSection, + setSectionHeights, + setSections, + modifiedEditorsRef, + handleSectionSaveRef +}: { + section: DiffSection + index: number + isBranchMode: boolean + sideBySide: boolean + isDark: boolean + settings: { terminalFontSize?: number; terminalFontFamily?: string } | null + sectionHeight: number | undefined + loadSection: (index: number) => void + toggleSection: (index: number) => void + setSectionHeights: React.Dispatch>> + setSections: React.Dispatch> + modifiedEditorsRef: MutableRefObject> + handleSectionSaveRef: MutableRefObject<(index: number) => Promise> +}): React.JSX.Element { + const language = detectLanguage(section.path) + const fileName = basename(section.path) + const parentDir = dirname(section.path) + const dirPath = parentDir === '.' ? '' : parentDir + const isEditable = section.area === 'unstaged' + + const handleMount: DiffOnMount = (editor, monaco) => { + const modifiedEditor = editor.getModifiedEditor() + + const updateHeight = (): void => { + const contentHeight = editor.getModifiedEditor().getContentHeight() + setSectionHeights((prev) => { + if (prev[index] === contentHeight) { + return prev + } + return { ...prev, [index]: contentHeight } + }) + } + modifiedEditor.onDidContentSizeChange(updateHeight) + updateHeight() + + if (!isEditable) { + return + } + + modifiedEditorsRef.current.set(index, modifiedEditor) + modifiedEditor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => + handleSectionSaveRef.current(index) + ) + modifiedEditor.onDidChangeModelContent(() => { + const current = modifiedEditor.getValue() + setSections((prev) => + prev.map((s, i) => (i === index ? { ...s, dirty: current !== s.modifiedContent } : s)) + ) + }) + } + + return ( + + + + {!section.collapsed && ( +
+ {section.loading ? ( +
+ Loading... +
+ ) : section.diffResult?.kind === 'binary' ? ( +
+
+
Binary file changed
+
+ {isBranchMode + ? 'Text diff is unavailable for this file in branch compare.' + : 'Text diff is unavailable for this file.'} +
+
+
+ ) : ( + + )} +
+ )} + + ) +} diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx new file mode 100644 index 00000000..77f76773 --- /dev/null +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -0,0 +1,182 @@ +import React, { lazy } from 'react' +import { detectLanguage } from '@/lib/language-detect' +import { useAppStore } from '@/store' +import { ConflictBanner, ConflictPlaceholderView, ConflictReviewPanel } from './ConflictComponents' +import type { OpenFile } from '@/store/slices/editor' +import type { GitStatusEntry, GitDiffResult } from '../../../../shared/types' + +const MonacoEditor = lazy(() => import('./MonacoEditor')) +const DiffViewer = lazy(() => import('./DiffViewer')) +const CombinedDiffViewer = lazy(() => import('./CombinedDiffViewer')) +const MarkdownPreview = lazy(() => import('./MarkdownPreview')) + +type FileContent = { + content: string + isBinary: boolean +} + +type MarkdownViewMode = 'source' | 'preview' + +export function EditorContent({ + activeFile, + fileContents, + diffContents, + editBuffers, + worktreeEntries, + resolvedLanguage, + isMarkdown, + mdViewMode, + sideBySide, + pendingEditorReveal, + handleContentChange, + handleSave +}: { + activeFile: OpenFile + fileContents: Record + diffContents: Record + editBuffers: Record + worktreeEntries: GitStatusEntry[] + resolvedLanguage: string + isMarkdown: boolean + mdViewMode: MarkdownViewMode + sideBySide: boolean + pendingEditorReveal: { line?: number; column?: number; matchLength?: number } | null + handleContentChange: (content: string) => void + handleSave: (content: string) => Promise +}): React.JSX.Element { + const openConflictFile = useAppStore((s) => s.openConflictFile) + const openConflictReview = useAppStore((s) => s.openConflictReview) + const closeFile = useAppStore((s) => s.closeFile) + const setRightSidebarTab = useAppStore((s) => s.setRightSidebarTab) + + const activeConflictEntry = + worktreeEntries.find((entry) => entry.path === activeFile.relativePath) ?? null + + const isCombinedDiff = + activeFile.mode === 'diff' && + (activeFile.diffSource === 'combined-uncommitted' || + activeFile.diffSource === 'combined-branch') + + const renderMonacoEditor = (fc: FileContent): React.JSX.Element => ( + + ) + + const renderMarkdownContent = (fc: FileContent): React.JSX.Element => { + const currentContent = editBuffers[activeFile.id] ?? fc.content + if (mdViewMode === 'preview') { + return + } + return renderMonacoEditor(fc) + } + + if (activeFile.mode === 'conflict-review') { + return ( + + openConflictFile( + activeFile.worktreeId, + activeFile.filePath, + entry, + detectLanguage(entry.path) + ) + } + onDismiss={() => closeFile(activeFile.id)} + onRefreshSnapshot={() => + openConflictReview( + activeFile.worktreeId, + activeFile.filePath, + worktreeEntries + .filter((entry) => entry.conflictStatus === 'unresolved' && entry.conflictKind) + .map((entry) => ({ + path: entry.path, + conflictKind: entry.conflictKind! + })), + 'live-summary' + ) + } + onReturnToSourceControl={() => setRightSidebarTab('source-control')} + /> + ) + } + + if (isCombinedDiff) { + return + } + + if (activeFile.mode === 'edit') { + if (activeFile.conflict?.kind === 'conflict-placeholder') { + return + } + const fc = fileContents[activeFile.id] + if (!fc) { + return ( +
+ Loading... +
+ ) + } + if (fc.isBinary) { + return ( +
+ Binary file — cannot display +
+ ) + } + return ( +
+ {activeFile.conflict && } +
+ {isMarkdown ? renderMarkdownContent(fc) : renderMonacoEditor(fc)} +
+
+ ) + } + + // Diff mode + const dc = diffContents[activeFile.id] + if (!dc) { + return ( +
+ Loading diff... +
+ ) + } + const isEditable = activeFile.diffSource === 'unstaged' + if (dc.kind === 'binary') { + return ( +
+
+
Binary file changed
+
+ {activeFile.diffSource === 'branch' + ? 'Text diff is unavailable for this file in branch compare.' + : 'Text diff is unavailable for this file.'} +
+
+
+ ) + } + return ( + + ) +} diff --git a/src/renderer/src/components/editor/EditorPanel.tsx b/src/renderer/src/components/editor/EditorPanel.tsx index 2005558d..35673a4a 100644 --- a/src/renderer/src/components/editor/EditorPanel.tsx +++ b/src/renderer/src/components/editor/EditorPanel.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState, lazy, Suspense } from 'react' +import React, { useCallback, useEffect, useState, Suspense } from 'react' import { Columns2, Rows2 } from 'lucide-react' import { useAppStore } from '@/store' import { detectLanguage } from '@/lib/language-detect' @@ -6,13 +6,9 @@ import { getEditorHeaderCopyState } from './editor-header' import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip' import type { MarkdownViewMode } from '@/store/slices/editor' import MarkdownViewToggle from './MarkdownViewToggle' +import { EditorContent } from './EditorContent' import type { GitDiffResult } from '../../../../shared/types' -const MonacoEditor = lazy(() => import('./MonacoEditor')) -const DiffViewer = lazy(() => import('./DiffViewer')) -const CombinedDiffViewer = lazy(() => import('./CombinedDiffViewer')) -const MarkdownPreview = lazy(() => import('./MarkdownPreview')) - type FileContent = { content: string isBinary: boolean @@ -25,7 +21,7 @@ export default function EditorPanel(): React.JSX.Element | null { const activeFileId = useAppStore((s) => s.activeFileId) const markFileDirty = useAppStore((s) => s.markFileDirty) const pendingEditorReveal = useAppStore((s) => s.pendingEditorReveal) - + const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) const markdownViewMode = useAppStore((s) => s.markdownViewMode) const setMarkdownViewMode = useAppStore((s) => s.setMarkdownViewMode) @@ -44,7 +40,13 @@ export default function EditorPanel(): React.JSX.Element | null { if (!activeFile) { return } + if (activeFile.mode === 'conflict-review') { + return + } if (activeFile.mode === 'edit') { + if (activeFile.conflict?.kind === 'conflict-placeholder') { + return + } if (fileContents[activeFile.id]) { return } @@ -278,6 +280,7 @@ export default function EditorPanel(): React.JSX.Element | null { (activeFile.diffSource === 'combined-uncommitted' || activeFile.diffSource === 'combined-branch') const headerCopyState = getEditorHeaderCopyState(activeFile) + const worktreeEntries = gitStatusByWorktree[activeFile.worktreeId] ?? [] const resolvedLanguage = activeFile.mode === 'diff' ? detectLanguage(activeFile.relativePath) @@ -295,30 +298,6 @@ export default function EditorPanel(): React.JSX.Element | null {
) - const renderMonacoEditor = (fc: FileContent): React.JSX.Element => ( - - ) - - const renderMarkdownContent = (fc: FileContent): React.JSX.Element => { - const currentContent = editBuffers[activeFile.id] ?? fc.content - - if (mdViewMode === 'preview') { - return - } - - return renderMonacoEditor(fc) - } - return (
{!isCombinedDiff && ( @@ -367,65 +346,20 @@ export default function EditorPanel(): React.JSX.Element | null {
)} - {isCombinedDiff ? ( - - ) : activeFile.mode === 'edit' ? ( - (() => { - const fc = fileContents[activeFile.id] - if (!fc) { - return ( -
- Loading... -
- ) - } - if (fc.isBinary) { - return ( -
- Binary file — cannot display -
- ) - } - return isMarkdown ? renderMarkdownContent(fc) : renderMonacoEditor(fc) - })() - ) : ( - (() => { - const dc = diffContents[activeFile.id] - if (!dc) { - return ( -
- Loading diff... -
- ) - } - const isEditable = activeFile.diffSource === 'unstaged' - if (dc.kind === 'binary') { - return ( -
-
-
Binary file changed
-
- {activeFile.diffSource === 'branch' - ? 'Text diff is unavailable for this file in branch compare.' - : 'Text diff is unavailable for this file.'} -
-
-
- ) - } - return ( - - ) - })() - )} + ) diff --git a/src/renderer/src/components/editor/editor-header.ts b/src/renderer/src/components/editor/editor-header.ts index ac8b12d5..f3412f97 100644 --- a/src/renderer/src/components/editor/editor-header.ts +++ b/src/renderer/src/components/editor/editor-header.ts @@ -9,6 +9,15 @@ export type EditorHeaderCopyState = { } export function getEditorHeaderCopyState(file: OpenFile): EditorHeaderCopyState { + if (file.mode === 'conflict-review') { + return { + copyText: file.filePath, + copyToastLabel: 'Worktree path copied', + pathLabel: 'Conflict Review', + pathTitle: file.filePath + } + } + const isCombinedDiff = file.mode === 'diff' && (file.diffSource === 'combined-uncommitted' || file.diffSource === 'combined-branch') diff --git a/src/renderer/src/components/editor/editor-labels.ts b/src/renderer/src/components/editor/editor-labels.ts index fa66ad02..6f23759f 100644 --- a/src/renderer/src/components/editor/editor-labels.ts +++ b/src/renderer/src/components/editor/editor-labels.ts @@ -24,6 +24,10 @@ export function getEditorDisplayLabel( file: OpenFile, variant: EditorLabelVariant = 'fileName' ): string { + if (file.mode === 'conflict-review') { + return 'Conflict Review' + } + if (file.mode !== 'diff') { return getBaseLabel(file, variant) } diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index b51c45be..c758f24c 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -13,7 +13,10 @@ import { FilePlus, FileQuestion, ArrowRightLeft, - FolderOpen + FolderOpen, + GitMerge, + TriangleAlert, + CircleCheck } from 'lucide-react' import { useAppStore } from '@/store' import { detectLanguage } from '@/lib/language-detect' @@ -39,10 +42,11 @@ import { PullRequestIcon } from './checks-helpers' import type { GitBranchChangeEntry, GitBranchCompareSummary, + GitConflictKind, + GitConflictOperation, GitStatusEntry, PRInfo } from '../../../../shared/types' -import { getSourceControlActions } from './source-control-actions' import { STATUS_COLORS, STATUS_LABELS } from './status-display' type SourceControlScope = 'all' | 'uncommitted' @@ -68,12 +72,37 @@ const SECTION_LABELS: Record<(typeof SECTION_ORDER)[number], string> = { const BRANCH_REFRESH_INTERVAL_MS = 5000 +const CONFLICT_KIND_LABELS: Record = { + both_modified: 'Both modified', + both_added: 'Both added', + deleted_by_us: 'Deleted by us', + deleted_by_them: 'Deleted by them', + added_by_us: 'Added by us', + added_by_them: 'Added by them', + both_deleted: 'Both deleted' +} + +// Why: hint text is derived at render time in the renderer, not returned by the +// main process on GitUncommittedEntry. This keeps UI copy out of the IPC layer +// and the main-process parser. See also ConflictComponents.tsx for the editor- +// side copy of this map. +const CONFLICT_HINT_MAP: Record = { + both_modified: 'Open and edit the final contents', + both_added: 'Choose which version to keep, or combine them', + deleted_by_us: 'Decide whether to restore the file', + deleted_by_them: 'Decide whether to keep the file or accept deletion', + added_by_us: 'Review whether to keep the added file', + added_by_them: 'Review the added file before keeping it', + both_deleted: 'Resolve in Git or restore one side before editing' +} + export default function SourceControl(): React.JSX.Element { const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const rightSidebarTab = useAppStore((s) => s.rightSidebarTab) const repos = useAppStore((s) => s.repos) const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) + const gitConflictOperationByWorktree = useAppStore((s) => s.gitConflictOperationByWorktree) const gitBranchChangesByWorktree = useAppStore((s) => s.gitBranchChangesByWorktree) const gitBranchCompareSummaryByWorktree = useAppStore((s) => s.gitBranchCompareSummaryByWorktree) const prCache = useAppStore((s) => s.prCache) @@ -81,7 +110,10 @@ export default function SourceControl(): React.JSX.Element { const beginGitBranchCompareRequest = useAppStore((s) => s.beginGitBranchCompareRequest) const setGitBranchCompareResult = useAppStore((s) => s.setGitBranchCompareResult) const revealInExplorer = useAppStore((s) => s.revealInExplorer) + const trackConflictPath = useAppStore((s) => s.trackConflictPath) const openDiff = useAppStore((s) => s.openDiff) + const openConflictFile = useAppStore((s) => s.openConflictFile) + const openConflictReview = useAppStore((s) => s.openConflictReview) const openBranchDiff = useAppStore((s) => s.openBranchDiff) const openAllDiffs = useAppStore((s) => s.openAllDiffs) const openBranchAllDiffs = useAppStore((s) => s.openBranchAllDiffs) @@ -120,6 +152,9 @@ export default function SourceControl(): React.JSX.Element { const branchSummary = activeWorktreeId ? (gitBranchCompareSummaryByWorktree[activeWorktreeId] ?? null) : null + const conflictOperation = activeWorktreeId + ? (gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown') + : 'unknown' const isBranchVisible = rightSidebarTab === 'source-control' useEffect(() => { @@ -151,7 +186,7 @@ export default function SourceControl(): React.JSX.Element { const branchCompareAvailable = branchSummary?.status === 'ready' const hasBranchEntries = branchCompareAvailable && branchEntries.length > 0 const branchName = activeWorktree?.branch.replace(/^refs\/heads\//, '') ?? 'HEAD' - const prCacheKey = activeRepo ? `${activeRepo.path}::${branchName}` : null + const prCacheKey = activeRepo && branchName ? `${activeRepo.path}::${branchName}` : null const prInfo: PRInfo | null = prCacheKey ? (prCache[prCacheKey]?.data ?? null) : null const grouped = useMemo(() => { @@ -163,9 +198,25 @@ export default function SourceControl(): React.JSX.Element { for (const entry of entries) { groups[entry.area].push(entry) } + for (const area of SECTION_ORDER) { + groups[area].sort(compareGitStatusEntries) + } return groups }, [entries]) + const unresolvedConflicts = useMemo( + () => entries.filter((entry) => entry.conflictStatus === 'unresolved' && entry.conflictKind), + [entries] + ) + const unresolvedConflictReviewEntries = useMemo( + () => + unresolvedConflicts.map((entry) => ({ + path: entry.path, + conflictKind: entry.conflictKind! + })), + [unresolvedConflicts] + ) + const refreshBranchCompare = useCallback(async () => { if (!activeWorktreeId || !worktreePath || !effectiveBaseRef) { return @@ -250,6 +301,19 @@ export default function SourceControl(): React.JSX.Element { if (!activeWorktreeId || !worktreePath) { return } + // Why: unresolved conflicts must NOT go through openDiff(). The current + // diff pipeline reads normal index and worktree content, not merge stages, + // so routing conflicts into the two-way diff viewer would show misleading + // content. Instead, we use the conflict-aware open path (openConflictFile) + // which opens an editable file view or a placeholder, depending on whether + // a working-tree file exists. + if (entry.conflictKind && entry.conflictStatus) { + if (entry.conflictStatus === 'unresolved') { + trackConflictPath(activeWorktreeId, entry.path, entry.conflictKind) + } + openConflictFile(activeWorktreeId, worktreePath, entry, detectLanguage(entry.path)) + return + } openDiff( activeWorktreeId, joinPath(worktreePath, entry.path), @@ -258,7 +322,7 @@ export default function SourceControl(): React.JSX.Element { entry.area === 'staged' ) }, - [activeWorktreeId, openDiff, worktreePath] + [activeWorktreeId, openConflictFile, openDiff, trackConflictPath, worktreePath] ) const openCommittedDiff = useCallback( @@ -390,6 +454,26 @@ export default function SourceControl(): React.JSX.Element { )}
+ {unresolvedConflictReviewEntries.length > 0 && ( +
+ { + if (!activeWorktreeId || !worktreePath) { + return + } + openConflictReview( + activeWorktreeId, + worktreePath, + unresolvedConflictReviewEntries, + 'live-summary' + ) + }} + /> +
+ )} + {scope === 'all' && showGenericEmptyState ? ( entry.conflictStatus === 'unresolved').length + } isCollapsed={isCollapsed} onToggle={() => toggleSection(area)} actions={ - { - e.stopPropagation() - if (activeWorktreeId && worktreePath) { - openAllDiffs(activeWorktreeId, worktreePath, undefined, area) - } - }} - /> + items.some((entry) => entry.conflictStatus === 'unresolved') ? ( + + ) : ( + { + e.stopPropagation() + if (activeWorktreeId && worktreePath) { + openAllDiffs(activeWorktreeId, worktreePath, undefined, area) + } + }} + /> + ) } /> {!isCollapsed && @@ -647,12 +751,14 @@ function CompareUnavailable({ function SectionHeader({ label, count, + conflictCount = 0, isCollapsed, onToggle, actions }: { label: string count: number + conflictCount?: number isCollapsed: boolean onToggle: () => void actions?: React.ReactNode @@ -669,12 +775,63 @@ function SectionHeader({ /> {label} {count} + {conflictCount > 0 && ( + + · {conflictCount} conflict{conflictCount === 1 ? '' : 's'} + + )}
{actions}
) } +function ConflictSummaryCard({ + conflictOperation, + unresolvedCount, + onReview +}: { + conflictOperation: GitConflictOperation + unresolvedCount: number + onReview: () => void +}): React.JSX.Element { + const operationLabel = + conflictOperation === 'merge' + ? 'Merge conflicts' + : conflictOperation === 'rebase' + ? 'Rebase conflicts' + : conflictOperation === 'cherry-pick' + ? 'Cherry-pick conflicts' + : 'Conflicts' + + return ( +
+
+ +
+
{`${operationLabel}: ${unresolvedCount} unresolved`}
+
+ Resolved files move back to normal changes after they leave the live conflict state. +
+
+ +
+
+ ) +} + function UncommittedEntryRow({ entry, worktreePath, @@ -696,7 +853,26 @@ function UncommittedEntryRow({ const fileName = basename(entry.path) const parentDir = dirname(entry.path) const dirPath = parentDir === '.' ? '' : parentDir - const actions = getSourceControlActions(entry.area) + const isUnresolvedConflict = entry.conflictStatus === 'unresolved' + const isResolvedLocally = entry.conflictStatus === 'resolved_locally' + const conflictLabel = entry.conflictKind ? CONFLICT_KIND_LABELS[entry.conflictKind] : null + const conflictHint = entry.conflictKind ? CONFLICT_HINT_MAP[entry.conflictKind] : null + // Why: Stage is suppressed for unresolved conflicts because `git add` would + // immediately erase the `u` record — the only live conflict signal in the + // sidebar — before the user has actually reviewed the file. The user should + // resolve in the editor first, then stage from the post-resolution state. + // + // Discard is hidden for both unresolved AND resolved_locally rows in v1. + // For unresolved: discarding is too easy to misfire on a high-risk file. + // For resolved_locally: discarding can silently re-create the conflict or + // lose the resolution, and v1 does not have UX to explain this clearly. + const canDiscard = + !isUnresolvedConflict && + !isResolvedLocally && + (entry.area === 'unstaged' || entry.area === 'untracked') + const canStage = + !isUnresolvedConflict && (entry.area === 'unstaged' || entry.area === 'untracked') + const canUnstage = entry.area === 'staged' return ( { + if (isUnresolvedConflict && entry.status === 'deleted') { + e.preventDefault() + return + } const absolutePath = joinPath(worktreePath, entry.path) e.dataTransfer.setData('text/x-orca-file-path', absolutePath) e.dataTransfer.effectAllowed = 'copy' @@ -714,18 +894,31 @@ function UncommittedEntryRow({ onClick={onOpen} > - - {fileName} - {dirPath && {dirPath}} - - - {STATUS_LABELS[entry.status]} - +
+
+ {fileName} + {dirPath && ( + {dirPath} + )} +
+ {conflictLabel && conflictHint && ( +
+ {conflictLabel} · {conflictHint} +
+ )} +
+ {entry.conflictStatus ? ( + + ) : ( + + {STATUS_LABELS[entry.status]} + + )}
- {actions.includes('discard') && ( + {canDiscard && ( )} - {actions.includes('stage') && ( + {canStage && ( )} - {actions.includes('unstage') && ( + {canUnstage && ( + + {label} + + ) + + if (isUnresolvedConflict) { + return badge + } + + return ( + + + {badge} + + Local session state derived from a conflict you opened here. + + + + ) +} + function BranchEntryRow({ entry, worktreePath, @@ -874,3 +1103,20 @@ function ActionButton({ ) } + +function compareGitStatusEntries(a: GitStatusEntry, b: GitStatusEntry): number { + return ( + getConflictSortRank(a) - getConflictSortRank(b) || + a.path.localeCompare(b.path, undefined, { numeric: true }) + ) +} + +function getConflictSortRank(entry: GitStatusEntry): number { + if (entry.conflictStatus === 'unresolved') { + return 0 + } + if (entry.conflictStatus === 'resolved_locally') { + return 1 + } + return 2 +} diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts index 1a69707b..4c8083a2 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useMemo } from 'react' import { useAppStore } from '@/store' -import type { GitStatusEntry } from '../../../../shared/types' +import type { GitStatusResult } from '../../../../shared/types' const POLL_INTERVAL_MS = 3000 @@ -27,8 +27,8 @@ export function useGitStatusPolling(): void { return } try { - const entries = (await window.api.git.status({ worktreePath })) as GitStatusEntry[] - setGitStatus(activeWorktreeId, entries) + const status = (await window.api.git.status({ worktreePath })) as GitStatusResult + setGitStatus(activeWorktreeId, status) } catch { // ignore } diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index d29d7df1..43dcf68a 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -124,3 +124,133 @@ describe('createEditorSlice markdown preview state', () => { ]) }) }) + +describe('createEditorSlice conflict status reconciliation', () => { + it('tracks unresolved conflicts when opened through the conflict-safe entry point', () => { + const store = createEditorStore() + + store.getState().openConflictFile( + 'wt-1', + '/repo', + { + path: 'src/conflict.ts', + status: 'modified', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved', + conflictStatusSource: 'git' + }, + 'typescript' + ) + store.getState().setGitStatus('wt-1', { + conflictOperation: 'merge', + entries: [{ path: 'src/conflict.ts', status: 'modified', area: 'staged' }] + }) + + expect(store.getState().trackedConflictPathsByWorktree['wt-1']).toEqual({ + 'src/conflict.ts': 'both_modified' + }) + expect(store.getState().gitStatusByWorktree['wt-1']).toEqual([ + { + path: 'src/conflict.ts', + status: 'modified', + area: 'staged', + conflictKind: 'both_modified', + conflictStatus: 'resolved_locally', + conflictStatusSource: 'session' + } + ]) + }) + + it('marks tracked conflicts as resolved locally after live conflict state disappears', () => { + const store = createEditorStore() + + store.getState().trackConflictPath('wt-1', 'src/conflict.ts', 'both_modified') + store.getState().setGitStatus('wt-1', { + conflictOperation: 'merge', + entries: [ + { + path: 'src/conflict.ts', + status: 'modified', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + } + ] + }) + store.getState().setGitStatus('wt-1', { + conflictOperation: 'merge', + entries: [{ path: 'src/conflict.ts', status: 'modified', area: 'staged' }] + }) + + expect(store.getState().gitStatusByWorktree['wt-1']).toEqual([ + { + path: 'src/conflict.ts', + status: 'modified', + area: 'staged', + conflictKind: 'both_modified', + conflictStatus: 'resolved_locally', + conflictStatusSource: 'session' + } + ]) + }) + + it('clears tracked conflict continuity on abort-like transitions', () => { + const store = createEditorStore() + + store.getState().trackConflictPath('wt-1', 'src/conflict.ts', 'both_modified') + store.getState().setGitStatus('wt-1', { + conflictOperation: 'merge', + entries: [ + { + path: 'src/conflict.ts', + status: 'modified', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + } + ] + }) + store.getState().setGitStatus('wt-1', { + conflictOperation: 'unknown', + entries: [{ path: 'src/conflict.ts', status: 'modified', area: 'unstaged' }] + }) + + expect(store.getState().gitStatusByWorktree['wt-1']).toEqual([ + { path: 'src/conflict.ts', status: 'modified', area: 'unstaged' } + ]) + expect(store.getState().trackedConflictPathsByWorktree['wt-1']).toEqual({}) + }) +}) + +describe('createEditorSlice combined diff exclusions', () => { + it('stores skipped unresolved conflicts on combined diff tabs', () => { + const store = createEditorStore() + + store.getState().setGitStatus('wt-1', { + conflictOperation: 'merge', + entries: [ + { + path: 'src/conflict.ts', + status: 'modified', + area: 'unstaged', + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + }, + { + path: 'src/normal.ts', + status: 'modified', + area: 'unstaged' + } + ] + }) + store.getState().openAllDiffs('wt-1', '/repo') + + expect(store.getState().openFiles[0]).toEqual( + expect.objectContaining({ + id: 'wt-1::all-diffs::uncommitted', + skippedConflicts: [{ path: 'src/conflict.ts', conflictKind: 'both_modified' }] + }) + ) + }) +}) diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index a511258e..280d6fea 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -1,10 +1,16 @@ /* eslint-disable max-lines */ import type { StateCreator } from 'zustand' import type { AppState } from '../types' +import { joinPath } from '@/lib/path' import type { GitBranchChangeEntry, GitBranchCompareSummary, + GitConflictKind, + GitConflictOperation, + GitConflictResolutionStatus, + GitConflictStatusSource, GitStatusEntry, + GitStatusResult, SearchResult } from '../../../../shared/types' @@ -27,6 +33,41 @@ type CombinedDiffAlternate = { branchCompare?: BranchCompareSnapshot } +export type OpenConflictMetadata = { + kind: 'conflict-editable' | 'conflict-placeholder' + conflictKind: GitConflictKind + conflictStatus: GitConflictResolutionStatus + conflictStatusSource: GitConflictStatusSource + message?: string + guidance?: string +} + +export type ConflictReviewEntry = { + path: string + conflictKind: GitConflictKind +} + +export type ConflictReviewState = { + source: 'live-summary' | 'combined-diff-exclusion' + snapshotTimestamp: number + entries: ConflictReviewEntry[] +} + +export type CombinedDiffSkippedConflict = { + path: string + conflictKind: GitConflictKind +} + +// Why: OpenFile is a single type (not a discriminated union on `mode`) because +// the tab plumbing (reorder, close, activate) treats all tabs uniformly. However, +// consumers that access `filePath` must be aware that conflict-review tabs use +// the worktree root as filePath, not a real file. Any code that assumes filePath +// points to an actual file should check `mode` first. +// +// `skippedConflicts` is stored directly on the tab state so the exclusion notice +// in combined-diff views is stable for the tab's lifetime. It must NOT be +// reconstructed from live status on every render — the live set can change +// between polls, which would make the notice flicker or become inaccurate. export type OpenFile = { id: string // use filePath as unique key filePath: string // absolute path @@ -34,13 +75,16 @@ export type OpenFile = { worktreeId: string language: string isDirty: boolean - mode: 'edit' | 'diff' diffSource?: DiffSource branchCompare?: BranchCompareSnapshot branchOldPath?: string combinedAlternate?: CombinedDiffAlternate combinedAreaFilter?: string // filter combined diff to a specific area (e.g. 'staged', 'unstaged', 'untracked') + conflict?: OpenConflictMetadata + skippedConflicts?: CombinedDiffSkippedConflict[] + conflictReview?: ConflictReviewState isPreview?: boolean // preview tabs are replaced when another file is single-clicked + mode: 'edit' | 'diff' | 'conflict-review' } export type RightSidebarTab = 'explorer' | 'search' | 'source-control' | 'checks' @@ -109,6 +153,18 @@ export type EditorSlice = { alternate?: CombinedDiffAlternate, areaFilter?: string ) => void + openConflictFile: ( + worktreeId: string, + worktreePath: string, + entry: GitStatusEntry, + language: string + ) => void + openConflictReview: ( + worktreeId: string, + worktreePath: string, + entries: ConflictReviewEntry[], + source: ConflictReviewState['source'] + ) => void openBranchAllDiffs: ( worktreeId: string, worktreePath: string, @@ -122,7 +178,10 @@ export type EditorSlice = { // Git status cache gitStatusByWorktree: Record - setGitStatus: (worktreeId: string, entries: GitStatusEntry[]) => void + gitConflictOperationByWorktree: Record + trackedConflictPathsByWorktree: Record> + trackConflictPath: (worktreeId: string, path: string, conflictKind: GitConflictKind) => void + setGitStatus: (worktreeId: string, status: GitStatusResult) => void gitBranchChangesByWorktree: Record gitBranchCompareSummaryByWorktree: Record gitBranchCompareRequestKeyByWorktree: Record @@ -244,6 +303,10 @@ export const createEditorSlice: StateCreator = (s existing.mode === file.mode && existing.diffSource === file.diffSource && existing.branchCompare?.compareVersion === file.branchCompare?.compareVersion && + existing.conflict?.kind === file.conflict?.kind && + existing.conflict?.conflictKind === file.conflict?.conflictKind && + existing.conflict?.conflictStatus === file.conflict?.conflictStatus && + existing.conflictReview?.snapshotTimestamp === file.conflictReview?.snapshotTimestamp && existing.isPreview === updatedPreview ) { return activeResult @@ -258,6 +321,10 @@ export const createEditorSlice: StateCreator = (s branchCompare: file.branchCompare, branchOldPath: file.branchOldPath, combinedAlternate: file.combinedAlternate, + combinedAreaFilter: file.combinedAreaFilter, + conflict: file.conflict, + skippedConflicts: file.skippedConflicts, + conflictReview: file.conflictReview, isPreview: updatedPreview } : f @@ -314,6 +381,11 @@ export const createEditorSlice: StateCreator = (s } }), + // Why: closing a tab does NOT clear Resolved locally state. If the file is + // still present in Changes or Staged Changes, the continuity badge should + // remain visible until the file leaves the sidebar, the session resets, or + // the file becomes live-unresolved again. trackedConflictPaths is tied to + // sidebar presence, not tab lifecycle. closeFile: (fileId) => set((s) => { const closedFile = s.openFiles.find((f) => f.id === fileId) @@ -451,7 +523,16 @@ export const createEditorSlice: StateCreator = (s return { openFiles: needsUpdate ? s.openFiles.map((f) => - f.id === id ? { ...f, mode: 'diff' as const, diffSource } : f + f.id === id + ? { + ...f, + mode: 'diff' as const, + diffSource, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined + } + : f ) : s.openFiles, activeFileId: id, @@ -468,7 +549,10 @@ export const createEditorSlice: StateCreator = (s language, isDirty: false, mode: 'diff', - diffSource + diffSource, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined } return { openFiles: [...s.openFiles, newFile], @@ -493,7 +577,10 @@ export const createEditorSlice: StateCreator = (s mode: 'diff' as const, diffSource: 'branch' as const, branchCompare, - branchOldPath: entry.oldPath + branchOldPath: entry.oldPath, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined } : f ), @@ -505,7 +592,7 @@ export const createEditorSlice: StateCreator = (s } const newFile: OpenFile = { id, - filePath: `${worktreePath}/${entry.path}`, + filePath: joinPath(worktreePath, entry.path), relativePath: entry.path, worktreeId, language, @@ -513,7 +600,10 @@ export const createEditorSlice: StateCreator = (s mode: 'diff', diffSource: 'branch', branchCompare, - branchOldPath: entry.oldPath + branchOldPath: entry.oldPath, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined } return { openFiles: [...s.openFiles, newFile], @@ -526,6 +616,15 @@ export const createEditorSlice: StateCreator = (s openAllDiffs: (worktreeId, worktreePath, alternate, areaFilter) => set((s) => { + const relevantEntries = (s.gitStatusByWorktree[worktreeId] ?? []).filter((entry) => { + if (areaFilter) { + return entry.area === areaFilter + } + return entry.area !== 'untracked' + }) + const skippedConflicts = relevantEntries + .filter((entry) => entry.conflictStatus === 'unresolved' && entry.conflictKind) + .map((entry) => ({ path: entry.path, conflictKind: entry.conflictKind! })) const id = areaFilter ? `${worktreeId}::all-diffs::uncommitted::${areaFilter}` : `${worktreeId}::all-diffs::uncommitted` @@ -538,7 +637,16 @@ export const createEditorSlice: StateCreator = (s if (existing) { return { openFiles: s.openFiles.map((f) => - f.id === id ? { ...f, combinedAlternate: alternate, combinedAreaFilter: areaFilter } : f + f.id === id + ? { + ...f, + combinedAlternate: alternate, + combinedAreaFilter: areaFilter, + skippedConflicts, + conflictReview: undefined, + conflict: undefined + } + : f ), activeFileId: id, activeTabType: 'editor', @@ -556,7 +664,10 @@ export const createEditorSlice: StateCreator = (s mode: 'diff', diffSource: 'combined-uncommitted', combinedAlternate: alternate, - combinedAreaFilter: areaFilter + combinedAreaFilter: areaFilter, + skippedConflicts, + conflictReview: undefined, + conflict: undefined } return { openFiles: [...s.openFiles, newFile], @@ -567,6 +678,134 @@ export const createEditorSlice: StateCreator = (s } }), + openConflictFile: (worktreeId, worktreePath, entry, language) => + set((s) => { + const absolutePath = joinPath(worktreePath, entry.path) + const id = absolutePath + const conflict = toOpenConflictMetadata(entry) + const existing = s.openFiles.find((f) => f.id === id) + const nextTracked = + entry.conflictStatus === 'unresolved' && entry.conflictKind + ? { + ...s.trackedConflictPathsByWorktree[worktreeId], + [entry.path]: entry.conflictKind + } + : s.trackedConflictPathsByWorktree[worktreeId] + + if (!conflict) { + return s + } + + if (existing) { + return { + openFiles: s.openFiles.map((f) => + f.id === id + ? { + ...f, + mode: 'edit' as const, + language, + relativePath: entry.path, + filePath: absolutePath, + conflict, + diffSource: undefined, + skippedConflicts: undefined, + conflictReview: undefined + } + : f + ), + activeFileId: id, + activeTabType: 'editor', + activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id }, + activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' }, + trackedConflictPathsByWorktree: + nextTracked === s.trackedConflictPathsByWorktree[worktreeId] + ? s.trackedConflictPathsByWorktree + : { ...s.trackedConflictPathsByWorktree, [worktreeId]: nextTracked } + } + } + + const newFile: OpenFile = { + id, + filePath: absolutePath, + relativePath: entry.path, + worktreeId, + language, + isDirty: false, + mode: 'edit', + conflict + } + + return { + openFiles: [...s.openFiles, newFile], + activeFileId: id, + activeTabType: 'editor', + activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id }, + activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' }, + trackedConflictPathsByWorktree: + nextTracked === s.trackedConflictPathsByWorktree[worktreeId] + ? s.trackedConflictPathsByWorktree + : { ...s.trackedConflictPathsByWorktree, [worktreeId]: nextTracked } + } + }), + + // Why: Review conflicts is launched from Source Control into the editor area, + // not from Checks. Merge-conflict review is source-control work, not CI/PR + // status. The tab renders from a stored snapshot (entries + timestamp), not + // from live status on every paint, so the list is stable even if the live + // unresolved set changes between polls. + openConflictReview: (worktreeId, worktreePath, entries, source) => + set((s) => { + const id = `${worktreeId}::conflict-review` + const conflictReview: ConflictReviewState = { + source, + snapshotTimestamp: Date.now(), + entries + } + const existing = s.openFiles.find((f) => f.id === id) + + if (existing) { + return { + openFiles: s.openFiles.map((f) => + f.id === id + ? { + ...f, + mode: 'conflict-review' as const, + relativePath: 'Conflict Review', + filePath: worktreePath, + language: 'plaintext', + conflictReview, + conflict: undefined, + skippedConflicts: undefined + } + : f + ), + activeFileId: id, + activeTabType: 'editor', + activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id }, + activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' } + } + } + + const newFile: OpenFile = { + id, + filePath: worktreePath, + relativePath: 'Conflict Review', + worktreeId, + language: 'plaintext', + isDirty: false, + mode: 'conflict-review', + conflictReview + } + + return { + openFiles: [...s.openFiles, newFile], + activeFileId: id, + activeTabType: 'editor', + activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id }, + activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' } + } + }), + openBranchAllDiffs: (worktreeId, worktreePath, compare, alternate) => set((s) => { const branchCompare = toBranchCompareSnapshot(compare) @@ -575,7 +814,16 @@ export const createEditorSlice: StateCreator = (s if (existing) { return { openFiles: s.openFiles.map((f) => - f.id === id ? { ...f, branchCompare, combinedAlternate: alternate } : f + f.id === id + ? { + ...f, + branchCompare, + combinedAlternate: alternate, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined + } + : f ), activeFileId: id, activeTabType: 'editor', @@ -593,7 +841,10 @@ export const createEditorSlice: StateCreator = (s mode: 'diff', diffSource: 'combined-branch', branchCompare, - combinedAlternate: alternate + combinedAlternate: alternate, + conflict: undefined, + skippedConflicts: undefined, + conflictReview: undefined } return { openFiles: [...s.openFiles, newFile], @@ -613,22 +864,110 @@ export const createEditorSlice: StateCreator = (s // Git status gitStatusByWorktree: {}, - setGitStatus: (worktreeId, entries) => + gitConflictOperationByWorktree: {}, + trackedConflictPathsByWorktree: {}, + trackConflictPath: (worktreeId, path, conflictKind) => set((s) => { - const prev = s.gitStatusByWorktree[worktreeId] + const nextTracked = { + ...s.trackedConflictPathsByWorktree[worktreeId], + [path]: conflictKind + } + return { + trackedConflictPathsByWorktree: { + ...s.trackedConflictPathsByWorktree, + [worktreeId]: nextTracked + } + } + }), + // Why: session-local conflict tracking (trackedConflictPaths, Resolved locally + // state) lives entirely in the renderer and never crosses the IPC boundary. + // The main process returns only what `git status` reports. The renderer is + // responsible for setting conflictStatusSource ('git' for live u-records, + // 'session' for Resolved locally) and for all Resolved locally lifecycle. + setGitStatus: (worktreeId, status) => + set((s) => { + const prevEntries = s.gitStatusByWorktree[worktreeId] ?? [] + const prevOperation = s.gitConflictOperationByWorktree[worktreeId] ?? 'unknown' + const currentTracked = { ...s.trackedConflictPathsByWorktree[worktreeId] } + // Why: conflictStatusSource is NOT set by the main process. The renderer + // stamps 'git' here for live u-records, and 'session' below when applying + // Resolved locally state. This keeps the main process free of session + // awareness while letting the renderer distinguish the two sources. + const normalizedEntries = status.entries.map((entry) => + entry.conflictStatus === 'unresolved' + ? { ...entry, conflictStatusSource: 'git' as const } + : entry + ) + const unresolvedEntries = normalizedEntries.filter( + (entry) => entry.conflictStatus === 'unresolved' && entry.conflictKind + ) + const unresolvedByPath = new Map(unresolvedEntries.map((entry) => [entry.path, entry])) + + // Why: when the operation is aborted (git merge --abort, etc.), all u-records + // disappear and the HEAD file is cleaned up simultaneously. We detect this as + // the operation transitioning to 'unknown' with zero unresolved entries. In + // this case we clear the entire trackedConflictPaths set rather than + // transitioning each path to Resolved locally — abort is NOT resolution, and + // showing "Resolved locally" on every previously-conflicted file after an + // abort would be misleading. if ( - prev && - prev.length === entries.length && - prev.every( - (e, i) => - e.path === entries[i].path && - e.status === entries[i].status && - e.area === entries[i].area - ) + status.conflictOperation === 'unknown' && + prevOperation !== 'unknown' && + unresolvedByPath.size === 0 ) { + for (const path of Object.keys(currentTracked)) { + delete currentTracked[path] + } + } + + const nextEntries = normalizedEntries.map((entry) => { + if (entry.conflictStatus === 'unresolved') { + return entry + } + const trackedConflictKind = currentTracked[entry.path] + if (!trackedConflictKind) { + return entry + } + return { + ...entry, + conflictKind: trackedConflictKind, + conflictStatus: 'resolved_locally' as const, + conflictStatusSource: 'session' as const + } + }) + + const visiblePaths = new Set(nextEntries.map((entry) => entry.path)) + for (const path of Object.keys(currentTracked)) { + if (!visiblePaths.has(path) && !unresolvedByPath.has(path)) { + delete currentTracked[path] + } + } + + const nextOpenFiles = reconcileOpenFilesForStatus(s.openFiles, worktreeId, nextEntries) + const statusUnchanged = areGitStatusEntriesEqual(prevEntries, nextEntries) + const trackedUnchanged = areTrackedConflictMapsEqual( + s.trackedConflictPathsByWorktree[worktreeId] ?? {}, + currentTracked + ) + const openFilesUnchanged = nextOpenFiles === s.openFiles + const operationUnchanged = prevOperation === status.conflictOperation + + if (statusUnchanged && trackedUnchanged && openFilesUnchanged && operationUnchanged) { return s } - return { gitStatusByWorktree: { ...s.gitStatusByWorktree, [worktreeId]: entries } } + + return { + openFiles: nextOpenFiles, + gitStatusByWorktree: statusUnchanged + ? s.gitStatusByWorktree + : { ...s.gitStatusByWorktree, [worktreeId]: nextEntries }, + gitConflictOperationByWorktree: operationUnchanged + ? s.gitConflictOperationByWorktree + : { ...s.gitConflictOperationByWorktree, [worktreeId]: status.conflictOperation }, + trackedConflictPathsByWorktree: trackedUnchanged + ? s.trackedConflictPathsByWorktree + : { ...s.trackedConflictPathsByWorktree, [worktreeId]: currentTracked } + } }), gitBranchChangesByWorktree: {}, gitBranchCompareSummaryByWorktree: {}, @@ -752,3 +1091,105 @@ function toBranchCompareSnapshot(compare: GitBranchCompareSummary): BranchCompar compareVersion: getCompareVersion(compare) } } + +function toOpenConflictMetadata(entry: GitStatusEntry): OpenConflictMetadata | undefined { + if (!entry.conflictKind || !entry.conflictStatus || !entry.conflictStatusSource) { + return undefined + } + + const hasWorkingTreeFile = entry.status !== 'deleted' + return hasWorkingTreeFile + ? { + kind: 'conflict-editable', + conflictKind: entry.conflictKind, + conflictStatus: entry.conflictStatus, + conflictStatusSource: entry.conflictStatusSource + } + : { + kind: 'conflict-placeholder', + conflictKind: entry.conflictKind, + conflictStatus: entry.conflictStatus, + conflictStatusSource: entry.conflictStatusSource, + message: 'This file is in a conflict state, but no working-tree file is available to edit.', + guidance: 'Resolve the conflict in Git or restore one side before reopening it.' + } +} + +// Why: equality checks comparing only path/status/area are insufficient. A row +// can change from unresolved to resolved_locally (or vice versa) without its +// base GitFileStatus changing. Without checking conflictKind, conflictStatus, +// and conflictStatusSource here, the affected row would remain visually stale. +function areGitStatusEntriesEqual(prev: GitStatusEntry[], next: GitStatusEntry[]): boolean { + return ( + prev.length === next.length && + prev.every( + (entry, index) => + entry.path === next[index].path && + entry.status === next[index].status && + entry.area === next[index].area && + entry.oldPath === next[index].oldPath && + entry.conflictKind === next[index].conflictKind && + entry.conflictStatus === next[index].conflictStatus && + entry.conflictStatusSource === next[index].conflictStatusSource + ) + ) +} + +function areTrackedConflictMapsEqual( + prev: Record, + next: Record +): boolean { + const prevKeys = Object.keys(prev) + const nextKeys = Object.keys(next) + return prevKeys.length === nextKeys.length && prevKeys.every((key) => prev[key] === next[key]) +} + +function reconcileOpenFilesForStatus( + openFiles: OpenFile[], + worktreeId: string, + nextEntries: GitStatusEntry[] +): OpenFile[] { + const entriesByPath = new Map(nextEntries.map((entry) => [entry.path, entry])) + let changed = false + + const nextOpenFiles = openFiles.flatMap((file) => { + if (file.worktreeId !== worktreeId) { + return [file] + } + + if (file.mode === 'conflict-review') { + return [file] + } + + const entry = entriesByPath.get(file.relativePath) + if (!file.conflict) { + return [file] + } + + if (!entry || !entry.conflictKind || !entry.conflictStatus || !entry.conflictStatusSource) { + changed = true + return file.conflict.kind === 'conflict-placeholder' ? [] : [{ ...file, conflict: undefined }] + } + + const nextConflict = toOpenConflictMetadata(entry) + if (!nextConflict) { + return [file] + } + + if ( + file.conflict.kind === nextConflict.kind && + file.conflict.conflictKind === nextConflict.conflictKind && + file.conflict.conflictStatus === nextConflict.conflictStatus && + file.conflict.conflictStatusSource === nextConflict.conflictStatusSource && + file.conflict.message === nextConflict.message && + file.conflict.guidance === nextConflict.guidance + ) { + return [file] + } + + changed = true + return [{ ...file, conflict: nextConflict }] + }) + + return changed ? nextOpenFiles : openFiles +} diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 43058103..69bf594e 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -292,8 +292,8 @@ export const createGitHubSlice: StateCreator = (s return updates }) - // Re-fetch - if (!worktree.isBare) { + // Re-fetch (skip when branch is empty — detached HEAD during rebase) + if (!worktree.isBare && branch) { void get().fetchPRForBranch(repo.path, branch, { force: true }) } if (worktree.linkedIssue) { diff --git a/src/shared/types.ts b/src/shared/types.ts index e7b0823b..c5228d03 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -215,16 +215,47 @@ export type DirEntry = { // ─── Git Status ───────────────────────────────────────────── export type GitFileStatus = 'modified' | 'added' | 'deleted' | 'renamed' | 'untracked' | 'copied' export type GitStagingArea = 'staged' | 'unstaged' | 'untracked' +export type GitConflictKind = + | 'both_modified' + | 'both_added' + | 'both_deleted' + | 'added_by_us' + | 'added_by_them' + | 'deleted_by_us' + | 'deleted_by_them' +export type GitConflictResolutionStatus = 'unresolved' | 'resolved_locally' +export type GitConflictStatusSource = 'git' | 'session' +export type GitConflictOperation = 'merge' | 'rebase' | 'cherry-pick' | 'unknown' + +// Compatibility note for non-upgraded consumers: +// Any consumer that has not been upgraded to read `conflictStatus` may still +// render `modified` styling via the `status` field (which is a compatibility +// fallback, not a semantic claim). However, such consumers must NOT offer +// file-existence-dependent affordances (diff loading, drag payloads, editable- +// file opening) for entries where `conflictStatus === 'unresolved'` — the file +// may not exist on disk (e.g. both_deleted). This affects file explorer +// decorations, tab badges, and any surface outside Source Control. +// +// `conflictStatusSource` is never set by the main process. The renderer stamps +// 'git' for live u-records and 'session' for Resolved locally state. export type GitUncommittedEntry = { path: string status: GitFileStatus area: GitStagingArea oldPath?: string + conflictKind?: GitConflictKind + conflictStatus?: GitConflictResolutionStatus + conflictStatusSource?: GitConflictStatusSource } export type GitStatusEntry = GitUncommittedEntry +export type GitStatusResult = { + entries: GitStatusEntry[] + conflictOperation: GitConflictOperation +} + export type GitBranchChangeStatus = 'modified' | 'added' | 'deleted' | 'renamed' | 'copied' export type GitBranchChangeEntry = {