feat(automation): guard prepare-pr pushes against stale merge bases

[tlobinger]

Problem

A force-push automation loop can resolve upstream/main to a stale local ref when the fetch step fails silently or the worktree was branched from an integration trunk carrying unshipped commits. When this happens, the force-push replays dozens of already-merged commits onto the remote feature branch, clobbering maintainer work and requiring a manual restore.

Why it matters

Every automated rebase/push flow in the prepare-pr skill is vulnerable to this failure mode. Without a pre-push guard, a single network hiccup or stale worktree can destroy upstream progress on a feature branch.

Fix

Add a three-check pre-push guard (push_guard.py) to the prepare-pr skill:

1. Fetch must succeed — fail closed on any network/auth error rather than operating on a potentially stale origin/<base> ref.
2. Merge-base ancestry — verify the merge-base of HEAD and the freshly-fetched origin/<base> is actually an ancestor of origin/<base> (catches branches forked from local integration trunks).
3. Commit count ceiling — refuse when HEAD is implausibly far ahead of origin/<base> (default threshold: 5 commits for a single-commit PR workflow).

Also hardens preflight.py to fail closed when git fetch fails and adds a high-commit-count warning.

The SKILL.md documentation is updated to integrate the guard into the Phase 3 push flow.

Tests

• test/test_push_guard.py — 8 tests covering:
  • Normal single-commit PR (safe, exit 0)
  • At-threshold commit count (safe)
  • Too many commits ahead (refused, exit 40)
  • Merge-base not ancestor of origin (refused)
  • Fetch failure (refused — fail closed)
  • Full clobber scenario reproduction (10 local-only commits + 1 fix = 11 ahead → refused)
  • Not a git repo (exit 2)
  • Custom --max-ahead threshold

Summary

This PR combines 8 related changes:

• Find and fix how a pr-maintenance rebase loop resolved upstream/main to a stale mirror (PR 918 clobber)
• prepare-pr guard: gate on single_commit, fix vacuous ancestry claim, detect replayed commits
• Republish PR 1418: push guard-fix commit onto feature/stale-base-rebase-root-cause
• PR 1418: fix reversed ancestry check in push_guard pre-squash mode
• PR 1418: redact query-string credentials + register redactor call sites in the posture allowlist
• PR 1418: rebuild branch from the 6 intended files — stale-tree squash reverted ~104 merged files
• PR 1418: redact dotless/short-host and bracketed scp userinfo
• push_guard: pin base fetch refspec so narrow clones cannot validate a stale tip

Changes and rationale

Find and fix how a pr-maintenance rebase loop resolved upstream/main to a stale mirror (PR 918 clobber)

Problem: On 2026-07-31 22:24 UTC the loop pr-918-fix-rebase-preexec-conflict force-pushed feature/registry-ssh-url-parity from a STALE base: merge-base d6d8a0d instead of current upstream/main, carrying 114 duplicate re-creations of already-merged PRs (e.g. #971 exists on main as c4a1f68, appeared on the branch as f5946fe). The parent commit 509551b existed on NO shared branch. The push clobbered the maintainer's admission fix and cost a manual restore, a dismissed approval, and several CI rounds. Full incident…
Why it matters: Any future pr-maintenance loop that rebases and force-pushes a feature/* branch can clobber human work on an upstream PR the same way. This is the highest-blast- radius failure mode our automation has exhibited: it destroys maintainer commits on a repo where we hold direct write access.
Fix (symptoms → root cause → change): Chain from symptom to root cause before changing anything: Reconstruct where the stale ref came from: inspect the pr-maintenance loop worktree creation path and every place the automation resolves 'upstream/main' (worktree-local refs, an unfetched upstream remote, the local-origin.git bare mirror, or a clone made from kiki-trunk state). Commits 509551b / d6d8a0d may still exist locally — 'git cat-file -t' and 'git branch --contains' across worktrees/mirrors to locate the stale copy. Fix the resolution so…
Tests: Unit/integration test proving a push candidate with a stale merge-base is refused before any network push. Existing pr-maintenance flow tests still pass. No behavior change for the normal single-commit fast path.
Review focus: automation, security boundaries, regression coverage

prepare-pr guard: gate on single_commit, fix vacuous ancestry claim, detect replayed commits

Tests: Regression pinning finding 1 is prose-only (SKILL.md), no test needed — but keep all 12 existing tests green after the rename. New: replayed-commit refusal — branch whose ahead-commits include a patch-equivalent of an upstream commit (cherry-picked/replayed onto a stale fork point, <=5 total so the count check alone passes) → exit 40. Fails at 221c7a8…
Review focus: automation

Republish PR 1418: push guard-fix commit onto feature/stale-base-rebase-root-cause

Review focus: pr-maintenance

PR 1418: fix reversed ancestry check in push_guard pre-squash mode

Review focus: pr-maintenance

PR 1418: redact query-string credentials + register redactor call sites in the posture allowlist

Review focus: pr-maintenance

PR 1418: rebuild branch from the 6 intended files — stale-tree squash reverted ~104 merged files

Review focus: pr-maintenance

PR 1418: redact dotless/short-host and bracketed scp userinfo

Review focus: pr-maintenance

push_guard: pin base fetch refspec so narrow clones cannot validate a stale tip

Tests: Regression pinning finding 1: repo with a narrow remote.origin.fetch refspec that excludes the base (e.g. set remote.origin.fetch to a non-base branch), base advances on the remote, branch sits on the OLD tip -> guard must REFUSE (exit 40) after the fix (fails at 70cafcc: guard passes because origin/ never updates). Normal full-refspec repo…
Review focus: pr-maintenance

Review map

Change	Primary files
Find and fix how a pr-maintenance rebase loop resolved upstream/main to a stale mirror (PR 918 clobber)	—
prepare-pr guard: gate on single_commit, fix vacuous ancestry claim, detect replayed commits	src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/SKILL.md src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/preflight.py test/test_push_guard.py
Republish PR 1418: push guard-fix commit onto feature/stale-base-rebase-root-cause	src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/SKILL.md src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/preflight.py test/test_push_guard.py test/test_pip_deps_consistency.py
PR 1418: fix reversed ancestry check in push_guard pre-squash mode	src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py test/test_push_guard.py
PR 1418: redact query-string credentials + register redactor call sites in the posture allowlist	src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/preflight.py src/kiro_crew/security_posture.py test/test_push_guard.py
PR 1418: rebuild branch from the 6 intended files — stale-tree squash reverted ~104 merged files	src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/SKILL.md src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/preflight.py src/kiro_crew/security_posture.py test/test_push_guard.py +1 more
PR 1418: redact dotless/short-host and bracketed scp userinfo	src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/preflight.py test/test_push_guard.py
push_guard: pin base fetch refspec so narrow clones cannot validate a stale tip	src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/preflight.py test/test_push_guard.py

Commits

• 72507ff fix(push_guard): redact SCP userinfo for dotless/IPv6 hosts; Windows-…

Validation

Local build, static-analysis, and test gates completed before publication. Upstream CI and review checks on the current PR revision remain the authoritative merge signal.

Changed files (6 files, +2278/-37)

• src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/SKILL.md (+9/-7)
• src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/preflight.py (+48/-8)
• src/kiro_crew/builtin_skills/kirocrew-dev/prepare-pr/scripts/push_guard.py (+467/-0)
• src/kiro_crew/security_posture.py (+8/-1)
• test/test_pip_deps_consistency.py (+29/-21)
• test/test_push_guard.py (+1717/-0)

This description updates automatically as related changes land; manually written text outside this block is preserved.

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Advisory design-level review of 18b6fce35c7c4c883330eb16057c6536e46ed2c3 — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS

Sound root-cause fix, but the guard binds to the push only by SKILL.md prose — the automation that failed must voluntarily run it.

Watch

• The clobber happened because an automation loop skipped/ignored a step; this fix adds more steps ("run push_guard.py now", "Record LEASE_SHA=... at iteration start") enforced only by prose in SKILL.md. Nothing couples the check to the push itself, so the same non-compliant loop can still push unguarded. A single guarded_push.py that fetches, checks, and executes the SHA-pinned push atomically would make the guard un-skippable.
• Description↔diff drift: the summary block claims "redact query-string credentials + register redactor call sites in the posture allowlist" and "redact dotless/short-host and bracketed scp userinfo", but the final diff contains no scrubber (replaced by the _classify_fetch_error allowlist; security_posture.py is quote-style-only). Refresh the body so reviewers aren't auditing removed code.
• test_pip_deps_consistency.py now excludes all of builtin_skills/ to accommodate one sibling import — the deps gate silently stops covering every future bundled-skill script, not just this pair. Scope the exclusion to prepare-pr/scripts/.

Suggestions

• Replace the ~150-line double diff-tree/patch-id loop (≈1000 git subprocesses per run at the 500-commit window) with git cherry origin/<base> HEAD or git rev-list --cherry-mark, which compute patch-equivalence natively in one call.

[DESIGN-REVIEWED] 18b6fce

[github-actions]

Opus 5 Review — ✅ no blocking findings

Reviewed 18b6fce35c7c4c883330eb16057c6536e46ed2c3 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 18b6fce

Verdict parsed from the review's SHA-scoped output markers for commit 18b6fce35c7c4c883330eb16057c6536e46ed2c3.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 18b6fce35c7c4c883330eb16057c6536e46ed2c3: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 18b6fce35c7c4c883330eb16057c6536e46ed2c3 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 18b6fce

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 18b6fce35c7c4c883330eb16057c6536e46ed2c3: <one-sentence reason>

[tlobinger]

Addressed GPT 5.6 blocking finding (SKILL.md:120 guard-after-squash):

The guard now runs at two points in the workflow:

1. Phase 1.3 (pre-squash): After git rebase origin/<base> and BEFORE git reset --soft squash — at this point rev-list --count still reflects the branch's real history, so a polluted trunk (100+ replayed commits from a stale local integration branch) trips --max-ahead and is refused.

2. Phase 3.1 (post-squash, --require-single-on-base): Asserts HEAD~1 == origin/<base> after a fresh fetch — the only meaningful post-squash invariant since commit count is always 1 after squash. Catches the case where the squash landed on a stale ref.

Additional fixes:

• Removed the tautological ancestry check (git merge-base --is-ancestor merge_base origin/<base> is true by definition since merge-base IS the common ancestor); replaced with a common-history existence check that still catches orphan branches.
• Added 4 regression tests pinning both failure modes (12 total, all green).
• All existing 8 tests remain green.

Rebased onto fresh upstream/main (b15c412). One commit, 4-file diff, merge-tree clean.

[tlobinger]

Ancestry check now asserts origin/<base> is an ancestor of HEAD (not the vacuous merge-base test). Recovery text in _check_single_on_base prescribes rebase before re-squash rather than bare reset --soft.

[tlobinger]

Re-squashed on ae859c7 (current main) — preserves the _free_ports autouse fixture from PR #1626 that the previous squash (4b1f08a, based on d11c89a) inadvertently removed. No code change to the PR's own files; same 6-file diff. URL redaction now strips everything after the authority (scheme+host), closing path-embedded credentials along with userinfo and query strings; diagnostics keep scheme+host and the git error class.

[tlobinger]

Pushed fd85c9c: credential redaction now covers bracketed IPv6 authorities ([::1], zone-ids like %25eth0) and query-only URLs (remainder starts at /, ?, or #); 30-case RFC 3986 authority-shape matrix pins the coverage. CodeQL bare-substring test assertion (py/incomplete-url-substring-sanitization alert #569) precised to exact redacted-form check. The unintended test_instances.py reversion of #1626's _free_ports fixture remains absent from this push (file not in diff). Squashed on fresh upstream/main (a09374c).

[tlobinger]

Phase-1 sync fetch now uses an explicit +refs/heads/<base>:refs/remotes/origin/<base> refspec for every profile, so narrow-refspec clones cannot rebase onto a stale base in multi-commit workflows. No script changes — push_guard.py and preflight.py already use explicit refspecs internally.

[github-actions]

UX Review (Fable 5) — ⏭️ skipped

Revision 18b6fce35c7c4c883330eb16057c6536e46ed2c3 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[tlobinger]

The previous push (5a7b535) accidentally squashed a stale worktree tree onto main, reverting ~104 merged files. This force-push rebuilds the branch on fresh upstream/main (48a8765) with the identical 6-file change set — no functional change to the reviewed content.

[tlobinger]

Fetch diagnostics now redact before truncating — a slice can no longer split a credential out of regex reach (redact_credentials(fetch_err)[:300] instead of redact_credentials(fetch_err[:300])). The two unrelated security_posture.py description hunks (Slack session mirror revert + line-109 quote churn) are dropped; diff is now NON_EGRESS additions only. Split-boundary regression test added.

[tlobinger]

Pushed 44a9fe6: fetch diagnostics now also redact scheme-less scp-style userinfo (user@host:path), closing the last URL-credential shape class. Hosts remain visible for diagnostic value.

[tlobinger]

SCP userinfo redaction (pass 3) now covers dotless colon-path hosts (tok@forge:path), bracketed IPv6 (tok@[::1]:path), and un-bracketed IPv6 as echoed by SSH (tok@::1:). Hosts stay visible in diagnostic output. Fake-SSH test fixture rewritten as Python for Windows portability.

[tlobinger]

Content-identical rebase onto current main (6415868) so CI evaluates against the merged fixes for the four inherited lanes (docker-detection fixtures #1923, js-yaml audit override, ja catalog parity #1939). No code change — 6 files byte-identical to previous head 72507ff.

[tlobinger]

Rebased onto fresh main (5cb9801). Post-authority URL redaction (_URL_AFTER_AUTHORITY_RE) now uses a generic RFC 3986 scheme pattern ([a-zA-Z][a-zA-Z0-9+.\-]*://) instead of the enumerated https?|ssh|git alternation, closing ftp/ftps and every other scheme-bearing URL in one pass. Hosts remain visible in diagnostics. Regression tests pin ftps path-token, ftp query-token, and svn+ssh exotic-scheme cases.

[tlobinger]

Force-pushed to bee1a6b00 (1 commit ahead of main ecadd7ff).

Fixes in this push:

1. security_posture.py restored — the previous rebase accidentally carried a pre-drift version of this file, reverting all 9 auto_improvement _REDACTION_SINKS registrations and the push_policy.py NON_EGRESS entry. Rebuilt from fresh main + the intended two-entry prepare-pr delta (push_guard.py + preflight.py NON_EGRESS entries).

2. push_guard.py replay check fails closed — every git-error path (rev-list, diff-tree, patch-id) now exits 40 with a named diagnostic. Only genuinely-successful empty outputs (no ahead commits, or empty diff for a skip-this-commit) remain SAFE. Prevents the fail-open class where a transient lazy-fetch error on partial clones would skip the clobber-prevention check.

3. CodeQL ci: present AI reviewers by model name (hide vendor/harness branding) #577-App Store: move trust/provenance derivation from client to /api/apps/registry #580 cleared — the four test assertions that flagged py/incomplete-url-substring-sanitization now assert exact redacted URL forms (e.g. ftps://git.example.com/<redacted>) instead of bare host substrings.

Verification: pytest push_guard + security_posture + pip_deps suites pass (144 tests); black/isort/flake8/mypy clean; brand gate clean; Auto-Improvement count matches main (9); push_guard.py self-test passes on this commit.

[tlobinger]

Rebased to adc164d60: the TestReplayFailClosed fake-git wrappers now use _PUSH_GUARD_GIT_CMD env var override (a Python-script-direct approach) instead of PATH-prepended shell launchers. This fixes the three replay-check regression tests on Windows CI where .bat/.cmd files cannot be launched by CreateProcess without cmd.exe, and shebangs/colon PATH separators don't exist. The run() helper also gains a shutil.which fallback for production use. No behavioral change to the guard's fail-closed contract.

[tlobinger]

Force-pushed 4c18522bc (was adc164d60): the four CodeQL-flagged assertions in TestSchemeAgnosticRedaction (lines 1697/1723/1757/1768) now assert exact redacted URL forms instead of host substrings — same fix class as #568/#569. Clears CodeQL alerts #577–#580.

No other changes; the TestReplayFailClosed _make_fake_git helper was already Windows-compatible in the previous push (uses _PUSH_GUARD_GIT_CMD env var + sys.executable, no PATH manipulation or shell shebangs).

[tlobinger]

Round 12+13 fix pushed (head 7905efbe9, squashed on fresh 984e0b18):

1. IPv6 userinfo composition (GPT round-12): _URL_AFTER_AUTHORITY_RE pass-2 authority alternation now accepts optional userinfo — including the pass-1 <redacted>@ rewritten form — before both bracketed-IPv6 and reg-name host patterns. Path/query tokens after credentialed [::1] origins are now redacted.

2. SHA-pinned force-with-lease (GPT round-12): SKILL.md Phase 3 now records LEASE_SHA before fetch, requires merge-base --is-ancestor origin/<branch> HEAD before any rewrite, and pushes with the explicit --force-with-lease=<branch>:<sha> form. No implicit lease remains.

3. Derived fetch diagnostics (GPT round-13): Raw fetch_err passthrough removed from both push_guard _fetch_base and preflight fetch-failure paths. Replaced with a classifier that emits the matched git error class + redacted scheme://host; unrecognized free text is withheld (fetch failed (details withheld)).

4. SAST env-to-argv fix: Removed _PUSH_GUARD_GIT_CMD env-derived subprocess.run argv (Semgrep dangerous-subprocess-use-tainted-env-args). Guard now uses a module-level _GIT_CMD: list[str] that tests monkeypatch directly — no env injection, no PATH wrappers. TestReplayFailClosed runs on both Windows and POSIX.

5. security_posture.py: Unchanged from prior head (two prepare-pr NON_EGRESS entries; Auto-Improvement count == 9, matching fresh main).

All 116 test_push_guard tests pass locally; black/isort/flake8 clean; brand gate green; merge-tree clean.

[tlobinger]

Amended push (head 2769076bf, supersedes 7905efbe9):

Additional fix folded in: Derived fetch diagnostics (GPT round-13) — raw fetch_err passthrough removed from both push_guard _fetch_base and preflight fetch-failure paths. Replaced with _classify_fetch_error that matches stderr against an allowlist of known git fetch failure classes and returns only the hardcoded label. Unrecognized free text is withheld entirely (fetch failed (details withheld — run git fetch manually to see the error)). This closes the free-text credential egress class that shape-enumeration cannot converge on.

14 new tests added: TestClassifyFetchError (12 unit tests covering all known classes + bare-token/free-text withholding + empty stderr) and TestFetchDiagnosticIntegration (2 integration tests verifying the monkeypatched fetch-failure path never leaks tokens). Full suite: 130 tests pass.

[tlobinger]

Push a331766: remove stale preflight.py entry from NON_EGRESS_REDACTION_MODULES — the round-13 derived-diagnostic fix delegates to push_guard._classify_fetch_error() so preflight no longer calls any redactor directly. Fixes test_allowlist_has_no_stale_entries (Backend Tests shard 3 failure at 2769076).

[tlobinger]

Pushed 7e302f5 (rebased onto fresh main 7e1f744).

Changes at this head:

1. Dead scrubber removed. redact_credentials, its three URL regexes (_URL_USERINFO_RE, _URL_AFTER_AUTHORITY_RE, _SCP_USERINFO_RE), and all unit tests exercising them are deleted — the function had zero call sites after round 13 replaced both scripts' fetch-error paths with the derived _classify_fetch_error classifier. The stale push_guard.py entry in NON_EGRESS_REDACTION_MODULES is also removed (resolves the deterministic CI failure on test_allowlist_has_no_stale_entries).

2. SKILL.md Phase 3 step 1 — ancestry check fixed. The pre-push maintainer-clobber check now runs pre-squash (against the pre-squash HEAD which still carries prior-push history), is skipped entirely on first push (when origin/<branch> does not exist), and is explicitly not re-run after the squash. The SHA-pinned --force-with-lease remains as the at-push protection against concurrent pushes.

Closes the GPT round-14/15 blocking findings (dead-code incoherence + post-squash ancestry guard that always rejects).

[tlobinger]

Content-identical rebase onto current main (baa524e) so CI evaluates against the merged #2049 fixture fix that the failing Windows shard 2 inherits; no code change.