From 75dc1d4c666c549da7d5c03e5af0e6cea112434d Mon Sep 17 00:00:00 2001 From: Mujtaba Badat <1470881+mujtaba3B@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:03:39 -0500 Subject: [PATCH] fix(eng): slide the ship/land sentinel arm window so long runs stay cleared The ship-PR gate became unpassable from within Claude for any /ship whose invocation-to-`gh pr create` span exceeded the sentinel TTL (1200s). The sentinel minter's arm marker was written ONCE at /ship invocation and never refreshed, so ARM_TTL later the per-Bash re-mint went inert, the clearance sentinel froze at its last mint, and by create time it was long expired (observed 2026-07-21 in mutwo-skills: minted 11:44, blocked 15:10 with [sentinel: expired]). Fix: every armed Bash command that mints into the ~/dev target repo now SLIDES the session arm window forward. An actively-working /ship keeps itself armed, so the per-Bash re-mint keeps the sentinel fresh for the whole run. The 1200s TTL becomes a max-IDLE-gap rather than a whole-run budget, keeping the tight accident-guard while spanning arbitrarily long active runs. The accident-guard is intact: the slide only EXTENDS an already-armed session (guarded by session_armed_fresh) and only on a real target-repo mint, so it never arms an unarmed session nor resurrects an expired window. A bare `cd ~/dev/repo && gh pr create` with no prior /ship still blocks. Applied symmetrically to land-deploy-sentinel.sh (identical latent flaw on the merge path). Adds bats coverage for the slide, the incident repro, and the no-self-arm backstop in both suites. Bumps eng 2.8.0 -> 2.8.1 so the install cache refreshes. Co-Authored-By: Claude Opus 4.8 (1M context) --- eng/.claude-plugin/plugin.json | 2 +- eng/hooks/scripts/land-deploy-sentinel.sh | 36 ++++++++--- eng/hooks/scripts/ship-gate-sentinel.sh | 49 ++++++++++++--- eng/hooks/tests/land-deploy-sentinel.bats | 51 +++++++++++++++ eng/hooks/tests/ship-pr-gate.bats | 76 +++++++++++++++++++++++ 5 files changed, 195 insertions(+), 19 deletions(-) diff --git a/eng/.claude-plugin/plugin.json b/eng/.claude-plugin/plugin.json index e7b9f8e..aab43af 100644 --- a/eng/.claude-plugin/plugin.json +++ b/eng/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "eng", - "version": "2.8.0", + "version": "2.8.1", "description": "Engineer Ernie, the engineering persona. eng:cr is his master code-review skill and the single local review path the ~/dev merge gate keys on: it risk-tiers depth, runs the pr-review-toolkit lenses, and mints the merge-clearance stamp. It routes to cr-teammate (review someone else's PR and post a comment) and to address-pr-feedback / pr-watcher (respond to review feedback). He also spikes the riskiest unknown before building, generates CodeRabbit config, and builds macOS Shortcuts. The plugin also SHIPS Ernie's PR-lifecycle enforcement hooks (hooks/hooks.json): the ship-PR gate (PRs only via /ship), the merge-clearance gate (no merge without the cleared gauntlet AND a /land-and-deploy sentinel, making /land-and-deploy the single CLI merge path), the /ship and /land-and-deploy sentinels, the review stamp recorder, and an after-ship CodeRabbit-watcher nudge (after a genuine /ship opens a PR, points the agent at /eng:pr-watcher, or when CodeRabbit is rate-limited routes to /land-and-deploy if a current /eng:cr review backstops the head, else to /eng:cr and then /land-and-deploy), active in opted-in repos (.ship-gate.json / .merge-clearance.json) under ~/dev. Skills: eng:cr, eng:cr-teammate, eng:address-pr-feedback, eng:pr-watcher, eng:spike, eng:coderabbit-config, eng:shortcut.", "author": { "name": "Mujtaba Badat", diff --git a/eng/hooks/scripts/land-deploy-sentinel.sh b/eng/hooks/scripts/land-deploy-sentinel.sh index f16ad6f..64f76ff 100755 --- a/eng/hooks/scripts/land-deploy-sentinel.sh +++ b/eng/hooks/scripts/land-deploy-sentinel.sh @@ -69,11 +69,19 @@ CWD=$(printf '%s' "$PAYLOAD" | jq -r '.cwd // empty') [ -n "$CWD" ] && [ -d "$CWD" ] || CWD="$PWD" SESSION=$(printf '%s' "$PAYLOAD" | jq -r '.session_id // empty') -# ARM_TTL bounds how long after a /land-and-deploy invocation an armed Bash command -# may still mint. It must span a whole run: invocation -> wait on CI + the merge -# queue -> the actual `gh pr merge`, which can be many minutes. 1800s (30m) matches -# the sentinel's own default TTL; the sentinel is head_sha-bound, so a long window -# is safe (a stale commit cannot be merged on an old sentinel). +# ARM_TTL bounds how long the armed window may go IDLE (no ~/dev-repo Bash activity) +# before the Bash-mint path goes inert. It is NOT a hard ceiling on the whole run: +# the Bash branch SLIDES the window forward on every armed Bash command that mints +# into the target repo (see the Bash case below), so an actively-working +# /land-and-deploy keeps itself armed while it runs repo commands (invocation -> +# wait on CI + the merge queue -> the actual `gh pr merge`). This mirrors the +# ship-gate fix: previously the arm marker was written once at invocation and never +# refreshed, so ARM_TTL later the Bash-mint died and the sentinel froze - a +# /land-and-deploy longer than the window found it expired at merge time. With +# sliding, only an idle gap LONGER than ARM_TTL lapses the window. 1800s (30m) is +# the per-slide idle budget, matched to the sentinel's own default TTL; the +# sentinel is also head_sha-bound, so a live window is doubly safe (a stale commit +# cannot be merged on an old sentinel). ARM_TTL=1800 ARM_DIR="${TMPDIR:-/tmp}" arm_session() { ga_arm "land" "$SESSION" "$ARM_DIR" "$(date +%s)"; } @@ -82,10 +90,13 @@ land_armed_fresh() { ga_armed_fresh "land" "$SESSION" "$ARM_DIR" "$(date +%s)" " # mint : write the target-bound sentinel for the # repo that resolves to, iff it is a ~/dev repo. head_sha + repo are read # from THAT checkout (not the session cwd), so the bindings match what the gate -# validates against the target PR. +# validates against the target PR. Returns 0 when is an in-scope ~/dev +# repo (the mint path ran; the sentinel write itself is best-effort), non-zero when +# out of scope - the Bash branch reads this to slide the arm window only for an +# in-scope target repo. Skill/prompt callers ignore the return value. mint() { local workdir="$1" pr="$2" source="$3" resolved top gitdir head_sha remote repo ttl marker now sentinel tmp - resolved=$(sg_dev_repo_gitdir "$workdir") || return 0 + resolved=$(sg_dev_repo_gitdir "$workdir") || return 1 top=${resolved%%$'\t'*} gitdir=${resolved#*$'\t'} head_sha=$(git -C "$workdir" rev-parse HEAD 2>/dev/null || echo "") @@ -142,7 +153,16 @@ case "$EVENT" in CMD=$(printf '%s' "$PAYLOAD" | jq -r '.tool_input.command // empty') [ -n "$CMD" ] || exit 0 WORKDIR=$(sg_workdir_from_cmd "$CMD" "$CWD") - mint "$WORKDIR" "$(pr_from_cmd "$CMD")" "land-bash" + if mint "$WORKDIR" "$(pr_from_cmd "$CMD")" "land-bash"; then + # SLIDE the arm window forward (mirrors ship-gate-sentinel.sh): an + # actively-working /land-and-deploy keeps itself armed, so the Bash-mint + # keeps the head_sha-bound sentinel fresh (and current with HEAD) for the + # whole run instead of freezing ARM_TTL after invocation. Only EXTENDS an + # already-armed session (we passed land_armed_fresh above) and only when the + # command resolved to an in-scope ~/dev repo (mint returned 0), so it never + # arms an unarmed session nor resurrects an expired window. + arm_session + fi ;; *) exit 0 ;; esac diff --git a/eng/hooks/scripts/ship-gate-sentinel.sh b/eng/hooks/scripts/ship-gate-sentinel.sh index 5fdc689..0f211c9 100755 --- a/eng/hooks/scripts/ship-gate-sentinel.sh +++ b/eng/hooks/scripts/ship-gate-sentinel.sh @@ -67,13 +67,25 @@ SESSION=$(printf '%s' "$PAYLOAD" | jq -r '.session_id // empty') # Session arming (shared ship-gate-arm-lib.sh, kind "ship"). The marker lets the # later PreToolUse:Bash event know a real /ship is in flight. # -# ARM_TTL bounds how long after a /ship invocation an armed Bash command may still -# mint, i.e. it must span a whole /ship run (invocation -> review/build -> the -# final `gh pr create`). Kept at 1200s to MATCH the legacy window: the old design -# minted a 1200s freshness sentinel at /ship invocation, so the create already had -# to land within 1200s of invocation. Do not widen this without reason; a longer -# window only enlarges the post-/ship blast radius (any ~/dev repo cd'd into during -# the window self-clears), which is the accident-guard's main cost. +# ARM_TTL bounds how long the armed window may go IDLE (no ~/dev-repo Bash activity) +# before the Bash-mint path goes inert. It is NOT a hard ceiling on the whole /ship +# run: the Bash branch SLIDES the window forward on every armed Bash command that +# mints into the target repo (see the Bash case below), so an actively-working +# /ship keeps itself armed for as long as it keeps running repo commands. This is +# the fix for the "unpassable long ship" bug: previously the arm marker was written +# ONCE at /ship invocation and never refreshed, so ARM_TTL after invocation the +# Bash-mint died, the sentinel froze at its last mint, and any /ship whose +# invocation->`gh pr create` span exceeded 1200s (routine: review + CodeRabbit + a +# QA-plan detour) found the sentinel expired at create time. With sliding, only an +# idle gap LONGER than ARM_TTL (a run left untouched for 20m) lapses the window; a +# genuine active run never does. +# +# 1200s is the per-slide IDLE budget, matched to the sentinel's own TTL. Do not +# widen it without reason: the sliding property (not a larger fixed window) is what +# spans a long run, and a bigger fixed TTL only makes clearance linger longer AFTER +# a ship ends. The post-/ship blast radius (any ~/dev repo cd'd into while armed +# self-clears) is unchanged in KIND and now activity-bounded rather than fixed at +# 1200s-from-invocation. ARM_TTL=1200 ARM_DIR="${TMPDIR:-/tmp}" arm_session() { ga_arm "ship" "$SESSION" "$ARM_DIR" "$(date +%s)"; } @@ -97,10 +109,14 @@ mint() { printf '%s\n' "$sentinel" > "$tmp" 2>/dev/null && mv -f "$tmp" "$gitdir/ship-pr-clearance" 2>/dev/null || rm -f "$tmp" 2>/dev/null } -# mint_for_dir : mint iff is a ~/dev repo. +# mint_for_dir : mint iff is a ~/dev repo. Returns 0 +# when is an in-scope ~/dev repo (the mint path ran; the sentinel write +# itself is best-effort), non-zero when out of scope - the Bash branch reads this to +# slide the arm window only for an in-scope target repo. Skill/prompt callers ignore +# the return value. mint_for_dir() { local resolved top gitdir - resolved=$(sg_dev_repo_gitdir "$1") || return 0 + resolved=$(sg_dev_repo_gitdir "$1") || return 1 top=${resolved%%$'\t'*} gitdir=${resolved#*$'\t'} mint "$gitdir" "$top" "$2" @@ -157,7 +173,20 @@ case "$EVENT" in CMD=$(printf '%s' "$PAYLOAD" | jq -r '.tool_input.command // empty') [ -n "$CMD" ] || exit 0 WORKDIR=$(sg_workdir_from_cmd "$CMD" "$CWD") - mint_for_dir "$WORKDIR" "ship-bash" + if mint_for_dir "$WORKDIR" "ship-bash"; then + # SLIDE the arm window forward: an actively-working /ship (one still + # running repo commands) keeps itself armed, so the Bash-mint keeps the + # sentinel fresh for the WHOLE run instead of freezing ARM_TTL after + # invocation. This is the core of the long-ship fix. It can only EXTEND an + # already-armed session: we reach here only past `session_armed_fresh` + # above, and only re-arm when the command resolved to an in-scope ~/dev + # repo (mint_for_dir returned 0). It therefore never ARMS an unarmed session - + # a bare `cd ~/dev/repo && gh pr create` with no prior /ship is still + # never armed and still blocks - and never resurrects an expired window + # (an idle gap > ARM_TTL fails session_armed_fresh and we exit before + # here). The window thus tracks genuine, still-active /ship repo activity. + arm_session + fi ;; *) exit 0 ;; esac diff --git a/eng/hooks/tests/land-deploy-sentinel.bats b/eng/hooks/tests/land-deploy-sentinel.bats index 3eb5159..7a784a2 100644 --- a/eng/hooks/tests/land-deploy-sentinel.bats +++ b/eng/hooks/tests/land-deploy-sentinel.bats @@ -158,3 +158,54 @@ sentinel_bash_payload() { printf '{"hook_event_name":"PreToolUse","tool_name":"B [ ! -f "$SENTINEL" ] rm -f "$ARM" } + +# ---- sliding arm window (mirrors the ship-gate long-run fix) ---------------- + +@test "armed mint: an armed Bash command SLIDES the land arm window forward (long /land-and-deploy stays armed past ARM_TTL)" { + # Same long-run fix as ship-gate-sentinel.sh: the arm marker used to be written + # once at invocation and never refreshed, so ARM_TTL(1800s) later the Bash-mint + # went inert and the head_sha-bound sentinel froze - a /land-and-deploy longer than + # the window found it stale at merge time. Now every armed Bash command that mints + # into the target repo re-arms the session. Assert the slide directly. + SID="ldtest-$BATS_TEST_NUMBER" + ARM="${TMPDIR:-/tmp}/gstack-land-armed-$SID" + NOWS=$(date +%s) + printf '%s\n' "$((NOWS-1700))" > "$ARM" # armed 1700s ago: still fresh (<1800), near lapsing + bash -c "printf '%s' '$(sentinel_bash_payload "cd $REPO && git status" "$HOME" "$SID")' | bash '$WRITER'" + [ -f "$SENTINEL" ] # minted into the target repo + [ "$(head -1 "$ARM")" -ge "$((NOWS-10))" ] # land arm window slid forward to ~now (was T-1700) + rm -f "$ARM" +} + +@test "armed mint: an EXPIRED land arm marker neither mints nor self-arms (slide only extends a live window)" { + # The slide must not resurrect a dead merge window: under an expired arm marker a + # Bash command must not mint AND must not re-arm, so an abandoned /land-and-deploy + # cannot keep authorizing merges. + SID="ldtest-$BATS_TEST_NUMBER" + ARM="${TMPDIR:-/tmp}/gstack-land-armed-$SID" + NOWS=$(date +%s) + printf '%s\n' "$((NOWS-4000))" > "$ARM" # expired (age 4000 > ARM_TTL 1800) + bash -c "printf '%s' '$(sentinel_bash_payload "cd $REPO && git status" "$HOME" "$SID")' | bash '$WRITER'" + [ ! -f "$SENTINEL" ] # did not mint + [ "$(head -1 "$ARM")" = "$((NOWS-4000))" ] # arm marker unchanged: no self-arm + rm -f "$ARM" +} + +@test "armed mint: an armed Bash command OUT of ~/dev scope does NOT slide the land arm window (the return-1 guard)" { + # Pins the mint `return 0`->`return 1` flip: an armed, still-fresh session running a + # Bash command that resolves OUTSIDE ~/dev must NOT re-arm, so the window stays + # bound to genuine target-repo activity. Arm at a distinctly OLDER time so a + # wrongful slide (to ~now) is detectable; a regression to `return 0` would silently + # re-arm off any out-of-scope Bash activity and this test would catch it. + SID="ldtest-$BATS_TEST_NUMBER" + ARM="${TMPDIR:-/tmp}/gstack-land-armed-$SID" + NOWS=$(date +%s) + OUT=$(mktemp -d "/tmp/.ldtestout.XXXXXX") + git -C "$OUT" init -q; git -C "$OUT" commit -q --allow-empty -m init + OUTGIT=$(git -C "$OUT" rev-parse --absolute-git-dir) + printf '%s\n' "$((NOWS-1700))" > "$ARM" # armed, fresh, near-lapse + bash -c "printf '%s' '$(sentinel_bash_payload "cd $OUT && git status" "$HOME" "$SID")' | bash '$WRITER'" + [ ! -f "$OUTGIT/land-deploy-clearance" ] # out of scope -> not minted + [ "$(head -1 "$ARM")" = "$((NOWS-1700))" ] # arm marker UNCHANGED: no slide out of scope + rm -rf "$OUT"; rm -f "$ARM" +} diff --git a/eng/hooks/tests/ship-pr-gate.bats b/eng/hooks/tests/ship-pr-gate.bats index 15e2e29..20d074c 100644 --- a/eng/hooks/tests/ship-pr-gate.bats +++ b/eng/hooks/tests/ship-pr-gate.bats @@ -373,6 +373,82 @@ sentinel_bash_payload() { printf '{"hook_event_name":"PreToolUse","tool_name":"B [ -f "$GITDIR/ship-pr-clearance" ] } +# ---- sliding arm window (the long-ship fix) -------------------------------- + +@test "armed mint: an armed Bash command SLIDES the arm window forward (long /ship stays armed past the initial ARM_TTL)" { + # THE long-ship fix. The arm marker used to be written once at /ship invocation and + # never refreshed, so ARM_TTL(1200s) later the Bash-mint went inert, the sentinel + # froze at its last mint, and any /ship whose invocation->create span exceeded the + # TTL found it expired (the 2026-07-21 mutwo-skills incident). Now every armed Bash + # command that mints into the target repo re-arms the session, so an actively- + # working /ship keeps itself armed. Assert the slide directly: an arm marker near + # expiry is advanced to ~now by a mint. + opt_in + SID="sgtest-$BATS_TEST_NUMBER" + ARM="${TMPDIR:-/tmp}/gstack-ship-armed-$SID" + NOWS=$(date +%s) + printf '%s\n' "$((NOWS-1100))" > "$ARM" # armed 1100s ago: still fresh (<1200), 100s from lapsing + bash -c "printf '%s' '$(sentinel_bash_payload "cd $REPO && git status" "$HOME" "$SID")' | bash '$SENTINEL_HOOK'" + [ -f "$GITDIR/ship-pr-clearance" ] # minted into the target repo + [ "$(head -1 "$ARM")" -ge "$((NOWS-10))" ] # arm window slid forward to ~now (was T-1100) + rm -f "$ARM" +} + +@test "armed mint + gate: a stale sentinel from early in a long run is refreshed by a later armed Bash command -> create allowed (incident repro)" { + # Reproduces the incident end to end: the FIRST mint has since expired, yet the + # session is still armed and running repo commands. A later armed Bash command must + # re-mint a fresh sentinel so the eventual `gh pr create` PASSES instead of the + # blocking [sentinel: expired] observed in the field. + opt_in + SID="sgtest-$BATS_TEST_NUMBER" + ARM="${TMPDIR:-/tmp}/gstack-ship-armed-$SID" + NOWS=$(date +%s) + write_sentinel "$((NOWS-3000))" 1200 # an early mint, now long expired + run bash -c "printf '%s' '$(bash_payload "cd $REPO && gh pr create --base main")' | bash '$GATE'" + echo "$output" | grep -q '"decision":"block"' # stale sentinel blocks: the bug's symptom + printf '%s\n' "$NOWS" > "$ARM" # session still armed (slid by ongoing repo work) + bash -c "printf '%s' '$(sentinel_bash_payload "cd $REPO && git status" "$HOME" "$SID")' | bash '$SENTINEL_HOOK'" + run bash -c "printf '%s' '$(bash_payload "cd $REPO && gh pr create --base main")' | bash '$GATE'" + [ "$status" -eq 0 ]; [ -z "$output" ] # refreshed sentinel -> allowed + rm -f "$ARM" +} + +@test "armed mint: an EXPIRED arm marker neither mints nor self-arms (slide only extends a live window)" { + # The slide must not resurrect a dead window: under an expired arm marker a Bash + # command must not mint AND must not re-arm, so an abandoned /ship (idle > ARM_TTL) + # cannot keep authorizing creates. This is the accident-guard backstop for sliding. + opt_in + SID="sgtest-$BATS_TEST_NUMBER" + ARM="${TMPDIR:-/tmp}/gstack-ship-armed-$SID" + NOWS=$(date +%s) + printf '%s\n' "$((NOWS-3000))" > "$ARM" # expired (age 3000 > ARM_TTL 1200) + bash -c "printf '%s' '$(sentinel_bash_payload "cd $REPO && git status" "$HOME" "$SID")' | bash '$SENTINEL_HOOK'" + [ ! -f "$GITDIR/ship-pr-clearance" ] # did not mint + [ "$(head -1 "$ARM")" = "$((NOWS-3000))" ] # arm marker unchanged: no self-arm + rm -f "$ARM" +} + +@test "armed mint: an armed Bash command OUT of ~/dev scope does NOT slide the arm window (the return-1 guard)" { + # Pins the mint_for_dir `return 0`->`return 1` flip, the whole mechanism gating the + # slide: an armed, still-fresh session running a Bash command that resolves OUTSIDE + # ~/dev must NOT re-arm, so the window stays bound to genuine target-repo activity. + # Arm at a distinctly OLDER time so a wrongful slide (to ~now) is detectable; a + # regression to `return 0` would silently re-arm off any out-of-scope Bash activity + # and this test would catch it (the "non-dev repo mints nothing" test cannot: it + # arms at ~now, so a bad slide to ~now is invisible). + SID="sgtest-$BATS_TEST_NUMBER" + ARM="${TMPDIR:-/tmp}/gstack-ship-armed-$SID" + NOWS=$(date +%s) + OUT=$(mktemp -d "/tmp/.sgtestout.XXXXXX") + git -C "$OUT" init -q; git -C "$OUT" commit -q --allow-empty -m init + OUTGIT=$(git -C "$OUT" rev-parse --absolute-git-dir) + printf '%s\n' "$((NOWS-1100))" > "$ARM" # armed, fresh, near-lapse + bash -c "printf '%s' '$(sentinel_bash_payload "cd $OUT && git status" "$HOME" "$SID")' | bash '$SENTINEL_HOOK'" + [ ! -f "$OUTGIT/ship-pr-clearance" ] # out of scope -> not minted + [ "$(head -1 "$ARM")" = "$((NOWS-1100))" ] # arm marker UNCHANGED: no slide out of scope + rm -rf "$OUT"; rm -f "$ARM" +} + # ======================================================================== # Out-of-~/dev binding: a `gh pr create --repo ` run from a cwd # OUTSIDE ~/dev (e.g. a session anchored in a Google Drive folder) must still be