From 2624804b0bdec074f79f1ca243c0cf12a898e385 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Sun, 28 Jun 2026 16:45:43 -0600 Subject: [PATCH 1/2] feat: numeric-id positional heuristic for PR commands bb pr 42 from inside a checkout now treats 42 as the PR id and auto-detects the repo from git context, instead of treating 42 as a repo slug and erroring with 'Usage: bb pr-view [repo] '. Mirrors the state-recognition heuristic v1.2.0 added for cmd_pr_list (bb prs MERGED). Same gotcha class, same fix shape. New helper _resolve_pr_args centralizes the heuristic: if $1 is purely digits, treat it as the id (auto-detect repo); otherwise the existing positional form ([repo] ) is preserved. Sets caller-scope repo / pr_id / pr_args_consumed so callers with trailing args (cmd_pr_merge strategy at slot 3-or-2, cmd_pr_comment_add body at slot 3-or-2) 'shift $pr_args_consumed' to reach extras uniformly. Applied to all 8 PR-id commands: cmd_pr_view, cmd_pr_diff, cmd_pr_comments, cmd_pr_approve, cmd_pr_unapprove, cmd_pr_merge, cmd_pr_decline, cmd_pr_comment_add. Edge case: a repo literally named with pure digits (slug '42') would be shadowed. Acceptable since Bitbucket slugs are lowercase-hyphenated by convention; the explicit 'workspace/42 ' form remains an escape hatch (the heuristic only triggers on the bare-digits form). End-to-end verification against daniel-pittman/bb-cli-test PR#2: bb pr 2 -> shows PR with auto-detected repo bb pr-diff 2 -> shows diff bb pr-merge 2 -> merges (also exercises v1.4.3's POST fix) bb pr / 2 -> old form still works bb pr -> usage error still fires, message updated MCP side unaffected: Python tools take named args (pr_id=42), not positionals, so this gotcha can't fire there. 565 pytest tests pass. --- bb | 84 ++++++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 27 deletions(-) diff --git a/bb b/bb index 8f0cdbd..9509ede 100755 --- a/bb +++ b/bb @@ -233,6 +233,38 @@ resolve_workspace() { fi } +# Resolve positional args for PR commands of shape `bb [repo] [extras...]`. +# Sets caller-scope: +# repo -- via resolve_repo (caller must `local repo`) +# pr_id -- the PR id (caller must `local pr_id`) +# pr_args_consumed -- 1 or 2 (caller `shift $pr_args_consumed` to reach extras) +# +# Heuristic: if $1 is purely digits, treat it as the id and auto-detect the +# repo from git. Otherwise $1 is the [repo] slot and $2 is the id (the +# existing positional form, unchanged). This makes `bb pr 42` Just Work +# from inside a checkout instead of treating 42 as a repo slug. Mirrors +# the state-recognition heuristic in cmd_pr_list (bb prs MERGED). +# +# Tradeoff: a repo literally named with pure digits (e.g. slug "42") would +# be shadowed by the heuristic. Acceptable: Bitbucket slugs by convention +# are lowercase-hyphenated, pure-digit slugs are vanishingly rare, and the +# explicit "workspace/42" form remains as a clean escape hatch (the +# heuristic only triggers on the bare-digits form). +_resolve_pr_args() { + case "${1:-}" in + ''|*[!0-9]*) + resolve_repo "${1:-}" + pr_id="${2:-}" + pr_args_consumed=2 + ;; + *) + resolve_repo "" + pr_id="$1" + pr_args_consumed=1 + ;; + esac +} + repo_path() { local repo="$1" # Validate inputs at the boundary so a malformed slug doesn't @@ -935,12 +967,11 @@ cmd_pr_list() { } cmd_pr_view() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" if [[ -z "$pr_id" ]]; then - echo "Usage: bb pr-view [repo] " >&2 + echo "Usage: bb pr-view [repo] (or: bb pr from inside a checkout)" >&2 exit 1 fi @@ -1077,9 +1108,8 @@ cmd_pr_create() { } cmd_pr_approve() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" if [[ -z "$pr_id" ]]; then echo "Usage: bb pr-approve [repo] " >&2 @@ -1099,9 +1129,8 @@ cmd_pr_approve() { } cmd_pr_unapprove() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" if [[ -z "$pr_id" ]]; then echo "Usage: bb pr-unapprove [repo] " >&2 @@ -1120,10 +1149,10 @@ cmd_pr_unapprove() { } cmd_pr_merge() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" - local strategy="${3:-merge_commit}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" + shift $pr_args_consumed + local strategy="${1:-merge_commit}" if [[ -z "$pr_id" ]]; then echo "Usage: bb pr-merge [repo] [strategy]" >&2 @@ -1168,9 +1197,8 @@ cmd_pr_merge() { } cmd_pr_decline() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" if [[ -z "$pr_id" ]]; then echo "Usage: bb pr-decline [repo] " >&2 @@ -1188,9 +1216,8 @@ cmd_pr_decline() { } cmd_pr_diff() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" if [[ -z "$pr_id" ]]; then echo "Usage: bb pr-diff [repo] " >&2 @@ -1207,9 +1234,8 @@ cmd_pr_diff() { } cmd_pr_comments() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" if [[ -z "$pr_id" ]]; then echo "Usage: bb pr-comments [repo] " >&2 @@ -1231,10 +1257,10 @@ cmd_pr_comments() { } cmd_pr_comment_add() { - local repo - resolve_repo "${1:-}" - local pr_id="${2:-}" - local body="${3:-}" + local repo pr_id pr_args_consumed + _resolve_pr_args "$@" + shift $pr_args_consumed + local body="${1:-}" if [[ -z "$pr_id" || -z "$body" ]]; then echo "Usage: bb pr-comment [repo] " >&2 @@ -2745,6 +2771,10 @@ GLOBAL FLAGS NOTES [repo] is auto-detected from the current git remote if omitted. + For PR-id commands (pr / pr-merge / pr-diff / pr-comments / pr-approve / + pr-unapprove / pr-decline / pr-comment), a bare numeric first arg is + treated as the PR id and the repo is auto-detected (e.g. `bb pr 42` from + inside the checkout). Pass `workspace/slug ` for an explicit repo. Config: ~/.config/bb/config or env vars BB_USER, BB_TOKEN, BB_WORKSPACE. Auth uses Atlassian API tokens with HTTP Basic auth. From 63920308ce6483810a2bb488d43b4a2b60271abc Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Sun, 28 Jun 2026 16:54:23 -0600 Subject: [PATCH 2/2] Address PR #40 MEDIUM: usage check must precede 'shift $pr_args_consumed' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer caught a real regression my happy-path smoke missed: `bb pr-merge myrepo` (a single non-numeric arg, repo without id) hits `shift 2` against $# == 1, which returns non-zero, which under 'set -euo pipefail' aborts the script BEFORE the friendly Usage: message can fire. User sees a silent exit 1 where they previously got a clear usage hint. Fix: move the empty-pr_id check above the shift in both functions that shift. Once pr_id is verified non-empty, the consumed args are guaranteed present so the shift is safe. cmd_pr_merge — single check before shift. cmd_pr_comment_add — split into two checks (pr_id before shift, body after) because body lives in the post-shift slot. Factored the Usage message into a local _usage_pr_comment helper so both checks emit the same text. Also: extended both Usage messages to surface the new short form (`bb pr-merge [strategy] from inside a checkout` etc.) so the heuristic is discoverable from the error path too. Smoke-verified the 4 regression cases (bb pr-merge myrepo, bb pr-comment myrepo, bb pr-comment myrepo 42, bb pr-merge): all now print the Usage message with exit=1. Happy path (bb pr 1 in a checkout) unchanged. --- bb | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/bb b/bb index 9509ede..04e6d08 100755 --- a/bb +++ b/bb @@ -1151,14 +1151,18 @@ cmd_pr_unapprove() { cmd_pr_merge() { local repo pr_id pr_args_consumed _resolve_pr_args "$@" - shift $pr_args_consumed - local strategy="${1:-merge_commit}" - + # Check usage BEFORE `shift $pr_args_consumed`. If the user passed a + # single non-numeric arg (e.g. `bb pr-merge myrepo` with no id), + # pr_args_consumed=2 but only 1 positional exists, so shift 2 returns + # non-zero, which under `set -euo pipefail` aborts the script before + # the friendly Usage message can print. if [[ -z "$pr_id" ]]; then - echo "Usage: bb pr-merge [repo] [strategy]" >&2 + echo "Usage: bb pr-merge [repo] [strategy] (or: bb pr-merge [strategy] from inside a checkout)" >&2 echo " Strategies: merge_commit (default), squash, fast_forward" >&2 exit 1 fi + shift $pr_args_consumed + local strategy="${1:-merge_commit}" # Validate strategy against Bitbucket's accepted set so a typo # (e.g. "squash_commit") fails locally with a clear message @@ -1259,15 +1263,23 @@ cmd_pr_comments() { cmd_pr_comment_add() { local repo pr_id pr_args_consumed _resolve_pr_args "$@" - shift $pr_args_consumed - local body="${1:-}" - - if [[ -z "$pr_id" || -z "$body" ]]; then - echo "Usage: bb pr-comment [repo] " >&2 + # Same ordering rule as cmd_pr_merge: check pr_id BEFORE shift to + # avoid `shift 2` aborting under `set -euo pipefail` when the user + # passed only one (non-numeric) arg. + _usage_pr_comment() { + echo "Usage: bb pr-comment [repo] (or: bb pr-comment from inside a checkout)" >&2 echo "" >&2 echo " Add a top-level comment to PR #." >&2 echo " Use single quotes around if it contains spaces or shell metacharacters." >&2 exit 1 + } + if [[ -z "$pr_id" ]]; then + _usage_pr_comment + fi + shift $pr_args_consumed + local body="${1:-}" + if [[ -z "$body" ]]; then + _usage_pr_comment fi # Bitbucket's contract: POST {"content": {"raw": ""}}.