fix(source-control): gate push -u on absent-upstream + resolved-remote match (#442)

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.15.4",
+  "version": "0.15.5",
   "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 — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /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 — 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 — 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,41 @@
 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.15.5]
+
+### Fixed
+
+- **`/pull-request` create flow no longer silently corrupts a branch's fetch/rebase upstream when
+  publishing it for a PR (#442, residual finding from PR #763).** A post-merge review empirically
+  reproduced a residual silent clobber: §2.4.1's conditional `-u` gate keyed on the LITERAL
+  `branch.<name>.remote` config being set. In the triangular shape where `remote.pushDefault` names a
+  fork globally but `branch.<name>.remote` is unset (so fetch/rebase falls back to `origin`), the gate
+  read "unset", took the `-u` bootstrap path, and `git push -u <fork>` rewrote `branch.<name>.remote`
+  to the fork — so the next fetch/rebase silently targeted the fork instead of `origin`. The gate now
+  fires `-u` only when the branch has NO existing upstream (`branch.<name>.remote` AND
+  `branch.<name>.merge` both literally unset) AND its fetch and push remotes resolve to the same name
+  (`resolve-remote.sh` fetch-mode vs `--push`); otherwise it pushes plain and writes no branch config.
+  This closes the reported `pushDefault`-only clobber (fetch resolves `origin`, push resolves the fork →
+  they differ → plain push, upstream untouched) and a broader corruption family the fix surfaced:
+  `git push -u` rewrites the branch's WHOLE upstream — both `branch.<name>.remote` and
+  `branch.<name>.merge` — so a branch with any configured tracking kept its merge ref overwritten under
+  a resolved-name-only comparison. Three such shapes: an already-tracked branch; a deliberate local-only
+  `.` upstream (`git branch --track . <ref>`); and merge-only tracking (`branch.<name>.merge` set with
+  `branch.<name>.remote` unset — valid, since Git defaults the remote to `origin`, so the branch tracks
+  `origin/<merge-ref>`). Requiring BOTH upstream keys to be absent before bootstrapping preserves any
+  existing tracking via plain push. This also changes #763's behavior for the `.` case (it took the
+  `-u` path); publishing a branch for a PR no longer mutates a deliberate local-only or merge-only
+  upstream — a strict improvement. An ambiguous fetch resolution (empty) is unequal to any push remote →
+  plain push, never an abort. The conditional moved out of the `create.md` prose into a new co-located
+  `scripts/push-branch.sh` (§2.4.1 now delegates to it), so the gate sequence is executable and testable
+  rather than living only in markdown; the normalized `.`-as-unset / `\r`-strip handling stays solely in
+  `resolve-remote.sh` and is not duplicated (the upstream-absent probe reads both keys raw — any
+  non-empty value means "has an upstream"). New `push-branch.test.sh` drives the full resolve-fetch →
+  resolve-push → conditional-push → re-resolve-fetch sequence against real bare remotes across the
+  pushRemote-triangular, `pushDefault`-only triangular, non-triangular (asserting the merge ref is
+  preserved), fresh-branch bootstrap, local-only `.`, merge-only tracking, and fetch-ambiguous shapes —
+  the integration coverage whose absence let this escape `resolve-remote.test.sh`'s resolver-only cases.
+
 ## [0.15.4]
 
 ### Fixed

plugins/source-control/skills/pull-request/reference/create.md

@@ -172,36 +172,19 @@ Persist chosen line(s) into `${CLOSES_LINE}`. NEVER wrap a closing keyword in an
 ### 2.4.1 Push and assemble PR body
 
 ```bash
-# Push via the shared resolver in --push mode, which applies Git's documented
-# push precedence: branch.<name>.pushRemote, else remote.pushDefault, else the
-# §2.2 fetch order (branch.<name>.remote, else `origin`, else the sole other
-# configured remote). This is why the push resolves separately from §2.2's
-# fetch remote — Git lets the push destination differ, so a triangular fork
-# flow that fetches from `upstream` but sets pushRemote/pushDefault to the fork
-# pushes to the fork, not `upstream`. A fork may be named `origin`, `fork`, or
-# anything else, and a repo cloned with a non-origin sole remote (`git clone -o
-# vendor`) must push there too, not `origin`. Two or more non-origin candidates
-# with no `origin` set fails loudly (see §2.2) rather than pushing to an
-# arbitrary remote.
-#
-# `-u` is CONDITIONAL: `git push -u` rewrites `branch.<name>.remote` to the
-# push target, so on a triangular fork (fetch `upstream`, push a fork via
-# pushRemote/pushDefault) an unconditional `-u` would silently repoint the
-# FETCH remote §2.2 reads to the fork — breaking the next rebase. So bootstrap
-# tracking with `-u` only when the branch has no real `branch.<name>.remote`
-# yet (a fresh feature branch, or a local-only `.` upstream): the common first
-# push still sets upstream to `origin` exactly as before. When a real fetch
-# remote is already configured, push WITHOUT `-u` to preserve it. Either way
-# later pushes need no remote argument (tracking was already set, or `-u` just
-# set it). Match resolve-remote.sh's `\r` strip and `.`-as-unset convention.
-PUSH_REMOTE=$(bash "${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/resolve-remote.sh" --push) || exit 1
-BRANCH_NAME=$(git branch --show-current)
-EXISTING_FETCH_REMOTE=$(git config "branch.${BRANCH_NAME}.remote" 2>/dev/null | tr -d '\r')
-if [[ -n "$EXISTING_FETCH_REMOTE" && "$EXISTING_FETCH_REMOTE" != "." ]]; then
-  git push "$PUSH_REMOTE" "$BRANCH_NAME"
-else
-  git push -u "$PUSH_REMOTE" "$BRANCH_NAME"
-fi
+# Push via push-branch.sh, which resolves the push and fetch/rebase remotes
+# independently (resolve-remote.sh --push vs plain) and sets upstream (`-u`)
+# ONLY for a branch with NO existing upstream — branch.<name>.remote AND
+# branch.<name>.merge both unset — whose fetch and push resolve to the same
+# remote (a fresh feature branch's first push). `git push -u` rewrites the
+# branch's whole upstream — both keys — so any existing upstream (a real remote,
+# a deliberate local-only `.`, or a merge ref set with the remote defaulting to
+# `origin`) is preserved by a plain push instead. This closes two silent
+# corruptions: a triangular fork (push a fork via pushRemote/pushDefault, fetch
+# `origin`/`upstream`) no longer repoints the fetch remote to the fork, and a
+# branch with any configured tracking no longer has its merge ref overwritten.
+# See the script header for the full rationale.
+bash "${CLAUDE_PLUGIN_ROOT}/skills/pull-request/scripts/push-branch.sh" || exit 1
 ```
 
 Derive PR title from the commit subject, shaped to satisfy the resolved subject/title convention (the ladder in [SKILL.md](../SKILL.md): layered `source-control.md` config → project convention → Conventional Commits default). Build body with `${CLOSES_LINE}` at top, followed by Summary + Test plan + a `## Related` section + a config-gated attribution line:

plugins/source-control/skills/pull-request/scripts/push-branch.sh

@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+# Push the current (or named) branch to its resolved push remote, setting
+# upstream tracking (`-u`) ONLY for a branch that has no existing upstream and
+# whose fetch and push remotes resolve to the same name.
+#
+# Push destination and fetch/rebase remote are resolved independently by the
+# sibling resolve-remote.sh (Git lets them differ in a triangular fork flow):
+#   PUSH_REMOTE  = resolve-remote.sh --push   (pushRemote -> pushDefault -> fetch order)
+#   FETCH_REMOTE = resolve-remote.sh          (branch.<name>.remote -> origin -> sole other)
+#
+# `-u` is CONDITIONAL because `git push -u` rewrites the branch's ENTIRE upstream
+# — both branch.<name>.remote AND branch.<name>.merge (to refs/heads/<name>). Two
+# distinct ways that silently corrupts where the branch fetches/rebases from:
+#
+#   1. Retarget the remote. On a triangular fork (push to a fork via
+#      pushRemote/pushDefault, fetch from origin/upstream) `-u` would repoint
+#      branch.<name>.remote to the fork, so the next rebase targets the wrong
+#      base. The trap is the shape where the fetch remote is NOT literally
+#      configured but resolves via fallback: remote.pushDefault=fork with
+#      branch.<name>.remote unset fetches from `origin` (fallback) yet pushes to
+#      the fork.
+#   2. Overwrite an existing upstream. The upstream is TWO keys —
+#      branch.<name>.remote AND branch.<name>.merge — and `-u` rewrites both.
+#      Any of these partial or full configs is an upstream the user chose that a
+#      bootstrap must not clobber:
+#        - both set to a real remote/ref (an ordinary tracked branch);
+#        - branch.<name>.remote="." (a deliberate local-only upstream, e.g.
+#          `git branch --track . <ref>`), merge naming the tracked local ref;
+#        - branch.<name>.merge set with branch.<name>.remote UNSET — valid: Git
+#          defaults the missing remote to `origin`, so the branch tracks
+#          `origin/<merge-ref>`. `-u` here replaces the chosen merge ref with
+#          refs/heads/<name>, the same clobber family as (1).
+#
+# The gate therefore fires `-u` only when BOTH hold:
+#   * the branch has NO existing upstream — branch.<name>.remote AND
+#     branch.<name>.merge are BOTH literally unset (a genuinely fresh feature
+#     branch, nothing to preserve), AND
+#   * FETCH_REMOTE == PUSH_REMOTE (bootstrapping tracking cannot misdirect the
+#     fetch remote, because it can only be set to what fetch already resolves to).
+# Any existing upstream — full, ".", or merge-only — is preserved by a plain
+# push, which never writes branch config. An ambiguous fetch (FETCH_REMOTE empty:
+# 2+ non-origin remotes, no origin) is unequal to any push remote -> plain push
+# -> no write.
+#
+# The upstream-absent check reads both keys raw (no "." or CRLF normalization):
+# any non-empty value -> "has an upstream" -> plain push preserves it; only the
+# truly-unset (empty) keys are a bootstrap candidate. That normalized "."
+# handling lives once, in resolve-remote.sh, and is not duplicated here.
+#
+# Push resolution must be determinate (it names the destination), so a failed
+# --push resolution aborts; a failed fetch resolution does not.
+#
+# Usage: push-branch.sh [branch-name]   (defaults to the current branch)
+set -uo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+RESOLVER="${SCRIPT_DIR}/resolve-remote.sh"
+
+BRANCH="${1:-}"
+if [[ -z "$BRANCH" ]]; then
+  BRANCH="$(git branch --show-current 2>/dev/null | tr -d '\r')"
+fi
+
+PUSH_REMOTE=$(bash "$RESOLVER" --push "$BRANCH") || exit 1
+FETCH_REMOTE=$(bash "$RESOLVER" "$BRANCH" 2>/dev/null)
+EXISTING_REMOTE=$(git config "branch.${BRANCH}.remote" 2>/dev/null)
+EXISTING_MERGE=$(git config "branch.${BRANCH}.merge" 2>/dev/null)
+
+if [[ -z "$EXISTING_REMOTE" && -z "$EXISTING_MERGE" && "$FETCH_REMOTE" == "$PUSH_REMOTE" ]]; then
+  git push -u "$PUSH_REMOTE" "$BRANCH"
+else
+  git push "$PUSH_REMOTE" "$BRANCH"
+fi