From 5bdf05b43868ff7ca41da3bad78fa53c46ca92d4 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:44:49 -0400 Subject: [PATCH 1/5] perf(source-control): merge a stale base before push in push-branch.sh A push that waits in the rate gate while main moves is refused by the pre-push stale-base overlap check, costing a manual merge, re-test and re-queue each time. push-branch.sh now fetches the PR's base just before the push and, when the branch is behind it on overlapping paths (the repo's scripts/check-stale-base-overlap.sh when present, else git path lists), merges it with --no-edit, signing per the repo. A conflict aborts the merge and exits 4 without pushing. Off by default; on through sync_stale_base in docs/conventions/source-control.yaml (on for this repository) or --sync-stale-base, off through --no-sync-stale-base. Co-Authored-By: Claude Opus 5.5 --- ...rf-push-branch-sync-stale-base-3f9a1c62.md | 7 + docs/conventions/source-control.yaml | 4 + .../reference/config-resolution.md | 10 + .../schemas/source-control.schema.json | 5 + .../skills/pull-request/reference/create.md | 7 + .../reference/ready-for-review.md | 5 +- .../pull-request/scripts/push-branch.sh | 169 ++++++++++++-- .../pull-request/scripts/push-branch.test.sh | 213 +++++++++++++++++- 8 files changed, 396 insertions(+), 24 deletions(-) create mode 100644 .changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md diff --git a/.changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md b/.changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md new file mode 100644 index 0000000000..ae4488a18b --- /dev/null +++ b/.changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md @@ -0,0 +1,7 @@ +--- +bump: minor +--- + +### Added + +- `push-branch.sh` can merge the pull request's base into the branch just before it pushes, when the branch is behind the base and both changed overlapping paths since the merge-base. A push that waited in a rate gate while the base moved is then not refused by a pre-push stale-base check. The overlap rule is the checkout's `scripts/check-stale-base-overlap.sh --check /` when the file exists, else the paths both sides changed. The merge is `git merge --no-edit`, never a rebase. It is signed when `commit.gpgsign` is true or HEAD is signed. A conflict aborts the merge, pushes nothing and exits 4. The step is off by default. Turn it on with `sync_stale_base: true` in `docs/conventions/source-control.yaml`, or `--sync-stale-base` for one push; `--no-sync-stale-base` turns it off for one push. A lane push never runs it. The `create` and `ready` references describe the new step and exit code. diff --git a/docs/conventions/source-control.yaml b/docs/conventions/source-control.yaml index cd2795c24e..4b30741dcf 100644 --- a/docs/conventions/source-control.yaml +++ b/docs/conventions/source-control.yaml @@ -18,6 +18,10 @@ # grace window. Landing them here is the lane's tracked adoption. .claude/source-control.md keeps # the same four values until the cleanup slice deletes it; change both while it exists. pr_open_state: ready +# sync_stale_base: push-branch.sh merges the base into a branch behind it on paths +# scripts/check-stale-base-overlap.sh calls overlapping, just before the push, so a push that +# waited in the rate gate while main moved is not refused by the pre-push stale-base check. +sync_stale_base: true pr_body_required_sections: - Summary - Fix diff --git a/plugins/source-control/reference/config-resolution.md b/plugins/source-control/reference/config-resolution.md index c9667a23bd..ee61c08cfc 100644 --- a/plugins/source-control/reference/config-resolution.md +++ b/plugins/source-control/reference/config-resolution.md @@ -218,6 +218,16 @@ reads the file itself, through a script where the key names one below. `userConfig` option declares `ready` as its default, so a substituted `ready` is reported as the default level (the two give the same result) and a substituted `draft` as the `userConfig` level. +- `sync_stale_base`: `true` or `false`, whether + [`push-branch.sh`](../skills/pull-request/scripts/push-branch.sh) merges the pull request's + base into a branch behind it on overlapping paths just before it pushes (the script header + holds the base, overlap, signing and skip rules, and exit 4 for a conflict). Resolution, lowest + first: the default, `false`; `sync_stale_base` in the checkout's tracked + `docs/conventions/source-control.yaml`; `--sync-stale-base` or `--no-sync-stale-base` on the + call. No `userConfig` option. The script reads the file from the checkout it pushes, not the + base branch: either value leaves the push going to the same place, and `true` only adds a merge + of the base. A file the parser rejects, or another value, reads as `false`. A lane push never + syncs. - `pre_ready`: which pre-flip steps `/source-control:pull-request ready` runs, a list of `{step, enabled}` mappings over `base-merge`, `plan-evidence`, `code-review`, `security-review` and `verify`; a step the list leaves out runs. Team level only, with no `userConfig` option: a diff --git a/plugins/source-control/schemas/source-control.schema.json b/plugins/source-control/schemas/source-control.schema.json index c10cb86cf9..f0d88fec84 100644 --- a/plugins/source-control/schemas/source-control.schema.json +++ b/plugins/source-control/schemas/source-control.schema.json @@ -12,6 +12,11 @@ "default": "ready", "description": "State /source-control:pull-request create leaves a pull request in. ready runs /source-control:pull-request ready on the new pull request, which flips it after its gates; draft leaves it a draft for that action to flip later." }, + "sync_stale_base": { + "type": "boolean", + "default": false, + "description": "When true, push-branch.sh merges the pull request's base into a branch that is behind it on overlapping paths just before it pushes, using the repository's scripts/check-stale-base-overlap.sh as the overlap rule when the checkout has one. A merge, never a rebase; a conflict aborts it and pushes nothing (exit 4). --sync-stale-base and --no-sync-stale-base override it for one push." + }, "subject_pattern": { "type": "string", "minLength": 1, diff --git a/plugins/source-control/skills/pull-request/reference/create.md b/plugins/source-control/skills/pull-request/reference/create.md index 50d52d882b..69a662f809 100644 --- a/plugins/source-control/skills/pull-request/reference/create.md +++ b/plugins/source-control/skills/pull-request/reference/create.md @@ -226,6 +226,13 @@ Persist chosen line(s) into `${CLOSES_LINE}`. NEVER wrap a closing keyword in an bash "/scripts/push-branch.sh" --dir "" || exit 1 ``` +With `sync_stale_base` on ([config-resolution.md](../../../reference/config-resolution.md#yaml-keys-docsconventionssource-controlyaml)), +`push-branch.sh` first merges the base into a branch that is behind it on overlapping paths, so a +push that waited in a rate gate while the base moved is not refused by a pre-push stale-base +check; it reports the merge on stderr. Exit 4 means that merge conflicted and was aborted and +nothing was pushed: resolve it with `/source-control:resolve-conflicts` (merge the base by hand), +then push again. + 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 config → project convention → Conventional Commits default). Build body with `${CLOSES_LINE}` at top, followed by the resolved section scaffold and a config-gated attribution line. **Resolve the required section scaffold first.** Run `bash "/lib/config-root.sh" classify` before reading any layer: for `home` or `non-repo`, team and overlay are not applicable and only the user-global layer is read ([../../../reference/config-resolution.md](../../../reference/config-resolution.md), "The three layers"). Read `pr_body_required_sections` across the applicable layers per [../../../reference/config-resolution.md](../../../reference/config-resolution.md) (per-key override: a winning layer's value is taken whole, never merged with an earlier layer's). The team layer is `docs/conventions/source-control.yaml`: `bash "/lib/convention-key.sh" layer "" pr_body_required_sections` prints `yaml`, `legacy-md` or `none`, and `list` or `value` prints the YAML's heading list or keyword. Only when it prints `legacy-md` is the key read from the legacy markdown file's `## pr_body_required_sections` section, the compatibility read kept for one release. Either team file is a layer only when git tracks it, as in `parse-branch-issue.sh` and `/source-control:commit`: check `git ls-files --error-unmatch -- ":/docs/conventions/source-control.yaml"` before trusting `layer`, `list` or `value`, and `git ls-files --error-unmatch -- ":/.claude/source-control.md"` before the legacy read, since `convention-key.sh` does not check trackedness itself. An untracked or gitignored team file is skipped with a note saying so; when only the YAML is untracked, the key is read from the legacy file's section if that file is tracked and carries it. The value is a heading list or one of three keywords, each a resolved value that overrides a lower layer the same way a list would (never an absence; parallel to `trailer_policy`/`pr_body_attribution`): diff --git a/plugins/source-control/skills/pull-request/reference/ready-for-review.md b/plugins/source-control/skills/pull-request/reference/ready-for-review.md index 90531f266c..1b3ef51bf8 100644 --- a/plugins/source-control/skills/pull-request/reference/ready-for-review.md +++ b/plugins/source-control/skills/pull-request/reference/ready-for-review.md @@ -248,7 +248,10 @@ on github.com (an SSH host alias or an enterprise host refuses). Inside a lane `lane-push.sh` instead, as the `merge.agent-identity` the repository configures, and refuses when the session does not act as it, or when `--pr` names a fork head and no `--dir ` names the checkout to push (loop-lane convention, "Lane write identity"). For a fork head in a lane, add -`--dir` with the PR's checkout. +`--dir` with the PR's checkout. With `sync_stale_base` on, a base that moved on overlapping paths +since 2.5.2 is merged in before the push (see [create.md](create.md) §2.4.1, exit 4 on a +conflict). That merge commit has no verify-gate record, so the flip below refuses and the PR stays +a draft; run `ready` again. ```bash bash "/scripts/push-branch.sh" --pr "$PR_NUMBER" || exit 1 # only when 2.5.3 committed anything diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.sh index 3c099691ba..39c55c7310 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.sh @@ -71,7 +71,29 @@ # above runs: the push goes to the plugin's scripts/lane-push.sh (see the lane # block below), and --pr for a fork head needs --dir. # -# Usage: push-branch.sh [--check] [--dir ] [branch-name | --pr ] +# sync-stale-base: on the operator path, just before the push, a branch behind +# its base on overlapping paths gets the base merged in, so a pre-push hook +# that refuses a stale base with overlapping paths does not bounce a push that +# waited in a rate gate while the base moved. The base is the PR's base branch +# (--pr's own read, else `gh pr view `), or the fetch remote's default +# branch when the branch has no PR yet; it is fetched from the fetch remote. +# The overlap rule is the checkout's scripts/check-stale-base-overlap.sh +# --check / when that file exists (exit 1 merges, 0 does not, +# anything else skips), else paths changed on both sides since the merge-base. +# The merge is `git merge --no-edit`, never a rebase, signed when +# commit.gpgsign is true (git's own setting) or HEAD is signed (-S). It is +# skipped, with a note, when the base cannot be read or fetched, the branch is +# checked out in no worktree, tracked files have uncommitted edits, or a merge +# is in progress. A conflict aborts the merge and exits 4 without pushing. +# On: sync_stale_base: true in the checkout's tracked +# docs/conventions/source-control.yaml, or --sync-stale-base; --no-sync-stale-base +# turns it off. Off by default. A lane (below) never syncs. --check never syncs. +# +# Exit: 0 pushed (or --check passed); 1 refused or the push failed; 4 the +# sync-stale-base merge conflicted and was aborted, nothing pushed. +# +# Usage: push-branch.sh [--check] [--dir ] [--sync-stale-base | --no-sync-stale-base] +# [branch-name | --pr ] # (options in any order; defaults to the current branch; any other argument # refuses) set -uo pipefail @@ -80,6 +102,7 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" RESOLVER="${SCRIPT_DIR}/resolve-remote.sh" +CONVENTION_KEY="${SCRIPT_DIR}/../../../lib/convention-key.sh" DIR="" BRANCH="" @@ -87,12 +110,22 @@ PR="" HAS_DIR="" HAS_PR="" CHECK="" +SYNC="" while (($#)); do case "$1" in --check) CHECK=1 shift ;; + --sync-stale-base | --no-sync-stale-base) + if [[ -n "$SYNC" ]]; then + echo "push-branch.sh: --sync-stale-base and --no-sync-stale-base are given once, one of them; not pushing" >&2 + exit 1 + fi + SYNC=true + [[ "$1" == --sync-stale-base ]] || SYNC=false + shift + ;; --dir | --pr) if [[ $# -lt 2 || ("$1" == --dir && -n "$HAS_DIR") || ("$1" == --pr && -n "$HAS_PR") ]]; then echo "push-branch.sh: $1 needs one value and is given once; not pushing" >&2 @@ -135,8 +168,8 @@ if [[ -n "$HAS_PR" ]]; then exit 1 fi # The PR's URL names its base repository. - read -r BRANCH HEAD_REPO PR_BASE_REPO < <(gh pr view "$PR" --json headRefName,headRepository,headRepositoryOwner,url \ - -q '.headRefName + " " + .headRepositoryOwner.login + "/" + .headRepository.name + " " + (.url | split("/")[3:5] | join("/"))' 2>/dev/null | tr -d '\r') + read -r BRANCH HEAD_REPO PR_BASE_REPO PR_BASE_REF < <(gh pr view "$PR" --json headRefName,headRepository,headRepositoryOwner,url,baseRefName \ + -q '.headRefName + " " + .headRepositoryOwner.login + "/" + .headRepository.name + " " + (.url | split("/")[3:5] | join("/")) + " " + .baseRefName' 2>/dev/null | tr -d '\r') if [[ "${BRANCH:-}" == -* ]]; then echo "push-branch.sh: PR '$PR' head branch '$BRANCH' starts with '-'; not pushing" >&2 exit 1 @@ -149,13 +182,36 @@ elif [[ -z "$BRANCH" ]]; then BRANCH="$(git branch --show-current 2>/dev/null | tr -d '\r')" fi +# remote_default : the remote's default branch as `ls-remote --symref` +# reports it, or nothing. +remote_default() { + local out + out=$(git ls-remote --symref "$1" HEAD 2>/dev/null) || return 0 + sed -n 's|^ref: refs/heads/\([^[:space:]]*\)[[:space:]]HEAD$|\1|p' <<<"${out//$'\r'/}" +} + +# worktree_of : the worktree that has checked out, or nothing. +worktree_of() { + local line wt="" + while IFS= read -r line; do + case "$line" in + "worktree "*) wt="${line#worktree }" ;; + "branch refs/heads/$1") + echo "$wt" + return 0 + ;; + *) ;; + esac + done < <(git worktree list --porcelain 2>/dev/null) +} + # refuse_default : exit 1 when is main, master, or # 's default branch. The default is read from the remote itself # (`ls-remote --symref`), never the local /HEAD, which can be unset or # stale. A failed read refuses, and so does a read that names no default (an # empty remote, a server that does not advertise the symref, a detached HEAD). refuse_default() { - local out="" default="" what="'$2'" + local default="" what="'$2'" # A fork head pushes to the contributor's fork, never the base repository, so # its default branch may be pushed (lane-push.sh's is_fork rule). An unread # base repository counts as no fork. @@ -165,9 +221,7 @@ refuse_default() { fi what="PR '$PR' head '$2'" fi - if out=$(git ls-remote --symref "$1" HEAD 2>/dev/null); then - default=$(sed -n 's|^ref: refs/heads/\([^[:space:]]*\)[[:space:]]HEAD$|\1|p' <<<"${out//$'\r'/}") - fi + default=$(remote_default "$1") if [[ -z "$default" ]]; then echo "push-branch.sh: the default branch of $1 could not be read (git ls-remote failed or named no HEAD branch); not pushing" >&2 exit 1 @@ -228,21 +282,10 @@ if [[ "$ACTING" == *"[bot]" ]]; then echo "push-branch.sh: PR '$PR' head repository '$HEAD_REPO' is not '$BASE_REPO'; a lane pushes a fork head only with --dir; not pushing" >&2 exit 1 fi - SRC_DIR="" if [[ -n "$DIR" ]]; then SRC_DIR=$(pwd -P) else - WT="" - while IFS= read -r LINE; do - case "$LINE" in - "worktree "*) WT="${LINE#worktree }" ;; - "branch refs/heads/$BRANCH") - SRC_DIR="$WT" - break - ;; - *) ;; - esac - done < <(git worktree list --porcelain 2>/dev/null) + SRC_DIR=$(worktree_of "$BRANCH") fi if [[ -z "$SRC_DIR" ]]; then echo "push-branch.sh: '$BRANCH' is checked out in no worktree, so lane-push.sh has no HEAD to push; not pushing" >&2 @@ -281,11 +324,95 @@ if [[ -n "${HEAD_REPO:-}" ]]; then done <<<"$URLS" fi +FETCH_REMOTE=$(bash "$RESOLVER" "$BRANCH" 2>/dev/null) + +sync_skip() { echo "push-branch.sh: sync-stale-base skipped: $1; pushing as is" >&2; } + +# sync_stale_base : the sync-stale-base step of the header. Returns +# 0 to push; exits 4 on a conflict. +sync_stale_base() { + local wd="$1" root="" on="$SYNC" base="${PR_BASE_REF:-}" out ref rc mb tip paths_base paths_head sign=() + [[ -n "$wd" ]] && root=$(git -C "$wd" rev-parse --show-toplevel 2>/dev/null) + if [[ -z "$on" && -n "$root" ]] && + git -C "$root" ls-files --error-unmatch -- docs/conventions/source-control.yaml >/dev/null 2>&1; then + on=$(bash "$CONVENTION_KEY" value "$root" sync_stale_base) || + echo "push-branch.sh: docs/conventions/source-control.yaml does not parse; sync_stale_base read as false" >&2 + fi + [[ "$on" == true ]] || return 0 + [[ -n "$root" ]] || { + sync_skip "'$BRANCH' is checked out in no worktree" + return 0 + } + [[ -n "$FETCH_REMOTE" ]] || { + sync_skip "no fetch remote resolves for '$BRANCH'" + return 0 + } + if [[ -z "$PR" ]]; then + if out=$(gh pr view "$BRANCH" --json baseRefName -q .baseRefName 2>&1); then + base="${out//$'\r'/}" + elif [[ "$out" == *"no pull requests found"* ]]; then + base=$(cd "$wd" && remote_default "$FETCH_REMOTE") + fi + fi + [[ -n "$base" ]] || { + sync_skip "the base branch of '$BRANCH' could not be read" + return 0 + } + ref="$FETCH_REMOTE/$base" + git -C "$wd" fetch --quiet "$FETCH_REMOTE" "+refs/heads/$base:refs/remotes/$ref" || { + sync_skip "git fetch of $ref failed" + return 0 + } + git -C "$wd" diff --quiet HEAD -- || { + sync_skip "tracked files in $wd have uncommitted edits" + return 0 + } + ! git -C "$wd" rev-parse -q --verify MERGE_HEAD >/dev/null || { + sync_skip "a merge is in progress in $wd" + return 0 + } + if [[ -f "$root/scripts/check-stale-base-overlap.sh" ]]; then + (cd "$wd" && bash "$root/scripts/check-stale-base-overlap.sh" --check "$ref") >&2 + rc=$? + ((rc != 0)) || return 0 + ((rc == 1)) || { + sync_skip "scripts/check-stale-base-overlap.sh exited $rc" + return 0 + } + else + if ! mb=$(git -C "$wd" merge-base HEAD "$ref") || ! tip=$(git -C "$wd" rev-parse "$ref^{commit}") || + ! paths_base=$(git -C "$wd" diff --name-only --no-renames "$mb" "$tip") || + ! paths_head=$(git -C "$wd" diff --name-only --no-renames "$mb" HEAD); then + sync_skip "the paths changed since the merge-base of HEAD and $ref could not be read" + return 0 + fi + [[ "$mb" != "$tip" ]] || return 0 + LC_ALL=C comm -12 <(LC_ALL=C sort -u <<<"$paths_base") <(LC_ALL=C sort -u <<<"$paths_head") | + grep -q . || return 0 + fi + if [[ "$(git -C "$wd" config --bool commit.gpgsign)" != true ]] && + git -C "$wd" cat-file commit HEAD | grep -qE '^gpgsig(-sha256)? '; then + sign=(-S) + fi + if git -C "$wd" merge --no-edit ${sign[@]+"${sign[@]}"} "$ref" >&2; then + echo "push-branch.sh: merged $ref into '$BRANCH' (stale base with overlapping paths)" >&2 + return 0 + fi + out=$(git -C "$wd" diff --name-only --diff-filter=U) + git -C "$wd" merge --abort 2>/dev/null + if [[ -n "$out" ]]; then + echo "push-branch.sh: merging $ref into '$BRANCH' conflicted; merge aborted; not pushing. Merge it by hand (/source-control:resolve-conflicts), then push again" >&2 + exit 4 + fi + echo "push-branch.sh: merging $ref into '$BRANCH' failed; not pushing" >&2 + exit 1 +} + if [[ -n "$DIR" ]]; then + sync_stale_base "$(pwd -P)" exec git push "$PUSH_REMOTE" "HEAD:refs/heads/$BRANCH" fi - -FETCH_REMOTE=$(bash "$RESOLVER" "$BRANCH" 2>/dev/null) +[[ -z "$BRANCH" ]] || sync_stale_base "$(worktree_of "$BRANCH")" EXISTING_REMOTE=$(git config "branch.${BRANCH}.remote" 2>/dev/null) EXISTING_MERGE=$(git config "branch.${BRANCH}.merge" 2>/dev/null) diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh index 784761b989..9aef651b49 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh @@ -70,7 +70,13 @@ case "$1 $2" in ;; "repo view") echo "example-org/app" ;; "pr view") - if [[ " $* " == *" -R "* ]]; then + if [[ " $* " == *" baseRefName "* ]]; then + [[ -n ${PB_PR_BASE:-} ]] || { + echo "no pull requests found for branch \"$3\"" >&2 + exit 1 + } + echo "$PB_PR_BASE" + elif [[ " $* " == *" -R "* ]]; then printf '%s\n' "${PB_PR_HEAD_REPO:-example-org/app}" feat/b else echo "feat/b ${PB_PR_HEAD_REPO:-example-org/app}" @@ -82,6 +88,7 @@ EOF cat >"$SHIM/git" <>"$LOG/fetch.log" if [[ \$a == push && -n \${PB_RECORD_PUSH:-} ]]; then printf '%s\n' "\$@" >"$LOG/push.argv" pwd -P >"$LOG/push.cwd" @@ -104,8 +111,9 @@ export PATH="$SHIM:$PATH" # passing with PB_IDENTITY as the configured identity, else refusing as a # target that names none. lane-push.sh is the real one. PLUGIN="$WORKDIR/plugin" -mkdir -p "$PLUGIN/skills/pull-request/scripts" "$PLUGIN/skills/babysit-prs/scripts" "$PLUGIN/scripts" +mkdir -p "$PLUGIN/skills/pull-request/scripts" "$PLUGIN/skills/babysit-prs/scripts" "$PLUGIN/scripts" "$PLUGIN/lib" cp -p "${SCRIPT_DIR}/push-branch.sh" "$RESOLVER" "$PLUGIN/skills/pull-request/scripts/" +cp -p "${SCRIPT_DIR}/../../../lib/convention-key.sh" "${SCRIPT_DIR}/../../../lib/yaml-subset.awk" "$PLUGIN/lib/" cp -p "${SCRIPT_DIR}/../../../scripts/lane-push.sh" "$PLUGIN/scripts/" cat >"$PLUGIN/skills/babysit-prs/scripts/babysit-postmerge.sh" <"$WORKDIR/gitconfig-global" +export GIT_CONFIG_GLOBAL="$WORKDIR/gitconfig-global" GIT_CONFIG_NOSYSTEM=1 +# sync_clone : make_clone plus a ten-line c.txt (and, for yes, the +# config turning the sync on) committed on feat/x and pushed as the remote's +# trunk, so both sides share it. +sync_clone() { + local wc + wc=$(make_clone "$1") + printf '%s\n' 1 2 3 4 5 6 7 8 9 10 >"$wc/c.txt" + if [[ $2 == yes ]]; then + mkdir -p "$wc/docs/conventions" + echo 'sync_stale_base: true' >"$wc/docs/conventions/source-control.yaml" + fi + git -C "$wc" add -A + git -C "$wc" commit -q -m shared + git -C "$wc" push -q origin HEAD:refs/heads/trunk 2>/dev/null + echo "$wc" +} +# set_line : rewrite one line and commit it. +set_line() { + awk -v n="$3" -v t="$4" 'NR == n { $0 = t } { print }' "$1/$2" >"$1/$2.new" && mv "$1/$2.new" "$1/$2" + git -C "$1" add -A + git -C "$1" commit -q -m "edit $2" +} +# advance_base : move the remote's trunk on in a +# second clone, as another pull request landing would. +advance_base() { + local o="$WORKDIR/$1/other" + [[ -d $o ]] || { + git clone -q -b trunk "$WORKDIR/$1/origin.git" "$o" 2>/dev/null + git -C "$o" config user.email other@example.com + git -C "$o" config user.name Other + } + [[ -f $o/$2 ]] || : >"$o/$2" + set_line "$o" "$2" "$3" "$4" + git -C "$o" push -q origin trunk 2>/dev/null +} +sync_run() { + local wc="$1" + shift + rm -f "$LOG"/* + OLD_TIP=$(git -C "$wc" rev-parse HEAD) + (cd "$wc" && bash "$PUSH_BRANCH" "$@") >/dev/null 2>"$LOG/err" + RC=$? +} +parents() { git -C "$1" rev-list --parents -n 1 HEAD | wc -w | tr -d ' '; } +pushed() { git -C "$WORKDIR/$1/origin.git" rev-parse --verify --quiet "refs/heads/$BRANCH" || echo none; } +fetched() { [[ -s $LOG/fetch.log ]] && echo fetched || echo none; } +trunk_tip() { git -C "$WORKDIR/$1/origin.git" rev-parse refs/heads/trunk; } + +# Current base: nothing to merge, a plain push. +wc=$(sync_clone sync-current yes) +set_line "$wc" c.txt 1 branch-one +sync_run "$wc" +check "sync, current base: exit 0" "$RC" "0" +check "sync, current base: the base was fetched" "$(fetched)" "fetched" +check "sync, current base: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" +check "sync, current base: the remote received HEAD" "$(pushed sync-current)" "$OLD_TIP" + +# Stale base, no overlap: no merge (the hook allows it), a plain push. +wc=$(sync_clone sync-disjoint yes) +set_line "$wc" c.txt 1 branch-one +advance_base sync-disjoint other.txt 1 base-only +sync_run "$wc" +check "sync, stale no overlap: exit 0" "$RC" "0" +check "sync, stale no overlap: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" +check "sync, stale no overlap: the remote received HEAD" "$(pushed sync-disjoint)" "$OLD_TIP" + +# Stale base with overlap, unsigned repository: a --no-edit merge commit whose +# first parent is the old tip and second the base tip, no signature, pushed. +wc=$(sync_clone sync-overlap yes) +set_line "$wc" c.txt 1 branch-one +advance_base sync-overlap c.txt 10 base-ten +sync_run "$wc" +check "sync, overlap: exit 0" "$RC" "0" +check "sync, overlap: HEAD is a merge commit" "$(parents "$wc")" "3" +check "sync, overlap: first parent is the old tip (history kept)" "$(git -C "$wc" rev-parse HEAD^1)" "$OLD_TIP" +check "sync, overlap: second parent is the base tip" "$(git -C "$wc" rev-parse HEAD^2)" "$(trunk_tip sync-overlap)" +check "sync, overlap: both edits are in the tree" "$(sed -n '1p;10p' "$wc/c.txt" | paste -sd' ')" "branch-one base-ten" +check "sync, overlap: unsigned repository, unsigned merge" \ + "$(git -C "$wc" cat-file commit HEAD | grep -c '^gpgsig')" "0" +check "sync, overlap: the remote received the merge" "$(pushed sync-overlap)" "$(git -C "$wc" rev-parse HEAD)" + +# Signing follows the repository: commit.gpgsign=true signs the merge, and so +# does a signed tip with commit.gpgsign unset. +ssh-keygen -q -t ed25519 -N '' -f "$WORKDIR/signkey" >/dev/null +sign_cfg() { + git -C "$1" config gpg.format ssh + git -C "$1" config user.signingkey "$WORKDIR/signkey" +} +wc=$(sync_clone sync-gpgsign yes) +sign_cfg "$wc" +git -C "$wc" config commit.gpgsign true +set_line "$wc" c.txt 1 branch-one +advance_base sync-gpgsign c.txt 10 base-ten +sync_run "$wc" +check "sync, commit.gpgsign: exit 0" "$RC" "0" +check "sync, commit.gpgsign: HEAD is a merge commit" "$(parents "$wc")" "3" +check "sync, commit.gpgsign: the merge is signed" \ + "$(git -C "$wc" cat-file commit HEAD | grep -c '^gpgsig')" "1" +wc=$(sync_clone sync-signedtip yes) +sign_cfg "$wc" +printf 'branch-one\n' >"$wc/tip.txt" +awk 'NR == 1 { $0 = "branch-one" } { print }' "$wc/c.txt" >"$wc/c.new" && mv "$wc/c.new" "$wc/c.txt" +git -C "$wc" add -A +git -C "$wc" commit -q -S -m "signed tip" +advance_base sync-signedtip c.txt 10 base-ten +sync_run "$wc" +check "sync, signed tip: exit 0" "$RC" "0" +check "sync, signed tip: the merge is signed" \ + "$(git -C "$wc" cat-file commit HEAD | grep -c '^gpgsig')" "1" +check "sync, signed tip: HEAD is a merge commit" "$(parents "$wc")" "3" + +# Conflict: the merge is aborted, nothing is pushed, exit 4. +wc=$(sync_clone sync-conflict yes) +set_line "$wc" c.txt 1 branch-one +advance_base sync-conflict c.txt 1 base-one +sync_run "$wc" +check "sync, conflict: exit 4" "$RC" "4" +check "sync, conflict: names the conflict" "$(grep -c 'conflicted; merge aborted; not pushing' "$LOG/err")" "1" +check "sync, conflict: no merge in progress" \ + "$(git -C "$wc" rev-parse -q --verify MERGE_HEAD >/dev/null && echo merging || echo clean)" "clean" +check "sync, conflict: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" +check "sync, conflict: the remote did not receive the branch" "$(pushed sync-conflict)" "none" + +# Disabled: no config, or --no-sync-stale-base over the config, fetches nothing +# and merges nothing. +wc=$(sync_clone sync-off no) +set_line "$wc" c.txt 1 branch-one +advance_base sync-off c.txt 10 base-ten +sync_run "$wc" +check "sync, no config: exit 0" "$RC" "0" +check "sync, no config: nothing fetched" "$(fetched)" "none" +check "sync, no config: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" +wc=$(sync_clone sync-flag-off yes) +set_line "$wc" c.txt 1 branch-one +advance_base sync-flag-off c.txt 10 base-ten +sync_run "$wc" --no-sync-stale-base +check "sync, --no-sync-stale-base: exit 0" "$RC" "0" +check "sync, --no-sync-stale-base: nothing fetched" "$(fetched)" "none" +check "sync, --no-sync-stale-base: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" +check "sync, --no-sync-stale-base: the remote received HEAD" "$(pushed sync-flag-off)" "$OLD_TIP" +sync_run "$wc" --check +check "sync, --check: nothing fetched" "$(fetched)" "none" + +# --sync-stale-base turns it on with no config. +wc=$(sync_clone sync-flag-on no) +set_line "$wc" c.txt 1 branch-one +advance_base sync-flag-on c.txt 10 base-ten +sync_run "$wc" --sync-stale-base +check "sync, --sync-stale-base: exit 0" "$RC" "0" +check "sync, --sync-stale-base: HEAD is a merge commit" "$(parents "$wc")" "3" +for args in "--sync-stale-base --no-sync-stale-base" "--sync-stale-base --sync-stale-base"; do + # shellcheck disable=SC2086 # word-split on purpose: each entry is an argv. + sync_run "$wc" $args + check "sync, '$args': refused" "$RC" "1" +done + +# The pull request's base, not the default branch: a PR based on 'stack', +# which is current, merges nothing even though trunk moved on c.txt. +wc=$(sync_clone sync-stacked yes) +git -C "$wc" push -q origin HEAD:refs/heads/stack 2>/dev/null +set_line "$wc" c.txt 1 branch-one +advance_base sync-stacked c.txt 10 base-ten +PB_PR_BASE=stack sync_run "$wc" +check "sync, stacked PR: exit 0" "$RC" "0" +check "sync, stacked PR: fetched its base" "$(grep -c 'refs/heads/stack:' "$LOG/fetch.log")" "1" +check "sync, stacked PR: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" + +# The repository's own overlap rule wins when the checkout has one: a +# scripts/check-stale-base-overlap.sh that passes (exit 0) means no merge, +# though git sees an overlap. +wc=$(sync_clone sync-repo-rule yes) +mkdir -p "$wc/scripts" +printf '#!/usr/bin/env bash\necho "$*" >"%s/rule.argv"\nexit 0\n' "$LOG" >"$wc/scripts/check-stale-base-overlap.sh" +git -C "$wc" add scripts +git -C "$wc" commit -q -m "repo rule" +set_line "$wc" c.txt 1 branch-one +advance_base sync-repo-rule c.txt 10 base-ten +sync_run "$wc" +check "sync, repo rule: exit 0" "$RC" "0" +check "sync, repo rule: called with --check and the fetched base" "$(cat "$LOG/rule.argv" 2>/dev/null)" "--check origin/trunk" +check "sync, repo rule: its pass means no merge" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" + +# Uncommitted edits to tracked files: the sync is skipped, the push goes on. +wc=$(sync_clone sync-dirty yes) +set_line "$wc" c.txt 1 branch-one +advance_base sync-dirty c.txt 10 base-ten +echo dirty >>"$wc/c.txt" +sync_run "$wc" +check "sync, dirty tree: exit 0" "$RC" "0" +check "sync, dirty tree: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" +check "sync, dirty tree: says why" "$(grep -c 'uncommitted' "$LOG/err")" "1" + echo echo "Results: ${PASS} passed, ${FAIL} failed" [[ $FAIL -eq 0 ]] From a72ee8710ec7c5b61c0d43686e81f4af9f1de55f Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:52:05 -0400 Subject: [PATCH 2/5] perf(source-control): report the synced HEAD and note it for callers Capture the overlap and signature reads instead of piping into grep -q, which pipefail can misread; name the new HEAD after a sync merge; and say in the babysit safety rules and ready 2.5.4 what a sync merge or exit 4 means for those callers. Co-Authored-By: Claude Opus 5.5 --- .../skills/babysit-prs/reference/safety.md | 5 ++++- .../pull-request/reference/ready-for-review.md | 5 +++-- .../skills/pull-request/scripts/push-branch.sh | 12 ++++++------ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 5e0989fad9..043058c3be 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -77,7 +77,10 @@ value and its unset fallback. depend on `git checkout ` to reach the branch: when it is locked by a sibling worktree that command dead-ends (`fatal: '' is already used by worktree at ...`). Once the head assertion holds, push through pull-request's `push-branch.sh --dir --pr ` - and nothing else: it pushes the checkout's `HEAD:refs/heads/`. Inside a lane (a + and nothing else: it pushes the checkout's `HEAD:refs/heads/`. Attended, with + `sync_stale_base` on, it may first add one base-merge commit whose first parent is that asserted + `HEAD`, or exit 4 on a conflict with nothing pushed, which stops the push like any refusal + ([create.md](../../pull-request/reference/create.md) §2.4.1). Inside a lane (a `[bot]` login) it hands the push to `lane-push.sh --pr ` as the target's `merge.agent-identity` (the loop-lane convention's "Lane write identity"), which reads the PR's head repository and branch from the forge and pushes to `https://github.com//.git`, diff --git a/plugins/source-control/skills/pull-request/reference/ready-for-review.md b/plugins/source-control/skills/pull-request/reference/ready-for-review.md index 1b3ef51bf8..1bcfa294bb 100644 --- a/plugins/source-control/skills/pull-request/reference/ready-for-review.md +++ b/plugins/source-control/skills/pull-request/reference/ready-for-review.md @@ -250,8 +250,9 @@ the session does not act as it, or when `--pr` names a fork head and no `--dir < the checkout to push (loop-lane convention, "Lane write identity"). For a fork head in a lane, add `--dir` with the PR's checkout. With `sync_stale_base` on, a base that moved on overlapping paths since 2.5.2 is merged in before the push (see [create.md](create.md) §2.4.1, exit 4 on a -conflict). That merge commit has no verify-gate record, so the flip below refuses and the PR stays -a draft; run `ready` again. +conflict), and its stderr names the new HEAD. That merge commit is not the head the verify gate +recorded, so the head wait below can time out and the flip refuses, leaving the PR a draft; run +`ready` again. ```bash bash "/scripts/push-branch.sh" --pr "$PR_NUMBER" || exit 1 # only when 2.5.3 committed anything diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.sh index 39c55c7310..ea9d6a0682 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.sh @@ -331,7 +331,7 @@ sync_skip() { echo "push-branch.sh: sync-stale-base skipped: $1; pushing as is" # sync_stale_base : the sync-stale-base step of the header. Returns # 0 to push; exits 4 on a conflict. sync_stale_base() { - local wd="$1" root="" on="$SYNC" base="${PR_BASE_REF:-}" out ref rc mb tip paths_base paths_head sign=() + local wd="$1" root="" on="$SYNC" base="${PR_BASE_REF:-}" out ref rc mb tip paths_base paths_head overlap sign=() [[ -n "$wd" ]] && root=$(git -C "$wd" rev-parse --show-toplevel 2>/dev/null) if [[ -z "$on" && -n "$root" ]] && git -C "$root" ls-files --error-unmatch -- docs/conventions/source-control.yaml >/dev/null 2>&1; then @@ -387,15 +387,15 @@ sync_stale_base() { return 0 fi [[ "$mb" != "$tip" ]] || return 0 - LC_ALL=C comm -12 <(LC_ALL=C sort -u <<<"$paths_base") <(LC_ALL=C sort -u <<<"$paths_head") | - grep -q . || return 0 + overlap=$(LC_ALL=C comm -12 <(LC_ALL=C sort -u <<<"$paths_base") <(LC_ALL=C sort -u <<<"$paths_head")) + [[ -n "${overlap//$'\n'/}" ]] || return 0 fi - if [[ "$(git -C "$wd" config --bool commit.gpgsign)" != true ]] && - git -C "$wd" cat-file commit HEAD | grep -qE '^gpgsig(-sha256)? '; then + out=$(git -C "$wd" cat-file commit HEAD) + if [[ "$(git -C "$wd" config --bool commit.gpgsign)" != true && "$out" =~ (^|$'\n')gpgsig(-sha256)?\ ]]; then sign=(-S) fi if git -C "$wd" merge --no-edit ${sign[@]+"${sign[@]}"} "$ref" >&2; then - echo "push-branch.sh: merged $ref into '$BRANCH' (stale base with overlapping paths)" >&2 + echo "push-branch.sh: merged $ref into '$BRANCH' (stale base with overlapping paths); HEAD is now $(git -C "$wd" rev-parse HEAD)" >&2 return 0 fi out=$(git -C "$wd" diff --name-only --diff-filter=U) From 17bc8f547a35b419d26c1a22127bf1717890823b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:10:57 -0400 Subject: [PATCH 3/5] test(source-control): keep fixture commits off the machine's signing key On a machine with a global commit.gpgsign, fixture commits in push-branch.test.sh and two babysit-prs tests signed through the machine's signer, which failed intermittently when the pre-push check ran the suites in parallel ("failed to write commit object", exit 128), blocking the push. push-branch.test.sh now gives its fixtures an empty global config, and the two babysit-prs fixtures set commit.gpgsign=false repo-locally, as test_prune_babysit_worktrees.py already does. Co-Authored-By: Claude Opus 5.5 --- .../babysit-prs/scripts/tests/test_babysit_lane_room.py | 3 +++ .../scripts/tests/test_babysit_postmerge_quarantine.py | 3 +++ .../skills/pull-request/scripts/push-branch.test.sh | 9 +++++---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_room.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_room.py index e0da9604d2..5eeefc563e 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_room.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_lane_room.py @@ -1183,6 +1183,9 @@ def setUp(self) -> None: self.root = pathlib.Path(tempfile.mkdtemp(prefix="check-code-")) self.addCleanup(shutil.rmtree, self.root, ignore_errors=True) self.git("init", "-q") + # Repo-local, as in test_prune_babysit_worktrees.py: a global + # commit.gpgsign would sign every fixture commit with the machine's key. + self.git("config", "commit.gpgsign", "false") self.commits: dict[str, str] = {} for name, path in ( ("init", "plugins/source-control/skills/babysit-prs/scripts/a.py"), diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py index 9001816324..cd8a8041a9 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_quarantine.py @@ -684,6 +684,9 @@ def setUp(self) -> None: self.git(self.root, "init", "-q") self.git(self.root, "config", "user.email", "t@example.com") self.git(self.root, "config", "user.name", "t") + # Repo-local, as in test_prune_babysit_worktrees.py: a global + # commit.gpgsign would sign every fixture commit with the machine's key. + self.git(self.root, "config", "commit.gpgsign", "false") self.green = self.commit( {pm.PLAN_SCRIPT: self.PLAN, "tree.txt": "green"}, "green" ) diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh index 9aef651b49..e667c3ed2d 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh @@ -40,6 +40,11 @@ PASS=0 FAIL=0 WORKDIR="$(mktemp -d)" trap 'rm -rf "$WORKDIR"' EXIT +# Fixtures read only their own git config: a global commit.gpgsign or signing +# program would sign every fixture commit with the machine's key, and decide +# what the sync-stale-base cases sign. +: >"$WORKDIR/gitconfig-global" +export GIT_CONFIG_GLOBAL="$WORKDIR/gitconfig-global" GIT_CONFIG_NOSYSTEM=1 BRANCH="feat/x" @@ -588,10 +593,6 @@ unset PB_GITHUB_ROOT # rebase), so the pre-push stale-base refusal does not bounce it. Off by # default; on through sync_stale_base in docs/conventions/source-control.yaml # or --sync-stale-base, off again through --no-sync-stale-base. -# Signing is read from the fixture alone: a global commit.gpgsign or signing -# program would decide what these cases sign. -: >"$WORKDIR/gitconfig-global" -export GIT_CONFIG_GLOBAL="$WORKDIR/gitconfig-global" GIT_CONFIG_NOSYSTEM=1 # sync_clone : make_clone plus a ten-line c.txt (and, for yes, the # config turning the sync on) committed on feat/x and pushed as the remote's # trunk, so both sides share it. From 3fef0038fcd27f10f4a498f82b77588047665274 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:24:17 -0400 Subject: [PATCH 4/5] fix(source-control): take sync-stale-base's setting and rule from the base The sync step read sync_stale_base from the pushed checkout and ran that checkout's scripts/check-stale-base-overlap.sh, so a head checked out to review or fix could switch the step on and run its own script. The setting is now read from the base repository's default branch as the clone last fetched it, and the overlap script (with the base's scripts/) is extracted from the base ref with git archive; nothing in the head is run. The base is fetched from the remote that is the PR's base repository, not the fetch remote, so a head that fetches from a fork never syncs against the fork's same-named branch; with no such remote the step is skipped with a note. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0144XNKz9sKFKYo4gQtVW78R --- ...rf-push-branch-sync-stale-base-3f9a1c62.md | 2 +- .../reference/config-resolution.md | 12 +- .../pull-request/scripts/push-branch.sh | 111 ++++++++++++------ .../pull-request/scripts/push-branch.test.sh | 86 +++++++++++--- 4 files changed, 154 insertions(+), 57 deletions(-) diff --git a/.changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md b/.changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md index ae4488a18b..1179f3d5c0 100644 --- a/.changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md +++ b/.changes/source-control/perf-push-branch-sync-stale-base-3f9a1c62.md @@ -4,4 +4,4 @@ bump: minor ### Added -- `push-branch.sh` can merge the pull request's base into the branch just before it pushes, when the branch is behind the base and both changed overlapping paths since the merge-base. A push that waited in a rate gate while the base moved is then not refused by a pre-push stale-base check. The overlap rule is the checkout's `scripts/check-stale-base-overlap.sh --check /` when the file exists, else the paths both sides changed. The merge is `git merge --no-edit`, never a rebase. It is signed when `commit.gpgsign` is true or HEAD is signed. A conflict aborts the merge, pushes nothing and exits 4. The step is off by default. Turn it on with `sync_stale_base: true` in `docs/conventions/source-control.yaml`, or `--sync-stale-base` for one push; `--no-sync-stale-base` turns it off for one push. A lane push never runs it. The `create` and `ready` references describe the new step and exit code. +- `push-branch.sh` can merge the pull request's base into the branch just before it pushes, when the branch is behind the base and both changed overlapping paths since the merge-base. A push that waited in a rate gate while the base moved is then not refused by a pre-push stale-base check. The base is fetched from the remote that is the PR's base repository; with none, the step is skipped. The overlap rule is the base ref's `scripts/check-stale-base-overlap.sh --check /` when the base has one, else the paths both sides changed; nothing in the pushed head is run. The merge is `git merge --no-edit`, never a rebase. It is signed when `commit.gpgsign` is true or HEAD is signed. A conflict aborts the merge, pushes nothing and exits 4. The step is off by default. Turn it on with `sync_stale_base: true` in `docs/conventions/source-control.yaml` on the base repository's default branch, or `--sync-stale-base` for one push; `--no-sync-stale-base` turns it off for one push. A lane push never runs it. The `create` and `ready` references describe the new step and exit code. diff --git a/plugins/source-control/reference/config-resolution.md b/plugins/source-control/reference/config-resolution.md index ee61c08cfc..5c7d97ae29 100644 --- a/plugins/source-control/reference/config-resolution.md +++ b/plugins/source-control/reference/config-resolution.md @@ -222,12 +222,12 @@ reads the file itself, through a script where the key names one below. [`push-branch.sh`](../skills/pull-request/scripts/push-branch.sh) merges the pull request's base into a branch behind it on overlapping paths just before it pushes (the script header holds the base, overlap, signing and skip rules, and exit 4 for a conflict). Resolution, lowest - first: the default, `false`; `sync_stale_base` in the checkout's tracked - `docs/conventions/source-control.yaml`; `--sync-stale-base` or `--no-sync-stale-base` on the - call. No `userConfig` option. The script reads the file from the checkout it pushes, not the - base branch: either value leaves the push going to the same place, and `true` only adds a merge - of the base. A file the parser rejects, or another value, reads as `false`. A lane push never - syncs. + first: the default, `false`; `sync_stale_base` in `docs/conventions/source-control.yaml` at the + base repository's default branch, as the clone last fetched it; `--sync-stale-base` or + `--no-sync-stale-base` on the call. No `userConfig` option. Neither the setting nor the overlap + script is read from the branch being pushed: a head must not switch on a step that runs a + script, or choose that script. A file the parser rejects, or another value, reads as `false`. A + lane push never syncs. - `pre_ready`: which pre-flip steps `/source-control:pull-request ready` runs, a list of `{step, enabled}` mappings over `base-merge`, `plan-evidence`, `code-review`, `security-review` and `verify`; a step the list leaves out runs. Team level only, with no `userConfig` option: a diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.sh index ea9d6a0682..e2eae25f6b 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.sh @@ -76,17 +76,21 @@ # that refuses a stale base with overlapping paths does not bounce a push that # waited in a rate gate while the base moved. The base is the PR's base branch # (--pr's own read, else `gh pr view `), or the fetch remote's default -# branch when the branch has no PR yet; it is fetched from the fetch remote. -# The overlap rule is the checkout's scripts/check-stale-base-overlap.sh -# --check / when that file exists (exit 1 merges, 0 does not, -# anything else skips), else paths changed on both sides since the merge-base. +# branch when the branch has no PR yet. It is fetched from the remote whose URL +# is the PR's base repository on github.com (the fetch remote when there is no +# PR); with no such remote the step is skipped. Nothing the pushed head holds +# decides the step or runs in it: the overlap rule is the base ref's +# scripts/check-stale-base-overlap.sh (with the base's scripts/ beside it), +# run as --check / (exit 1 merges, 0 does not, anything else +# skips), else paths changed on both sides since the merge-base. # The merge is `git merge --no-edit`, never a rebase, signed when # commit.gpgsign is true (git's own setting) or HEAD is signed (-S). It is # skipped, with a note, when the base cannot be read or fetched, the branch is # checked out in no worktree, tracked files have uncommitted edits, or a merge # is in progress. A conflict aborts the merge and exits 4 without pushing. -# On: sync_stale_base: true in the checkout's tracked -# docs/conventions/source-control.yaml, or --sync-stale-base; --no-sync-stale-base +# On: sync_stale_base: true in docs/conventions/source-control.yaml at the +# base repository's default branch, as this clone last fetched it, or +# --sync-stale-base; --no-sync-stale-base # turns it off. Off by default. A lane (below) never syncs. --check never syncs. # # Exit: 0 pushed (or --check passed); 1 refused or the push failed; 4 the @@ -190,6 +194,32 @@ remote_default() { sed -n 's|^ref: refs/heads/\([^[:space:]]*\)[[:space:]]HEAD$|\1|p' <<<"${out//$'\r'/}" } +# github_repo_of : the owner/repo a github.com URL (https, ssh:// or +# scp-style) names, or nothing. +github_repo_of() { + local url="${1%/}" + url="${url%.git}" + case "${url,,}" in + https://github.com/*) echo "${url:19}" ;; + ssh://git@github.com/*) echo "${url:21}" ;; + git@github.com:*) echo "${url:15}" ;; + *) ;; + esac +} + +# remote_of_repo : the first remote whose fetch URL is that +# github.com repository, or nothing. +remote_of_repo() { + local r repo + for r in $(git remote); do + repo=$(github_repo_of "$(git remote get-url "$r" 2>/dev/null)") + if [[ -n "$repo" && "${repo,,}" == "${1,,}" ]]; then + echo "$r" + return 0 + fi + done +} + # worktree_of : the worktree that has checked out, or nothing. worktree_of() { local line wt="" @@ -308,15 +338,7 @@ PUSH_REMOTE=$(bash "$RESOLVER" --push "$BRANCH") || exit 1 if [[ -n "${HEAD_REPO:-}" ]]; then URLS=$(git remote get-url --push --all "$PUSH_REMOTE" 2>/dev/null) while IFS= read -r URL; do - URL="${URL%/}" - URL="${URL%.git}" - URL_REPO="" - case "${URL,,}" in - https://github.com/*) URL_REPO="${URL:19}" ;; - ssh://git@github.com/*) URL_REPO="${URL:21}" ;; - git@github.com:*) URL_REPO="${URL:15}" ;; - *) ;; - esac + URL_REPO=$(github_repo_of "$URL") if [[ -z "$URL_REPO" || "${URL_REPO,,}" != "${HEAD_REPO,,}" ]]; then echo "push-branch.sh: '$BRANCH' pushes to '$PUSH_REMOTE' ($URL), not PR '$PR' head repository '$HEAD_REPO' on github.com (https://github.com/, ssh://git@github.com/ or git@github.com:); not pushing" >&2 exit 1 @@ -329,37 +351,51 @@ FETCH_REMOTE=$(bash "$RESOLVER" "$BRANCH" 2>/dev/null) sync_skip() { echo "push-branch.sh: sync-stale-base skipped: $1; pushing as is" >&2; } # sync_stale_base : the sync-stale-base step of the header. Returns -# 0 to push; exits 4 on a conflict. +# 0 to push; exits 4 on a conflict. Nothing the pushed head holds decides +# whether it runs or is run by it: the setting is read from the base +# repository's default branch and the overlap script from the base ref. sync_stale_base() { - local wd="$1" root="" on="$SYNC" base="${PR_BASE_REF:-}" out ref rc mb tip paths_base paths_head overlap sign=() - [[ -n "$wd" ]] && root=$(git -C "$wd" rev-parse --show-toplevel 2>/dev/null) - if [[ -z "$on" && -n "$root" ]] && - git -C "$root" ls-files --error-unmatch -- docs/conventions/source-control.yaml >/dev/null 2>&1; then - on=$(bash "$CONVENTION_KEY" value "$root" sync_stale_base) || - echo "push-branch.sh: docs/conventions/source-control.yaml does not parse; sync_stale_base read as false" >&2 + local wd="$1" on="$SYNC" base="${PR_BASE_REF:-}" base_repo="${PR_BASE_REPO:-}" remote="$FETCH_REMOTE" + local tmp out ref rc mb tip paths_base paths_head overlap default sign=() + [[ "$on" != false ]] || return 0 + if [[ -n "$base_repo" ]]; then + remote=$(cd "${wd:-.}" && remote_of_repo "$base_repo") fi - [[ "$on" == true ]] || return 0 - [[ -n "$root" ]] || { + if [[ -z "$on" ]]; then + [[ -n "$remote" ]] || return 0 + default=$(git symbolic-ref --short "refs/remotes/$remote/HEAD" 2>/dev/null) + default="${default#"$remote"/}" + [[ -n "$default" ]] || default=$(remote_default "$remote") + out=$(git show "refs/remotes/$remote/$default:docs/conventions/source-control.yaml" 2>/dev/null) || return 0 + tmp=$(mktemp -d) || return 0 + mkdir -p "$tmp/docs/conventions" && printf '%s\n' "$out" >"$tmp/docs/conventions/source-control.yaml" + on=$(bash "$CONVENTION_KEY" value "$tmp" sync_stale_base) || + echo "push-branch.sh: $remote/$default:docs/conventions/source-control.yaml does not parse; sync_stale_base read as false" >&2 + rm -rf "$tmp" + [[ "$on" == true ]] || return 0 + fi + [[ -n "$wd" ]] || { sync_skip "'$BRANCH' is checked out in no worktree" return 0 } - [[ -n "$FETCH_REMOTE" ]] || { - sync_skip "no fetch remote resolves for '$BRANCH'" - return 0 - } if [[ -z "$PR" ]]; then - if out=$(gh pr view "$BRANCH" --json baseRefName -q .baseRefName 2>&1); then - base="${out//$'\r'/}" + if out=$(gh pr view "$BRANCH" --json baseRefName,url -q '.baseRefName + " " + (.url | split("/")[3:5] | join("/"))' 2>&1); then + read -r base base_repo <<<"${out//$'\r'/}" + remote=$(cd "$wd" && remote_of_repo "$base_repo") elif [[ "$out" == *"no pull requests found"* ]]; then base=$(cd "$wd" && remote_default "$FETCH_REMOTE") fi fi + [[ -n "$remote" ]] || { + sync_skip "no remote fetches the base repository ${base_repo:-of $BRANCH}" + return 0 + } [[ -n "$base" ]] || { sync_skip "the base branch of '$BRANCH' could not be read" return 0 } - ref="$FETCH_REMOTE/$base" - git -C "$wd" fetch --quiet "$FETCH_REMOTE" "+refs/heads/$base:refs/remotes/$ref" || { + ref="$remote/$base" + git -C "$wd" fetch --quiet "$remote" "+refs/heads/$base:refs/remotes/$ref" || { sync_skip "git fetch of $ref failed" return 0 } @@ -371,12 +407,17 @@ sync_stale_base() { sync_skip "a merge is in progress in $wd" return 0 } - if [[ -f "$root/scripts/check-stale-base-overlap.sh" ]]; then - (cd "$wd" && bash "$root/scripts/check-stale-base-overlap.sh" --check "$ref") >&2 + if git -C "$wd" cat-file -e "$ref:scripts/check-stale-base-overlap.sh" 2>/dev/null; then + # The base's copy of the repository's rule (and the libraries beside it), + # never the pushed head's. + tmp=$(mktemp -d) || return 0 + git -C "$wd" archive "$ref" -- scripts | tar -x -C "$tmp" + (cd "$wd" && bash "$tmp/scripts/check-stale-base-overlap.sh" --check "$ref") >&2 rc=$? + rm -rf "$tmp" ((rc != 0)) || return 0 ((rc == 1)) || { - sync_skip "scripts/check-stale-base-overlap.sh exited $rc" + sync_skip "the base's scripts/check-stale-base-overlap.sh exited $rc" return 0 } else diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh index e667c3ed2d..025ecd6e8e 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh @@ -75,12 +75,12 @@ case "$1 $2" in ;; "repo view") echo "example-org/app" ;; "pr view") - if [[ " $* " == *" baseRefName "* ]]; then + if [[ " $* " == *" baseRefName,url "* ]]; then [[ -n ${PB_PR_BASE:-} ]] || { echo "no pull requests found for branch \"$3\"" >&2 exit 1 } - echo "$PB_PR_BASE" + echo "$PB_PR_BASE ${PB_PR_BASE_REPO:-example-org/app}" elif [[ " $* " == *" -R "* ]]; then printf '%s\n' "${PB_PR_HEAD_REPO:-example-org/app}" feat/b else @@ -99,7 +99,7 @@ for a in "\$@"; do pwd -P >"$LOG/push.cwd" exit 0 fi - if [[ (\$a == push || \$a == ls-remote) && -n \${PB_GITHUB_ROOT:-} ]]; then + if [[ (\$a == push || \$a == ls-remote || \$a == fetch) && -n \${PB_GITHUB_ROOT:-} ]]; then exec "$REAL_GIT" -c "url.\$PB_GITHUB_ROOT/.insteadOf=https://github.com/" \ -c "url.\$PB_GITHUB_ROOT/.insteadOf=ssh://git@github.com/" \ -c "url.\$PB_GITHUB_ROOT/.insteadOf=git@github.com:" "\$@" @@ -750,29 +750,85 @@ for args in "--sync-stale-base --no-sync-stale-base" "--sync-stale-base --sync-s done # The pull request's base, not the default branch: a PR based on 'stack', -# which is current, merges nothing even though trunk moved on c.txt. +# which is current, merges nothing even though trunk moved on c.txt. The base +# is fetched from the remote that is the PR's base repository. wc=$(sync_clone sync-stacked yes) git -C "$wc" push -q origin HEAD:refs/heads/stack 2>/dev/null +git -C "$wc" remote set-url origin https://github.com/sync-stacked/origin.git set_line "$wc" c.txt 1 branch-one advance_base sync-stacked c.txt 10 base-ten -PB_PR_BASE=stack sync_run "$wc" +PB_GITHUB_ROOT="$WORKDIR" PB_PR_BASE=stack PB_PR_BASE_REPO=sync-stacked/origin sync_run "$wc" check "sync, stacked PR: exit 0" "$RC" "0" -check "sync, stacked PR: fetched its base" "$(grep -c 'refs/heads/stack:' "$LOG/fetch.log")" "1" +check "sync, stacked PR: fetched its base" "$(grep -c 'origin +refs/heads/stack:' "$LOG/fetch.log")" "1" check "sync, stacked PR: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" - -# The repository's own overlap rule wins when the checkout has one: a -# scripts/check-stale-base-overlap.sh that passes (exit 0) means no merge, -# though git sees an overlap. -wc=$(sync_clone sync-repo-rule yes) -mkdir -p "$wc/scripts" +# A PR whose base repository no remote fetches skips the sync and still pushes. +PB_GITHUB_ROOT="$WORKDIR" PB_PR_BASE=stack PB_PR_BASE_REPO=elsewhere/app sync_run "$wc" +check "sync, base repository with no remote: exit 0" "$RC" "0" +check "sync, base repository with no remote: says why" "$(grep -c 'no remote fetches the base repository elsewhere/app' "$LOG/err")" "1" +check "sync, base repository with no remote: nothing fetched" "$(fetched)" "none" +check "sync, base repository with no remote: pushed" "$(pushed sync-stacked)" "$OLD_TIP" + +# --pr on a head that fetches from a fork: the base is fetched from the remote +# that is the PR's base repository (upstream), never the fork's same-named +# branch, and the fork's stale trunk is not what gets merged. +wc=$(sync_clone sync-fork yes) +git init -q --bare "$WORKDIR/sync-fork/upstream.git" +git -C "$wc" push -q "$WORKDIR/sync-fork/upstream.git" HEAD:refs/heads/trunk 2>/dev/null +git -C "$WORKDIR/sync-fork/upstream.git" symbolic-ref HEAD refs/heads/trunk +git -C "$wc" remote add upstream https://github.com/sync-fork/upstream.git +git -C "$wc" remote set-url origin https://github.com/sync-fork/origin.git +PB_GITHUB_ROOT="$WORKDIR" git -C "$wc" fetch -q upstream +set_line "$wc" c.txt 1 branch-one +up="$WORKDIR/sync-fork/up" +git clone -q -b trunk "$WORKDIR/sync-fork/upstream.git" "$up" 2>/dev/null +git -C "$up" config user.email up@example.com +git -C "$up" config user.name Up +set_line "$up" c.txt 10 upstream-ten +git -C "$up" push -q origin trunk 2>/dev/null +fork_stub="$WORKDIR/sync-fork/bin" +mkdir -p "$fork_stub" +# shellcheck disable=SC2016 # the stub body expands when it runs, not here. +printf '#!/usr/bin/env bash\n[[ "$1" == api ]] && echo example-user && exit 0\necho "%s sync-fork/origin sync-fork/upstream trunk"\n' "$BRANCH" >"$fork_stub/gh" +chmod +x "$fork_stub/gh" +PATH="$fork_stub:$PATH" PB_GITHUB_ROOT="$WORKDIR" sync_run "$wc" --pr 50 +check "sync, fork head: exit 0" "$RC" "0" +check "sync, fork head: fetched the base from upstream" "$(grep -c 'upstream +refs/heads/trunk:' "$LOG/fetch.log")" "1" +check "sync, fork head: merged upstream's trunk" "$(git -C "$wc" rev-parse HEAD^2)" "$(git -C "$WORKDIR/sync-fork/upstream.git" rev-parse trunk)" + +# Nothing the pushed head holds decides the step or runs in it. A head that +# turns sync_stale_base on, where the base's config does not, fetches nothing. +wc=$(sync_clone sync-head-config no) +mkdir -p "$wc/docs/conventions" +echo 'sync_stale_base: true' >"$wc/docs/conventions/source-control.yaml" +git -C "$wc" add -A +git -C "$wc" commit -q -m "head turns it on" +set_line "$wc" c.txt 1 branch-one +advance_base sync-head-config c.txt 10 base-ten +sync_run "$wc" +check "sync, head-only config: exit 0" "$RC" "0" +check "sync, head-only config: nothing fetched" "$(fetched)" "none" +check "sync, head-only config: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" + +# The overlap rule is the base's scripts/check-stale-base-overlap.sh: it runs +# with --check and the fetched base, and its pass (exit 0) means no merge +# though git sees an overlap. The head's edited copy, which would merge and +# leave a marker, is never run. +wc=$(make_clone sync-repo-rule) +printf '%s\n' 1 2 3 4 5 6 7 8 9 10 >"$wc/c.txt" +mkdir -p "$wc/docs/conventions" "$wc/scripts" +echo 'sync_stale_base: true' >"$wc/docs/conventions/source-control.yaml" printf '#!/usr/bin/env bash\necho "$*" >"%s/rule.argv"\nexit 0\n' "$LOG" >"$wc/scripts/check-stale-base-overlap.sh" -git -C "$wc" add scripts -git -C "$wc" commit -q -m "repo rule" +git -C "$wc" add -A +git -C "$wc" commit -q -m "shared with the base rule" +git -C "$wc" push -q origin HEAD:refs/heads/trunk 2>/dev/null +printf '#!/usr/bin/env bash\ntouch "%s/head-rule-ran"\nexit 1\n' "$LOG" >"$wc/scripts/check-stale-base-overlap.sh" +git -C "$wc" commit -q -am "head edits the rule" set_line "$wc" c.txt 1 branch-one advance_base sync-repo-rule c.txt 10 base-ten sync_run "$wc" check "sync, repo rule: exit 0" "$RC" "0" -check "sync, repo rule: called with --check and the fetched base" "$(cat "$LOG/rule.argv" 2>/dev/null)" "--check origin/trunk" +check "sync, repo rule: the base's copy ran with --check and the fetched base" "$(cat "$LOG/rule.argv" 2>/dev/null)" "--check origin/trunk" +check "sync, repo rule: the head's copy did not run" "$([[ -e $LOG/head-rule-ran ]] && echo ran || echo none)" "none" check "sync, repo rule: its pass means no merge" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" # Uncommitted edits to tracked files: the sync is skipped, the push goes on. From d0a420be6d235404fe521880e6e6a1b28a377f58 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 28 Oct 2026 00:19:32 -0400 Subject: [PATCH 5/5] fix(source-control): read sync_stale_base from the base repo, quietly Without --pr, sync_stale_base was read from the fetch remote's default branch before the PR's base repository was known, so a fork's copy decided the step. It is now read from the remote that is the base repository: the forge is asked for it only when some fetched remote turns the setting on, so with the setting unset everywhere the step makes no network call. The default branch is taken from the clone's fetched refs (the remote HEAD symref, else main, master or trunk), never ls-remote, on that off path. Co-Authored-By: Claude Opus 5.5 --- .../reference/config-resolution.md | 11 +-- .../pull-request/scripts/push-branch.sh | 76 ++++++++++++++----- .../pull-request/scripts/push-branch.test.sh | 63 +++++++++++++++ 3 files changed, 125 insertions(+), 25 deletions(-) diff --git a/plugins/source-control/reference/config-resolution.md b/plugins/source-control/reference/config-resolution.md index 3e4b9a4737..5174b136a4 100644 --- a/plugins/source-control/reference/config-resolution.md +++ b/plugins/source-control/reference/config-resolution.md @@ -223,11 +223,12 @@ reads the file itself, through a script where the key names one below. base into a branch behind it on overlapping paths just before it pushes (the script header holds the base, overlap, signing and skip rules, and exit 4 for a conflict). Resolution, lowest first: the default, `false`; `sync_stale_base` in `docs/conventions/source-control.yaml` at the - base repository's default branch, as the clone last fetched it; `--sync-stale-base` or - `--no-sync-stale-base` on the call. No `userConfig` option. Neither the setting nor the overlap - script is read from the branch being pushed: a head must not switch on a step that runs a - script, or choose that script. A file the parser rejects, or another value, reads as `false`. A - lane push never syncs. + base repository's default branch, as the clone last fetched it, with no network read (without + `--pr`, the forge is asked for the base repository only when a fetched remote turns it on); + `--sync-stale-base` or `--no-sync-stale-base` on the call. No `userConfig` option. Neither the + setting nor the overlap script is read from the branch being pushed: a head must not switch on a + step that runs a script, or choose that script. A file the parser rejects, or another value, + reads as `false`. A lane push never syncs. - `dialect`: the regex dialect `subject_pattern` and `pr_title_pattern` are written in, default `posix-ere`. No skill here reads it. The guardrails enforcement resolver does, and any other value turns its enforcement off with a diagnostic; see the diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.sh index e2eae25f6b..7ebc97127b 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.sh @@ -89,8 +89,10 @@ # checked out in no worktree, tracked files have uncommitted edits, or a merge # is in progress. A conflict aborts the merge and exits 4 without pushing. # On: sync_stale_base: true in docs/conventions/source-control.yaml at the -# base repository's default branch, as this clone last fetched it, or -# --sync-stale-base; --no-sync-stale-base +# base repository's default branch, as this clone last fetched it (no network +# read; without --pr the base repository is asked of the forge only when some +# fetched remote turns the setting on), or --sync-stale-base; +# --no-sync-stale-base # turns it off. Off by default. A lane (below) never syncs. --check never syncs. # # Exit: 0 pushed (or --check passed); 1 refused or the push failed; 4 the @@ -350,29 +352,58 @@ FETCH_REMOTE=$(bash "$RESOLVER" "$BRANCH" 2>/dev/null) sync_skip() { echo "push-branch.sh: sync-stale-base skipped: $1; pushing as is" >&2; } +# local_default : 's default branch as this clone last fetched +# it (the refs/remotes//HEAD symref, else the first of main, master or +# trunk it holds), or nothing. No network call. +local_default() { + local d b + d=$(git symbolic-ref --short "refs/remotes/$1/HEAD" 2>/dev/null) && { + echo "${d#"$1"/}" + return 0 + } + for b in main master trunk; do + git rev-parse -q --verify "refs/remotes/$1/$b" >/dev/null && { + echo "$b" + return 0 + } + done +} + +# setting_at : sync_stale_base in docs/conventions/source-control.yaml +# on 's default branch as this clone last fetched it, or nothing. +setting_at() { + local d out tmp on="" + d=$(local_default "$1") + [[ -n "$d" ]] || return 0 + out=$(git show "refs/remotes/$1/$d:docs/conventions/source-control.yaml" 2>/dev/null) || return 0 + tmp=$(mktemp -d) || return 0 + mkdir -p "$tmp/docs/conventions" && printf '%s\n' "$out" >"$tmp/docs/conventions/source-control.yaml" + on=$(bash "$CONVENTION_KEY" value "$tmp" sync_stale_base) || + echo "push-branch.sh: $1/$d:docs/conventions/source-control.yaml does not parse; sync_stale_base read as false" >&2 + rm -rf "$tmp" + echo "$on" +} + # sync_stale_base : the sync-stale-base step of the header. Returns # 0 to push; exits 4 on a conflict. Nothing the pushed head holds decides # whether it runs or is run by it: the setting is read from the base -# repository's default branch and the overlap script from the base ref. +# repository's default branch and the overlap script from the base ref. With +# the setting unset everywhere this clone has fetched, it makes no network call. sync_stale_base() { local wd="$1" on="$SYNC" base="${PR_BASE_REF:-}" base_repo="${PR_BASE_REPO:-}" remote="$FETCH_REMOTE" - local tmp out ref rc mb tip paths_base paths_head overlap default sign=() + local tmp out ref rc mb tip paths_base paths_head overlap r sign=() [[ "$on" != false ]] || return 0 - if [[ -n "$base_repo" ]]; then - remote=$(cd "${wd:-.}" && remote_of_repo "$base_repo") - fi - if [[ -z "$on" ]]; then - [[ -n "$remote" ]] || return 0 - default=$(git symbolic-ref --short "refs/remotes/$remote/HEAD" 2>/dev/null) - default="${default#"$remote"/}" - [[ -n "$default" ]] || default=$(remote_default "$remote") - out=$(git show "refs/remotes/$remote/$default:docs/conventions/source-control.yaml" 2>/dev/null) || return 0 - tmp=$(mktemp -d) || return 0 - mkdir -p "$tmp/docs/conventions" && printf '%s\n' "$out" >"$tmp/docs/conventions/source-control.yaml" - on=$(bash "$CONVENTION_KEY" value "$tmp" sync_stale_base) || - echo "push-branch.sh: $remote/$default:docs/conventions/source-control.yaml does not parse; sync_stale_base read as false" >&2 - rm -rf "$tmp" - [[ "$on" == true ]] || return 0 + [[ -z "$base_repo" ]] || remote=$(cd "${wd:-.}" && remote_of_repo "$base_repo") + if [[ -z "$on" && -z "$PR" ]]; then + # The base repository is not known yet: it takes a forge read, made only + # when some remote's fetched default branch turns the setting on. + for r in $(cd "${wd:-.}" && git remote); do + [[ "$(cd "${wd:-.}" && setting_at "$r")" != true ]] || on=pending + done + [[ -n "$on" ]] || return 0 + elif [[ -z "$on" ]]; then + [[ -n "$remote" && "$(cd "${wd:-.}" && setting_at "$remote")" == true ]] || return 0 + on=true fi [[ -n "$wd" ]] || { sync_skip "'$BRANCH' is checked out in no worktree" @@ -383,7 +414,12 @@ sync_stale_base() { read -r base base_repo <<<"${out//$'\r'/}" remote=$(cd "$wd" && remote_of_repo "$base_repo") elif [[ "$out" == *"no pull requests found"* ]]; then - base=$(cd "$wd" && remote_default "$FETCH_REMOTE") + base=$(cd "$wd" && local_default "$FETCH_REMOTE") + [[ -n "$base" ]] || base=$(cd "$wd" && remote_default "$FETCH_REMOTE") + fi + # With no remote for the base repository, the skip below says so. + if [[ "$on" == pending && -n "$remote" ]]; then + [[ "$(cd "$wd" && setting_at "$remote")" == true ]] || return 0 fi fi [[ -n "$remote" ]] || { diff --git a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh index 025ecd6e8e..90e06f7a73 100755 --- a/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/push-branch.test.sh @@ -76,6 +76,7 @@ case "$1 $2" in "repo view") echo "example-org/app" ;; "pr view") if [[ " $* " == *" baseRefName,url "* ]]; then + echo base-read >>"$PB_GH_LOG" [[ -n ${PB_PR_BASE:-} ]] || { echo "no pull requests found for branch \"$3\"" >&2 exit 1 @@ -94,6 +95,7 @@ cat >"$SHIM/git" <>"$LOG/fetch.log" + [[ \$a != ls-remote ]] || printf '%s\n' "\$*" >>"$LOG/ls-remote.log" if [[ \$a == push && -n \${PB_RECORD_PUSH:-} ]]; then printf '%s\n' "\$@" >"$LOG/push.argv" pwd -P >"$LOG/push.cwd" @@ -108,6 +110,7 @@ done exec "$REAL_GIT" "\$@" EOF chmod +x "$SHIM/gh" "$SHIM/git" +export PB_GH_LOG="$LOG/gh.log" export PATH="$SHIM:$PATH" # push-branch.sh runs from a copy of the plugin's layout, so the lane identity @@ -795,6 +798,66 @@ check "sync, fork head: exit 0" "$RC" "0" check "sync, fork head: fetched the base from upstream" "$(grep -c 'upstream +refs/heads/trunk:' "$LOG/fetch.log")" "1" check "sync, fork head: merged upstream's trunk" "$(git -C "$wc" rev-parse HEAD^2)" "$(git -C "$WORKDIR/sync-fork/upstream.git" rev-parse trunk)" +# Without --pr, the setting is read from the PR's base repository, which the +# forge names, not the fetch remote. Here origin is the fork and upstream the +# base. A fork that turns the setting on, where upstream does not, syncs +# nothing; upstream turning it on, where the fork does not, syncs from upstream. +# fork_base_clone : adds the base repository +# `upstream` (the setting on for yes) beside the fork `origin`, then moves +# upstream's trunk on c.txt so the branch is stale on an overlapping path. +fork_base_clone() { + local wc="$1" up_on="$2" up + git init -q --bare "$WORKDIR/$3/upstream.git" + up="$WORKDIR/$3/upseed" + git clone -q "$WORKDIR/$3/upstream.git" "$up" 2>/dev/null + git -C "$up" config user.email up@example.com + git -C "$up" config user.name Up + git -C "$up" checkout -q -b trunk + git -C "$wc" show HEAD:c.txt >"$up/c.txt" + if [[ $up_on == yes ]]; then + mkdir -p "$up/docs/conventions" + echo 'sync_stale_base: true' >"$up/docs/conventions/source-control.yaml" + fi + git -C "$up" add -A + git -C "$up" commit -q -m upstream + git -C "$up" push -q origin trunk 2>/dev/null + git -C "$WORKDIR/$3/upstream.git" symbolic-ref HEAD refs/heads/trunk + git -C "$wc" remote add upstream "https://github.com/$3/upstream.git" + git -C "$wc" remote set-url origin "https://github.com/$3/origin.git" + PB_GITHUB_ROOT="$WORKDIR" git -C "$wc" fetch -q upstream + git -C "$wc" merge -q --allow-unrelated-histories -X ours --no-edit upstream/trunk >/dev/null 2>&1 + set_line "$wc" c.txt 1 branch-one + set_line "$up" c.txt 10 upstream-ten + git -C "$up" push -q origin trunk 2>/dev/null +} +wc=$(sync_clone sync-fork-on yes) +fork_base_clone "$wc" no sync-fork-on +PB_GITHUB_ROOT="$WORKDIR" PB_PR_BASE=trunk PB_PR_BASE_REPO=sync-fork-on/upstream sync_run "$wc" +check "sync, fork turns it on, base does not: exit 0" "$RC" "0" +check "sync, fork turns it on, base does not: nothing fetched" "$(fetched)" "none" +check "sync, fork turns it on, base does not: HEAD unchanged" "$(git -C "$wc" rev-parse HEAD)" "$OLD_TIP" +wc=$(sync_clone sync-base-on no) +fork_base_clone "$wc" yes sync-base-on +PB_GITHUB_ROOT="$WORKDIR" PB_PR_BASE=trunk PB_PR_BASE_REPO=sync-base-on/upstream sync_run "$wc" +check "sync, base turns it on, fork does not: exit 0" "$RC" "0" +check "sync, base turns it on, fork does not: fetched from upstream" "$(grep -c 'upstream +refs/heads/trunk:' "$LOG/fetch.log")" "1" +check "sync, base turns it on, fork does not: merged upstream's trunk" "$(git -C "$wc" rev-parse HEAD^2)" "$(git -C "$WORKDIR/sync-base-on/upstream.git" rev-parse trunk)" + +# With the setting unset everywhere the clone has fetched, the step makes no +# network call: no forge read of the base, no fetch, and no ls-remote beyond +# the default-branch check every push makes, even with a remote that has no +# refs/remotes//HEAD. +wc=$(sync_clone sync-quiet no) +git -C "$wc" remote add upstream "$WORKDIR/sync-quiet/origin.git" +git -C "$wc" fetch -q upstream +set_line "$wc" c.txt 1 branch-one +advance_base sync-quiet c.txt 10 base-ten +PB_PR_BASE=trunk sync_run "$wc" +check "sync, off everywhere: exit 0" "$RC" "0" +check "sync, off everywhere: no forge read of the base" "$(grep -c . "$LOG/gh.log" 2>/dev/null || echo 0)" "0" +check "sync, off everywhere: nothing fetched" "$(fetched)" "none" +check "sync, off everywhere: one ls-remote (the default-branch check)" "$(wc -l <"$LOG/ls-remote.log" | tr -d ' ')" "1" + # Nothing the pushed head holds decides the step or runs in it. A head that # turns sync_stale_base on, where the base's config does not, fetches nothing. wc=$(sync_clone sync-head-config no)