From b3a47fc9c65058a97da46d74bc3f5086847afd0e Mon Sep 17 00:00:00 2001 From: Logan Gagne Date: Sat, 29 Aug 2026 16:54:04 -0400 Subject: [PATCH 1/2] fix(git-tools): correlate Gitea PR merge state and CI runs (#140) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Gitea-only defects made `pr show` and `run watch` report the wrong thing. Both fixes were written against an older git-cli and never landed; this rebases them onto current master, which has since gained the progress-aware idle-timeout handling that the original branch predated. - `pr show` on Gitea sourced from `tea pr list`, whose JSON omits the `merged` boolean and `merged_at` timestamp entirely, so a merged PR reported merged:false and callers treated it as still open. Fetch the PR detail via `tea api repos/{owner}/{repo}/pulls/` instead, mirroring the existing run:show/run:list pattern, and derive `state` from `merged` so it matches the GitHub path - Emit `merged_at` on the GitHub path too, so both platforms return the same shape - `run watch` timed out into status:no-workflow on Gitea even when CI ran: Gitea leaves head_branch empty on pull_request-triggered runs, so `run list --branch` correlates nothing. Resolve the branch head SHA up front and fall back to head_sha correlation when the branch-filtered lookup comes back empty. GitHub is unaffected — its server-side --branch filter populates the first call, so the SHA path is never taken - Add regression suites for both (6 tests, mock git/tea via PATH injection) - Bump git-tools 2.2.1 -> 2.2.2 and session 4.5.0 -> 4.5.1 for the vendored git-cli Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EddJxaTMXyj7o4ndrqfCxa --- .../git-tools/.claude-plugin/plugin.json | 2 +- plugins-claude/git-tools/scripts/git-cli | 81 +++++- .../session/.claude-plugin/plugin.json | 2 +- plugins-claude/session/scripts/git-cli | 81 +++++- .../git-tools/.claude-plugin/plugin.json | 2 +- .../session/.claude-plugin/plugin.json | 2 +- tests/git-cli/test-pr-show-gitea.sh | 246 ++++++++++++++++++ tests/git-cli/test-run-watch-gitea.sh | 167 ++++++++++++ utils/git-cli | 81 +++++- 9 files changed, 636 insertions(+), 28 deletions(-) create mode 100755 tests/git-cli/test-pr-show-gitea.sh create mode 100755 tests/git-cli/test-run-watch-gitea.sh diff --git a/plugins-claude/git-tools/.claude-plugin/plugin.json b/plugins-claude/git-tools/.claude-plugin/plugin.json index 518a5e1..50a3461 100644 --- a/plugins-claude/git-tools/.claude-plugin/plugin.json +++ b/plugins-claude/git-tools/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "git-tools", - "version": "2.2.1", + "version": "2.2.2", "description": "GitHub and Gitea tooling — unified CLI wrapper (issues, PRs, CI runs) plus a ship orchestrator that drives the full branch/commit/push/PR/watch/cleanup lifecycle", "author": { "name": "Logan Gagne" diff --git a/plugins-claude/git-tools/scripts/git-cli b/plugins-claude/git-tools/scripts/git-cli index 16ff305..679ad34 100755 --- a/plugins-claude/git-tools/scripts/git-cli +++ b/plugins-claude/git-tools/scripts/git-cli @@ -606,15 +606,46 @@ case "$cmd:$sub" in [[ -n "$num" ]] || die_usage "usage: git-tools pr show | --branch NAME" case "$PLATFORM" in github) - cli_json '{number, title, body: (.body // ""), state: (.state | ascii_downcase), merged: ((.state | ascii_downcase) == "merged"), author: .author.login, head: .headRefName, base: .baseRefName, labels: [.labels[].name], assignees: [.assignees[].login], mergeable: (.mergeable | ascii_downcase? // null), created_at: .createdAt, updated_at: .updatedAt, url, comments: [.comments[] | {author: .author.login, body, created_at: .createdAt}]}' \ + cli_json '{number, title, body: (.body // ""), state: (.state | ascii_downcase), merged: ((.state | ascii_downcase) == "merged"), author: .author.login, head: .headRefName, base: .baseRefName, labels: [.labels[].name], assignees: [.assignees[].login], mergeable: (.mergeable | ascii_downcase? // null), created_at: .createdAt, updated_at: .updatedAt, merged_at: .mergedAt, url, comments: [.comments[] | {author: .author.login, body, created_at: .createdAt}]}' \ env GH_NO_COLOR=1 gh pr view "$num" \ - --json "number,title,body,state,author,headRefName,baseRefName,labels,assignees,mergeable,createdAt,updatedAt,url,comments" + --json "number,title,body,state,author,headRefName,baseRefName,labels,assignees,mergeable,createdAt,updatedAt,mergedAt,url,comments" ;; gitea) - # tea has no single-PR view command; get from list filtered by number - cli_json "[.[] | select((.index | tonumber? // .index) == ${num})] | first | {number: (.index | tonumber? // .index), title, body: (.body // \"\"), state, merged: (.merged // false), author: (.author // \"\"), head: (.head // \"\"), base: (.base // \"\"), labels: (if .labels and (.labels | type) == \"array\" then [.labels[].name] else [] end), assignees: (if .assignees and (.assignees | type) == \"array\" then [.assignees[] | .login // empty] else [] end), mergeable: (.mergeable // null), created_at: (.created // \"\"), updated_at: (.updated // null), url: (.url // \"\"), comments: []}" \ - env NO_COLOR=1 tea pr list --output json --state all --limit 200 \ - --fields "index,title,body,state,author,head,base,labels,assignees,mergeable,created,updated,url" + # `tea pr list` omits the `merged` boolean and `merged_at` timestamp + # entirely (tea only encodes merge state into the `state` string) and + # emits `mergeable` as a string, so a merged PR reports + # merged:false / mergedAt:null and consumers think it is still open + # (#140). Fetch the PR detail via the REST API instead, mirroring the + # run:show/run:list `tea api` pattern, so merge fields are reliable. + pr_stderr=$(mktemp); pr_rc=0 + pr_detail=$(NO_COLOR=1 tea api "repos/{owner}/{repo}/pulls/${num}" 2>"$pr_stderr") \ + || pr_rc=$? + if [[ $pr_rc -ne 0 ]]; then + err=$(cat "$pr_stderr"); rm -f "$pr_stderr" + die "tea api repos/{owner}/{repo}/pulls/${num} failed: $err" + fi + rm -f "$pr_stderr" + # Derive `state` from `merged` so it stays consistent with the merge + # flags (Gitea REST reports state:"closed" for a merged PR) and matches + # the GitHub path, which emits state:"merged". + echo "$pr_detail" | jq '{ + number: (.number // .index), + title: (.title // ""), + body: (.body // ""), + state: (if (.merged // false) then "merged" else (.state // "") end), + merged: (.merged // false), + merged_at: (.merged_at // null), + author: (.user.login // .user.username // ""), + head: (.head.ref // .head.label // ""), + base: (.base.ref // .base.label // ""), + labels: (if (.labels | type) == "array" then [.labels[].name] else [] end), + assignees: (if (.assignees | type) == "array" then [.assignees[] | (.login // .username // empty)] else [] end), + mergeable: (.mergeable // null), + created_at: (.created_at // ""), + updated_at: (.updated_at // null), + url: (.html_url // .url // ""), + comments: [] + }' ;; esac ;; @@ -1052,6 +1083,40 @@ case "$cmd:$sub" in elapsed=0 idle=0 + # Gitea leaves head_branch/branch empty on pull_request-triggered runs, so + # `run list --branch` correlates nothing and the watcher times out into + # no-workflow even when CI ran (#140). Resolve the branch head SHA up front + # (only on Gitea) and fall back to head_sha correlation when the + # branch-filtered lookup is empty. The run object's head_sha equals the + # branch head on Gitea (confirmed in #140); if a variant ever records a + # merge-ref SHA the fallback simply misses and degrades to today's + # behaviour rather than returning a wrong run. + _watch_branch_sha="" + if [[ "$PLATFORM" == "gitea" ]]; then + _watch_branch_sha=$(git rev-parse "$branch" 2>/dev/null \ + || git rev-parse "origin/$branch" 2>/dev/null || true) + fi + + # Echo a run-list JSON array for $branch: branch-name correlation first, + # head_sha correlation as a fallback (Gitea). Platform-safe — on GitHub the + # server-side --branch filter populates the first call and _watch_branch_sha + # stays empty, so the SHA path is never taken. + _runs_for_branch() { + local lim="${1:-1}" out + out=$("$0" run list --branch "$branch" --limit "$lim" 2>/dev/null) || out="[]" + if [[ "$(echo "$out" | jq -r 'length' 2>/dev/null || echo 0)" -gt 0 ]]; then + echo "$out"; return 0 + fi + if [[ -n "$_watch_branch_sha" ]]; then + "$0" run list --limit 50 2>/dev/null | jq \ + --arg s "$_watch_branch_sha" --argjson n "$lim" \ + '[.[] | select(.head_sha != "" and (.head_sha[0:12]) == ($s[0:12]))][0:$n]' \ + 2>/dev/null || echo "[]" + return 0 + fi + echo "[]" + } + # On GitHub, prefer PR-based status checking — one API call gives us both # merge state and all CI check results via statusCheckRollup, avoiding the # flaky run-level polling that produces "unknown" statuses. @@ -1136,7 +1201,7 @@ case "$cmd:$sub" in fi else # Fallback path: check if a completed run already exists - _pre_list=$("$0" run list --branch "$branch" --limit 1 2>/dev/null) || _pre_list="[]" + _pre_list=$(_runs_for_branch 1) || _pre_list="[]" _pre_latest=$(echo "$_pre_list" | jq -r '.[0] // empty') if [[ -n "$_pre_latest" && "$_pre_latest" != "null" ]]; then _pre_status=$(echo "$_pre_latest" | jq -r '.status // "unknown"') @@ -1321,7 +1386,7 @@ case "$cmd:$sub" in # run_url="" while true; do - list_json=$("$0" run list --branch "$branch" --limit 1 2>/dev/null) || list_json="[]" + list_json=$(_runs_for_branch 1) || list_json="[]" latest=$(echo "$list_json" | jq -r '.[0] // empty') if [[ -z "$latest" || "$latest" == "null" ]]; then diff --git a/plugins-claude/session/.claude-plugin/plugin.json b/plugins-claude/session/.claude-plugin/plugin.json index 5c64db7..850b42c 100644 --- a/plugins-claude/session/.claude-plugin/plugin.json +++ b/plugins-claude/session/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "session", - "version": "4.5.0", + "version": "4.5.1", "description": "Work session management — issue-driven and freeform doors sharing an explore-then-plan spine, with multi-agent orchestration and a review-gated PR finalizer", "author": { "name": "Logan Gagne" diff --git a/plugins-claude/session/scripts/git-cli b/plugins-claude/session/scripts/git-cli index 16ff305..679ad34 100755 --- a/plugins-claude/session/scripts/git-cli +++ b/plugins-claude/session/scripts/git-cli @@ -606,15 +606,46 @@ case "$cmd:$sub" in [[ -n "$num" ]] || die_usage "usage: git-tools pr show | --branch NAME" case "$PLATFORM" in github) - cli_json '{number, title, body: (.body // ""), state: (.state | ascii_downcase), merged: ((.state | ascii_downcase) == "merged"), author: .author.login, head: .headRefName, base: .baseRefName, labels: [.labels[].name], assignees: [.assignees[].login], mergeable: (.mergeable | ascii_downcase? // null), created_at: .createdAt, updated_at: .updatedAt, url, comments: [.comments[] | {author: .author.login, body, created_at: .createdAt}]}' \ + cli_json '{number, title, body: (.body // ""), state: (.state | ascii_downcase), merged: ((.state | ascii_downcase) == "merged"), author: .author.login, head: .headRefName, base: .baseRefName, labels: [.labels[].name], assignees: [.assignees[].login], mergeable: (.mergeable | ascii_downcase? // null), created_at: .createdAt, updated_at: .updatedAt, merged_at: .mergedAt, url, comments: [.comments[] | {author: .author.login, body, created_at: .createdAt}]}' \ env GH_NO_COLOR=1 gh pr view "$num" \ - --json "number,title,body,state,author,headRefName,baseRefName,labels,assignees,mergeable,createdAt,updatedAt,url,comments" + --json "number,title,body,state,author,headRefName,baseRefName,labels,assignees,mergeable,createdAt,updatedAt,mergedAt,url,comments" ;; gitea) - # tea has no single-PR view command; get from list filtered by number - cli_json "[.[] | select((.index | tonumber? // .index) == ${num})] | first | {number: (.index | tonumber? // .index), title, body: (.body // \"\"), state, merged: (.merged // false), author: (.author // \"\"), head: (.head // \"\"), base: (.base // \"\"), labels: (if .labels and (.labels | type) == \"array\" then [.labels[].name] else [] end), assignees: (if .assignees and (.assignees | type) == \"array\" then [.assignees[] | .login // empty] else [] end), mergeable: (.mergeable // null), created_at: (.created // \"\"), updated_at: (.updated // null), url: (.url // \"\"), comments: []}" \ - env NO_COLOR=1 tea pr list --output json --state all --limit 200 \ - --fields "index,title,body,state,author,head,base,labels,assignees,mergeable,created,updated,url" + # `tea pr list` omits the `merged` boolean and `merged_at` timestamp + # entirely (tea only encodes merge state into the `state` string) and + # emits `mergeable` as a string, so a merged PR reports + # merged:false / mergedAt:null and consumers think it is still open + # (#140). Fetch the PR detail via the REST API instead, mirroring the + # run:show/run:list `tea api` pattern, so merge fields are reliable. + pr_stderr=$(mktemp); pr_rc=0 + pr_detail=$(NO_COLOR=1 tea api "repos/{owner}/{repo}/pulls/${num}" 2>"$pr_stderr") \ + || pr_rc=$? + if [[ $pr_rc -ne 0 ]]; then + err=$(cat "$pr_stderr"); rm -f "$pr_stderr" + die "tea api repos/{owner}/{repo}/pulls/${num} failed: $err" + fi + rm -f "$pr_stderr" + # Derive `state` from `merged` so it stays consistent with the merge + # flags (Gitea REST reports state:"closed" for a merged PR) and matches + # the GitHub path, which emits state:"merged". + echo "$pr_detail" | jq '{ + number: (.number // .index), + title: (.title // ""), + body: (.body // ""), + state: (if (.merged // false) then "merged" else (.state // "") end), + merged: (.merged // false), + merged_at: (.merged_at // null), + author: (.user.login // .user.username // ""), + head: (.head.ref // .head.label // ""), + base: (.base.ref // .base.label // ""), + labels: (if (.labels | type) == "array" then [.labels[].name] else [] end), + assignees: (if (.assignees | type) == "array" then [.assignees[] | (.login // .username // empty)] else [] end), + mergeable: (.mergeable // null), + created_at: (.created_at // ""), + updated_at: (.updated_at // null), + url: (.html_url // .url // ""), + comments: [] + }' ;; esac ;; @@ -1052,6 +1083,40 @@ case "$cmd:$sub" in elapsed=0 idle=0 + # Gitea leaves head_branch/branch empty on pull_request-triggered runs, so + # `run list --branch` correlates nothing and the watcher times out into + # no-workflow even when CI ran (#140). Resolve the branch head SHA up front + # (only on Gitea) and fall back to head_sha correlation when the + # branch-filtered lookup is empty. The run object's head_sha equals the + # branch head on Gitea (confirmed in #140); if a variant ever records a + # merge-ref SHA the fallback simply misses and degrades to today's + # behaviour rather than returning a wrong run. + _watch_branch_sha="" + if [[ "$PLATFORM" == "gitea" ]]; then + _watch_branch_sha=$(git rev-parse "$branch" 2>/dev/null \ + || git rev-parse "origin/$branch" 2>/dev/null || true) + fi + + # Echo a run-list JSON array for $branch: branch-name correlation first, + # head_sha correlation as a fallback (Gitea). Platform-safe — on GitHub the + # server-side --branch filter populates the first call and _watch_branch_sha + # stays empty, so the SHA path is never taken. + _runs_for_branch() { + local lim="${1:-1}" out + out=$("$0" run list --branch "$branch" --limit "$lim" 2>/dev/null) || out="[]" + if [[ "$(echo "$out" | jq -r 'length' 2>/dev/null || echo 0)" -gt 0 ]]; then + echo "$out"; return 0 + fi + if [[ -n "$_watch_branch_sha" ]]; then + "$0" run list --limit 50 2>/dev/null | jq \ + --arg s "$_watch_branch_sha" --argjson n "$lim" \ + '[.[] | select(.head_sha != "" and (.head_sha[0:12]) == ($s[0:12]))][0:$n]' \ + 2>/dev/null || echo "[]" + return 0 + fi + echo "[]" + } + # On GitHub, prefer PR-based status checking — one API call gives us both # merge state and all CI check results via statusCheckRollup, avoiding the # flaky run-level polling that produces "unknown" statuses. @@ -1136,7 +1201,7 @@ case "$cmd:$sub" in fi else # Fallback path: check if a completed run already exists - _pre_list=$("$0" run list --branch "$branch" --limit 1 2>/dev/null) || _pre_list="[]" + _pre_list=$(_runs_for_branch 1) || _pre_list="[]" _pre_latest=$(echo "$_pre_list" | jq -r '.[0] // empty') if [[ -n "$_pre_latest" && "$_pre_latest" != "null" ]]; then _pre_status=$(echo "$_pre_latest" | jq -r '.status // "unknown"') @@ -1321,7 +1386,7 @@ case "$cmd:$sub" in # run_url="" while true; do - list_json=$("$0" run list --branch "$branch" --limit 1 2>/dev/null) || list_json="[]" + list_json=$(_runs_for_branch 1) || list_json="[]" latest=$(echo "$list_json" | jq -r '.[0] // empty') if [[ -z "$latest" || "$latest" == "null" ]]; then diff --git a/plugins-copilot/git-tools/.claude-plugin/plugin.json b/plugins-copilot/git-tools/.claude-plugin/plugin.json index 518a5e1..50a3461 100644 --- a/plugins-copilot/git-tools/.claude-plugin/plugin.json +++ b/plugins-copilot/git-tools/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "git-tools", - "version": "2.2.1", + "version": "2.2.2", "description": "GitHub and Gitea tooling — unified CLI wrapper (issues, PRs, CI runs) plus a ship orchestrator that drives the full branch/commit/push/PR/watch/cleanup lifecycle", "author": { "name": "Logan Gagne" diff --git a/plugins-copilot/session/.claude-plugin/plugin.json b/plugins-copilot/session/.claude-plugin/plugin.json index 5c64db7..850b42c 100644 --- a/plugins-copilot/session/.claude-plugin/plugin.json +++ b/plugins-copilot/session/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "session", - "version": "4.5.0", + "version": "4.5.1", "description": "Work session management — issue-driven and freeform doors sharing an explore-then-plan spine, with multi-agent orchestration and a review-gated PR finalizer", "author": { "name": "Logan Gagne" diff --git a/tests/git-cli/test-pr-show-gitea.sh b/tests/git-cli/test-pr-show-gitea.sh new file mode 100755 index 0000000..4ea084a --- /dev/null +++ b/tests/git-cli/test-pr-show-gitea.sh @@ -0,0 +1,246 @@ +#!/usr/bin/env bash +# test-pr-show-gitea.sh — Test harness for git-cli `pr show` on the Gitea path. +# Regression test for #140: the old code sourced from `tea pr list`, whose JSON +# omits the `merged` boolean and `merged_at` timestamp and emits `mergeable` as +# a string, so a merged PR reported merged:false / mergedAt:null and consumers +# thought it was still open. The fix fetches the PR detail via +# `tea api repos/{owner}/{repo}/pulls/` and derives the merge fields from it. +# +# Uses mock git/tea scripts via PATH injection. +# +# Usage: bash tests/git-cli/test-pr-show-gitea.sh [filter] + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" + +PASS=0 +FAIL=0 +SKIP=0 +FILTER="${1:-}" + +MOCK_DIR="" +cleanup() { [[ -n "$MOCK_DIR" ]] && rm -rf "$MOCK_DIR"; } +trap cleanup EXIT +MOCK_DIR=$(mktemp -d) + +SENTINEL="$MOCK_DIR/.pr_list_called" + +pass() { + printf " \033[32m✓\033[0m %s\n" "$1" + ((PASS++)) || true +} + +fail() { + printf " \033[31m✗\033[0m %s (%s)\n" "$1" "$2" + ((FAIL++)) || true +} + +skip_filter() { + [[ -n "$FILTER" ]] && ! echo "$1" | grep -qi "$FILTER" +} + +# --------------------------------------------------------------------------- +# Mocks +# --------------------------------------------------------------------------- + +# Mock git: report a Gitea remote so platform detection resolves to "gitea". +cat >"$MOCK_DIR/git" <<'EOF' +#!/usr/bin/env bash +case "$*" in + "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; + *) command git "$@" ;; +esac +EOF +chmod +x "$MOCK_DIR/git" + +# Mock tea: +# - login list → advertise a login for the remote host (so platform == gitea) +# - pr list → MUST NOT be called by the fixed code; record a sentinel +# - api .../pulls/7 → canned merged PR object (Gitea REST shape) +cat >"$MOCK_DIR/tea" <&2 + exit 1 + ;; +esac +EOF +chmod +x "$MOCK_DIR/tea" + +# --------------------------------------------------------------------------- +# Test: pr show emits reliable merge fields for a merged PR +# --------------------------------------------------------------------------- + +echo "── pr show: Gitea REST merge fields (#140) ──" + +label="pr show reports merged:true + merged_at for a merged PR" +if ! skip_filter "$label"; then + exit_code=0 + output=$(PATH="$MOCK_DIR:$PATH" bash "$GIT_CLI" pr show 7 2>"$MOCK_DIR/stderr") || exit_code=$? + stderr=$(cat "$MOCK_DIR/stderr") + + if [[ "$exit_code" != "0" ]]; then + fail "$label" "exit=$exit_code stderr=$stderr" + elif ! echo "$output" | jq -e . >/dev/null 2>&1; then + fail "$label (valid JSON)" "output=$output stderr=$stderr" + else + errs=() + [[ "$(echo "$output" | jq -r '.number')" == "7" ]] || errs+=("number != 7") + [[ "$(echo "$output" | jq -r '.merged')" == "true" ]] || errs+=("merged != true") + [[ "$(echo "$output" | jq -r '.merged | type')" == "boolean" ]] || errs+=("merged not a boolean") + [[ "$(echo "$output" | jq -r '.merged_at')" == "2026-06-01T10:00:00Z" ]] || errs+=("merged_at wrong") + [[ "$(echo "$output" | jq -r '.state')" == "merged" ]] || errs+=("state != merged (should derive from merged)") + [[ "$(echo "$output" | jq -r '.mergeable')" == "true" ]] || errs+=("mergeable != true") + [[ "$(echo "$output" | jq -r '.mergeable | type')" == "boolean" ]] || errs+=("mergeable not a boolean") + [[ "$(echo "$output" | jq -r '.author')" == "stonefish" ]] || errs+=("author != stonefish") + [[ "$(echo "$output" | jq -r '.head')" == "feature-widget" ]] || errs+=("head != feature-widget") + [[ "$(echo "$output" | jq -r '.base')" == "master" ]] || errs+=("base != master") + [[ "$(echo "$output" | jq -r '.labels[0]')" == "enhancement" ]] || errs+=("labels[0] != enhancement") + [[ "$(echo "$output" | jq -r '.assignees[0]')" == "stonefish" ]] || errs+=("assignees[0] != stonefish") + [[ "$(echo "$output" | jq -r '.url')" == "https://git.stonefish.tech/owner/repo/pulls/7" ]] || errs+=("url wrong") + + if [[ ${#errs[@]} -eq 0 ]]; then + pass "$label" + else + fail "$label" "$( + IFS='; ' + echo "${errs[*]}" + ); output=$output" + fi + fi +fi + +# --------------------------------------------------------------------------- +# Test: regression guard — `tea pr list` is never invoked +# --------------------------------------------------------------------------- + +label="pr show does not call 'tea pr list'" +if ! skip_filter "$label"; then + if [[ -f "$SENTINEL" ]]; then + fail "$label" "sentinel present — old list-based path was used" + else + pass "$label" + fi +fi + +# --------------------------------------------------------------------------- +# Test: an open (not merged) PR reports merged:false and keeps its real state +# --------------------------------------------------------------------------- + +label="pr show: open PR reports merged:false, state:open" +if ! skip_filter "$label"; then + cat >"$MOCK_DIR/tea" <&2; exit 1 ;; +esac +EOF + chmod +x "$MOCK_DIR/tea" + + exit_code=0 + output=$(PATH="$MOCK_DIR:$PATH" bash "$GIT_CLI" pr show 7 2>"$MOCK_DIR/stderr") || exit_code=$? + stderr=$(cat "$MOCK_DIR/stderr") + if [[ "$exit_code" != "0" ]]; then + fail "$label" "exit=$exit_code stderr=$stderr" + else + errs=() + [[ "$(echo "$output" | jq -r '.merged')" == "false" ]] || errs+=("merged != false") + [[ "$(echo "$output" | jq -r '.merged_at')" == "null" ]] || errs+=("merged_at != null") + [[ "$(echo "$output" | jq -r '.state')" == "open" ]] || errs+=("state != open") + if [[ ${#errs[@]} -eq 0 ]]; then + pass "$label" + else + fail "$label" "$( + IFS='; ' + echo "${errs[*]}" + ); output=$output" + fi + fi +fi + +# --------------------------------------------------------------------------- +# Test: REST fetch failure → non-zero exit with helpful message +# --------------------------------------------------------------------------- + +label="pr show: REST fetch failure → die" +if ! skip_filter "$label"; then + cat >"$MOCK_DIR/tea" <&2; exit 1 ;; + *) echo "unexpected tea call: \$*" >&2; exit 1 ;; +esac +EOF + chmod +x "$MOCK_DIR/tea" + + exit_code=0 + PATH="$MOCK_DIR:$PATH" bash "$GIT_CLI" pr show 7 >/dev/null 2>"$MOCK_DIR/stderr" || exit_code=$? + stderr=$(cat "$MOCK_DIR/stderr") + if [[ "$exit_code" == "1" ]] && echo "$stderr" | grep -q "pulls/7 failed"; then + pass "$label" + else + fail "$label" "exit=$exit_code stderr=$stderr" + fi +fi + +# --------------------------------------------------------------------------- +# Summary +# --------------------------------------------------------------------------- + +echo "" +echo "Total: $((PASS + FAIL)) PASS: $PASS FAIL: $FAIL SKIP: $SKIP" +[[ "$FAIL" -eq 0 ]] || exit 1 diff --git a/tests/git-cli/test-run-watch-gitea.sh b/tests/git-cli/test-run-watch-gitea.sh new file mode 100755 index 0000000..5c02dfe --- /dev/null +++ b/tests/git-cli/test-run-watch-gitea.sh @@ -0,0 +1,167 @@ +#!/usr/bin/env bash +# test-run-watch-gitea.sh — Test harness for git-cli `run watch` head-SHA +# fallback on the Gitea path. +# +# Regression test for #140: Gitea leaves head_branch/branch empty on +# pull_request-triggered runs, so `run list --branch` correlates nothing and +# `run watch` times out into status:no-workflow even when CI ran and passed. +# The fix resolves the branch head SHA up front and falls back to head_sha +# correlation when the branch-filtered lookup is empty. +# +# The tests exercise the pre-check fallback (terminal-state-before-sleep) so no +# real waiting is needed. Uses mock git/tea via PATH injection. +# +# Usage: bash tests/git-cli/test-run-watch-gitea.sh [filter] + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" + +PASS=0 +FAIL=0 +SKIP=0 +FILTER="${1:-}" + +MOCK_DIR="" +cleanup() { [[ -n "$MOCK_DIR" ]] && rm -rf "$MOCK_DIR"; } +trap cleanup EXIT +MOCK_DIR=$(mktemp -d) + +# The branch head SHA the watcher resolves via `git rev-parse`, and the SHA the +# (empty-branch) Gitea run reports as head_sha. They share a 12-char prefix. +BRANCH_SHA="abcdef1234567890abcdef1234567890abcdef12" +RUN_SHA="abcdef1234567890ffffffffffffffffffffffff" # same first 12 chars + +pass() { + printf " \033[32m✓\033[0m %s\n" "$1" + ((PASS++)) || true +} + +fail() { + printf " \033[31m✗\033[0m %s (%s)\n" "$1" "$2" + ((FAIL++)) || true +} + +skip_filter() { + [[ -n "$FILTER" ]] && ! echo "$1" | grep -qi "$FILTER" +} + +# --------------------------------------------------------------------------- +# Mock builders +# --------------------------------------------------------------------------- + +# Mock git: Gitea remote + a configurable rev-parse SHA for the branch. +write_git_mock() { + local sha="$1" + cat >"$MOCK_DIR/git" < → run detail (success) +# - api .../actions/runs//jobs → all jobs success (so _run_failed_jobs empty) +write_tea_mock() { + cat >"$MOCK_DIR/tea" <&2; exit 1 ;; + esac + ;; + *) echo "unexpected tea call: \$*" >&2; exit 1 ;; +esac +EOF + chmod +x "$MOCK_DIR/tea" +} + +# --------------------------------------------------------------------------- +# Test: empty-branch run is correlated by head_sha → status: pass +# --------------------------------------------------------------------------- + +echo "── run watch: Gitea head-SHA fallback (#140) ──" + +label="run watch correlates empty-branch run via head_sha → pass" +if ! skip_filter "$label"; then + write_git_mock "$BRANCH_SHA" + write_tea_mock + + exit_code=0 + output=$(PATH="$MOCK_DIR:$PATH" bash "$GIT_CLI" run watch --branch feature-widget \ + --initial-delay 0 --interval 0 2>"$MOCK_DIR/stderr") || exit_code=$? + stderr=$(cat "$MOCK_DIR/stderr") + + if [[ "$exit_code" == "0" ]] && echo "$output" | grep -q "^status: pass"; then + pass "$label" + else + fail "$label" "exit=$exit_code output=$output stderr=$stderr" + fi +fi + +# --------------------------------------------------------------------------- +# Test: SHA mismatch → fallback finds nothing → no-workflow (scoped, not blanket) +# --------------------------------------------------------------------------- + +label="run watch: SHA mismatch does not blanket-match → no-workflow" +if ! skip_filter "$label"; then + write_git_mock "0000000000000000000000000000000000000000" # different prefix + write_tea_mock + + exit_code=0 + output=$(PATH="$MOCK_DIR:$PATH" bash "$GIT_CLI" run watch --branch feature-widget \ + --initial-delay 0 --interval 0 2>"$MOCK_DIR/stderr") || exit_code=$? + stderr=$(cat "$MOCK_DIR/stderr") + + if [[ "$exit_code" == "3" ]] && echo "$output" | grep -q "^status: no-workflow"; then + pass "$label" + else + fail "$label" "exit=$exit_code output=$output stderr=$stderr" + fi +fi + +# --------------------------------------------------------------------------- +# Summary +# --------------------------------------------------------------------------- + +echo "" +echo "Total: $((PASS + FAIL)) PASS: $PASS FAIL: $FAIL SKIP: $SKIP" +[[ "$FAIL" -eq 0 ]] || exit 1 diff --git a/utils/git-cli b/utils/git-cli index 16ff305..679ad34 100755 --- a/utils/git-cli +++ b/utils/git-cli @@ -606,15 +606,46 @@ case "$cmd:$sub" in [[ -n "$num" ]] || die_usage "usage: git-tools pr show | --branch NAME" case "$PLATFORM" in github) - cli_json '{number, title, body: (.body // ""), state: (.state | ascii_downcase), merged: ((.state | ascii_downcase) == "merged"), author: .author.login, head: .headRefName, base: .baseRefName, labels: [.labels[].name], assignees: [.assignees[].login], mergeable: (.mergeable | ascii_downcase? // null), created_at: .createdAt, updated_at: .updatedAt, url, comments: [.comments[] | {author: .author.login, body, created_at: .createdAt}]}' \ + cli_json '{number, title, body: (.body // ""), state: (.state | ascii_downcase), merged: ((.state | ascii_downcase) == "merged"), author: .author.login, head: .headRefName, base: .baseRefName, labels: [.labels[].name], assignees: [.assignees[].login], mergeable: (.mergeable | ascii_downcase? // null), created_at: .createdAt, updated_at: .updatedAt, merged_at: .mergedAt, url, comments: [.comments[] | {author: .author.login, body, created_at: .createdAt}]}' \ env GH_NO_COLOR=1 gh pr view "$num" \ - --json "number,title,body,state,author,headRefName,baseRefName,labels,assignees,mergeable,createdAt,updatedAt,url,comments" + --json "number,title,body,state,author,headRefName,baseRefName,labels,assignees,mergeable,createdAt,updatedAt,mergedAt,url,comments" ;; gitea) - # tea has no single-PR view command; get from list filtered by number - cli_json "[.[] | select((.index | tonumber? // .index) == ${num})] | first | {number: (.index | tonumber? // .index), title, body: (.body // \"\"), state, merged: (.merged // false), author: (.author // \"\"), head: (.head // \"\"), base: (.base // \"\"), labels: (if .labels and (.labels | type) == \"array\" then [.labels[].name] else [] end), assignees: (if .assignees and (.assignees | type) == \"array\" then [.assignees[] | .login // empty] else [] end), mergeable: (.mergeable // null), created_at: (.created // \"\"), updated_at: (.updated // null), url: (.url // \"\"), comments: []}" \ - env NO_COLOR=1 tea pr list --output json --state all --limit 200 \ - --fields "index,title,body,state,author,head,base,labels,assignees,mergeable,created,updated,url" + # `tea pr list` omits the `merged` boolean and `merged_at` timestamp + # entirely (tea only encodes merge state into the `state` string) and + # emits `mergeable` as a string, so a merged PR reports + # merged:false / mergedAt:null and consumers think it is still open + # (#140). Fetch the PR detail via the REST API instead, mirroring the + # run:show/run:list `tea api` pattern, so merge fields are reliable. + pr_stderr=$(mktemp); pr_rc=0 + pr_detail=$(NO_COLOR=1 tea api "repos/{owner}/{repo}/pulls/${num}" 2>"$pr_stderr") \ + || pr_rc=$? + if [[ $pr_rc -ne 0 ]]; then + err=$(cat "$pr_stderr"); rm -f "$pr_stderr" + die "tea api repos/{owner}/{repo}/pulls/${num} failed: $err" + fi + rm -f "$pr_stderr" + # Derive `state` from `merged` so it stays consistent with the merge + # flags (Gitea REST reports state:"closed" for a merged PR) and matches + # the GitHub path, which emits state:"merged". + echo "$pr_detail" | jq '{ + number: (.number // .index), + title: (.title // ""), + body: (.body // ""), + state: (if (.merged // false) then "merged" else (.state // "") end), + merged: (.merged // false), + merged_at: (.merged_at // null), + author: (.user.login // .user.username // ""), + head: (.head.ref // .head.label // ""), + base: (.base.ref // .base.label // ""), + labels: (if (.labels | type) == "array" then [.labels[].name] else [] end), + assignees: (if (.assignees | type) == "array" then [.assignees[] | (.login // .username // empty)] else [] end), + mergeable: (.mergeable // null), + created_at: (.created_at // ""), + updated_at: (.updated_at // null), + url: (.html_url // .url // ""), + comments: [] + }' ;; esac ;; @@ -1052,6 +1083,40 @@ case "$cmd:$sub" in elapsed=0 idle=0 + # Gitea leaves head_branch/branch empty on pull_request-triggered runs, so + # `run list --branch` correlates nothing and the watcher times out into + # no-workflow even when CI ran (#140). Resolve the branch head SHA up front + # (only on Gitea) and fall back to head_sha correlation when the + # branch-filtered lookup is empty. The run object's head_sha equals the + # branch head on Gitea (confirmed in #140); if a variant ever records a + # merge-ref SHA the fallback simply misses and degrades to today's + # behaviour rather than returning a wrong run. + _watch_branch_sha="" + if [[ "$PLATFORM" == "gitea" ]]; then + _watch_branch_sha=$(git rev-parse "$branch" 2>/dev/null \ + || git rev-parse "origin/$branch" 2>/dev/null || true) + fi + + # Echo a run-list JSON array for $branch: branch-name correlation first, + # head_sha correlation as a fallback (Gitea). Platform-safe — on GitHub the + # server-side --branch filter populates the first call and _watch_branch_sha + # stays empty, so the SHA path is never taken. + _runs_for_branch() { + local lim="${1:-1}" out + out=$("$0" run list --branch "$branch" --limit "$lim" 2>/dev/null) || out="[]" + if [[ "$(echo "$out" | jq -r 'length' 2>/dev/null || echo 0)" -gt 0 ]]; then + echo "$out"; return 0 + fi + if [[ -n "$_watch_branch_sha" ]]; then + "$0" run list --limit 50 2>/dev/null | jq \ + --arg s "$_watch_branch_sha" --argjson n "$lim" \ + '[.[] | select(.head_sha != "" and (.head_sha[0:12]) == ($s[0:12]))][0:$n]' \ + 2>/dev/null || echo "[]" + return 0 + fi + echo "[]" + } + # On GitHub, prefer PR-based status checking — one API call gives us both # merge state and all CI check results via statusCheckRollup, avoiding the # flaky run-level polling that produces "unknown" statuses. @@ -1136,7 +1201,7 @@ case "$cmd:$sub" in fi else # Fallback path: check if a completed run already exists - _pre_list=$("$0" run list --branch "$branch" --limit 1 2>/dev/null) || _pre_list="[]" + _pre_list=$(_runs_for_branch 1) || _pre_list="[]" _pre_latest=$(echo "$_pre_list" | jq -r '.[0] // empty') if [[ -n "$_pre_latest" && "$_pre_latest" != "null" ]]; then _pre_status=$(echo "$_pre_latest" | jq -r '.status // "unknown"') @@ -1321,7 +1386,7 @@ case "$cmd:$sub" in # run_url="" while true; do - list_json=$("$0" run list --branch "$branch" --limit 1 2>/dev/null) || list_json="[]" + list_json=$(_runs_for_branch 1) || list_json="[]" latest=$(echo "$list_json" | jq -r '.[0] // empty') if [[ -z "$latest" || "$latest" == "null" ]]; then From 32ad8b195d4688d0ccb976888c2c056da082a502 Mon Sep 17 00:00:00 2001 From: Logan Gagne Date: Sat, 29 Aug 2026 17:12:26 -0400 Subject: [PATCH 2/2] fix(tests): stop git mocks fork-bombing on unmatched subcommands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every test that PATH-injects a fake `git` fell through unmatched subcommands to `command git "$@"`. `command` bypasses functions and aliases but NOT PATH lookup, and the mock's own directory is prepended to PATH — so the mock re-executed itself, recursing without bound. This was latent for as long as the mocks have existed: nothing ever called git with a subcommand other than `remote get-url origin`. The `run watch` head-SHA fix in the preceding commit calls `git rev-parse`, which made every affected suite spawn processes until the terminal's cgroup pids controller started rejecting forks. Observed at 56,311 processes in one scope against a pids.max of 114,647, which takes down the whole terminal, not just the test run. Delegate to the real git by stripping the mock's own directory from PATH first, so `exec git` resolves past it: *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; - Fix all 11 fallbacks across 9 suites - `exec` rather than a nested call, so the mock does not linger as a parent - Verified: matched cases still return mock values; unmatched delegate to real git and terminate; `git rev-parse origin/` now returns git's own error instead of recursing tests/test.sh: 32 suites, 0 failed — previously it hung indefinitely. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EddJxaTMXyj7o4ndrqfCxa --- tests/git-cli/test-body-args.sh | 2 +- tests/git-cli/test-issue-write-json.sh | 4 ++-- tests/git-cli/test-pr-create.sh | 2 +- tests/git-cli/test-pr-show-gitea.sh | 2 +- tests/git-cli/test-run-show.sh | 2 +- tests/git-cli/test-run-watch-gitea.sh | 2 +- tests/session/test-ci-poll.sh | 6 +++--- tests/session/test-pr-auto-merge-status.sh | 2 +- tests/session/test-pr-wait.sh | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/git-cli/test-body-args.sh b/tests/git-cli/test-body-args.sh index 86cc73a..66e3784 100644 --- a/tests/git-cli/test-body-args.sh +++ b/tests/git-cli/test-body-args.sh @@ -48,7 +48,7 @@ cat >"$MOCK_DIR/git" <<'EOF' case "$*" in "remote get-url origin") echo "https://github.com/owner/repo.git" ;; "config user.name") echo "testuser" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/git-cli/test-issue-write-json.sh b/tests/git-cli/test-issue-write-json.sh index 37a89d8..c81cef9 100644 --- a/tests/git-cli/test-issue-write-json.sh +++ b/tests/git-cli/test-issue-write-json.sh @@ -107,7 +107,7 @@ set_platform_github() { #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://github.com/owner/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" @@ -126,7 +126,7 @@ set_platform_gitea() { #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/git-cli/test-pr-create.sh b/tests/git-cli/test-pr-create.sh index f4619fa..874f4f7 100644 --- a/tests/git-cli/test-pr-create.sh +++ b/tests/git-cli/test-pr-create.sh @@ -75,7 +75,7 @@ cat >"$MOCK_DIR/git" <<'EOF' case "$*" in "remote get-url origin") echo "https://github.com/owner/repo.git" ;; "config user.name") echo "testuser" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/git-cli/test-pr-show-gitea.sh b/tests/git-cli/test-pr-show-gitea.sh index 4ea084a..8b03a71 100755 --- a/tests/git-cli/test-pr-show-gitea.sh +++ b/tests/git-cli/test-pr-show-gitea.sh @@ -50,7 +50,7 @@ cat >"$MOCK_DIR/git" <<'EOF' #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/git-cli/test-run-show.sh b/tests/git-cli/test-run-show.sh index 8ea3d3d..50921a6 100755 --- a/tests/git-cli/test-run-show.sh +++ b/tests/git-cli/test-run-show.sh @@ -49,7 +49,7 @@ cat >"$MOCK_DIR/git" <<'EOF' #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/git-cli/test-run-watch-gitea.sh b/tests/git-cli/test-run-watch-gitea.sh index 5c02dfe..92d5c0d 100755 --- a/tests/git-cli/test-run-watch-gitea.sh +++ b/tests/git-cli/test-run-watch-gitea.sh @@ -59,7 +59,7 @@ write_git_mock() { case "\$*" in "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; "rev-parse feature-widget"|"rev-parse origin/feature-widget") echo "$sha" ;; - *) command git "\$@" ;; + *) PATH=\${PATH#"\${0%/*}":}; exec git "\$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/session/test-ci-poll.sh b/tests/session/test-ci-poll.sh index eca3eca..b129e1d 100644 --- a/tests/session/test-ci-poll.sh +++ b/tests/session/test-ci-poll.sh @@ -63,7 +63,7 @@ cat >"$MOCK_DIR/git" <<'EOF' #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" @@ -619,7 +619,7 @@ cat >"$MOCK_DIR/git" <<'EOF' #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://gitea.example.com/owner/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" @@ -1046,7 +1046,7 @@ cat >"$MOCK_DIR/git" <<'EOF' #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/session/test-pr-auto-merge-status.sh b/tests/session/test-pr-auto-merge-status.sh index f0565d6..3898228 100644 --- a/tests/session/test-pr-auto-merge-status.sh +++ b/tests/session/test-pr-auto-merge-status.sh @@ -65,7 +65,7 @@ cat >"$MOCK_DIR/git" <<'EOF' #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git" diff --git a/tests/session/test-pr-wait.sh b/tests/session/test-pr-wait.sh index 117e3d4..b62912f 100644 --- a/tests/session/test-pr-wait.sh +++ b/tests/session/test-pr-wait.sh @@ -63,7 +63,7 @@ cat >"$MOCK_DIR/git" <<'EOF' #!/usr/bin/env bash case "$*" in "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) command git "$@" ;; + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; esac EOF chmod +x "$MOCK_DIR/git"