#1629

[BUG] Git merge stages duplicate files in cloud source archives

Opensamirhvbropened 9d ago

Describe the bug

Local source packaging adds the same working-tree file multiple times when Git's index contains unresolved merge stages for that path. _git_candidate_paths() runs git ls-files --cached --others --exclude-standard, which can return one pathname for each index stage. select_source() treats every occurrence as a separate file.

On current main, one 66-byte app.py becomes three manifest entries, 198 reported source bytes, and three ZIP members all named app.py. This inflates file/expanded-size accounting and creates duplicate archive members even though only one working-tree file exists.

This affects the packaging path used by strix cloud scans start --source ..., including --dry-run.

To Reproduce

From a source checkout with frozen development dependencies and Python 3.12, run this local-only reproduction. It creates a disposable Git index with stages 1, 2 and 3, as an unresolved merge does; it does not make an API request or start a scan.

LITELLM_MODE=PRODUCTION STRIX_TELEMETRY=0 uv run --frozen python - <<'PY'
import json
import pathlib
import subprocess
import tempfile
import warnings
import zipfile

from strix.interface.cloud import source_upload

with tempfile.TemporaryDirectory() as directory:
    root = pathlib.Path(directory)
    subprocess.run(["git", "init", "-q", str(root)], check=True)
    entries = []
    for stage, content in enumerate(("base\n", "ours\n", "theirs\n"), start=1):
        blob = subprocess.run(
            ["git", "-C", str(root), "hash-object", "-w", "--stdin"],
            input=content, text=True, capture_output=True, check=True,
        ).stdout.strip()
        entries.append(f"100644 {blob} {stage}\tapp.py\n")
    subprocess.run(
        ["git", "-C", str(root), "update-index", "--index-info"],
        input="".join(entries), text=True, check=True,
    )
    source = root / "app.py"
    source.write_text(
        '<<<<<<< ours\nprint("ours")\n=======\nprint("theirs")\n>>>>>>> theirs\n',
        encoding="utf-8",
    )
    with warnings.catch_warnings(record=True) as notices:
        warnings.simplefilter("always")
        bundle = source_upload.prepare_source(
            str(root), include_hidden=False, include_sensitive=False,
            include_archives=False, exclude=[],
        )
    try:
        with zipfile.ZipFile(bundle.archive_path) as archive:
            print(json.dumps({
                "working_tree_bytes": source.stat().st_size,
                "manifest_files": len(bundle.manifest.files),
                "manifest_bytes": bundle.manifest.total_bytes,
                "archive_names": archive.namelist(),
                "warnings": [str(notice.message) for notice in notices],
            }, indent=2))
    finally:
        source_upload.remove_bundle(bundle)
PY

Actual output:

{
  "working_tree_bytes": 66,
  "manifest_files": 3,
  "manifest_bytes": 198,
  "archive_names": ["app.py", "app.py", "app.py"],
  "warnings": ["Duplicate name: 'app.py'", "Duplicate name: 'app.py'"]
}

Expected behavior

Package the selected working-tree path once: one manifest entry, 66 source bytes, and one ZIP member containing the current working-tree content. Preserve the existing tracked/untracked selection, exclusions, deterministic ordering and resource limits.

System Information:

  • OS: macOS, ARM64
  • Strix commit: 6b7ae2ebe4d47037a927aade23265ff4e4f27e84 (main, package version 1.7.0)
  • Python: 3.12.14, frozen dependencies
  • Git: 2.54.0 (Apple Git-157)
  • LLM used: none; no managed cloud, Docker or target scan was used

Additional context

A small fix can suppress repeated relative paths in the existing streaming Git enumerator before yielding them for selection. The raw candidate bound can remain intact, and Python-side deduplication avoids requiring a newer Git command-line option. A regression should build an actual unmerged Git index and check the manifest and ZIP contents, including a source subdirectory and ordinary tracked/untracked files.

I checked the current open and closed issue/PR catalog and the neighboring source-packaging proposals #1298 and #1300. Those address Windows file-stat consistency, while this occurs during Git path enumeration. I have not tested whether the managed service accepts or rejects duplicate ZIP members; the local archive and incorrect accounting above are the reproduced defect.

Activity

1
ToniAdreal7d ago

Independently confirmed the mechanism at current main (6b7ae2e), with the exact spot where the duplication survives:

  • The enumeration trusts git's output shape: _git_candidate_paths (strix/interface/cloud/source_upload.py:369) runs git ls-files -z --cached --others --exclude-standard and yields every NUL-delimited record verbatim — the only gate in the loop is _check_candidate_limit, a count cap, not a seen-set. For an unmerged path git emits one record per stage, so the same working-tree file arrives three times.
  • select_source (:266) does not collapse them either: the loop appends one SelectedFile per candidate and only sorts by archive_name at the end (:340). Nothing keys on path, and nothing keys on the device/inode fields that SelectedFile already records (:135-136) — so a 66-byte app.py in a 3-stage merge becomes three manifest entries, 198 bytes counted, and three same-name members in the ZIP, matching the report's numbers exactly.
  • The fix can be narrow because the records are byte-identical under -z (no --stage flag is passed, so stage numbers never even reach the parser): dedupe the yielded candidates by relative path in _git_candidate_paths, or by archive_name in select_source before appending. The recorded device+inode pair is a belt-and-braces second key if a path-based dedupe is ever considered too aggressive, but for the reported case path dedupe alone is sufficient and cannot drop a genuinely distinct file.

Scope honesty: the git behavior (one line per stage for an unmerged path) was verified in a local empty repo, and the no-dedupe chain above is source-verified at the pinned tip; the full strix upload path was not runtime-executed, so the ZIP-member claim rests on the report's observation plus this chain, not on my own run.