Skip to content

fix(decision-brief): a 404 on the branch-ref lookup is a fresh branch, not a parent SHA #2375

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.

Sign up for GitHub

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

Merged
don-petry merged 2 commits into from
Oct 20, 2026
Merged

fix(decision-brief): a 404 on the branch-ref lookup is a fresh branch, not a parent SHA #2375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4776ce1
fix(decision-brief): a 404 on the branch-ref lookup is a fresh branch…
Oct 20, 2026
165a915
fix(reviews): match HTTP 404 status, not the 'Not Found' message, in …
don-petry Oct 20, 2026
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
19 changes: 17 additions & 2 deletions scripts/decision-brief/open-pr.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,22 @@ fi
# ── base + branch state ───────────────────────────────────────────────────────
default_branch="$(gh api "repos/${REPO}" --jq '.default_branch')"
base_sha="$(gh api "repos/${REPO}/git/ref/heads/${default_branch}" --jq '.object.sha')"
parent_sha="$(gh api "repos/${REPO}/git/ref/heads/${branch}" --jq '.object.sha' 2>/dev/null || true)"
[[ "$base_sha" =~ ^[0-9a-f]{40}$ ]] || { echo "::error::could not resolve ${default_branch} on ${REPO} to a commit (got '${base_sha:0:80}')" >&2; exit 1; }
# `gh api` prints the error BODY to stdout on a non-2xx (the HTTP error goes to
# stderr), so `$(gh api … || true)` captures `{"message":"Not Found",…}` as the
# SHA on the first run for a slug and the next call becomes
# git/commits/{"message"…}. Only a 404 means "no branch yet"; anything else
# must fail closed rather than be read as a fresh branch.
err_file="$(mktemp)"
if parent_sha="$(gh api "repos/${REPO}/git/ref/heads/${branch}" --jq '.object.sha' 2>"$err_file")"; then
[[ "$parent_sha" =~ ^[0-9a-f]{40}$ ]] || { echo "::error::${branch} resolved to '${parent_sha:0:80}', not a commit SHA" >&2; rm -f "$err_file"; exit 1; }
elif grep -qE 'HTTP 404([^0-9]|$)' "$err_file"; then
parent_sha=""
else
echo "::error::could not resolve ${branch} on ${REPO} — $(tr '\n' ' ' <"$err_file" | cut -c1-200); not landing" >&2
rm -f "$err_file"; exit 1
fi
rm -f "$err_file"
branch_exists=0
if [[ -n "$parent_sha" ]]; then
branch_exists=1
Expand All @@ -149,7 +164,7 @@ if [[ -n "$parent_sha" ]]; then
err_file="$(mktemp)"
if branch_brief_b64="$(gh api "repos/${REPO}/contents/${dest_prefix}/brief.md?ref=${branch}" --jq '.content' 2>"$err_file")"; then
branch_brief="$(printf '%s' "$branch_brief_b64" | base64 -d)"
elif grep -qE 'HTTP 404|Not Found' "$err_file"; then
elif grep -qE 'HTTP 404([^0-9]|$)' "$err_file"; then
branch_brief=""
else
echo "::error::could not read ${dest_prefix}/brief.md on ${branch} to check for a slug collision — $(tr '\n' ' ' <"$err_file" | cut -c1-200); not appending" >&2
Expand Down
49 changes: 45 additions & 4 deletions tests/test_decision_brief.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,13 @@ mock_gh_live() {
mock_gh '
case "$*" in
"api repos/acme/widgets --jq .default_branch") echo main ;;
"api repos/acme/widgets/git/ref/heads/main --jq .object.sha") echo base000 ;;
"api repos/acme/widgets/git/ref/heads/main --jq .object.sha") echo ba5e000000000000000000000000000000000000 ;;
"api repos/acme/widgets/git/ref/heads/incubate/widget-lint --jq .object.sha")
if [ "${BRANCH_EXISTS:-0}" = 1 ]; then echo head111; else echo "not found" >&2; exit 1; fi ;;
# Real gh prints the error BODY to stdout and the HTTP line to stderr.
if [ "${BRANCH_EXISTS:-0}" = 1 ]; then echo 4ead111111111111111111111111111111111111;
elif [ "${BRANCH_REF_MODE:-absent}" = outage ]; then echo "{\"message\":\"Server Error\"}"; echo "gh: HTTP 503" >&2; exit 1;
elif [ "${BRANCH_REF_MODE:-absent}" = notfound503 ]; then echo "{\"message\":\"Not Found\"}"; echo "gh: Not Found (HTTP 503)" >&2; exit 1;
else echo "{\"message\":\"Not Found\",\"status\":\"404\"}"; echo "gh: Not Found (HTTP 404)" >&2; exit 1; fi ;;
*"contents/ideas/widget-lint/brief.md?ref=incubate/widget-lint"*)
case "${BRANCH_BRIEF_MODE:-ok}" in
ok) printf "%s" "${BRANCH_BRIEF:-# brief for https://github.com/acme/widgets/discussions/413}" | base64 | tr -d "\n" ;;
Expand Down Expand Up @@ -515,7 +519,7 @@ exit 0'
[ "$(grep -c 'git/blobs -X POST' "$GH_LOG")" -eq 3 ]
grep -q '"base_tree":"tree222"' "$GH_LOG.tree"
grep -q '"path":"ideas/widget-lint/brief.md"' "$GH_LOG.tree"
grep -q '"parents":\["base000"\]' "$GH_LOG.commit"
grep -q '"parents":\["ba5e000000000000000000000000000000000000"\]' "$GH_LOG.commit"
grep -q '"author":{"name":"don-petry","email":"don-petry@users.noreply.github.com"}' "$GH_LOG.commit"
grep -qF 'api repos/acme/widgets/git/refs -X POST -f ref=refs/heads/incubate/widget-lint -f sha=commit555' "$GH_LOG"
grep -q 'pr create --repo acme/widgets --base main --head incubate/widget-lint' "$GH_LOG"
Expand All @@ -535,7 +539,7 @@ exit 0'
BRANCH_EXISTS=1 PR_EXISTS=1 REPO="acme/widgets" SLUG="widget-lint" PACKAGE_DIR="$pkg" DISCUSSION_NUMBER=413 \
GIT_USER_NAME=t run bash "$DB_DIR/open-pr.sh"
[ "$status" -eq 0 ]
grep -q '"parents":\["head111"\]' "$GH_LOG.commit"
grep -q '"parents":\["4ead111111111111111111111111111111111111"\]' "$GH_LOG.commit"
grep -qF 'api repos/acme/widgets/git/refs/heads/incubate/widget-lint -X PATCH -f sha=commit555' "$GH_LOG"
! grep -q 'pr create' "$GH_LOG" || false
grep -q 'pr edit https://github.com/acme/widgets/pull/9 --repo acme/widgets --title brief(widget-lint)' "$GH_LOG"
Expand All @@ -555,6 +559,43 @@ exit 0'
! grep -q 'git/refs' "$GH_LOG" || false
}

@test "open-pr (live): a 404 BODY on the branch-ref lookup is a fresh branch, never a parent SHA" {
# The first run for a slug: the branch does not exist. gh prints the 404 body
# to stdout, so the old `$(gh api … || true)` captured it as the SHA and the
# next call became git/commits/{"message"…} (the first live BlockVault run).
pkg="$(copy_good)"
mock_gh_live
REPO="acme/widgets" SLUG="widget-lint" PACKAGE_DIR="$pkg" DISCUSSION_NUMBER=413 GIT_USER_NAME=t \
run bash "$DB_DIR/open-pr.sh"
[ "$status" -eq 0 ]
! grep -q 'git/commits/{' "$GH_LOG" || false
grep -qF 'api repos/acme/widgets/git/commits/ba5e000000000000000000000000000000000000 --jq .tree.sha' "$GH_LOG"
grep -q '"parents":\["ba5e000000000000000000000000000000000000"\]' "$GH_LOG.commit"
grep -qF 'git/refs -X POST -f ref=refs/heads/incubate/widget-lint' "$GH_LOG"
}

@test "open-pr (live): a non-404 failure resolving the branch ref fails closed before any write" {
pkg="$(copy_good)"
mock_gh_live
BRANCH_REF_MODE=outage REPO="acme/widgets" SLUG="widget-lint" PACKAGE_DIR="$pkg" DISCUSSION_NUMBER=413 \
GIT_USER_NAME=t run bash "$DB_DIR/open-pr.sh"
[ "$status" -ne 0 ]
[[ "$output" == *"could not resolve incubate/widget-lint"* ]]
! grep -q 'git/blobs' "$GH_LOG" || false
! grep -q 'git/refs' "$GH_LOG" || false
! grep -q 'pr create' "$GH_LOG" || false
}

@test "open-pr (live): a 'Not Found' message on a non-404 status is not read as an absent branch" {
pkg="$(copy_good)"
mock_gh_live
BRANCH_REF_MODE=notfound503 REPO="acme/widgets" SLUG="widget-lint" PACKAGE_DIR="$pkg" DISCUSSION_NUMBER=413 \
GIT_USER_NAME=t run bash "$DB_DIR/open-pr.sh"
[ "$status" -ne 0 ]
! grep -q 'git/blobs' "$GH_LOG" || false
! grep -q 'git/refs' "$GH_LOG" || false
}

@test "open-pr (live): a 404 on the branch brief means no collision; any other read failure fails closed" {
pkg="$(copy_good)"
mock_gh_live
Expand Down
Loading