Skip to content

Commit 698de40

Browse files
committed
ghx: fix pr create compatibility
1 parent fc7368d commit 698de40

File tree

2 files changed

+121
-11
lines changed

2 files changed

+121
-11
lines changed

skills/ghx/scripts/lib/publish.sh

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ resolve_output_path_for_read() {
8080
esac
8181
}
8282

83+
extract_pr_url_from_text() {
84+
local text="$1"
85+
printf '%s\n' "$text" | grep -Eo 'https://github\.com/[^[:space:]]+/pull/[0-9]+' | tail -n 1
86+
}
87+
8388
# Find existing comment by marker
8489
# Usage: find_existing_comment <pr_number> <marker>
8590
# Output: Comment ID if found, empty string otherwise
@@ -234,17 +239,26 @@ action_create_pr() {
234239
return 0
235240
fi
236241

237-
# Check if head branch exists (best-effort, don't fail for remote branches)
242+
# Check if head branch exists (best-effort, don't fail for remote branches).
243+
# Avoid assuming the caller's current directory is a git worktree.
238244
log_info "Verifying head branch (best-effort): $head"
239-
if ! git rev-parse --verify "$head" >/dev/null 2>&1; then
240-
log_warn "Unable to verify local branch '$head'; it may be remote-only or a cross-fork ref. Proceeding and letting 'gh pr create' validate the head."
245+
if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
246+
if ! git rev-parse --verify "$head" >/dev/null 2>&1; then
247+
log_warn "Unable to verify local branch '$head'; it may be remote-only or a cross-fork ref. Proceeding and letting 'gh pr create' validate the head."
248+
fi
249+
else
250+
log_info "Current directory is not a git worktree; skipping local branch verification."
241251
fi
242252

253+
local body_file
254+
body_file=$(mktemp "${TMPDIR:-/tmp}/ghx-pr-body-XXXXXX.md")
255+
printf '%s' "$body_content" > "$body_file"
256+
243257
# Build gh pr create command
244258
local cmd=(gh pr create)
245259
cmd+=("--repo" "$PR_OWNER/$PR_REPO")
246260
cmd+=("--title" "$title")
247-
cmd+=("--body" "$body_content")
261+
cmd+=("--body-file" "$body_file")
248262
cmd+=("--base" "$base")
249263
cmd+=("--head" "$head")
250264

@@ -259,18 +273,34 @@ action_create_pr() {
259273
cmd+=("--label" "$label_list")
260274
fi
261275

262-
# Create PR
276+
# Create PR. Older gh versions do not support `gh pr create --json`.
263277
log_info "Creating PR..."
264-
local pr_json
265-
if ! pr_json=$("${cmd[@]}" --json number,url 2>&1); then
266-
log_error "Failed to create PR: $pr_json"
278+
local pr_output
279+
if ! pr_output=$("${cmd[@]}" 2>&1); then
280+
rm -f "$body_file"
281+
log_error "Failed to create PR: $pr_output"
267282
return 1
268283
fi
284+
rm -f "$body_file"
269285

270-
# Extract PR info from structured JSON output
286+
# gh pr create typically prints the PR URL on stdout. Fall back to pr list if needed.
271287
local pr_number pr_url
272-
pr_number=$(echo "$pr_json" | jq -r '.number')
273-
pr_url=$(echo "$pr_json" | jq -r '.url')
288+
pr_url=$(extract_pr_url_from_text "$pr_output" || true)
289+
if [[ -z "$pr_url" ]]; then
290+
local created_pr
291+
created_pr=$(gh pr list --head "$head" --repo "$PR_OWNER/$PR_REPO" --json number,url --jq '.[0] // empty' 2>/dev/null || echo "")
292+
if [[ -n "$created_pr" ]]; then
293+
pr_number=$(echo "$created_pr" | jq -r '.number // empty')
294+
pr_url=$(echo "$created_pr" | jq -r '.url // empty')
295+
fi
296+
fi
297+
if [[ -z "${pr_number:-}" && "$pr_url" =~ /pull/([0-9]+)$ ]]; then
298+
pr_number="${BASH_REMATCH[1]}"
299+
fi
300+
if [[ -z "${pr_number:-}" || -z "$pr_url" ]]; then
301+
log_error "Created PR but could not determine PR number/url from gh output: $pr_output"
302+
return 1
303+
fi
274304

275305
log_info "✅ Created PR #$pr_number"
276306
log_info " URL: $pr_url"

tests/skill-mode/test_publisher.sh

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,85 @@ INNEREOF
518518
cleanup_test_env "$tmp_dir"
519519
}
520520

521+
test_create_pr_compat_and_cwd_stable() {
522+
local test_name="create_pr_compat_and_cwd_stable"
523+
log_info "Running test: $test_name"
524+
525+
local tmp_dir
526+
tmp_dir=$(setup_test_env "$test_name")
527+
local output_dir="$tmp_dir/output"
528+
local bin_dir="$tmp_dir/bin"
529+
local caller_dir="$tmp_dir/caller"
530+
local body_file_rel="pr-body.md"
531+
532+
mkdir -p "$output_dir" "$bin_dir" "$caller_dir"
533+
printf 'PR body\n' > "$output_dir/$body_file_rel"
534+
535+
cat > "$bin_dir/gh" << 'INNEREOF'
536+
#!/usr/bin/env bash
537+
set -euo pipefail
538+
539+
if [[ "${1:-}" == "auth" && "${2:-}" == "status" ]]; then
540+
echo "github.com"
541+
echo " ✓ Logged in to github.com"
542+
exit 0
543+
fi
544+
545+
if [[ "${1:-}" == "pr" && "${2:-}" == "list" ]]; then
546+
echo "[]"
547+
exit 0
548+
fi
549+
550+
if [[ "${1:-}" == "pr" && "${2:-}" == "create" ]]; then
551+
for arg in "$@"; do
552+
if [[ "$arg" == "--json" ]]; then
553+
echo "unknown flag: --json" >&2
554+
exit 1
555+
fi
556+
done
557+
echo "https://github.com/owner/repo/pull/123"
558+
exit 0
559+
fi
560+
561+
exit 0
562+
INNEREOF
563+
chmod +x "$bin_dir/gh"
564+
export PATH="$bin_dir:$PATH"
565+
export GITHUB_OUTPUT_DIR="$output_dir"
566+
567+
local before_pwd after_pwd output status=0
568+
before_pwd=$(cd "$caller_dir" && pwd)
569+
if output=$(cd "$caller_dir" && bash "$GHX_SCRIPT" pr create --repo=owner/repo --title="Compat PR" --body-file="$body_file_rel" --head=feature/test --base=main 2>&1); then
570+
status=0
571+
else
572+
status=$?
573+
fi
574+
after_pwd=$(cd "$caller_dir" && pwd)
575+
576+
TESTS_RUN=$((TESTS_RUN + 1))
577+
if [[ $status -eq 0 ]]; then
578+
TESTS_PASSED=$((TESTS_PASSED + 1))
579+
log_info "✓ create-pr works without gh --json support"
580+
else
581+
TESTS_FAILED=$((TESTS_FAILED + 1))
582+
log_error "✗ create-pr should work with older gh (status=$status, output: $output)"
583+
fi
584+
585+
TESTS_RUN=$((TESTS_RUN + 1))
586+
if [[ "$before_pwd" == "$after_pwd" ]]; then
587+
TESTS_PASSED=$((TESTS_PASSED + 1))
588+
log_info "✓ create-pr does not disturb caller working directory"
589+
else
590+
TESTS_FAILED=$((TESTS_FAILED + 1))
591+
log_error "✗ create-pr should not change caller working directory"
592+
fi
593+
594+
assert_file_exists "$output_dir/publish-results.json" "publish-results.json should be generated for create-pr"
595+
assert_json_valid "$output_dir/publish-results.json" "create-pr publish-results.json should be valid JSON"
596+
597+
cleanup_test_env "$tmp_dir"
598+
}
599+
521600
test_reply_review_multiline_messages() {
522601
local test_name="reply_review_multiline"
523602
log_info "Running test: $test_name (regression test for issue #551)"
@@ -619,6 +698,7 @@ main() {
619698
test_body_file_stdin
620699
test_batch_and_direct_conflict
621700
test_batch_path_traversal_rejected
701+
test_create_pr_compat_and_cwd_stable
622702
test_reply_review_multiline_messages
623703

624704
# Summary

0 commit comments

Comments
 (0)