From 47cff065f081b5160875831ff20a321f5b2c58ac Mon Sep 17 00:00:00 2001 From: lick <2188718831@qq.com> Date: Wed, 4 Nov 2026 21:46:20 -0800 Subject: [PATCH 1/3] =?UTF-8?q?feat(core):=20#1662=20=E8=BD=A6=E9=81=93=20?= =?UTF-8?q?B=20Git=20=E5=86=99=E5=85=A5=E4=B8=8B=E6=B2=89=EF=BC=88GIT-21/2?= =?UTF-8?q?2=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Core 新增 git.planDiffBlocks / git.applyDiffBlock / git.stageGutterChange：由 Core 计划提交评审块、持有 include/exclude/rollback patch，并在仓库写锁内校验快照令牌后写入 - 完整文件 diff 的 hunk 由 Core 按 Git 默认上下文拆分，并返回逐 hunk 字节保真 patch 与 lineStart/lineEnd - git.apply discard 在写锁内检查 filter 与 working-tree-encoding，删除 unidiffZero 参数 - Windows 删除 TS 侧块规划、gutter patch 拼接、check-attr 与 U0 patch 切片，以及 Tauri host 的 hunk_patch 重建 - 新增共享 fixture 与 Core 集成测试，更新功能矩阵与 rust-core-api 契约 Co-Authored-By: Claude Opus 5.5 --- rust/lithe-core/src/git/diff_blocks.rs | 992 ++++++++++++++++++ rust/lithe-core/src/git/mod.rs | 107 +- rust/lithe-core/src/git/shelf.rs | 1 - rust/lithe-core/src/protocol/command.rs | 9 + rust/lithe-core/src/protocol/contracts.rs | 7 + rust/lithe-core/src/runtime/dispatcher.rs | 9 + rust/lithe-core/src/tests/git_diff_blocks.rs | 391 +++++++ rust/lithe-core/src/tests/mod.rs | 1 + shared/contracts/rust-core-api.md | 31 +- shared/fixtures/git/diff-blocks-v1.json | 309 ++++++ .../features/editor-git-gutter-stage.json | 2 + .../features/git-diff-review.json | 2 +- .../features/git-status-commit.json | 2 +- windows/tauri/src-tauri/src/platform.rs | 129 +-- .../editor/components/monaco-editor.tsx | 4 +- .../src/features/git/api/git-diff-api.ts | 129 --- .../features/git/api/git-diff-block-api.ts | 133 +++ .../git/components/diff/monaco-git-diff.tsx | 6 +- .../git/hooks/use-commit-diff-review.test.tsx | 90 +- .../git/hooks/use-commit-diff-review.ts | 33 +- .../git/services/commit-diff-review.test.ts | 129 ++- .../git/services/commit-diff-review.ts | 45 +- .../git/services/git-gutter-actions.ts | 25 +- .../tauri/src/features/git/types/git.types.ts | 14 +- .../commit-diff-blocks.integration.test.ts | 151 --- .../git/utils/commit-diff-blocks.test.ts | 68 -- .../features/git/utils/commit-diff-blocks.ts | 158 +-- .../git/utils/git-gutter-changes.test.ts | 71 -- .../features/git/utils/git-gutter-changes.ts | 40 - .../utils/lossy-rollback-acceptance.test.ts | 241 ----- .../git/utils/lossy-rollback-attrs.ts | 77 -- .../git/utils/lossy-rollback-channel.test.ts | 248 ----- .../git/utils/lossy-rollback-patch.test.ts | 213 ---- .../git/utils/lossy-rollback-patch.ts | 351 ------- .../utils/monaco-diff-hunk-actions.test.ts | 78 +- .../git/utils/monaco-diff-hunk-actions.ts | 29 +- .../git/utils/monaco-diff-rows.test.ts | 26 +- .../features/git/utils/monaco-diff-rows.ts | 93 +- .../tauri/src/platform/core-result-adapter.ts | 12 +- 39 files changed, 2274 insertions(+), 2182 deletions(-) create mode 100644 rust/lithe-core/src/git/diff_blocks.rs create mode 100644 rust/lithe-core/src/tests/git_diff_blocks.rs create mode 100644 shared/fixtures/git/diff-blocks-v1.json create mode 100644 windows/tauri/src/features/git/api/git-diff-block-api.ts delete mode 100644 windows/tauri/src/features/git/utils/commit-diff-blocks.integration.test.ts delete mode 100644 windows/tauri/src/features/git/utils/commit-diff-blocks.test.ts delete mode 100644 windows/tauri/src/features/git/utils/lossy-rollback-acceptance.test.ts delete mode 100644 windows/tauri/src/features/git/utils/lossy-rollback-attrs.ts delete mode 100644 windows/tauri/src/features/git/utils/lossy-rollback-channel.test.ts delete mode 100644 windows/tauri/src/features/git/utils/lossy-rollback-patch.test.ts delete mode 100644 windows/tauri/src/features/git/utils/lossy-rollback-patch.ts diff --git a/rust/lithe-core/src/git/diff_blocks.rs b/rust/lithe-core/src/git/diff_blocks.rs new file mode 100644 index 0000000000..c01b56b1eb --- /dev/null +++ b/rust/lithe-core/src/git/diff_blocks.rs @@ -0,0 +1,992 @@ +//! Plans byte-faithful writable diff blocks, Git-default stageable hunks and +//! editor line-range staging, then performs those writes after re-validating +//! the planned snapshot under the repository write lease. +//! +//! Hosts never build patch text. Core reads complete-file patches, keeps every +//! original byte (CR, missing final newline), and derives the index or worktree +//! patch itself. A write request names a planned block and the snapshot token +//! it was planned from; Core rebuilds the plan under the lease and refuses the +//! write when the repository moved, instead of applying lines that shifted. + +use super::{ + command, diff_impl, execute_git_readonly, is_safe_pathspec, rewrite::RewriteLease, + validate_root, GitCommandRequest, GitDiffRequest, FULL_FILE_CONTEXT_LINES, +}; +use crate::protocol::{CoreError, ErrorCode, GitDiffFileResponse, GitDiffHunkResponse}; +use serde::{Deserialize, Serialize}; +use serde_json::{json, Value}; +use sha2::{Digest, Sha256}; + +/// Context Git uses by default around a change; staged patches carry the same +/// amount so `git apply` still rejects a target that moved since planning. +const PATCH_CONTEXT_LINES: usize = 3; +/// Marker Git prints after a line that has no terminating newline. +const NO_NEWLINE_MARKER: &str = "\\ No newline at end of file"; + +/// Stable failure: the planned snapshot no longer matches the repository. +pub(crate) const DIFF_BLOCK_STALE: &str = "GIT_DIFF_BLOCK_STALE"; +/// Stable failure: the named block exists but cannot perform the action. +pub(crate) const DIFF_BLOCK_UNAVAILABLE: &str = "GIT_DIFF_BLOCK_UNAVAILABLE"; +/// Stable failure: attributes may rewrite worktree bytes, so a hunk rollback +/// could not be proven byte-faithful and must fall back to whole-file actions. +pub(crate) const HUNK_ROLLBACK_UNSAFE: &str = "GIT_HUNK_ROLLBACK_UNSAFE"; +/// Stable failure: the editor change no longer matches the index or the file on disk. +pub(crate) const GUTTER_CHANGE_STALE: &str = "GIT_GUTTER_CHANGE_STALE"; + +fn invalid(message: &'static str) -> CoreError { + CoreError::new(ErrorCode::InvalidRequest, message) +} + +/// One file line exactly as Git printed it inside a patch. +#[derive(Clone, Debug, PartialEq, Eq)] +struct RawLine { + /// Line text without the patch prefix and terminating LF; a CR is kept. + text: String, + /// False only for a final line Git marked with `\ No newline at end of file`. + newline: bool, +} + +impl RawLine { + /// Text as an editor shows it: one trailing CR belongs to the line break. + fn display(&self) -> &str { + self.text.strip_suffix('\r').unwrap_or(&self.text) + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Operation { + Context, + Removed, + Added, +} + +/// One body line of a single-hunk patch with its original marker, if any. +struct PatchEntry { + operation: Operation, + line: RawLine, + /// The exact `\` marker line Git printed after this entry. + marker: Option, +} + +/// A patch section with exactly one hunk, parsed without losing bytes. +struct SingleHunkDocument { + /// File header lines before the hunk (`diff --git`, `index`, `---`, `+++`). + header: Vec, + entries: Vec, + old_start: usize, + new_start: usize, +} + +impl SingleHunkDocument { + /// Parses one file section. `None` means the section has no hunk, more + /// than one hunk, or a body Git would not produce; callers fail closed. + fn parse(section: &str) -> Option { + let body = section.strip_suffix('\n').unwrap_or(section); + let mut lines = body.split('\n'); + let mut header = Vec::new(); + let hunk_header = loop { + let line = lines.next()?; + if line.starts_with("@@") { + break line; + } + header.push(line.to_string()); + }; + let (old_start, old_count, new_start, new_count) = parse_range_header(hunk_header)?; + let mut entries: Vec = Vec::new(); + for line in lines { + let operation = match line.as_bytes().first() { + Some(b' ') => Operation::Context, + Some(b'-') => Operation::Removed, + Some(b'+') => Operation::Added, + Some(b'\\') => { + let entry = entries.last_mut()?; + if entry.marker.is_some() { + return None; + } + entry.marker = Some(line.to_string()); + entry.line.newline = false; + continue; + } + // `diff.suppressBlankEmpty` prints an empty context line bare. + None => Operation::Context, + Some(_) => return None, + }; + if entries.last().is_some_and(|entry| entry.marker.is_some()) { + // Only the final line of a side can lack its newline. + let previous = entries.last().expect("checked above"); + if previous.operation == Operation::Context || previous.operation == operation { + return None; + } + } + entries.push(PatchEntry { + operation, + line: RawLine { + text: line.get(1..).unwrap_or_default().to_string(), + newline: true, + }, + marker: None, + }); + } + let document = Self { + header, + entries, + old_start, + new_start, + }; + (document.old().len() == old_count && document.new().len() == new_count) + .then_some(document) + } + + /// Parses a section whose hunk spans both complete files. + fn parse_complete(section: &str) -> Option { + let document = Self::parse(section)?; + let expected = |count: usize| usize::from(count > 0); + (document.old_start == expected(document.old().len()) + && document.new_start == expected(document.new().len())) + .then_some(document) + } + + fn old(&self) -> Vec { + self.side(Operation::Removed) + } + + fn new(&self) -> Vec { + self.side(Operation::Added) + } + + fn side(&self, changed: Operation) -> Vec { + self.entries + .iter() + .filter(|entry| entry.operation == Operation::Context || entry.operation == changed) + .map(|entry| entry.line.clone()) + .collect() + } + + /// Minimal header that applies content only. Mode, rename and index lines + /// are dropped so a block write never stages a mode change or a rename. + fn content_header(&self) -> Option<[String; 3]> { + let diff = self.header.iter().find(|line| line.starts_with("diff --git "))?; + let old = self.header.iter().find(|line| line.starts_with("--- "))?; + let new = self.header.iter().find(|line| line.starts_with("+++ "))?; + if old == "--- /dev/null" || new == "+++ /dev/null" { + return None; + } + Some([diff.clone(), old.clone(), new.clone()]) + } + + /// Contiguous change runs in patch order, with zero-based side positions. + fn runs(&self) -> Vec { + let mut runs: Vec = Vec::new(); + let mut open = false; + let (mut old, mut new) = (0, 0); + for (index, entry) in self.entries.iter().enumerate() { + // Unified line 0 is the hunk header, so body entries start at 1. + let unified = index + 1; + if entry.operation == Operation::Context { + open = false; + old += 1; + new += 1; + continue; + } + if !open { + runs.push(ChangeRun { + old_start: old, + old_end: old, + new_start: new, + new_end: new, + first_removed: None, + first_added: None, + }); + open = true; + } + let run = runs.last_mut().expect("run was opened"); + if entry.operation == Operation::Removed { + old += 1; + run.old_end = old; + run.first_removed.get_or_insert(unified); + } else { + new += 1; + run.new_end = new; + run.first_added.get_or_insert(unified); + } + } + runs + } +} + +fn parse_range_header(header: &str) -> Option<(usize, usize, usize, usize)> { + let mut columns = header.split(' '); + if columns.next()? != "@@" { + return None; + } + let range = |value: &str| -> Option<(usize, usize)> { + let mut parts = value.splitn(2, ','); + let start = parts.next()?.parse().ok()?; + let count = parts.next().map_or(Some(1), |count| count.parse().ok())?; + Some((start, count)) + }; + let (old_start, old_count) = range(columns.next()?.strip_prefix('-')?)?; + let (new_start, new_count) = range(columns.next()?.strip_prefix('+')?)?; + (columns.next()? == "@@").then_some((old_start, old_count, new_start, new_count)) +} + +/// One contiguous run of removed and added lines. +#[derive(Clone, Copy, Debug)] +struct ChangeRun { + old_start: usize, + old_end: usize, + new_start: usize, + new_end: usize, + /// Unified line index of the first removed line in the run. + first_removed: Option, + /// Unified line index of the first added line in the run. + first_added: Option, +} + +impl ChangeRun { + /// Identity shared with the per-file unified `lines`: hosts address rows + /// by `line-`, the first old-side row of the run, else its first + /// new-side row. + fn id(&self) -> String { + format!( + "line-{}", + self.first_removed.or(self.first_added).unwrap_or_default() + ) + } +} + +fn emit(patch: &mut String, prefix: char, line: &RawLine) { + patch.push(prefix); + patch.push_str(&line.text); + patch.push('\n'); + if !line.newline { + patch.push_str(NO_NEWLINE_MARKER); + patch.push('\n'); + } +} + +/// Builds a patch that replaces `base[start..end]` with `replacement`, with +/// Git's default context from `base`. Returns `None` for a no-op, or when the +/// result would leave a line without its newline in the middle of the file. +fn replacement_patch( + header: &[String; 3], + base: &[RawLine], + start: usize, + end: usize, + replacement: &[RawLine], +) -> Option { + if start > end || end > base.len() || base[start..end] == *replacement { + return None; + } + let result = base[..start] + .iter() + .chain(replacement) + .chain(&base[end..]) + .collect::>(); + if result + .iter() + .take(result.len().saturating_sub(1)) + .any(|line| !line.newline) + { + return None; + } + let context_start = start.saturating_sub(PATCH_CONTEXT_LINES); + let context_end = (end + PATCH_CONTEXT_LINES).min(base.len()); + let old_count = context_end - context_start; + let new_count = (start - context_start) + replacement.len() + (context_end - end); + // Git names the line before an empty range, or 0 at the start of the file. + let first = |count: usize| { + if count == 0 { + context_start + } else { + context_start + 1 + } + }; + let mut patch = header.join("\n"); + patch.push('\n'); + patch.push_str(&format!( + "@@ -{},{old_count} +{},{new_count} @@\n", + first(old_count), + first(new_count) + )); + for line in &base[context_start..start] { + emit(&mut patch, ' ', line); + } + for line in &base[start..end] { + emit(&mut patch, '-', line); + } + for line in replacement { + emit(&mut patch, '+', line); + } + for line in &base[end..context_end] { + emit(&mut patch, ' ', line); + } + Some(patch) +} + +// --------------------------------------------------------------------------- +// Git-default stageable hunks for complete-file diffs +// --------------------------------------------------------------------------- + +/// Splits a complete-file section into the hunks Git would print with its +/// default three context lines. Each hunk keeps the original bytes and the +/// section's file header so `git apply` reproduces Git's own `-U3` patch. +/// `None` means the section cannot be split safely; callers then expose no +/// writable hunk rather than one that would stage the whole file. +pub(super) fn stageable_hunks(section: &str) -> Option> { + let document = SingleHunkDocument::parse(section)?; + let changed = |index: usize| document.entries[index].operation != Operation::Context; + let mut hunks = Vec::new(); + let mut index = 0; + while index < document.entries.len() { + if !changed(index) { + index += 1; + continue; + } + let mut last = index; + // Like Git, changes whose surrounding context would touch share one hunk. + let mut next = index + 1; + while next < document.entries.len() && next - last - 1 <= 2 * PATCH_CONTEXT_LINES { + if changed(next) { + last = next; + } + next += 1; + } + let start = index.saturating_sub(PATCH_CONTEXT_LINES); + let end = (last + PATCH_CONTEXT_LINES + 1).min(document.entries.len()); + hunks.push(split_hunk(&document, hunks.len(), start, end)); + index = last + 1; + } + Some(hunks) +} + +fn split_hunk( + document: &SingleHunkDocument, + ordinal: usize, + start: usize, + end: usize, +) -> GitDiffHunkResponse { + let side_before = |changed: Operation| { + document.entries[..start] + .iter() + .filter(|entry| entry.operation == Operation::Context || entry.operation == changed) + .count() + }; + let side_count = |changed: Operation| { + document.entries[start..end] + .iter() + .filter(|entry| entry.operation == Operation::Context || entry.operation == changed) + .count() + }; + let range = |first: usize, before: usize, count: usize| { + // Git names the line before an empty range, or 0 at the start of the file. + let line = first.max(1) + before; + format!("{},{count}", if count == 0 { line - 1 } else { line }) + }; + let old_count = side_count(Operation::Removed); + let new_count = side_count(Operation::Added); + let header = format!( + "@@ -{} +{} @@", + range( + document.old_start, + side_before(Operation::Removed), + old_count + ), + range(document.new_start, side_before(Operation::Added), new_count) + ); + let mut patch = document.header.join("\n"); + if !patch.is_empty() { + patch.push('\n'); + } + patch.push_str(&header); + patch.push('\n'); + for entry in &document.entries[start..end] { + patch.push(match entry.operation { + Operation::Context => ' ', + Operation::Removed => '-', + Operation::Added => '+', + }); + patch.push_str(&entry.line.text); + patch.push('\n'); + if let Some(marker) = &entry.marker { + patch.push_str(marker); + patch.push('\n'); + } + } + GitDiffHunkResponse { + id: format!("hunk-{ordinal}"), + header, + patch, + content_patch: None, + // Unified line 0 is the section's single hunk header. + line_start: Some(start + 1), + line_end: Some(end + 1), + } +} + +// --------------------------------------------------------------------------- +// Commit review blocks: HEAD -> worktree display, HEAD -> index inclusion +// --------------------------------------------------------------------------- + +/// One side of a complete-file comparison as Git reported it. +#[derive(Clone, Copy)] +pub(super) struct PlanSide<'a> { + pub patch: &'a str, + /// Git status letter of the section: `A`, `D`, `R` or `M`. + pub status: &'a str, + pub is_binary: bool, +} + +/// One planned block and the Core-owned patches its actions apply. +#[derive(Clone, Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub(super) struct DiffBlock { + /// `line-` into the HEAD -> worktree section's unified lines. + id: String, + /// The index already holds exactly this block's worktree content. + checked: bool, + /// The index holds content that is neither HEAD's nor the worktree's. + indeterminate: bool, + can_toggle: bool, + can_rollback: bool, + /// New and deleted files toggle as a whole through ordinary staging. + #[serde(skip_serializing_if = "std::ops::Not::not")] + whole_file: bool, + /// Forward index patch that makes the index hold this block's worktree text. + #[serde(skip)] + include: Option, + /// Forward index patch that restores HEAD's text for this block in the index. + #[serde(skip)] + exclude: Option, + /// Forward worktree patch that restores the index text for this block. + #[serde(skip)] + rollback: Option, + /// Rollback is otherwise possible; only attribute safety may still block it. + #[serde(skip)] + rollback_candidate: bool, +} + +fn whole_file_block(run: &ChangeRun, checked: bool, indeterminate: bool) -> DiffBlock { + DiffBlock { + id: run.id(), + checked, + indeterminate, + can_toggle: true, + can_rollback: false, + whole_file: true, + include: None, + exclude: None, + rollback: None, + rollback_candidate: false, + } +} + +/// Plans commit-review blocks. The HEAD -> worktree comparison owns the +/// visible blocks and the HEAD -> index comparison owns inclusion. A block can +/// hold only some staged lines, and earlier staged edits shift index +/// coordinates, so each block's index span is mapped explicitly. `None` means +/// the file only supports whole-file inclusion. `writable` is false when Git +/// output lost bytes; `rollback_safe` reports the worktree attribute check. +pub(super) fn plan_blocks( + worktree: PlanSide, + index: Option, + writable: bool, + rollback_safe: bool, +) -> Option> { + if worktree.is_binary || worktree.status == "R" { + return None; + } + let visible = SingleHunkDocument::parse_complete(worktree.patch)?; + let index = index.filter(|side| !side.patch.is_empty()); + let indexed = match index { + Some(side) if side.is_binary => return None, + Some(side) => Some(SingleHunkDocument::parse_complete(side.patch)?), + None => None, + }; + let runs = visible.runs(); + let (head_lines, worktree_lines) = (visible.old(), visible.new()); + if matches!(worktree.status, "A" | "D") { + let [run] = runs.as_slice() else { + return None; + }; + let checked = indexed + .as_ref() + .is_some_and(|indexed| indexed.new() == worktree_lines); + // Whole-file inclusion stages the file through `git add`, which keeps + // its bytes even when the patch text could not be decoded faithfully. + return Some(vec![whole_file_block( + run, + checked, + !checked && indexed.is_some(), + )]); + } + let (index_head, index_lines, index_runs) = match &indexed { + Some(indexed) => (indexed.old(), indexed.new(), indexed.runs()), + None => (head_lines.clone(), head_lines.clone(), Vec::new()), + }; + if index_head != head_lines { + return None; + } + let header = visible.content_header()?; + let mut prior_cursor = 0; + let mut shift_before: isize = 0; + let blocks = runs + .iter() + .map(|run| { + // Index changes are disjoint and sorted; those wholly before this + // block form a prefix whose size changes shift its index span. + while let Some(prior) = index_runs.get(prior_cursor) { + if prior.old_end <= run.old_start && prior.old_start < run.old_start { + shift_before += size_delta(prior); + prior_cursor += 1; + } else { + break; + } + } + let start = offset(run.old_start, shift_before); + let mut end = offset(run.old_end, shift_before); + let mut overlaps_boundary = false; + for prior in index_runs[prior_cursor..] + .iter() + .take_while(|prior| prior.old_start <= run.old_end) + { + // A staged insertion at a replacement's outer boundary can be an + // index-only edit absent from this worktree block. Do not erase it. + if prior.old_start == prior.old_end + && prior.old_start == run.old_start + && run.old_start != run.old_end + { + overlaps_boundary = true; + } + if prior.old_start >= run.old_start + && prior.old_end <= run.old_end + && (prior.old_start < run.old_end || run.old_start == run.old_end) + { + end = offset(end, size_delta(prior)); + } else if prior.old_start < run.old_end && prior.old_end > run.old_start { + overlaps_boundary = true; + } + } + let before = &head_lines[run.old_start..run.old_end]; + let after = &worktree_lines[run.new_start..run.new_end]; + let span = index_lines.get(start..end).unwrap_or_default(); + let checked = span == after; + let indeterminate = !checked && span != before; + let include = replacement_patch(&header, &index_lines, start, end, after); + let exclude = replacement_patch(&header, &index_lines, start, end, before); + let rollback = replacement_patch( + &header, + &worktree_lines, + run.new_start, + run.new_end, + span, + ); + // The checkbox includes an unchecked or partial block and excludes a checked one. + let toggle_patch = if checked { + exclude.is_some() + } else { + include.is_some() + }; + let consistent = end <= index_lines.len(); + let rollback_candidate = + writable && consistent && !overlaps_boundary && !checked && rollback.is_some(); + DiffBlock { + id: run.id(), + checked, + indeterminate, + can_toggle: writable && consistent && !overlaps_boundary && toggle_patch, + can_rollback: rollback_candidate && rollback_safe, + whole_file: false, + include, + exclude, + rollback, + rollback_candidate, + } + }) + .collect(); + Some(blocks) +} + +fn size_delta(run: &ChangeRun) -> isize { + (run.new_end - run.new_start) as isize - (run.old_end - run.old_start) as isize +} + +fn offset(position: usize, delta: isize) -> usize { + position.saturating_add_signed(delta) +} + +// --------------------------------------------------------------------------- +// Repository reads and commands +// --------------------------------------------------------------------------- + +fn complete_file_request(root: &str, pathspecs: Vec) -> GitDiffRequest { + GitDiffRequest { + root: root.to_string(), + stash_reference: None, + include_file_sections: true, + omit_file_patch: false, + pathspecs, + reference: None, + empty_tree_base: false, + git_reference: None, + target_git_reference: None, + commit: None, + staged: false, + untracked: false, + worktree_snapshot: false, + context_lines: FULL_FILE_CONTEXT_LINES, + ignore_all_whitespace: false, + } +} + +/// Reads one complete-file section; prefers the section named `path`. +fn read_section( + request: GitDiffRequest, + path: &str, +) -> Result<(Option, bool), CoreError> { + let response = diff_impl(request)?; + let mut files = response.files.unwrap_or_default(); + let position = files.iter().position(|file| file.path == path).unwrap_or(0); + let file = (!files.is_empty()).then(|| files.swap_remove(position)); + Ok((file, response.has_decoding_loss)) +} + +fn safe_path(path: &str) -> bool { + is_safe_pathspec(path) && !path.contains('\0') +} + +/// Returns whether Git's effective attributes let a worktree patch round-trip: +/// a clean/smudge `filter` or a `working-tree-encoding` may rewrite bytes the +/// patch was computed from, so either one fails closed. `text` and `eol` are +/// conversions `git apply` performs itself and stay allowed. +pub(super) fn worktree_attributes_safe(root: &str, paths: &[String]) -> Result { + if paths.is_empty() || !paths.iter().all(|path| safe_path(path)) { + return Ok(false); + } + let mut arguments = vec![ + "check-attr".to_string(), + "-z".to_string(), + "filter".to_string(), + "working-tree-encoding".to_string(), + "--".to_string(), + ]; + arguments.extend(paths.iter().cloned()); + let output = execute_git_readonly(root, &arguments, None)?; + if output.exit_code != 0 { + return Ok(false); + } + let fields = output.stdout.split('\0').collect::>(); + let mut reported = 0; + for record in fields.chunks_exact(3) { + if record[1] == "filter" || record[1] == "working-tree-encoding" { + reported += 1; + if record[2] != "unspecified" && record[2] != "unset" { + return Ok(false); + } + } + } + // A missing record means the check itself is unreliable. + Ok(reported == paths.len() * 2) +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct BlockTarget { + root: String, + path: String, + #[serde(default)] + original_path: Option, + /// Review the index (HEAD -> index) instead of the worktree. + #[serde(default)] + staged: bool, +} + +/// One complete read of the files a commit review compares. +struct BlockSnapshot { + worktree: Option, + index: Option, + decoding_loss: bool, + rollback_safe: bool, + token: String, +} + +impl BlockSnapshot { + fn read(target: &BlockTarget, root: &str) -> Result { + let mut pathspecs = Vec::new(); + for path in target.original_path.iter().chain([&target.path]) { + if !safe_path(path) { + return Err(invalid("Git diff contains an invalid path")); + } + if !pathspecs.contains(path) { + pathspecs.push(path.clone()); + } + } + let mut staged = complete_file_request(root, vec![target.path.clone()]); + staged.staged = true; + let (index, index_loss) = read_section(staged, &target.path)?; + let (worktree, worktree_loss) = if target.staged { + (index.clone(), index_loss) + } else { + let mut request = complete_file_request(root, pathspecs.clone()); + request.worktree_snapshot = true; + read_section(request, &target.path)? + }; + let rollback_safe = !target.staged && worktree_attributes_safe(root, &pathspecs)?; + let mut digest = Sha256::new(); + for file in [&worktree, &index] { + match file { + Some(file) => { + digest.update(file.status.as_bytes()); + digest.update([0]); + digest.update(file.patch.as_bytes()); + } + None => digest.update(b"-"), + } + digest.update([0]); + } + let token = digest + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect(); + Ok(Self { + worktree, + index, + decoding_loss: worktree_loss || index_loss, + rollback_safe, + token, + }) + } + + fn blocks(&self) -> Vec { + let Some(worktree) = &self.worktree else { + return Vec::new(); + }; + fn side(file: &GitDiffFileResponse) -> PlanSide<'_> { + PlanSide { + patch: &file.patch, + status: &file.status, + is_binary: file.is_binary, + } + } + plan_blocks( + side(worktree), + self.index.as_ref().map(side), + !self.decoding_loss, + self.rollback_safe, + ) + .unwrap_or_default() + } +} + +/// Response section for one side: the display document without patch text. +fn section_value(file: Option, decoding_loss: bool) -> Value { + let files = file + .map(|mut file| { + file.patch = String::new(); + file.hunks = Vec::new(); + vec![file] + }) + .unwrap_or_default(); + json!({ "hasDecodingLoss": decoding_loss, "files": files }) +} + +/// Plans commit-review blocks for one file (`git.planDiffBlocks`). +pub(crate) fn plan_diff_blocks_command(payload: Value) -> Result { + let target: BlockTarget = serde_json::from_value(payload) + .map_err(|_| invalid("Invalid Git diff block request"))?; + let root = validate_root(&target.root)?; + let snapshot = BlockSnapshot::read(&target, &root)?; + let blocks = snapshot.blocks(); + let decoding_loss = snapshot.decoding_loss; + Ok(json!({ + "token": snapshot.token, + "blocks": blocks, + "worktree": section_value(snapshot.worktree, decoding_loss), + "index": section_value(snapshot.index, decoding_loss), + })) +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct ApplyBlockRequest { + #[serde(flatten)] + target: BlockTarget, + block_id: String, + /// `include`, `exclude` or `rollback`. + action: String, + /// Token returned by the `git.planDiffBlocks` result the user acted on. + token: String, +} + +/// Applies one planned block action (`git.applyDiffBlock`). The plan is rebuilt +/// under the write lease; a different token means the file, the index or the +/// attributes changed after the user saw the block, so nothing is written. +pub(crate) fn apply_diff_block_command(payload: Value) -> Result { + let request: ApplyBlockRequest = serde_json::from_value(payload) + .map_err(|_| invalid("Invalid Git diff block request"))?; + let root = validate_root(&request.target.root)?; + let _lease = RewriteLease::acquire(&root)?; + let snapshot = BlockSnapshot::read(&request.target, &root)?; + if snapshot.token != request.token { + return Err(invalid(DIFF_BLOCK_STALE)); + } + let block = snapshot + .blocks() + .into_iter() + .find(|block| block.id == request.block_id) + .ok_or_else(|| invalid(DIFF_BLOCK_STALE))?; + if block.whole_file { + return Err(invalid(DIFF_BLOCK_UNAVAILABLE)); + } + let (patch, arguments) = match request.action.as_str() { + "include" | "exclude" if !block.can_toggle => return Err(invalid(DIFF_BLOCK_UNAVAILABLE)), + "include" => (block.include, vec!["apply", "--cached"]), + "exclude" => (block.exclude, vec!["apply", "--cached"]), + "rollback" if block.rollback_candidate && !snapshot.rollback_safe => { + return Err(invalid(HUNK_ROLLBACK_UNSAFE)) + } + "rollback" if !block.can_rollback => return Err(invalid(DIFF_BLOCK_UNAVAILABLE)), + "rollback" => (block.rollback, vec!["apply"]), + _ => return Err(invalid("Unsupported Git diff block action")), + }; + let patch = patch.ok_or_else(|| invalid(DIFF_BLOCK_UNAVAILABLE))?; + run_apply(&root, arguments, patch) +} + +fn run_apply(root: &str, arguments: Vec<&str>, patch: String) -> Result { + let mut arguments = arguments + .into_iter() + .map(str::to_string) + .collect::>(); + arguments.push("--whitespace=nowarn".to_string()); + let response = command(GitCommandRequest { + root: root.to_string(), + arguments, + input: Some(patch), + })?; + serde_json::to_value(response) + .map_err(|_| CoreError::new(ErrorCode::Unknown, "Could not encode Git apply response")) +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct GutterChangeRequest { + root: String, + path: String, + /// One-based, end-exclusive index line range the editor compared. + original_start: usize, + original_end: usize, + /// One-based, end-exclusive editor line range of the same change. + modified_start: usize, + modified_end: usize, + /// Index lines the editor showed for the change, without line breaks. + original_lines: Vec, + /// Editor lines of the change, without line breaks. + modified_lines: Vec, +} + +fn displayed(lines: &[RawLine], start: usize, end: usize, expected: &[String]) -> bool { + start >= 1 + && start <= end + && end - 1 <= lines.len() + && lines[start - 1..end - 1] + .iter() + .map(RawLine::display) + .eq(expected.iter().map(String::as_str)) +} + +/// Stages one editor gutter change (`git.stageGutterChange`). The editor sends +/// what it compared; Core re-reads the index -> worktree patch under the lease, +/// confirms both sides still show those lines and stages them with the +/// original bytes, so a moved index or an unsaved edit is never staged. +pub(crate) fn stage_gutter_change_command(payload: Value) -> Result { + let request: GutterChangeRequest = serde_json::from_value(payload) + .map_err(|_| invalid("Invalid Git gutter change request"))?; + if !safe_path(&request.path) { + return Err(invalid("Git diff contains an invalid path")); + } + let root = validate_root(&request.root)?; + let _lease = RewriteLease::acquire(&root)?; + let (file, decoding_loss) = read_section( + complete_file_request(&root, vec![request.path.clone()]), + &request.path, + )?; + let file = file + .filter(|file| file.path == request.path && file.status == "M" && !file.is_binary) + .ok_or_else(|| invalid(GUTTER_CHANGE_STALE))?; + if decoding_loss { + return Err(invalid(DIFF_BLOCK_UNAVAILABLE)); + } + let document = + SingleHunkDocument::parse_complete(&file.patch).ok_or_else(|| invalid(GUTTER_CHANGE_STALE))?; + let (index_lines, worktree_lines) = (document.old(), document.new()); + if !displayed( + &index_lines, + request.original_start, + request.original_end, + &request.original_lines, + ) || !displayed( + &worktree_lines, + request.modified_start, + request.modified_end, + &request.modified_lines, + ) { + return Err(invalid(GUTTER_CHANGE_STALE)); + } + let header = document + .content_header() + .ok_or_else(|| invalid(DIFF_BLOCK_UNAVAILABLE))?; + let patch = replacement_patch( + &header, + &index_lines, + request.original_start - 1, + request.original_end - 1, + &worktree_lines[request.modified_start - 1..request.modified_end - 1], + ) + .ok_or_else(|| invalid(DIFF_BLOCK_UNAVAILABLE))?; + run_apply(&root, vec!["apply", "--cached"], patch) +} + +#[cfg(test)] +pub(crate) mod fixture_support { + //! Pure entry points the shared fixture tests exercise without a repository. + + use super::*; + + /// Plans fixture blocks and exposes the Core-owned patches for assertions. + pub(crate) fn plan( + worktree: (&str, &str), + index: Option<(&str, &str)>, + writable: bool, + rollback_safe: bool, + ) -> Option> { + let worktree = PlanSide { + patch: worktree.1, + status: worktree.0, + is_binary: false, + }; + let index = index.map(|(status, patch)| PlanSide { + patch, + status, + is_binary: false, + }); + plan_blocks(worktree, index, writable, rollback_safe).map(|blocks| { + blocks + .into_iter() + .map(|block| { + let mut value = serde_json::to_value(&block).expect("block should encode"); + value["include"] = json!(block.include); + value["exclude"] = json!(block.exclude); + value["rollback"] = json!(block.rollback); + value + }) + .collect() + }) + } + + /// Splits a complete-file section into Git-default hunks. + pub(crate) fn hunks(section: &str) -> Option> { + stageable_hunks(section) + } +} diff --git a/rust/lithe-core/src/git/mod.rs b/rust/lithe-core/src/git/mod.rs index 042e6d79f5..9c244956be 100644 --- a/rust/lithe-core/src/git/mod.rs +++ b/rust/lithe-core/src/git/mod.rs @@ -10,6 +10,7 @@ mod commit_message_context; mod commit_selection; mod commit_state; mod deleted_selection; +mod diff_blocks; pub(crate) mod workspace_commit; pub(crate) mod workspace_status; pub use commit_state::{inspect as commit_state, GitCommitGitlink, GitCommitState}; @@ -59,6 +60,11 @@ pub(crate) use changelists::changelists_command; pub(crate) use commit_message_context::commit_message_context_command; pub(crate) use commit_selection::prepare_commit_selection_command; pub(crate) use deleted_selection::reconcile_deleted_selection_command; +#[cfg(test)] +pub(crate) use diff_blocks::fixture_support as diff_blocks_fixture; +pub(crate) use diff_blocks::{ + apply_diff_block_command, plan_diff_blocks_command, stage_gutter_change_command, +}; pub use github_checkout::{ checkout as checkout_pull_request, preview as checkout_pull_request_preview, PullRequestCheckoutPreviewRequest, PullRequestCheckoutRequest, @@ -790,10 +796,6 @@ pub struct GitApplyRequest { pub patch: String, /// Patch target or validation mode, including `stage`, `unstage`, and `worktree`. pub mode: String, - /// True when the patch carries zero context lines from the byte-faithful - /// raw-slicing channel; only then may git apply bypass the context check. - #[serde(default)] - pub unidiff_zero: bool, } #[derive(Debug, Deserialize)] @@ -2055,10 +2057,22 @@ fn git_process_creation_flags() -> u32 { /// Builds a structured working-tree, staged, untracked, or commit diff. pub fn diff(request: GitDiffRequest) -> Result { let omit_file_patch = request.include_file_sections && request.omit_file_patch; + // A complete-file review prints one hunk per file. Staging that hunk would + // write the whole file, so Core splits it into Git-default hunks. + let complete_file = request.include_file_sections + && request.context_lines >= FULL_FILE_CONTEXT_LINES + && request.stash_reference.is_none(); let mut response = diff_impl(request)?; - if omit_file_patch { - if let Some(files) = &mut response.files { - for file in files { + if let Some(files) = &mut response.files { + for file in files { + if complete_file && !file.is_binary && !file.hunks.is_empty() { + // Fail closed: an unsplittable section offers no writable hunk. + file.hunks = diff_blocks::stageable_hunks(&file.patch).unwrap_or_default(); + if file.status == "R" { + attach_rename_content_patches(&file.patch, &mut file.hunks); + } + } + if omit_file_patch { file.patch = String::new(); } } @@ -2537,6 +2551,35 @@ fn structured_diff_from_output(output: GitProcessOutput) -> GitDiffResponse { } } +/// Adds a content-only patch to every hunk of a rename section, so applying +/// one hunk changes the target file's text without reversing the rename. +fn attach_rename_content_patches(section: &str, hunks: &mut [GitDiffHunkResponse]) { + let target = section + .lines() + .take_while(|line| !line.starts_with("@@")) + .find(|line| line.starts_with("+++ b/") || line.starts_with("+++ \"b/")); + let Some(target) = target else { + return; + }; + let old = target + .strip_prefix("+++ b/") + .map(|path| format!("--- a/{path}")) + .or_else(|| { + target + .strip_prefix("+++ \"b/") + .map(|path| format!("--- \"a/{path}")) + }); + let Some(old) = old else { + return; + }; + for hunk in hunks { + let marker = format!("\n{}", hunk.header); + if let Some(offset) = hunk.patch.find(&marker) { + hunk.content_patch = Some(format!("{old}\n{target}\n{}", &hunk.patch[offset + 1..])); + } + } +} + // Git owns filename decoding. Pair its ordered NUL-delimited metadata with // byte-preserving section boundaries, then parse each document once. fn structured_patch_sections( @@ -2585,29 +2628,7 @@ fn structured_patch_sections( }); let (rows, mut hunks, lines) = parse_diff(section); if status == "R" { - let target = section - .lines() - .take_while(|line| !line.starts_with("@@")) - .find(|line| line.starts_with("+++ b/") || line.starts_with("+++ \"b/")); - if let Some(target) = target { - let old = target - .strip_prefix("+++ b/") - .map(|path| format!("--- a/{path}")) - .or_else(|| { - target - .strip_prefix("+++ \"b/") - .map(|path| format!("--- \"a/{path}")) - }); - if let Some(old) = old { - for hunk in &mut hunks { - let marker = format!("\n{}", hunk.header); - if let Some(offset) = hunk.patch.find(&marker) { - hunk.content_patch = - Some(format!("{old}\n{target}\n{}", &hunk.patch[offset + 1..])); - } - } - } - } + attach_rename_content_patches(section, &mut hunks); } files.push(GitDiffFileResponse { path: identity.path, @@ -2723,18 +2744,24 @@ pub fn apply(request: GitApplyRequest) -> Result "--whitespace=nowarn".to_string(), ], "discard" => { - let mut args = vec![ + // A hunk rollback rewrites worktree bytes; a clean/smudge filter or + // working-tree encoding could change bytes the patch never saw. + let mut paths = Vec::new(); + for file in patch_exchange::ordered_patch_files(&root, &request.patch)? { + paths.extend(file.original_path); + paths.push(file.path); + } + if !diff_blocks::worktree_attributes_safe(&root, &paths)? { + return Err(CoreError::new( + ErrorCode::InvalidRequest, + diff_blocks::HUNK_ROLLBACK_UNSAFE, + )); + } + vec![ "apply".to_string(), "--reverse".to_string(), "--whitespace=nowarn".to_string(), - ]; - // Only the byte-faithful channel produces zero-context hunks; - // applying the flag universally would weaken the context safety - // check for every well-formed discard patch too. - if request.unidiff_zero { - args.push("--unidiff-zero".to_string()); - } - args + ] } // Reconstruct a saved index snapshot and its worktree in one step. // `--cached` alone would leave the worktree at HEAD, which makes a @@ -8364,6 +8391,8 @@ fn parse_diff( header: record.header, patch, content_patch: None, + line_start: None, + line_end: None, } }) .collect(); diff --git a/rust/lithe-core/src/git/shelf.rs b/rust/lithe-core/src/git/shelf.rs index 4c64b4f343..a8a81d2f11 100644 --- a/rust/lithe-core/src/git/shelf.rs +++ b/rust/lithe-core/src/git/shelf.rs @@ -215,7 +215,6 @@ fn execute(request: Request) -> Result { Layer::Working => "worktreeCheck", } .into(), - unidiff_zero: false, })?; if check.exit_code != 0 || check.operation_error.is_some() { return Err(original); diff --git a/rust/lithe-core/src/protocol/command.rs b/rust/lithe-core/src/protocol/command.rs index c386266893..b0334e15f3 100644 --- a/rust/lithe-core/src/protocol/command.rs +++ b/rust/lithe-core/src/protocol/command.rs @@ -652,6 +652,12 @@ pub enum CoreCommand { GitParsePatch, /// Applies a patch to the index or working tree (`git.apply`). GitApply, + /// Plans commit-review blocks and their Core-owned patches (`git.planDiffBlocks`). + GitPlanDiffBlocks, + /// Includes, excludes or rolls back one planned block (`git.applyDiffBlock`). + GitApplyDiffBlock, + /// Stages one editor gutter change with its original bytes (`git.stageGutterChange`). + GitStageGutterChange, /// Lists references and bounded commit history (`git.history`). GitHistory, /// Filters commit dates against explicit absolute bounds (`git.historyQuery`). @@ -1056,6 +1062,9 @@ impl CoreCommand { "git.diff" => Some(Self::GitDiff), "git.parsePatch" => Some(Self::GitParsePatch), "git.apply" => Some(Self::GitApply), + "git.planDiffBlocks" => Some(Self::GitPlanDiffBlocks), + "git.applyDiffBlock" => Some(Self::GitApplyDiffBlock), + "git.stageGutterChange" => Some(Self::GitStageGutterChange), "git.history" => Some(Self::GitHistory), "git.historyQuery" => Some(Self::GitHistoryQuery), "git.references" => Some(Self::GitReferences), diff --git a/rust/lithe-core/src/protocol/contracts.rs b/rust/lithe-core/src/protocol/contracts.rs index 880e8f82b9..43a363dc2d 100644 --- a/rust/lithe-core/src/protocol/contracts.rs +++ b/rust/lithe-core/src/protocol/contracts.rs @@ -1048,6 +1048,13 @@ pub struct GitDiffHunkResponse { /// A rename's content-only patch preserves its file identity when applied. #[serde(skip_serializing_if = "Option::is_none")] pub content_patch: Option, + /// First unified line (index into the file's `lines`) this hunk covers. + /// Present for complete-file diffs, which Core splits into Git-default hunks. + #[serde(skip_serializing_if = "Option::is_none")] + pub line_start: Option, + /// End-exclusive unified line index; paired with `line_start`. + #[serde(skip_serializing_if = "Option::is_none")] + pub line_end: Option, } #[derive(Debug, Clone, Serialize)] diff --git a/rust/lithe-core/src/runtime/dispatcher.rs b/rust/lithe-core/src/runtime/dispatcher.rs index 70f4d778e9..ca0926cce1 100644 --- a/rust/lithe-core/src/runtime/dispatcher.rs +++ b/rust/lithe-core/src/runtime/dispatcher.rs @@ -3518,6 +3518,15 @@ fn execute_parsed(mut parsed: CoreRequest) -> CoreResponse { ), Err(error) => CoreResponse::failure(id, error), }, + CoreCommand::GitPlanDiffBlocks => { + plugin_response(id, parsed.payload, crate::git::plan_diff_blocks_command) + } + CoreCommand::GitApplyDiffBlock => { + plugin_response(id, parsed.payload, crate::git::apply_diff_block_command) + } + CoreCommand::GitStageGutterChange => { + plugin_response(id, parsed.payload, crate::git::stage_gutter_change_command) + } CoreCommand::GitApply => match serde_json::from_value::(parsed.payload) .map_err(|error| { CoreError::new(ErrorCode::InvalidRequest, "Invalid Git apply request") diff --git a/rust/lithe-core/src/tests/git_diff_blocks.rs b/rust/lithe-core/src/tests/git_diff_blocks.rs new file mode 100644 index 0000000000..f4664c70cf --- /dev/null +++ b/rust/lithe-core/src/tests/git_diff_blocks.rs @@ -0,0 +1,391 @@ +//! Shared fixtures and real-Git round trips for Core-owned diff block writes. + +use super::support::temporary_root; +use crate::execute_json; +use crate::git::diff_blocks_fixture as fixture_support; +use serde_json::{json, Value}; +use std::fs; +use std::path::PathBuf; +use std::process::Command; + +fn fixture() -> Value { + serde_json::from_str(include_str!( + "../../../../shared/fixtures/git/diff-blocks-v1.json" + )) + .expect("diff block fixture should be JSON") +} + +fn side(value: &Value) -> (&str, &str) { + ( + value["status"].as_str().unwrap(), + value["patch"].as_str().unwrap(), + ) +} + +#[test] +fn diff_blocks_match_the_shared_fixture() { + for case in fixture()["blockCases"].as_array().unwrap() { + let name = case["name"].as_str().unwrap(); + let blocks = fixture_support::plan( + side(&case["worktree"]), + case["index"].is_object().then(|| side(&case["index"])), + case["writable"].as_bool().unwrap_or(true), + case["rollbackSafe"].as_bool().unwrap_or(true), + ); + let Some(expected) = case["blocks"].as_array() else { + assert!(blocks.is_none(), "{name}: {blocks:?}"); + continue; + }; + let blocks = blocks.unwrap_or_else(|| panic!("{name}: expected blocks")); + assert_eq!(blocks.len(), expected.len(), "{name}: {blocks:?}"); + for (block, expected) in blocks.iter().zip(expected) { + // Only the fields a case names are part of its contract. + for (field, value) in expected.as_object().unwrap() { + assert_eq!(&block[field], value, "{name}: {field} of {block}"); + } + if expected.get("wholeFile").is_none() { + assert!(block.get("wholeFile").is_none(), "{name}: {block}"); + } + } + } +} + +#[test] +fn complete_file_hunks_match_the_shared_fixture() { + for case in fixture()["hunkCases"].as_array().unwrap() { + let name = case["name"].as_str().unwrap(); + let hunks = fixture_support::hunks(case["patch"].as_str().unwrap()) + .map(|hunks| serde_json::to_value(hunks).unwrap()); + match case["hunks"].as_array() { + None => assert!(hunks.is_none(), "{name}: {hunks:?}"), + Some(expected) => assert_eq!(hunks.as_ref(), Some(&json!(expected)), "{name}"), + } + } +} + +/// A temporary repository with no user configuration; Drop removes it even +/// when an assertion unwinds. Every Git call is a short local command. +struct Repository(PathBuf); + +impl Repository { + fn new(label: &str) -> Self { + let root = temporary_root(label); + fs::create_dir_all(&root).unwrap(); + let repository = Self(root); + repository.git(&["init", "-q"]); + repository.git(&["config", "user.name", "Diff Blocks"]); + repository.git(&["config", "user.email", "blocks@example.invalid"]); + repository.git(&["config", "commit.gpgSign", "false"]); + repository.git(&["config", "core.autocrlf", "false"]); + repository + } + + fn git_output(&self, arguments: &[&str]) -> std::process::Output { + Command::new("git") + .args(["-c", "core.hooksPath=", "-c", "core.attributesFile="]) + .args(arguments) + .current_dir(&self.0) + .output() + .expect("git should run") + } + + fn git(&self, arguments: &[&str]) -> Vec { + let output = self.git_output(arguments); + assert!( + output.status.success(), + "git {arguments:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + output.stdout + } + + fn write(&self, path: &str, bytes: &[u8]) { + fs::write(self.0.join(path), bytes).unwrap(); + } + + fn read(&self, path: &str) -> Vec { + fs::read(self.0.join(path)).unwrap() + } + + fn commit(&self, path: &str, bytes: &[u8]) { + self.write(path, bytes); + self.git(&["add", "--", path]); + self.git(&["commit", "-qm", "base"]); + } + + fn index(&self, path: &str) -> Vec { + self.git(&["show", &format!(":{path}")]) + } + + fn request(&self, command: &str, payload: Value) -> Value { + let mut payload = payload; + payload["root"] = json!(self.0); + serde_json::from_str(&execute_json( + &json!({ "id": command, "command": command, "payload": payload }).to_string(), + )) + .unwrap() + } + + fn plan(&self, path: &str) -> Value { + let response = self.request("git.planDiffBlocks", json!({ "path": path })); + assert_eq!(response["ok"], true, "{response}"); + response["data"].clone() + } + + fn apply(&self, path: &str, plan: &Value, block: usize, action: &str) -> Value { + self.request( + "git.applyDiffBlock", + json!({ + "path": path, + "blockId": plan["blocks"][block]["id"], + "action": action, + "token": plan["token"], + }), + ) + } +} + +impl Drop for Repository { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +fn applied(response: &Value) { + assert_eq!(response["ok"], true, "{response}"); + assert_eq!(response["data"]["exitCode"], 0, "{response}"); +} + +fn failure(response: &Value) -> &str { + assert_eq!(response["ok"], false, "{response}"); + response["error"]["message"].as_str().unwrap() +} + +#[test] +fn planned_blocks_include_exclude_and_roll_back_through_real_git() { + let repository = Repository::new("diff-blocks-round-trip"); + repository.commit("file.txt", b"a\nb\nc\nd\ne\n"); + repository.write("file.txt", b"a\nnew\nb\nc\nD\ne\n"); + + let plan = repository.plan("file.txt"); + assert_eq!(plan["blocks"].as_array().unwrap().len(), 2, "{plan}"); + assert!(plan["worktree"]["files"][0]["patch"] + .as_str() + .unwrap() + .is_empty()); + applied(&repository.apply("file.txt", &plan, 0, "include")); + assert_eq!(repository.index("file.txt"), b"a\nnew\nb\nc\nd\ne\n"); + + let plan = repository.plan("file.txt"); + let checked = |plan: &Value| { + plan["blocks"] + .as_array() + .unwrap() + .iter() + .map(|block| block["checked"].as_bool().unwrap()) + .collect::>() + }; + assert_eq!(checked(&plan), [true, false]); + applied(&repository.apply("file.txt", &plan, 1, "include")); + let plan = repository.plan("file.txt"); + assert_eq!(checked(&plan), [true, true]); + applied(&repository.apply("file.txt", &plan, 0, "exclude")); + assert_eq!(repository.index("file.txt"), b"a\nb\nc\nD\ne\n"); + + // Rollback restores the index text for the block and leaves the index alone. + let plan = repository.plan("file.txt"); + assert_eq!(checked(&plan), [false, true]); + applied(&repository.apply("file.txt", &plan, 0, "rollback")); + assert_eq!(repository.read("file.txt"), b"a\nb\nc\nD\ne\n"); + assert_eq!(repository.index("file.txt"), b"a\nb\nc\nD\ne\n"); +} + +#[test] +fn a_block_write_is_refused_when_the_file_changed_after_planning() { + let repository = Repository::new("diff-blocks-stale"); + repository.commit("file.txt", b"a\nb\nc\n"); + repository.write("file.txt", b"a\nB\nc\n"); + let plan = repository.plan("file.txt"); + repository.write("file.txt", b"a\nB\nc\nlater\n"); + + for action in ["include", "rollback"] { + let response = repository.apply("file.txt", &plan, 0, action); + assert_eq!(failure(&response), "GIT_DIFF_BLOCK_STALE"); + } + assert_eq!(repository.index("file.txt"), b"a\nb\nc\n"); + assert_eq!(repository.read("file.txt"), b"a\nB\nc\nlater\n"); +} + +#[test] +fn lossy_line_endings_round_trip_byte_for_byte() { + // (label, configuration, committed bytes, edited bytes, bytes after rollback) + for (label, configure, committed, edited, restored) in [ + ( + "crlf", + vec![], + &b"one\r\ntwo\r\nthree\r\n"[..], + &b"one\r\nTWO\r\nthree\r\n"[..], + &b"one\r\ntwo\r\nthree\r\n"[..], + ), + ("crlf-only", vec![], b"one\ntwo\n", b"one\r\ntwo\n", b"one\ntwo\n"), + ("no-final-newline", vec![], b"one\ntwo", b"one\nTWO", b"one\ntwo"), + ( + // Git converts the worktree to its clean form and back, so the + // restored line keeps the worktree's CRLF convention. + "autocrlf", + vec![("core.autocrlf", "true")], + b"one\ntwo\nthree\n", + b"one\r\nTWO\r\nthree\r\n", + b"one\r\ntwo\r\nthree\r\n", + ), + ] { + let repository = Repository::new(&format!("diff-blocks-{label}")); + repository.commit("file.txt", committed); + for (key, value) in &configure { + repository.git(&["config", key, value]); + } + repository.write("file.txt", edited); + let index_before = repository.index("file.txt"); + + let plan = repository.plan("file.txt"); + let block = &plan["blocks"][0]; + assert_eq!(block["canToggle"], true, "{label}: {plan}"); + assert_eq!(block["canRollback"], true, "{label}: {plan}"); + applied(&repository.apply("file.txt", &plan, 0, "rollback")); + assert_eq!(repository.read("file.txt"), restored, "{label}"); + assert_eq!(repository.index("file.txt"), index_before, "{label}"); + + repository.write("file.txt", edited); + let plan = repository.plan("file.txt"); + applied(&repository.apply("file.txt", &plan, 0, "include")); + repository.git(&["commit", "-qm", "block"]); + let status = repository.git(&["status", "--porcelain"]); + assert!(status.is_empty(), "{label}: {}", String::from_utf8_lossy(&status)); + } +} + +#[test] +fn eol_attribute_rollback_restores_worktree_bytes() { + let repository = Repository::new("diff-blocks-eol-attribute"); + repository.commit(".gitattributes", b"*.txt text eol=crlf\n"); + repository.write("file.txt", b"one\r\ntwo\r\nthree\r\n"); + repository.git(&["add", "file.txt"]); + repository.git(&["commit", "-qm", "crlf"]); + repository.write("file.txt", b"one\r\nTWO\r\nthree\r\n"); + + let plan = repository.plan("file.txt"); + applied(&repository.apply("file.txt", &plan, 0, "rollback")); + assert_eq!(repository.read("file.txt"), b"one\r\ntwo\r\nthree\r\n"); +} + +#[test] +fn filtered_files_keep_inclusion_but_refuse_hunk_rollback() { + let repository = Repository::new("diff-blocks-filter"); + repository.git(&["config", "filter.lithe.clean", "cat"]); + repository.git(&["config", "filter.lithe.smudge", "cat"]); + repository.commit(".gitattributes", b"*.txt filter=lithe\n"); + repository.commit("file.txt", b"a\nb\nc\n"); + repository.write("file.txt", b"a\nB\nc\n"); + + let plan = repository.plan("file.txt"); + assert_eq!(plan["blocks"][0]["canToggle"], true, "{plan}"); + assert_eq!(plan["blocks"][0]["canRollback"], false, "{plan}"); + let response = repository.apply("file.txt", &plan, 0, "rollback"); + assert_eq!(failure(&response), "GIT_HUNK_ROLLBACK_UNSAFE"); + assert_eq!(repository.read("file.txt"), b"a\nB\nc\n"); + + // The ordinary hunk discard of other surfaces applies the same rule. + let diff = repository.request( + "git.diff", + json!({ "pathspecs": ["file.txt"], "includeFileSections": true }), + ); + let patch = diff["data"]["files"][0]["hunks"][0]["patch"].clone(); + let response = repository.request("git.apply", json!({ "patch": patch, "mode": "discard" })); + assert_eq!(failure(&response), "GIT_HUNK_ROLLBACK_UNSAFE"); + assert_eq!(repository.read("file.txt"), b"a\nB\nc\n"); +} + +#[test] +fn complete_file_diff_hunks_equal_git_default_hunks_and_stage_alone() { + let repository = Repository::new("diff-blocks-complete-hunks"); + let before = (1..=40).map(|line| format!("line {line}\n")).collect::(); + let mut after = before.lines().map(str::to_string).collect::>(); + after[5] = "first".into(); + after[19] = "second\r".into(); + after[33] = "third".into(); + let after = after.join("\n") + "\n"; + repository.commit("file.txt", before.as_bytes()); + repository.write("file.txt", after.as_bytes()); + + let response = repository.request( + "git.diff", + json!({ "pathspecs": ["file.txt"], "includeFileSections": true, + "contextLines": 1_073_741_823usize }), + ); + assert_eq!(response["ok"], true, "{response}"); + let file = &response["data"]["files"][0]; + let hunks = file["hunks"].as_array().unwrap(); + let default = String::from_utf8(repository.git(&["diff", "--no-color", "--", "file.txt"])) + .unwrap(); + // Default hunks carry Git's function-name hint after the range; compare + // the range and every body byte. + let default_hunks = default + .split("\n@@") + .skip(1) + .map(|hunk| { + let (header, body) = hunk.split_once('\n').unwrap(); + let range = &header[..header[1..].find(" @@").unwrap() + 4]; + (format!("@@{range}"), body.to_string()) + }) + .collect::>(); + assert_eq!(hunks.len(), default_hunks.len(), "{default}"); + for (hunk, (range, expected)) in hunks.iter().zip(&default_hunks) { + let patch = hunk["patch"].as_str().unwrap(); + let (header, body) = patch[patch.find("\n@@").unwrap() + 1..] + .split_once('\n') + .unwrap(); + assert_eq!(header, range); + assert_eq!(body.trim_end_matches('\n'), expected.trim_end_matches('\n')); + let lines = &file["lines"].as_array().unwrap()[hunk["lineStart"].as_u64().unwrap() + as usize + ..hunk["lineEnd"].as_u64().unwrap() as usize]; + assert!(lines.iter().all(|line| line["lineType"] != "header")); + } + + let response = repository.request( + "git.apply", + json!({ "patch": hunks[1]["patch"], "mode": "stage" }), + ); + applied(&response); + let mut expected = before.lines().map(str::to_string).collect::>(); + expected[19] = "second\r".into(); + assert_eq!( + repository.index("file.txt"), + (expected.join("\n") + "\n").into_bytes() + ); +} + +#[test] +fn gutter_changes_stage_original_bytes_and_refuse_moved_lines() { + let repository = Repository::new("diff-blocks-gutter"); + repository.commit("file.txt", b"one\r\ntwo\r\nthree\r\nfour\r\n"); + repository.write("file.txt", b"ONE\r\ntwo\r\nthree\r\nFOUR\r\n"); + let change = |original: &str, modified: &str, line: usize| { + json!({ + "path": "file.txt", + "originalStart": line, "originalEnd": line + 1, + "modifiedStart": line, "modifiedEnd": line + 1, + "originalLines": [original], "modifiedLines": [modified], + }) + }; + + let response = repository.request("git.stageGutterChange", change("four", "FOUR", 4)); + applied(&response); + assert_eq!(repository.index("file.txt"), b"one\r\ntwo\r\nthree\r\nFOUR\r\n"); + + // The editor still shows a line the file no longer has. + repository.write("file.txt", b"Uno\r\ntwo\r\nthree\r\nFOUR\r\n"); + let response = repository.request("git.stageGutterChange", change("one", "ONE", 1)); + assert_eq!(failure(&response), "GIT_GUTTER_CHANGE_STALE"); + assert_eq!(repository.index("file.txt"), b"one\r\ntwo\r\nthree\r\nFOUR\r\n"); +} diff --git a/rust/lithe-core/src/tests/mod.rs b/rust/lithe-core/src/tests/mod.rs index 1e5f6df077..771850a81a 100644 --- a/rust/lithe-core/src/tests/mod.rs +++ b/rust/lithe-core/src/tests/mod.rs @@ -9,6 +9,7 @@ mod declarative_plugins; mod detectors; mod file_name_order; mod git; +mod git_diff_blocks; mod git_fetch; mod git_history_rewrite; mod git_patch_exchange; diff --git a/shared/contracts/rust-core-api.md b/shared/contracts/rust-core-api.md index c58f324e5a..bb9e8faa71 100644 --- a/shared/contracts/rust-core-api.md +++ b/shared/contracts/rust-core-api.md @@ -1314,6 +1314,9 @@ package manager owns the download and Lithe does not infer bytes from logs. | `git.diff` | Produce a structured working-tree, index, reference, commit, or stash patch | | `git.parsePatch` | Parse a complete saved patch into Core-owned per-file documents | | `git.apply` | Apply or check a patch in `stage`, `unstage`, `discard`, or Shelf restore mode | +| `git.planDiffBlocks` | Plan commit-review blocks for one file; Core owns every block patch | +| `git.applyDiffBlock` | Include, exclude or roll back one planned block under the write lease | +| `git.stageGutterChange` | Stage one editor gutter change with its original bytes | | `git.history` | Return the legacy combined reference snapshot and first bounded commit page | | `git.references` | Return deterministic refs, recent local branches, ahead/behind state, and effective Git identity without scanning commit history | | `git.remotes` | Return configured fetch remote names and URLs | @@ -2284,7 +2287,33 @@ already applies, so Shelf restoration can be retried after a partial failure. It returns the normal Git process result. `restoreIndex` applies a saved index patch to both the index and worktree; `worktree` applies only to the worktree. Pathspecs must be workspace-relative and must not contain absolute -paths or `..` components. +paths or `..` components. A `discard` patch is refused with +`GIT_HUNK_ROLLBACK_UNSAFE` when Git's attributes for any touched path name a +`filter` or `working-tree-encoding`, because those can rewrite bytes the patch +never saw. + +`git.diff` with `includeFileSections` and a complete-file context splits each +file's single hunk into Git-default (3 context line) hunks. Each hunk carries +its own byte-faithful `patch` plus `lineStart` / `lineEnd` (end-exclusive +indices into the file's `lines`); a section that cannot be split yields no +hunks, so no writable hunk is offered. Hosts never build patch text. + +`git.planDiffBlocks` accepts `{root, path, originalPath?, staged?}` and returns +`{token, blocks, worktree, index}`. `worktree` is the HEAD to worktree (or, with +`staged`, HEAD to index) complete-file section and `index` is HEAD to index, +both without patch text. Each block is `{id, checked, indeterminate, canToggle, +canRollback, wholeFile?}`; `id` is `line-` into the section's unified +lines. `token` identifies the read. `git.applyDiffBlock` accepts the same +target plus `{blockId, action, token}` where `action` is `include`, `exclude` +or `rollback`. Core rebuilds the plan under the repository write lease and +refuses with `GIT_DIFF_BLOCK_STALE` when the token no longer matches, +`GIT_DIFF_BLOCK_UNAVAILABLE` for a block that cannot take the action, and +`GIT_HUNK_ROLLBACK_UNSAFE` for a rollback blocked by attributes. +`git.stageGutterChange` accepts `{root, path, originalStart, originalEnd, +modifiedStart, modifiedEnd, originalLines, modifiedLines}` (one-based, +end-exclusive) and stages exactly that change from the original bytes, or fails +with `GIT_GUTTER_CHANGE_STALE` when the index or saved file no longer shows the +compared lines. See `shared/fixtures/git/diff-blocks-v1.json`. `git.remotes` returns `{remotes: [{name, url}]}` for fetch entries. `git.tags` returns `{tags: [{name, objectId, targetId, message, date, isAnnotated}]}`; diff --git a/shared/fixtures/git/diff-blocks-v1.json b/shared/fixtures/git/diff-blocks-v1.json new file mode 100644 index 0000000000..607cf12255 --- /dev/null +++ b/shared/fixtures/git/diff-blocks-v1.json @@ -0,0 +1,309 @@ +{ + "version": 1, + "description": "Core-owned commit-review blocks (git.planDiffBlocks) and Git-default stageable hunks for complete-file diffs (git.diff). Patches are real Git output; expected write patches are what Core applies with git apply --cached (include/exclude) or git apply (rollback).", + "blockCases": [ + { + "name": "unstaged insertion and replacement stay separate blocks", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..94836e8 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n-d\n+D\n e\n" + }, + "index": null, + "blocks": [ + { + "id": "line-2", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": true, + "include": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,4 +1,5 @@\n a\n+new\n b\n c\n d\n", + "exclude": null, + "rollback": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,4 @@\n a\n-new\n b\n c\n D\n" + }, + { + "id": "line-5", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": true, + "include": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,5 @@\n a\n b\n c\n-d\n+D\n e\n", + "exclude": null, + "rollback": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -2,5 +2,5 @@\n new\n b\n c\n-D\n+d\n e\n" + } + ] + }, + { + "name": "an earlier staged insertion shifts the next block's index span", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..94836e8 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n-d\n+D\n e\n" + }, + "index": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..6d3092f 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n d\n e\n" + }, + "blocks": [ + { + "id": "line-2", + "checked": true, + "indeterminate": false, + "canToggle": true, + "canRollback": false, + "include": null, + "exclude": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,4 @@\n a\n-new\n b\n c\n d\n", + "rollback": null + }, + { + "id": "line-5", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": true, + "include": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -2,5 +2,5 @@\n new\n b\n c\n-d\n+D\n e\n", + "exclude": null, + "rollback": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -2,5 +2,5 @@\n new\n b\n c\n-D\n+d\n e\n" + } + ] + }, + { + "name": "index content that is neither HEAD nor worktree is partial", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex de98044..3b6f40a 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n a\n-b\n+B2\n c\n" + }, + "index": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex de98044..f4ea702 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n a\n-b\n+B1\n c\n" + }, + "blocks": [ + { + "id": "line-2", + "checked": false, + "indeterminate": true, + "canToggle": true, + "canRollback": true, + "include": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n a\n-B1\n+B2\n c\n", + "exclude": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n a\n-B1\n+b\n c\n", + "rollback": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n a\n-B2\n+B1\n c\n" + } + ] + }, + { + "name": "a CRLF-only edit is a writable block with its original bytes", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 422c2b7..4565b4d 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,2 +1,2 @@\n-a\n+a\r\n b\n" + }, + "index": null, + "blocks": [ + { + "id": "line-1", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": true, + "include": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,2 +1,2 @@\n-a\n+a\r\n b\n", + "exclude": null, + "rollback": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,2 +1,2 @@\n-a\r\n+a\n b\n" + } + ] + }, + { + "name": "a missing final newline is kept on both sides", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 0a207c0..817f660 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,2 +1,2 @@\n a\n-b\n\\ No newline at end of file\n+c\n\\ No newline at end of file\n" + }, + "index": null, + "blocks": [ + { + "id": "line-2", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": true, + "include": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,2 +1,2 @@\n a\n-b\n\\ No newline at end of file\n+c\n\\ No newline at end of file\n", + "exclude": null, + "rollback": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,2 +1,2 @@\n a\n-c\n\\ No newline at end of file\n+b\n\\ No newline at end of file\n" + } + ] + }, + { + "name": "a staged insertion at a replacement boundary blocks writes", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex de98044..7be73ce 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n a\n-b\n+B\n c\n" + }, + "index": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex de98044..5eff2a3 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,4 @@\n a\n+X\n b\n c\n" + }, + "blocks": [ + { + "id": "line-2", + "checked": false, + "indeterminate": true, + "canToggle": false, + "canRollback": false + } + ] + }, + { + "name": "a new file toggles as a whole", + "worktree": { + "status": "A", + "patch": "diff --git a/file.txt b/file.txt\nnew file mode 100644\nindex 0000000..814f4a4\n--- /dev/null\n+++ b/file.txt\n@@ -0,0 +1,2 @@\n+one\n+two\n" + }, + "index": null, + "blocks": [ + { + "id": "line-1", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": false, + "wholeFile": true, + "include": null, + "exclude": null, + "rollback": null + } + ] + }, + { + "name": "decoded output with replaced bytes is read-only", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..94836e8 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n-d\n+D\n e\n" + }, + "index": null, + "writable": false, + "blocks": [ + { + "id": "line-2", + "checked": false, + "indeterminate": false, + "canToggle": false, + "canRollback": false + }, + { + "id": "line-5", + "checked": false, + "indeterminate": false, + "canToggle": false, + "canRollback": false + } + ] + }, + { + "name": "unsafe worktree attributes keep inclusion but block rollback", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..94836e8 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n-d\n+D\n e\n" + }, + "index": null, + "rollbackSafe": false, + "blocks": [ + { + "id": "line-2", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": false + }, + { + "id": "line-5", + "checked": false, + "indeterminate": false, + "canToggle": true, + "canRollback": false + } + ] + }, + { + "name": "an index review only excludes", + "worktree": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..6d3092f 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n d\n e\n" + }, + "index": { + "status": "M", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..6d3092f 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n d\n e\n" + }, + "blocks": [ + { + "id": "line-2", + "checked": true, + "indeterminate": false, + "canToggle": true, + "canRollback": false, + "include": null, + "exclude": "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,4 @@\n a\n-new\n b\n c\n d\n", + "rollback": null + } + ] + }, + { + "name": "a renamed file only supports whole-file actions", + "worktree": { + "status": "R", + "patch": "diff --git a/file.txt b/file.txt\nindex 9405325..94836e8 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,5 +1,6 @@\n a\n+new\n b\n c\n-d\n+D\n e\n" + }, + "index": null, + "blocks": null + } + ], + "hunkCases": [ + { + "name": "distant changes split like git diff -U3 and nearby changes share a hunk", + "patch": "diff --git a/file.txt b/file.txt\nindex bab081f..95bcc4e 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,40 +1,40 @@\n line 1\n line 2\n line 3\n line 4\n line 5\n-line 6\n+first\n line 7\n line 8\n line 9\n line 10\n line 11\n line 12\n line 13\n line 14\n line 15\n line 16\n line 17\n line 18\n line 19\n-line 20\n+second\n line 21\n line 22\n line 23\n line 24\n line 25\n line 26\n line 27\n line 28\n line 29\n line 30\n line 31\n line 32\n line 33\n-line 34\n+third\n line 35\n line 36\n-line 37\n+fourth\n line 38\n line 39\n line 40\n", + "hunks": [ + { + "id": "hunk-0", + "header": "@@ -3,7 +3,7 @@", + "lineStart": 3, + "lineEnd": 11, + "patch": "diff --git a/file.txt b/file.txt\nindex bab081f..95bcc4e 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -3,7 +3,7 @@\n line 3\n line 4\n line 5\n-line 6\n+first\n line 7\n line 8\n line 9\n" + }, + { + "id": "hunk-1", + "header": "@@ -17,7 +17,7 @@", + "lineStart": 18, + "lineEnd": 26, + "patch": "diff --git a/file.txt b/file.txt\nindex bab081f..95bcc4e 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -17,7 +17,7 @@\n line 17\n line 18\n line 19\n-line 20\n+second\n line 21\n line 22\n line 23\n" + }, + { + "id": "hunk-2", + "header": "@@ -31,10 +31,10 @@", + "lineStart": 33, + "lineEnd": 45, + "patch": "diff --git a/file.txt b/file.txt\nindex bab081f..95bcc4e 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -31,10 +31,10 @@\n line 31\n line 32\n line 33\n-line 34\n+third\n line 35\n line 36\n-line 37\n+fourth\n line 38\n line 39\n line 40\n" + } + ] + }, + { + "name": "end-of-file markers stay attached to their lines", + "patch": "diff --git a/file.txt b/file.txt\nindex a5d34a2..3d60a4b 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,11 +1,11 @@\n-a\n+A\n b\n c\n d\n e\n f\n g\n h\n i\n j\n-k\n\\ No newline at end of file\n+K\n\\ No newline at end of file\n", + "hunks": [ + { + "id": "hunk-0", + "header": "@@ -1,4 +1,4 @@", + "lineStart": 1, + "lineEnd": 6, + "patch": "diff --git a/file.txt b/file.txt\nindex a5d34a2..3d60a4b 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -1,4 +1,4 @@\n-a\n+A\n b\n c\n d\n" + }, + { + "id": "hunk-1", + "header": "@@ -8,4 +8,4 @@", + "lineStart": 9, + "lineEnd": 14, + "patch": "diff --git a/file.txt b/file.txt\nindex a5d34a2..3d60a4b 100644\n--- a/file.txt\n+++ b/file.txt\n@@ -8,4 +8,4 @@\n h\n i\n j\n-k\n\\ No newline at end of file\n+K\n\\ No newline at end of file\n" + } + ] + }, + { + "name": "a section with two hunks is not split", + "patch": "diff --git a/f b/f\n--- a/f\n+++ b/f\n@@ -1 +1 @@\n-a\n+b\n@@ -9 +9 @@\n-c\n+d\n", + "hunks": null + } + ] +} diff --git a/shared/platform-feature-matrix/features/editor-git-gutter-stage.json b/shared/platform-feature-matrix/features/editor-git-gutter-stage.json index d41768fc7e..30f6fdddfb 100644 --- a/shared/platform-feature-matrix/features/editor-git-gutter-stage.json +++ b/shared/platform-feature-matrix/features/editor-git-gutter-stage.json @@ -16,6 +16,8 @@ "verificationStatus": "pending", "evidence": [ "windows/tauri/src/features/git/services/git-gutter-actions.ts", + "windows/tauri/src/features/git/api/git-diff-block-api.ts", + "rust/lithe-core/src/git/diff_blocks.rs", "windows/tauri/src/features/git/utils/git-gutter-changes.test.ts" ] }, diff --git a/shared/platform-feature-matrix/features/git-diff-review.json b/shared/platform-feature-matrix/features/git-diff-review.json index 6e831d88b3..30d6f6578a 100644 --- a/shared/platform-feature-matrix/features/git-diff-review.json +++ b/shared/platform-feature-matrix/features/git-diff-review.json @@ -42,7 +42,7 @@ ".agents/notes/implemented/feature/2026-10-04-windows-commit-file-diff-preview.md", "windows/tauri/src/features/git/hooks/use-commit-diff-review.ts", "windows/tauri/src/features/git/services/commit-diff-review.ts", - "windows/tauri/src/features/git/utils/commit-diff-blocks.integration.test.ts", + "rust/lithe-core/src/tests/git_diff_blocks.rs", "windows/tauri/src/features/git/components/diff/monaco-commit-block-controls.ts", "windows/tauri/src/features/tabs/utils/tab-chrome-buffer.ts", "windows/tauri/src/features/git/hooks/use-git-diff-actions.test.tsx", diff --git a/shared/platform-feature-matrix/features/git-status-commit.json b/shared/platform-feature-matrix/features/git-status-commit.json index 7e95f5e0b4..1513a81f44 100644 --- a/shared/platform-feature-matrix/features/git-status-commit.json +++ b/shared/platform-feature-matrix/features/git-status-commit.json @@ -47,7 +47,7 @@ "windows/tauri/src/features/git/hooks/use-commit-diff-review.ts", "windows/tauri/src/features/git/services/commit-diff-review.ts", "windows/tauri/src/features/git/utils/commit-diff-blocks.ts", - "windows/tauri/src/features/git/utils/commit-diff-blocks.integration.test.ts", + "rust/lithe-core/src/tests/git_diff_blocks.rs", "windows/tauri/src/features/git/hooks/use-git-commit-area-resize.test.tsx", "windows/tauri/src/features/git/components/status/git-workspace-status-panel.test.tsx", "windows/tauri/src/features/git/components/stash/git-saved-changes.tsx", diff --git a/windows/tauri/src-tauri/src/platform.rs b/windows/tauri/src-tauri/src/platform.rs index 3675e9c5ac..b05c99e897 100644 --- a/windows/tauri/src-tauri/src/platform.rs +++ b/windows/tauri/src-tauri/src/platform.rs @@ -262,6 +262,8 @@ fn is_interactive_git(core_command: &str, command: &str) -> bool { | "git.workspaceCommitStep" | "git.commit" | "git.apply" + | "git.applyDiffBlock" + | "git.stageGutterChange" | "git.patchApply" | "git.shelf" | "git.rebaseStart" @@ -802,22 +804,14 @@ fn translate(command: &str, args: Value) -> Result<(String, Value), String> { let hunk = payload .remove("hunk") .ok_or_else(|| "Hunk payload is required".to_string())?; - // A raw patch from the byte-faithful channel carries original CR - // and EOF markers and zero context; reconstructing from display - // lines would lose both. Fall back to reconstruction (which - // propagates its own error) only when no raw patch exists. - let raw_patch = hunk + // Only Core builds patch text: the hunk carries the byte-faithful + // patch Core returned with the diff, never display lines to rebuild. + let patch = hunk .get("rawPatch") .and_then(Value::as_str) - .filter(|value| !value.is_empty()); - let (patch, unidiff_zero) = match raw_patch { - Some(raw) => (raw.to_string(), true), - None => (hunk_patch(&hunk)?, false), - }; + .filter(|value| !value.is_empty()) + .ok_or_else(|| "Hunk requires a Core patch".to_string())?; payload.insert("patch".into(), json!(patch)); - if unidiff_zero { - payload.insert("unidiffZero".into(), json!(true)); - } payload.insert( "mode".into(), // `discard` reverse-applies the worktree hunk without touching @@ -843,39 +837,6 @@ fn translate(command: &str, args: Value) -> Result<(String, Value), String> { Ok((core_command.to_string(), Value::Object(payload))) } -fn hunk_patch(hunk: &Value) -> Result { - let path = hunk - .get("file_path") - .and_then(Value::as_str) - .filter(|value| !value.is_empty()) - .ok_or_else(|| "Hunk requires file_path".to_string())?; - let lines = hunk - .get("lines") - .and_then(Value::as_array) - .ok_or_else(|| "Hunk requires lines".to_string())?; - let mut patch = format!("diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n"); - for line in lines { - let kind = line - .get("line_type") - .and_then(Value::as_str) - .unwrap_or("context"); - let content = line - .get("content") - .and_then(Value::as_str) - .unwrap_or_default(); - let prefix = match kind { - "added" => "+", - "removed" => "-", - "header" => "", - _ => " ", - }; - patch.push_str(prefix); - patch.push_str(content); - patch.push('\n'); - } - Ok(patch) -} - fn move_field(payload: &mut Map, from: &str, to: &str) { if let Some(value) = payload.remove(from) { payload.insert(to.to_string(), value); @@ -2027,64 +1988,42 @@ mod tests { } #[test] - fn translates_hunk_lines_to_apply_patch() { - let (command, payload) = translate( - "git_stage_hunk", - json!({ - "repoPath": "C:/work", - "hunk": { - "file_path": "src/main.rs", - "lines": [ - { "line_type": "header", "content": "@@ -1 +1 @@" }, - { "line_type": "removed", "content": "old" }, - { "line_type": "added", "content": "new" } - ] - } - }), - ) - .unwrap(); - - assert_eq!(command, "git.apply"); - assert_eq!(payload["mode"], "stage"); - assert_eq!( - payload["patch"], - "diff --git a/src/main.rs b/src/main.rs\n--- a/src/main.rs\n+++ b/src/main.rs\n@@ -1 +1 @@\n-old\n+new\n" - ); + fn translates_core_hunk_patches_to_apply_modes() { + let patch = "diff --git a/src/main.rs b/src/main.rs\n--- a/src/main.rs\n+++ b/src/main.rs\n@@ -1,2 +1,2 @@\n keep\n-old\r\n+new\r\n"; + let hunk = json!({ "file_path": "src/main.rs", "lines": [], "rawPatch": patch }); + for (name, mode) in [ + ("git_stage_hunk", "stage"), + ("git_unstage_hunk", "unstage"), + ("git_discard_hunk", "discard"), + ] { + let (command, payload) = + translate(name, json!({ "repoPath": "C:/work", "hunk": hunk.clone() })).unwrap(); + assert_eq!(command, "git.apply"); + assert_eq!( + payload, + json!({ "root": "C:/work", "mode": mode, "patch": patch }), + "{name}" + ); + } } #[test] - fn translates_hunk_discard_to_reverse_worktree_apply() { - let hunk = json!({ + fn hunk_writes_never_rebuild_patches_from_display_lines() { + let display_only = json!({ "file_path": "src/main.rs", "lines": [ - { "line_type": "header", "content": "@@ -1,2 +1,2 @@" }, - { "line_type": "context", "content": "keep" }, + { "line_type": "header", "content": "@@ -1 +1 @@" }, { "line_type": "removed", "content": "old" }, { "line_type": "added", "content": "new" } ] }); - let (command, payload) = translate( - "git_discard_hunk", - json!({ "repoPath": "C:/work", "hunk": hunk.clone() }), - ) - .unwrap(); - - assert_eq!(command, "git.apply"); - assert_eq!( - payload, - json!({ - "root": "C:/work", - "mode": "discard", - "patch": "diff --git a/src/main.rs b/src/main.rs\n--- a/src/main.rs\n+++ b/src/main.rs\n@@ -1,2 +1,2 @@\n keep\n-old\n+new\n" - }) - ); - - // Stage and unstage keep their modes when the discard arm is shared. - for (name, mode) in [("git_stage_hunk", "stage"), ("git_unstage_hunk", "unstage")] { - let (_, payload) = - translate(name, json!({ "repoPath": "C:/work", "hunk": hunk.clone() })).unwrap(); - assert_eq!(payload["mode"], mode); + for name in ["git_stage_hunk", "git_unstage_hunk", "git_discard_hunk"] { + assert!(translate( + name, + json!({ "repoPath": "C:/work", "hunk": display_only.clone() }) + ) + .is_err()); + assert!(translate(name, json!({ "repoPath": "C:/work" })).is_err()); } - assert!(translate("git_discard_hunk", json!({ "repoPath": "C:/work" })).is_err()); } } diff --git a/windows/tauri/src/features/editor/components/monaco-editor.tsx b/windows/tauri/src/features/editor/components/monaco-editor.tsx index 0790d3c38c..7534599a29 100644 --- a/windows/tauri/src/features/editor/components/monaco-editor.tsx +++ b/windows/tauri/src/features/editor/components/monaco-editor.tsx @@ -217,10 +217,10 @@ function gitGutterControllerOptions( }), markerTooltip: (kind) => translate(`git.gutter.${kind}`), canStage: () => !!context()?.base && !!context()?.canStage, - stage: async (change, baseLines) => { + stage: async (change) => { const base = context()?.base; if (!base) return false; - const staged = await stageGitGutterChange(base, baseLines, change); + const staged = await stageGitGutterChange(base, change); if (!staged) toast.error(translate("git.operationFailed")); return staged; }, diff --git a/windows/tauri/src/features/git/api/git-diff-api.ts b/windows/tauri/src/features/git/api/git-diff-api.ts index a65bd0c318..e60eb7b9e6 100644 --- a/windows/tauri/src/features/git/api/git-diff-api.ts +++ b/windows/tauri/src/features/git/api/git-diff-api.ts @@ -4,7 +4,6 @@ import { registerGitCacheInvalidator } from "../runtime/git-cache-registry"; import { runGitRead } from "../runtime/git-read-coordinator"; import { gitDiffCache } from "../utils/git-diff-cache"; import { FULL_FILE_CONTEXT_LINES } from "../utils/git-diff-helpers"; -import { checkHunkRollbackAttributes } from "../utils/lossy-rollback-attrs"; import { isNotGitRepositoryError, resolveRepositoryForFile, @@ -245,19 +244,6 @@ export const getWorkingTreePathDiff = async ( }), ); if (!diff) return null; - // The rollback channel needs the raw patch bytes for lossy files; the - // display adapter strips CR and EOF markers. Attach writePatch the same - // way getFullContextFileDiff does, gated by the attribute safety check - // and a snapshot consistency verification. - if (diff.has_lossy_line_endings) { - const safety = await checkHunkRollbackAttributes(resolved.repoPath, resolved.filePath); - if (safety.byteSafe) { - const raw = await fetchRawWorktreePatch(resolved.repoPath, resolved.filePath); - if (raw && writePatchMatchesDisplay(raw, diff)) { - return fullContext ? markFullContext({ ...diff, writePatch: raw }) : { ...diff, writePatch: raw }; - } - } - } return fullContext ? markFullContext(diff) : diff; } catch (error) { if (!isNotGitRepositoryError(error) && !isNoDiffFoundError(error)) { @@ -307,20 +293,6 @@ export const getResolvedFullContextFileDiff = async ( }), ); if (!diff) return null; - // The rollback channel needs the raw patch bytes (CR, EOF markers) that - // the display adapter strips. Only the unstaged read (index → worktree) - // is the correct rollback basis, so only that direction carries it, and - // only when Git's effective attributes prove the bytes can round-trip - // and the display and raw reads describe the same snapshot. - if (!staged && diff.has_lossy_line_endings) { - const safety = await checkHunkRollbackAttributes(resolved.repoPath, resolved.filePath); - if (safety.byteSafe) { - const raw = await fetchRawWorktreePatch(resolved.repoPath, resolved.filePath); - if (raw && writePatchMatchesDisplay(raw, diff)) { - return markFullContext({ ...diff, writePatch: raw }); - } - } - } return markFullContext(diff); } catch (error) { if (!isNotGitRepositoryError(error) && !isNoDiffFoundError(error)) { @@ -331,107 +303,6 @@ export const getResolvedFullContextFileDiff = async ( }; -/** Fetches the unmodified index-to-worktree patch text for byte-faithful - * hunk rollback; returns null when Git cannot produce one. */ -const fetchRawWorktreePatch = async ( - repoPath: string, - filePath: string, -): Promise => { - try { - const response = await tauriInvoke<{ - patch: string; - exitCode: number; - hasDecodingLoss?: boolean; - }>("git.diff", { - root: repoPath, - pathspecs: [filePath], - contextLines: 3, - // The display adapter projects `git.diff` rows and would - // strip the very CR and EOF bytes this channel exists to preserve. - }, { rawCoreResult: true }); - if (typeof response?.patch !== "string") return null; - // Core replaced invalid UTF-8 with U+FFFD; writing this back would - // permanently corrupt the file. Also detect the replacement character - // directly as a belt-and-suspenders check for older core builds. - if (response.hasDecodingLoss || response.patch.includes("�")) return null; - return response.patch; - } catch { - return null; - } -}; - -/** - * Pre-write probe for zero-context rollback: re-reads the raw patch a lossy - * diff's `writePatch` came from, with the same repository resolution and fixed - * context, and compares it byte for byte. The display diff's context width - * (full-file or three lines) never enters the comparison, and CR-only edits the - * display strips still count. Any read failure resolves false, so the write is - * refused rather than applied to lines that may have moved. - */ -export const isWorktreeWritePatchCurrent = async ( - repoPath: string | undefined, - filePath: string, - writePatch: string | undefined, -): Promise => { - if (!repoPath || !writePatch) return false; - try { - const resolved = await resolveRepositoryForFile(repoPath, filePath); - if (!resolved) return false; - return await fetchRawWorktreePatch(resolved.repoPath, resolved.filePath) === writePatch; - } catch { - return false; - } -}; - -/** - * Verifies that a raw writePatch and a display diff describe the same - * worktree snapshot. The display read strips CR; the raw read preserves it. - * Comparing the structured entries — change type, line number, and stripped - * text — catches edits that land between the two reads, including changes - * that relocate to different lines with identical text. Mixing snapshots - * would let a rollback discard content the user never saw. - */ -const writePatchMatchesDisplay = (rawPatch: string, display: GitDiff): boolean => { - // Parse the raw patch into structured entries with line numbers. - const rawEntries: Array<{ type: string; line: number | null; text: string }> = []; - let oldLine = 0; - let newLine = 0; - let inHunk = false; - for (const line of rawPatch.split("\n")) { - if (line.startsWith("@@")) { - inHunk = true; - const match = line.match(/@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/); - if (match) { - oldLine = parseInt(match[1], 10); - newLine = parseInt(match[3], 10); - } - continue; - } - if (!inHunk || line === "" || line.startsWith("\\")) continue; - const prefix = line[0]; - const text = line.slice(1).replace(/\r$/, ""); - if (prefix === "-") { - rawEntries.push({ type: "removed", line: oldLine, text }); - } else if (prefix === "+") { - rawEntries.push({ type: "added", line: newLine, text }); - } - if (prefix === " " || prefix === "-") oldLine++; - if (prefix === " " || prefix === "+") newLine++; - } - const displayEntries = display.lines - .filter(l => l.line_type === "added" || l.line_type === "removed") - .map(l => ({ - type: l.line_type, - line: l.line_type === "removed" ? l.old_line_number : l.new_line_number, - text: l.content.replace(/\r$/, ""), - })); - return rawEntries.length === displayEntries.length - && rawEntries.every((entry, index) => { - const other = displayEntries[index]; - return entry.type === other.type && entry.line === other.line && entry.text === other.text; - }); -}; - export const getUntrackedFileDiff = async ( repoPath: string, filePath: string, diff --git a/windows/tauri/src/features/git/api/git-diff-block-api.ts b/windows/tauri/src/features/git/api/git-diff-block-api.ts new file mode 100644 index 0000000000..efe7456f41 --- /dev/null +++ b/windows/tauri/src/features/git/api/git-diff-block-api.ts @@ -0,0 +1,133 @@ +import { invoke as tauriInvoke } from "@/platform/tauri-core"; +import { emitGitChanged } from "../events/git-events"; +import type { GitDiff } from "../types/git.types"; +import { commitDiffChangeID } from "../utils/commit-diff-blocks"; +import { planIndependentCommitDiff } from "../utils/independent-commit-diff"; +import { monacoDiffRows } from "../utils/monaco-diff-rows"; + +/** One Core-planned block. Hosts address it by `id`; Core owns its patches. */ +export interface CommitDiffBlock { + /** `line-` into the compared file's unified lines. */ + id: string; + checked: boolean; + indeterminate: boolean; + canToggle: boolean; + canRollback: boolean; + /** New and deleted files toggle as a whole through ordinary staging. */ + wholeFile?: boolean; + /** Unfolded model indices; presentation maps the stable row ID after folding. */ + leftStart: number; + rightStart: number; +} + +/** Core's block as sent over the wire, before display positions are attached. */ +type PlannedBlock = Omit; + +/** One file a commit review compares, as Core resolves it. */ +export interface CommitDiffBlockTarget { + repoPath: string; + /** Repository-relative path. */ + filePath: string; + originalPath?: string; + /** Review HEAD -> index instead of HEAD -> worktree. */ + staged?: boolean; +} + +/** Core's commit-review read: both comparisons, their blocks and the snapshot token. */ +export interface CommitDiffBlockPlan { + /** HEAD -> worktree (or HEAD -> index for an index review), complete-file. */ + diff: GitDiff; + /** HEAD -> index, complete-file; no lines when the index matches HEAD. */ + staged: GitDiff; + blocks: CommitDiffBlock[]; + /** Identifies the read; a write planned from another read is refused. */ + token: string; +} + +export type CommitDiffBlockAction = "include" | "exclude" | "rollback"; +export type CommitDiffBlockWriteResult = "applied" | "stale" | "unsafe" | "failed"; + +const payload = (target: CommitDiffBlockTarget) => ({ + root: target.repoPath, + path: target.filePath, + ...(target.originalPath ? { originalPath: target.originalPath } : {}), + ...(target.staged ? { staged: true } : {}), +}); + +const errorMessage = (error: unknown) => (error instanceof Error ? error.message : String(error)); + +/** Reads one commit review. Core reads Git, plans blocks and owns every patch. */ +export async function planCommitDiffBlocks(target: CommitDiffBlockTarget): Promise { + const plan = await tauriInvoke & { blocks: PlannedBlock[] }>( + "git.planDiffBlocks", payload(target)); + // Every line of the file is present as context, so reviews fold locally. + const diff = { ...plan.diff, is_full_context: true }; + // Locate each Core block in the display rows; the block's patches stay in Core. + const display = planIndependentCommitDiff(monacoDiffRows(diff, { hideHunkHeaders: true }), false, new Set()); + const positions = new Map(display.changes.map(change => + [commitDiffChangeID(display, change), change] as const)); + const blocks = plan.blocks.flatMap((block): CommitDiffBlock[] => { + const change = positions.get(block.id); + return change ? [{ ...block, leftStart: change.leftStart, rightStart: change.rightStart }] : []; + }); + return { ...plan, diff, staged: { ...plan.staged, is_full_context: true }, blocks }; +} + +/** + * Applies one planned block action. Core re-reads the file under its write + * lease and refuses the write when the token no longer describes it. + */ +export async function applyCommitDiffBlock( + target: CommitDiffBlockTarget, + blockId: string, + action: CommitDiffBlockAction, + token: string, +): Promise { + try { + await tauriInvoke("git.applyDiffBlock", { ...payload(target), blockId, action, token }); + } catch (error) { + const message = errorMessage(error); + if (message.includes("GIT_DIFF_BLOCK_STALE")) return "stale"; + if (message.includes("GIT_HUNK_ROLLBACK_UNSAFE")) return "unsafe"; + console.error("Failed to write Git diff block:", error); + return "failed"; + } + emitGitChanged({ + repoPath: target.repoPath, + filePath: target.filePath, + scopes: ["working-tree"], + source: action === "rollback" ? "discard-hunk" : action === "include" ? "stage-hunk" : "unstage-hunk", + }); + return "applied"; +} + +/** One editor gutter change, in Monaco's one-based, end-exclusive ranges. */ +export interface GitGutterChangeRequest { + repoPath: string; + filePath: string; + originalStart: number; + originalEnd: number; + modifiedStart: number; + modifiedEnd: number; + originalLines: readonly string[]; + modifiedLines: readonly string[]; +} + +/** + * Stages exactly one editor change. Core confirms the index and the saved file + * still show the compared lines and stages them with their original bytes. + */ +export async function stageGitGutterChangeInCore( + change: GitGutterChangeRequest, +): Promise<"applied" | "stale" | "failed"> { + const { repoPath, filePath, ...range } = change; + try { + await tauriInvoke("git.stageGutterChange", { root: repoPath, path: filePath, ...range }); + } catch (error) { + if (errorMessage(error).includes("GIT_GUTTER_CHANGE_STALE")) return "stale"; + console.error("Failed to stage Git gutter change:", error); + return "failed"; + } + emitGitChanged({ repoPath, filePath, scopes: ["working-tree"], source: "stage-hunk" }); + return "applied"; +} diff --git a/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx b/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx index 5b2681c5e1..ab2cffdfc3 100644 --- a/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx +++ b/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx @@ -16,7 +16,6 @@ import { useFileSystemStore } from "@/features/file-system/stores/file-system.st import { useTranslation } from "@/i18n/locale-provider"; import { joinPath } from "@/utils/path-helpers"; import { useSettingsStore } from "@/features/settings/stores/settings.store"; -import { isWorktreeWritePatchCurrent } from "@/features/git/api/git-diff-api"; import { showConfirmDialog } from "@/ui/dialog"; import { discardHunk, stageHunk, unstageHunk } from "../../api/git-status-api"; import { @@ -359,10 +358,7 @@ export default function MonacoGitDiff({ useEffect(() => { const owner = createMonacoDiffHunkActions(diff, repoPath ? { repoPath, isStaged, canDiscard } : undefined, - { stage: stageHunk, unstage: unstageHunk, discard: discardHunk, confirmDiscard: confirmDiscardRef.current, - // Zero-context hunks are sliced from `writePatch`; confirm that raw - // patch, not the display diff, is still current before writing. - validate: () => isWorktreeWritePatchCurrent(repoPath, sourcePath, diff.writePatch) }); + { stage: stageHunk, unstage: unstageHunk, discard: discardHunk, confirmDiscard: confirmDiscardRef.current }); hunkActions.current = owner; setActionError(null); return () => { diff --git a/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx b/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx index 61621a2af8..c681343716 100644 --- a/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx +++ b/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx @@ -9,6 +9,7 @@ import { useSettingsStore } from "@/features/settings/stores/settings.store"; import * as dialog from "@/ui/dialog"; import * as statusApi from "../api/git-status-api"; import * as diffApi from "../api/git-diff-api"; +import * as blockApi from "../api/git-diff-block-api"; import * as repoApi from "../api/git-repo-api"; import { emitGitChanged } from "../events/git-events"; import { createSingleFileWorkingTreeDiff } from "../utils/working-tree-multi-diff"; @@ -22,6 +23,31 @@ const diff = fullDiff(head, work); let current = createSingleFileWorkingTreeDiff({ repoPath: target.repoPath, fileKey, diff, target, commitPreview: true }); let indexed = head; +// A fake Core: it owns the worktree, the index, the planned blocks and the token. +let planWork = work, version = 0; +let planMode: "normal" | "index-only" | "binary" | "renamed" = "normal"; +const blockIDs: Record = { 1: "line-2", 3: "line-5" }; +const fakePlan = async (request: blockApi.CommitDiffBlockTarget): Promise => { + const index = fullDiff(head, indexed); + const base = request.staged ? index : fullDiff(head, planWork); + const flags = planMode === "binary" ? { is_binary: true } : planMode === "renamed" ? { is_renamed: true } : {}; + const shown = planMode === "index-only" ? { ...base, lines: [] } : { ...base, ...flags }; + const blocks = planMode === "normal" ? head.flatMap((_, at) => planWork[at] === head[at] ? [] : [{ + id: blockIDs[at], checked: indexed[at] === planWork[at], indeterminate: false, canToggle: true, + canRollback: indexed[at] !== planWork[at], leftStart: 0, rightStart: 0 }]) : []; + return { diff: shown, staged: index, blocks, token: `t${version}` }; +}; +const fakeApply = async (request: blockApi.CommitDiffBlockTarget, blockID: string, + action: blockApi.CommitDiffBlockAction, token: string): Promise => { + if (token !== `t${version}`) return "stale"; + const at = blockID === blockIDs[1] ? 1 : 3; + if (action === "include") indexed = indexed.map((value, k) => k === at ? planWork[k] : value); + else if (action === "exclude") indexed = indexed.map((value, k) => k === at ? head[k] : value); + else planWork = planWork.map((value, k) => k === at ? indexed[k] : value); + version++; + emitGitChanged({ repoPath: request.repoPath, filePath: request.filePath, source: "stage-hunk" }); + return "applied"; +}; let review: ReturnType; let restore: () => void, root: Root, container: HTMLElement; let stage: ReturnType, status: ReturnType, workingDiff: ReturnType; @@ -44,6 +70,7 @@ beforeEach(() => { document.body.append(container); root = createRoot(container); indexed = head; + planWork = work; version = 0; planMode = "normal"; current = createSingleFileWorkingTreeDiff({ repoPath: target.repoPath, fileKey, diff, target, commitPreview: true }); const state = useBufferStore.getState(); spies.push(spyOn(useBufferStore, "getState").mockImplementation(() => ({ ...state, @@ -53,11 +80,8 @@ beforeEach(() => { } as ReturnType))); status = spyOn(statusApi, "getGitStatus").mockImplementation(async () => ({ branch: "main", ahead: 0, behind: 0, files: [{ path: "file.txt", status: "modified", staged: indexed !== head, worktree: true }] })); - stage = spyOn(statusApi, "stageHunk").mockImplementation(async () => { - indexed = ["a", "B", "c", "d"]; - emitGitChanged({ repoPath: target.repoPath, filePath: target.filePath, source: "stage-hunk" }); - return true; - }); + stage = spyOn(blockApi, "applyCommitDiffBlock").mockImplementation(fakeApply); + spies.push(spyOn(blockApi, "planCommitDiffBlocks").mockImplementation(fakePlan)); workingDiff = spyOn(diffApi, "getWorkingTreePathDiff").mockResolvedValue(diff); spies.push(status, stage, workingDiff, spyOn(diffApi, "getFullContextFileDiff").mockImplementation(async () => fullDiff(head, indexed))); @@ -80,7 +104,8 @@ test("Commit preview uses the owning repository, writes one block, retains other expect(review.blocks).toHaveLength(2); await act(async () => { await review.toggle(review.blocks[0].id, true); }); expect(stage).toHaveBeenCalledTimes(1); - expect(stage.mock.calls[0][0]).toBe(target.repoPath); + expect(stage.mock.calls[0][0].repoPath).toBe(target.repoPath); + expect(stage.mock.calls[0].slice(1)).toEqual(["line-2", "include", "t0"]); expect(review.blocks.map(block => block.checked)).toEqual([true, false]); expect(current.files[0].lines).toEqual(diff.lines); expect(current.workingTreeTargets![fileKey].hasStagedChanges).toBe(true); @@ -90,11 +115,8 @@ test("Commit preview uses the owning repository, writes one block, retains other test("block rollback confirms once when discard confirmation is enabled and retains included changes", async () => { const settings = useSettingsStore.getState(); const confirm = spyOn(dialog, "showConfirmDialog").mockResolvedValue(true); - const rollback = spyOn(statusApi, "discardHunk").mockImplementation(async () => { - workingDiff.mockResolvedValue(fullDiff(head, indexed)); - return true; - }); - spies.push(confirm, rollback, spyOn(useSettingsStore, "getState").mockReturnValue({ + const rollback = stage; + spies.push(confirm, spyOn(useSettingsStore, "getState").mockReturnValue({ ...settings, settings: { ...settings.settings, confirmBeforeDiscard: true }, })); await act(async () => { root.render(); }); @@ -104,9 +126,10 @@ test("block rollback confirms once when discard confirmation is enabled and reta await act(async () => { result = await review.rollback(unselected); }); expect(result).toBe("applied"); expect(confirm).toHaveBeenCalledTimes(1); - expect(rollback).toHaveBeenCalledTimes(1); - expect(rollback.mock.calls[0][0]).toBe(target.repoPath); - expect(rollback.mock.calls[0][1].file_path).toBe(target.filePath); + expect(rollback).toHaveBeenCalledTimes(2); + expect(rollback.mock.calls[1][0].repoPath).toBe(target.repoPath); + expect(rollback.mock.calls[1][0].filePath).toBe(target.filePath); + expect(rollback.mock.calls[1][2]).toBe("rollback"); expect(review.blocks.map(block => block.checked)).toEqual([true]); expect(indexed).toEqual(["a", "B", "c", "d"]); expect(commitDiffWritePending()).toBe(false); @@ -115,8 +138,8 @@ test("block rollback confirms once when discard confirmation is enabled and reta test("block rollback cancellation skips the discard entirely", async () => { const settings = useSettingsStore.getState(); const confirm = spyOn(dialog, "showConfirmDialog").mockResolvedValue(false); - const rollback = spyOn(statusApi, "discardHunk"); - spies.push(confirm, rollback, spyOn(useSettingsStore, "getState").mockReturnValue({ + const rollback = stage; + spies.push(confirm, spyOn(useSettingsStore, "getState").mockReturnValue({ ...settings, settings: { ...settings.settings, confirmBeforeDiscard: true }, })); await act(async () => { root.render(); }); @@ -130,11 +153,8 @@ test("block rollback cancellation skips the discard entirely", async () => { test("block rollback applies immediately when discard confirmation is disabled", async () => { const settings = useSettingsStore.getState(); const confirm = spyOn(dialog, "showConfirmDialog"); - const rollback = spyOn(statusApi, "discardHunk").mockImplementation(async () => { - workingDiff.mockResolvedValue(fullDiff(head, indexed)); - return true; - }); - spies.push(confirm, rollback, spyOn(useSettingsStore, "getState").mockReturnValue({ + const rollback = stage; + spies.push(confirm, spyOn(useSettingsStore, "getState").mockReturnValue({ ...settings, settings: { ...settings.settings, confirmBeforeDiscard: false }, })); await act(async () => { root.render(); }); @@ -151,8 +171,8 @@ test("a confirmed rollback does not survive a snapshot refresh during the dialog const confirm = spyOn(dialog, "showConfirmDialog").mockImplementation( () => new Promise((resolve) => { releaseDialog = resolve; }), ); - const rollback = spyOn(statusApi, "discardHunk"); - spies.push(confirm, rollback, spyOn(useSettingsStore, "getState").mockReturnValue({ + const rollback = stage; + spies.push(confirm, spyOn(useSettingsStore, "getState").mockReturnValue({ ...settings, settings: { ...settings.settings, confirmBeforeDiscard: true }, })); await act(async () => { root.render(); }); @@ -177,8 +197,8 @@ test("a confirmation from the previous owner does not publish stale error onto t const confirm = spyOn(dialog, "showConfirmDialog").mockImplementation( () => new Promise((resolve) => { releaseDialog = resolve; }), ); - const rollback = spyOn(statusApi, "discardHunk"); - spies.push(confirm, rollback, spyOn(useSettingsStore, "getState").mockReturnValue({ + const rollback = stage; + spies.push(confirm, spyOn(useSettingsStore, "getState").mockReturnValue({ ...settings, settings: { ...settings.settings, confirmBeforeDiscard: true }, })); const nextKey = "unstaged:next.txt"; @@ -212,11 +232,8 @@ test("repeat clicks while the confirmation is open stay on one dialog", async () const confirm = spyOn(dialog, "showConfirmDialog").mockImplementation( () => new Promise((resolve) => { releaseDialog = resolve; }), ); - const rollback = spyOn(statusApi, "discardHunk").mockImplementation(async () => { - workingDiff.mockResolvedValue(fullDiff(head, indexed)); - return true; - }); - spies.push(confirm, rollback, spyOn(useSettingsStore, "getState").mockReturnValue({ + const rollback = stage; + spies.push(confirm, spyOn(useSettingsStore, "getState").mockReturnValue({ ...settings, settings: { ...settings.settings, confirmBeforeDiscard: true }, })); await act(async () => { root.render(); }); @@ -244,7 +261,7 @@ test("automatic refresh ignores events belonging to a different repository", asy test("index-only changes retain a file, index title and whole-file inclusion without inventing worktree blocks", async () => { indexed = ["a", "B", "c", "d"]; - workingDiff.mockResolvedValue({ ...diff, lines: [] }); + planMode = "index-only"; const include = spyOn(statusApi, "setFilesStaged").mockImplementation(async () => { indexed = head; return true; }); spies.push(include); await act(async () => { root.render(); }); @@ -258,10 +275,9 @@ test("index-only changes retain a file, index title and whole-file inclusion wit expect(stage).not.toHaveBeenCalled(); }); -test.each(["binary", "renamed", "lossy"])("%s file inclusion stays mixed with staged and worktree changes", async kind => { +test.each(["binary", "renamed"] as const)("%s file inclusion stays mixed with staged and worktree changes", async kind => { indexed = ["a", "B", "c", "d"]; - workingDiff.mockResolvedValue({ ...diff, ...(kind === "binary" ? { is_binary: true } - : kind === "renamed" ? { is_renamed: true } : { has_lossy_line_endings: true }) }); + planMode = kind; const include = spyOn(statusApi, "setFilesStaged").mockResolvedValue(true); spies.push(include); await act(async () => { root.render(); }); @@ -372,15 +388,15 @@ test("acceptance: refresh and confirm in the same batch must not roll back the r const confirm = spyOn(dialog, "showConfirmDialog").mockImplementation( () => new Promise((resolve) => { releaseDialog = resolve; }), ); - const rollback = spyOn(statusApi, "discardHunk").mockResolvedValue(true); - spies.push(confirm, rollback, spyOn(useSettingsStore, "getState").mockReturnValue({ + const rollback = stage; + spies.push(confirm, spyOn(useSettingsStore, "getState").mockReturnValue({ ...settings, settings: { ...settings.settings, confirmBeforeDiscard: true }, })); await act(async () => { root.render(); }); const pending = review.rollback(review.blocks[0].id); let result: unknown; try { - workingDiff.mockResolvedValue(fullDiff(head, ["a", "NEW-UNCONFIRMED", "c", "D"])); + planWork = ["a", "NEW-UNCONFIRMED", "c", "D"]; version++; await act(async () => { await review.refresh(); releaseDialog?.(true); diff --git a/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts b/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts index eea9059145..cdf5a90160 100644 --- a/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts +++ b/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts @@ -3,8 +3,9 @@ import equal from "fast-deep-equal"; import { useBufferStore } from "@/features/editor/stores/buffer.store"; import { getBufferById } from "@/features/editor/utils/buffer-index"; import { getFullContextFileDiff, getWorkingTreePathDiff } from "../api/git-diff-api"; +import { applyCommitDiffBlock, planCommitDiffBlocks } from "../api/git-diff-block-api"; import { resolveRepositoryForFile } from "../api/git-repo-api"; -import { discardHunk, getGitStatus, setFilesStaged, stageHunk, unstageHunk } from "../api/git-status-api"; +import { getGitStatus, setFilesStaged } from "../api/git-status-api"; import { isGitChangeRelevant, subscribeToGitChanges } from "../events/git-events"; import { commitDiffPresentation, createCommitDiffReview, type CommitDiffReviewState } from "../services/commit-diff-review"; import { beginCommitDiffWrite, commitDiffWritePending } from "../runtime/commit-diff-write-state"; @@ -91,27 +92,17 @@ export function useCommitDiffReview(bufferId: string | null, fileKey: string, return null; } statusFile = file; - const [diff, indexed] = await Promise.all([ - stagedView ? getFullContextFileDiff(repoPath, filePath, true) - : getWorkingTreePathDiff(repoPath, filePath, file.status === "untracked", file.originalPath, true), - file.staged ? getFullContextFileDiff(repoPath, filePath, true) : Promise.resolve(null), - ]); - if (!diff || (file.staged && !indexed)) throw new Error("Git diff unavailable"); - // The rollback channel needs the index→worktree raw patch for lossy - // files; the display diff (HEAD→worktree) cannot carry it. Read it - // alongside so the block computation receives writePatch. - if (!stagedView && diff.has_lossy_line_endings) { - const faithful = await getFullContextFileDiff(repoPath, filePath, false); - if (faithful?.writePatch) { - return { diff: { ...diff, writePatch: faithful.writePatch }, - staged: indexed ?? { ...diff, lines: [] }, file }; - } - } - return { diff, staged: indexed ?? { ...diff, lines: [] }, file }; + // Core reads both comparisons, plans the blocks and keeps their patches. + const target = { repoPath, filePath, ...(file.originalPath ? { originalPath: file.originalPath } : {}), + ...(stagedView ? { staged: true } : {}) }; + const plan = await planCommitDiffBlocks(target); + return { diff: plan.diff, staged: plan.staged, file, blocks: plan.blocks, token: plan.token }; }, - stage: hunk => current() ? stageHunk(repoPath, hunk) : Promise.resolve(false), - unstage: hunk => current() ? unstageHunk(repoPath, hunk) : Promise.resolve(false), - rollback: hunk => current() ? discardHunk(repoPath, hunk) : Promise.resolve(false), + writeBlock: (snapshot, blockId, action) => current() + ? applyCommitDiffBlock({ repoPath, filePath, + ...(snapshot.file.originalPath ? { originalPath: snapshot.file.originalPath } : {}), + ...(stagedView ? { staged: true } : {}) }, blockId, action, snapshot.token) + : Promise.resolve("failed"), includeFile: included => current() && statusFile ? setFilesStaged(repoPath, resolveGitFileMutationPaths([statusFile]), included) : Promise.resolve(false), changed: next => { diff --git a/windows/tauri/src/features/git/services/commit-diff-review.test.ts b/windows/tauri/src/features/git/services/commit-diff-review.test.ts index ca208d5c27..c74935b21b 100644 --- a/windows/tauri/src/features/git/services/commit-diff-review.test.ts +++ b/windows/tauri/src/features/git/services/commit-diff-review.test.ts @@ -1,18 +1,24 @@ import { expect, test } from "bun:test"; import { fullDiff } from "@/test-utils/commit-diff-fixtures"; +import type { CommitDiffBlock } from "../api/git-diff-block-api"; import { createCommitDiffReview, type CommitDiffSnapshot, type CommitDiffReviewState } from "./commit-diff-review"; -function snapshot(index = ["a", "b", "c", "d"]) : CommitDiffSnapshot { +const block = (id: string, checked: boolean): CommitDiffBlock => ({ + id, checked, indeterminate: false, canToggle: true, canRollback: !checked, leftStart: 0, rightStart: 0, +}); +/** Core owns the blocks; the token names the read each block was planned from. */ +function snapshot(included = false, token = "t0"): CommitDiffSnapshot { return { diff: fullDiff(["a", "b", "c", "d"], ["a", "B", "c", "D"]), - staged: fullDiff(["a", "b", "c", "d"], index), - file: { path: "file.txt", status: "modified", staged: index[1] === "B", worktree: true } }; + staged: fullDiff(["a", "b", "c", "d"], ["a", "b", "c", "d"]), + file: { path: "file.txt", status: "modified", staged: included, worktree: true }, + blocks: [block("line-1", included), block("line-3", false)], token }; } function deferred() { let resolve!: (value: T) => void; const promise = new Promise(settle => { resolve = settle; }); return { promise, resolve }; } -function fixture() { +function fixture(outcome: "applied" | "stale" | "unsafe" | "failed" = "applied") { let current = snapshot(); let state!: CommitDiffReviewState; const writes: string[] = []; @@ -20,9 +26,11 @@ function fixture() { let released = 0; const owner = createCommitDiffReview({ read: () => read(), - stage: async () => { writes.push("stage"); current = snapshot(["a", "B", "c", "d"]); return true; }, - unstage: async () => { writes.push("unstage"); current = snapshot(); return true; }, - rollback: async () => { writes.push("rollback"); return true; }, + writeBlock: async (taken, id, action) => { + writes.push(`${action}:${id}:${taken.token}`); + if (outcome === "applied") current = snapshot(action === "include", "t1"); + return outcome; + }, includeFile: async () => { writes.push("file"); return true; }, changed: next => { state = next; }, beginWrite: () => () => { released++; }, }); @@ -31,50 +39,69 @@ function fixture() { get state() { return state; }, get released() { return released; } }; } -test("inclusion writes one block and republishes its actual index state", async () => { +test("a block write names the planned block and snapshot token, then republishes Core's state", async () => { const f = fixture(); try { await f.owner.refresh(); - const id = f.state.blocks[0].id; - expect(await f.owner.apply(id, "include")).toBe("applied"); - expect(f.writes).toEqual(["stage"]); - expect(f.state.blocks.map(block => block.checked)).toEqual([true, false]); - expect(await f.owner.apply(id, "exclude")).toBe("applied"); - expect(f.state.blocks.map(block => block.checked)).toEqual([false, false]); + expect(await f.owner.apply("line-1", "include")).toBe("applied"); + expect(f.writes).toEqual(["include:line-1:t0"]); + expect(f.state.blocks.map(item => item.checked)).toEqual([true, false]); + expect(await f.owner.apply("line-1", "exclude")).toBe("applied"); + expect(f.writes[1]).toBe("exclude:line-1:t1"); + expect(f.state.blocks.map(item => item.checked)).toEqual([false, false]); expect(f.released).toBe(2); } finally { f.owner.dispose(); } }); -test("a changed snapshot during rollback validation refreshes without applying an old patch", async () => { - const f = fixture(), gate = deferred(); - let action: Promise | undefined; +test("a write Core refuses as stale surfaces the stale notice and refreshes", async () => { + const f = fixture("stale"); try { await f.owner.refresh(); - f.setRead(() => gate.promise); - action = f.owner.apply(f.state.blocks[0].id, "rollback"); - const updated = snapshot(["a", "B", "c", "d"]); - f.setSnapshot(updated); - gate.resolve(updated); - expect(await action).toBe("stale"); - expect(f.writes).toEqual([]); - expect(f.state.blocks[0].checked).toBe(true); - } finally { gate.resolve(null); await action; f.owner.dispose(); } + expect(await f.owner.apply("line-1", "rollback")).toBe("stale"); + expect(f.state.error).toBe("stale"); + expect(f.state.busy).toBe(false); + expect(f.released).toBe(1); + } finally { f.owner.dispose(); } }); -test("disposal during rollback validation suppresses writes and still releases the commit guard", async () => { - const f = fixture(), gate = deferred(); - let action: Promise | undefined; +test("an unsafe or failed Core write surfaces a write error", async () => { + for (const outcome of ["unsafe", "failed"] as const) { + const f = fixture(outcome); + try { + await f.owner.refresh(); + expect(await f.owner.apply("line-1", "rollback")).toBe("failed"); + expect(f.state.error).toBe("write"); + } finally { f.owner.dispose(); } + } +}); + +test("blocks Core cannot toggle are never sent", async () => { + const f = fixture(); try { + f.setSnapshot({ ...snapshot(), blocks: [{ ...block("line-1", false), canToggle: false, canRollback: false }] }); await f.owner.refresh(); - f.setRead(() => gate.promise); - action = f.owner.apply(f.state.blocks[0].id, "rollback"); - expect(await f.owner.apply(f.state.blocks[1].id, "include")).toBe("ignored"); - f.owner.dispose(); - gate.resolve(snapshot()); - expect(await action).toBe("ignored"); + expect(await f.owner.apply("line-1", "include")).toBe("ignored"); + expect(await f.owner.apply("line-1", "rollback")).toBe("ignored"); + expect(await f.owner.apply("line-9", "include")).toBe("ignored"); expect(f.writes).toEqual([]); - expect(f.released).toBe(1); - } finally { gate.resolve(null); await action; f.owner.dispose(); } + } finally { f.owner.dispose(); } +}); + +test("disposal during a write still releases the commit guard", async () => { + const gate = deferred<"applied">(); + let released = 0, state!: CommitDiffReviewState; + const owner = createCommitDiffReview({ + read: async () => snapshot(), writeBlock: () => gate.promise, includeFile: async () => false, + beginWrite: () => () => { released++; }, changed: next => { state = next; }, + }); + await owner.refresh(); + const action = owner.apply("line-1", "include"); + expect(await owner.apply("line-3", "include")).toBe("ignored"); + owner.dispose(); + gate.resolve("applied"); + await action; + expect(released).toBe(1); + expect(state.snapshot).not.toBeNull(); }); test("a Git event arriving during a refresh triggers a new read instead of publishing a stale result", async () => { @@ -82,7 +109,7 @@ test("a Git event arriving during a refresh triggers a new read instead of publi let pending: Promise | undefined; let reads = 0; try { - f.setRead(() => ++reads === 1 ? gate.promise : Promise.resolve(snapshot(["a", "B", "c", "d"]))); + f.setRead(() => ++reads === 1 ? gate.promise : Promise.resolve(snapshot(true, "t1"))); pending = f.owner.refresh(); const joined = f.owner.refresh(); gate.resolve(snapshot()); @@ -104,23 +131,15 @@ test("read failures retain the existing diff and surface a refresh error", async } finally { f.owner.dispose(); } }); -test("entering commit review during a pending validation read prevents the block write", async () => { - const gate = deferred(); - const current = snapshot(); - let writable = true, reads = 0, writes = 0, state!: CommitDiffReviewState; +test("leaving commit review mid-write prevents the block write", async () => { + let writable = true, writes = 0, state!: CommitDiffReviewState; const owner = createCommitDiffReview({ - read: () => ++reads === 2 ? gate.promise : Promise.resolve(current), - stage: async () => { writes++; return true; }, unstage: async () => false, - rollback: async () => false, includeFile: async () => false, - canWrite: () => writable, changed: next => { state = next; }, + read: async () => snapshot(), writeBlock: async () => { writes++; return "applied"; }, + includeFile: async () => false, canWrite: () => writable, changed: next => { state = next; }, }); - let action: Promise | undefined; - try { - await owner.refresh(); - action = owner.apply(state.blocks[0].id, "include"); - writable = false; - gate.resolve(current); - expect(await action).toBe("ignored"); - expect(writes).toBe(0); - } finally { gate.resolve(current); await action; owner.dispose(); } + await owner.refresh(); + writable = false; + expect(await owner.apply(state.blocks[0].id, "include")).toBe("ignored"); + expect(writes).toBe(0); + owner.dispose(); }); diff --git a/windows/tauri/src/features/git/services/commit-diff-review.ts b/windows/tauri/src/features/git/services/commit-diff-review.ts index 6ceac5ec76..608d439a3e 100644 --- a/windows/tauri/src/features/git/services/commit-diff-review.ts +++ b/windows/tauri/src/features/git/services/commit-diff-review.ts @@ -1,12 +1,14 @@ -import equal from "fast-deep-equal"; -import type { GitDiff, GitFile, GitHunk } from "../types/git.types"; -import type { CommitDiffBlock } from "../utils/commit-diff-blocks"; -import { planCommitDiffBlocks } from "../utils/commit-diff-blocks"; +import type { GitDiff, GitFile } from "../types/git.types"; +import type { CommitDiffBlock, CommitDiffBlockAction, CommitDiffBlockWriteResult } from "../api/git-diff-block-api"; +/** One Core read: both comparisons, the planned blocks and the token that + * identifies the read. Core refuses any write planned from another read. */ export interface CommitDiffSnapshot { diff: GitDiff; staged: GitDiff; file: GitFile; + blocks: CommitDiffBlock[]; + token: string; } export interface CommitDiffReviewState { snapshot: CommitDiffSnapshot | null; @@ -30,9 +32,9 @@ export function commitDiffPresentation(snapshot: CommitDiffSnapshot, stagedView: } interface Dependencies { read: () => Promise; - stage: (hunk: GitHunk) => Promise; - unstage: (hunk: GitHunk) => Promise; - rollback: (hunk: GitHunk) => Promise; + /** Core re-plans under its write lock and refuses a stale token. */ + writeBlock: (snapshot: CommitDiffSnapshot, blockId: string, action: CommitDiffBlockAction) + => Promise; includeFile: (included: boolean) => Promise; changed: (state: CommitDiffReviewState) => void; beginWrite?: () => () => void; @@ -41,8 +43,8 @@ interface Dependencies { } /** One live file review owns reads and block writes, including direct rollback. - * Git index state is the only source of selection. Re-read before writing; - * stale controls must never apply an old snapshot. */ + * Git index state is the only source of selection. Core validates the snapshot + * token under its write lock; stale controls never apply an old snapshot. */ export function createCommitDiffReview(dependencies: Dependencies) { let state: CommitDiffReviewState = { snapshot: null, blocks: [], busy: false }; let closed = false, generation = 0, dirty = false; @@ -53,7 +55,7 @@ export function createCommitDiffReview(dependencies: Dependencies) { dependencies.changed(state); }; const accept = (snapshot: CommitDiffSnapshot | null, error?: CommitDiffReviewState["error"]) => { - publish({ snapshot, blocks: snapshot ? planCommitDiffBlocks(snapshot.diff, snapshot.staged) ?? [] : [], + publish({ snapshot, blocks: snapshot?.blocks ?? [], busy: state.busy, ...(error ? { error } : {}) }); }; async function refresh(): Promise { @@ -83,23 +85,22 @@ export function createCommitDiffReview(dependencies: Dependencies) { || dependencies.writeAvailable?.() === false) return "ignored"; if (id !== null && (!block || (action === "rollback" ? !block.canRollback : !block.canToggle))) return "ignored"; if (id === null && action === "rollback") return "ignored"; - const hunk = action === "include" ? block?.stage : action === "exclude" ? block?.unstage : block?.rollback; - if (id !== null && !hunk && !block?.wholeFile) return "ignored"; const endWrite = dependencies.beginWrite?.(); publish({ ...state, busy: true, error: undefined }); try { if (closed || dependencies.canWrite?.() === false) return "ignored"; - const request = generation; - const fresh = await dependencies.read(); - if (closed || dependencies.canWrite?.() === false) return "ignored"; - if (request !== generation || !equal(fresh, snapshot)) { - accept(fresh, "stale"); - return "stale"; + let result: boolean; + if (id === null || block?.wholeFile) { + result = await dependencies.includeFile(action === "include"); + } else { + const outcome = await dependencies.writeBlock(snapshot, id, action); + if (outcome === "stale") { + // The file moved after the user saw the block: show what is there now. + publish({ ...state, error: "stale" }); + return "stale"; + } + result = outcome === "applied"; } - const result = id === null || block?.wholeFile - ? await dependencies.includeFile(action === "include") - : await (action === "include" ? dependencies.stage : action === "exclude" - ? dependencies.unstage : dependencies.rollback)(hunk!); if (!closed && !result) publish({ ...state, error: "write" }); return result ? "applied" : "failed"; } catch { diff --git a/windows/tauri/src/features/git/services/git-gutter-actions.ts b/windows/tauri/src/features/git/services/git-gutter-actions.ts index 00263317aa..fe2b5732fc 100644 --- a/windows/tauri/src/features/git/services/git-gutter-actions.ts +++ b/windows/tauri/src/features/git/services/git-gutter-actions.ts @@ -1,22 +1,31 @@ import { activateMainEditorPane } from "@/features/editor/stores/buffer-pane-sync"; import { useBufferStore } from "@/features/editor/stores/buffer.store"; import { getWorkingTreePathDiff } from "../api/git-diff-api"; -import { stageHunk } from "../api/git-status-api"; +import { stageGitGutterChangeInCore } from "../api/git-diff-block-api"; import type { GitGutterBase } from "../hooks/use-git-gutter-base"; -import { gitGutterChangeHunk, type GitGutterChange } from "../utils/git-gutter-changes"; +import type { GitGutterChange } from "../utils/git-gutter-changes"; import { createSingleFileWorkingTreeDiff } from "../utils/working-tree-multi-diff"; /** - * Stages one gutter change through the existing hunk API. The patch is built - * against the index text the change was computed from, so Git rejects it - * instead of staging the wrong lines if the index moved in the meantime. + * Stages one gutter change through Core. Core confirms the index and the saved + * file still show the compared lines, then builds the patch from original + * bytes; the host sends only the change's ranges and lines. */ -export function stageGitGutterChange( +export async function stageGitGutterChange( base: GitGutterBase, - baseLines: readonly string[], change: GitGutterChange, ): Promise { - return stageHunk(base.repoPath, gitGutterChangeHunk(base.filePath, baseLines, change)); + const result = await stageGitGutterChangeInCore({ + repoPath: base.repoPath, + filePath: base.filePath, + originalStart: change.originalStart, + originalEnd: change.originalEnd, + modifiedStart: change.modifiedStart, + modifiedEnd: change.modifiedEnd, + originalLines: change.originalLines, + modifiedLines: change.modifiedLines, + }); + return result === "applied"; } /** diff --git a/windows/tauri/src/features/git/types/git.types.ts b/windows/tauri/src/features/git/types/git.types.ts index 0156793d39..2e516f6706 100644 --- a/windows/tauri/src/features/git/types/git.types.ts +++ b/windows/tauri/src/features/git/types/git.types.ts @@ -164,16 +164,11 @@ export interface GitDiff { old_blob_base64?: string; new_blob_base64?: string; raw_patch?: string; - /** - * Original index-to-worktree patch bytes from the dedicated full-context - * read, preserved for byte-faithful hunk rollback. Never populated by the - * generic adapter for all diffs — only by the rollback channel. - */ - writePatch?: string; /** Display normalization lost EOF/CRLF bytes; only whole-file writes are safe. */ has_lossy_line_endings?: boolean; has_decoding_loss?: boolean; - core_hunks?: { id: string; header: string; patch: string }[]; + /** Core-split Git-default hunks; `lineStart`/`lineEnd` index `lines` for complete-file diffs. */ + core_hunks?: { id: string; header: string; patch: string; lineStart?: number; lineEnd?: number }[]; additions?: number; deletions?: number; is_truncated?: boolean; @@ -196,9 +191,8 @@ export interface GitHunk { file_path: string; lines: GitDiffLine[]; /** - * Complete pre-built patch text with original bytes (CR, EOF markers) from - * the index-to-worktree read. When present, the host applies this verbatim - * through Core's git.apply instead of reconstructing from display lines. + * Core-owned patch text with original bytes (CR, EOF markers). The host + * applies it verbatim through Core's git.apply; hosts never rebuild patches. */ rawPatch?: string; } diff --git a/windows/tauri/src/features/git/utils/commit-diff-blocks.integration.test.ts b/windows/tauri/src/features/git/utils/commit-diff-blocks.integration.test.ts deleted file mode 100644 index a1dd316850..0000000000 --- a/windows/tauri/src/features/git/utils/commit-diff-blocks.integration.test.ts +++ /dev/null @@ -1,151 +0,0 @@ -import { expect, test } from "bun:test"; -import { execFileSync } from "node:child_process"; -import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import type { GitDiff, GitHunk } from "../types/git.types"; -import { parseRawDiffContent } from "@/test-utils/git-diff-fixture-parser"; -import { planCommitDiffBlocks } from "./commit-diff-blocks"; -import { commitDiffPresentation } from "../services/commit-diff-review"; -import { FULL_FILE_CONTEXT_LINES } from "./git-diff-helpers"; -import { monacoDiffRows } from "./monaco-diff-rows"; -import { planIndependentCommitDiff } from "./independent-commit-diff"; - -// Integration lane: actual local Git, with a deadline on every subprocess and -// exclusive temporary ownership. No worktree or index from the user's repo is used. -test("real Git commits only a checked block, unchecks it independently, and rolls back without changing the index", () => { - const root = mkdtempSync(join(tmpdir(), "lithe-commit-blocks-")); - const git = (args: string[], input?: string) => execFileSync("git", ["-c", "core.autocrlf=false", - "-c", "commit.gpgsign=false", "-c", "core.hooksPath=", ...args], - { cwd: root, encoding: "utf8", timeout: 5000, input, windowsHide: true }); - const hunkPatch = (hunk: GitHunk) => `diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n` - + hunk.lines.map(line => (line.line_type === "header" ? "" : line.line_type === "added" ? "+" - : line.line_type === "removed" ? "-" : " ") + line.content + "\n").join(""); - const read = (staged: boolean): GitDiff => { - const patch = git(["diff", ...(staged ? ["--cached"] : ["HEAD"]), "--no-ext-diff", `--unified=${FULL_FILE_CONTEXT_LINES}`, "--", "file.txt"]); - return { ...parseRawDiffContent(patch, "file.txt") as GitDiff, is_full_context: true }; - }; - try { - git(["init", "--quiet"]); - git(["config", "user.name", "Diff integration"]); - git(["config", "user.email", "diff@example.invalid"]); - writeFileSync(join(root, "file.txt"), "a\nb\nc\nd\ne\n"); - git(["add", "file.txt"]); - git(["commit", "--quiet", "-m", "base"]); - writeFileSync(join(root, "file.txt"), "a\nnew\nb\nc\nD\ne\n"); - let blocks = planCommitDiffBlocks(read(false), read(true))!; - git(["apply", "--cached", "--whitespace=nowarn", "-"], hunkPatch(blocks[0].stage!)); - blocks = planCommitDiffBlocks(read(false), read(true))!; - expect(blocks.map(block => block.checked)).toEqual([true, false]); - git(["apply", "--cached", "--whitespace=nowarn", "-"], hunkPatch(blocks[1].stage!)); - blocks = planCommitDiffBlocks(read(false), read(true))!; - expect(blocks.every(block => block.checked)).toBe(true); - git(["apply", "--cached", "--reverse", "--whitespace=nowarn", "-"], hunkPatch(blocks[0].unstage!)); - blocks = planCommitDiffBlocks(read(false), read(true))!; - expect(blocks.map(block => block.checked)).toEqual([false, true]); - expect(git(["show", ":file.txt"])).toBe("a\nb\nc\nD\ne\n"); - git(["apply", "--reverse", "--whitespace=nowarn", "-"], hunkPatch(blocks[0].rollback!)); - expect(git(["show", ":file.txt"])).toBe("a\nb\nc\nD\ne\n"); - git(["commit", "--quiet", "-m", "selected block"]); - expect(git(["show", "HEAD:file.txt"])).toBe("a\nb\nc\nD\ne\n"); - expect(git(["status", "--porcelain"])).toBe(""); - } finally { rmSync(root, { recursive: true, force: true }); } -}, 15000); - -test.each(["a", "B", "a\r\n", "B\r\n"])("real Git preserves exact %j bytes through whole-file fallback", content => { - const root = mkdtempSync(join(tmpdir(), "lithe-commit-bytes-")); - const git = (args: string[]) => execFileSync("git", ["-c", "core.autocrlf=false", - "-c", "core.attributesFile=", "-c", "commit.gpgsign=false", "-c", "core.hooksPath=", ...args], - { cwd: root, timeout: 5000, windowsHide: true }); - const read = (staged: boolean): GitDiff => ({ ...parseRawDiffContent(git(["diff", - ...(staged ? ["--cached"] : ["HEAD"]), `--unified=${FULL_FILE_CONTEXT_LINES}`, "--", "file.txt"]).toString("utf8"), - "file.txt") as GitDiff, is_full_context: true }); - try { - git(["init", "--quiet"]); - git(["config", "user.name", "Diff integration"]); - git(["config", "user.email", "diff@example.invalid"]); - writeFileSync(join(root, "file.txt"), "a\n"); - git(["add", "file.txt"]); - git(["commit", "--quiet", "-m", "base"]); - writeFileSync(join(root, "file.txt"), content); - expect(read(false).has_lossy_line_endings).toBe(true); - expect(planCommitDiffBlocks(read(false), read(true))).toBeNull(); - expect(git(["show", ":file.txt"])).toEqual(Buffer.from("a\n")); - git(["add", "file.txt"]); - expect(git(["show", ":file.txt"])).toEqual(Buffer.from(content)); - git(["commit", "--quiet", "-m", "exact bytes"]); - expect(git(["show", "HEAD:file.txt"])).toEqual(Buffer.from(content)); - expect(git(["status", "--porcelain"]).toString()).toBe(""); - } finally { rmSync(root, { recursive: true, force: true }); } -}, 15000); - -test.each([false, true])("real Git index-only/binary inclusion uses actual status (binary=%j)", binary => { - const root = mkdtempSync(join(tmpdir(), "lithe-commit-index-")); - const git = (args: string[]) => execFileSync("git", ["-c", "core.autocrlf=false", - "-c", "commit.gpgsign=false", "-c", "core.hooksPath=", ...args], - { cwd: root, timeout: 5000, windowsHide: true }); - const read = (staged: boolean): GitDiff => ({ ...parseRawDiffContent(git(["diff", - ...(staged ? ["--cached"] : ["HEAD"]), `--unified=${FULL_FILE_CONTEXT_LINES}`, "--", "file.txt"]).toString("utf8"), - "file.txt") as GitDiff, is_full_context: true }); - const base = binary ? "a\0\n" : "a\n", indexed = binary ? "B\0\n" : "B\n"; - const work = binary ? "C\0\n" : base; - try { - git(["init", "--quiet"]); - git(["config", "user.name", "Diff integration"]); - git(["config", "user.email", "diff@example.invalid"]); - writeFileSync(join(root, "file.txt"), base); - git(["add", "file.txt"]); - git(["commit", "--quiet", "-m", "base"]); - writeFileSync(join(root, "file.txt"), indexed); - git(["add", "file.txt"]); - writeFileSync(join(root, "file.txt"), work); - const status = git(["status", "--porcelain"]).toString(); - expect(status.startsWith("MM ")).toBe(true); - const snapshot = { diff: read(false), staged: read(true), file: { - path: "file.txt", status: "modified" as const, staged: status[0] === "M", worktree: status[1] === "M", - } }; - const presentation = commitDiffPresentation(snapshot, false); - expect(presentation.indeterminate).toBe(true); - expect(presentation.staged).toBe(!binary); - expect(binary ? presentation.diff.is_binary : presentation.diff.lines.some(line => line.content === "B")).toBe(true); - expect(planCommitDiffBlocks(snapshot.diff, snapshot.staged)).toBeNull(); - expect(git(["show", ":file.txt"])).toEqual(Buffer.from(indexed)); - git(["add", "file.txt"]); - expect(git(["show", ":file.txt"])).toEqual(Buffer.from(work)); - } finally { rmSync(root, { recursive: true, force: true }); } -}, 15000); - -test.each(["worktree", "snapshot", "staged"])("real Git full-context %s review projects multiple edits without repeating source lines", mode => { - const root = mkdtempSync(join(tmpdir(), "lithe-diff-context-")); - const git = (args: string[]) => execFileSync("git", ["-c", "core.autocrlf=false", - "-c", "commit.gpgsign=false", "-c", "core.hooksPath=", ...args], - { cwd: root, encoding: "utf8", timeout: 5000, windowsHide: true }); - const before = Array.from({ length: 40 }, (_, index) => `line ${index + 1}`); - const after = [...before]; - after[5] = "first replacement"; - after[19] = "second replacement"; - after[33] = "third replacement"; - try { - git(["init", "--quiet"]); - git(["config", "user.name", "Diff integration"]); - git(["config", "user.email", "diff@example.invalid"]); - writeFileSync(join(root, "file.txt"), before.join("\n") + "\n"); - git(["add", "file.txt"]); - git(["commit", "--quiet", "-m", "base"]); - writeFileSync(join(root, "file.txt"), after.join("\n") + "\n"); - if (mode === "staged") git(["add", "file.txt"]); - const patch = git([mode === "snapshot" ? "diff-files" : "diff", - ...(mode === "staged" ? ["--cached"] : []), "--no-ext-diff", - `--unified=${FULL_FILE_CONTEXT_LINES}`, "--", "file.txt"]); - const diff = { ...parseRawDiffContent(patch, "file.txt") as GitDiff, is_full_context: true }; - expect(diff.lines.filter(line => line.line_type === "header")).toHaveLength(1); - const rows = monacoDiffRows(diff, { hideHunkHeaders: true }); - const plan = planIndependentCommitDiff(rows, false, new Set()); - expect(plan.left.map(row => row.left)).toEqual(before); - expect(plan.right.map(row => row.right)).toEqual(after); - expect(plan.left.map(row => row.oldLine)).toEqual(Array.from({ length: 40 }, (_, index) => index + 1)); - expect(plan.right.map(row => row.newLine)).toEqual(Array.from({ length: 40 }, (_, index) => index + 1)); - expect(plan.changes).toHaveLength(3); - expect(planCommitDiffBlocks(diff, { ...diff, lines: [] })).toHaveLength(3); - } finally { rmSync(root, { recursive: true, force: true }); } -}, 15000); diff --git a/windows/tauri/src/features/git/utils/commit-diff-blocks.test.ts b/windows/tauri/src/features/git/utils/commit-diff-blocks.test.ts deleted file mode 100644 index 19eb023ecd..0000000000 --- a/windows/tauri/src/features/git/utils/commit-diff-blocks.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { expect, test } from "bun:test"; -import { fullDiff } from "@/test-utils/commit-diff-fixtures"; -import { planCommitDiffBlocks } from "./commit-diff-blocks"; - -test("adjacent changes inside one Git context hunk remain separately selectable", () => { - const head = ["a", "b", "c", "d", "e"]; - const work = ["a", "B", "c", "D", "e"]; - const blocks = planCommitDiffBlocks(fullDiff(head, work), fullDiff(head, head))!; - expect(blocks).toHaveLength(2); - expect(blocks.map(block => block.checked)).toEqual([false, false]); - expect(blocks[1].stage!.lines.filter(line => line.line_type === "added").map(line => line.content)).toEqual(["D"]); - expect(blocks[1].stage!.lines.filter(line => line.line_type === "context").map(line => line.content)).toContain("b"); -}); - -test("earlier staged additions shift the index span without selecting another block", () => { - const head = ["a", "b", "c", "d", "e"]; - const index = ["a", "new", "b", "c", "d", "e"]; - const work = ["a", "new", "b", "c", "D", "e"]; - const blocks = planCommitDiffBlocks(fullDiff(head, work), fullDiff(head, index))!; - expect(blocks.map(block => block.checked)).toEqual([true, false]); - expect(blocks[1].stage!.lines[0].content).toBe("@@ -2,5 +2,5 @@"); - expect(blocks[1].stage!.lines.filter(line => line.line_type === "added").map(line => line.content)).toEqual(["D"]); -}); - -test("partly staged lines in one visible block have mixed inclusion and reversible index patches", () => { - const head = ["a", "b", "c", "d"]; - const blocks = planCommitDiffBlocks(fullDiff(head, ["a", "B", "C", "d"]), - fullDiff(head, ["a", "B", "c", "d"]))!; - expect(blocks).toHaveLength(1); - expect(blocks[0]).toMatchObject({ checked: false, indeterminate: true, canToggle: true, canRollback: true }); - expect(blocks[0].unstage!.lines.filter(line => line.line_type === "removed").map(line => line.content)).toEqual(["b", "c"]); - expect(blocks[0].rollback!.lines.filter(line => line.line_type === "removed").map(line => line.content)).toEqual(["B", "c"]); -}); - -test("a staged span crossing a visible block boundary is not silently rewritten", () => { - const head = ["a", "b", "c", "d", "e"]; - const blocks = planCommitDiffBlocks(fullDiff(head, ["a", "B", "c", "d", "e"]), - fullDiff(head, ["a", "B", "C", "d", "e"]))!; - expect(blocks[0].canToggle).toBe(false); - expect(blocks[0].canRollback).toBe(false); -}); - -test("an index-only insertion at the outer edge of a visible replacement is preserved", () => { - const head = ["a", "b", "c"]; - const blocks = planCommitDiffBlocks(fullDiff(head, ["a", "B", "c"]), - fullDiff(head, ["a", "index only", "b", "c"]))!; - expect(blocks[0].canToggle).toBe(false); - expect(blocks[0].canRollback).toBe(false); -}); - -test("sparse, truncated and mismatched HEAD snapshots cannot produce writable blocks", () => { - const diff = fullDiff(["a", "b"], ["a", "B"]); - expect(planCommitDiffBlocks({ ...diff, is_full_context: false }, diff)).toBeNull(); - expect(planCommitDiffBlocks({ ...diff, is_truncated: true }, diff)).toBeNull(); - expect(planCommitDiffBlocks(diff, fullDiff(["wrong", "b"], ["wrong", "B"]))).toBeNull(); -}); - -test("whole-file addition uses one whole-path include action, not an invalid empty-index patch", () => { - const added = { ...fullDiff([], ["first", "second"]), is_new: true }; - // The fixture helper appends a newline to an empty stream; use Git's real empty-side header. - added.lines = [{ line_type: "header", content: "@@ -0,0 +1,2 @@" }, - { line_type: "added", content: "first", new_line_number: 1 }, - { line_type: "added", content: "second", new_line_number: 2 }]; - const unchecked = planCommitDiffBlocks(added, { ...added, lines: [] })!; - expect(unchecked[0]).toMatchObject({ wholeFile: true, canToggle: true, checked: false }); - expect(unchecked[0].stage).toBeUndefined(); - expect(planCommitDiffBlocks(added, added)![0].checked).toBe(true); -}); diff --git a/windows/tauri/src/features/git/utils/commit-diff-blocks.ts b/windows/tauri/src/features/git/utils/commit-diff-blocks.ts index ca4e995eb2..ce43b389de 100644 --- a/windows/tauri/src/features/git/utils/commit-diff-blocks.ts +++ b/windows/tauri/src/features/git/utils/commit-diff-blocks.ts @@ -1,160 +1,14 @@ -import equal from "fast-deep-equal"; -import type { GitDiff, GitHunk } from "../types/git.types"; -import { parseDiffHunkRange } from "./git-diff-helpers"; -import { gitGutterChangeHunk, type GitGutterChange } from "./git-gutter-changes"; -import { monacoDiffRows } from "./monaco-diff-rows"; -import { buildByteFaithfulHunk, buildFaithfulHunkFromRange, parseRawPatchLines } from "./lossy-rollback-patch"; -import { planIndependentCommitDiff, type IndependentChange } from "./independent-commit-diff"; +import type { CommitDiffBlock } from "../api/git-diff-block-api"; +import type { IndependentChange, planIndependentCommitDiff } from "./independent-commit-diff"; -export interface CommitDiffBlock { - id: string; - checked: boolean; - indeterminate: boolean; - canToggle: boolean; - canRollback: boolean; - /** Unfolded model indices; presentation maps the stable row ID after folding. */ - leftStart: number; - rightStart: number; - wholeFile?: boolean; - stage?: GitHunk; - unstage?: GitHunk; - rollback?: GitHunk; -} +export type { CommitDiffBlock }; +/** Row identity of a change, matching the block ID Core plans: the first old-side + * row of the change, else its first new-side row. Core owns the blocks; this + * only locates them in the display plan. */ export function commitDiffChangeID( plan: ReturnType, change: IndependentChange, ): string { return change.leftStart < change.leftEnd ? plan.left[change.leftStart].id : plan.right[change.rightStart].id; } - -/** Only complete, validated Git streams may supply index patches. Never use - * folded display coordinates, an aligned spacer, or sparse context as a base. */ -function projection(diff: GitDiff, rollbackOnly = false) { - if (!diff.is_full_context || diff.is_truncated || diff.is_binary || diff.is_image - || (diff.has_lossy_line_endings && !rollbackOnly)) return null; - const headers = diff.lines.filter(line => line.line_type === "header"); - if (headers.length !== 1) return null; - const range = parseDiffHunkRange(headers[0].content); - if (!range) return null; - const plan = planIndependentCommitDiff(monacoDiffRows(diff, { hideHunkHeaders: true }), false, new Set()); - if (plan.left.length !== range.oldCount || plan.right.length !== range.newCount - || range.oldStart !== (range.oldCount ? 1 : 0) - || range.newStart !== (range.newCount ? 1 : 0)) return null; - return { - plan, - before: plan.left.map(row => row.left!), - after: plan.right.map(row => row.right!), - }; -} - -function edit(start: number, before: string[], after: string[]): GitGutterChange { - return { - originalStart: start + 1, originalEnd: start + before.length + 1, - modifiedStart: start + 1, modifiedEnd: start + after.length + 1, - originalLines: before, modifiedLines: after, - kind: !before.length ? "added" : !after.length ? "deleted" : "modified", - }; -} - -/** HEAD -> worktree owns visible blocks; HEAD -> index owns inclusion. A block - * can contain only some staged lines, and earlier staged additions shift index - * coordinates. Rebuild just its index span with the existing gutter patch builder. */ -export function planCommitDiffBlocks(worktree: GitDiff, staged: GitDiff): CommitDiffBlock[] | null { - const lossyRollbackEligible = Boolean( - worktree.has_lossy_line_endings && worktree.writePatch && !worktree.is_renamed, - ); - const visible = projection(worktree, lossyRollbackEligible); - if (!visible || worktree.is_renamed) return null; - if (worktree.is_new || worktree.is_deleted) { - const indexed = staged.lines.length ? projection(staged) : null; - const change = visible.plan.changes[0]; - if (!change || visible.plan.changes.length !== 1 || (staged.lines.length && !indexed)) return null; - const checked = Boolean(indexed && equal(indexed.after, visible.after)); - return [{ id: commitDiffChangeID(visible.plan, change), checked, - indeterminate: !checked && staged.lines.length > 0, canToggle: true, - canRollback: false, wholeFile: true, leftStart: change.leftStart, rightStart: change.rightStart }]; - } - // The staged projection feeds both toggle and rollback calculations; when - // only rollback runs the byte-faithful channel, the staged side may use - // the lossy projection for coordinate math — canToggle stays separately - // gated and stage/unstage hunks are never built from lossy data. - const indexed = staged.lines.length === 0 - ? { before: visible.before, after: visible.before, plan: { changes: [] as IndependentChange[] } } - : projection(staged, lossyRollbackEligible); - if (!indexed || !equal(indexed.before, visible.before)) return null; - return visible.plan.changes.map(change => { - let start = change.leftStart, end = change.leftEnd, overlapsBoundary = false; - for (const prior of indexed.plan.changes) { - // A staged insertion at a replacement's outer boundary can be an - // index-only edit absent from this worktree block. Do not erase it. - if (prior.leftStart === prior.leftEnd && prior.leftStart === change.leftStart - && change.leftStart !== change.leftEnd) overlapsBoundary = true; - const delta = prior.rightEnd - prior.rightStart - (prior.leftEnd - prior.leftStart); - if (prior.leftEnd <= change.leftStart && prior.leftStart < change.leftStart) { - start += delta; end += delta; - } else if (prior.leftStart >= change.leftStart && prior.leftEnd <= change.leftEnd - && (prior.leftStart < change.leftEnd || change.leftStart === change.leftEnd)) { - end += delta; - } else if (prior.leftStart < change.leftEnd && prior.leftEnd > change.leftStart) { - overlapsBoundary = true; - } - } - const before = visible.before.slice(change.leftStart, change.leftEnd); - const after = visible.after.slice(change.rightStart, change.rightEnd); - const indexLines = indexed.after.slice(start, end); - // Display-stripped text comparison loses EOF/CRLF information; for lossy - // diffs a pure newline-only change would falsely read as already staged. - // When the writePatch is available, its changed lines within this block's - // span are the authoritative answer: no changes means fully included. - let checked = equal(indexLines, after); - if (checked && worktree.has_lossy_line_endings && worktree.writePatch) { - const rawChanged = parseRawPatchLines(worktree.writePatch).some(line => { - if (line.isEofMarker || (line.prefix !== "+" && line.prefix !== "-")) return false; - const oldOk = line.oldLine !== null && line.oldLine >= start + 1 && line.oldLine <= end; - const newOk = line.newLine !== null - && line.newLine >= change.rightStart + 1 && line.newLine <= change.rightEnd; - return oldOk || newOk; - }); - // Raw patch still reports changes inside this block: not fully included. - if (rawChanged) checked = false; - } - const indeterminate = !checked && !equal(indexLines, before); - const revertedIndex = [...indexed.after.slice(0, start), ...before, ...indexed.after.slice(end)]; - const rollbackBase = [...visible.after.slice(0, change.rightStart), ...indexLines, - ...visible.after.slice(change.rightEnd)]; - // Byte-faithful rollback passes the block's full index-side and - // worktree-side spans directly to the range builder — no line-by-line - // comparison needed. The builder collects all raw-patch changed lines - // within these spans. Zero-based slice indices convert to one-based - // inclusive: [start, end) → [start+1, end]; for the new side, - // rightEnd (not +1) yields an empty range for pure deletions. - const block: CommitDiffBlock = { - id: commitDiffChangeID(visible.plan, change), checked, indeterminate, - canToggle: !overlapsBoundary && !worktree.has_lossy_line_endings, - canRollback: !overlapsBoundary && !checked, - leftStart: change.leftStart, rightStart: change.rightStart, - stage: gitGutterChangeHunk(worktree.file_path, indexed.after, edit(start, indexLines, after)), - unstage: gitGutterChangeHunk(worktree.file_path, revertedIndex, edit(start, before, indexLines)), - rollback: worktree.writePatch - ? (buildFaithfulHunkFromRange( - worktree, - start + 1, - change.rightStart + 1, - end, - change.rightEnd, - ) ?? buildByteFaithfulHunk( - worktree, - start + 1, - change.rightStart + 1, - end, - change.rightEnd, - ) ?? undefined) - : (!worktree.has_lossy_line_endings - ? gitGutterChangeHunk(worktree.file_path, rollbackBase, edit(change.rightStart, indexLines, after)) - : undefined), - }; - block.canRollback = block.canRollback && block.rollback !== undefined; - return block; - }); -} diff --git a/windows/tauri/src/features/git/utils/git-gutter-changes.test.ts b/windows/tauri/src/features/git/utils/git-gutter-changes.test.ts index 1c612f3a34..f90d8d15f1 100644 --- a/windows/tauri/src/features/git/utils/git-gutter-changes.test.ts +++ b/windows/tauri/src/features/git/utils/git-gutter-changes.test.ts @@ -7,7 +7,6 @@ import { type GitGutterChange, computeGitGutterChanges, gitGutterBaseFromDiff, - gitGutterChangeHunk, gitGutterChangeIndexAtLine, gitGutterMarkerLines, gitGutterPeekAnchorLine, @@ -164,54 +163,6 @@ describe("Git gutter inline review rows", () => { }); }); -describe("Git gutter staging patch", () => { - test("builds a hunk whose context comes from the index text", () => { - const base = ["1", "2", "3", "4", "5", "6", "7", "8"]; - const [change] = changesOf(base, ["1", "2", "3", "4", "FIVE", "6", "7", "8"]); - const hunk = gitGutterChangeHunk("src/app.ts", base, change); - - expect(hunk.file_path).toBe("src/app.ts"); - expect(hunk.lines.map((line) => `${line.line_type}:${line.content}`)).toEqual([ - "header:@@ -2,7 +2,7 @@", - "context:2", - "context:3", - "context:4", - "removed:5", - "added:FIVE", - "context:6", - "context:7", - "context:8", - ]); - }); - - test("uses index context even when a neighbouring unstaged change is nearby", () => { - // Line 2 and line 4 both changed; staging the second must not stage "B". - const base = ["a", "b", "c", "d", "e"]; - const editor = ["a", "B", "c", "D", "e"]; - const changes = changesOf(base, editor); - const hunk = gitGutterChangeHunk("f.txt", base, changes[1]); - - expect(hunk.lines.map((line) => `${line.line_type}:${line.content}`)).toEqual([ - "header:@@ -1,5 +1,5 @@", - "context:a", - "context:b", - "context:c", - "removed:d", - "added:D", - "context:e", - ]); - }); - - test("names Git's empty ranges for an insertion at the start and a full deletion", () => { - const [insertion] = changesOf([], ["x"]); - expect(gitGutterChangeHunk("f", [], insertion).lines[0].content).toBe("@@ -0,0 +1,1 @@"); - - const [deletion] = changesOf(["x"], []); - expect(gitGutterChangeHunk("f", ["x"], deletion).lines[0].content).toBe("@@ -1,1 +0,0 @@"); - }); -}); - - describe("Git gutter repository identity", () => { test("an untracked sibling path does not hide the tracked file's markers", () => { const files = [{ @@ -229,25 +180,3 @@ describe("Git gutter repository identity", () => { ], "C:/workspace/a", "src/app.ts")).toBe(true); }); }); - -describe("Git gutter isolated patch coordinates", () => { - test("earlier unstaged deletions do not make the selected patch start negative", () => { - const base = Array.from({ length: 10 }, (_, index) => String(index + 1)); - // Exercise an isolated range directly: Monaco may coalesce nearby edits, - // but patch coordinates must remain valid for any selected range. - const change: GitGutterChange = { - kind: "modified", originalStart: 10, originalEnd: 11, - modifiedStart: 2, modifiedEnd: 3, originalLines: ["10"], modifiedLines: ["TEN"], - }; - const hunk = gitGutterChangeHunk("f", base, change); - expect(hunk.lines[0].content).toBe("@@ -7,4 +7,4 @@"); - expect(hunk.lines.slice(1).map((line) => line.content)).toEqual(["7", "8", "9", "10", "TEN"]); - }); - - test("earlier unstaged insertions do not shift the selected patch", () => { - const base = ["a", "b", "c", "d", "e", "f"]; - const changes = changesOf(base, ["new", ...base.slice(0, -1), "F"]); - expect(changes).toHaveLength(2); - expect(gitGutterChangeHunk("f", base, changes[1]).lines[0].content).toBe("@@ -3,4 +3,4 @@"); - }); -}); diff --git a/windows/tauri/src/features/git/utils/git-gutter-changes.ts b/windows/tauri/src/features/git/utils/git-gutter-changes.ts index e8d822b6b0..fe2fdf2a59 100644 --- a/windows/tauri/src/features/git/utils/git-gutter-changes.ts +++ b/windows/tauri/src/features/git/utils/git-gutter-changes.ts @@ -250,43 +250,3 @@ export function gitGutterChangeKey(change: GitGutterChange): string { change.modifiedLines, ]); } - -/** - * Builds a patch that stages exactly one change against the index. Context - * comes from the base (index) text, never from the editor, so a neighbouring - * unstaged change inside the context window stays out of the index. - */ -export function gitGutterChangeHunk( - filePath: string, - base: readonly string[], - change: GitGutterChange, -): GitHunk { - const context = GITGUTTER_HUNK_CONTEXT_LINES; - const removedStart = change.originalStart - 1; - const removedEnd = change.originalEnd - 1; - const contextStart = Math.max(0, removedStart - context); - const contextEnd = Math.min(base.length, removedEnd + context); - const leading = removedStart - contextStart; - const trailing = contextEnd - removedEnd; - const contextLine = (content: string): GitDiffLine => ({ line_type: "context", content }); - - const oldCount = contextEnd - contextStart; - const newCount = leading + change.modifiedLines.length + trailing; - // Git names the line before an empty range, or 0 at the start of the file. - const oldStart = oldCount === 0 ? contextStart : contextStart + 1; - // This patch applies only this change to the index. Earlier unstaged - // insertions/deletions must not shift its new-side coordinates. - const firstNewLine = contextStart + 1; - const newStart = newCount === 0 ? firstNewLine - 1 : firstNewLine; - - return { - file_path: filePath, - lines: [ - { line_type: "header", content: `@@ -${oldStart},${oldCount} +${newStart},${newCount} @@` }, - ...base.slice(contextStart, removedStart).map(contextLine), - ...change.originalLines.map((content): GitDiffLine => ({ line_type: "removed", content })), - ...change.modifiedLines.map((content): GitDiffLine => ({ line_type: "added", content })), - ...base.slice(removedEnd, contextEnd).map(contextLine), - ], - }; -} diff --git a/windows/tauri/src/features/git/utils/lossy-rollback-acceptance.test.ts b/windows/tauri/src/features/git/utils/lossy-rollback-acceptance.test.ts deleted file mode 100644 index 13c5642ea5..0000000000 --- a/windows/tauri/src/features/git/utils/lossy-rollback-acceptance.test.ts +++ /dev/null @@ -1,241 +0,0 @@ -import { expect, mock, test } from "bun:test"; -import { execFileSync } from "node:child_process"; -import { mkdtempSync, writeFileSync, readFileSync, rmSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { basename, dirname, join, resolve } from "node:path"; -import { parseRawDiffContent } from "@/test-utils/git-diff-fixture-parser"; -import { planCommitDiffBlocks } from "@/features/git/utils/commit-diff-blocks"; -import { monacoDiffHunk, monacoDiffRows } from "@/features/git/utils/monaco-diff-rows"; -import { createMonacoDiffHunkActions } from "@/features/git/utils/monaco-diff-hunk-actions"; - -function fixture(base: string, indexed: string, worktree: string) { - const root = mkdtempSync(join(tmpdir(), "lithe-lossy-acc-")); - const env = { ...process.env, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: "NUL", GIT_TERMINAL_PROMPT: "0" }; - const git = (args: string[], input?: string) => execFileSync("git", [ - "-c", "user.name=T", "-c", "user.email=t@t", - "-c", "commit.gpgsign=false", "-c", "core.hooksPath=", ...args, - ], { cwd: root, env, encoding: "utf8", timeout: 10000, windowsHide: true, input }); - const dispose = () => { - if (dirname(resolve(root)).toLowerCase() !== resolve(tmpdir()).toLowerCase() - || !basename(root).startsWith("lithe-lossy-acc-")) throw new Error("Unsafe cleanup target"); - rmSync(root, { recursive: true, force: true }); - }; - try { - git(["init", "-q"]); git(["config", "core.autocrlf", "false"]); - const file = join(root, "file.txt"); - writeFileSync(file, base); git(["add", "file.txt"]); git(["commit", "-qm", "base"]); - writeFileSync(file, indexed); git(["add", "file.txt"]); - writeFileSync(file, worktree); - const readDiff = (args: string[]) => { - const parsed = parseRawDiffContent(git(["diff", ...args, "--unified=9999", "--", "file.txt"]), "file.txt"); - return { ...("files" in parsed ? parsed.files[0] : parsed), is_full_context: true }; - }; - const raw = git(["diff", "--unified=3", "--", "file.txt"]); - const indexDiff = { ...readDiff([]), writePatch: raw }; - const display = { ...readDiff(["HEAD"]), writePatch: raw }; - const staged = readDiff(["--cached"]); - return { root, file, git, indexDiff, unstaged: indexDiff, display, staged, dispose }; - } catch (error) { dispose(); throw error; } -} -const crlf = (...lines: string[]) => lines.join("\r\n") + "\r\n"; -test("partial staging in one display block restores only unstaged content", () => { - const f = fixture("a\r\nb\r\nc\r\n", "A\r\nb\r\nc\r\n", "A\r\nB\r\nc\r\n"); - try { - const before = f.git(["diff", "--cached", "--binary"]); - const selected = planCommitDiffBlocks(f.display, f.staged)?.find(block => block.indeterminate); - expect(selected?.canRollback).toBe(true); - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], selected!.rollback!.rawPatch!); - expect(readFileSync(f.file, "utf8")).toBe("A\r\nb\r\nc\r\n"); - expect(f.git(["diff", "--cached", "--binary"])).toBe(before); - } finally { f.dispose(); } -}, 15000); - -test("CRLF pure insertion exposes a byte faithful rollback", () => { - const f = fixture("a\r\nb\r\nc\r\n", "a\r\nb\r\nc\r\n", "a\r\nNEW\r\nb\r\nc\r\n"); - try { - const blocks = planCommitDiffBlocks(f.display, f.staged); - console.log("pure insertion", JSON.stringify(blocks?.map(b => ({ canRollback: b.canRollback, leftStart: b.leftStart, rightStart: b.rightStart })))); - expect(blocks?.[0]?.canRollback).toBe(true); - } finally { f.dispose(); } -}, 15000); - -test("selected display block does not rollback neighboring reverted staged content", () => { - const f = fixture("a\r\nb\r\nc\r\nd\r\n", "A\r\nB\r\nc\r\nd\r\n", "A\r\nb\r\nC\r\nd\r\n"); - try { - const indexBefore = f.git(["rev-parse", ":file.txt"]); - const stagedBefore = f.git(["diff", "--cached", "--binary"]); - const blocks = planCommitDiffBlocks(f.display, f.staged)!; - const selected = blocks.find(block => block.rightStart === 2)!; - expect(selected.canRollback).toBe(true); - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], selected.rollback!.rawPatch!); - const after = readFileSync(f.file, "utf8"); - console.log("neighbor staged reversion", JSON.stringify({ blockCount: blocks.length, selectedLine: 3, patch: selected.rollback!.rawPatch, after })); - expect(f.git(["rev-parse", ":file.txt"])).toBe(indexBefore); - expect(f.git(["diff", "--cached", "--binary"])).toBe(stagedBefore); - expect(after).toBe("A\r\nb\r\nc\r\nd\r\n"); - } finally { f.dispose(); } -}, 15000); - -test("Monaco CRLF replacement carries rawPatch through discard operation", async () => { - const f = fixture("a\r\nb\r\nc\r\n", "a\r\nb\r\nc\r\n", "a\r\nB\r\nc\r\n"); - let owner: ReturnType | undefined; - try { - const target = monacoDiffRows(f.indexDiff).find(row => row.actionAnchor)!; - let received: any; - let probed: any; - owner = createMonacoDiffHunkActions(f.indexDiff, { repoPath: f.root, isStaged: false, canDiscard: true }, { - stage: async () => false, unstage: async () => false, - discard: async (_repo, hunk) => { received = hunk; return true; }, - validate: async (hunk) => { probed = hunk; return true; }, - }); - expect(owner.actions).toEqual(["discard"]); - expect(await owner.apply(target.hunkID!, "discard")).toBe("applied"); - console.log("monaco replacement", JSON.stringify({ hasRaw: Boolean(received?.rawPatch), lines: received?.lines })); - expect(received?.rawPatch).toBeTruthy(); - // The zero-context write was confirmed first, against the same hunk. - expect(probed).toBe(received); - } finally { owner?.dispose(); f.dispose(); } -}, 15000); - -test("Monaco hunk containing two CRLF changes rolls back both", () => { - const f = fixture("a\r\nb\r\nc\r\nd\r\ne\r\nf\r\n", "a\r\nb\r\nc\r\nd\r\ne\r\nf\r\n", "a\r\nNEW\r\nb\r\nc\r\nD\r\ne\r\nf\r\n"); - try { - const anchors = monacoDiffRows(f.indexDiff).filter(row => row.actionAnchor); - expect(anchors.length).toBe(1); - const hunk = monacoDiffHunk(f.indexDiff, anchors[0].hunkID!); - expect(hunk?.rawPatch).toBeTruthy(); - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], hunk!.rawPatch!); - console.log("monaco grouped after", JSON.stringify(readFileSync(f.file, "utf8"))); - expect(readFileSync(f.file, "utf8")).toBe("a\r\nb\r\nc\r\nd\r\ne\r\nf\r\n"); - } finally { f.dispose(); } -}, 15000); - -test("Monaco EOF rollback preserves missing newline through the host patch format", () => { - for (const [indexed, worktree] of [["a\nb\nc", "a\nB\nc"], ["a\nb\nc\n", "a\nb\nC"], ["a\nb\nc", "a\nb\nC\n"]]) { - const f = fixture(indexed, indexed, worktree); - try { - const anchor = monacoDiffRows(f.indexDiff).find(row => row.actionAnchor)!; - const hunk = monacoDiffHunk(f.indexDiff, anchor.hunkID!)!; - // This mirrors the existing hunk_patch host branch when rawPatch is absent. - const patch = hunk.rawPatch ?? `diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n${hunk.lines.map(line => - (line.line_type === "header" ? "" : line.line_type === "added" ? "+" : line.line_type === "removed" ? "-" : " ") + line.content + "\n").join("")}`; - let failure: string | undefined; - try { f.git(["apply", "--reverse", ...(hunk.rawPatch ? ["--unidiff-zero"] : []), "--whitespace=nowarn", "-"], patch); } - catch (error) { failure = String(error); } - const after = readFileSync(f.file, "utf8"); - console.log("monaco EOF", JSON.stringify({ indexed, worktree, hasRaw: Boolean(hunk.rawPatch), after, failure })); - expect(failure).toBeUndefined(); - expect(after).toBe(indexed); - } finally { f.dispose(); } - } -}, 15000); - -test("Commit deletion does not consume the following unrelated worktree replacement", () => { - const base = crlf("a", "b", "c", "d", "e", "f", "g", "h"); - const indexed = crlf("a", "b", "c", "d", "e", "f", "g", "STAGED"); - const f = fixture(base, indexed, crlf("a", "d", "E", "f", "g", "STAGED")); - try { - const selected = planCommitDiffBlocks(f.display, f.staged)!.find(b => b.leftStart === 1)!; - expect(selected.canRollback).toBe(true); - const indexBefore = f.git(["rev-parse", ":file.txt"]); - const stagedBefore = f.git(["diff", "--cached", "--binary"]); - const patch = selected.rollback!.rawPatch!; - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], patch); - console.log("deletion scope", JSON.stringify({ patch, after: readFileSync(f.file, "utf8") })); - expect(f.git(["rev-parse", ":file.txt"])).toBe(indexBefore); - expect(f.git(["diff", "--cached", "--binary"])).toBe(stagedBefore); - expect(readFileSync(f.file, "utf8")).toBe(crlf("a", "b", "c", "d", "E", "f", "g", "STAGED")); - } finally { f.dispose(); } -}, 15000); - -test("Commit rollback preserves old-side EOF marker ownership", () => { - const f = fixture("a\nb\nc", "A\nb\nc", "A\nb\nC\n"); - try { - const selected = planCommitDiffBlocks(f.display, f.staged)!.find(b => b.rightStart === 2)!; - expect(selected.canRollback).toBe(true); - const patch = selected.rollback!.rawPatch!; - console.log("commit EOF patch", JSON.stringify(patch)); - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], patch); - expect(readFileSync(f.file, "utf8")).toBe("A\nb\nc"); - } finally { f.dispose(); } -}, 15000); - -test("Monaco grouped pure deletions restore at distinct original boundaries", () => { - const base = crlf("a", "b", "c", "d", "e"); - const f = fixture(base, base, crlf("a", "c", "e")); - try { - const anchors = monacoDiffRows(f.unstaged).filter(row => row.actionAnchor); - expect(anchors.length).toBe(1); - const patch = monacoDiffHunk(f.unstaged, anchors[0].hunkID!)!.rawPatch!; - console.log("monaco deletions patch", JSON.stringify(patch)); - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], patch); - console.log("monaco deletions after", JSON.stringify(readFileSync(f.file, "utf8"))); - expect(readFileSync(f.file, "utf8")).toBe(base); - } finally { f.dispose(); } -}, 15000); - -test("Commit partial staging followed by an insertion exposes faithful rollback", () => { - const f = fixture(crlf("a", "b", "c"), crlf("A", "b", "c"), crlf("A", "NEW", "b", "c")); - try { - const blocks = planCommitDiffBlocks(f.display, f.staged)!; - expect(blocks[0].canRollback).toBe(true); - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], blocks[0].rollback!.rawPatch!); - expect(readFileSync(f.file, "utf8")).toBe(crlf("A", "b", "c")); - } finally { f.dispose(); } -}, 15000); - -test("acceptance: staged prefix must not redirect rollback to an unselected block", async () => { - const root = mkdtempSync(join(tmpdir(), "lithe-api-block-acceptance-")); - const env = { ...process.env, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: "NUL", GIT_TERMINAL_PROMPT: "0" }; - const git = (args: string[], input?: string) => execFileSync("git", [ - "-c", "user.name=Reviewer", "-c", "user.email=review@example.invalid", - "-c", "commit.gpgsign=false", "-c", "core.hooksPath=", ...args, - ], { cwd: root, env, encoding: "utf8", timeout: 5000, windowsHide: true, input }); - const core = await import("@/platform/tauri-core"); - mock.module("@/platform/tauri-core", () => ({ ...core, invoke: async (command: string, args: any) => { - if (command === "git_discover_repo") return root; - if (command === "git.resolveFileContext") return { root, path: "file.txt", ownership: "git" }; - if (command === "git.command") return { output: git(args.arguments), exitCode: 0 }; - if (command === "git.diff") return { patch: git(["diff", "--unified=3", "--", "file.txt"]) }; - if (command === "git_diff_file") return parseRawDiffContent(git([ - "diff", ...(args.worktreeSnapshot ? ["HEAD"] : args.staged ? ["--cached"] : []), - "--unified=9999", "--", "file.txt", - ]), "file.txt"); - throw new Error(`Unexpected command: ${command}`); - } })); - try { - const { getWorkingTreePathDiff, getFullContextFileDiff } = await import("@/features/git/api/git-diff-api"); - const { planCommitDiffBlocks } = await import("@/features/git/utils/commit-diff-blocks"); - git(["init", "-q"]); git(["config", "core.autocrlf", "false"]); - const file = join(root, "file.txt"); - const base = Array.from({ length: 10 }, (_, i) => `line-${i + 1}`); - writeFileSync(file, base.join("\r\n") + "\r\n"); - git(["add", "file.txt"]); git(["commit", "-qm", "base"]); - const indexed = ["S1", "S2", "S3", ...base]; - writeFileSync(file, indexed.join("\r\n") + "\r\n"); git(["add", "file.txt"]); - const indexBefore = git(["rev-parse", ":file.txt"]); - const stagedBefore = git(["diff", "--cached", "--binary"]); - const work = [...indexed]; work[5] = "WT-THREE"; work[8] = "WT-SIX"; - writeFileSync(file, work.join("\r\n") + "\r\n"); - const visible = await getWorkingTreePathDiff(root, "file.txt", false, undefined, true); - const staged = await getFullContextFileDiff(root, "file.txt", true); - const faithful = await getFullContextFileDiff(root, "file.txt", false); - const blocks = planCommitDiffBlocks({ ...visible!, writePatch: faithful?.writePatch }, staged!); - const selected = blocks?.find(block => block.rightStart + 1 === 9); - expect(selected?.canRollback).toBe(true); - const patch = selected?.rollback?.rawPatch; - expect(patch).toBeTruthy(); - git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], patch!); - const after = readFileSync(file, "utf8").split("\r\n"); - console.log(JSON.stringify({ selectedLine: 9, patchHeader: patch!.match(/@@[^\n]+/g), - unselectedLine6: after[5], selectedLine9: after[8] })); - expect(git(["rev-parse", ":file.txt"])).toBe(indexBefore); - expect(git(["diff", "--cached", "--binary"])).toBe(stagedBefore); - expect(after[5]).toBe("WT-THREE"); - expect(after[8]).toBe("line-6"); - } finally { - if (dirname(resolve(root)).toLowerCase() !== resolve(tmpdir()).toLowerCase() - || !basename(root).startsWith("lithe-api-block-acceptance-")) throw new Error("Unsafe cleanup target"); - rmSync(root, { recursive: true, force: true }); - } -}, 15000); diff --git a/windows/tauri/src/features/git/utils/lossy-rollback-attrs.ts b/windows/tauri/src/features/git/utils/lossy-rollback-attrs.ts deleted file mode 100644 index 8c421dc8c8..0000000000 --- a/windows/tauri/src/features/git/utils/lossy-rollback-attrs.ts +++ /dev/null @@ -1,77 +0,0 @@ -import { invoke as tauriInvoke } from "@/platform/tauri-core"; - -/** Attributes that decide whether raw patch bytes can round-trip through - * `git apply` without silent conversion or filtering. */ -export interface GitAttributeSafety { - /** Safe to route hunk rollback through the raw patch channel. */ - byteSafe: boolean; - /** Human-readable reason when unsafe, for diagnostics. */ - reason?: string; -} - -/** - * Parses `git check-attr` output. Each line is `path: attr: value`; the - * attribute name is the key, not the path. Missing or unparseable output - * fails closed (unsafe). - */ -function parseCheckAttr(output: string): Map { - const attributes = new Map(); - for (const line of output.split("\n")) { - const parts = line.split(":"); - // path : attrName : value — three colon-separated fields; the path may - // contain colons on some platforms but check-attr on a single file - // always emits exactly `path: attr: value` per line. - if (parts.length < 3) continue; - const attrName = parts[parts.length - 2].trim(); - const value = parts[parts.length - 1].trim(); - if (attrName) { - attributes.set(attrName, value); - } - } - return attributes; -} - -/** Checks Git's effective attributes for the file: any filter or non-trivial - * working-tree-encoding that could rewrite bytes means the raw patch cannot - * be proven byte-faithful, and rollback must stay in the protective - * whole-file mode. `text` and `eol` are conversion directives that Git - * itself handles faithfully through `git apply`, so they stay safe. */ -export async function checkHunkRollbackAttributes( - rootPath: string, - filePath: string, -): Promise { - try { - const response = await tauriInvoke<{ output: string; exitCode: number }>("git.command", { - root: rootPath, - arguments: [ - "check-attr", - "filter", - "working-tree-encoding", - "text", - "eol", - "--", - filePath, - ], - }); - if (response.exitCode !== 0) { - return { byteSafe: false, reason: "attribute check failed" }; - } - const attrs = parseCheckAttr(response.output); - // Fail closed: if the required attributes are absent from the output, - // the check itself is unreliable. - const filter = attrs.get("filter"); - if (filter === undefined) { - return { byteSafe: false, reason: "filter attribute not reported" }; - } - if (filter !== "unspecified" && filter !== "unset") { - return { byteSafe: false, reason: `filter=${filter}` }; - } - const encoding = attrs.get("working-tree-encoding"); - if (encoding !== undefined && encoding !== "unspecified" && encoding !== "unset") { - return { byteSafe: false, reason: `working-tree-encoding=${encoding}` }; - } - return { byteSafe: true }; - } catch { - return { byteSafe: false, reason: "attribute check failed" }; - } -} diff --git a/windows/tauri/src/features/git/utils/lossy-rollback-channel.test.ts b/windows/tauri/src/features/git/utils/lossy-rollback-channel.test.ts deleted file mode 100644 index 6ef324aac0..0000000000 --- a/windows/tauri/src/features/git/utils/lossy-rollback-channel.test.ts +++ /dev/null @@ -1,248 +0,0 @@ -import { expect, mock, spyOn, test } from "bun:test"; -import { execFileSync } from "node:child_process"; -import { mkdtempSync, writeFileSync, readFileSync, rmSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { basename, dirname, join, resolve } from "node:path"; -import { parseRawDiffContent } from "@/test-utils/git-diff-fixture-parser"; -import { monacoDiffRows } from "@/features/git/utils/monaco-diff-rows"; -import { createMonacoDiffHunkActions } from "@/features/git/utils/monaco-diff-hunk-actions"; - -let current: ReturnType | undefined; -let afterDisplayRead: (() => void) | undefined; -function fixture() { - const root = mkdtempSync(join(tmpdir(), "lithe-channel-review-")); - const file = join(root, "file.txt"); - const env = { ...process.env, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: "NUL", GIT_TERMINAL_PROMPT: "0" }; - const git = (args: string[], input?: string) => execFileSync("git", [ - "-c", "user.name=Reviewer", "-c", "user.email=review@example.invalid", - "-c", "commit.gpgsign=false", "-c", "core.hooksPath=", ...args, - ], { cwd: root, env, encoding: "utf8", timeout: 5000, windowsHide: true, input }); - const dispose = () => { - if (dirname(resolve(root)).toLowerCase() !== resolve(tmpdir()).toLowerCase() - || !basename(root).startsWith("lithe-channel-review-")) throw new Error("Unsafe cleanup target"); - rmSync(root, { recursive: true, force: true }); - }; - try { - git(["init", "-q"]); git(["config", "core.autocrlf", "false"]); - writeFileSync(file, "a\r\nb\r\nc\r\n"); git(["add", "file.txt"]); git(["commit", "-qm", "base"]); - writeFileSync(file, "a\r\nDISPLAYED\r\nc\r\n"); - return { root, file, git, dispose }; - } catch (error) { dispose(); throw error; } -} - -const core = await import("@/platform/tauri-core"); -mock.module("@/platform/tauri-core", () => ({ ...core, invoke: async (command: string, args: any, options?: { rawCoreResult?: boolean }) => { - if (!current) throw new Error("Fixture is not active"); - if (command === "git_discover_repo") return current.root; - if (command === "git.command") return { output: current.git(args.arguments), exitCode: 0 }; - // Like the real compatibility adapter, `git.diff` yields display diffs - // unless the caller asks for Core's raw response. - if (command === "git.diff") { - const patch = current.git(["diff", "--unified=3", "--", "file.txt"]); - return options?.rawCoreResult ? { patch, exitCode: 0, hasDecodingLoss: false } : [parseRawDiffContent(patch, "file.txt")]; - } - if (command === "git_diff_file") { - const diff = parseRawDiffContent(current.git(["diff", ...(args.worktreeSnapshot ? ["HEAD"] : args.staged ? ["--cached"] : []), - "--unified=9999", "--", "file.txt"]), "file.txt"); - const mutate = afterDisplayRead; - afterDisplayRead = undefined; - mutate?.(); - return diff; - } - if (command === "git.resolveFileContext") return { root: current.root, path: "file.txt", ownership: "git" }; - throw new Error(`Unexpected command: ${command}`); -} })); -const { getWorkingTreePathDiff, isWorktreeWritePatchCurrent } = await import("@/features/git/api/git-diff-api"); - -// The same operations MonacoGitDiff wires: a real zero-context apply guarded -// by the real pre-write probe. -function monacoOperations(f: ReturnType, writePatch: string | undefined) { - return { - stage: async () => false, unstage: async () => false, - discard: async (_repo: string, hunk: { rawPatch?: string }) => { - if (!hunk.rawPatch) return false; - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], hunk.rawPatch); - return true; - }, - validate: () => isWorktreeWritePatchCurrent(f.root, "file.txt", writePatch), - }; -} -const { loadWorkingTreeFileDiff } = await import("@/features/git/services/working-tree-diff-refresh"); - -test("normal Monaco loading and refresh attach writePatch and perform faithful discard", async () => { - const f = current = fixture(); - let owner: ReturnType | undefined; - try { - const diff = await loadWorkingTreeFileDiff({ repoPath: f.root, filePath: "file.txt", untracked: false }); - expect(diff?.writePatch).toBeTruthy(); - owner = createMonacoDiffHunkActions(diff!, { repoPath: f.root, isStaged: false, canDiscard: true }, - monacoOperations(f, diff!.writePatch)); - const target = monacoDiffRows(diff!).find(row => row.actionAnchor)!; - expect(await owner.apply(target.hunkID!, "discard")).toBe("applied"); - expect(readFileSync(f.file, "utf8")).toBe("a\r\nb\r\nc\r\n"); - } finally { owner?.dispose(); afterDisplayRead = undefined; current = undefined; f.dispose(); } -}, 15000); - -// Regression: the probe once re-read a three-line-context display diff and -// compared it with the full-context review, so every discard there was -// refused. The raw patch it now compares has a fixed context width. -test("a full-context review discards a current block through the pre-write probe", async () => { - const f = current = fixture(); - let owner: ReturnType | undefined; - try { - const lines = (...content: string[]) => content.join("\r\n") + "\r\n"; - const body = Array.from({ length: 12 }, (_, index) => `line ${index + 1}`); - writeFileSync(f.file, lines(...body)); f.git(["add", "file.txt"]); f.git(["commit", "-qm", "long-base"]); - writeFileSync(f.file, lines(...body.slice(0, 6), "CHANGED", ...body.slice(7))); - const diff = await getWorkingTreePathDiff(f.root, "file.txt", false, undefined, true); - expect(diff?.is_full_context).toBe(true); - expect(diff?.writePatch).toBeTruthy(); - owner = createMonacoDiffHunkActions(diff!, { repoPath: f.root, isStaged: false, canDiscard: true }, - monacoOperations(f, diff!.writePatch)); - const target = monacoDiffRows(diff!).find(row => row.actionAnchor)!; - expect(await owner.apply(target.hunkID!, "discard")).toBe("applied"); - expect(readFileSync(f.file, "utf8")).toBe(lines(...body)); - } finally { owner?.dispose(); afterDisplayRead = undefined; current = undefined; f.dispose(); } -}, 15000); - -test("an edit after the diff loaded refuses the zero-context discard as stale", async () => { - for (const later of ["a\r\nDISPLAYED\r\nLATER\r\n", "a\r\nDISPLAYED\r\nc\n"]) { - // The second edit only drops a CR, which the display strips. - const f = current = fixture(); - let owner: ReturnType | undefined; - try { - const diff = await getWorkingTreePathDiff(f.root, "file.txt", false, undefined, true); - expect(diff?.writePatch).toBeTruthy(); - owner = createMonacoDiffHunkActions(diff!, { repoPath: f.root, isStaged: false, canDiscard: true }, - monacoOperations(f, diff!.writePatch)); - writeFileSync(f.file, later); - const target = monacoDiffRows(diff!).find(row => row.actionAnchor)!; - expect(await owner.apply(target.hunkID!, "discard")).toBe("stale"); - expect(readFileSync(f.file, "utf8")).toBe(later); - } finally { owner?.dispose(); afterDisplayRead = undefined; current = undefined; f.dispose(); } - } -}, 15000); - -test("a writePatch fetched after a file change must not discard content absent from the displayed snapshot", async () => { - const f = current = fixture(); - let owner: ReturnType | undefined; - try { - const unseen = "a\r\nUNSEEN-LATER-EDIT\r\nc\r\n"; - afterDisplayRead = () => writeFileSync(f.file, unseen); - const diff = await getWorkingTreePathDiff(f.root, "file.txt", false, undefined, true); - expect(diff?.lines.some(line => line.content === "DISPLAYED")).toBe(true); - expect(diff?.lines.some(line => line.content === "UNSEEN-LATER-EDIT")).toBe(false); - if (!diff?.writePatch) { - expect(readFileSync(f.file, "utf8")).toBe(unseen); - return; - } - owner = createMonacoDiffHunkActions(diff!, { repoPath: f.root, isStaged: false, canDiscard: true }, { - stage: async () => false, unstage: async () => false, - discard: async (_repo, hunk) => { - if (!hunk.rawPatch) return false; - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], hunk.rawPatch); - return true; - }, - }); - const target = monacoDiffRows(diff!).find(row => row.actionAnchor)!; - const result = await owner.apply(target.hunkID!, "discard"); - console.log("mixed snapshots", JSON.stringify({ displayed: diff?.lines.filter(l => l.line_type === "added"), - raw: diff?.writePatch, result, after: readFileSync(f.file, "utf8") })); - expect(readFileSync(f.file, "utf8")).toBe(unseen); - } finally { owner?.dispose(); afterDisplayRead = undefined; current = undefined; f.dispose(); } -}, 15000); - -test("Commit hook must not reattach a later faithful patch to an old display rejected by the API guard", async () => { - const { act, createElement } = await import("react"); - const { createRoot } = await import("react-dom/client"); - const { installHappyDom } = await import("@/test-utils/happy-dom"); - const { useBufferStore } = await import("@/features/editor/stores/buffer.store"); - const statusApi = await import("@/features/git/api/git-status-api"); - const { useCommitDiffReview } = await import("@/features/git/hooks/use-commit-diff-review"); - const { createSingleFileWorkingTreeDiff } = await import("@/features/git/utils/working-tree-multi-diff"); - const { LocaleProvider } = await import("@/i18n/locale-provider"); - const settingsStore = await import("@/features/settings/stores/settings.store"); - const restoreDom = installHappyDom(); - const actGlobal = globalThis as any; - const priorAct = actGlobal.IS_REACT_ACT_ENVIRONMENT; - const priorEvent = Object.getOwnPropertyDescriptor(globalThis, "CustomEvent"); - actGlobal.IS_REACT_ACT_ENVIRONMENT = true; - Object.defineProperty(globalThis, "CustomEvent", { configurable: true, writable: true, value: window.CustomEvent }); - const container = document.createElement("div"); document.body.append(container); - const root = createRoot(container); - const f = current = fixture(); - const spies: Array<{ mockRestore(): void }> = []; - try { - const target = { repoPath: f.root, filePath: "file.txt", untracked: false }; - const fileKey = "unstaged:file.txt"; - const parsedHead = parseRawDiffContent(f.git(["diff", "HEAD", "--unified=9999", "--", "file.txt"]), "file.txt"); - let data = createSingleFileWorkingTreeDiff({ repoPath: f.root, fileKey, target, commitPreview: true, - diff: { ...("files" in parsedHead ? parsedHead.files[0] : parsedHead), is_full_context: true } }); - const state = useBufferStore.getState(); - spies.push(spyOn(useBufferStore, "getState").mockImplementation(() => ({ ...state, - activeBufferId: "review-buffer", buffers: [{ id: "review-buffer", type: "diff", path: "diff://review", diffData: data }], - actions: { ...state.actions, updateBufferContent: (_id: string, _text: string, _dirty: boolean, next: typeof data) => { data = next; } }, - } as any))); - spies.push(spyOn(statusApi, "getGitStatus").mockResolvedValue({ branch: "main", ahead: 0, behind: 0, - files: [{ path: "file.txt", status: "modified", staged: false, worktree: true }] })); - spies.push(spyOn(statusApi, "discardHunk").mockImplementation(async (_repo, hunk) => { - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], hunk.rawPatch!); - return true; - })); - let review: ReturnType; - function Harness() { review = useCommitDiffReview("review-buffer", fileKey, target); return null; } - // The merged hook localizes its rollback dialog; render inside the - // provider and keep the direct-discards path this test exercises. - const settings = settingsStore.useSettingsStore.getState(); - spies.push(spyOn(settingsStore.useSettingsStore, "getState").mockReturnValue({ - ...settings, settings: { ...settings.settings, confirmBeforeDiscard: false }, - })); - const unseen = "a\r\nUNSEEN-LATER-EDIT\r\nc\r\n"; - afterDisplayRead = () => writeFileSync(f.file, unseen); - await act(async () => root.render(createElement(LocaleProvider, { language: "en-US", children: createElement(Harness) }))); - expect(review!.snapshot?.diff.lines.some(line => line.content === "DISPLAYED")).toBe(true); - let result: unknown = "disabled"; - const selected = review!.blocks.find(block => block.canRollback); - if (selected) await act(async () => { result = await review!.rollback(selected.id); }); - console.log("Commit hook mixed snapshot", JSON.stringify({ result, displayed: review!.snapshot?.diff.lines, - raw: review!.snapshot?.diff.writePatch, after: readFileSync(f.file, "utf8") })); - expect(readFileSync(f.file, "utf8")).toBe(unseen); - } finally { - try { await act(async () => root.unmount()); } - finally { - spies.forEach(spy => spy.mockRestore()); container.remove(); - if (priorAct === undefined) delete actGlobal.IS_REACT_ACT_ENVIRONMENT; else actGlobal.IS_REACT_ACT_ENVIRONMENT = priorAct; - if (priorEvent) Object.defineProperty(globalThis, "CustomEvent", priorEvent); else Reflect.deleteProperty(globalThis, "CustomEvent"); - restoreDom(); afterDisplayRead = undefined; current = undefined; f.dispose(); - } - } -}, 15000); - -test("Monaco must reject relocated edits even when the changed text sequence is identical", async () => { - const f = current = fixture(); - let owner: ReturnType | undefined; - try { - const lines = (...content: string[]) => content.join("\r\n") + "\r\n"; - const base = lines("a", "b", "c", "b", "e", "f", "g", "h"); - writeFileSync(f.file, base); f.git(["add", "file.txt"]); f.git(["commit", "-qm", "larger-base"]); - writeFileSync(f.file, lines("a", "DISPLAYED", "c", "b", "e", "F", "g", "h")); - const unseen = lines("a", "b", "c", "DISPLAYED", "e", "F", "g", "h"); - afterDisplayRead = () => writeFileSync(f.file, unseen); - const diff = await getWorkingTreePathDiff(f.root, "file.txt", false, undefined, true); - expect(diff?.lines.filter(line => line.line_type === "added").map(line => line.new_line_number)).toEqual([2, 6]); - if (!diff?.writePatch) { expect(readFileSync(f.file, "utf8")).toBe(unseen); return; } - owner = createMonacoDiffHunkActions(diff, { repoPath: f.root, isStaged: false, canDiscard: true }, { - stage: async () => false, unstage: async () => false, - discard: async (_repo, hunk) => { - if (!hunk.rawPatch) return false; - f.git(["apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", "-"], hunk.rawPatch); - return true; - }, - }); - const anchor = monacoDiffRows(diff).find(row => row.actionAnchor)!; - const result = await owner.apply(anchor.hunkID!, "discard"); - console.log("relocated edits", JSON.stringify({ displayedAddedLines: diff.lines.filter(l => l.line_type === "added"), - raw: diff.writePatch, result, after: readFileSync(f.file, "utf8") })); - expect(readFileSync(f.file, "utf8")).toBe(unseen); - } finally { owner?.dispose(); afterDisplayRead = undefined; current = undefined; f.dispose(); } -}, 15000); diff --git a/windows/tauri/src/features/git/utils/lossy-rollback-patch.test.ts b/windows/tauri/src/features/git/utils/lossy-rollback-patch.test.ts deleted file mode 100644 index c7bd269ecb..0000000000 --- a/windows/tauri/src/features/git/utils/lossy-rollback-patch.test.ts +++ /dev/null @@ -1,213 +0,0 @@ -import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { execSync } from "node:child_process"; -import { mkdtempSync, rmSync, readFileSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; -import { tmpdir } from "node:os"; - -const invoke = mock(async (command: string, _args?: unknown): Promise => { - if (command === "git_discover_repo") return null; - if (command === "git.command") { - return { output: "doc.txt: filter: unspecified\ndoc.txt: working-tree-encoding: unspecified\n", exitCode: 0 }; - } - if (command === "git.diff") return { patch: "", exitCode: 0 }; - throw new Error(`Unexpected command: ${command}`); -}); -const tauriCoreModule = await import("@/platform/tauri-core"); -mock.module("@/platform/tauri-core", () => ({ ...tauriCoreModule, invoke })); - -const { checkHunkRollbackAttributes } = await import("./lossy-rollback-attrs"); -const { parseRawPatchLines, sliceRawHunkPatch, buildByteFaithfulHunk } = await import("./lossy-rollback-patch"); - -function git(dir: string, ...args: string[]): string { - return execSync(`git ${args.join(" ")}`, { cwd: dir, encoding: "utf-8" }); -} - -function makeRepo(label: string, config: { autocrlf?: string; gitattributes?: string }): string { - const dir = mkdtempSync(join(tmpdir(), `lithe-lossy-${label}-`)); - git(dir, "init", "-q"); - git(dir, "config", "user.name", "T"); - git(dir, "config", "user.email", "t@t"); - git(dir, "config", "core.autocrlf", config.autocrlf ?? "false"); - if (config.gitattributes) writeFileSync(join(dir, ".gitattributes"), config.gitattributes); - return dir; -} - -const repos: string[] = []; -afterEach(() => { - for (const dir of repos.splice(0)) { - try { rmSync(dir, { recursive: true, force: true }); } catch { /* best effort */ } - } -}); - -describe("lossy hunk rollback patch slicing", () => { - test("parseRawPatchLines preserves CR bytes and EOF markers", () => { - const patch = "diff --git a/doc b/doc\n--- a/doc\n+++ b/doc\n@@ -1,3 +1,3 @@\n alpha\r\n-beta\r\n+BETA\r\n gamma\r\n\\ No newline at end of file\n"; - const lines = parseRawPatchLines(patch); - // 4 content lines (alpha, -beta, +BETA, gamma) + 1 EOF marker; header skipped. - expect(lines.length).toBe(5); - expect(lines[0].content).toContain("\r"); - expect(lines[4].isEofMarker).toBe(true); - expect(lines[0].oldLine).toBe(1); - expect(lines[0].newLine).toBe(1); - }); - - test("sliceRawHunkPatch isolates the target hunk with original bytes", () => { - const patch = "diff --git a/doc b/doc\n--- a/doc\n+++ b/doc\n@@ -1,3 +1,3 @@\n alpha\r\n-beta\r\n+BETA\r\n gamma\r\n"; - const sliced = sliceRawHunkPatch(patch, "doc", 2, 2); - expect(sliced).toBeTruthy(); - expect(sliced!).toContain("-beta\r"); - expect(sliced!).toContain("+BETA\r"); - // U0: context lines are stripped from the output hunk. - expect(sliced!).not.toContain(" alpha"); - }); - - test("buildByteFaithfulHunk carries CR content in the hunk lines", () => { - const diff = { - file_path: "doc.txt", is_new: false, is_deleted: false, is_renamed: false, - has_lossy_line_endings: true, - writePatch: "diff --git a/doc.txt b/doc.txt\n--- a/doc.txt\n+++ b/doc.txt\n@@ -1,3 +1,3 @@\n alpha\r\n-beta\r\n+BETA\r\n gamma\r\n", - lines: [], - }; - const hunk = buildByteFaithfulHunk(diff as never, 2, 2); - expect(hunk).toBeTruthy(); - expect(hunk!.lines.some((line) => line.content.includes("\r"))).toBe(true); - }); -}); - -describe("byte-faithful rollback with real Git", () => { - test("autocrlf=false: CRLF file, rollback restores index content with CR preserved", () => { - const dir = makeRepo("crlf", { autocrlf: "false" }); - repos.push(dir); - const file = join(dir, "doc.txt"); - - // HEAD: all CRLF - writeFileSync(file, "alpha\r\nbravo\r\ncharlie\r\ndelta\r\necho\r\nfoxtrot\r\ngolf\r\nhotel\r\n"); - git(dir, "add", "-A"); - git(dir, "commit", "-qm", "initial"); - - // Index: line 4 changed to STAGED (partial stage) - writeFileSync(file, "alpha\r\nbravo\r\ncharlie\r\nSTAGED\r\necho\r\nfoxtrot\r\ngolf\r\nhotel\r\n"); - git(dir, "add", "doc.txt"); - - // Worktree: line 2 also changed (unstaged), line 4 further changed - writeFileSync(file, "alpha\r\nBRAVO-WT\r\ncharlie\r\nWORKTREE\r\necho\r\nfoxtrot\r\ngolf\r\nhotel\r\n"); - - const staged = readFileSync(file); - - // Extract the worktree diff's raw patch (index → worktree) - const rawPatch = git(dir, "diff", "--", "doc.txt"); - expect(rawPatch).toContain("\r"); - - // Slice the hunk containing line 2 (the unstaged BRAVO change) - const sliced = sliceRawHunkPatch(rawPatch, "doc.txt", 2, 2); - expect(sliced).toBeTruthy(); - - // Reverse-apply the sliced patch (the rollback operation) - // Write the patch to a temp file and apply - const patchFile = join(dir, ".test-patch"); - writeFileSync(patchFile, sliced!); - git(dir, "apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", ".test-patch"); - - const result = readFileSync(file); - // Line 2 restored to index version (bravo), line 4 keeps WORKTREE - expect(result.includes("bravo\r\n")).toBe(true); - expect(result.includes("WORKTREE\r\n")).toBe(true); - expect(result.includes("BRAVO-WT")).toBe(false); - // CR bytes preserved throughout - expect(result.equals( - Buffer.from("alpha\r\nbravo\r\ncharlie\r\nWORKTREE\r\necho\r\nfoxtrot\r\ngolf\r\nhotel\r\n"), - )).toBe(true); - - // Index unchanged - const indexContent = git(dir, "show", ":doc.txt"); - expect(indexContent).toContain("STAGED"); - }); - - test("autocrlf=true: Git converts LF patch to CRLF worktree faithfully", () => { - const dir = makeRepo("autocrlf", { autocrlf: "true" }); - repos.push(dir); - const file = join(dir, "doc.txt"); - - writeFileSync(file, "alpha\r\nbravo\r\ncharlie\r\ndelta\r\necho\r\n"); - git(dir, "add", "-A"); - git(dir, "commit", "-qm", "initial"); - - // Change line 2 - writeFileSync(file, "alpha\r\nBRAVO\r\ncharlie\r\ndelta\r\necho\r\n"); - - // Git may output the patch with LF (converted), but git apply converts back - const rawPatch = git(dir, "diff", "--", "doc.txt"); - const sliced = sliceRawHunkPatch(rawPatch, "doc.txt", 2, 2); - expect(sliced).toBeTruthy(); - - const patchFile = join(dir, ".test-patch"); - writeFileSync(patchFile, sliced!); - git(dir, "apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", ".test-patch"); - - const result = readFileSync(file); - expect(result.includes("bravo")).toBe(true); - expect(result.includes("BRAVO")).toBe(false); - // The file maintains its line ending convention - expect(result.includes("\r\n")).toBe(true); - }); - - test("gitattributes eol=crlf: rollback preserves CRLF", () => { - const dir = makeRepo("attrs", { autocrlf: "false", gitattributes: "*.txt text eol=crlf\n" }); - repos.push(dir); - const file = join(dir, "doc.txt"); - - writeFileSync(file, "one\r\ntwo\r\nthree\r\nfour\r\nfive\r\n"); - git(dir, "add", "-A"); - git(dir, "commit", "-qm", "initial"); - - writeFileSync(file, "one\r\nTWO-CHANGED\r\nthree\r\nfour\r\nfive\r\n"); - - const rawPatch = git(dir, "diff", "--", "doc.txt"); - const sliced = sliceRawHunkPatch(rawPatch, "doc.txt", 2, 2); - expect(sliced).toBeTruthy(); - - const patchFile = join(dir, ".test-patch"); - writeFileSync(patchFile, sliced!); - git(dir, "apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", ".test-patch"); - - const result = readFileSync(file); - expect(result.equals(Buffer.from("one\r\ntwo\r\nthree\r\nfour\r\nfive\r\n"))).toBe(true); - }); - - test("EOF marker in target block restores the missing newline", () => { - const dir = makeRepo("eof", { autocrlf: "false" }); - repos.push(dir); - const file = join(dir, "doc.txt"); - - // HEAD has trailing newline - writeFileSync(file, "alpha\nbravo\ncharlie\n"); - git(dir, "add", "-A"); - git(dir, "commit", "-qm", "initial"); - - // Worktree: line 3 changed AND trailing newline removed - writeFileSync(file, "alpha\nbravo\nCHARLIE-NO-EOL"); - - const rawPatch = git(dir, "diff", "--", "doc.txt"); - // Verify the patch has the EOF marker - expect(rawPatch).toContain("\\ No newline at end of file"); - - const sliced = sliceRawHunkPatch(rawPatch, "doc.txt", 3, 3); - expect(sliced).toBeTruthy(); - expect(sliced!).toContain("\\ No newline"); - - const patchFile = join(dir, ".test-patch"); - writeFileSync(patchFile, sliced!); - git(dir, "apply", "--reverse", "--unidiff-zero", "--whitespace=nowarn", ".test-patch"); - - const result = readFileSync(file); - // Line 3 restored to index version WITH the trailing newline - expect(result.equals(Buffer.from("alpha\nbravo\ncharlie\n"))).toBe(true); - }); -}); - -describe("attribute safety check", () => { - test("clean attributes pass the safety check", async () => { - const safety = await checkHunkRollbackAttributes("C:/repo", "doc.txt"); - expect(safety.byteSafe).toBe(true); - }); -}); diff --git a/windows/tauri/src/features/git/utils/lossy-rollback-patch.ts b/windows/tauri/src/features/git/utils/lossy-rollback-patch.ts deleted file mode 100644 index ef531b86f6..0000000000 --- a/windows/tauri/src/features/git/utils/lossy-rollback-patch.ts +++ /dev/null @@ -1,351 +0,0 @@ -import type { GitDiff, GitDiffLine, GitHunk } from "../types/git.types"; - -/** One raw patch line with its original bytes (including CR) intact. */ -interface RawPatchLine { - /** Prefix character: ' ', '+', '-', or '\\' for the no-newline marker. */ - prefix: string; - /** Line content after the prefix, with original line endings preserved. */ - content: string; - /** True for `\\ No newline at end of file` markers. */ - isEofMarker: boolean; - /** One-based old-side line number, when the prefix occupies an old line. */ - oldLine: number | null; - /** One-based new-side line number, when the prefix occupies a new line. */ - newLine: number | null; -} - -/** - * Parses a unified diff patch into raw lines with original bytes. Unlike the - * display parser, this never strips CR or skips EOF markers — the whole point - * is byte fidelity for `git apply`. - */ -export function parseRawPatchLines(patch: string): RawPatchLine[] { - const result: RawPatchLine[] = []; - let oldLine = 0; - let newLine = 0; - let inHunk = false; - - for (const raw of patch.split("\n")) { - if (raw.startsWith("@@")) { - inHunk = true; - const match = raw.match(/@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/); - if (match) { - oldLine = parseInt(match[1], 10); - newLine = parseInt(match[3], 10); - } - continue; - } - if (!inHunk) continue; - if (raw === "") continue; - - if (raw.startsWith("\\ No newline")) { - // The marker belongs to the previous line's old side, new side, or - // both, but never occupies a source line number itself. - result.push({ prefix: "\\", content: raw, isEofMarker: true, oldLine: null, newLine: null }); - continue; - } - - const prefix = raw[0]; - const content = raw.slice(1); - const entry: RawPatchLine = { - prefix, - content, - isEofMarker: false, - oldLine: null, - newLine: null, - }; - if (prefix === " " || prefix === "-") { - entry.oldLine = oldLine++; - } - if (prefix === " " || prefix === "+") { - entry.newLine = newLine++; - } - result.push(entry); - } - return result; -} - -/** - * Extracts a single hunk's raw patch text covering the target old/new line - * range from a full raw patch. The result preserves CR bytes and EOF markers - * so `git apply` can reverse it byte-faithfully. - * - * When targetOldEnd/targetNewEnd are provided, the extracted contiguous run - * is verified not to extend beyond the target range — a run that would affect - * changes outside the selected block returns null (conservative disable). - * - * @returns null when the range cannot be unambiguously isolated. - */ -export function sliceRawHunkPatch( - patch: string, - filePath: string, - targetOldStart: number, - targetNewStart: number, - targetOldEnd?: number, - targetNewEnd?: number, -): string | null { - const lines = parseRawPatchLines(patch); - if (lines.length === 0) return null; - - // Build a minimal U0 hunk: only the changed lines covering the target, - // context stripped, original bytes (CR, EOF markers) preserved. This pairs - // with git apply --unidiff-zero for byte-faithful reverse application. - // A replacement block is a contiguous run of `-` and `+` lines with no - // context line in between; the target identifies which block by its - // one-based new-side line (for replacements and insertions) or old-side - // line (for pure deletions, which have no new-side lines). - let blockStart = -1; - let blockEnd = -1; - let currentStart = -1; - let found = false; - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - if (line.isEofMarker) continue; - if (line.prefix === "+" || line.prefix === "-") { - if (currentStart === -1) currentStart = i; - if (!found) { - // Match by new-side line for replacement/insertion blocks, or by - // old-side line for pure deletion blocks (newLine is null). - const newMatch = line.newLine !== null && line.newLine === targetNewStart; - const oldMatch = - line.newLine === null && line.oldLine !== null && line.oldLine === targetOldStart; - if (newMatch || oldMatch) { - found = true; - blockStart = currentStart; - } - } - } else { - // Context line terminates the current run. - if (found && currentStart !== -1) { - blockEnd = i - 1; - break; - } - currentStart = -1; - } - } - if (found && blockEnd === -1) blockEnd = lines.length - 1; - if (!found || blockStart === -1) return null; - - // Range verification: when target endpoints are provided, the contiguous - // run must not extend beyond them in either direction — a run that - // includes changes outside the selected block would roll back content - // the user did not choose. Additionally, the run must COVER all changed - // lines within the target range: when the range contains separated - // changes, a single contiguous run cannot represent them and the caller - // needs the range-based builder instead. - if (targetNewEnd !== undefined && targetOldEnd !== undefined) { - for (let i = blockStart; i <= blockEnd; i++) { - const line = lines[i]; - if (line.isEofMarker) continue; - if (line.newLine !== null && (line.newLine > targetNewEnd || line.newLine < targetNewStart)) return null; - if (line.oldLine !== null && (line.oldLine > targetOldEnd || line.oldLine < targetOldStart)) return null; - } - // Coverage check: every changed line in the raw patch within the target - // range must be inside this contiguous run. If any is outside (separated - // by context from this run), the single-run slicer is insufficient. - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - if (line.isEofMarker || (line.prefix !== "+" && line.prefix !== "-")) continue; - const inTargetRange = - (line.oldLine !== null && line.oldLine >= targetOldStart && line.oldLine <= targetOldEnd) || - (line.newLine !== null && line.newLine >= targetNewStart && line.newLine <= targetNewEnd); - if (inTargetRange && (i < blockStart || i > blockEnd)) return null; - } - } - - // Include trailing EOF markers attached to the last changed line. - while (blockEnd + 1 < lines.length && lines[blockEnd + 1].isEofMarker) { - blockEnd++; - } - - const hunkLines = lines.slice(blockStart, blockEnd + 1); - if (hunkLines.length === 0) return null; - - // U0 hunk: count only changed lines for each side. - let oldCount = 0; - let newCount = 0; - let firstOldLine: number | null = null; - let firstNewLine: number | null = null; - for (const line of hunkLines) { - if (line.isEofMarker) continue; - if (line.prefix === "-") { - oldCount++; - if (firstOldLine === null && line.oldLine !== null) firstOldLine = line.oldLine; - } - if (line.prefix === "+") { - newCount++; - if (firstNewLine === null && line.newLine !== null) firstNewLine = line.newLine; - } - } - // When one side has zero lines, git uses the position-before convention: - // the hunk header's start is the line number preceding the insertion or - // the deletion boundary. - const oldStart = oldCount === 0 - ? Math.max(0, (firstOldLine ?? targetOldStart) - 1) - : (firstOldLine ?? targetOldStart); - const newStart = newCount === 0 - ? Math.max(0, (firstNewLine ?? targetNewStart) - 1) - : (firstNewLine ?? targetNewStart); - - const header = `@@ -${oldStart},${oldCount} +${newStart},${newCount} @@`; - const body = hunkLines.map((line) => (line.isEofMarker ? line.content : line.prefix + line.content)); - - return `diff --git a/${filePath} b/${filePath}\n--- a/${filePath}\n+++ b/${filePath}\n${header}\n${body.join("\n")}\n`; -} - -/** - * Builds a GitHunk whose lines carry original bytes (CR, EOF markers) from - * the writePatch, so the host-side patch reconstruction is byte-faithful. - * When target endpoints are provided, the slice is verified not to extend - * beyond them (a contiguous run crossing block boundaries returns null). - */ -export function buildByteFaithfulHunk( - diff: GitDiff, - targetOldStart: number, - targetNewStart: number, - targetOldEnd?: number, - targetNewEnd?: number, -): GitHunk | null { - if (!diff.writePatch) return null; - const patch = sliceRawHunkPatch( - diff.writePatch, - diff.new_path || diff.file_path, - targetOldStart, - targetNewStart, - targetOldEnd, - targetNewEnd, - ); - if (!patch) return null; - const rawLines = parseRawPatchLines(patch); - const hunkLines: GitDiffLine[] = rawLines - .filter((line) => !line.isEofMarker && line.prefix !== "@") - .map((line) => ({ - line_type: line.prefix === "+" ? "added" : line.prefix === "-" ? "removed" : "context", - content: line.content, - old_line_number: line.oldLine ?? undefined, - new_line_number: line.newLine ?? undefined, - })); - if (hunkLines.length === 0) return null; - return { - file_path: diff.new_path || diff.file_path, - lines: hunkLines, - // The host checks rawPatch to bypass display-line reconstruction; without - // it, EOF markers embedded in the patch text would be lost. - rawPatch: patch, - }; -} - -/** - * Builds a byte-faithful GitHunk from ALL changed raw-patch lines within the - * given one-based range, regardless of contiguity. Used when a selection - * spans multiple separated change runs (e.g. insert + nearby replacement). - */ -export function buildFaithfulHunkFromRange( - diff: GitDiff, - oldStart: number, - newStart: number, - oldEnd: number, - newEnd: number, -): GitHunk | null { - if (!diff.writePatch) return null; - const lines = parseRawPatchLines(diff.writePatch); - const inRange = lines.filter((line) => { - if (line.isEofMarker) return false; - if (line.prefix !== "+" && line.prefix !== "-") return false; - const oldOk = line.oldLine === null || (line.oldLine >= oldStart && line.oldLine <= oldEnd); - const newOk = line.newLine === null || (line.newLine >= newStart && line.newLine <= newEnd); - return oldOk && newOk; - }); - if (inRange.length === 0) return null; - // Preserve original ordering: include EOF markers adjacent to their - //所属 changed line in the raw patch, never appended after other lines. - // An old-side no-newline marker must stay after its `-` line, not after - // a following `+` line, or reverse-application restores the wrong side. - const ordered = lines.filter((line) => { - if (inRange.includes(line)) return true; - if (line.isEofMarker) { - const idx = lines.indexOf(line); - return idx > 0 && inRange.includes(lines[idx - 1]); - } - return false; - }); - const filePath = diff.new_path || diff.file_path; - - // Group into contiguous runs (adjacent in the original patch, no context - // gap) — each run becomes its own @@ hunk so separated changes apply - // correctly instead of sharing one wrong-range header. - const runs: RawPatchLine[][] = []; - let currentRun: RawPatchLine[] = []; - for (const line of ordered) { - if (line.isEofMarker) { - if (currentRun.length > 0) currentRun.push(line); - continue; - } - const prev = currentRun[currentRun.length - 1]; - // Adjacent if both are changed lines from the same contiguous segment - // (their positions in the original `lines` array are consecutive). - const prevIdx = prev ? lines.indexOf(prev) : -1; - const currIdx = lines.indexOf(line); - if (prev && !prev.isEofMarker && currIdx === prevIdx + 1) { - currentRun.push(line); - } else { - if (currentRun.length > 0) runs.push(currentRun); - currentRun = [line]; - } - } - if (currentRun.length > 0) runs.push(currentRun); - - // Build one @@ header per run. - const hunkHeaders: string[] = []; - const allBodyLines: string[] = []; - for (const run of runs) { - let oldCount = 0; - let newCount = 0; - let firstOld: number | null = null; - let firstNew: number | null = null; - for (const line of run) { - if (line.isEofMarker) continue; - if (line.prefix === "-") { - oldCount++; - if (firstOld === null && line.oldLine !== null) firstOld = line.oldLine; - } - if (line.prefix === "+") { - newCount++; - if (firstNew === null && line.newLine !== null) firstNew = line.newLine; - } - } - const oStart = oldCount === 0 ? Math.max(0, (firstOld ?? oldStart) - 1) : (firstOld ?? oldStart); - const nStart = newCount === 0 ? Math.max(0, (firstNew ?? newStart) - 1) : (firstNew ?? newStart); - hunkHeaders.push(`@@ -${oStart},${oldCount} +${nStart},${newCount} @@`); - for (const line of run) { - allBodyLines.push(line.isEofMarker ? line.content : line.prefix + line.content); - } - } - - // Assemble the multi-hunk patch. - const parts: string[] = [`diff --git a/${filePath} b/${filePath}`, `--- a/${filePath}`, `+++ b/${filePath}`]; - for (let i = 0; i < hunkHeaders.length; i++) { - parts.push(hunkHeaders[i]); - } - // Interleave headers with their body lines correctly. - const patchLines: string[] = [`diff --git a/${filePath} b/${filePath}`, `--- a/${filePath}`, `+++ b/${filePath}`]; - let bodyIdx = 0; - for (const run of runs) { - const hdr = hunkHeaders[runs.indexOf(run)]; - patchLines.push(hdr); - for (const line of run) { - patchLines.push(line.isEofMarker ? line.content : line.prefix + line.content); - } - } - const patch = patchLines.join("\n") + "\n"; - - const hunkLines: GitDiffLine[] = ordered - .filter((line) => !line.isEofMarker) - .map((line) => ({ - line_type: line.prefix === "+" ? "added" : "removed", - content: line.content, - old_line_number: line.oldLine ?? undefined, - new_line_number: line.newLine ?? undefined, - })); - return { file_path: filePath, lines: hunkLines, rawPatch: patch }; -} diff --git a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts index 18ca6faeb8..95e59f82ad 100644 --- a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts +++ b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts @@ -15,6 +15,10 @@ const diff: GitDiff = { { line_type: "removed", content: "oldTail", old_line_number: 502 }, { line_type: "added", content: "newTail", new_line_number: 502 }, ], + core_hunks: [ + { id: "hunk-0", header: "@@ -91,2 +91,2 @@", patch: "core patch 0\n" }, + { id: "hunk-1", header: "@@ -501,2 +501,2 @@", patch: "core patch 1\n" }, + ], }; const context = { repoPath: "C:/workspace/nested-repo", isStaged: false }; function operations() { @@ -37,7 +41,7 @@ function deferred() { } // A full-context review of a CRLF file whose display lines lost their CR: -// discard is sliced from the raw writePatch into a zero-context hunk (`rawPatch`). +// every action writes Core's byte-faithful hunk patch verbatim. const lossyDiff: GitDiff = { file_path: "f.txt", is_new: false, is_deleted: false, is_renamed: false, has_lossy_line_endings: true, is_full_context: true, @@ -48,55 +52,41 @@ const lossyDiff: GitDiff = { { line_type: "added", content: "B", new_line_number: 2 }, { line_type: "context", content: "c", old_line_number: 3, new_line_number: 3 }, ], - writePatch: "diff --git a/f.txt b/f.txt\n--- a/f.txt\n+++ b/f.txt\n@@ -1,3 +1,3 @@\n a\r\n-b\r\n+B\r\n c\r\n", + core_hunks: [{ + id: "hunk-0", header: "@@ -1,3 +1,3 @@", lineStart: 1, lineEnd: 5, + patch: "diff --git a/f.txt b/f.txt\n--- a/f.txt\n+++ b/f.txt\n@@ -1,3 +1,3 @@\n a\r\n-b\r\n+B\r\n c\r\n", + }], }; -describe("zero-context discard probe", () => { - const discardHeader = (target: GitDiff) => { +describe("Core-owned hunk patches", () => { + const hunkID = (target: GitDiff) => { const rows = monacoDiffRows(target); return (rows.find(row => row.actionAnchor) ?? rows.find(row => row.hunkID))!.hunkID!; }; - // Regression: ordinary hunks keep their context lines for git apply; probing - // them re-read a differently shaped diff and refused every rollback. - test("an ordinary context hunk discards without running the probe", async () => { - const api = { ...rollbackOperations(), validate: mock(async () => false) }; + test("a sparse hunk is written with the patch Core returned for its header", async () => { + const api = rollbackOperations(); const owner = createMonacoDiffHunkActions(diff, discardContext, api); - expect(await owner.apply(discardHeader(diff), "discard")).toBe("applied"); - expect(api.validate).not.toHaveBeenCalled(); - expect(api.discard.mock.calls[0]![1].rawPatch).toBeUndefined(); + expect(await owner.apply("hunk-4", "discard")).toBe("applied"); + expect(api.discard.mock.calls[0]![1].rawPatch).toBe("core patch 1\n"); }); - test("a confirmed zero-context hunk is written with its raw patch", async () => { - const api = { ...rollbackOperations(), validate: mock(async (_hunk: GitHunk) => true) }; + test("a complete-file hunk of a CRLF file keeps its original bytes for every action", async () => { + const api = rollbackOperations(); const owner = createMonacoDiffHunkActions(lossyDiff, discardContext, api); - expect(await owner.apply(discardHeader(lossyDiff), "discard")).toBe("applied"); - const written = api.discard.mock.calls[0]![1]; - expect(written.rawPatch).toContain("-b\r\n+B\r\n"); - expect(api.validate.mock.calls[0]![0]).toBe(written); + expect(owner.actions).toEqual(["stage", "discard"]); + expect(await owner.apply(hunkID(lossyDiff), "discard")).toBe("applied"); + expect(api.discard.mock.calls[0]![1].rawPatch).toContain("-b\r\n+B\r\n"); }); - for (const [name, validate, expected] of [ - ["a failed probe", async () => false, "stale"], - ["a missing probe", undefined, "stale"], - ["a throwing probe", async () => { throw new Error("git unavailable"); }, "failed"], - ] as const) { - test(`${name} refuses the zero-context write`, async () => { - const api = { ...rollbackOperations(), ...(validate ? { validate } : {}) }; - const owner = createMonacoDiffHunkActions(lossyDiff, discardContext, api); - expect(await owner.apply(discardHeader(lossyDiff), "discard")).toBe(expected); - expect(api.discard).not.toHaveBeenCalled(); - }); - } - - test("a stale refusal does not lock the owner against a retry", async () => { - let current = false; - const api = { ...rollbackOperations(), validate: mock(async () => current) }; - const owner = createMonacoDiffHunkActions(lossyDiff, discardContext, api); - expect(await owner.apply(discardHeader(lossyDiff), "discard")).toBe("stale"); - current = true; - expect(await owner.apply(discardHeader(lossyDiff), "discard")).toBe("applied"); - expect(api.discard).toHaveBeenCalledTimes(1); + test("a hunk Core offered no patch for is never written", async () => { + const api = rollbackOperations(); + const bare = { ...diff, core_hunks: undefined }; + const owner = createMonacoDiffHunkActions(bare, discardContext, api); + expect(await owner.apply("hunk-0", "stage")).toBe("ignored"); + expect(await owner.apply("hunk-0", "discard")).toBe("ignored"); + expect(api.stage).not.toHaveBeenCalled(); + expect(api.discard).not.toHaveBeenCalled(); }); }); @@ -123,7 +113,7 @@ describe("Windows Monaco diff hunk actions", () => { expect(owner.action).toBe(action); expect(await owner.apply(header.hunkID!, action)).toBe("applied"); expect(api[action]).toHaveBeenCalledWith(context.repoPath, { - file_path: "src/Sample.java", lines: diff.lines.slice(4), + file_path: "src/Sample.java", lines: diff.lines.slice(4), rawPatch: "core patch 1\n", }); expect(api[other]).not.toHaveBeenCalled(); owner.dispose(); @@ -148,7 +138,7 @@ describe("Windows Monaco diff hunk actions", () => { const refreshed = createMonacoDiffHunkActions({ ...diff, lines: diff.lines.slice(4) }, context, api); expect(await refreshed.apply("hunk-0", "stage")).toBe("applied"); expect(api.stage).toHaveBeenLastCalledWith(context.repoPath, { - file_path: diff.file_path, lines: diff.lines.slice(4), + file_path: diff.file_path, lines: diff.lines.slice(4), rawPatch: "core patch 1\n", }); refreshed.dispose(); }); @@ -179,7 +169,7 @@ describe("Windows Monaco diff hunk actions", () => { expect(await inFlight).toBe("ignored"); expect(await current.apply("hunk-4", "unstage")).toBe("applied"); expect(api.stage).toHaveBeenCalledTimes(1); - expect(api.unstage).toHaveBeenCalledWith("D:/other", { file_path: diff.file_path, lines: diff.lines.slice(4) }); + expect(api.unstage).toHaveBeenCalledWith("D:/other", { file_path: diff.file_path, lines: diff.lines.slice(4), rawPatch: "core patch 1\n" }); current.dispose(); expect(await current.apply("hunk-0", "unstage")).toBe("ignored"); expect(api.unstage).toHaveBeenCalledTimes(1); @@ -209,7 +199,7 @@ describe("Windows Monaco diff hunk actions", () => { const owner = createMonacoDiffHunkActions(diff, discardContext, api); expect(owner.actions).toEqual(["stage", "discard"]); expect(await owner.apply("hunk-4", "discard")).toBe("applied"); - const hunk = { file_path: "src/Sample.java", lines: diff.lines.slice(4) }; + const hunk = { file_path: "src/Sample.java", lines: diff.lines.slice(4), rawPatch: "core patch 1\n" }; expect(api.discard).toHaveBeenCalledWith(context.repoPath, hunk); expect(api.stage).not.toHaveBeenCalled(); // The patch is stale until the Git change event replaces it. @@ -223,6 +213,8 @@ describe("Windows Monaco diff hunk actions", () => { const full: GitDiff = { ...diff, is_full_context: true, + core_hunks: [{ id: "hunk-0", header: "@@ -7,6 +7,6 @@", patch: "core patch full\n", + lineStart: 7, lineEnd: 14 }], lines: [ { line_type: "header", content: "@@ -1,12 +1,12 @@" }, ...Array.from({ length: 12 }, (_, index) => index === 9 @@ -239,6 +231,7 @@ describe("Windows Monaco diff hunk actions", () => { expect(api.discard).toHaveBeenCalledWith(context.repoPath, { file_path: "src/Sample.java", lines: [{ line_type: "header", content: "@@ -7,6 +7,6 @@" }, ...full.lines.slice(7, 14)], + rawPatch: "core patch full\n", }); owner.dispose(); }); @@ -346,7 +339,6 @@ describe("Windows Monaco diff hunk actions", () => { for (const disabled of [ createMonacoDiffHunkActions(diff, undefined, api), createMonacoDiffHunkActions({ ...diff, is_truncated: true }, context, api), - createMonacoDiffHunkActions({ ...diff, has_lossy_line_endings: true }, context, api), ]) { expect(disabled.action).toBeNull(); expect(await disabled.apply("hunk-0", "stage")).toBe("ignored"); diff --git a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts index ee02db11c6..5c08108a2a 100644 --- a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts +++ b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts @@ -38,12 +38,6 @@ export interface MonacoDiffHunkOperations { /** Asks the user to confirm a discard when the setting is enabled; the * component supplies the dialog and pending gate. */ confirmDiscard?: (path: string) => Promise; - /** Pre-write staleness probe for zero-context hunks (`rawPatch`): resolves - * false when the patch the hunk was sliced from no longer matches the - * repository. `git apply --unidiff-zero` has no surrounding context to - * reject a relocated change, so the surface verifies currency itself. - * Ordinary hunks keep their context lines and rely on git apply. */ - validate?: (hunk: GitHunk) => Promise; } /** Discard rewrites working-tree content. New and deleted files have no @@ -61,16 +55,12 @@ export function createMonacoDiffHunkActions( operations: MonacoDiffHunkOperations, ) { const writable = Boolean(context?.repoPath) && !diff.is_truncated; - // Stage/unstage keep the lossy protection unconditionally: their write - // hunks come from display lines which lack original bytes. - const toggleAction: "stage" | "unstage" | null = writable && !diff.has_lossy_line_endings && context + // Hunks carry Core-owned patches with original bytes; without one an + // action stays unavailable. Core re-checks attributes before a rollback. + const toggleAction: "stage" | "unstage" | null = writable && context ? context.isStaged ? "unstage" : "stage" : null; - // Discard rides the byte-faithful channel: monacoDiffHunk slices from - // writePatch when available, so lossy files can discard with original - // bytes. Without writePatch, lossy files keep the protective fallback. const discardEnabled = writable && !!context && !context.isStaged && context.canDiscard === true - && !!operations.discard && canDiscardDiff(diff) - && (!diff.has_lossy_line_endings || Boolean(diff.writePatch)); + && !!operations.discard && canDiscardDiff(diff); const actions: readonly HunkActionID[] = toggleAction ? discardEnabled ? [toggleAction, "discard"] : [toggleAction] : discardEnabled ? ["discard"] : []; @@ -85,7 +75,8 @@ export function createMonacoDiffHunkActions( if (disposed || pending || applied || !context || !(actions as readonly string[]).includes(requestedAction)) return "ignored"; const hunk = monacoDiffHunk(diff, hunkID); - if (!hunk) return "ignored"; + // Only Core's patch may be written; a hunk without one is read-only. + if (!hunk?.rawPatch) return "ignored"; const requested = requestedAction as HunkActionID; // Duplicate clicks stay locked from here until the write completes. pending = true; @@ -99,14 +90,6 @@ export function createMonacoDiffHunkActions( if (disposed) return "ignored"; if (!confirmed) return "cancelled"; } - // A zero-context discard has no git-apply context check, so it writes - // only after the probe confirms its source patch is still current. A - // failed or missing probe refuses rather than risk the wrong lines. - if (requested === "discard" && hunk.rawPatch) { - const current = operations.validate ? await operations.validate(hunk) : false; - if (disposed) return "ignored"; - if (!current) return "stale"; - } const operation = requested === "discard" ? operations.discard! : operations[requested]; const success = await operation(context.repoPath, hunk); if (disposed) return "ignored"; diff --git a/windows/tauri/src/features/git/utils/monaco-diff-rows.test.ts b/windows/tauri/src/features/git/utils/monaco-diff-rows.test.ts index 9f705ca5d6..0007581a60 100644 --- a/windows/tauri/src/features/git/utils/monaco-diff-rows.test.ts +++ b/windows/tauri/src/features/git/utils/monaco-diff-rows.test.ts @@ -13,7 +13,7 @@ const fileLines = (from: number, to: number) => // Both patches are real `git diff` output for one edit of a 40-line file: // `--unified=1073741823` for the review, and Git's default `--unified=3`. -const fullContext: GitDiff = { +const fullContextBase: GitDiff = { ...parse([ "@@ -1,40 +1,40 @@", " line 1", "-line 2", "+line 2 changed", ...fileLines(3, 12), "+inserted after 12", @@ -22,6 +22,16 @@ const fullContext: GitDiff = { ].join("\n")), is_full_context: true, }; +// Core splits the complete-file patch at Git's default context and owns each +// patch; this mirrors only the line ranges Core reports for this fixture. +const coreRanges = [[1, 7], [11, 25], [37, 45]] as const; +const fullContext: GitDiff = { + ...fullContextBase, + core_hunks: ["@@ -1,5 +1,5 @@", "@@ -10,13 +10,13 @@", "@@ -35,6 +35,6 @@"].map((header, index) => ({ + id: `hunk-${index}`, header, patch: `core patch ${index}\n`, + lineStart: coreRanges[index][0], lineEnd: coreRanges[index][1], + })), +}; const gitDefaultHunks = parse([ "@@ -1,5 +1,5 @@", " line 1", "-line 2", "+line 2 changed", ...fileLines(3, 5), "@@ -10,13 +10,13 @@ line 9", ...fileLines(10, 12), "+inserted after 12", ...fileLines(13, 18), @@ -74,7 +84,7 @@ describe("full-context Monaco diff rows", () => { test("anchor one action band per hunk and leave distant context unowned", () => { const rows = monacoDiffRows(fullContext); const anchors = rows.filter(row => row.actionAnchor); - expect(anchors.map(row => row.hunkID)).toEqual(["hunk-2", "hunk-14", "hunk-40"]); + expect(anchors.map(row => row.hunkID)).toEqual(["hunk-0", "hunk-1", "hunk-2"]); expect(rows.find(row => row.oldLine === 30)?.hunkID).toBeNull(); }); @@ -90,13 +100,17 @@ describe("full-context Monaco diff rows", () => { ? { line_type: "header" as const, content: line.content.replace(/ @@.*$/, " @@") } : line), }; - expect(derived).toEqual(expected.map(withoutHint)); + expect(derived.map(hunk => hunk && { ...hunk, rawPatch: undefined })).toEqual( + expected.map(withoutHint).map(hunk => hunk && { ...hunk, rawPatch: undefined })); + expect(derived.map(hunk => hunk?.rawPatch)).toEqual(["core patch 0\n", "core patch 1\n", "core patch 2\n"]); }); test("name the preceding line for a hunk with an empty side", () => { const added = parse("@@ -0,0 +1,2 @@\n+first\n+second\n"); - const hunk = monacoDiffHunk({ ...added, is_full_context: true }, "hunk-1"); + const hunk = monacoDiffHunk({ ...added, is_full_context: true, core_hunks: [{ + id: "hunk-0", header: "@@ -0,0 +1,2 @@", patch: "core patch\n", lineStart: 1, lineEnd: 3 }] }, "hunk-0"); expect(hunk?.lines[0]).toEqual({ line_type: "header", content: "@@ -0,0 +1,2 @@" }); + expect(hunk?.rawPatch).toBe("core patch\n"); }); test("keep sparse-patch hunks when the patch is not marked full-context", () => { @@ -106,9 +120,9 @@ describe("full-context Monaco diff rows", () => { }); test("reject identities that do not start a derived hunk", () => { - for (const id of ["hunk-0", "hunk-3", "hunk-30", "line-2"]) { + for (const id of ["hunk-3", "hunk-30", "line-2"]) { expect(monacoDiffHunk(fullContext, id)).toBeNull(); } - expect(monacoDiffHunk({ ...fullContext, is_truncated: true }, "hunk-2")).toBeNull(); + expect(monacoDiffHunk({ ...fullContext, is_truncated: true }, "hunk-0")).toBeNull(); }); }); diff --git a/windows/tauri/src/features/git/utils/monaco-diff-rows.ts b/windows/tauri/src/features/git/utils/monaco-diff-rows.ts index 02bfb14652..a81f79f63d 100644 --- a/windows/tauri/src/features/git/utils/monaco-diff-rows.ts +++ b/windows/tauri/src/features/git/utils/monaco-diff-rows.ts @@ -1,13 +1,8 @@ import type { ReviewRow } from "@lithe/editor/diff-review"; import type { GitDiff, GitDiffLine, GitHunk } from "../types/git.types"; import { parseDiffHunkRange } from "./git-diff-helpers"; -import { buildByteFaithfulHunk, buildFaithfulHunkFromRange } from "./lossy-rollback-patch"; -/** Context kept around each change when a full-file patch is split back into - * stageable hunks. Matches Git's default `--unified=3`. */ -const FULL_CONTEXT_HUNK_CONTEXT_LINES = 3; - -/** A stageable range of a full-context patch, as indices into `diff.lines`. */ +/** A Core-split Git-default hunk of a complete-file patch, as indices into `diff.lines`. */ interface FullContextHunk { id: string; /** First changed line; the hunk's action band is rendered above it. */ @@ -15,51 +10,31 @@ interface FullContextHunk { start: number; /** Exclusive end index. */ end: number; + header: string; + patch: string; } const isChange = (line: GitDiffLine) => line.line_type === "added" || line.line_type === "removed"; -/** Splits a full-context patch the way Git would at three context lines, so - * staging granularity does not grow to the whole file. Returns null for sparse - * patches, which keep the host's own hunks. */ +/** Reads the stageable hunks Core split from a complete-file patch. Core owns + * both the split and each hunk's patch bytes; this only locates them in the + * display lines. Returns null for sparse patches, which keep the host's own + * hunks, and an empty list when Core offered no writable hunk. */ function fullContextHunks(diff: GitDiff): FullContextHunk[] | null { const { lines } = diff; if (!diff.is_full_context || lines[0]?.line_type !== "header" || lines.some((line, index) => index > 0 && line.line_type === "header")) return null; - const context = FULL_CONTEXT_HUNK_CONTEXT_LINES; const hunks: FullContextHunk[] = []; - for (let index = 1; index < lines.length; index++) { - if (!isChange(lines[index])) continue; - let last = index; - // Like Git, changes whose surrounding context would touch share one hunk. - for (let next = index + 1; next < lines.length && next - last - 1 <= 2 * context; next++) { - if (isChange(lines[next])) last = next; - } - hunks.push({ - id: `hunk-${index}`, - anchor: index, - start: Math.max(1, index - context), - end: Math.min(lines.length, last + context + 1), - }); - index = last; + for (const core of diff.core_hunks ?? []) { + const { lineStart: start, lineEnd: end } = core; + if (start === undefined || end === undefined || !core.patch) continue; + const anchor = lines.findIndex((line, index) => index >= start && index < end && isChange(line)); + if (anchor < 0) continue; + hunks.push({ id: core.id, anchor, start, end, header: core.header, patch: core.patch }); } return hunks; } -function hunkHeader(lines: GitDiffLine[], start: number, end: number): string { - const range = (key: "old_line_number" | "new_line_number") => { - const numbers = lines.slice(start, end).flatMap(line => line[key] ?? []); - if (numbers.length > 0) return `${numbers[0]},${numbers.length}`; - // Git names the line before an empty range, or 0 at the start of the file. - for (let index = start - 1; index >= 0; index--) { - const previous = lines[index][key]; - if (previous !== undefined) return `${previous},0`; - } - return "0,0"; - }; - return `@@ -${range("old_line_number")} +${range("new_line_number")} @@`; -} - const rowKind = (line: GitDiffLine): ReviewRow["kind"] => line.line_type === "header" ? "information" : line.line_type === "added" ? "addition" @@ -108,53 +83,31 @@ export function monacoDiffRows(diff: GitDiff, options: { hideHunkHeaders?: boole /** Resolve the host's patch identity, never Monaco's aligned display ranges. */ export function monacoDiffHunk(diff: GitDiff, hunkID: string): GitHunk | null { - if (diff.is_truncated || !/^hunk-(0|[1-9]\d*)$/.test(hunkID)) return null; + if (diff.is_truncated) return null; const derived = fullContextHunks(diff); if (derived) { const hunk = derived.find(candidate => candidate.id === hunkID); if (!hunk) return null; - // When the diff carries a byte-faithful writePatch, slice the target - // hunk from the raw patch so the host receives original bytes; fall - // back to display lines for non-lossy diffs. - if (diff.writePatch) { - const changed = diff.lines - .slice(hunk.start, hunk.end) - .filter(line => line.line_type === "added" || line.line_type === "removed"); - if (changed.length > 0) { - // Collect the full range of old-side and new-side line numbers from - // all changed lines in this hunk; slice each contiguous run and - // concatenate into one patch covering the entire selection. - const oldNumbers = changed - .map(line => line.old_line_number) - .filter((n): n is number => n !== undefined && n !== null); - const newNumbers = changed - .map(line => line.new_line_number) - .filter((n): n is number => n !== undefined && n !== null); - const oldStart = oldNumbers.length > 0 ? Math.min(...oldNumbers) : Math.min(...newNumbers); - const newStart = newNumbers.length > 0 ? Math.min(...newNumbers) : oldStart; - const oldEnd = oldNumbers.length > 0 ? Math.max(...oldNumbers) : newStart; - const newEnd = newNumbers.length > 0 ? Math.max(...newNumbers) : oldEnd; - // Try the contiguous-run slicer first (handles single-run cases - // correctly); when its result doesn't cover the full target range - // (separated changes), fall through to the range-based builder. - const faithful = buildByteFaithfulHunk(diff, oldStart, newStart, oldEnd, newEnd) - ?? buildFaithfulHunkFromRange(diff, oldStart, newStart, oldEnd, newEnd); - if (faithful) return faithful; - } - } + // The patch is Core's, with original bytes; display lines only label it. return { file_path: diff.file_path, lines: [ - { line_type: "header", content: hunkHeader(diff.lines, hunk.start, hunk.end) }, + { line_type: "header", content: hunk.header }, ...diff.lines.slice(hunk.start, hunk.end), ], + rawPatch: hunk.patch, }; } + if (!/^hunk-(0|[1-9]\d*)$/.test(hunkID)) return null; const start = Number(hunkID.slice(5)); const header = diff.lines[start]; if (header?.line_type !== "header" || !parseDiffHunkRange(header.content)) return null; let end = start + 1; while (end < diff.lines.length && diff.lines[end].line_type !== "header") end++; if (end === start + 1) return null; - return { file_path: diff.file_path, lines: diff.lines.slice(start, end) }; + // Sparse hunks are Core's own; the patch is found by its header, and a + // missing or ambiguous match leaves the hunk without a writable patch. + const matches = (diff.core_hunks ?? []).filter(core => core.header === header.content && core.patch); + return { file_path: diff.file_path, lines: diff.lines.slice(start, end), + ...(matches.length === 1 ? { rawPatch: matches[0].patch } : {}) }; } diff --git a/windows/tauri/src/platform/core-result-adapter.ts b/windows/tauri/src/platform/core-result-adapter.ts index 49a7b96403..bf5cb70e9a 100644 --- a/windows/tauri/src/platform/core-result-adapter.ts +++ b/windows/tauri/src/platform/core-result-adapter.ts @@ -100,7 +100,9 @@ function adaptDiff(command: string, args: JsonRecord | undefined, value: unknown core_hunks: decodingLoss ? undefined : (Array.isArray(file.hunks) ? file.hunks : []).map((entry: unknown) => { const hunk = asRecord(entry); return { id: String(hunk.id ?? ""), header: String(hunk.header ?? ""), - patch: String((status === "R" ? hunk.contentPatch : hunk.patch) ?? "") }; + patch: String((status === "R" ? hunk.contentPatch : hunk.patch) ?? ""), + ...(typeof hunk.lineStart === "number" && typeof hunk.lineEnd === "number" + ? { lineStart: hunk.lineStart, lineEnd: hunk.lineEnd } : {}) }; }), }; }); @@ -303,6 +305,14 @@ export function adaptCoreResult( case "git_working_tree_ref_diff": case "git_stash_diff": return adaptDiff(command, args, value) as T; + case "git.planDiffBlocks": { + // Core plans blocks and owns every patch; only display sections adapt. + const section = (key: string) => (adaptDiff("git.diff", args, data[key]) as GitDiff[])[0] + ?? { file_path: String(args?.path ?? ""), is_new: false, is_deleted: false, + is_renamed: false, lines: [] }; + return { token: String(data.token ?? ""), blocks: Array.isArray(data.blocks) ? data.blocks : [], + diff: section("worktree"), staged: section("index") } as T; + } case "git_status_diff_stats": return adaptDiffStats(args, value) as T; case "git_discover_repo": From 84141d65fefcdaaee79589e3856ee1e10315c38c Mon Sep 17 00:00:00 2001 From: lick <2188718831@qq.com> Date: Wed, 4 Nov 2026 21:47:30 -0800 Subject: [PATCH 2/3] =?UTF-8?q?style(core):=20=E6=A0=BC=E5=BC=8F=E5=8C=96?= =?UTF-8?q?=20diff=5Fblocks=20=E4=B8=8E=E5=85=B6=E6=B5=8B=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- rust/lithe-core/src/git/diff_blocks.rs | 29 ++++++------- rust/lithe-core/src/tests/git_diff_blocks.rs | 43 +++++++++++++++----- 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/rust/lithe-core/src/git/diff_blocks.rs b/rust/lithe-core/src/git/diff_blocks.rs index c01b56b1eb..86b6a9eafa 100644 --- a/rust/lithe-core/src/git/diff_blocks.rs +++ b/rust/lithe-core/src/git/diff_blocks.rs @@ -133,8 +133,7 @@ impl SingleHunkDocument { old_start, new_start, }; - (document.old().len() == old_count && document.new().len() == new_count) - .then_some(document) + (document.old().len() == old_count && document.new().len() == new_count).then_some(document) } /// Parses a section whose hunk spans both complete files. @@ -165,7 +164,10 @@ impl SingleHunkDocument { /// Minimal header that applies content only. Mode, rename and index lines /// are dropped so a block write never stages a mode change or a rename. fn content_header(&self) -> Option<[String; 3]> { - let diff = self.header.iter().find(|line| line.starts_with("diff --git "))?; + let diff = self + .header + .iter() + .find(|line| line.starts_with("diff --git "))?; let old = self.header.iter().find(|line| line.starts_with("--- "))?; let new = self.header.iter().find(|line| line.starts_with("+++ "))?; if old == "--- /dev/null" || new == "+++ /dev/null" { @@ -574,13 +576,8 @@ pub(super) fn plan_blocks( let indeterminate = !checked && span != before; let include = replacement_patch(&header, &index_lines, start, end, after); let exclude = replacement_patch(&header, &index_lines, start, end, before); - let rollback = replacement_patch( - &header, - &worktree_lines, - run.new_start, - run.new_end, - span, - ); + let rollback = + replacement_patch(&header, &worktree_lines, run.new_start, run.new_end, span); // The checkbox includes an unchecked or partial block and excludes a checked one. let toggle_patch = if checked { exclude.is_some() @@ -793,8 +790,8 @@ fn section_value(file: Option, decoding_loss: bool) -> Valu /// Plans commit-review blocks for one file (`git.planDiffBlocks`). pub(crate) fn plan_diff_blocks_command(payload: Value) -> Result { - let target: BlockTarget = serde_json::from_value(payload) - .map_err(|_| invalid("Invalid Git diff block request"))?; + let target: BlockTarget = + serde_json::from_value(payload).map_err(|_| invalid("Invalid Git diff block request"))?; let root = validate_root(&target.root)?; let snapshot = BlockSnapshot::read(&target, &root)?; let blocks = snapshot.blocks(); @@ -823,8 +820,8 @@ struct ApplyBlockRequest { /// under the write lease; a different token means the file, the index or the /// attributes changed after the user saw the block, so nothing is written. pub(crate) fn apply_diff_block_command(payload: Value) -> Result { - let request: ApplyBlockRequest = serde_json::from_value(payload) - .map_err(|_| invalid("Invalid Git diff block request"))?; + let request: ApplyBlockRequest = + serde_json::from_value(payload).map_err(|_| invalid("Invalid Git diff block request"))?; let root = validate_root(&request.target.root)?; let _lease = RewriteLease::acquire(&root)?; let snapshot = BlockSnapshot::read(&request.target, &root)?; @@ -918,8 +915,8 @@ pub(crate) fn stage_gutter_change_command(payload: Value) -> Result(); + let before = (1..=40) + .map(|line| format!("line {line}\n")) + .collect::(); let mut after = before.lines().map(str::to_string).collect::>(); after[5] = "first".into(); after[19] = "second\r".into(); @@ -325,8 +343,8 @@ fn complete_file_diff_hunks_equal_git_default_hunks_and_stage_alone() { assert_eq!(response["ok"], true, "{response}"); let file = &response["data"]["files"][0]; let hunks = file["hunks"].as_array().unwrap(); - let default = String::from_utf8(repository.git(&["diff", "--no-color", "--", "file.txt"])) - .unwrap(); + let default = + String::from_utf8(repository.git(&["diff", "--no-color", "--", "file.txt"])).unwrap(); // Default hunks carry Git's function-name hint after the range; compare // the range and every body byte. let default_hunks = default @@ -346,8 +364,7 @@ fn complete_file_diff_hunks_equal_git_default_hunks_and_stage_alone() { .unwrap(); assert_eq!(header, range); assert_eq!(body.trim_end_matches('\n'), expected.trim_end_matches('\n')); - let lines = &file["lines"].as_array().unwrap()[hunk["lineStart"].as_u64().unwrap() - as usize + let lines = &file["lines"].as_array().unwrap()[hunk["lineStart"].as_u64().unwrap() as usize ..hunk["lineEnd"].as_u64().unwrap() as usize]; assert!(lines.iter().all(|line| line["lineType"] != "header")); } @@ -381,11 +398,17 @@ fn gutter_changes_stage_original_bytes_and_refuse_moved_lines() { let response = repository.request("git.stageGutterChange", change("four", "FOUR", 4)); applied(&response); - assert_eq!(repository.index("file.txt"), b"one\r\ntwo\r\nthree\r\nFOUR\r\n"); + assert_eq!( + repository.index("file.txt"), + b"one\r\ntwo\r\nthree\r\nFOUR\r\n" + ); // The editor still shows a line the file no longer has. repository.write("file.txt", b"Uno\r\ntwo\r\nthree\r\nFOUR\r\n"); let response = repository.request("git.stageGutterChange", change("one", "ONE", 1)); assert_eq!(failure(&response), "GIT_GUTTER_CHANGE_STALE"); - assert_eq!(repository.index("file.txt"), b"one\r\ntwo\r\nthree\r\nFOUR\r\n"); + assert_eq!( + repository.index("file.txt"), + b"one\r\ntwo\r\nthree\r\nFOUR\r\n" + ); } From 273522856bab16abd34de71484fee03ce2045243 Mon Sep 17 00:00:00 2001 From: lick <2188718831@qq.com> Date: Wed, 4 Nov 2026 22:14:52 -0800 Subject: [PATCH 3/3] =?UTF-8?q?fix(core):=20#1662=20=E6=95=B4=E6=96=87?= =?UTF-8?q?=E4=BB=B6=E5=86=99=E5=85=A5=E7=BB=91=E5=AE=9A=E7=8A=B6=E6=80=81?= =?UTF-8?q?=E4=BB=A4=E7=89=8C=EF=BC=8C=E5=9B=9E=E6=BB=9A=E4=B8=8D=E5=AE=89?= =?UTF-8?q?=E5=85=A8=E6=98=BE=E7=A4=BA=E4=B8=93=E9=97=A8=E6=96=87=E6=A1=88?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - git.applyDiffBlock 新增 includeFile / excludeFile，blockId 可选；写锁内重新规划，令牌不一致返回 GIT_DIFF_BLOCK_STALE，校验统一在 Core，不恢复 TS 快照比对 - Windows 整文件暂存/取消暂存改走 Core 令牌写入 - 新增 git.diff.rollbackUnsafe 中英文案，块评审与 hunk 回滚遇 GIT_HUNK_ROLLBACK_UNSAFE 时显示该原因 - 共享 fixture 增加 wholeFileWriteCases，补 Rust 与 Windows 测试 Co-Authored-By: Claude Opus 5.5 --- rust/lithe-core/src/git/diff_blocks.rs | 21 ++++++++++- rust/lithe-core/src/tests/git_diff_blocks.rs | 37 +++++++++++++++++++ shared/fixtures/git/diff-blocks-v1.json | 15 ++++++++ shared/i18n/locales/en-US.json | 1 + shared/i18n/locales/zh-CN.json | 1 + shared/i18n/messages.json | 7 ++++ .../features/git/api/git-diff-block-api.ts | 13 +++++-- .../src/features/git/api/git-status-api.ts | 6 ++- .../diff/commit-file-diff-preview.tsx | 3 +- .../git/components/diff/monaco-git-diff.tsx | 6 +-- .../git/hooks/use-commit-diff-review.test.tsx | 23 +++++++----- .../git/hooks/use-commit-diff-review.ts | 8 +--- .../git/services/commit-diff-review.test.ts | 27 +++++++++++--- .../git/services/commit-diff-review.ts | 33 +++++++++-------- .../utils/monaco-diff-hunk-actions.test.ts | 10 ++++- .../git/utils/monaco-diff-hunk-actions.ts | 5 ++- windows/tauri/src/i18n/shared.generated.ts | 2 + 17 files changed, 168 insertions(+), 50 deletions(-) diff --git a/rust/lithe-core/src/git/diff_blocks.rs b/rust/lithe-core/src/git/diff_blocks.rs index 86b6a9eafa..ca38720fec 100644 --- a/rust/lithe-core/src/git/diff_blocks.rs +++ b/rust/lithe-core/src/git/diff_blocks.rs @@ -809,8 +809,9 @@ pub(crate) fn plan_diff_blocks_command(payload: Value) -> Result Result>(); + let response = if request.action == "includeFile" { + super::stage_paths(&root, &paths)? + } else { + super::unstage_paths(&root, &paths)? + }; + return serde_json::to_value(response) + .map_err(|_| CoreError::new(ErrorCode::Unknown, "Could not encode Git response")); + } let block = snapshot .blocks() .into_iter() diff --git a/rust/lithe-core/src/tests/git_diff_blocks.rs b/rust/lithe-core/src/tests/git_diff_blocks.rs index 34979512eb..4a81acc9c3 100644 --- a/rust/lithe-core/src/tests/git_diff_blocks.rs +++ b/rust/lithe-core/src/tests/git_diff_blocks.rs @@ -412,3 +412,40 @@ fn gutter_changes_stage_original_bytes_and_refuse_moved_lines() { b"one\r\ntwo\r\nthree\r\nFOUR\r\n" ); } + +#[test] +fn whole_file_inclusion_is_bound_to_the_planned_token() { + for case in fixture()["wholeFileWriteCases"].as_array().unwrap() { + let text = |field: &str| case[field].as_str().unwrap().as_bytes().to_vec(); + let stale = case["staleError"].as_str().unwrap(); + let repository = Repository::new("diff-blocks-file"); + repository.commit("file.txt", &text("head")); + repository.write("file.txt", &text("worktree")); + let plan = repository.plan("file.txt"); + let file = |action: &str, token: &Value| { + repository.request( + "git.applyDiffBlock", + json!({ "path": "file.txt", "action": action, "token": token }), + ) + }; + + // The file moved after planning: neither direction writes anything. + repository.write("file.txt", &text("movedWorktree")); + for action in case["actions"].as_array().unwrap() { + assert_eq!( + failure(&file(action.as_str().unwrap(), &plan["token"])), + stale + ); + } + assert_eq!(repository.index("file.txt"), text("head")); + + let fresh = repository.plan("file.txt"); + applied(&file("includeFile", &fresh["token"])); + assert_eq!(repository.index("file.txt"), text("indexAfterInclude")); + // Staging moved the index, so the old token no longer describes it. + assert_eq!(failure(&file("excludeFile", &fresh["token"])), stale); + let staged = repository.plan("file.txt"); + applied(&file("excludeFile", &staged["token"])); + assert_eq!(repository.index("file.txt"), text("indexAfterExclude")); + } +} diff --git a/shared/fixtures/git/diff-blocks-v1.json b/shared/fixtures/git/diff-blocks-v1.json index 607cf12255..0bb862c6f4 100644 --- a/shared/fixtures/git/diff-blocks-v1.json +++ b/shared/fixtures/git/diff-blocks-v1.json @@ -305,5 +305,20 @@ "patch": "diff --git a/f b/f\n--- a/f\n+++ b/f\n@@ -1 +1 @@\n-a\n+b\n@@ -9 +9 @@\n-c\n+d\n", "hunks": null } + ], + "wholeFileWriteCases": [ + { + "name": "whole-file inclusion is bound to the planned token", + "head": "a\nb\nc\n", + "worktree": "a\nB\nc\n", + "movedWorktree": "a\nB\nc\nlater\n", + "staleError": "GIT_DIFF_BLOCK_STALE", + "actions": [ + "includeFile", + "excludeFile" + ], + "indexAfterInclude": "a\nB\nc\nlater\n", + "indexAfterExclude": "a\nb\nc\n" + } ] } diff --git a/shared/i18n/locales/en-US.json b/shared/i18n/locales/en-US.json index 50b5399c6b..a0a94bfaab 100644 --- a/shared/i18n/locales/en-US.json +++ b/shared/i18n/locales/en-US.json @@ -1785,6 +1785,7 @@ "git.diff.repositoryPreviewTitle": "Repository Diff: {file}", "git.diff.rollbackHunk": "Rollback Change Block", "git.diff.rollbackHunkConfirm": "Roll back this change block in \"{path}\"? Its unstaged edits will be discarded. This cannot be undone.", + "git.diff.rollbackUnsafe": "This change cannot be rolled back safely because a Git filter or working-tree-encoding applies to the file.", "git.diff.search": "Search Diff", "git.diff.searchChanges": "Search changes", "git.diff.selectedCommitsTitle": "{count} selected commits", diff --git a/shared/i18n/locales/zh-CN.json b/shared/i18n/locales/zh-CN.json index e89cd3d8b7..71f125aab4 100644 --- a/shared/i18n/locales/zh-CN.json +++ b/shared/i18n/locales/zh-CN.json @@ -1785,6 +1785,7 @@ "git.diff.repositoryPreviewTitle": "仓库差异: {file}", "git.diff.rollbackHunk": "回滚修改块", "git.diff.rollbackHunkConfirm": "回滚 “{path}” 中的这个修改块吗？该块未暂存的修改将被丢弃，此操作无法撤销。", + "git.diff.rollbackUnsafe": "该文件配置了 Git filter 或 working-tree-encoding，无法安全回滚此改动。", "git.diff.search": "搜索差异", "git.diff.searchChanges": "搜索更改", "git.diff.selectedCommitsTitle": "{count} 个选中提交", diff --git a/shared/i18n/messages.json b/shared/i18n/messages.json index ad719c7ed0..aa1465eeb5 100644 --- a/shared/i18n/messages.json +++ b/shared/i18n/messages.json @@ -14055,6 +14055,13 @@ "path": "string" } }, + { + "id": "git.diff.rollbackUnsafe", + "description": "Windows UI: rolling back a block is refused because a Git filter or working-tree-encoding could change bytes the patch never saw", + "platforms": [ + "windows" + ] + }, { "id": "git.diff.search", "description": "Windows UI: git.diff.search", diff --git a/windows/tauri/src/features/git/api/git-diff-block-api.ts b/windows/tauri/src/features/git/api/git-diff-block-api.ts index efe7456f41..a3fc96fa3f 100644 --- a/windows/tauri/src/features/git/api/git-diff-block-api.ts +++ b/windows/tauri/src/features/git/api/git-diff-block-api.ts @@ -45,6 +45,8 @@ export interface CommitDiffBlockPlan { } export type CommitDiffBlockAction = "include" | "exclude" | "rollback"; +/** Whole-file inclusion; Core checks the same token under its write lock. */ +export type CommitDiffFileAction = "includeFile" | "excludeFile"; export type CommitDiffBlockWriteResult = "applied" | "stale" | "unsafe" | "failed"; const payload = (target: CommitDiffBlockTarget) => ({ @@ -79,12 +81,13 @@ export async function planCommitDiffBlocks(target: CommitDiffBlockTarget): Promi */ export async function applyCommitDiffBlock( target: CommitDiffBlockTarget, - blockId: string, - action: CommitDiffBlockAction, + blockId: string | null, + action: CommitDiffBlockAction | CommitDiffFileAction, token: string, ): Promise { try { - await tauriInvoke("git.applyDiffBlock", { ...payload(target), blockId, action, token }); + await tauriInvoke("git.applyDiffBlock", { + ...payload(target), ...(blockId === null ? {} : { blockId }), action, token }); } catch (error) { const message = errorMessage(error); if (message.includes("GIT_DIFF_BLOCK_STALE")) return "stale"; @@ -96,7 +99,9 @@ export async function applyCommitDiffBlock( repoPath: target.repoPath, filePath: target.filePath, scopes: ["working-tree"], - source: action === "rollback" ? "discard-hunk" : action === "include" ? "stage-hunk" : "unstage-hunk", + source: action === "rollback" ? "discard-hunk" + : action === "include" ? "stage-hunk" : action === "exclude" ? "unstage-hunk" + : action === "includeFile" ? "stage-files" : "unstage-files", }); return "applied"; } diff --git a/windows/tauri/src/features/git/api/git-status-api.ts b/windows/tauri/src/features/git/api/git-status-api.ts index 3e5a756f1e..e3d8a95d2c 100644 --- a/windows/tauri/src/features/git/api/git-status-api.ts +++ b/windows/tauri/src/features/git/api/git-status-api.ts @@ -379,7 +379,8 @@ export const unstageHunk = async (repoPath: string, hunk: GitHunk): Promise => { +/** `"unsafe"` means Core refused because a Git filter or working-tree-encoding applies. */ +export const discardHunk = async (repoPath: string, hunk: GitHunk): Promise => { try { const resolvedRepoPath = await resolveRepositoryPathOrThrow(repoPath); await tauriInvoke("git_discard_hunk", { repoPath: resolvedRepoPath, hunk }); @@ -391,6 +392,9 @@ export const discardHunk = async (repoPath: string, hunk: GitHunk): Promise {t(review.error === "read" ? "git.diff.refreshFailed" : review.error === "stale" - ? "git.diff.selectionChanged" : "git.diff.stageFailed")} + ? "git.diff.selectionChanged" : review.error === "unsafe" + ? "git.diff.rollbackUnsafe" : "git.diff.stageFailed")} }
{!diff ? ( diff --git a/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx b/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx index ab2cffdfc3..d581b9c378 100644 --- a/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx +++ b/windows/tauri/src/features/git/components/diff/monaco-git-diff.tsx @@ -112,7 +112,7 @@ export default function MonacoGitDiff({ const [error, setError] = useState(); // `stale` is a refused zero-context discard: the file changed after this // diff was read, and the watcher replaces the diff (clearing the notice). - const [actionError, setActionError] = useState<"failed" | "stale" | null>(null); + const [actionError, setActionError] = useState<"failed" | "stale" | "unsafe" | null>(null); const [height, setHeight] = useState(MIN_REVIEW_HEIGHT); const blockWidgets = useRef | null>(null); const latestBlockControls = useRef(blockControls); @@ -171,7 +171,7 @@ export default function MonacoGitDiff({ if (closed || updating.current || !owner) return; setActionError(null); void owner.apply(hunkID, action).then((result) => { - if (!closed && hunkActions.current === owner && (result === "failed" || result === "stale")) setActionError(result); + if (!closed && hunkActions.current === owner && (result === "failed" || result === "stale" || result === "unsafe")) setActionError(result); }); }); review.current = instance; @@ -443,7 +443,7 @@ export default function MonacoGitDiff({ return
- {actionError &&
{t(actionError === "stale" ? "git.diff.hunkStale" : "git.operationFailed")}
} + {actionError &&
{t(actionError === "stale" ? "git.diff.hunkStale" : actionError === "unsafe" ? "git.diff.rollbackUnsafe" : "git.operationFailed")}
} {error &&
{error}
}
; } diff --git a/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx b/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx index c681343716..22870a59a3 100644 --- a/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx +++ b/windows/tauri/src/features/git/hooks/use-commit-diff-review.test.tsx @@ -37,9 +37,14 @@ const fakePlan = async (request: blockApi.CommitDiffBlockTarget): Promise => { +const fakeApply = async (request: blockApi.CommitDiffBlockTarget, blockID: string | null, + action: blockApi.CommitDiffBlockAction | blockApi.CommitDiffFileAction, token: string): Promise => { if (token !== `t${version}`) return "stale"; + if (action === "includeFile" || action === "excludeFile") { + indexed = action === "includeFile" ? planWork : head; + version++; + return "applied"; + } const at = blockID === blockIDs[1] ? 1 : 3; if (action === "include") indexed = indexed.map((value, k) => k === at ? planWork[k] : value); else if (action === "exclude") indexed = indexed.map((value, k) => k === at ? head[k] : value); @@ -262,8 +267,6 @@ test("automatic refresh ignores events belonging to a different repository", asy test("index-only changes retain a file, index title and whole-file inclusion without inventing worktree blocks", async () => { indexed = ["a", "B", "c", "d"]; planMode = "index-only"; - const include = spyOn(statusApi, "setFilesStaged").mockImplementation(async () => { indexed = head; return true; }); - spies.push(include); await act(async () => { root.render(); }); expect(current.files).toHaveLength(1); expect(current.files[0].lines).toEqual(fullDiff(head, indexed).lines); @@ -271,21 +274,21 @@ test("index-only changes retain a file, index title and whole-file inclusion wit expect(review.snapshot!.diff.lines).toHaveLength(0); expect(review.blocks).toHaveLength(0); await act(async () => { await review.toggle(null, true); }); - expect(include).toHaveBeenCalledWith(target.repoPath, [target.filePath], true); - expect(stage).not.toHaveBeenCalled(); + expect(stage).toHaveBeenCalledTimes(1); + expect(stage.mock.calls[0][0]).toMatchObject({ repoPath: target.repoPath, filePath: target.filePath }); + expect(stage.mock.calls[0].slice(1)).toEqual([null, "includeFile", "t0"]); }); test.each(["binary", "renamed"] as const)("%s file inclusion stays mixed with staged and worktree changes", async kind => { indexed = ["a", "B", "c", "d"]; planMode = kind; - const include = spyOn(statusApi, "setFilesStaged").mockResolvedValue(true); - spies.push(include); await act(async () => { root.render(); }); expect(review.blocks).toHaveLength(0); expect(review.presentation).toMatchObject({ included: true, indeterminate: true }); await act(async () => { await review.toggle(null, true); }); - expect(include).toHaveBeenCalledWith(target.repoPath, [target.filePath], true); - expect(stage).not.toHaveBeenCalled(); + expect(stage).toHaveBeenCalledTimes(1); + expect(stage.mock.calls[0][0]).toMatchObject({ repoPath: target.repoPath, filePath: target.filePath }); + expect(stage.mock.calls[0].slice(1)).toEqual([null, "includeFile", "t0"]); }); test("a restored preview without target metadata resolves its owner before enabling block controls", async () => { diff --git a/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts b/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts index cdf5a90160..f24b6bf451 100644 --- a/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts +++ b/windows/tauri/src/features/git/hooks/use-commit-diff-review.ts @@ -5,7 +5,7 @@ import { getBufferById } from "@/features/editor/utils/buffer-index"; import { getFullContextFileDiff, getWorkingTreePathDiff } from "../api/git-diff-api"; import { applyCommitDiffBlock, planCommitDiffBlocks } from "../api/git-diff-block-api"; import { resolveRepositoryForFile } from "../api/git-repo-api"; -import { getGitStatus, setFilesStaged } from "../api/git-status-api"; +import { getGitStatus } from "../api/git-status-api"; import { isGitChangeRelevant, subscribeToGitChanges } from "../events/git-events"; import { commitDiffPresentation, createCommitDiffReview, type CommitDiffReviewState } from "../services/commit-diff-review"; import { beginCommitDiffWrite, commitDiffWritePending } from "../runtime/commit-diff-write-state"; @@ -15,7 +15,7 @@ import { showConfirmDialog } from "@/ui/dialog"; import { useTranslation } from "@/i18n/locale-provider"; import type { MultiFileDiff, WorkingTreeDiffTarget } from "../types/git-diff.types"; import { createSingleFileWorkingTreeDiff } from "../utils/working-tree-multi-diff"; -import { getGitFileRepositoryRelativePath, resolveGitFileMutationPaths } from "../utils/git-status-selection"; +import { getGitFileRepositoryRelativePath } from "../utils/git-status-selection"; import { createRequestGeneration } from "../utils/request-generation"; export function useCommitDiffReview(bufferId: string | null, fileKey: string, @@ -74,7 +74,6 @@ export function useCommitDiffReview(bufferId: string | null, fileKey: string, && identity?.repoPath === repoPath && identity.filePath === filePath && Boolean(identity.staged) === stagedView ? data : null; }; - let statusFile: Parameters[0][number] | undefined; const instance = createCommitDiffReview({ beginWrite: beginCommitDiffWrite, writeAvailable: () => !commitDiffWritePending(), @@ -91,7 +90,6 @@ export function useCommitDiffReview(bufferId: string | null, fileKey: string, if (!closed && current()) useBufferStore.getState().actions.closeBuffer(bufferId); return null; } - statusFile = file; // Core reads both comparisons, plans the blocks and keeps their patches. const target = { repoPath, filePath, ...(file.originalPath ? { originalPath: file.originalPath } : {}), ...(stagedView ? { staged: true } : {}) }; @@ -103,8 +101,6 @@ export function useCommitDiffReview(bufferId: string | null, fileKey: string, ...(snapshot.file.originalPath ? { originalPath: snapshot.file.originalPath } : {}), ...(stagedView ? { staged: true } : {}) }, blockId, action, snapshot.token) : Promise.resolve("failed"), - includeFile: included => current() && statusFile - ? setFilesStaged(repoPath, resolveGitFileMutationPaths([statusFile]), included) : Promise.resolve(false), changed: next => { if (closed || !current()) return; setState(next); diff --git a/windows/tauri/src/features/git/services/commit-diff-review.test.ts b/windows/tauri/src/features/git/services/commit-diff-review.test.ts index c74935b21b..6faf1d340e 100644 --- a/windows/tauri/src/features/git/services/commit-diff-review.test.ts +++ b/windows/tauri/src/features/git/services/commit-diff-review.test.ts @@ -31,7 +31,6 @@ function fixture(outcome: "applied" | "stale" | "unsafe" | "failed" = "applied") if (outcome === "applied") current = snapshot(action === "include", "t1"); return outcome; }, - includeFile: async () => { writes.push("file"); return true; }, changed: next => { state = next; }, beginWrite: () => () => { released++; }, }); return { owner, writes, setSnapshot: (value: CommitDiffSnapshot) => { current = value; }, @@ -64,17 +63,33 @@ test("a write Core refuses as stale surfaces the stale notice and refreshes", as } finally { f.owner.dispose(); } }); -test("an unsafe or failed Core write surfaces a write error", async () => { - for (const outcome of ["unsafe", "failed"] as const) { +test("an unsafe rollback keeps its specific reason and a failed write is generic", async () => { + for (const [outcome, error] of [["unsafe", "unsafe"], ["failed", "write"]] as const) { const f = fixture(outcome); try { await f.owner.refresh(); expect(await f.owner.apply("line-1", "rollback")).toBe("failed"); - expect(f.state.error).toBe("write"); + expect(f.state.error).toBe(error); } finally { f.owner.dispose(); } } }); +test("whole-file inclusion carries the snapshot token and a stale refusal is surfaced", async () => { + const f = fixture(); + try { + await f.owner.refresh(); + expect(await f.owner.apply(null, "include")).toBe("applied"); + expect(f.writes).toEqual(["includeFile:null:t0"]); + } finally { f.owner.dispose(); } + const stale = fixture("stale"); + try { + await stale.owner.refresh(); + expect(await stale.owner.apply(null, "exclude")).toBe("stale"); + expect(stale.writes).toEqual(["excludeFile:null:t0"]); + expect(stale.state.error).toBe("stale"); + } finally { stale.owner.dispose(); } +}); + test("blocks Core cannot toggle are never sent", async () => { const f = fixture(); try { @@ -91,7 +106,7 @@ test("disposal during a write still releases the commit guard", async () => { const gate = deferred<"applied">(); let released = 0, state!: CommitDiffReviewState; const owner = createCommitDiffReview({ - read: async () => snapshot(), writeBlock: () => gate.promise, includeFile: async () => false, + read: async () => snapshot(), writeBlock: () => gate.promise, beginWrite: () => () => { released++; }, changed: next => { state = next; }, }); await owner.refresh(); @@ -135,7 +150,7 @@ test("leaving commit review mid-write prevents the block write", async () => { let writable = true, writes = 0, state!: CommitDiffReviewState; const owner = createCommitDiffReview({ read: async () => snapshot(), writeBlock: async () => { writes++; return "applied"; }, - includeFile: async () => false, canWrite: () => writable, changed: next => { state = next; }, + canWrite: () => writable, changed: next => { state = next; }, }); await owner.refresh(); writable = false; diff --git a/windows/tauri/src/features/git/services/commit-diff-review.ts b/windows/tauri/src/features/git/services/commit-diff-review.ts index 608d439a3e..56f6e87950 100644 --- a/windows/tauri/src/features/git/services/commit-diff-review.ts +++ b/windows/tauri/src/features/git/services/commit-diff-review.ts @@ -1,5 +1,5 @@ import type { GitDiff, GitFile } from "../types/git.types"; -import type { CommitDiffBlock, CommitDiffBlockAction, CommitDiffBlockWriteResult } from "../api/git-diff-block-api"; +import type { CommitDiffBlock, CommitDiffBlockAction, CommitDiffFileAction, CommitDiffBlockWriteResult } from "../api/git-diff-block-api"; /** One Core read: both comparisons, the planned blocks and the token that * identifies the read. Core refuses any write planned from another read. */ @@ -14,7 +14,7 @@ export interface CommitDiffReviewState { snapshot: CommitDiffSnapshot | null; blocks: CommitDiffBlock[]; busy: boolean; - error?: "read" | "write" | "stale"; + error?: "read" | "write" | "stale" | "unsafe"; } /** Display index-only changes without using them as a working-tree write base. @@ -33,9 +33,10 @@ export function commitDiffPresentation(snapshot: CommitDiffSnapshot, stagedView: interface Dependencies { read: () => Promise; /** Core re-plans under its write lock and refuses a stale token. */ - writeBlock: (snapshot: CommitDiffSnapshot, blockId: string, action: CommitDiffBlockAction) + writeBlock: (snapshot: CommitDiffSnapshot, blockId: string | null, + action: CommitDiffBlockAction | CommitDiffFileAction) => Promise; - includeFile: (included: boolean) => Promise; + changed: (state: CommitDiffReviewState) => void; beginWrite?: () => () => void; canWrite?: () => boolean; @@ -89,18 +90,20 @@ export function createCommitDiffReview(dependencies: Dependencies) { publish({ ...state, busy: true, error: undefined }); try { if (closed || dependencies.canWrite?.() === false) return "ignored"; - let result: boolean; - if (id === null || block?.wholeFile) { - result = await dependencies.includeFile(action === "include"); - } else { - const outcome = await dependencies.writeBlock(snapshot, id, action); - if (outcome === "stale") { - // The file moved after the user saw the block: show what is there now. - publish({ ...state, error: "stale" }); - return "stale"; - } - result = outcome === "applied"; + // Whole-file inclusion carries the same token as a block write. + const outcome = await dependencies.writeBlock(snapshot, + id === null || block?.wholeFile ? null : id, + id === null || block?.wholeFile ? (action === "include" ? "includeFile" : "excludeFile") : action); + if (outcome === "stale") { + // The file moved after the user saw it: show what is there now. + publish({ ...state, error: "stale" }); + return "stale"; + } + if (outcome === "unsafe") { + publish({ ...state, error: "unsafe" }); + return "failed"; } + const result = outcome === "applied"; if (!closed && !result) publish({ ...state, error: "write" }); return result ? "applied" : "failed"; } catch { diff --git a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts index 95e59f82ad..8bf6bcd5c2 100644 --- a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts +++ b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.test.ts @@ -31,7 +31,7 @@ const discardContext = { ...context, canDiscard: true }; function rollbackOperations() { return { ...operations(), - discard: mock(async (_repo: string, _hunk: GitHunk) => true), + discard: mock(async (_repo: string, _hunk: GitHunk): Promise => true), }; } function deferred() { @@ -79,6 +79,14 @@ describe("Core-owned hunk patches", () => { expect(api.discard.mock.calls[0]![1].rawPatch).toContain("-b\r\n+B\r\n"); }); + test("a rollback Core refuses as unsafe is reported as such and can be retried", async () => { + const api = rollbackOperations(); + api.discard.mockResolvedValueOnce("unsafe"); + const owner = createMonacoDiffHunkActions(lossyDiff, discardContext, api); + expect(await owner.apply(hunkID(lossyDiff), "discard")).toBe("unsafe"); + expect(await owner.apply(hunkID(lossyDiff), "discard")).toBe("applied"); + }); + test("a hunk Core offered no patch for is never written", async () => { const api = rollbackOperations(); const bare = { ...diff, core_hunks: undefined }; diff --git a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts index 5c08108a2a..9cd68ccd6e 100644 --- a/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts +++ b/windows/tauri/src/features/git/utils/monaco-diff-hunk-actions.ts @@ -26,9 +26,9 @@ export function workingTreeStagingContext( return { repoPath: review.repoPath, isStaged, ...(canDiscard ? { canDiscard: true } : {}) }; } -type HunkOperation = (repoPath: string, hunk: GitHunk) => Promise; +type HunkOperation = (repoPath: string, hunk: GitHunk) => Promise; export type HunkActionID = "stage" | "unstage" | "discard"; -type ActionResult = "applied" | "failed" | "ignored" | "cancelled" | "stale"; +type ActionResult = "applied" | "failed" | "ignored" | "cancelled" | "stale" | "unsafe"; export interface MonacoDiffHunkOperations { stage: HunkOperation; @@ -95,6 +95,7 @@ export function createMonacoDiffHunkActions( if (disposed) return "ignored"; // The patch is stale after a successful mutation. Wait for the host's // Git change event to replace it before accepting another action. + if (success === "unsafe") return "unsafe"; applied = success; return success ? "applied" : "failed"; } catch { diff --git a/windows/tauri/src/i18n/shared.generated.ts b/windows/tauri/src/i18n/shared.generated.ts index ad0053a2bc..0687644398 100644 --- a/windows/tauri/src/i18n/shared.generated.ts +++ b/windows/tauri/src/i18n/shared.generated.ts @@ -1831,6 +1831,7 @@ export const sharedCatalogs = { "git.diff.repositoryPreviewTitle": "Repository Diff: {file}", "git.diff.rollbackHunk": "Rollback Change Block", "git.diff.rollbackHunkConfirm": "Roll back this change block in \"{path}\"? Its unstaged edits will be discarded. This cannot be undone.", + "git.diff.rollbackUnsafe": "This change cannot be rolled back safely because a Git filter or working-tree-encoding applies to the file.", "git.diff.search": "Search Diff", "git.diff.searchChanges": "Search changes", "git.diff.selectedCommitsTitle": "{count} selected commits", @@ -6882,6 +6883,7 @@ export const sharedCatalogs = { "git.diff.repositoryPreviewTitle": "仓库差异: {file}", "git.diff.rollbackHunk": "回滚修改块", "git.diff.rollbackHunkConfirm": "回滚 “{path}” 中的这个修改块吗？该块未暂存的修改将被丢弃，此操作无法撤销。", + "git.diff.rollbackUnsafe": "该文件配置了 Git filter 或 working-tree-encoding，无法安全回滚此改动。", "git.diff.search": "搜索差异", "git.diff.searchChanges": "搜索更改", "git.diff.selectedCommitsTitle": "{count} 个选中提交",