refactor(artifacts): harmonize Windows to Linux raw-file collection

Windows produced an intermediate C:\CI\output\artifacts.zip that
upload-artifact then re-zipped (zip-in-zip), while Linux deposited raw
files. Make both paths consistent: the build stages raw outputs into a
collect dir; collect zips it only as a transport archive, fetches, and
extracts into the host artifact dir; actions/upload-artifact produces
the single final archive.

- _windows_build: param artifact_zip -> output_dir; stage raw files
  (in-place when source IS the collect dir, else wipe+copy), no
  Compress-Archive; absolute artifact-source resolved.
- _windows_collect: zip guest dir as transport, fetch, extract (mirror
  _linux_collect); guest_path is now the collect directory.
- job.py / build_run CLI: pass output_dir; rename --guest-artifact-zip
  -> --guest-output-dir.
- Tests updated for the raw-file contract.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

src/ci_orchestrator/commands/artifacts.py

@@ -16,6 +16,7 @@ import json
 import shlex
 import tarfile
 import tempfile
+import zipfile
 from datetime import UTC, datetime
 from pathlib import Path
 
@@ -133,19 +134,44 @@ def _windows_collect(
     include_logs: bool,
 ) -> None:
     cred = KeyringCredentialStore().get(credential_target)
+    remote_zip = "C:\\CI\\ci-artifacts-transfer.zip"
     with WinRmTransport(ip_address, cred.username, cred.password) as t:
-        # Verify artifact exists.
+        # guest_path is the collect directory (raw build outputs).
+        # Verify it exists and is non-empty.
         probe = t.run(
-            f"if (Test-Path {_ps_quote(guest_path)}) {{ 'OK' }} else {{ 'MISSING' }}",
+            f"if ((Test-Path {_ps_quote(guest_path)}) -and "
+            f"(Get-ChildItem -Force -- {_ps_quote(guest_path)})) "
+            f"{{ 'OK' }} else {{ 'MISSING' }}",
             check=False,
         )
         if "MISSING" in probe.stdout:
-            raise click.ClickException(f"artifact not found in guest: {guest_path}")
+            raise click.ClickException(
+                f"artifact not found or empty in guest: {guest_path}"
+            )
 
-        artifact_name = Path(guest_path).name
-        host_dest = host_dir / artifact_name
-        click.echo(f"[artifacts collect] fetching {artifact_name}")
-        t.fetch(guest_path, str(host_dest))
+        # Zip the directory contents as a single transport file, fetch,
+        # extract into host_dir. Mirrors the Linux tar flow so both OSes
+        # deposit raw files; the final single archive is produced by
+        # actions/upload-artifact (no zip-in-zip).
+        t.run(
+            f"if (Test-Path {_ps_quote(remote_zip)}) "
+            f"{{ Remove-Item {_ps_quote(remote_zip)} -Force }}; "
+            f"Compress-Archive -Path (Join-Path {_ps_quote(guest_path)} '*') "
+            f"-DestinationPath {_ps_quote(remote_zip)} -Force"
+        )
+        click.echo(f"[artifacts collect] fetching artifacts from {guest_path}")
+        try:
+            with tempfile.TemporaryDirectory() as td:
+                local_zip = Path(td) / "artifacts.zip"
+                t.fetch(remote_zip, str(local_zip))
+                with zipfile.ZipFile(local_zip) as zf:
+                    zf.extractall(host_dir)
+        finally:
+            t.run(
+                f"Remove-Item {_ps_quote(remote_zip)} -Force "
+                f"-ErrorAction SilentlyContinue",
+                check=False,
+            )
 
         if include_logs:
             # List log files in the build dir and fetch each one.
@@ -169,9 +195,9 @@ def _windows_collect(
                         err=True,
                     )
 
-        if not host_dest.is_file() or host_dest.stat().st_size == 0:
+        if not any(host_dir.iterdir()):
             raise click.ClickException(
-                f"artifact missing or empty after fetch: {host_dest}"
+                f"no artifacts after collect from guest: {guest_path}"
             )
 
 
@@ -190,7 +216,7 @@ def _windows_collect(
     "--guest-artifact-path",
     "guest_artifact_path",
     required=True,
-    help="Guest path: file (Windows) or directory (Linux).",
+    help="Guest collect directory (Windows and Linux).",
 )
 @click.option(
     "--host-artifact-dir",

src/ci_orchestrator/commands/build.py

@@ -231,7 +231,7 @@ def _windows_build(
     ip_address: str,
     credential_target: str,
     workdir: str,
-    artifact_zip: str,
+    output_dir: str,
     host_source_dir: str | None,
     clone_url: str | None,
     clone_branch: str,
@@ -251,7 +251,7 @@ def _windows_build(
         # Prepare workdir + artifact output directory.
         prep_ps = (
             f"$work = {_ps_quote(workdir)}; "
-            f"$out  = Split-Path {_ps_quote(artifact_zip)} -Parent; "
+            f"$out  = {_ps_quote(output_dir)}; "
             "if (-not (Test-Path $out)) { New-Item -ItemType Directory -Path $out -Force | Out-Null }; "
             "if (Test-Path $work) { Remove-Item $work -Recurse -Force }; "
             "New-Item -ItemType Directory -Path $work -Force | Out-Null"
@@ -346,34 +346,44 @@ def _windows_build(
             click.echo("[build run] artifact packaging skipped (--skip-artifact).")
             return
 
-        # Package the artifact source into the requested zip on the guest.
+        # Stage the artifact source into the collect dir as raw files
+        # (no intermediate zip — `actions/upload-artifact` produces the
+        # single final archive; mirrors the Linux flow, avoids zip-in-zip).
         # artifact_source may be absolute (e.g. C:\CI\output) or relative
-        # to the build workdir. PowerShell Join-Path does NOT treat a
-        # rooted second segment as absolute (it concatenates, yielding
-        # C:\CI\build\C:\CI\output), so resolve it explicitly.
-        if build_command:
-            src_rel = artifact_source or "dist"
-        else:
-            src_rel = "bin\\CIOutput"
+        # to the build workdir; PowerShell Join-Path does not treat a
+        # rooted segment as absolute, so resolve explicitly. When the
+        # source IS the collect dir (build wrote there directly), collect
+        # in place — do NOT wipe it (that deleted the build output).
+        src_rel = (artifact_source or "dist") if build_command else "bin\\CIOutput"
 
         pkg_ps = (
             f"$work = {_ps_quote(workdir)}; "
             f"$rel  = {_ps_quote(src_rel)}; "
             "$src = if ([System.IO.Path]::IsPathRooted($rel)) "
             "{ $rel } else { Join-Path $work $rel }; "
-            f"$zip = {_ps_quote(artifact_zip)}; "
-            "if (-not (Test-Path $src)) { "
-            "  throw \"[build run] artifact source not found: $src\" }; "
-            "$dir = Split-Path $zip -Parent; "
-            "if (-not (Test-Path $dir)) { New-Item -ItemType Directory -Path $dir -Force | Out-Null }; "
-            "if (Test-Path $zip) { Remove-Item $zip -Force }; "
-            "if (Test-Path $src -PathType Leaf) { "
-            "  Compress-Archive -Path $src -DestinationPath $zip -Force "
+            f"$out = {_ps_quote(output_dir)}; "
+            "$sf = [System.IO.Path]::GetFullPath($src).TrimEnd('\\'); "
+            "$of = [System.IO.Path]::GetFullPath($out).TrimEnd('\\'); "
+            "$inPlace = ($sf -ieq $of) -or "
+            "$sf.StartsWith($of + '\\', "
+            "[System.StringComparison]::OrdinalIgnoreCase); "
+            "if ($inPlace) { "
+            "  if (-not (Test-Path $out) -or "
+            "      -not (Get-ChildItem -Force -- $out)) { "
+            "    throw \"[build run] artifact source not found or empty: $out\" } "
             "} else { "
-            "  Compress-Archive -Path (Join-Path $src '*') -DestinationPath $zip -Force }"
+            "  if (-not (Test-Path $src)) { "
+            "    throw \"[build run] artifact source not found: $src\" }; "
+            "  if (Test-Path $out) { Remove-Item $out -Recurse -Force }; "
+            "  New-Item -ItemType Directory -Path $out -Force | Out-Null; "
+            "  if (Test-Path $src -PathType Leaf) { "
+            "    Copy-Item $src -Destination $out -Force "
+            "  } else { "
+            "    Copy-Item (Join-Path $src '*') -Destination $out -Recurse -Force } "
+            "}"
         )
         t.run(pkg_ps)
-        click.echo(f"[build run] artifact written to {artifact_zip}")
+        click.echo(f"[build run] artifact staged at {output_dir}")
 
 
 # ---------------------------------------------------------------- CLI
@@ -426,10 +436,11 @@ def _windows_build(
     help="Workdir inside the guest (defaults: Win=C:\\CI\\build, Linux=/opt/ci/build).",
 )
 @click.option(
-    "--guest-artifact-zip",
-    "guest_artifact_zip",
+    "--guest-output-dir",
+    "guest_output_dir",
     default=None,
-    help="Artifact zip path inside the Windows guest (default: C:\\CI\\output\\artifacts.zip).",
+    help="Collect dir inside the Windows guest (default: C:\\CI\\output). "
+    "Raw build outputs are staged here; upload-artifact zips them.",
 )
 @click.option(
     "--guest-linux-work-dir",
@@ -468,7 +479,7 @@ def build_run(
     gitea_credential_target: str,
     build_command: str,
     guest_work_dir: str | None,
-    guest_artifact_zip: str | None,
+    guest_output_dir: str | None,
     guest_linux_work_dir: str | None,
     guest_linux_output_dir: str,
     guest_artifact_source: str,
@@ -485,7 +496,7 @@ def build_run(
     * ``--host-source-dir`` — local directory zipped/tarred and uploaded;
     * ``--clone-url``       — ``git clone`` invoked inside the guest.
 
-    Phase C hook: ``--guest-work-dir`` and ``--guest-artifact-zip`` are
+    Phase C hook: ``--guest-work-dir`` and ``--guest-output-dir`` are
     parameterised; defaults are applied here, never inside the transport
     layer.
     """
@@ -533,14 +544,14 @@ def build_run(
             raise click.ClickException(str(exc)) from exc
     else:
         workdir = guest_work_dir or "C:\\CI\\build"
-        artifact_zip = guest_artifact_zip or "C:\\CI\\output\\artifacts.zip"
+        output_dir = guest_output_dir or "C:\\CI\\output"
         target = credential_target or config.guest_cred_target
         try:
             _windows_build(
                 ip_address=ip_address,
                 credential_target=target,
                 workdir=workdir,
-                artifact_zip=artifact_zip,
+                output_dir=output_dir,
                 host_source_dir=host_source_dir,
                 clone_url=clone_url,
                 clone_branch=clone_branch,

src/ci_orchestrator/commands/job.py

@@ -492,7 +492,7 @@ def job(
                 ip_address=ip_address,
                 credential_target=target,
                 workdir="C:\\CI\\build",
-                artifact_zip="C:\\CI\\output\\artifacts.zip",
+                output_dir="C:\\CI\\output",
                 host_source_dir=None,
                 clone_url=authed_repo_url,
                 clone_branch=branch,
@@ -526,7 +526,7 @@ def job(
                 _windows_collect(
                     ip_address=ip_address,
                     credential_target=target,
-                    guest_path="C:\\CI\\output\\artifacts.zip",
+                    guest_path="C:\\CI\\output",
                     host_dir=job_artifact_dir,
                     include_logs=True,
                 )

tests/python/test_commands_artifacts.py

@@ -4,6 +4,7 @@ from __future__ import annotations
 
 import json
 import tarfile
+import zipfile
 from pathlib import Path
 from typing import Any, ClassVar
 
@@ -78,8 +79,34 @@ def _patch(monkeypatch: pytest.MonkeyPatch) -> None:
 # ── Windows happy path ────────────────────────────────────────────────────
 
 
+def _zip_payload(tmp_path: Path) -> bytes:
+    """Build a real zip whose contents the fake transport will 'fetch'."""
+    src = tmp_path / "zsrc"
+    src.mkdir()
+    (src / "binary.bin").write_bytes(b"\x01\x02\x03")
+    (src / "log.txt").write_text("hello", encoding="utf-8")
+    zip_local = tmp_path / "transfer.zip"
+    with zipfile.ZipFile(zip_local, "w") as zf:
+        for f in src.iterdir():
+            zf.write(f, arcname=f.name)
+    return zip_local.read_bytes()
+
+
+def _win_transport_with_zip(payload: bytes) -> type:
+    class _WinTransport(_FakeTransport):
+        def fetch(self, remote_path: str, local_path: str) -> None:
+            self.fetched.append((remote_path, str(local_path)))
+            Path(local_path).write_bytes(payload)
+
+    return _WinTransport
+
+
 def test_collect_windows_happy_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
     _patch(monkeypatch)
+    payload = _zip_payload(tmp_path)
+    monkeypatch.setattr(
+        artifacts_module, "WinRmTransport", _win_transport_with_zip(payload)
+    )
     out_dir = tmp_path / "artifacts"
     result = CliRunner().invoke(
         cli,
@@ -89,18 +116,19 @@ def test_collect_windows_happy_path(monkeypatch: pytest.MonkeyPatch, tmp_path: P
             "--ip-address",
             "10.0.0.7",
             "--guest-artifact-path",
-            "C:\\CI\\output\\artifacts.zip",
+            "C:\\CI\\output",
             "--host-artifact-dir",
             str(out_dir),
         ],
     )
     assert result.exit_code == 0, result.output
-    assert (out_dir / "artifacts.zip").is_file()
-    instance = _FakeTransport.instances[0]
-    assert any("Test-Path" in c for c in instance.runs)
-    assert instance.fetched == [
-        ("C:\\CI\\output\\artifacts.zip", str(out_dir / "artifacts.zip"))
-    ]
+    # Raw files extracted into host_dir (no zip-in-zip).
+    assert (out_dir / "binary.bin").read_bytes() == b"\x01\x02\x03"
+    assert (out_dir / "log.txt").read_text(encoding="utf-8") == "hello"
+    cmds = _FakeTransport.instances[0].runs
+    assert any("Compress-Archive" in c for c in cmds)
+    # Temp transport zip on the guest is cleaned up.
+    assert any("Remove-Item" in c and "ci-artifacts-transfer.zip" in c for c in cmds)
 
 
 def test_collect_windows_artifact_missing(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
@@ -109,7 +137,9 @@ def test_collect_windows_artifact_missing(monkeypatch: pytest.MonkeyPatch, tmp_p
     class _Missing(_FakeTransport):
         def __init__(self, *a: Any, **kw: Any) -> None:
             super().__init__(*a, **kw)
-            self.run_overrides["Test-Path"] = _FakeResult(stdout="MISSING\r\n")
+            self.run_overrides["-and (Get-ChildItem"] = _FakeResult(
+                stdout="MISSING\r\n"
+            )
 
     monkeypatch.setattr(artifacts_module, "WinRmTransport", _Missing)
     result = CliRunner().invoke(
@@ -120,22 +150,23 @@ def test_collect_windows_artifact_missing(monkeypatch: pytest.MonkeyPatch, tmp_p
             "--ip-address",
             "10.0.0.7",
             "--guest-artifact-path",
-            "C:\\CI\\output\\artifacts.zip",
+            "C:\\CI\\output",
             "--host-artifact-dir",
             str(tmp_path / "out"),
         ],
     )
     assert result.exit_code != 0
-    assert "artifact not found" in result.output
+    assert "artifact not found or empty" in result.output
 
 
 def test_collect_windows_include_logs(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
     _patch(monkeypatch)
+    payload = _zip_payload(tmp_path)
 
-    class _WithLogs(_FakeTransport):
+    class _WithLogs(_win_transport_with_zip(payload)):  # type: ignore[misc]
         def __init__(self, *a: Any, **kw: Any) -> None:
             super().__init__(*a, **kw)
-            self.run_overrides["Get-ChildItem"] = _FakeResult(
+            self.run_overrides["C:\\CI\\build"] = _FakeResult(
                 stdout="C:\\CI\\build\\one.log\r\nC:\\CI\\build\\sub\\two.log\r\n"
             )
 
@@ -148,7 +179,7 @@ def test_collect_windows_include_logs(monkeypatch: pytest.MonkeyPatch, tmp_path:
             "--ip-address",
             "10.0.0.7",
             "--guest-artifact-path",
-            "C:\\CI\\output\\artifacts.zip",
+            "C:\\CI\\output",
             "--host-artifact-dir",
             str(tmp_path / "out"),
             "--include-logs",
@@ -164,6 +195,10 @@ def test_collect_writes_manifest_when_job_id_set(
     monkeypatch: pytest.MonkeyPatch, tmp_path: Path
 ) -> None:
     _patch(monkeypatch)
+    payload = _zip_payload(tmp_path)
+    monkeypatch.setattr(
+        artifacts_module, "WinRmTransport", _win_transport_with_zip(payload)
+    )
     out = tmp_path / "out"
     result = CliRunner().invoke(
         cli,
@@ -173,7 +208,7 @@ def test_collect_writes_manifest_when_job_id_set(
             "--ip-address",
             "10.0.0.7",
             "--guest-artifact-path",
-            "C:\\CI\\output\\artifacts.zip",
+            "C:\\CI\\output",
             "--host-artifact-dir",
             str(out),
             "--job-id",
@@ -189,7 +224,7 @@ def test_collect_writes_manifest_when_job_id_set(
     assert data["jobId"] == "run-42"
     assert data["commit"] == "abc1234"
     names = [f["name"] for f in data["files"]]
-    assert "artifacts.zip" in names
+    assert "binary.bin" in names and "log.txt" in names
 
 
 # ── Linux happy path ──────────────────────────────────────────────────────

tests/python/test_commands_build.py

@@ -360,5 +360,6 @@ def test_build_run_windows_host_source_dir(
     assert instance.copies[0][1] == "C:\\CI\\src-transfer.zip"
     cmds = [c[0] for c in instance.runs]
     assert any("Expand-Archive" in c for c in cmds)
-    # Packaging step references the configured artifact source dir.
-    assert any("Compress-Archive" in c and "'dist'" in c for c in cmds)
+    # Staging copies the raw source into the collect dir (no zip-in-zip).
+    assert any("Copy-Item" in c and "'dist'" in c for c in cmds)
+    assert not any("Compress-Archive" in c and "'dist'" in c for c in cmds)

tests/python/test_commands_job.py

@@ -322,8 +322,8 @@ def test_job_windows_branch_with_overrides(
     # CPU/RAM override patched the clone VMX.
     clone_vmx = next((tmp_path / "vms").rglob("*.vmx"), None)
     # After successful job the clone dir is deleted, so the file is gone.
-    # Check that the Windows collect helper got the correct artifact path instead.
-    assert calls["windows_collect"][0]["guest_path"].endswith("artifacts.zip")
+    # Check that the Windows collect helper got the collect dir instead.
+    assert calls["windows_collect"][0]["guest_path"] == "C:\\CI\\output"
     assert clone_vmx is None  # cleaned up
     assert backend.calls[-1] == "delete"