fix(source-control): propagate fetch-all-pr-comments jq merge status

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",

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

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
+}

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" <<STUB
+#!/usr/bin/env bash
+for arg in "\$@"; do
+  if [[ "\$arg" == "-s" ]]; then
+    printf 'jq: merge stub failure\n' >&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" <<STUB
+#!/usr/bin/env bash
+case "\$*" in
+  *"repo view"*owner*) printf 'testowner' ;;
+  *"repo view"*name*) printf 'testrepo' ;;
+  *issues/${PR_NUM}/comments*) cat "$FIXTURE_GENERAL" ;;
+  *) printf '[]' ;;
+esac
+STUB
+chmod +x "$PARTIAL_STUB"
+
+out=$(PATH="$TEST_TMPDIR/bin-partial:$PATH" bash "$SCRIPT" "$PR_NUM" 2>/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