From c8875320a147a77911aa3cdfde2adad34291379b Mon Sep 17 00:00:00 2001 From: lukyturnflow <3421906768@qq.com> Date: Thu, 22 Oct 2026 15:42:32 +0800 Subject: [PATCH] fix(workspace): publish the snapshot text cache byte-exactly on every host (#7110) * fix(workspace): publish the snapshot text cache byte-exactly on every host * test(workspace): pin diff line counts and guard the text cache on Windows CI * ci(workspace): run Windows text-cache guards on source changes Include the workspace_changes package in the Windows workflow's PR path filter so scanner or diff regressions run the native Windows guards before merge, rather than waiting for the weekly schedule. --------- Co-authored-by: Willem Jiang --- .../workflows/backend-windows-live-policy.yml | 14 ++++++- .../deerflow/workspace_changes/diff.py | 6 ++- .../deerflow/workspace_changes/scanner.py | 11 +++++- .../tests/test_scanner_text_cache_atomic.py | 15 ++++++++ backend/tests/test_workspace_changes.py | 38 ++++++++++++++++++- 5 files changed, 77 insertions(+), 7 deletions(-) diff --git a/.github/workflows/backend-windows-live-policy.yml b/.github/workflows/backend-windows-live-policy.yml index 0158d1edfa..d5b435dba6 100644 --- a/.github/workflows/backend-windows-live-policy.yml +++ b/.github/workflows/backend-windows-live-policy.yml @@ -1,6 +1,8 @@ name: Windows Live Test Policy -# These offline policy tests spawn pytest children that collect live tests. +# These offline policy tests spawn pytest children that collect live tests, +# and the workspace text-cache tests pin byte-exact cache round-trips whose +# regression only fails on native Windows (see test_scanner_text_cache_atomic). on: workflow_dispatch: schedule: @@ -9,8 +11,11 @@ on: types: [opened, synchronize, reopened, ready_for_review] paths: - '.github/workflows/backend-windows-live-policy.yml' + - 'backend/packages/harness/deerflow/workspace_changes/**' - 'backend/tests/test_client_live_policy.py' - 'backend/tests/test_client_live.py' + - 'backend/tests/test_scanner_text_cache_atomic.py' + - 'backend/tests/test_workspace_changes.py' - 'backend/pyproject.toml' - 'backend/uv.lock' @@ -48,8 +53,11 @@ jobs: - name: Install backend dependencies run: uv sync --locked --group dev - - name: Check isolated child environment and live-test collection policy + - name: Check isolated child environment, live-test collection policy, and text-cache byte exactness # Explicit nodes include all four collection cases and avoid make. + # The two workspace-changes modules pin byte-exact text-cache + # round-trips whose regression (newline translation) only fails on + # native Windows — ubuntu CI cannot catch it (PR #7110 review). run: >- uv run python -m pytest tests/test_client_live_policy.py::test_child_env_stays_isolated_but_bootable_on_windows @@ -57,4 +65,6 @@ jobs: tests/test_client_live_policy.py::test_explicit_opt_in_collects_live_tests tests/test_client_live_policy.py::test_ci_blocks_live_collection_even_with_opt_in tests/test_client_live_policy.py::test_opt_in_without_config_reports_missing_config + tests/test_scanner_text_cache_atomic.py + tests/test_workspace_changes.py -q diff --git a/backend/packages/harness/deerflow/workspace_changes/diff.py b/backend/packages/harness/deerflow/workspace_changes/diff.py index 2035dd4aab..1f81c1b5a1 100644 --- a/backend/packages/harness/deerflow/workspace_changes/diff.py +++ b/backend/packages/harness/deerflow/workspace_changes/diff.py @@ -198,7 +198,11 @@ def _snapshot_text(file: FileSnapshot | None) -> str | None: return file.text if file.text_path: try: - with open(file.text_path, encoding="utf-8") as cached: + # The cache is published byte-exactly (the scanner pins + # newline="\n"), so read it back without newline translation: + # universal newlines would silently rewrite a decoded CRLF + # snapshot into LF text and corrupt the served content. + with open(file.text_path, encoding="utf-8", newline="") as cached: return cached.read() except OSError: return None diff --git a/backend/packages/harness/deerflow/workspace_changes/scanner.py b/backend/packages/harness/deerflow/workspace_changes/scanner.py index 002041a189..fb7944b1db 100644 --- a/backend/packages/harness/deerflow/workspace_changes/scanner.py +++ b/backend/packages/harness/deerflow/workspace_changes/scanner.py @@ -328,10 +328,17 @@ def _snapshot_symlink(root: WorkspaceRoot, host_file: Path) -> FileSnapshot | No def _publish_text_atomically(path: Path, content: str) -> None: - """Publish text through a temporary file so a partial write cannot survive.""" + """Publish text through a temporary file so a partial write cannot survive. + + ``newline="\\n"`` keeps the publication byte-exact on every host: the + default text-mode translation would rewrite a decoded CRLF snapshot's + ``\\n`` characters into ``os.linesep`` on Windows, turning ``\\r\\n`` + content into ``\\r\\r\\n`` bytes that every later reader serves verbatim + as the file's text. + """ fd, temporary = tempfile.mkstemp(dir=str(path.parent), prefix=f".{path.name}.", suffix=".tmp") try: - with os.fdopen(fd, "w", encoding="utf-8") as handle: + with os.fdopen(fd, "w", encoding="utf-8", newline="\n") as handle: handle.write(content) handle.flush() os.fsync(handle.fileno()) diff --git a/backend/tests/test_scanner_text_cache_atomic.py b/backend/tests/test_scanner_text_cache_atomic.py index 12f010178b..823d6e67be 100644 --- a/backend/tests/test_scanner_text_cache_atomic.py +++ b/backend/tests/test_scanner_text_cache_atomic.py @@ -93,6 +93,21 @@ def test_cache_text_file_returns_a_complete_entry(tmp_path, scanner): assert Path(returned).name == hashlib.sha256(virtual_path.encode()).hexdigest() +def test_cache_text_file_publishes_decoded_crlf_content_byte_exactly(tmp_path, scanner): + """A decoded CRLF snapshot must reach the cache verbatim on every host. + + The default text-mode write translates ``\\n`` into ``os.linesep``, which + on native Windows turns ``\\r\\n`` content into ``\\r\\r\\n`` bytes -- the + cache is then served as the file's text with a spurious blank line. + """ + cache_dir = tmp_path / "cache" + cache_dir.mkdir() + + returned = scanner._cache_text_file("a,b\r\n1,2\r\n", "/workspace/report.csv", cache_dir) + + assert Path(returned).read_bytes() == b"a,b\r\n1,2\r\n" + + def test_cache_entry_survives_a_failed_publish(tmp_path, monkeypatch, scanner): """The real entry point: a failed publish must not damage the cached entry.""" cache_dir = tmp_path / "cache" diff --git a/backend/tests/test_workspace_changes.py b/backend/tests/test_workspace_changes.py index 40834f6715..c0f4d5c24e 100644 --- a/backend/tests/test_workspace_changes.py +++ b/backend/tests/test_workspace_changes.py @@ -77,15 +77,49 @@ def test_compare_snapshots_reports_text_file_changes(tmp_path): assert changes["/mnt/user-data/workspace/old.txt"].status == "deleted" +def test_cached_crlf_text_round_trips_byte_exactly_into_diffs(tmp_path): + """The text cache is published and read back byte-exactly on every host. + + The pre-run snapshot carries its text through ``_publish_text_atomically`` + (write) and ``_snapshot_text`` (read) while the post-run snapshot carries + text inline, exactly like ``record_workspace_changes``. Host newline + translation on either cache side would corrupt the served text and the + built diff (a spurious blank line per CRLF line on native Windows). + """ + roots = _roots(tmp_path) + (roots[0].host_path / "notes.md").write_bytes(b"first\r\nsecond\r\n") + before_cache = tmp_path / "cache-before" + before = scan_workspace_roots(roots, text_cache_dir=before_cache) + + (roots[0].host_path / "notes.md").write_bytes(b"first\r\nthird\r\n") + after = scan_workspace_roots(roots) + + assert before.files["/mnt/user-data/workspace/notes.md"].text_path is not None + result = compare_snapshots(before, after) + + change = {c.path: c for c in result.files}["/mnt/user-data/workspace/notes.md"] + assert "-second" in change.diff + assert "+third" in change.diff + # Pin the line counts: a host that translates newlines on the cache write + # and re-translates on read rounds the cached text up to "first\n\nsecond\n" + # (one spurious blank line per CRLF line), which shows up here as an extra + # deletion line. "-second"/"+third" alone stay present under that bug, so + # only the counts catch it — on any host, not just native Windows. + assert (change.additions, change.deletions) == (1, 1), change.diff + + @pytest.mark.parametrize("include_text", [False, True]) @pytest.mark.parametrize("size", [16, SAMPLE_BYTES + 1]) def test_scan_keeps_other_files_when_a_file_disappears_before_hashing(tmp_path, monkeypatch, include_text, size): roots = _roots(tmp_path) workspace, outputs = [root.host_path for root in roots] - (workspace / "a-kept.txt").write_text("kept\n", encoding="utf-8") + # Byte-pinned fixtures (tests/AGENTS.md): a text-mode write translates the + # LF byte into CRLF on native Windows, and the compared snapshot text + # would then round-trip as "report\r\n" instead of "report\n". + (workspace / "a-kept.txt").write_bytes(b"kept\n") vanished = workspace / "scratch.txt" vanished.write_text("x" * size, encoding="utf-8") - (outputs / "report.md").write_text("report\n", encoding="utf-8") + (outputs / "report.md").write_bytes(b"report\n") real_hash = scanner._sha256_file removed = []