-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(bin): add opt-in pre-acquire worktree pool safety sweep #2827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+1,375
−38
Closed
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
20043d3
feat(bin): add pre-acquire worktree pool sweep for reuse mitigation
ICGNU3 2c8b4c8
no-mistakes(review): fix errexit-swallowed sweep diagnostics and usag…
ICGNU3 ef42c90
no-mistakes(review): fail closed on unreadable reachability; harden s…
ICGNU3 3f82e4a
no-mistakes(review): resolve sweep config via FM_HOME; verify remote …
ICGNU3 bc71513
no-mistakes(review): refresh index before dirty check; hermetic sweep…
ICGNU3 dfd7ac7
no-mistakes(document): tighten worktree pool sweep config docs
ICGNU3 c2f3078
no-mistakes(lint): drop unused CASE_DIR from pool sweep wiring test r…
ICGNU3 839d689
chore: restart pipeline with updated no-mistakes
ICGNU3 7dd8c5d
no-mistakes(review): derive sweep help from header; distinct uninspec…
ICGNU3 fadecba
no-mistakes(review): distinguish unanswerable git probes from unsafe …
ICGNU3 153e172
no-mistakes(document): document unanswerable-probe refusals in sweep …
ICGNU3 afe18a5
no-mistakes: apply CI fixes
ICGNU3 5363065
no-mistakes: apply CI fixes
ICGNU3 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| #!/usr/bin/env bash | ||
| # bin/fm-treehouse-pool-sweep.sh - Pre-acquire worktree pool safety sweep. | ||
| # | ||
| # This is a MITIGATION (not a fix) for the worktree reuse incident. It inspects | ||
| # pooled worktrees before acquisition and refuses to request one when unsafe pool | ||
| # state is observed. The upstream invariant is: "No consumer can reuse a worktree | ||
| # whose state is unsafe." This mitigation can only observe and refuse; it cannot | ||
| # enforce the invariant across all consumers. | ||
| # | ||
| # Two structural gaps this mitigation cannot close: | ||
| # 1. A direct `treehouse get` by anything other than firstmate bypasses this sweep. | ||
| # 2. Another firstmate home can race between sweep and acquire. | ||
| # | ||
| # Usage: fm-treehouse-pool-sweep.sh <worktree-path> | ||
| # Exit codes: | ||
| # 0 - Worktree is safe to acquire (or sweep is disabled) | ||
| # 1 - Worktree is unsafe: dirty | ||
| # 2 - Worktree is unsafe: HEAD contains commits not reachable from durable refs | ||
| # 3 - Worktree is unsafe: HEAD covered only by remote-tracking refs (prunable) | ||
| # 4 - Worktree does not exist | ||
| # 64 - Usage error (no worktree path given) | ||
| set -euo pipefail | ||
|
|
||
| usage() { | ||
| cat <<EOF | ||
| fm-treehouse-pool-sweep.sh - Pre-acquire worktree pool safety sweep | ||
|
|
||
| This is a MITIGATION for the worktree reuse incident (not a fix for the | ||
| underlying invariant). It inspects pooled worktrees before acquisition and | ||
| refuses to request one when unsafe pool state is observed. | ||
|
|
||
| Usage: fm-treehouse-pool-sweep.sh <worktree-path> | ||
|
|
||
| The sweep checks two conditions and refuses on either: | ||
| 1. Dirty worktree: tracked modifications, staged changes, or untracked | ||
| non-ignored files. | ||
| 2. HEAD contains at least one commit not reachable from an approved durable ref: | ||
| - refs/heads/* (local branches) | ||
| - refs/tags/* (tags) | ||
| - refs/firstmate/rescue/* (reserved rescue namespace) | ||
|
|
||
| Reflogs are NOT refs. A commit reachable only from a reflog is unreferenced. | ||
|
|
||
| For refs/remotes/*: they are counted for reachability so an ordinary freshly- | ||
| checked-out pool worktree is not falsely refused, but the case where HEAD's | ||
| commits are covered ONLY by remote-tracking refs (and no local head or tag) is | ||
| classified as unsafe. | ||
|
|
||
| Exit codes: | ||
| 0 - Worktree is safe to acquire (or sweep is disabled) | ||
| 1 - Worktree is unsafe: dirty | ||
| 2 - Worktree is unsafe: HEAD contains commits not reachable from durable refs | ||
| 3 - Worktree is unsafe: HEAD covered only by remote-tracking refs (prunable) | ||
| 4 - Worktree does not exist | ||
| 64 - Usage error (no worktree path given) | ||
|
|
||
| Activation: | ||
| The sweep is disabled by default. To enable, create: | ||
| \$FM_HOME/config/worktree-pool-sweep | ||
| containing "on" (or any non-empty value other than "off"). | ||
| The config dir is \$FM_CONFIG_OVERRIDE when set, otherwise \$FM_HOME/config, | ||
| and \$FM_HOME defaults to the firstmate repo root - the same resolution every | ||
| other firstmate script uses, so an enable written for one home applies to | ||
| that home only. | ||
| A missing file, an empty file, or the value "off" leaves the sweep disabled. | ||
|
|
||
| This mitigation is distinct from the upstream Treehouse invariant: | ||
| - MITIGATION: "Firstmate refuses to request a worktree when it observes unsafe pool state." | ||
| - INVARIANT: "No consumer can reuse a worktree whose state is unsafe." | ||
|
|
||
| Structural gaps this mitigation cannot close: | ||
| 1. A direct treehouse get by anything other than firstmate bypasses the sweep. | ||
| 2. Another firstmate home can race between sweep and acquire: | ||
|
|
||
| T1 Firstmate A sweeps -> safe | ||
| T2 Firstmate B acquires/modifies the same pool | ||
| T3 Firstmate A calls treehouse get | ||
|
|
||
| This race can cause the worktree to be unsafe when Firstmate A uses it. | ||
| The eventual Treehouse fix must kill this atomically at allocation time. | ||
| EOF | ||
| } | ||
|
|
||
| if [ "${1:-}" = "-h" ] || [ "${1:-}" = "--help" ]; then | ||
| usage | ||
| exit 0 | ||
| fi | ||
|
|
||
| WT="${1:-}" | ||
| if [ -z "$WT" ]; then | ||
| echo "error: worktree path required" >&2 | ||
| usage >&2 | ||
| exit 64 | ||
| fi | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| FM_ROOT="${FM_ROOT_OVERRIDE:-$(cd "$SCRIPT_DIR/.." && pwd)}" | ||
| FM_HOME="${FM_HOME:-${FM_ROOT_OVERRIDE:-$FM_ROOT}}" | ||
| CONFIG_DIR="${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" | ||
| SWEEP_CONFIG="$CONFIG_DIR/worktree-pool-sweep" | ||
|
|
||
| is_sweep_enabled() { | ||
| if [ -f "$SWEEP_CONFIG" ]; then | ||
| local val | ||
| val=$(cat "$SWEEP_CONFIG" 2>/dev/null | tr -d '[:space:]') | ||
| [ -n "$val" ] && [ "$val" != "off" ] | ||
| else | ||
| return 1 | ||
| fi | ||
| } | ||
|
|
||
| if ! is_sweep_enabled; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [ ! -d "$WT" ]; then | ||
| exit 4 | ||
| fi | ||
|
|
||
| is_dirty() { | ||
| local wt=$1 | ||
| git -C "$wt" update-index -q --ignore-submodules --refresh >/dev/null 2>&1 || true | ||
| if ! git -C "$wt" diff-index --quiet --ignore-submodules HEAD 2>/dev/null; then | ||
| return 0 | ||
| fi | ||
| if ! git -C "$wt" diff-index --quiet --ignore-submodules --cached HEAD 2>/dev/null; then | ||
| return 0 | ||
| fi | ||
| local untracked | ||
| untracked=$(git -C "$wt" ls-files --others --exclude-standard 2>/dev/null) | ||
| if [ -n "$untracked" ]; then | ||
| return 0 | ||
| fi | ||
| return 1 | ||
| } | ||
|
|
||
| count_refs() { | ||
| local wt=$1 pattern=$2 | ||
| git -C "$wt" for-each-ref --format='%(refname)' "$pattern" 2>/dev/null | wc -l | ||
| } | ||
|
|
||
| has_durable_refs() { | ||
| local wt=$1 | ||
| local count | ||
| count=$(count_refs "$wt" 'refs/heads/') | ||
| count=$((count + $(count_refs "$wt" 'refs/tags/'))) | ||
| count=$((count + $(count_refs "$wt" 'refs/firstmate/rescue/'))) | ||
| [ "$count" -gt 0 ] | ||
| } | ||
|
|
||
| head_covered_by_remotes() { | ||
| local wt=$1 | ||
| local unremoted | ||
| unremoted=$(git -C "$wt" rev-list --count HEAD --not --remotes 2>/dev/null) || return 1 | ||
| case "$unremoted" in | ||
| '' | *[!0-9]*) return 1 ;; | ||
| esac | ||
| [ "$unremoted" -eq 0 ] | ||
| } | ||
|
|
||
| check_head_reachable() { | ||
| local wt=$1 | ||
| local unique_count | ||
| if ! has_durable_refs "$wt"; then | ||
| if head_covered_by_remotes "$wt"; then | ||
| return 3 | ||
| fi | ||
| return 2 | ||
| fi | ||
| unique_count=$(git -C "$wt" rev-list --count HEAD --not --branches --tags \ | ||
| --glob='refs/firstmate/rescue/*' 2>/dev/null) || return 2 | ||
| case "$unique_count" in | ||
| '' | *[!0-9]*) return 2 ;; | ||
| esac | ||
| if [ "$unique_count" -gt 0 ]; then | ||
| return 2 | ||
| fi | ||
| return 0 | ||
| } | ||
|
|
||
| if is_dirty "$WT"; then | ||
| echo "unsafe: dirty worktree at $WT" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| rc=0 | ||
| check_head_reachable "$WT" || rc=$? | ||
|
|
||
| if [ $rc -eq 2 ]; then | ||
| echo "unsafe: HEAD contains commits not reachable from durable refs in $WT" >&2 | ||
| exit 2 | ||
| elif [ $rc -eq 3 ]; then | ||
| echo "unsafe: HEAD commits covered only by remote-tracking refs (prunable) in $WT" >&2 | ||
| exit 3 | ||
| fi | ||
|
|
||
| exit 0 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Captain, when an acquired pool worktree is still checked out on a local task branch with unique committed work,
--branchessubtracts that branch and the sweep returns safe;freshen_spawn_worktree_basethen runsreset --hard origin/<default>while still on that branch, moving its only durable ref and leaving those commits reachable only through the reflog. Exclude the currently checked-out branch from this reachability proof or detach without deleting its ref before resetting, so this mitigation cannot discard unlanded commits.AGENTS.md reference: AGENTS.md:L30-L32
Useful? React with 👍 / 👎.