fix(boatstack): guard waits for hydration lock release, not helper presence

labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-25-guard-hydration-serialization.md

@@ -0,0 +1,20 @@
+### Concurrent first use no longer trips a false "unsafe runtime" denial
+
+When several tool calls hit an empty shared-runtime slot at the same time, one guard
+hydrates the slot and the others must wait. The installer copies the helper before the
+manifest, so for a short moment the slot holds the helper but not the manifest. A guard that
+looked during that moment judged the runtime "unsafe or incomplete" and denied the tool call
+by mistake. This showed up as a flaky CI failure on Linux under contention.
+
+The guard now closes both sides of the race. First, it treats the slot as ready only when
+both the helper and the manifest are present. So a guard that arrives during the gap joins
+the hydrate lock instead of denying a half-written slot. Second, a waiting guard now waits
+for the hydrating peer to release the clone-wide lock. The peer releases the lock only after
+its installer finishes, so a released lock means the slot is complete. The guard then runs
+the same checksum and safety gates, which still fail closed if hydration was disabled, timed
+out, or failed. The fix applies to both the bash and PowerShell guards.
+
+Three bounded conformance tests cover the fix. One runs many guards at once and confirms
+that exactly one hydrates and every guard proceeds. Two deterministic tests drive a slow,
+non-atomic peer — one from an empty slot and one from a half-written slot — and confirm the
+guard waits for the peer instead of judging an incomplete slot.

labs/12-product-engineering-loop/product-engineering-loop/hooks.go

@@ -155,15 +155,18 @@ esac
 
 HELPER="$COMMON/boatstack/runtimes/%s/%s/${OS_NAME}-${ARCH}/boatstack-helper${EXTENSION}"
 MANIFEST="$COMMON/boatstack/runtimes/%s/%s/${OS_NAME}-${ARCH}/runtime.lock.json"
-# Auto-hydrate a missing shared-runtime slot. A teammate who pulls a version
-# bump or clones fresh inherits the committed pointers (this guard's baked
-# version path) but an empty, gitignored slot, so without this the very next
-# tool call would hard-deny before any Go runs. On an absent slot we run the
+# Auto-hydrate a missing or incomplete shared-runtime slot. A teammate who pulls
+# a version bump or clones fresh inherits the committed pointers (this guard's
+# baked version path) but an empty, gitignored slot, so without this the very next
+# tool call would hard-deny before any Go runs. On an incomplete slot we run the
 # tag-pinned, checksum-verifying installer in branch-free hydrate mode, serialize
 # clone-wide with an atomic mkdir lock, and bound the attempt. This is purely
 # additive: the existing missing/symlink/checksum gates below stay authoritative
 # and fail-closed, so a disabled, timed-out, or failed hydration simply denies.
-if [[ ! -x "$HELPER" && "${BOATSTACK_AUTO_HYDRATE:-1}" != "0" ]]; then
+# The entry test mirrors those gates (helper AND manifest present, non-symlink):
+# an installer copies the helper before the manifest, so a peer arriving in that
+# window must join the lock and wait, not skip the block and deny a half-slot.
+if { [[ ! -x "$HELPER" || -L "$HELPER" || ! -f "$MANIFEST" || -L "$MANIFEST" ]]; } && [[ "${BOATSTACK_AUTO_HYDRATE:-1}" != "0" ]]; then
   mkdir -p "$COMMON/boatstack" 2>/dev/null || true
   HYDRATE_LOCK="$COMMON/boatstack/hydrate-%s.lock"
   if mkdir "$HYDRATE_LOCK" 2>/dev/null; then
@@ -184,9 +187,15 @@ if [[ ! -x "$HELPER" && "${BOATSTACK_AUTO_HYDRATE:-1}" != "0" ]]; then
     ) >&2 || true
     rmdir "$HYDRATE_LOCK" 2>/dev/null || true
   else
-    # A peer is hydrating the shared slot; wait briefly for it to appear.
-    for _ in $(seq 1 8); do
-      [[ -x "$HELPER" ]] && break
+    # A peer holds the hydrate lock. Wait for the peer to finish — it removes the
+    # lock only after its hydrate command returns — before inspecting the slot, so
+    # a waiter never observes a half-written runtime (for example the helper copied
+    # but the manifest not yet in place). A released lock means the slot is as
+    # complete as it will get; the authoritative gates below then accept it or fail
+    # closed. Bound the wait above the peer's own hydrate timeout so a slow but
+    # succeeding peer still wins.
+    for _ in $(seq 1 12); do
+      [[ -d "$HYDRATE_LOCK" ]] || break
       sleep 1
     done
   fi
@@ -249,7 +258,7 @@ $manifestPath = Join-Path $common "boatstack/runtimes/%s/%s/windows-$arch/runtim
 # checksum-verifying installer in branch-free hydrate mode, serialized clone-wide
 # with an atomic directory lock. Purely additive: the gates below stay
 # authoritative and fail-closed if hydration is disabled, fails, or is skipped.
-if ((-not (Test-Path -LiteralPath $helper -PathType Leaf)) -and $env:BOATSTACK_AUTO_HYDRATE -ne "0") {
+if (((-not (Test-Path -LiteralPath $helper -PathType Leaf)) -or (-not (Test-Path -LiteralPath $manifestPath -PathType Leaf))) -and $env:BOATSTACK_AUTO_HYDRATE -ne "0") {
   $bsCommon = Join-Path $common "boatstack"
   New-Item -ItemType Directory -Path $bsCommon -Force -ErrorAction SilentlyContinue | Out-Null
   $hydrateLock = Join-Path $bsCommon "hydrate-%s.lock"
@@ -270,8 +279,12 @@ if ((-not (Test-Path -LiteralPath $helper -PathType Leaf)) -and $env:BOATSTACK_A
       Remove-Item -LiteralPath $hydrateLock -Recurse -Force -ErrorAction SilentlyContinue
     }
   } else {
-    for ($i = 0; $i -lt 8; $i++) {
-      if (Test-Path -LiteralPath $helper -PathType Leaf) { break }
+    # Wait for the peer to release the lock (it does so only after its hydrate
+    # command returns) before inspecting the slot, so a waiter never observes a
+    # half-written runtime. The authoritative gates below then accept it or fail
+    # closed. Bound the wait above the peer's own hydrate timeout.
+    for ($i = 0; $i -lt 12; $i++) {
+      if (-not (Test-Path -LiteralPath $hydrateLock)) { break }
       Start-Sleep -Seconds 1
     }
   }

labs/12-product-engineering-loop/product-engineering-loop/hooks_hydrate_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"sync"
 	"testing"
+	"time"
 )
 
 func requireBash(t *testing.T) {
@@ -171,24 +172,34 @@ func TestGuardAutoHydrationInvokesPinnedHydrator(t *testing.T) {
 // TestGuardAutoHydrationSerializesConcurrentFirstUse proves the clone-wide lock:
 // two guards racing an absent slot invoke the hydrator at most once, and both
 // still proceed.
+// TestGuardAutoHydrationSerializesConcurrentFirstUse is a bounded conformance
+// test for the clone-wide serialization invariant: when many guards hit an empty
+// slot at once, exactly one hydrates and every guard proceeds. A start barrier
+// releases all guards together to force real contention on the mkdir lock. The
+// invariant holds for any interleaving because each losing guard waits for the
+// winner to release the lock before it inspects the slot, so no guard observes a
+// half-written runtime. Bounded: a fixed fan-out, a single round.
 func TestGuardAutoHydrationSerializesConcurrentFirstUse(t *testing.T) {
 	requireBash(t)
 	repo := runtimeTestRepo(t)
-	binaryPath, manifestPath, restore := stageVerifiedHelper(t, repo)
+	binaryPath, _, restore := stageVerifiedHelper(t, repo)
 	counter := filepath.Join(t.TempDir(), "count")
 	stub := fmt.Sprintf("echo x >> %q && %s", counter, restore)
-	_ = manifestPath
 
+	const guards = 8
+	start := make(chan struct{})
 	var wg sync.WaitGroup
-	outputs := make([]string, 2)
-	errs := make([]error, 2)
-	for i := 0; i < 2; i++ {
+	outputs := make([]string, guards)
+	errs := make([]error, guards)
+	for i := 0; i < guards; i++ {
 		wg.Add(1)
 		go func(idx int) {
 			defer wg.Done()
+			<-start // release every guard together for genuine contention
 			outputs[idx], errs[idx] = runGuard(t, repo, "claude", "BOATSTACK_HYDRATE_COMMAND="+stub)
 		}(i)
 	}
+	close(start)
 	wg.Wait()
 
 	for i := range errs {
@@ -207,3 +218,116 @@ func TestGuardAutoHydrationSerializesConcurrentFirstUse(t *testing.T) {
 		t.Fatalf("hydrator ran %d times under contention, want exactly 1", got)
 	}
 }
+
+// hydrateLockPath returns the clone-wide hydrate lock the guard uses, derived
+// from the shared binary path: <common>/boatstack/hydrate-<Version>.lock.
+func hydrateLockPath(t *testing.T, binaryPath string) string {
+	t.Helper()
+	// binaryPath = <common>/boatstack/runtimes/<Version>/<SourceCommit>/<os-arch>/boatstack-helper
+	bsCommon := filepath.Dir(filepath.Dir(filepath.Dir(filepath.Dir(filepath.Dir(binaryPath)))))
+	return filepath.Join(bsCommon, "hydrate-"+Version+".lock")
+}
+
+// TestGuardAutoHydrationWaiterAwaitsPeerCompletion is a bounded, deterministic
+// regression test for the exact failure mode that flaked in CI: a waiting guard
+// used to break as soon as the helper file appeared and then fail the manifest
+// gate ("unsafe or incomplete") while the peer was still mid-copy. Here a peer
+// holds the lock and hydrates non-atomically — it writes the helper, pauses, then
+// writes the manifest, then releases the lock, exactly the installer's copy order.
+// The waiting guard must not judge the slot until the peer releases the lock, so
+// it proceeds cleanly. Before the fix this test fails; after it, it passes on any
+// timing.
+func TestGuardAutoHydrationWaiterAwaitsPeerCompletion(t *testing.T) {
+	requireBash(t)
+	repo := runtimeTestRepo(t)
+	binaryPath, manifestPath := emptySharedSlot(t, repo)
+	lockDir := hydrateLockPath(t, binaryPath)
+
+	// A peer already holds the clone-wide hydrate lock.
+	if err := os.MkdirAll(lockDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	fakeHelper := []byte("#!/usr/bin/env bash\necho boatstack-guard-hydration-sentinel >&2\nexit 0\n")
+	manifestBytes := []byte(fmt.Sprintf(`{"binary_sha256":"%s"}`, SHA256Bytes(fakeHelper)))
+
+	peerDone := make(chan struct{})
+	go func() {
+		defer close(peerDone)
+		// Let the guard reach its waiter loop while the slot is still empty.
+		time.Sleep(300 * time.Millisecond)
+		if err := os.MkdirAll(filepath.Dir(binaryPath), 0o755); err != nil {
+			return
+		}
+		// The helper appears first — the non-atomic window that broke the old waiter.
+		if err := os.WriteFile(binaryPath, fakeHelper, 0o755); err != nil {
+			return
+		}
+		time.Sleep(1 * time.Second)
+		// The manifest lands only now; the slot becomes complete.
+		if err := os.WriteFile(manifestPath, manifestBytes, 0o644); err != nil {
+			return
+		}
+		// Release the lock last, signaling completion.
+		_ = os.Remove(lockDir)
+	}()
+
+	// The guard finds the lock held and the helper absent, so it enters the waiter
+	// branch. It must wait for the peer to release the lock, then clear every gate.
+	output, err := runGuard(t, repo, "claude")
+	<-peerDone
+	if err != nil {
+		t.Fatalf("waiter judged a slot mid-hydration instead of awaiting the peer: err=%v output=%s", err, output)
+	}
+	if _, statErr := os.Stat(binaryPath); statErr != nil {
+		t.Fatalf("shared slot was not populated after the peer finished: %v", statErr)
+	}
+}
+
+// TestGuardAutoHydrationWaitsWhenSlotHalfWritten is a bounded, deterministic
+// regression test for the skip-path variant of the same failure mode. A guard
+// that judged readiness by the helper alone would, on a half-written slot (helper
+// present, manifest not yet), skip the hydrate/wait block entirely and deny at the
+// manifest gate — even while a peer held the lock and was about to finish. The
+// entry test now mirrors the gates (helper AND manifest), so such a guard joins
+// the lock and waits instead. Before the fix this test fails with the exact CI
+// error; after it, it passes.
+func TestGuardAutoHydrationWaitsWhenSlotHalfWritten(t *testing.T) {
+	requireBash(t)
+	repo := runtimeTestRepo(t)
+	binaryPath, manifestPath := emptySharedSlot(t, repo)
+	lockDir := hydrateLockPath(t, binaryPath)
+
+	fakeHelper := []byte("#!/usr/bin/env bash\necho boatstack-guard-hydration-sentinel >&2\nexit 0\n")
+	manifestBytes := []byte(fmt.Sprintf(`{"binary_sha256":"%s"}`, SHA256Bytes(fakeHelper)))
+
+	// The slot is half-written — the helper is present but the manifest is not —
+	// and a peer holds the hydrate lock because it is still mid-copy.
+	if err := os.MkdirAll(filepath.Dir(binaryPath), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(binaryPath, fakeHelper, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(lockDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	peerDone := make(chan struct{})
+	go func() {
+		defer close(peerDone)
+		time.Sleep(1 * time.Second)
+		if err := os.WriteFile(manifestPath, manifestBytes, 0o644); err != nil {
+			return
+		}
+		_ = os.Remove(lockDir)
+	}()
+
+	// A guard seeing only the helper must not treat the slot as ready; it must join
+	// the lock, wait for the peer to finish, then clear every gate.
+	output, err := runGuard(t, repo, "claude")
+	<-peerDone
+	if err != nil {
+		t.Fatalf("guard skipped hydration on a half-written slot instead of waiting: err=%v output=%s", err, output)
+	}
+}