Skip to content

fix(scan): keep committed diff inventories consistent #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mldangelo-oai wants to merge 8 commits into
base: main
Choose a base branch
from
+83 −2
Open

fix(scan): keep committed diff inventories consistent #370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1c4e880
fix(scan): reject committed symlinks in diff inventories
mldangelo-oai Aug 12, 2026
de69cc5
fix(scan): batch committed symlink mode checks
mldangelo-oai Aug 12, 2026
7c49fe1
fix(scan): preserve exact Git path casing in diff inventory
mldangelo-oai Aug 12, 2026
c6b1dc0
fix(scan): reject symlinks across diff review workflows
mldangelo-oai Aug 12, 2026
6e81309
fix(scan): read diff previews from selected git revisions
mldangelo-oai Aug 12, 2026
106b1c0
fix(scan): stream regular committed source blobs
mldangelo-oai Aug 12, 2026
367dee5
fix(scan): preserve revision paths as git posix names
mldangelo-oai Aug 12, 2026
8a5b2ec
refactor(scan): narrow committed symlink inventory handling
mldangelo-oai Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions sdk/typescript/_bundled_plugin/scripts/generate_in_scope_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,39 @@ def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int:
return write_inventory(output, rows)


def committed_changed_paths(repository: Path, base: str, head: str) -> list[tuple[Path, str]]:
result = subprocess.run(
[
"git",
"-C",
str(repository),
"diff",
"--raw",
"-z",
"--diff-filter=ACMRD",
f"{base}..{head}",
],
capture_output=True,
text=True,

@pasmud pasmud Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hit a Windows-specific crash here when Git returns a path that the active ANSI code page cannot decode. On the current head, I ran with PYTHONUTF8=0 and a CP1252 locale, then committed a changed file named 丁.py. The subprocess reader raised UnicodeDecodeError on byte 0x81; result.stdout was then None, so the following .split("\0") raised AttributeError and aborted the revision scan.

Could we keep the NUL-delimited Git output as bytes (or decode it with an explicit Git-compatible encoding) instead of relying on locale-sensitive text=True? A Windows regression test using CP1252 and a non-ASCII filename would cover this.

check=True,
)
fields = result.stdout.split("\0")
changed: list[tuple[Path, str]] = []
index = 0
while index < len(fields) - 1:
metadata = fields[index].split()
status = metadata[-1][0]
index += 1
if status in {"C", "R"}:
index += 1
path = fields[index]
index += 1
selected_mode = metadata[0].removeprefix(":") if status == "D" else metadata[1]
if selected_mode != "120000":
changed.append((repository / path, status))
return changed


def generate_diff_in_scope_files(
repository: Path,
base: str,
Expand All @@ -114,7 +147,7 @@ def generate_diff_in_scope_files(
) -> int:
"""Reuse the existing diff selection without generating previews or duplicate worklists."""
sys.path.insert(0, str(Path(__file__).resolve().parent))
from generate_rank_input import git_changed_paths, path_is_excluded, run_git_changed_paths
from generate_rank_input import path_is_excluded, run_git_changed_paths
from rank_preview import (
DEFAULT_PREVIEW_BYTES,
TEXT_CODE_EXTENSIONS,
Expand All @@ -138,7 +171,7 @@ def generate_diff_in_scope_files(
if relative
)
else:
changed = git_changed_paths(repository, base, head, mode)
changed = committed_changed_paths(repository, base, head)

for path, status in changed:
relative = path.relative_to(repository)
Expand Down
48 changes: 48 additions & 0 deletions sdk/typescript/tests-ts/compact-diff-scan.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,54 @@ describe("compact diff scan", () => {
]);
});

test("omits committed symlinks from the revision inventory", () => {
const { root, repository } = createRepository();
writeSource(repository, "src/handler.py", "value = 1\n");
writeSource(repository, "src/deleted-link.py", "handler.py");
git(repository, "add", ".");
const deletedLink = git(repository, "hash-object", "src/deleted-link.py");
git(
repository,
"update-index",
"--cacheinfo",
`120000,${deletedLink},src/deleted-link.py`,
);
git(repository, "commit", "-qm", "base");
const base = git(repository, "rev-parse", "HEAD");

rmSync(join(repository, "src", "deleted-link.py"));
writeSource(repository, "src/handler.py", "value = 2\n");
writeSource(repository, "src/added-link.py", "handler.py");
git(repository, "add", ".");
const addedLink = git(repository, "hash-object", "src/added-link.py");
git(
repository,
"update-index",
"--cacheinfo",
`120000,${addedLink},src/added-link.py`,
);
git(repository, "commit", "-qm", "selected changes");
const head = git(repository, "rev-parse", "HEAD");
const output = join(root, "in-scope.txt");

const result = python(
"generate_in_scope_files.py",
"--repo",
repository,
"--scope",
".",
"--diff-base",
base,
"--diff-head",
head,
"--out",
output,
);

expect(result.status, result.stderr).toBe(0);
expect(readFileSync(output, "utf8").trim()).toBe("src/handler.py");
});

test("includes staged, unstaged, and untracked working-tree changes", () => {
const { root, repository } = createRepository();
writeSource(repository, "src/handler.py", "value = 1\n");
Expand Down
Loading