fix(release): repair macOS Chromium clone base (#2457)

Author: shadowfax92

.github/scripts/macos-chromium-workspace.sh

@@ -248,31 +248,54 @@ check_no_browseros_outputs() {
       \( -name 'Default_browseros_*' -o -name 'Default_browserclaw_*' \) \
       -print -quit
   )"
-  [ -z "$found" ] || die "Persistent Chromium base contains BrowserOS output state: $found; remove BrowserOS out dirs from the base before rerunning"
+  [ -z "$found" ] || die "Could not remove BrowserOS output state from the CI-owned Chromium base: $found"
 }
 
 check_git_repo_clean() {
   local repo="$1"
   local status
 
   status="$(git -C "$repo" status --porcelain=v1 --untracked-files=all)"
-  [ -z "$status" ] || die "Persistent Chromium base has tracked or untracked changes in $repo; reset the warm base checkout before rerunning"
+  [ -z "$status" ] || die "Could not restore the CI-owned Chromium base repository to a clean state: $repo"
 }
 
-check_nested_git_repos_clean() {
+repair_git_repo() {
+  local repo="$1"
+  local ref="$2"
+
+  git -C "$repo" reset --hard "$ref"
+  git -C "$repo" clean -fd
+  check_git_repo_clean "$repo"
+}
+
+repair_nested_git_repos() {
   local git_meta repo
 
   while IFS= read -r git_meta; do
     repo="$(resolve_existing_dir "$(dirname "$git_meta")")" || continue
     [ "$repo" != "$base_src" ] || continue
-    check_git_repo_clean "$repo"
+    repair_git_repo "$repo" HEAD
   done < <(
     find "$base_root" \
       \( -path "$base_src/out" -o -path "$base_src/out/*" \) -prune -o \
       -name .git -print -prune
   )
 }
 
+remove_browseros_outputs() {
+  local out_dir="$1/out"
+  local output
+
+  [ -d "$out_dir" ] || return 0
+  for output in \
+    "$out_dir"/Default_browseros_* \
+    "$out_dir"/Default_browserclaw_*; do
+    [ -d "$output" ] || continue
+    rm -rf "$output"
+  done
+  check_no_browseros_outputs "$1"
+}
+
 verify_cow_clone_support() {
   local parent="$1"
   local probe_dir="$parent/.browseros-ci-apfs-probe-$(run_tag)-$$"
@@ -293,25 +316,29 @@ verify_cow_clone_support() {
   rm -rf "$probe_dir"
 }
 
-verify_base() {
+prepare_base() {
   local version_file="$1"
-  local pin_head
+  local current_head pin_head
 
   [ -f "$base_root/.gclient" ] || die "Chromium base root is missing .gclient: $base_root"
   [ -e "$base_src/.git" ] || die "Chromium base src is missing .git: $base_src"
   [ -f "$version_file" ] || die "Chromium version file not found: $version_file"
 
   chromium_version="$(read_chromium_version "$version_file")" \
     || die "Could not parse Chromium version file: $version_file"
-  base_head="$(git -C "$base_src" rev-parse HEAD)"
+  current_head="$(git -C "$base_src" rev-parse HEAD)"
   pin_head="$(git -C "$base_src" rev-parse "refs/tags/$chromium_version^{commit}")" \
     || die "Chromium base is missing pinned tag $chromium_version; refresh the warm base checkout before rerunning"
-  [ "$base_head" = "$pin_head" ] \
-    || die "Chromium base HEAD $base_head does not match pinned $chromium_version ($pin_head); refresh the warm base checkout before rerunning"
-
-  check_git_repo_clean "$base_src"
-  check_nested_git_repos_clean
-  check_no_browseros_outputs "$base_src"
+  [ "$current_head" = "$pin_head" ] \
+    || die "Chromium base HEAD $current_head does not match pinned $chromium_version ($pin_head); refresh the warm base checkout before rerunning"
+
+  # BROWSEROS_CHROMIUM_SRC is infrastructure-owned clone input, never a
+  # developer workspace. Resetting it is intentionally destructive: all build
+  # mutations belong in the disposable APFS clone created after this seam.
+  repair_git_repo "$base_src" "$pin_head"
+  repair_nested_git_repos
+  remove_browseros_outputs "$base_src"
+  base_head="$(git -C "$base_src" rev-parse HEAD)"
 }
 
 cleanup_after_setup_error() {
@@ -359,7 +386,7 @@ setup_workspace() {
   tag="$(run_tag)"
   reap_stale_workspaces "$workspace_parent" "$tag" "$base_root"
   version_file="${version_file:-$(default_version_file)}"
-  verify_base "$version_file"
+  prepare_base "$version_file"
 
   workspace_root="$workspace_parent/$workspace_prefix$tag"
   workspace_src="$workspace_root/src"

packages/browseros/bos_build/ci_workflow_test.py

@@ -1641,6 +1641,7 @@ def setUp(self):
         self.github_env = self.root / "github_env"
         self.github_output = self.root / "github_output"
         self.git_log = self.root / "git.log"
+        self.git_clean_log = self.root / "git-clean.log"
         self.cp_log = self.root / "cp.log"
         self.version_file = self.root / "CHROMIUM_VERSION"
         self.version_file.write_text(
@@ -1701,6 +1702,14 @@ def _write_fake_git(self):
 fi
 cmd="${1:-}"
 shift || true
+repo="${repo:-.}"
+repo_was_cleaned() {
+  [ -f "$GIT_CLEAN_LOG" ] || return 1
+  while IFS= read -r cleaned_repo; do
+    [ "$cleaned_repo" = "$repo" ] && return 0
+  done < "$GIT_CLEAN_LOG"
+  return 1
+}
 case "$cmd" in
   rev-parse)
     target="${1:-}"
@@ -1717,6 +1726,7 @@ def _write_fake_git(self):
     esac
     ;;
   status)
+    repo_was_cleaned && exit 0
     if [ -n "${GIT_DIRTY_REPO:-}" ]; then
       if [ "$repo" = "$GIT_DIRTY_REPO" ]; then
         printf '%b' "${GIT_DIRTY_STATUS:- M nested-change\\n}"
@@ -1725,6 +1735,9 @@ def _write_fake_git(self):
       printf '%b' "${GIT_STATUS:-}"
     fi
     ;;
+  clean)
+    printf '%s\\n' "$repo" >> "$GIT_CLEAN_LOG"
+    ;;
 esac
 """
         )
@@ -1761,6 +1774,7 @@ def _env(self, **overrides):
                 "BROWSEROS_CHROMIUM_VERSION_FILE": str(self.version_file),
                 "CP_LOG": str(self.cp_log),
                 "GIT_HEAD": self.head,
+                "GIT_CLEAN_LOG": str(self.git_clean_log),
                 "GIT_LOG": str(self.git_log),
                 "GITHUB_ENV": str(self.github_env),
                 "GITHUB_OUTPUT": str(self.github_output),
@@ -1874,7 +1888,7 @@ def test_setup_reaps_only_marked_stale_owned_workspaces(self):
         self.assertTrue(Path(self._outputs()["workspace_root"]).exists())
         self.assertTrue(self.base_root.exists())
 
-    def test_setup_reaps_stale_workspaces_before_dirty_base_failure(self):
+    def test_setup_reaps_stale_workspaces_before_repairing_dirty_base(self):
         parent = self._workspace_parent()
         parent.mkdir()
         stale = self._workspace_root("old-1")
@@ -1888,10 +1902,9 @@ def test_setup_reaps_stale_workspaces_before_dirty_base_failure(self):
             GIT_STATUS=" M chrome/app/generated_resources.grd\n",
         )
 
-        self.assertNotEqual(result.returncode, 0)
-        self.assertIn("tracked or untracked changes", result.stderr + result.stdout)
+        self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
         self.assertFalse(stale.exists())
-        self.assertFalse(self._workspace_root().exists())
+        self.assertTrue(self._workspace_root().exists())
         self.assertTrue(self.base_root.exists())
 
     def test_cleanup_ignores_unsafe_state_target(self):
@@ -1969,29 +1982,27 @@ def test_setup_fails_when_base_is_not_at_pinned_chromium_tag(self):
         self.assertIn("does not match pinned", result.stderr + result.stdout)
         self.assertFalse(self._workspace_root().exists())
 
-    def test_setup_fails_when_base_has_tracked_changes(self):
+    def test_setup_repairs_base_with_tracked_changes(self):
         result = self._run_helper(
             "setup",
             self.base_src,
             GIT_STATUS=" M chrome/app/generated_resources.grd\n",
         )
 
-        self.assertNotEqual(result.returncode, 0)
-        self.assertIn("tracked or untracked changes", result.stderr + result.stdout)
-        self.assertFalse(self._workspace_root().exists())
+        self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
+        self.assertTrue(self._workspace_root().exists())
 
-    def test_setup_fails_when_base_has_untracked_changes(self):
+    def test_setup_repairs_base_with_untracked_changes(self):
         result = self._run_helper(
             "setup",
             self.base_src,
             GIT_STATUS="?? chrome/browser/browseros/generated_resources.grd\n",
         )
 
-        self.assertNotEqual(result.returncode, 0)
-        self.assertIn("tracked or untracked changes", result.stderr + result.stdout)
-        self.assertFalse(self._workspace_root().exists())
+        self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
+        self.assertTrue(self._workspace_root().exists())
 
-    def test_setup_fails_when_nested_gclient_repo_has_changes(self):
+    def test_setup_repairs_nested_gclient_repo_with_changes(self):
         nested_repo = self.base_src / "third_party" / "v8"
         nested_repo.mkdir(parents=True)
         (nested_repo / ".git").mkdir()
@@ -2003,20 +2014,18 @@ def test_setup_fails_when_nested_gclient_repo_has_changes(self):
             GIT_DIRTY_STATUS=" M src/builtins/generated.cc\n",
         )
 
-        self.assertNotEqual(result.returncode, 0)
-        self.assertIn(str(nested_repo.resolve()), result.stderr + result.stdout)
-        self.assertIn("tracked or untracked changes", result.stderr + result.stdout)
-        self.assertFalse(self._workspace_root().exists())
+        self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
+        self.assertTrue(self._workspace_root().exists())
 
-    def test_setup_fails_when_base_has_browseros_output_dirs(self):
+    def test_setup_removes_browseros_output_dirs_from_base(self):
         out_dir = self.base_src / "out" / "Default_browseros_arm64"
         out_dir.mkdir(parents=True)
 
         result = self._run_helper("setup", self.base_src)
 
-        self.assertNotEqual(result.returncode, 0)
-        self.assertIn("BrowserOS output state", result.stderr + result.stdout)
-        self.assertFalse(self._workspace_root().exists())
+        self.assertEqual(result.returncode, 0, result.stderr + result.stdout)
+        self.assertFalse(out_dir.exists())
+        self.assertTrue(self._workspace_root().exists())
 
 
 @unittest.skipIf(os.name == "nt", "macOS signing helper shell tests run on POSIX")

packages/browseros/bos_build/docs/nightly-macos-ci.md

@@ -140,8 +140,9 @@ commonly fail with `User interaction not allowed`.
 The machine needs:
 
 - A persistent BrowserOS checkout.
-- A persistent pristine Chromium `src` checkout at the repository pin, used
-  only as the APFS clone base.
+- A persistent Chromium `src` checkout at the repository pin, dedicated to CI
+  as the APFS clone base. Its local changes and BrowserOS outputs are
+  disposable.
 - `uv`, `gh`, depot_tools, Xcode tools, and Chrome.
 - The macOS signing identity and notarization credentials.
 - Enough disk for Chromium outputs and DMGs.
@@ -151,19 +152,22 @@ Set these repository variables:
 | Variable | Meaning |
 | --- | --- |
 | `BROWSEROS_REPO_PATH` | Absolute path to the persistent BrowserOS checkout |
-| `BROWSEROS_CHROMIUM_SRC` | Absolute path to the warm pristine Chromium base `src` |
+| `BROWSEROS_CHROMIUM_SRC` | Absolute path to the warm, CI-owned Chromium clone-base `src` |
 
 Component, R2, signing, and notarization credentials come from GitHub Actions
 secrets. `SLACK_WEBHOOK_URL` is optional and receives failures after the Mac job
 has started.
 
 Before each signed browser build, `.github/scripts/macos-chromium-workspace.sh`
-validates the base checkout and creates a run/attempt-specific APFS
-copy-on-write clone of the whole gclient root under
+proves that the base is at the configured Chromium tag, destructively resets
+tracked and untracked state in the base and nested gclient repositories, and
+removes BrowserOS-owned output directories. It then creates a
+run/attempt-specific APFS copy-on-write clone of the whole gclient root under
 `../browseros-ci-apfs-workspaces/`. `bos_build` receives the clone's `src`, so
 cleaning, patching, compiling, signing, packaging, and universal merge outputs
 stay inside the disposable workspace. Cleanup runs with `if: always()`, and the
-next setup reaps abandoned owned workspaces from killed jobs.
+next setup reaps abandoned owned workspaces from killed jobs. Never point
+`BROWSEROS_CHROMIUM_SRC` at a developer checkout.
 
 ## Troubleshooting
 
@@ -182,8 +186,9 @@ verify `MACOS_KEYCHAIN_PASSWORD` and the signing identity.
 
 APFS workspace setup fails: confirm the base checkout is on APFS, the helper can
 create a same-volume `browseros-ci-apfs-workspaces` sibling directory, the base
-`src` is at `packages/browseros/CHROMIUM_VERSION`, and the base has no
-BrowserOS output directories or tracked patch/resource changes.
+`src` is checked out at `packages/browseros/CHROMIUM_VERSION`, and that pinned
+tag exists locally. Ordinary tracked changes, untracked files, nested-repository
+changes, and BrowserOS output directories are repaired automatically.
 
 No browser version commit: inspect the hosted `Reserve new browser version on
 main` job. It requires `contents: write` and `pull-requests: write`, plus branch

packages/browseros/bos_build/docs/release-ci.md

@@ -186,8 +186,9 @@ matching signing key and build-time secrets.
 Windows signing needs the eSigner secrets and `SPARKLE_PRIVATE_KEY`. macOS uses
 repository variables `BROWSEROS_REPO_PATH` and `BROWSEROS_CHROMIUM_SRC` plus
 the signing and notarization secrets on the persistent builder.
-`BROWSEROS_CHROMIUM_SRC` is the pristine APFS clone base; the release build
-runs against a disposable copy-on-write workspace and cleans it under
-`if: always()`. Runner labels, cache behavior, and queue recovery are
-documented in `warpbuild-ci.md`; persistent macOS setup is in
-`nightly-macos-ci.md`.
+`BROWSEROS_CHROMIUM_SRC` is a dedicated, CI-owned APFS clone base. Setup keeps
+its pinned Chromium identity strict but repairs local Git changes and
+BrowserOS-owned output directories before the release runs against a disposable
+copy-on-write workspace. The workspace is cleaned under `if: always()`. Runner
+labels, cache behavior, and queue recovery are documented in `warpbuild-ci.md`;
+persistent macOS setup is in `nightly-macos-ci.md`.