Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions scripts/dev-lead-fix-ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ set -euo pipefail

source "$(dirname "$0")/engine.sh"
source "$(dirname "$0")/lib/git-identity.sh"
source "$(dirname "$0")/lib/pr-worktree.sh"

PR_NUMBER="${PR_NUMBER:-}"
HEAD_SHA="${HEAD_SHA:-}"
Expand All @@ -20,6 +21,11 @@ MAX_FAIL_ATTEMPTS="${MAX_FAIL_ATTEMPTS:-2}"
MARKER_PREFIX="<!-- dev-lead-fix-ci sha="
EXHAUSTION_MARKER="<!-- dev-lead-fix-ci pr=${PR_NUMBER} status=exhausted -->"
export PROMPTS_DIR="${PROMPTS_DIR:-prompts/dev-lead}"
# Pin PROMPTS_DIR to an absolute path now, while CWD still points at the agent
# checkout — checkout_pr_in_worktree cds into the PR worktree, after which a
# relative PROMPTS_DIR would resolve against the PR branch (issue #448).
PROMPTS_DIR="$(resolve_abs "$PROMPTS_DIR")"
export PROMPTS_DIR

# check_idempotency: returns 0 (skip) if this exact SHA was already TERMINALLY
# handled, OR if a PR-level exhaustion marker exists (blocks all future SHAs).
Expand Down Expand Up @@ -200,8 +206,9 @@ main() {
exit 0
fi

# Checkout the PR branch for modification
gh pr checkout "$PR_NUMBER" --repo "$REPO"
# Checkout the PR branch for modification, in an isolated worktree so the
# branch switch never overwrites the agent's own prompts/scripts (issue #448).
checkout_pr_in_worktree "$PR_NUMBER" "$REPO"

local cycle=1
while [ "$cycle" -le "$MAX_CI_CYCLES" ]; do
Expand Down
12 changes: 10 additions & 2 deletions scripts/dev-lead-fix-reviews.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@ set -euo pipefail

source "$(dirname "$0")/engine.sh"
source "$(dirname "$0")/lib/git-identity.sh"
source "$(dirname "$0")/lib/pr-worktree.sh"

INTENT_TYPE="${INTENT_TYPE:-fix-reviews}"
PR_NUMBER="${PR_NUMBER:-}"
REPO="${REPO:-${GITHUB_REPOSITORY:-}}"
HEAD_SHA="${HEAD_SHA:-}"
DEV_LEAD_DRY_RUN="${DEV_LEAD_DRY_RUN:-false}"
export PROMPTS_DIR="${PROMPTS_DIR:-prompts/dev-lead}"
# Pin PROMPTS_DIR to an absolute path now, while CWD still points at the agent
# checkout — checkout_pr_in_worktree cds into the PR worktree, after which a
# relative PROMPTS_DIR would resolve against the PR branch (issue #448).
PROMPTS_DIR="$(resolve_abs "$PROMPTS_DIR")"
export PROMPTS_DIR

REVIEWS_MARKER_PREFIX="<!-- dev-lead-fix-reviews pr="

Expand All @@ -27,9 +33,11 @@ if [ -z "${HEAD_SHA:-}" ] && [ -n "${PR_NUMBER:-}" ] && [ "${DEV_LEAD_DRY_RUN:-f
HEAD_SHA=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}" --jq '.head.sha' 2>/dev/null || true)
fi

# Checkout the PR branch for modification (Requirement 1)
# Checkout the PR branch for modification (Requirement 1).
# Use an isolated worktree so switching to the PR branch never overwrites the
# agent's own prompts/scripts in the working tree (issue #448).
if [ "${DEV_LEAD_DRY_RUN:-false}" = "false" ] && [ -n "${PR_NUMBER:-}" ]; then
gh pr checkout "$PR_NUMBER" --repo "$REPO"
checkout_pr_in_worktree "$PR_NUMBER" "$REPO"
setup_git_identity
fi

Expand Down
90 changes: 90 additions & 0 deletions scripts/lib/pr-worktree.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
#!/usr/bin/env bash
# pr-worktree.sh — check out a PR branch in an isolated git worktree.
#
# Why this exists
# ---------------
# dev-lead's prompt templates (prompts/dev-lead/*.md) and helper scripts
# (scripts/*) live in the SAME repository the agent operates on. The previous
# approach ran `gh pr checkout` in place, switching the agent's working tree to
# the PR branch. When that branch predated a prompt/script file, the file
# vanished mid-run, e.g.:
#
# scripts/dev-lead-fix-reviews.sh: line 51:
# prompts/dev-lead/on-mention.md: No such file or directory
#
# (See petry-projects/.github-private run 27077585464 and issue #448.)
#
# A worktree gives the PR branch its own directory, so the agent checkout — and
# everything under it — stays on the agent ref while the engine edits the PR.
#
# Usage
# -----
# source "$(dirname "$0")/lib/pr-worktree.sh"
# PROMPTS_DIR="$(resolve_abs "$PROMPTS_DIR")" # pin BEFORE entering worktree
# checkout_pr_in_worktree "$PR_NUMBER" "$REPO" # cds into the worktree
# ... run engine, commit, push (now operating on the PR branch) ...
# # the worktree is removed automatically on exit (EXIT trap)
#
# IMPORTANT: resolve any path the agent reads after checkout (PROMPTS_DIR, and
# anything else relative to the original CWD) to an absolute path with
# resolve_abs BEFORE calling checkout_pr_in_worktree — the cd changes what
# relative paths resolve to.

# resolve_abs <path> — echo an absolute form of <path> when it exists, else the
# original string unchanged. Used to pin PROMPTS_DIR before we cd elsewhere.
resolve_abs() {
case "$1" in
/*) printf '%s\n' "$1" ;;
*) ( cd "$1" 2>/dev/null && pwd ) || printf '%s\n' "$1" ;;
esac
}

# Directory of the active PR worktree (empty when none). Tracked so the EXIT
# trap can tear it down, and so the cleanup is idempotent.
PR_WORKTREE_DIR=""
# The agent checkout to return to during cleanup; captured at checkout time.
PR_WORKTREE_AGENT_DIR=""

# checkout_pr_in_worktree <pr_number> <repo>
# Creates a temp worktree, checks the PR out inside it, and cds into it.
# Registers an EXIT trap that removes the worktree.
checkout_pr_in_worktree() {
local pr="$1" repo="$2"

PR_WORKTREE_AGENT_DIR="$(pwd)"

# mktemp -d gives us a private parent dir; the worktree leaf must NOT pre-exist
# because `git worktree add` refuses a populated target directory.
local parent
parent="$(mktemp -d "${TMPDIR:-/tmp}/dev-lead-wt-XXXXXX")"
PR_WORKTREE_DIR="${parent}/pr-${pr}"

# Tear the worktree down on exit (success or failure) so /tmp and the repo's
# worktree registry don't accumulate stale entries across runs.
trap cleanup_pr_worktree EXIT

# Start detached at the agent checkout's HEAD so `gh pr checkout` can create or
# switch to the PR branch inside the worktree without colliding with whatever
# branch the agent checkout already has out.
git worktree add --detach "$PR_WORKTREE_DIR" HEAD >/dev/null

cd "$PR_WORKTREE_DIR" || return 1
gh pr checkout "$pr" --repo "$repo"
Comment thread
don-petry marked this conversation as resolved.
}

# cleanup_pr_worktree — return to the agent checkout and remove the worktree.
# Idempotent and safe to call from an EXIT trap or by hand.
cleanup_pr_worktree() {
[ -n "${PR_WORKTREE_DIR:-}" ] || return 0
local wt="$PR_WORKTREE_DIR" parent
parent="$(dirname "$wt")"
PR_WORKTREE_DIR=""

cd "${PR_WORKTREE_AGENT_DIR:-/}" 2>/dev/null || true
git worktree remove --force "$wt" 2>/dev/null || true
# Best-effort: drop the mktemp parent and prune any dangling registry entry.
if [ -n "$parent" ] && [ "$parent" != "/" ] && [[ "$parent" == *"/dev-lead-wt-"* ]]; then
rm -rf "$parent" 2>/dev/null || true
fi
git worktree prune 2>/dev/null || true
}
11 changes: 8 additions & 3 deletions tests/dev-lead/unit/test_fix_reviews.bats
Original file line number Diff line number Diff line change
Expand Up @@ -691,18 +691,21 @@ GHEOF
# Use absolute path so envsubst can find the prompt even when running from git_repo dir
export PROMPTS_DIR="$SCRIPT_DIR/prompts/dev-lead"

# Real temp git repo with one uncommitted change to trigger commit_and_push
# Real temp git repo; the engine (claude stub) makes the uncommitted change
# inside the PR worktree, which is what triggers commit_and_push.
local git_repo
git_repo="$(mktemp -d)"
git -C "$git_repo" init -q
echo "initial" > "$git_repo/file.txt"
git -C "$git_repo" add .
git -C "$git_repo" -c user.email="init@test" -c user.name="Init" commit -q -m "initial"
echo "change" >> "$git_repo/file.txt"

# The worktree checkout is clean; the engine appends to file.txt in its CWD
# (the worktree) so the script sees an uncommitted change to commit.
cat > "$STUB_BIN_DIR/claude" << 'STUB'
#!/usr/bin/env bash
echo "Changes applied."
echo "change" >> file.txt
STUB
chmod +x "$STUB_BIN_DIR/claude"

Expand Down Expand Up @@ -754,14 +757,16 @@ GITEOF
echo "initial" > "$git_repo/file.txt"
git -C "$git_repo" add .
git -C "$git_repo" -c user.email="init@test" -c user.name="Init" commit -q -m "initial"
echo "change" >> "$git_repo/file.txt"

local comment_file
comment_file="$(mktemp)"

# The engine makes its change inside the PR worktree (its CWD), triggering
# commit_and_push (whose commit then fails via the git stub below).
cat > "$STUB_BIN_DIR/claude" << 'STUB'
#!/usr/bin/env bash
echo "Changes applied."
echo "change" >> file.txt
STUB
chmod +x "$STUB_BIN_DIR/claude"

Expand Down
108 changes: 108 additions & 0 deletions tests/dev-lead/unit/test_pr_worktree.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
#!/usr/bin/env bats
# Unit tests for scripts/lib/pr-worktree.sh
#
# Regression coverage for issue #448: checking out a PR branch must NOT remove
# the agent's own prompt/script files from the working tree. The library
# isolates the PR branch in a separate git worktree, so the agent checkout —
# and an absolute PROMPTS_DIR pinned to it — survives the branch switch.

SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)"
LIB="$SCRIPT_DIR/scripts/lib/pr-worktree.sh"

setup() {
STUB_BIN_DIR="$(mktemp -d)"
# gh stub: `gh pr checkout <n>` switches the worktree to the PR branch,
# mirroring the real command (which is what made the old prompt files vanish).
cat > "$STUB_BIN_DIR/gh" <<'GHEOF'
#!/usr/bin/env bash
case "$1 $2" in
"pr checkout") git checkout -q old ;;
*) echo "{}" ;;
esac
GHEOF
chmod +x "$STUB_BIN_DIR/gh"
export PATH="$STUB_BIN_DIR:$PATH"

# Repo with a `main` branch that has prompts/x.md and an `old` branch that
# predates it — the exact shape that broke run 27077585464.
AGENT_REPO="$(mktemp -d)"
git -C "$AGENT_REPO" init -q -b main
git -C "$AGENT_REPO" -c user.email=t@t -c user.name=t commit -q --allow-empty -m base
git -C "$AGENT_REPO" branch old
mkdir -p "$AGENT_REPO/prompts"
echo "PROMPT BODY" > "$AGENT_REPO/prompts/x.md"
git -C "$AGENT_REPO" add prompts/x.md
git -C "$AGENT_REPO" -c user.email=t@t -c user.name=t commit -q -m "add prompt"
}

teardown() {
rm -rf "$STUB_BIN_DIR" "$AGENT_REPO"
}

@test "resolve_abs: leaves an absolute path unchanged" {
source "$LIB"
run resolve_abs /etc
[ "$status" -eq 0 ]
[ "$output" = "/etc" ]
}

@test "resolve_abs: converts an existing relative path to absolute" {
source "$LIB"
cd "$AGENT_REPO"
run resolve_abs prompts
[ "$status" -eq 0 ]
[ "$output" = "$AGENT_REPO/prompts" ]
}

@test "resolve_abs: returns a non-existent path unchanged" {
source "$LIB"
run resolve_abs does/not/exist
[ "$status" -eq 0 ]
[ "$output" = "does/not/exist" ]
}

@test "checkout_pr_in_worktree: PR branch switch does not clobber the agent prompts" {
source "$LIB"
cd "$AGENT_REPO"
# Pin PROMPTS_DIR before entering the worktree, as the real scripts do.
local prompts; prompts="$(resolve_abs prompts)"

checkout_pr_in_worktree 42 owner/repo

# We are now inside the worktree, on the `old` branch where prompts/x.md
# never existed — proving the branch switch really happened here.
[ "$PWD" = "$PR_WORKTREE_DIR" ]
[ ! -e "$PWD/prompts/x.md" ]

# The agent checkout (and the absolute PROMPTS_DIR pointing at it) is intact:
# this is the guarantee that issue #448 broke.
[ -f "$prompts/x.md" ]
[ "$(cat "$prompts/x.md")" = "PROMPT BODY" ]

cleanup_pr_worktree
}

@test "cleanup_pr_worktree: removes the worktree and returns to the agent checkout" {
source "$LIB"
cd "$AGENT_REPO"

checkout_pr_in_worktree 42 owner/repo
local wt="$PR_WORKTREE_DIR"
[ -d "$wt" ]

cleanup_pr_worktree

[ "$PWD" = "$AGENT_REPO" ]
[ ! -d "$wt" ]
[ -z "$PR_WORKTREE_DIR" ]
# Registry no longer references the removed worktree.
run git -C "$AGENT_REPO" worktree list
[[ "$output" != *"$wt"* ]]
}

@test "cleanup_pr_worktree: is a no-op when no worktree is active" {
source "$LIB"
PR_WORKTREE_DIR=""
run cleanup_pr_worktree
[ "$status" -eq 0 ]
}
Loading