diff --git a/scripts/lib/standards-deploy.sh b/scripts/lib/standards-deploy.sh index d18a2d60..1450eb4c 100644 --- a/scripts/lib/standards-deploy.sh +++ b/scripts/lib/standards-deploy.sh @@ -8,7 +8,8 @@ # # 1. checks for an already-open PR on the sync branch (idempotent skip), # 2. preflights that the token can write the repo (clear finding on a scope gap), -# 3. creates (or reuses) a sync branch off the repo's default branch, +# 3. creates a sync branch off the repo's default branch (an existing one with +# no open PR is reset to the default-branch tip, never reused as-is), # 4. PUTs the verbatim file content onto that branch via the Contents API, # 5. opens a labeled pull request. # @@ -33,7 +34,7 @@ _STANDARDS_DEPLOY_SOURCED=1 # repo_slug owner/name of the target repo (e.g. combsmith/markets) # path path of the file within the target repo # local_file local file whose contents are deployed verbatim -# branch head branch to create/reuse for the PR +# branch head branch to create (or reset) for the PR # label label applied to the opened PR (e.g. standards-sync) # title commit message and PR title # body PR body (markdown) @@ -113,7 +114,13 @@ sd_deploy_files_via_pr() { return 1 fi - # 4. Create the sync branch; reuse it if a prior run already created it. + # 4. Create the sync branch off the default-branch tip. If a prior run already + # created it (same-date name), it may carry that run's commits — or later + # pushes to them — which must NOT leak into this run's PR + # (combsmith/.github#1296). Step 1 found no open labeled PR; if no open + # PR of any label uses the branch either, force-reset it to the base tip so + # it holds only what this run writes. Otherwise, or if the probe or reset + # errors, fail loudly naming the branch rather than build on unknown commits. if ! gh api "repos/${repo}/git/refs" --method POST \ --raw-field "ref=refs/heads/${branch}" \ --raw-field "sha=${base_sha}" --silent 2>/dev/null; then @@ -121,11 +128,34 @@ sd_deploy_files_via_pr() { echo "FAILED no-branch" return 1 fi + # `gh pr list --head` matches the branch name only, so an open PR from a + # fork using the same name would match; keep only PRs whose head repo is $repo. + local branch_pr repo_l="${repo,,}" + if ! branch_pr=$(gh pr list --repo "$repo" --head "$branch" --state open \ + --json number,headRepository,headRepositoryOwner \ + --jq "[.[] | select((.headRepositoryOwner.login | ascii_downcase) == \"${repo_l%%/*}\" and (.headRepository.name | ascii_downcase) == \"${repo_l##*/}\")] | .[0].number // \"\"" \ + 2>/dev/null); then + echo "FAILED branch-pr-probe-failed:${branch}" + return 1 + fi + if [ -n "$branch_pr" ]; then + echo "FAILED branch-in-use:${branch}#${branch_pr}" + return 1 + fi + local reset_err reset_msg + reset_err=$(mktemp) + if ! gh api "repos/${repo}/git/refs/heads/${branch}" --method PATCH \ + --raw-field "sha=${base_sha}" -F force=true --silent 2>"$reset_err"; then + reset_msg=$(tr '\n' ' ' < "$reset_err"); rm -f "$reset_err" + echo "FAILED branch-reset-failed:${branch}${reset_msg:+:${reset_msg}}" + return 1 + fi + rm -f "$reset_err" fi # 5. PUT each file verbatim onto the branch. The blob SHA is looked up per file # on the branch so both create (absent → empty SHA) and update (drifted stub) - # work, whether the branch is fresh or reused. Errors are NOT swallowed: a + # work, whether the branch is fresh or reset. Errors are NOT swallowed: a # read failure that yields an empty SHA would make the subsequent PUT omit # `sha` and 422 against an existing file — the exact opaque failure in #864. local path local_file branch_sha encoded j err_file get_rc get_msg put_msg diff --git a/test/scripts/lib/standards-deploy.bats b/test/scripts/lib/standards-deploy.bats index 80dc2ce7..becbf3a3 100644 --- a/test/scripts/lib/standards-deploy.bats +++ b/test/scripts/lib/standards-deploy.bats @@ -23,7 +23,15 @@ args="$*" case "$sub" in pr) case "${1:-}" in - list) printf '%s' "${GH_EXISTING_PR:-}" ;; + list) + # --head : the "is the existing sync branch in use?" probe + # (any label). Otherwise: the label-keyed idempotency check. + if printf '%s' "$args" | grep -q -- '--head'; then + [ "${GH_BRANCH_PR_RC:-0}" -ne 0 ] && exit "${GH_BRANCH_PR_RC}" + printf '%s' "${GH_BRANCH_PR:-}" + else + printf '%s' "${GH_EXISTING_PR:-}" + fi ;; create) [ "${GH_PR_CREATE_RC:-0}" -ne 0 ] && exit "${GH_PR_CREATE_RC}" printf '%s\n' "${GH_PR_URL:-https://github.com/o/r/pull/1}" ;; @@ -33,6 +41,16 @@ case "$sub" in endpoint="${1:-}" case "$endpoint" in */git/refs) exit "${GH_REF_CREATE_RC:-0}" ;; + */git/refs/heads/*) + # Force-reset of an existing sync branch. When GH_BRANCH_FILES names a + # file it models the branch's content: a reset to the base tip drops + # every commit a prior run left on it. + if [ "${GH_REF_RESET_RC:-0}" -ne 0 ]; then + [ -n "${GH_REF_RESET_ERR:-}" ] && printf '%s\n' "$GH_REF_RESET_ERR" >&2 + exit "${GH_REF_RESET_RC}" + fi + [ -n "${GH_BRANCH_FILES:-}" ] && printf '' > "$GH_BRANCH_FILES" + exit 0 ;; */git/ref/heads/*) if printf '%s' "$args" | grep -q 'object.sha'; then printf '%s' "${GH_BASE_SHA-basesha111}" @@ -60,6 +78,10 @@ case "$sub" in printf '%s\n' "${GH_PUT_ERR:-gh: sha was not supplied (HTTP 422)}" >&2 exit "${GH_PUT_RC}" fi + if [ -n "${GH_BRANCH_FILES:-}" ]; then + p="${endpoint#*/contents/}" + grep -qxF "$p" "$GH_BRANCH_FILES" || printf '%s\n' "$p" >> "$GH_BRANCH_FILES" + fi exit 0 ;; repos/*) # repos/: the default-branch lookup AND the preflight write probe @@ -193,6 +215,91 @@ deploy() { grep -q 'gh pr create' "$GH_CALLS" } +# --------------------------------------------------------------------------- +# Stale same-date branch (combsmith/.github#1296): a prior run's branch — +# its PR closed — must not leak that run's commits into this run's PR. +# --------------------------------------------------------------------------- +@test "stale same-date branch with extra commits and no open PR: PR holds only this run's files" { + GH_REF_CREATE_RC="1"; export GH_REF_CREATE_RC # branch already exists + GH_BRANCH_FILES="${TT_TMP}/branch-files"; export GH_BRANCH_FILES + # The first run's 5 stubs, plus an agent's later "review fix" commit. + printf '%s\n' .github/workflows/a.yml .github/workflows/b.yml \ + .github/workflows/c.yml .github/workflows/d.yml .github/workflows/e.yml \ + > "$GH_BRANCH_FILES" + run sd_deploy_files_via_pr "combsmith/markets" "standards-sync/workflows-20261011" \ + "standards-sync" "chore: sync 1 stub" "body" \ + ".github/workflows/persona-mention.yml" "$LOCAL_FILE" + [ "$status" -eq 0 ] + [ "$output" = "OPENED https://github.com/o/r/pull/1" ] + [ "$(cat "$GH_BRANCH_FILES")" = ".github/workflows/persona-mention.yml" ] +} + +@test "stale branch is force-reset to the default-branch tip before any PUT" { + GH_REF_CREATE_RC="1"; export GH_REF_CREATE_RC + run deploy + [ "$status" -eq 0 ] + grep -q 'gh pr list --repo combsmith/markets --head standards-sync/add-to-project --state open' "$GH_CALLS" + grep -q 'gh api repos/combsmith/markets/git/refs/heads/standards-sync/add-to-project --method PATCH --raw-field sha=basesha111 -F force=true' "$GH_CALLS" + local reset_line put_line + reset_line=$(grep -n 'git/refs/heads/.* --method PATCH' "$GH_CALLS" | head -1 | cut -d: -f1) + put_line=$(grep -n 'contents/.* --method PUT' "$GH_CALLS" | head -1 | cut -d: -f1) + [ "$reset_line" -lt "$put_line" ] +} + +@test "a freshly created branch is not reset" { + run deploy + [ "$status" -eq 0 ] + [ "$(grep -c -- '--method PATCH' "$GH_CALLS")" -eq 0 ] + [ "$(grep -c 'gh pr list .*--head' "$GH_CALLS")" -eq 0 ] +} + +@test "existing branch used by an open (unlabeled) PR fails loudly naming the branch" { + GH_REF_CREATE_RC="1"; export GH_REF_CREATE_RC + GH_BRANCH_PR="77"; export GH_BRANCH_PR + run deploy + [ "$status" -ne 0 ] + [ "$output" = "FAILED branch-in-use:standards-sync/add-to-project#77" ] + [ "$(grep -c -- '--method PATCH' "$GH_CALLS")" -eq 0 ] + [ "$(grep -c 'contents/.* --method PUT' "$GH_CALLS")" -eq 0 ] + [ "$(grep -c 'gh pr create' "$GH_CALLS")" -eq 0 ] +} + +@test "existing branch whose PR probe errors is not reset" { + GH_REF_CREATE_RC="1"; export GH_REF_CREATE_RC + GH_BRANCH_PR_RC="1"; export GH_BRANCH_PR_RC + run deploy + [ "$status" -ne 0 ] + [ "$output" = "FAILED branch-pr-probe-failed:standards-sync/add-to-project" ] + [ "$(grep -c -- '--method PATCH' "$GH_CALLS")" -eq 0 ] + [ "$(grep -c 'contents/.* --method PUT' "$GH_CALLS")" -eq 0 ] +} + +@test "a failed reset of the stale branch fails loudly naming the branch" { + GH_REF_CREATE_RC="1"; export GH_REF_CREATE_RC + GH_REF_RESET_RC="1"; export GH_REF_RESET_RC + run deploy + [ "$status" -ne 0 ] + [ "$output" = "FAILED branch-reset-failed:standards-sync/add-to-project" ] + [ "$(grep -c 'contents/.* --method PUT' "$GH_CALLS")" -eq 0 ] + [ "$(grep -c 'gh pr create' "$GH_CALLS")" -eq 0 ] +} + +@test "a failed reset surfaces the API error text when one is available" { + GH_REF_CREATE_RC="1"; export GH_REF_CREATE_RC + GH_REF_RESET_RC="1"; export GH_REF_RESET_RC + GH_REF_RESET_ERR="gh: Cannot force-push (HTTP 422)"; export GH_REF_RESET_ERR + run deploy + [ "$status" -ne 0 ] + [ "$output" = "FAILED branch-reset-failed:standards-sync/add-to-project:gh: Cannot force-push (HTTP 422) " ] +} + +@test "the branch-in-use probe is scoped to PRs whose head repo is the target repo" { + GH_REF_CREATE_RC="1"; export GH_REF_CREATE_RC + run deploy + [ "$status" -eq 0 ] + grep -q 'headRepositoryOwner' "$GH_CALLS" +} + # --------------------------------------------------------------------------- # Update path: file already present on the branch → PUT carries its blob SHA # ---------------------------------------------------------------------------