Daoyuan Li b565e6c0f0
fix(workspace-changes): classify a symlink replacing a file distinctly from deleted (#4170)
* fix(workspace-changes): classify a symlink replacing a file distinctly from deleted

scan_workspace_roots() skipped every symlinked path entirely
(host_file.is_symlink() -> continue), so the path was completely absent
from a snapshot instead of being recorded as a metadata-only stub the
way binary/large/sensitive-looking files already are. When an agent run
replaces a tracked file with a symlink (e.g. rm config.txt && ln -s
/some/other/path config.txt), the after-snapshot never contained that
path at all, so compare_snapshots()'s _status() saw after_file=None and
reported plain "deleted" -- silently hiding that the path is still alive
on disk, now as a symlink that can point anywhere on the host, including
outside the workspace root.

Add a "symlink" classification mirroring the existing binary/large/
sensitive pattern: scan_workspace_roots() now records a symlink as a
metadata-only FileSnapshot stub (symlink=True, symlink_target from
os.readlink(), lstat'd without ever following the link) instead of
omitting it. _status() reports "symlink_created" whenever a symlink
newly occupies a path that was not already a symlink (brand new or
replacing a prior file), so the security-relevant fact surfaces
distinctly instead of collapsing into "deleted". A symlink genuinely
removed with nothing replacing it is unchanged: still "deleted".

Verified against a real POSIX symlink (WSL; native Windows symlink
creation needs elevated privilege) driving the unmodified
scan_workspace_roots()/compare_snapshots() functions, and via a
patch-file revert/reapply cycle on this same fix to confirm the added
regression tests fail before and pass after.

* fix(workspace-changes): count symlink_created in the changed-file badge

getChangedFileCount summed only created + modified + deleted, so a run
whose only change was a symlink replacing a file (reported as the new
symlink_created status, not deleted) produced a count of 0 and
WorkspaceChangeBadge hid the badge entirely -- the exact scenario this
PR targets, with the opposite of the intended result.

Add symlink_created to the frontend WorkspaceChangeSummary interface
(already emitted by the backend) and include it in the count. The
per-file StatusIcon/statusLabel "modified" fallthrough is unaffected
and left as a follow-up, per review.

Regression test reverts cleanly to reproduce a count of 0 pre-fix.

* fix(workspace-changes): rank symlink_created in file sort and complete the type contract

sortWorkspaceChanges's statusRank had no entry for the new
symlink_created status, so statusRank[left.status] - statusRank[right.status]
evaluated to NaN for any comparison involving a symlink-created file,
violating Array#sort's ordering contract instead of producing a
deterministic order. The frontend WorkspaceChangeStatus and
DiffUnavailableReason unions, and the WorkspaceFileChange symlink
fields, also stayed narrower than what the backend now emits, so
TypeScript's satisfies Record<...> guard on statusRank could not catch
the gap.

Widen WorkspaceChangeStatus to include "symlink_created" and
DiffUnavailableReason to include "symlink", add the matching
symlink/symlink_target_before/symlink_target_after fields to
WorkspaceFileChange, and give symlink_created a rank alongside
modified in statusRank -- restoring the satisfies guard's ability to
catch a future unranked status. unavailableLabel now has an explicit
"symlink" branch (new symlinkUnavailable i18n string, en-US + zh-CN)
instead of falling through to the generic label.

Also fixes the failing e2e-tests CI check: the existing
workspace-changes.spec.ts mock summary predates the symlink_created
field, so getChangedFileCount computed 1 + 1 + 0 + undefined = NaN and
the badge rendered "Edited NaN files" instead of "Edited 2 files".
Added symlink_created: 0 to the mock to match the real backend
contract.

New sortWorkspaceChanges unit tests revert cleanly against the
unranked statusRank to reproduce the NaN-driven misordering. pnpm test
(626 tests), pnpm check, and pnpm format are clean, and the
previously-failing e2e spec plus the full e2e suite (94 tests) pass.
2026-07-21 10:22:55 +08:00

212 lines
7.5 KiB
Python

from __future__ import annotations
import difflib
from .types import (
DiffUnavailableReason,
FileSnapshot,
WorkspaceChangeLimits,
WorkspaceChangeResult,
WorkspaceChangeStatus,
WorkspaceChangeSummary,
WorkspaceFileChange,
WorkspaceSnapshot,
)
def compare_snapshots(
before: WorkspaceSnapshot,
after: WorkspaceSnapshot,
*,
limits: WorkspaceChangeLimits | None = None,
) -> WorkspaceChangeResult:
resolved_limits = limits or WorkspaceChangeLimits()
all_paths = sorted(set(before.files) | set(after.files))
changes: list[WorkspaceFileChange] = []
created = modified = deleted = symlink_created = additions = deletions = 0
total_diff_bytes = 0
truncated = before.truncated or after.truncated
for path in all_paths:
before_file = before.files.get(path)
after_file = after.files.get(path)
if before_file and after_file and _same_file(before_file, after_file):
continue
status = _status(before_file, after_file)
if status == "created":
created += 1
elif status == "modified":
modified += 1
elif status == "symlink_created":
symlink_created += 1
else:
deleted += 1
diff, line_additions, line_deletions, diff_truncated, reason = _build_diff(
path,
before_file,
after_file,
remaining_bytes=max(0, resolved_limits.max_total_diff_bytes - total_diff_bytes),
)
if diff:
total_diff_bytes += len(diff.encode("utf-8"))
if diff_truncated or reason in {"large", "truncated"}:
truncated = True
additions += line_additions
deletions += line_deletions
if len(changes) < resolved_limits.max_files:
sample = after_file or before_file
assert sample is not None
changes.append(
WorkspaceFileChange(
path=path,
root=sample.root,
status=status,
binary=bool((after_file or before_file).binary if (after_file or before_file) else False),
sensitive=bool((after_file or before_file).sensitive if (after_file or before_file) else False),
size_before=before_file.size if before_file else None,
size_after=after_file.size if after_file else None,
sha256_before=before_file.sha256 if before_file else None,
sha256_after=after_file.sha256 if after_file else None,
diff=diff,
diff_truncated=diff_truncated,
diff_unavailable_reason=reason,
additions=line_additions,
deletions=line_deletions,
symlink=bool((after_file or before_file).symlink if (after_file or before_file) else False),
symlink_target_before=before_file.symlink_target if before_file else None,
symlink_target_after=after_file.symlink_target if after_file else None,
)
)
else:
truncated = True
return WorkspaceChangeResult(
summary=WorkspaceChangeSummary(
created=created,
modified=modified,
deleted=deleted,
symlink_created=symlink_created,
additions=additions,
deletions=deletions,
truncated=truncated,
),
files=changes,
limits=resolved_limits,
)
def get_changed_paths(before: WorkspaceSnapshot, after: WorkspaceSnapshot) -> set[str]:
changed: set[str] = set()
for path in set(before.files) | set(after.files):
before_file = before.files.get(path)
after_file = after.files.get(path)
if before_file and after_file and _same_file(before_file, after_file):
continue
changed.add(path)
return changed
def _status(
before_file: FileSnapshot | None,
after_file: FileSnapshot | None,
) -> WorkspaceChangeStatus:
# A symlink now occupying a path that was not already a symlink is always
# surfaced distinctly - whether it is brand new (before_file is None) or it
# just replaced a regular file (before_file is None => "deleted" would
# otherwise be reported even though the path is still alive on disk, just
# as a symlink that may point anywhere on the host).
before_was_symlink = before_file is not None and before_file.symlink
after_is_symlink = after_file is not None and after_file.symlink
if after_is_symlink and not before_was_symlink:
return "symlink_created"
if before_file is None:
return "created"
if after_file is None:
return "deleted"
return "modified"
def _same_file(before_file: FileSnapshot, after_file: FileSnapshot) -> bool:
if before_file.sha256 is not None and after_file.sha256 is not None:
return before_file.sha256 == after_file.sha256
return before_file.size == after_file.size and before_file.mtime_ns == after_file.mtime_ns
def _build_diff(
path: str,
before_file: FileSnapshot | None,
after_file: FileSnapshot | None,
*,
remaining_bytes: int,
) -> tuple[str, int, int, bool, DiffUnavailableReason | None]:
reason = _diff_unavailable_reason(before_file, after_file)
if reason is not None:
return "", 0, 0, False, reason
before_text = _snapshot_text(before_file) if before_file else ""
after_text = _snapshot_text(after_file) if after_file else ""
if before_file is not None and before_text is None:
return "", 0, 0, False, None
if after_file is not None and after_text is None:
return "", 0, 0, False, None
lines = list(
difflib.unified_diff(
before_text.splitlines(),
after_text.splitlines(),
fromfile=f"a{path}",
tofile=f"b{path}",
lineterm="",
)
)
diff = "\n".join(lines)
additions, deletions = _count_diff_lines(lines)
if len(diff.encode("utf-8")) > remaining_bytes:
return "", additions, deletions, True, "truncated"
return diff, additions, deletions, False, None
def _diff_unavailable_reason(
before_file: FileSnapshot | None,
after_file: FileSnapshot | None,
) -> DiffUnavailableReason | None:
files = [file for file in (before_file, after_file) if file is not None]
for preferred in ("symlink", "sensitive", "binary", "large"):
if any(file.content_unavailable_reason == preferred for file in files):
return preferred # type: ignore[return-value]
return None
def _snapshot_text(file: FileSnapshot | None) -> str | None:
if file is None:
return ""
if file.text is not None:
return file.text
if file.text_path:
try:
with open(file.text_path, encoding="utf-8") as cached:
return cached.read()
except OSError:
return None
return None
def _count_diff_lines(lines: list[str]) -> tuple[int, int]:
additions = 0
deletions = 0
for line in lines:
# Unified-diff file headers are "+++ " / "--- " with a trailing space;
# a bare "+++"/"---" prefix would also swallow real content lines whose
# text begins with those sequences (e.g. an added line "+++foo").
if line.startswith("+++ ") or line.startswith("--- "):
continue
if line.startswith("+"):
additions += 1
elif line.startswith("-"):
deletions += 1
return additions, deletions