diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 37acfab9dd..dfc394c48a 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.55.36", + "version": "0.55.37", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-authored-by trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop \u2014 safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and standing merge-rung raises binding from the team-tracked layer only \u2014 with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply \u2014 interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep \u2014 never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index c33461dee6..5c6a054357 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,21 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.55.37] + +### Fixed + +- **`fetch-all-pr-comments.sh` no longer reports a failed final jq merge as success.** The + script ended with `exit 0` after `{ surfaces } | jq -s 'sort_by(.created_at)'`, so a parse + error, truncated payload, or other jq failure still looked like a complete fetch. Callers + (the babysit-PRs path included) read that status to decide whether they have the full + comment set. The merge pipeline's status now propagates, mapped to exit 2 to match the + per-surface jq failures and to avoid colliding with jq's native parse-error 5 (already + documented here as "prerequisite missing"). Empty surfaces stay success: the three + `[[ -n ]] && printf` arms would return 1 when empty, and with `set -o pipefail` that + would false-fail a legitimate subset, so they are now `if` statements (`if` returns 0 + when no condition tested true). + ## [0.55.36] ### Changed diff --git a/plugins/source-control/scripts/fetch-all-pr-comments.sh b/plugins/source-control/scripts/fetch-all-pr-comments.sh index 5d595d78ad..e9cd316922 100755 --- a/plugins/source-control/scripts/fetch-all-pr-comments.sh +++ b/plugins/source-control/scripts/fetch-all-pr-comments.sh @@ -41,7 +41,7 @@ # Exit codes: # 0 success (zero or more comments emitted) # 1 invalid argument -# 2 gh api call failed +# 2 gh api call failed, or jq failed parsing a surface or merging # 5 prerequisite missing (gh, jq) set -uo pipefail # -e omitted: gh api failures explicitly guarded with || { exit N } @@ -205,11 +205,17 @@ fetch_surface "pulls/$PR_NUMBER/comments" ' INLINE="$SURFACE_JSON" # --- Merge and sort by created_at -------------------------------------------- - +# Empty surfaces are a legitimate PR (no comments on that surface). `[[ -n ]] && +# printf` returns 1 when empty, and with `set -o pipefail` that fails the merge +# pipeline even when jq succeeded. `if` returns 0 when no condition tested true +# (bash(1) Compound Commands), so an empty source is not a pipeline failure. +# jq merge failure is: map it to exit 2 rather than jq's native parse-error 5, +# which this script already documents as "prerequisite missing (gh, jq)". { - [[ -n "$GENERAL" ]] && printf '%s\n' "$GENERAL" - [[ -n "$REVIEWS" ]] && printf '%s\n' "$REVIEWS" - [[ -n "$INLINE" ]] && printf '%s\n' "$INLINE" -} | jq -s 'sort_by(.created_at)' - -exit 0 + if [[ -n "$GENERAL" ]]; then printf '%s\n' "$GENERAL"; fi + if [[ -n "$REVIEWS" ]]; then printf '%s\n' "$REVIEWS"; fi + if [[ -n "$INLINE" ]]; then printf '%s\n' "$INLINE"; fi +} | jq -s 'sort_by(.created_at)' || { + printf 'fetch-all-pr-comments: jq merge failed\n' >&2 + exit 2 +} diff --git a/plugins/source-control/scripts/fetch-all-pr-comments.test.sh b/plugins/source-control/scripts/fetch-all-pr-comments.test.sh index d29afe6ecf..85bccb8eff 100755 --- a/plugins/source-control/scripts/fetch-all-pr-comments.test.sh +++ b/plugins/source-control/scripts/fetch-all-pr-comments.test.sh @@ -10,6 +10,8 @@ # 4. gh api failure — exits 2 # 5. Output schema — every object has required fields # 6. Empty PR (no comments) — exits 0 with empty array +# 13. Final jq merge failure — exits 2 (not masked by a trailing exit 0) +# 14. Successful merge of a subset (INLINE empty) — still exits 0 set -uo pipefail @@ -261,6 +263,56 @@ assert_contains "--help names the UnicodeDecodeError failure mode" "$help_out" " assert_contains "--help reaches the end of the header (Exit codes block)" \ "$help_out" "prerequisite missing (gh, jq)" +# Case 13 (#3483): a failed final jq merge must exit non-zero. The three surface +# projections still run against real jq; only `jq -s` (the merge) is stubbed to +# fail, so this cannot be satisfied by a surface-parse exit 2. +REAL_JQ=$(command -v jq) +JQ_MERGE_FAIL="$TEST_TMPDIR/bin-jq-merge-fail" +mkdir -p "$JQ_MERGE_FAIL" +cat >"$JQ_MERGE_FAIL/jq" <&2 + exit 5 + fi +done +exec '$REAL_JQ' "\$@" +STUB +chmod +x "$JQ_MERGE_FAIL/jq" + +rc=$( + PATH="$JQ_MERGE_FAIL:$TEST_TMPDIR/bin:$PATH" bash "$SCRIPT" "$PR_NUM" >/dev/null 2>&1 + echo $? +) +assert_eq "jq merge failure exits 2" "2" "$rc" +err=$(PATH="$JQ_MERGE_FAIL:$TEST_TMPDIR/bin:$PATH" bash "$SCRIPT" "$PR_NUM" 2>&1 >/dev/null) +assert_contains "jq merge failure names the merge" "$err" "jq merge failed" + +# Case 14 (#3483): INLINE (the last source) empty is a legitimate subset. The +# old `[[ -n ]] && printf` arm returned 1 for that empty source; under pipefail +# that would fail the pipeline even when jq succeeded. Must still exit 0. +PARTIAL_STUB="$TEST_TMPDIR/bin-partial/gh" +mkdir -p "$TEST_TMPDIR/bin-partial" +cat >"$PARTIAL_STUB" </dev/null) +rc=$? +assert_eq "subset merge (INLINE empty) exits 0" "0" "$rc" +partial_count=$(printf '%s' "$out" | jq 'length') +assert_eq "subset merge emits the general comment" "1" "$partial_count" +partial_type=$(printf '%s' "$out" | jq -r '.[0].type') +assert_eq "subset merge type is general" "general" "$partial_type" + # ---- Summary ---------------------------------------------------------------- if [[ "$FAILED" -eq 0 ]]; then