From 91edd568df3c13f34a049f29c0bed32ef08db71c Mon Sep 17 00:00:00 2001 From: bigboateng Date: Sat, 25 Jul 2026 17:08:22 +0100 Subject: [PATCH 1/2] fix(boatstack): guard waits for hydration lock release, not helper presence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concurrent first use of an empty shared-runtime slot could falsely deny a tool call. One guard wins the mkdir lock and hydrates; the losers waited by watching for the helper file to appear. The installer copies the helper before the manifest, so a waiter could observe that gap, judge the runtime "unsafe or incomplete", and exit 2 — the exact flake seen on the public sync PR's ubuntu job (TestGuardAutoHydrationSerializesConcurrentFirstUse). The waiter now waits for the hydrating peer to release the clone-wide lock. The peer removes the lock only after its installer returns, so a released lock means the slot is complete; the authoritative checksum and safety gates then accept it or fail closed. Fixed in both the bash and PowerShell guards. Bounded conformance tests: strengthen the concurrency test to 8-way contention with a start barrier (exactly-one-hydration invariant), and add a deterministic waiter-vs-slow-non-atomic-peer test that reproduces the exact failure mode against the old logic and passes on the fix, on any timing. Disclosure-Reviewed: reviewed — public-safe only (public guard script + tests + release-note prose), no private facet in this commit --- ...026-07-25-guard-hydration-serialization.md | 16 ++++ .../product-engineering-loop/hooks.go | 20 +++-- .../hooks_hydrate_test.go | 86 +++++++++++++++++-- 3 files changed, 112 insertions(+), 10 deletions(-) create mode 100644 labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-25-guard-hydration-serialization.md 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..c31a8d154 --- /dev/null +++ b/labs/12-product-engineering-loop/boatstack-distribution/release-notes/2026-07-25-guard-hydration-serialization.md @@ -0,0 +1,16 @@ +### 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 wait. The waiting guard used to watch for the helper file +to appear, then check the runtime. The installer copies the helper before the manifest, so a +waiter could look during that gap. It saw the helper without the manifest, judged the +runtime "unsafe or incomplete", and denied the tool call by mistake. + +The 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. + +Two bounded conformance tests cover the fix. One runs many guards at once and confirms that +exactly one hydrates and every guard proceeds. The other drives a slow, non-atomic peer and +confirms the waiter never judges a half-written 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..f96e13675 100644 --- a/labs/12-product-engineering-loop/product-engineering-loop/hooks.go +++ b/labs/12-product-engineering-loop/product-engineering-loop/hooks.go @@ -184,9 +184,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 @@ -270,8 +276,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..fc1b4d1d2 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,68 @@ 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) + } +} From 1d0d72f540c2da3e5e1f4b8b8b053b1e4f67d937 Mon Sep 17 00:00:00 2001 From: bigboateng Date: Sat, 25 Jul 2026 19:05:03 +0100 Subject: [PATCH 2/2] fix(boatstack): treat a half-written runtime slot as not-ready so peers wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard judged slot readiness by the helper alone. An installer copies the helper before the manifest, so a guard arriving in that window skipped the hydrate/wait block and denied at the manifest gate while a peer held the lock and was about to finish. This surfaced as a flaky Linux CI denial under contention. The entry test now mirrors the downstream gates (helper AND manifest present, non-symlink) in both the bash and PowerShell guards, so a guard in the gap joins the hydrate lock and waits instead of denying a half-written slot. Adds a bounded, deterministic regression test driving a slow non-atomic peer from a half-written slot. Disclosure-Reviewed: reviewed — public-safe only, private facet kept out of this commit --- ...026-07-25-guard-hydration-serialization.md | 26 +++++----- .../product-engineering-loop/hooks.go | 15 +++--- .../hooks_hydrate_test.go | 48 +++++++++++++++++++ 3 files changed, 72 insertions(+), 17 deletions(-) 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 index c31a8d154..2ede672c9 100644 --- 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 @@ -1,16 +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 wait. The waiting guard used to watch for the helper file -to appear, then check the runtime. The installer copies the helper before the manifest, so a -waiter could look during that gap. It saw the helper without the manifest, judged the -runtime "unsafe or incomplete", and denied the tool call by mistake. +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 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 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. -Two bounded conformance tests cover the fix. One runs many guards at once and confirms that -exactly one hydrates and every guard proceeds. The other drives a slow, non-atomic peer and -confirms the waiter never judges a half-written slot. +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 f96e13675..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 @@ -255,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" 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 fc1b4d1d2..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 @@ -283,3 +283,51 @@ func TestGuardAutoHydrationWaiterAwaitsPeerCompletion(t *testing.T) { 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) + } +}