Fail a Menu Fetch Whose Old Hub or Marker Cannot Be Changed, and Take Its Token Only Once Written

host-setup/menu.ps1

@@ -257,10 +257,23 @@ function Invoke-FetchHubLocked {
     step "Fetching $script:HUB_REPO at $script:REF"
     if (-not (Test-HubRemovable)) { return $false }
     $hubPath = Join-Path $script:DIR 'hub'
-    if (Test-Path -LiteralPath $hubPath) { Remove-Item -LiteralPath $hubPath -Recurse -Force }
+    if (Test-Path -LiteralPath $hubPath) {
+        try {
+            Remove-Item -LiteralPath $hubPath -Recurse -Force
+        } catch {
+            fail "Could not remove $hubPath to fetch it again. Check that nothing holds a file under it: $($_.Exception.Message)"
+            return $false
+        }
+    }
     # Marked as ours before git can create anything under $hubPath, not only once the clone also succeeds: git can leave a partial directory behind on a failed or interrupted clone, and an unmarked one would then block every retry until removed by hand.
-    $script:HUB_FETCH_TOKEN = [guid]::NewGuid().ToString('N')
-    Set-Content -LiteralPath (Get-MarkerPath) -Value $script:HUB_FETCH_TOKEN
+    $token = [guid]::NewGuid().ToString('N')
+    try {
+        Set-Content -LiteralPath (Get-MarkerPath) -Value $token
+    } catch {
+        fail "Could not write $(Get-MarkerPath). Check that $script:DIR is writable: $($_.Exception.Message)"
+        return $false
+    }
+    $script:HUB_FETCH_TOKEN = $token
     # A full clone of the default branch first, whatever -Ref names: spec\audit.py walks the hub's own history to judge whether a carried copy is trailing the file it was copied from, and a shallow clone would read every file as changed at the truncation boundary.
     # Piped to Out-Host rather than left bare: an unassigned native call's stdout otherwise joins this function's own return value, which return $true/$false below would then be appended to instead of replacing.
     & git clone --quiet --branch $script:DEFAULT_REF --single-branch $script:HUB_URL $hubPath | Out-Host

host-setup/menu.sh

@@ -170,14 +170,23 @@ fetch_hub() {
 
 fetch_hub_locked() {
     step "Fetching $HUB_REPO at $REF"
-    if ! HUB_FETCH_TOKEN=$(od -An -N16 -tx1 /dev/urandom | tr -d ' \n') || [[ -z $HUB_FETCH_TOKEN ]]; then
+    local token
+    if ! token=$(od -An -N16 -tx1 /dev/urandom | tr -d ' \n') || [[ -z $token ]]; then
         fail "Could not read /dev/urandom for the token marking this fetch as this session's."
         return 1
     fi
     remove_unowned_hub_check || return 1
     rm -rf "$DIR/hub"
+    if [[ -e "$DIR/hub" || -L "$DIR/hub" ]]; then
+        fail "Could not remove $DIR/hub to fetch it again. Check that nothing holds a file under it."
+        return 1
+    fi
     # Marked as ours before git can create anything under $DIR/hub, not only once the clone also succeeds: git can leave a partial directory behind on a failed or interrupted clone, and an unmarked one would then block every retry until removed by hand.
-    printf '%s\n' "$HUB_FETCH_TOKEN" >"$(marker_path)"
+    if ! printf '%s\n' "$token" >"$(marker_path)"; then
+        fail "Could not write $(marker_path). Check that $DIR is writable."
+        return 1
+    fi
+    HUB_FETCH_TOKEN="$token"
     # A full clone of the default branch first, whatever $REF names: spec/audit.py walks the hub's own history to judge whether a carried copy is trailing the file it was copied from, and a shallow clone would read every file as changed at the truncation boundary and misreport every repo as stale.
     git clone --quiet --branch "$DEFAULT_REF" --single-branch "$HUB_URL" "$DIR/hub" ||
         {

tests/test_bootstrap.py

@@ -1451,14 +1451,40 @@ def test_a_token_that_cannot_be_read_fails_the_fetch_before_it_changes_anything(
         self.make_cache(self.TOKEN)
         result = self.run_body(
             'git() { mkdir -p "${!#}"; }\nod() { return 1; }\n'
-            'rc=0\nfetch_hub_locked || rc=$?\nprintf "rc=%s\\n" "$rc"\n'
+            f"HUB_FETCH_TOKEN={self.TOKEN}\nrc=0\nfetch_hub_locked || rc=$?\n"
+            'printf "rc=%s token=%s\\n" "$rc" "$HUB_FETCH_TOKEN"\n'
         )
         self.assertEqual(result.returncode, 0, result.stderr)
-        self.assertIn("rc=1", result.stdout)
+        self.assertIn(f"rc=1 token={self.TOKEN}", result.stdout)
         self.assertIn("Could not read /dev/urandom", result.stderr)
         self.assertTrue((self.dir / "hub" / "README.md").exists())
         self.assertEqual((self.dir / "hub.owned").read_text(encoding="utf-8"), self.TOKEN)
 
+    def test_a_tree_that_cannot_be_removed_fails_the_fetch_and_keeps_its_marker(self) -> None:
+        """A clone into the surviving tree would fail, and its cleanup would then drop the marker."""
+        self.make_cache(self.TOKEN)
+        result = self.run_body(
+            'git() { mkdir -p "${!#}"; }\nrm() { return 0; }\n'
+            f"HUB_FETCH_TOKEN={self.TOKEN}\nrc=0\nfetch_hub_locked || rc=$?\n"
+            'printf "rc=%s token=%s\\n" "$rc" "$HUB_FETCH_TOKEN"\n'
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn(f"rc=1 token={self.TOKEN}", result.stdout)
+        self.assertIn("Could not remove", result.stderr)
+        self.assertEqual((self.dir / "hub.owned").read_text(encoding="utf-8"), self.TOKEN)
+
+    def test_a_marker_that_cannot_be_written_fails_the_fetch_without_cloning(self) -> None:
+        """A directory at the marker's name refuses the write, which errexit would not catch here."""
+        (self.dir / "hub.owned").mkdir()
+        result = self.run_body(
+            f'git() {{ mkdir -p "${{!#}}"; }}\nHUB_FETCH_TOKEN={self.TOKEN}\nrc=0\n'
+            'fetch_hub_locked || rc=$?\nprintf "rc=%s token=%s\\n" "$rc" "$HUB_FETCH_TOKEN"\n'
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn(f"rc=1 token={self.TOKEN}", result.stdout)
+        self.assertIn("Could not write", result.stderr)
+        self.assertFalse((self.dir / "hub").exists())
+
     def run_body(self, body: str) -> subprocess.CompletedProcess[str]:
         script = self.scripts / "body.sh"
         script.write_text(
@@ -1658,6 +1684,13 @@ class TestPowerShellMenuHubCleanup(HubCleanupCases, unittest.TestCase):
     """`menu.ps1`'s `Invoke-Cleanup`, under its real lock, its marker ending CRLF as on Windows."""
 
     LINE_BREAK = "\r\n"
+    FETCH_SETUP = (
+        "$script:HUB_REPO = 'owner/hub'\n$script:HUB_URL = 'https://example.invalid/hub'\n"
+        "$script:DEFAULT_REF = 'main'\n$script:REF = 'main'\n$script:HUB_ROOT = ''\n"
+        "function step { param([string]$Message) }\n"
+        "function git { New-Item -ItemType Directory -Path $args[-1] -Force | Out-Null; "
+        "$global:LASTEXITCODE = 0 }\n"
+    )
 
     def setUp(self) -> None:
         self.dir = Path(self.enterContext(tempfile.TemporaryDirectory()))
@@ -1674,13 +1707,46 @@ def fetch_body(self, refetched: bool) -> str:
             "$script:HUB_FETCH_TOKEN = $mine\n"
         )
         return (
-            "$script:HUB_REPO = 'owner/hub'\n$script:HUB_URL = 'https://example.invalid/hub'\n"
-            "$script:DEFAULT_REF = 'main'\n$script:REF = 'main'\n$script:HUB_ROOT = ''\n"
-            "function step { param([string]$Message) }\n"
-            "function git { New-Item -ItemType Directory -Path $args[-1] -Force | Out-Null; "
-            "$global:LASTEXITCODE = 0 }\n"
-            "[void](Invoke-FetchHubLocked)\n" + (again if refetched else "") + "Invoke-Cleanup\n"
+            self.FETCH_SETUP
+            + "[void](Invoke-FetchHubLocked)\n"
+            + (again if refetched else "")
+            + "Invoke-Cleanup\n"
+        )
+
+    def test_a_tree_that_cannot_be_removed_fails_the_fetch_and_keeps_its_marker(self) -> None:
+        """It fails the one task rather than ending the menu's whole session.
+
+        The refusal is stubbed, since what holds a file open differs by platform.
+        """
+        self.make_cache(self.TOKEN)
+        result = self.run_body(
+            self.FETCH_SETUP
+            + f"$script:HUB_FETCH_TOKEN = '{self.TOKEN}'\n"
+            + "function Remove-Item { throw 'held' }\n"
+            "$ok = Invoke-FetchHubLocked\n"
+            '[Console]::Out.WriteLine("ok=$ok token=$script:HUB_FETCH_TOKEN")\n'
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(result.stdout.strip().splitlines()[-1], f"ok=False token={self.TOKEN}")
+        self.assertIn("Could not remove", result.stderr)
+        self.assertEqual((self.dir / "hub.owned").read_text(encoding="utf-8"), self.TOKEN)
+
+    def test_a_marker_that_cannot_be_written_fails_the_fetch_without_cloning(self) -> None:
+        """It fails the one task rather than ending the menu's whole session.
+
+        The refusal is stubbed, since what refuses a write differs by platform and by account.
+        """
+        result = self.run_body(
+            self.FETCH_SETUP
+            + f"$script:HUB_FETCH_TOKEN = '{self.TOKEN}'\n"
+            + "function Set-Content { throw 'refused' }\n"
+            "$ok = Invoke-FetchHubLocked\n"
+            '[Console]::Out.WriteLine("ok=$ok token=$script:HUB_FETCH_TOKEN")\n'
         )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(result.stdout.strip().splitlines()[-1], f"ok=False token={self.TOKEN}")
+        self.assertIn("Could not write", result.stderr)
+        self.assertFalse((self.dir / "hub").exists())
 
     def run_body(self, body: str) -> subprocess.CompletedProcess[str]:
         body_file = self.scripts / "body.ps1"