fix: unblock sandboxed worktree setup, stop E2E dirtying the tree, guard pushes server-side

.claude/settings.json

@@ -15,7 +15,10 @@
     "filesystem": {
       "allowWrite": [
         ".",
-        "~/GitHub/bess-manager"
+        "~/GitHub/bess-manager",
+        "~/Library/Caches/ms-playwright",
+        "~/.cache/ms-playwright",
+        "~/.npm"
       ]
     }
   },
@@ -86,9 +89,6 @@
       "Bash(git -* stash store*)"
     ],
     "ask": [
-      "Bash(git push)",
-      "Bash(git push *)",
-      "Bash(git -* push*)",
       "Bash(gh api)",
       "Bash(gh api *)",
       "Bash(gh pr merge*)",

.claude/skills/verify/SKILL.md

@@ -71,10 +71,16 @@ port to find real entity IDs/attributes to restore.
 ## Gotchas
 
 - `${BESS_SETTINGS:-./e2e/ci-bess-settings.json}` is mounted **read-write**
-  (no `:ro`). Running the app against it can silently write settings back
-  into the fixture (schema migrations, demo_mode defaults, etc). After
-  tearing down: `git diff -- e2e/` and `git checkout -- e2e/` if the only
-  changes are ones you didn't intend.
+  (no `:ro`), and it has to be — that mount *is* how the app persists
+  settings, so `:ro` would break the wizard rather than protect the file.
+  Running the app against it therefore writes settings back into the fixture
+  (schema migrations, demo_mode defaults, etc). After tearing down:
+  `git diff -- e2e/` and `git checkout -- e2e/` if the only changes are ones
+  you didn't intend.
+  `e2e/ci-wizard-settings.json` is the exception and needs no such reset: it
+  is gitignored, because every consumer truncates it to `{}` before mounting
+  it and nothing reads its content. The `ci-bess-settings*.json` files are
+  real fixtures whose content matters, so those still dirty the tree.
 - Many scenarios (e.g. `ci-wizard-entsoe.json`) pin `mock_time` to a fixed
   past date (`ci-normal-day.json` → `2025-01-15`), not "today" — the real
   container clock is today's date, so date-anchored service calls (Nordpool

.gitignore

@@ -71,6 +71,14 @@ e2e/test-results/
 # Mock HA settings generated for CI (derived from scenario)
 backend/mock-bess-settings.json
 
+# Wizard E2E scratch settings. NOT a fixture, despite living beside the real
+# ones: every consumer (e2e/run-e2e.sh, each wizard step in ci.yml) truncates
+# it to `{}` immediately before mounting it, precisely so the wizard triggers
+# fresh -- so nothing ever reads what is committed here. The container then
+# writes the wizard's result back through the read-write mount, which used to
+# leave the working tree dirty after every local E2E run.
+e2e/ci-wizard-settings.json
+
 # Scratch fixtures generated by scripts/demo_health_recovery.sh
 .demo-scratch/
 

scripts/quality-check.sh

@@ -171,13 +171,41 @@ if ! python3 - <<'PY'
 import json, re, sys
 
 # Patterns match the command AS WRITTEN -- prefix globbing, no normalisation.
-# `git push` and `gh api` are guarded by a BLANKET rule on purpose: the
-# dangerous shapes put their marker at an arbitrary argument position
-# (`git push origin main --force`, `git push origin +beta-release-9.9`,
-# `git push origin --delete release-X.Y`, `gh api <path> -X PUT`), which a
-# prefix glob cannot reach. Enumerating them left real holes twice. Narrowing
-# these two back to specific forms re-opens the holes, so the check requires
-# the blanket spelling rather than merely "some rule exists".
+# `gh api` is guarded by a BLANKET rule on purpose: the dangerous shapes put
+# their marker at an arbitrary argument position (`gh api <path> -X PUT`,
+# `gh api <path> -f k=v`), which a prefix glob cannot reach. Enumerating them
+# left real holes twice. Narrowing it back to specific forms re-opens the
+# holes, so the check requires the blanket spelling rather than merely "some
+# rule exists".
+#
+# `git push` USED to be in that same sentence and no longer is. It was never
+# the glob that made it safe -- the glob was a blunt instrument compensating
+# for having no guard at the only layer that can actually see a ref update.
+# The four leaks it was blanket-guarding against are now refused SERVER-SIDE
+# by GitHub rulesets, which evaluate the resulting ref rather than the command
+# string, so argument order cannot evade them:
+#
+#   git push origin main --force        -> non_fast_forward on ~DEFAULT_BRANCH
+#   git push origin +beta-release-9.9   -> non_fast_forward on beta-release-*
+#   git push origin --delete <ref>      -> deletion on both of the above
+#   git push origin v9.9.0 (force/move) -> non_fast_forward on ~ALL tags
+#
+# All rulesets are enforcement=active with an EMPTY bypass_actors list, so
+# they bind the repo owner too -- which matters, because local pushes
+# authenticate as the owner (osxkeychain), never as bess-agent.
+#
+# Do NOT re-add a `Bash(git push*)` ask on the grounds that "a guard is
+# missing". Check the rulesets first:
+#
+#   gh api repos/johanzander/bess-manager/rulesets
+#   gh api repos/johanzander/bess-manager-beta/rulesets
+#
+# What this deliberately does NOT cover: force-pushing or deleting a FEATURE
+# branch (fix/**, feat/**) on origin, nor `release-X.Y` (see MUST_NOT_BE_GUARDED
+# below). Accepted residuals -- but NOT because "the damage is bounded to your
+# own unmerged branch". ~20 worktrees push in parallel as the same identity, so
+# a misaimed --force destroys another agent's commits and closes its PR, with
+# the recovering reflog sitting in a different worktree.
 #
 # Every entry below is a rule whose deletion is the exact regression this gate
 # was written for -- the GitHub-reaching and history-destroying guards. Keep
@@ -191,7 +219,6 @@ REQUIRED = {
         "Bash(podman machine rm)", "Bash(podman system reset)",
     ],
     "ask": [
-        "Bash(git push)", "Bash(git push *)", "Bash(git -* push*)",
         "Bash(gh api)", "Bash(gh api *)",
         "Bash(gh pr merge*)", "Bash(gh repo edit*)",
         "Bash(gh release create*)", "Bash(gh release edit*)",
@@ -241,18 +268,9 @@ MUST_BE_GUARDED = [
     # git's global options may precede the subcommand -- the hook this
     # replaced normalised for exactly this, and CLAUDE.md teaches `git -C` as
     # the cross-checkout idiom, so it is the spelling most likely to be used.
-    "git -C .claude/worktrees/x push origin main",
-    "git -c push.default=current push beta main",
     "git --no-pager gc --prune=now",
     "git -C sub tag -d v9.9.0",
     "git -C sub update-ref -d refs/heads/x",
-    # the marker sits at an arbitrary argument position
-    "git push origin main --force",
-    "git push origin +beta-release-9.9",
-    "git push origin --delete release-9.9",
-    "git push origin v9.9.0",
-    "git push -u origin main",
-    "git push",
     # history destruction, incl. the spellings that are NOT `gc`/`reflog expire`
     "git tag --delete v9.9.0", "git tag -d v9.9.0",
     "git tag -f v9.9.0 abc123",
@@ -308,6 +326,35 @@ MUST_NOT_BE_GUARDED = [
     "git -C .claude/worktrees/x diff -- file.py",
     "git -C .claude/worktrees/x apply",
     "git status", "git diff", "git log --oneline",
+    # Pushing. Enforcement moved to GitHub rulesets (see the REQUIRED comment
+    # above), so these must run UNATTENDED -- implement-issue Step 9 and every
+    # PR-refresh push. Pinned in this direction on purpose: the failure mode
+    # being guarded against is someone re-adding a `Bash(git push*)` ask
+    # because it "looks unguarded", which would silently restore the stall the
+    # rulesets were created to remove. If you believe a guard is missing, read
+    # the rulesets before touching this list.
+    #
+    # These entries assert only that the commands are LOCALLY unguarded. Which
+    # of them GitHub also refuses is a separate question, and the answer is not
+    # "all of them" -- do not read this list as a protection matrix:
+    #
+    #   main --force, +beta-release-*, v9.9.0 (tag)  -> refused by a ruleset
+    #   --delete release-X.Y                         -> NOT refused; no ruleset
+    #                                                   covers `release-*`
+    #
+    # That last line is a real residual, not an oversight to "fix" by putting
+    # the prompt back: `release-X.Y` is the short-lived hotfix branch created
+    # by the release skill (steps 2-6), it is pushed and tagged, and nothing
+    # currently protects it at either layer. The protected TAG preserves the
+    # released commit, so the branch is recoverable after tagging but not
+    # before. Closing it means adding a `release-*` ruleset, not an ask rule.
+    "git push", "git push -u origin main",
+    "git push origin main --force",
+    "git push origin +beta-release-9.9",
+    "git push origin --delete release-9.9",
+    "git push origin v9.9.0",
+    "git -C .claude/worktrees/x push origin main",
+    "git -c push.default=current push beta main",
     # Read-only stash inspection, which CLAUDE.md and rules.md both promise
     # keeps working. A blanket `git -* stash *` DENY caught these, and deny has
     # no override -- so the cross-checkout recipe was hard-blocked, not merely

scripts/verify-sandbox.sh

@@ -178,6 +178,80 @@ else
     "add the main checkout to sandbox.filesystem.allowWrite -- nested worktrees and every symlink target are then covered by one entry. Without it vitest and vite build fail EPERM in every worktree. NOTE: a SIBLING checkout (../bess-manager-feature) is NOT under that path and needs its own entry."
 fi
 
+# 4d. THE TWO USER-LEVEL CACHES scripts/worktree-setup.sh WRITES. Both sit well
+#     outside the repo, so neither the "." nor the "~/GitHub/bess-manager"
+#     entry reaches them, and the built-in allowOnly list covers only
+#     ~/.npm/_logs -- not the package cache next to it. Setup does BOTH: it
+#     runs `npm install` when a lockfile has diverged from the main checkout,
+#     and then `npx playwright install chromium`.
+#
+#     What makes this worth a check rather than a comment is that NEITHER
+#     failure names the sandbox. npm reports EPERM on ~/.npm/_cacache as
+#     "Your cache folder contains root-owned files ... please run: sudo chown
+#     -R", sending you to `sudo` -- which is on the ask list and is entirely
+#     the wrong fix. And worktree-setup.sh's Playwright step is bounded by a
+#     timeout whose message blames a slow download, so a blocked cache reads
+#     as the unrelated extraction hang described in worktree-setup.sh (which
+#     is a real, separate failure -- do not conflate the two). Both the npm and
+#     the Playwright blocks were observed here in one session, and the
+#     Playwright one cost a whole worktree's setup.
+#
+# 4d-i. The Playwright browser cache. Browsers are deliberately NOT
+#       per-worktree -- one shared user-level cache serves every checkout, so
+#       redirecting PLAYWRIGHT_BROWSERS_PATH into the repo is not the fix.
+#       mkdir rather than touch, and create the cache dir first: it may not
+#       exist yet on a fresh machine, and creating it is the first thing a
+#       real install does.
+#       `PLAYWRIGHT_BROWSERS_PATH=0` is Playwright's documented sentinel for
+#       "no shared cache, keep browsers inside node_modules" -- NOT a path.
+#       Treating it as one makes the probe `mkdir -p 0` in the repo, which
+#       succeeds and reports a PASS that measured nothing.
+pw_sentinel=no
+if [ "${PLAYWRIGHT_BROWSERS_PATH:-}" = "0" ]; then
+  pw_sentinel=yes
+elif [ -n "${PLAYWRIGHT_BROWSERS_PATH:-}" ]; then
+  pw_cache="$PLAYWRIGHT_BROWSERS_PATH"
+elif [ "$(uname -s)" = "Darwin" ]; then
+  pw_cache="$HOME/Library/Caches/ms-playwright"
+else
+  pw_cache="$HOME/.cache/ms-playwright"
+fi
+
+if [ "$pw_sentinel" = yes ]; then
+  printf 'SKIP  PLAYWRIGHT_BROWSERS_PATH=0 -- browsers live in node_modules, no shared cache to probe\n'
+else
+  # A cache directory that cannot even be CREATED is the blocked case, not an
+  # inapplicable one. Reporting SKIP here (and counting no failure) is how a
+  # fresh sandboxed machine exits 0 saying the config is safe while both npm
+  # and Playwright are about to fail -- the exact condition this check exists
+  # to catch. Fold it into the same `check`, so it FAILS.
+  mkdir -p "$pw_cache" 2>/dev/null
+  if [ -d "$pw_cache" ]; then
+    mkdir "$pw_cache/.sandbox-probe" 2>/dev/null && pw=allowed || pw=blocked
+    rmdir "$pw_cache/.sandbox-probe" 2>/dev/null
+  else
+    pw=blocked
+  fi
+  check "writing the Playwright browser cache is allowed" allowed "$pw" \
+    "add the browser cache to sandbox.filesystem.allowWrite (~/Library/Caches/ms-playwright on macOS, ~/.cache/ms-playwright on Linux). Without it scripts/worktree-setup.sh cannot install browsers, and its timeout reports that as a stalled download -- indistinguishable from the separate extraction hang that timeout was written for. Do NOT point PLAYWRIGHT_BROWSERS_PATH into the repo to dodge this: one shared cache across every worktree is the design."
+fi
+
+# 4d-ii. The npm package cache. ~/.npm/_logs is writable by default and
+#        ~/.npm/_cacache is not, so npm looks fine right up until it has to
+#        fetch something -- `npm install` in worktree-setup.sh (diverged
+#        lockfile), `npm ci` in e2e/run-e2e.sh (missing frontend/dist), or any
+#        npx that misses the local node_modules.
+#        Same rule as above: "could not be created" IS blocked. Never SKIP it.
+mkdir -p "$HOME/.npm/_cacache" 2>/dev/null
+if [ -d "$HOME/.npm/_cacache" ]; then
+  mkdir "$HOME/.npm/_cacache/.sandbox-probe" 2>/dev/null && npmc=allowed || npmc=blocked
+  rmdir "$HOME/.npm/_cacache/.sandbox-probe" 2>/dev/null
+else
+  npmc=blocked
+fi
+check "writing the npm package cache is allowed" allowed "$npmc" \
+    "add ~/.npm to sandbox.filesystem.allowWrite. The built-in allow list covers ~/.npm/_logs only, so npm can write its log and not its cache. IGNORE the error npm prints -- it claims the cache 'contains root-owned files' and tells you to run 'sudo chown -R 502:20 ~/.npm'. That is npm guessing at EPERM; the files are not root-owned and sudo is not the fix."
+
 # 5. gh. `gh pr create` is the closing step of implement-issue, and gh is a Go
 #    binary: under the macOS sandbox it cannot reach trustd to verify TLS,
 #    which surfaces as x509: OSStatus -26276.