diff --git a/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-25-guard-hydration-serialization.md b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-25-guard-hydration-serialization.md new file mode 100644 index 000000000..2ede672c9 --- /dev/null +++ b/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. diff --git a/labs/12-product-engineering-loop/product-engineering-loop/hooks.go b/labs/12-product-engineering-loop/product-engineering-loop/hooks.go index 4c6981a78..68d107834 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/hooks.go +++ b/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 } } diff --git a/labs/12-product-engineering-loop/product-engineering-loop/hooks_hydrate_test.go b/labs/12-product-engineering-loop/product-engineering-loop/hooks_hydrate_test.go index 2433e2a6a..36f16cafb 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/hooks_hydrate_test.go +++ b/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: /boatstack/hydrate-.lock. +func hydrateLockPath(t *testing.T, binaryPath string) string { + t.Helper() + // binaryPath = /boatstack/runtimes////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) + } +}