From d18695c144939eb483afe94fee5ccdf577aea708 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:08:57 -0400 Subject: [PATCH 01/10] fix(guardrails): detect the new ghs__ GitHub App token format GitHub began issuing installation tokens as ghs__ (about 520 characters) on 2026-04-27. The old patterns required 36 or 20+ alphanumerics right after the prefix and stopped at the underscore after the app ID, so the new tokens went undetected or unredacted. Covers guardrails secret detection, the harness-config audit SECRET_RE, harness-ops machine-profile, and the disk-hygiene and session-flow redaction sites, each with a test that failed before the change. Co-Authored-By: Claude Opus 5.5 --- .../disk-hygiene/.claude-plugin/plugin.json | 2 +- plugins/disk-hygiene/CHANGELOG.md | 8 +++++ .../disk-hygiene/lib/guard_decision_log.py | 5 +++- .../lib/test_guard_decision_log.py | 11 +++++++ plugins/guardrails/.claude-plugin/plugin.json | 2 +- plugins/guardrails/CHANGELOG.md | 9 ++++++ .../hooks/secret-pattern-detection.test.sh | 16 ++++++++++ .../lib/secret-detection/secret-patterns.sh | 30 +++++++++++-------- .../harness-config/.claude-plugin/plugin.json | 2 +- plugins/harness-config/CHANGELOG.md | 6 ++++ .../skills/audit/scripts/audit-engine.sh | 2 +- .../skills/audit/scripts/audit-engine.test.sh | 11 +++++++ .../skills/machine-profile/scripts/profile.sh | 2 +- .../machine-profile/scripts/profile.test.sh | 1 + .../session-flow/.claude-plugin/plugin.json | 2 +- plugins/session-flow/CHANGELOG.md | 9 ++++++ plugins/session-flow/scripts/save_point.py | 8 ++++- .../scripts/tests/test_save_point.py | 21 +++++++++++++ .../skills/running-retro/scripts/observer.py | 8 ++++- .../running-retro/scripts/test_observer.py | 6 ++++ 20 files changed, 139 insertions(+), 22 deletions(-) diff --git a/plugins/disk-hygiene/.claude-plugin/plugin.json b/plugins/disk-hygiene/.claude-plugin/plugin.json index fd49616963..4b9d702163 100644 --- a/plugins/disk-hygiene/.claude-plugin/plugin.json +++ b/plugins/disk-hygiene/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "disk-hygiene", - "version": "0.42.9", + "version": "0.42.10", "description": "Context-aware disk hygiene for arbitrary directory trees: inventories orphaned and temporary artifacts, classifies evidence into review tiers, and offers exact-path cleanup only after a fresh safety preview and explicit per-tier approval. The target is read-only by default; OS-managed paths, links and mount points, VCS-tracked content without the complete checkout evidence bundle, changed entries, and live-handle uncertainty fail closed.", "author": { "name": "Melodic Software", diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index 45d66b254a..a5f853a825 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.42.10] - 2026-10-02 + +### Fixed + +- The guard decision log redacts GitHub App installation tokens in the `ghs__` format + GitHub rolls out from 2026-04-27. The old pattern stopped at the `_` after the app ID, so the + token was written to the log in full. + ## [0.42.9] - 2026-10-02 ### Changed diff --git a/plugins/disk-hygiene/lib/guard_decision_log.py b/plugins/disk-hygiene/lib/guard_decision_log.py index 3abe66dc4b..f103fff51d 100644 --- a/plugins/disk-hygiene/lib/guard_decision_log.py +++ b/plugins/disk-hygiene/lib/guard_decision_log.py @@ -94,7 +94,10 @@ re.DOTALL, ), re.compile(r"\b(?:sk|rk|pk)-[A-Za-z0-9_-]{16,}"), - re.compile(r"\bgh[pousr]_[A-Za-z0-9]{20,}"), + re.compile( + r"\b(?:ghs_[0-9]+_[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2}" + r"|gh[pousr]_[A-Za-z0-9]{20,})" + ), re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{10,}"), re.compile(r"\bAKIA[0-9A-Z]{16}\b"), re.compile(r"\beyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}"), diff --git a/plugins/disk-hygiene/lib/test_guard_decision_log.py b/plugins/disk-hygiene/lib/test_guard_decision_log.py index 82131d3f56..c326673a9b 100755 --- a/plugins/disk-hygiene/lib/test_guard_decision_log.py +++ b/plugins/disk-hygiene/lib/test_guard_decision_log.py @@ -143,6 +143,17 @@ def test_secret_shaped_command_and_reason_are_redacted_before_clip(self) -> None self.assertIn(decision_log.REDACTED, entry["command"]) self.assertIn(decision_log.REDACTED, entry["reason"]) + def test_github_app_installation_token_jwt_form_is_redacted(self) -> None: + # ghs__, about 520 characters; the segments spell FAKE. + payload = "FAKEpayload" + ("A" * 450) + token = ( + "ghs" + "_1234567_FAKEheaderNOTaJWT." + payload + ".FAKEsignatureNOTreal" + ) + self.write_one(command="echo " + token) + (entry,) = self.read_records() + self.assertNotIn(payload[:40], entry["command"]) + self.assertEqual("echo " + decision_log.REDACTED, entry["command"]) + def test_none_and_deny_by_default_persist_length_not_command_text(self) -> None: secret = "$env:AZURE_CLIENT_SECRET='s3cretvalue'; Get-Process" self.write_one( diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index 73c57ed71d..0bc9ee275e 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -171,5 +171,5 @@ "min": 1 } }, - "version": "0.46.8" + "version": "0.46.9" } diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index 6e6aa0f937..fa016cb097 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to the `guardrails` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.46.9] - 2026-10-02 + +### Fixed + +- Secret detection catches GitHub App installation tokens in the `ghs__` format + GitHub rolls out from 2026-04-27 (about 520 characters, length varies). The new pattern matches + `ghs_`, a numeric app ID, `_`, and three dot-separated base64url segments; the 36-character + `ghs_`/`ghu_` form is still detected. + ## [0.46.8] - 2026-10-02 ### Changed diff --git a/plugins/guardrails/hooks/secret-pattern-detection.test.sh b/plugins/guardrails/hooks/secret-pattern-detection.test.sh index f688ec9050..e0ff8272c6 100755 --- a/plugins/guardrails/hooks/secret-pattern-detection.test.sh +++ b/plugins/guardrails/hooks/secret-pattern-detection.test.sh @@ -52,6 +52,12 @@ AWS_PREFIX='AKIA' AWS_TOKEN="${AWS_PREFIX}IOSFODNN7EXAMPLE" GH_PREFIX='ghp_' GH_PAT="${GH_PREFIX}$(printf 'a%.0s' {1..36})" +GHS_PREFIX='ghs_' +GH_APP_TOKEN="${GHS_PREFIX}$(printf 'b%.0s' {1..36})" +# The ghs__ installation-token format GitHub rolls out from +# 2026-04-27, about 520 characters: dot-separated JWT segments that spell +# FAKE and are not base64 JSON. +GH_APP_TOKEN_JWT="${GHS_PREFIX}1234567_FAKEheaderNOTaJWT.FAKEpayload$(printf 'A%.0s' {1..450}).FAKEsignatureNOTreal" SLACK_PREFIX='xoxb-' SLACK_TOKEN="${SLACK_PREFIX}1234567890123-9876543210987" STRIPE_PREFIX='sk_live_' @@ -79,6 +85,16 @@ RC=$? assert_exit "GitHub PAT → exit 2" 2 "$RC" assert_contains "GH PAT → message" "$OUT" "GitHub PAT" +OUT=$(bash "$HOOK" <<<"$(write_json "$FIXTURE" "token = '$GH_APP_TOKEN'")" 2>&1) +RC=$? +assert_exit "GitHub App token, 36-char form → exit 2" 2 "$RC" +assert_contains "GH App token, 36-char form → message" "$OUT" "GitHub App Token" + +OUT=$(bash "$HOOK" <<<"$(write_json "$FIXTURE" "token = '$GH_APP_TOKEN_JWT'")" 2>&1) +RC=$? +assert_exit "GitHub App token, ghs__ form → exit 2" 2 "$RC" +assert_contains "GH App token, ghs__ form → message" "$OUT" "GitHub App Token" + OUT=$(bash "$HOOK" <<<"$(write_json "$FIXTURE" "SLACK='$SLACK_TOKEN'")" 2>&1) RC=$? assert_exit "Slack Bot Token → exit 2" 2 "$RC" diff --git a/plugins/guardrails/lib/secret-detection/secret-patterns.sh b/plugins/guardrails/lib/secret-detection/secret-patterns.sh index 312741f3c8..c262eb0b47 100644 --- a/plugins/guardrails/lib/secret-detection/secret-patterns.sh +++ b/plugins/guardrails/lib/secret-detection/secret-patterns.sh @@ -7,7 +7,9 @@ # Callers handle I/O, exemptions, and exit-code mapping. # # Pattern selection: only HIGH-confidence patterns with distinctive prefixes -# and fixed lengths. Generic patterns (password=, api_key=, secret=) are +# and fixed lengths or a fixed structure (the ghs__ installation +# token varies in length, so it is matched by its dot-separated JWT segments). +# Generic patterns (password=, api_key=, secret=) are # excluded — too many false positives for a real-time blocking hook. Sourced # from gitleaks, TruffleHog, and secrets-patterns-db. grep -E (POSIX ERE) only. @@ -19,6 +21,7 @@ SECRET_LABELS=( "GitHub PAT" "GitHub OAuth Token" "GitHub App Token" + "GitHub App Token" "GitHub Fine-grained PAT" "GitLab PAT" "Slack Bot Token" @@ -29,18 +32,19 @@ SECRET_LABELS=( "Private Key (PEM)" ) SECRET_PATTERNS=( - '(AKIA|ASIA|ABIA|ACCA)[A-Z0-9]{16}' # AWS (AKIA/ASIA/ABIA/ACCA + 16) - 'ghp_[0-9a-zA-Z]{36}' # GitHub PAT - 'gho_[0-9a-zA-Z]{36}' # GitHub OAuth - 'gh[us]_[0-9a-zA-Z]{36}' # GitHub app (ghu_/ghs_) - 'github_pat_[0-9a-zA-Z_]{82}' # GitHub fine-grained PAT - 'glpat-[0-9a-zA-Z_-]{20}' # GitLab PAT - 'xoxb-[0-9]{10,13}-[0-9]{10,13}' # Slack bot token - 'xox[pe]-[0-9]{10,13}-' # Slack user/app token - '[sr]k_(test|live|prod)_[0-9a-zA-Z]{10,99}' # Stripe key - 'sk-(proj|svcacct|admin)-[A-Za-z0-9_-]{20,}' # OpenAI prefixed API key - 'sk-[A-Za-z0-9]{20,}' # OpenAI legacy bare sk- key - '-----BEGIN [A-Z ]*PRIVATE KEY-----' # PEM private key header + '(AKIA|ASIA|ABIA|ACCA)[A-Z0-9]{16}' # AWS (AKIA/ASIA/ABIA/ACCA + 16) + 'ghp_[0-9a-zA-Z]{36}' # GitHub PAT + 'gho_[0-9a-zA-Z]{36}' # GitHub OAuth + 'gh[us]_[0-9a-zA-Z]{36}' # GitHub app (ghu_/ghs_) + 'ghs_[0-9]+_[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+){2}' # GitHub app ghs__ + 'github_pat_[0-9a-zA-Z_]{82}' # GitHub fine-grained PAT + 'glpat-[0-9a-zA-Z_-]{20}' # GitLab PAT + 'xoxb-[0-9]{10,13}-[0-9]{10,13}' # Slack bot token + 'xox[pe]-[0-9]{10,13}-' # Slack user/app token + '[sr]k_(test|live|prod)_[0-9a-zA-Z]{10,99}' # Stripe key + 'sk-(proj|svcacct|admin)-[A-Za-z0-9_-]{20,}' # OpenAI prefixed API key + 'sk-[A-Za-z0-9]{20,}' # OpenAI legacy bare sk- key + '-----BEGIN [A-Z ]*PRIVATE KEY-----' # PEM private key header ) # secrets::scan_text diff --git a/plugins/harness-config/.claude-plugin/plugin.json b/plugins/harness-config/.claude-plugin/plugin.json index 65218cf346..b5a45b9fe9 100644 --- a/plugins/harness-config/.claude-plugin/plugin.json +++ b/plugins/harness-config/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "harness-config", - "version": "1.3.3", + "version": "1.3.4", "description": "Nine configuration-health skills (plus setup) for a repo's Claude Code configuration: audit (settings.json / .mcp.json / hooks / plugins / permissions drift), audit-automation-gaps (evidence-gated verdicts on automation gaps), audit-permission-grants (allow-rule / allowed-tools grants for auto-mode durability and portability), audit-permission-state (the permission rules actually in effect: every settings scope merged with per-rule provenance, what auto mode drops on entry, config written where nothing reads it, and which managed intents are enforced versus loosenable), draft-auto-mode-rules (interview and draft a paste-ready autoMode classifier block; prints only, never writes), audit-instructions (locally-owned instruction surfaces vs current model capability, proposing removals/rewrites of instructions the model no longer needs, and detecting cross-surface instruction conflicts), audit-prompting-postures (the additive lane: posture guidance the prompting guide says a component's purpose needs but the component does not carry), audit-pass (one coordinated, ordered, resumable pass over a named target: three-scope inventory, run-time-derived exclusion set, stable finding identity, suppression memory, resume, one human gate, delegating every check to the plugin that owns it), and unhobble (the empirical bare-baseline experiment: reversibly strip a repo's standing instructions, log real stumbles against the current model, re-add only what evidence earns). Boundary: harness-memory owns the health of CLAUDE.md, AGENTS.md, CLAUDE.local.md, .claude/rules/ and auto-memory (structure, size, placement, index integrity); harness-config audit-instructions judges whether instruction text across those files and skills, agents and hooks still fits the current model, and runs no memory-file hygiene checks.", "author": { "name": "Melodic Software", diff --git a/plugins/harness-config/CHANGELOG.md b/plugins/harness-config/CHANGELOG.md index 4c8d2a0a1d..f5150c3c3f 100644 --- a/plugins/harness-config/CHANGELOG.md +++ b/plugins/harness-config/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to the `harness-config` plugin are documented here. Format f Versions 0.51.8 to 0.51.9 and 0.51.11 to 0.51.14 were reserved by parallel branches and never released. +## [1.3.4] - 2026-10-02 + +### Fixed + +- The audit engine's secret-shape check (`SECRET_RE`, which flags a token in tracked `settings.json` and redacts hook commands) covers GitHub OAuth, user, server and refresh tokens (`gho_`, `ghu_`, `ghs_`, `ghr_`) and the `ghs__` installation-token format GitHub rolls out from 2026-04-27. Before, only `ghp_` and `github_pat_` were matched. + ## [1.3.3] - 2026-10-02 ### Changed diff --git a/plugins/harness-config/skills/audit/scripts/audit-engine.sh b/plugins/harness-config/skills/audit/scripts/audit-engine.sh index 00dd13bb01..3f33c42c80 100755 --- a/plugins/harness-config/skills/audit/scripts/audit-engine.sh +++ b/plugins/harness-config/skills/audit/scripts/audit-engine.sh @@ -1362,7 +1362,7 @@ fi # Token shapes that never belong in a claim or detail. Used here to redact a # hook command and in category F to flag a tracked settings value. -SECRET_RE='ghp_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|eyJ[A-Za-z0-9_-]{15,}\.[A-Za-z0-9_-]{5,}|sk-[A-Za-z0-9_-]{16,}|AKIA[0-9A-Z]{16}|xox[abp]-[A-Za-z0-9-]{10,}' +SECRET_RE='gh[pousr]_[A-Za-z0-9]{20,}|ghs_[0-9]+_[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+){2}|github_pat_[A-Za-z0-9_]{20,}|eyJ[A-Za-z0-9_-]{15,}\.[A-Za-z0-9_-]{5,}|sk-[A-Za-z0-9_-]{16,}|AKIA[0-9A-Z]{16}|xox[abp]-[A-Za-z0-9-]{10,}' resolve_hook_path() { # resolve_hook_path -> the first token with placeholders expanded diff --git a/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh b/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh index 039e6c322f..b6ea3a4d5f 100755 --- a/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh +++ b/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh @@ -535,6 +535,17 @@ assert_eq "case 11: undocumented var is info" "info" "$(jq -r '.findings[] | sel mkdir -p "$m/nodocs" out=$(DOCS_FIXTURE="$m/nodocs" run "$m" --json 2>&1) || true assert_eq "case 11: without the env-vars page the documentation row is not inspectable" "not-inspectable" "$(jq -r '.rows[] | select(.claim=="env-page-not-fetched:MY_SINK") | .status' <<<"$out")" +# GitHub app and OAuth tokens, including the ghs__ installation +# token format GitHub rolls out from 2026-04-27. Assembled at runtime so no +# token-shaped literal sits in this file; the JWT segments spell FAKE and are +# not base64 JSON, so only the GitHub rule can match them. +ghs_prefix='ghs_' +for tok in "${ghs_prefix}1234567_FAKEheaderNOTaJWT.FAKEpayload$(printf 'A%.0s' {1..450}).FAKEsignatureNOTreal" \ + "${ghs_prefix}$(printf 'b%.0s' {1..36})" "gho""_$(printf 'c%.0s' {1..36})" "ghu""_$(printf 'd%.0s' {1..36})"; do + printf '%s\n' "$CLEAN_SETTINGS" | jq --arg t "$tok" '. + {env:{GH:$t}}' >"$m/project/.claude/settings.json" + out=$(run "$m" --json --docs-dir "$m/docs" 2>&1) || true + assert_eq "case 11: ${tok:0:12}... is a secret-shaped error" "error" "$(jq -r '.findings[] | select(.identity.claim=="secret-shaped-value") | .severity' <<<"$out")" +done # Keys keep their tab-separated-values encoding, so a control character never # splits one key into two rows and existing claim identities stay stable. printf '%s\n' "$CLEAN_SETTINGS" | jq '. + {env:{"A\nB":"1"}}' >"$m/project/.claude/settings.json" # portability-ok: a JSON newline escape, not a regex escape diff --git a/plugins/harness-ops/skills/machine-profile/scripts/profile.sh b/plugins/harness-ops/skills/machine-profile/scripts/profile.sh index fdd998941d..390e5af225 100755 --- a/plugins/harness-ops/skills/machine-profile/scripts/profile.sh +++ b/plugins/harness-ops/skills/machine-profile/scripts/profile.sh @@ -30,7 +30,7 @@ command -v jq >/dev/null 2>&1 || die "jq is required" CRED_RE='token|secret|passw(?:or)?d|credential|api.?key|private.?key' # Value shapes the validator refuses in any string of a record: GitHub, AWS, Slack and # sk- API tokens, JWTs, private-key blocks, and a password embedded in a URL. -VAL_RE='gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|(?_` format GitHub rolls out from + 2026-04-27. The old pattern stopped at the `_` after the app ID, so such a token was neither + redacted nor warned about. + ## [0.44.5] - 2026-10-02 ### Changed diff --git a/plugins/session-flow/scripts/save_point.py b/plugins/session-flow/scripts/save_point.py index ae0e1b8759..14521340e3 100755 --- a/plugins/session-flow/scripts/save_point.py +++ b/plugins/session-flow/scripts/save_point.py @@ -226,7 +226,13 @@ SECRET_SHAPES: tuple[tuple[re.Pattern[str], str], ...] = ( (re.compile(r"-----BEGIN[^-]+PRIVATE KEY-----"), "private key"), (re.compile(r"\b(?:sk|rk|pk)-[A-Za-z0-9_-]{16,}"), "API key"), - (re.compile(r"\bgh[pousr]_[A-Za-z0-9]{20,}"), "GitHub token"), + ( + re.compile( + r"\b(?:ghs_[0-9]+_[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2}" + r"|gh[pousr]_[A-Za-z0-9]{20,})" + ), + "GitHub token", + ), (re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{10,}"), "Slack token"), (re.compile(r"\bAKIA[0-9A-Z]{16}\b"), "AWS key id"), ( diff --git a/plugins/session-flow/scripts/tests/test_save_point.py b/plugins/session-flow/scripts/tests/test_save_point.py index 4836287f5f..bb85fb4d09 100644 --- a/plugins/session-flow/scripts/tests/test_save_point.py +++ b/plugins/session-flow/scripts/tests/test_save_point.py @@ -455,6 +455,27 @@ def test_validate_secret_shape_is_warn_only(tmp_path): ) +def test_validate_flags_a_github_app_installation_token_in_jwt_form(tmp_path): + handoffs = materialize(tmp_path, "good-chain") + target = handoffs / HOP1 + text = target.read_text(encoding="utf-8") + # ghs__, about 520 characters; the segments spell FAKE. + token = ( + "ghs" + + "_1234567_FAKEheaderNOTaJWT.FAKEpayload" + + "A" * 450 + + ".FAKEsignatureNOTreal" + ) + text = text.replace( + "None. Nothing waits on a person or an access grant.", + f"None. The token {token} was rotated.", + ) + target.write_text(text, encoding="utf-8", newline="\n") + result = run("validate", str(target), "--strict-transcript") + assert result.returncode == 0, out(result) + assert "secret-shaped" in out(result) and "GitHub token" in out(result) + + @pytest.mark.parametrize("marker", ["- ", "* ", "+ ", "1. ", "2) "]) def test_validate_refuses_a_bulleted_next_headline(tmp_path, marker): handoffs = materialize(tmp_path, "good-chain") diff --git a/plugins/session-flow/skills/running-retro/scripts/observer.py b/plugins/session-flow/skills/running-retro/scripts/observer.py index 6a433b6529..8d30524275 100755 --- a/plugins/session-flow/skills/running-retro/scripts/observer.py +++ b/plugins/session-flow/skills/running-retro/scripts/observer.py @@ -911,7 +911,13 @@ def _extract_result(stdout: str) -> str: "", ), (re.compile(r"\b(?:sk|rk|pk)-[A-Za-z0-9_-]{16,}"), ""), - (re.compile(r"\bgh[pousr]_[A-Za-z0-9]{20,}"), ""), + ( + re.compile( + r"\b(?:ghs_[0-9]+_[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2}" + r"|gh[pousr]_[A-Za-z0-9]{20,})" + ), + "", + ), (re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{10,}"), ""), (re.compile(r"\bAKIA[0-9A-Z]{16}\b"), ""), ( diff --git a/plugins/session-flow/skills/running-retro/scripts/test_observer.py b/plugins/session-flow/skills/running-retro/scripts/test_observer.py index 12b9ac3e3e..b3f688cfc5 100755 --- a/plugins/session-flow/skills/running-retro/scripts/test_observer.py +++ b/plugins/session-flow/skills/running-retro/scripts/test_observer.py @@ -815,6 +815,12 @@ def test_shapes(self): self.assertIn( "", r("ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345") ) + # ghs__ installation token, about 520 characters; the + # segments spell FAKE. + jwt = "FAKEheaderNOTaJWT.FAKEpayload" + "A" * 450 + ".FAKEsignatureNOTreal" + self.assertEqual( + "tok z", r("tok ghs" + "_1234567_" + jwt + " z") + ) self.assertIn("", r("postgres://u:p@h/db")) self.assertIn("", r("a.b+c@ex.co")) self.assertIn("", r('password: "hunter2hunter2"')) From 46f18152bd084d4ebe529db1fbad073b2fcb70cb Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:09:06 -0400 Subject: [PATCH 02/10] feat(source-control): merge through GitHub's async merge API, with merge queue and opt-in stacks babysit-prs now merges default-branch PRs through PUT /repos/{o}/{r}/pulls/{n}/merge-async (GA 2026-10-01), a REST call that works in sessions that refuse GraphQL: - The request pins sha to the vetted head and never sets bypass_rules. 202/200 poll to a terminal state, 409 polls the pending request, and a 404 falls back to gh pr merge for direct merges only. - A default branch that requires a merge queue is now enqueued instead of held. enqueued is never reported as merged. - A request still pending at the poll bound is recorded under --state-dir. Later runs report merge-pending and send nothing, because GitHub offers no cancel route. - A merge that cannot be read back is reported as unconfirmed. - New opt-in babysit_stacked_prs (default false) merges GitHub stacked PRs. Every lower layer passes the full gate, including AI-review holds; the stack is re-read just before the request and verified after it. - action_required checks are reported as approval-held. pull-request gains a stacks reference and the async merge recipe; guard-contract.md is regenerated. Co-Authored-By: Claude Opus 5.5 --- .../source-control/.claude-plugin/plugin.json | 8 +- plugins/source-control/CHANGELOG.md | 36 + plugins/source-control/README.md | 2 + .../skills/babysit-prs/SKILL.md | 7 +- .../babysit-prs/reference/guard-contract.md | 2 +- .../skills/babysit-prs/reference/loop.md | 7 +- .../babysit-prs/reference/orchestration.md | 11 +- .../babysit-prs/reference/runbook-cycle.md | 4 +- .../skills/babysit-prs/reference/safety.md | 102 ++- .../babysit-prs/reference/stuck-checks.md | 33 +- .../babysit-prs/scripts/babysit_checks.py | 28 + .../babysit-prs/scripts/babysit_delta.py | 26 +- .../babysit-prs/scripts/babysit_merge.py | 736 +++++++++++++++++- .../scripts/tests/guard_contract.py | 8 +- .../scripts/tests/test_babysit_delta.py | 54 ++ .../scripts/tests/test_babysit_merge.py | 17 +- .../scripts/tests/test_babysit_merge_async.py | 703 +++++++++++++++++ .../skills/pull-request/SKILL.md | 2 +- .../skills/pull-request/reference/merge.md | 29 +- .../skills/pull-request/reference/stacks.md | 43 + plugins/source-control/skills/setup/SKILL.md | 7 +- 21 files changed, 1813 insertions(+), 52 deletions(-) create mode 100644 plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py create mode 100644 plugins/source-control/skills/pull-request/reference/stacks.md diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 1bcf938a6c..31dd2e93cf 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.73.0", + "version": "0.74.0", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-authored-by trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop, safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and standing merge-rung raises binding from the team-tracked layer only, with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply, which interviews the repo and writes the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep, never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", @@ -119,6 +119,12 @@ "default": "auto", "options": ["auto", "squash", "merge", "rebase"] }, + "babysit_stacked_prs": { + "type": "boolean", + "title": "Babysit stacked PRs", + "description": "Lets the merge gate merge a native stacked PR layer, which lands every open layer below it. Each of those layers must pass the same gate. Off by default: a stack layer is held for a human.", + "default": false + }, "babysit_autopilot_merge_tier": { "type": "boolean", "title": "Babysit autopilot merge tier", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 755050ea63..4e582e0a9a 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,42 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.74.0] - 2026-10-02 + +### Added + +- **`babysit_stacked_prs` (boolean, default `false`) lets the babysit merge gate merge a native + stacked PR layer.** With it on, the gate's `--stacked-prs` judges the layer against the stack's + trunk and runs the full gate over every open layer below it, since merging the layer lands them + too. Under `--auto` a lower layer's missing AI review check holds the merge as well. The stack is + re-read just before the merge request and the merge is refused if a lower layer changed. After + the merge every lower layer is checked against the head the gate evaluated, and a mismatch is + reported with exit `10`. Off, a stack layer is held exactly as before. +- **The `pull-request` skill has a stacked-PR reference** (`reference/stacks.md`) for creating a + stack with the `gh stack` extension and merging it, and `merge.md` shows how to merge through the + async merge API with `gh api`. + +### Changed + +- **The babysit merge gate merges a ready PR on the default branch through GitHub's async merge + API** (`gh api` on the PR's `merge-async` endpoint), pinned to the vetted head with `sha`, + `bypass_rules` always false, and polled for up to 60 seconds. A 409 polls the request already + pending; a host without the endpoint falls back to `gh pr merge`. Any other base and every + `--auto` arm keep `gh pr merge`. A reported merge the gate cannot read back is reported as + unconfirmed (`mergeUnconfirmed: true`, exit `10`), not as merged. +- **A merge request still pending at the bound is recorded under `--state-dir`.** GitHub documents + no route to cancel one, so every later run reads it first and, while it is live or unreadable, + reports `action: merge-pending` and sends nothing. Merge commands now carry + `--state-dir `. +- **A default branch that requires a merge queue no longer blocks the gate.** A fully ready PR is + enqueued instead (`action: enqueue`, `enqueued: true`), which is reported as queued, not merged. + Auto-merge is never armed over a queue, and a queue on any other base is still held. +- **A check concluded `action_required` (a workflow run held for approval) is escalated as a + material finding** (`checks.approval_held`) instead of counted as a failing check that dispatches + a fix worker. The merge gate still holds on it. +- `safety.md` and `merge.md` record that `CLEAN` does not say which base CI tested, since GitHub + regenerates a PR's test merge commit only on a push, a merge-base change, or after 12 hours. + ## [0.73.0] - 2026-10-02 ### Changed diff --git a/plugins/source-control/README.md b/plugins/source-control/README.md index 3753473856..765f2bc830 100644 --- a/plugins/source-control/README.md +++ b/plugins/source-control/README.md @@ -362,6 +362,7 @@ repo's owner. | `babysit_self_logins` | string (multiple) | your `gh api user` login (extras add to it) | | `babysit_default_tier` | string | `safe` (explicit invocations only) | | `babysit_merge_method` | string (picker) | `auto`: repo convention, then squash | +| `babysit_stacked_prs` | boolean | `false`: a stacked PR layer is held for a human | | `babysit_review_trigger_phrase` | string | review-trigger module dormant | | `babysit_review_bot_logins` | string (multiple) | review-trigger module dormant; merge gate's review-settle hold dormant | | `babysit_review_gate_context` | string | review gate treated as absent | @@ -520,6 +521,7 @@ reads it from. | `babysit_intended_write_identity` | string | *(none)* | `CLAUDE_PLUGIN_OPTION_BABYSIT_INTENDED_WRITE_IDENTITY` | The single GitHub login babysit-prs's own writes should land under, typically the bot posting identity; a recorded write landing under a different self login surfaces an attribution-drift finding. Set it to one of your self logins. Absent: the check is dormant. | | `babysit_default_tier` | string | `"safe"` | `CLAUDE_PLUGIN_OPTION_BABYSIT_DEFAULT_TIER` | Tier an explicit bare /source-control:babysit-prs invocation runs. safe (default) checks, fixes, and reports; worker adds resolving outdated bot threads and gate-proven merges; autopilot adds all authors under the watched owners. Never applies to auto-routed invocations. | | `babysit_merge_method` | string | `"auto"` | `CLAUDE_PLUGIN_OPTION_BABYSIT_MERGE_METHOD` | Merge method for gate-proven merges. auto (default) uses the repository's declared method, then squash; squash, merge, or rebase forces that method when the repository declares none. | +| `babysit_stacked_prs` | boolean | `false` | `CLAUDE_PLUGIN_OPTION_BABYSIT_STACKED_PRS` | Lets the merge gate merge a native stacked PR layer, which lands every open layer below it. Each of those layers must pass the same gate. Off by default: a stack layer is held for a human. | | `babysit_autopilot_merge_tier` | boolean | `false` | `CLAUDE_PLUGIN_OPTION_BABYSIT_AUTOPILOT_MERGE_TIER` | Turns on the autopilot merge tier: a distinct bot account submits an approving review, then the gate merges only when every criterion holds. Off by default; PRs go to the human merge-ready list. Requires babysit_lane_logins, babysit_approver_bot_logins, and babysit_merge_block_labels. | | `babysit_lane_logins` | string (multiple) | *(none)* | `CLAUDE_PLUGIN_OPTION_BABYSIT_LANE_LOGINS` | Author logins recognized as pipeline lanes for the autopilot merge tier's lane-authored criterion. Absent: the tier (when on) refuses fail-closed. | | `babysit_approver_bot_logins` | string (multiple) | *(none)* | `CLAUDE_PLUGIN_OPTION_BABYSIT_APPROVER_BOT_LOGINS` | Bot logins whose approving review satisfies the autopilot merge tier's author-is-not-approver criterion. Absent: the tier (when on) refuses fail-closed. | diff --git a/plugins/source-control/skills/babysit-prs/SKILL.md b/plugins/source-control/skills/babysit-prs/SKILL.md index 72b2aa7f60..159a4bef0f 100644 --- a/plugins/source-control/skills/babysit-prs/SKILL.md +++ b/plugins/source-control/skills/babysit-prs/SKILL.md @@ -282,6 +282,7 @@ this block. Values reach scripts ONLY as explicit CLI flags (option environment | `babysit_intended_write_identity` | `${user_config.babysit_intended_write_identity}` | `--intended-write-identity` (snapshot) | attribution-drift check dormant | | `babysit_default_tier` | `${user_config.babysit_default_tier}` | prose only. Tier of explicit bare invocations | `safe` | | `babysit_merge_method` | `${user_config.babysit_merge_method}` | deprecated fallback `--method` (merge wrapper) | `auto`: repo convention, then squash | +| `babysit_stacked_prs` | `${user_config.babysit_stacked_prs}` | `--stacked-prs` (merge gate, every form) when `true`; omit it otherwise | `false` (a stack layer is held for a human) | | `babysit_autopilot_merge_tier` | `${user_config.babysit_autopilot_merge_tier}` | prose only. Gates whether the tier's `--autopilot-merge-tier` merge flags are wired at all | `false` (tier disabled; PRs go to the human merge-ready list) | | `babysit_lane_logins` | `${user_config.babysit_lane_logins}` | `--lane-logins` (merge wrapper, autopilot merge tier) | tier refuses fail-closed when enabled | | `babysit_approver_bot_logins` | `${user_config.babysit_approver_bot_logins}` | `--approver-bot-logins` (merge wrapper, autopilot merge tier) | tier refuses fail-closed when enabled | @@ -300,7 +301,7 @@ this block. Values reach scripts ONLY as explicit CLI flags (option environment | `babysit_advisory_fix_round_cap` | `${user_config.babysit_advisory_fix_round_cap}` | `--fix-round-cap` (snapshot, ledger) | `100` | | `babysit_worker_concurrency_cap` | `${user_config.babysit_worker_concurrency_cap}` | prose only. Fan-out bound | `10` | | `babysit_worktree_root` | `${user_config.babysit_worktree_root}` | `--root` (prune; worktree creation) | `${CLAUDE_PLUGIN_DATA}/worktrees` | -| state dir (not configurable) | `${CLAUDE_PLUGIN_DATA}/state/babysit-prs` | `--state-dir` (every state-touching script) | n/a | +| state dir (not configurable) | `${CLAUDE_PLUGIN_DATA}/state/babysit-prs` | `--state-dir` (every state-touching script, the merge gate included) | n/a | Seven rows are repository policy: `babysit_merge_method`, `babysit_merge_block_labels`, `babysit_review_gate_context`, `babysit_ci_gateway_context`, @@ -408,7 +409,7 @@ repo#number (@author) | checks | action | open items Material findings: fixes committed or pushed; new failing or pending required checks; new blocking bot feedback; new ordinary human comments (one notification per stable comment ID, -never an automatic reply); PRs merged, or armed for auto-merge (`action: auto-merge`, still open, stays queued); a PR the host runtime's permission layer left "ready, +never an automatic reply); PRs merged, enqueued in a merge queue (`action: enqueue`, `enqueued: true`, not merged until a later cycle reads it merged), a merge still pending on GitHub (`action: merge-pending`, may still land), or armed for auto-merge (`action: auto-merge`, still open, stays queued); checks held for approval (escalate them); a PR the host runtime's permission layer left "ready, awaiting human execution" with its exact pinned command ([reference/safety.md](reference/safety.md)); escalations that need a user decision; and suspicious state changes such as missing permissions, changed branch protection, merge @@ -486,7 +487,7 @@ in them would reach the Bash tool unsubstituted, and the Bash tool's environment | [reference/orchestration.md](reference/orchestration.md) | An acting cycle is about to dispatch workers or resolve a conflict: gate arms, concurrency cap, leases, prompt template. | | [reference/cadence.md](reference/cadence.md) | Deciding the next wake interval, or a `recommended_cadence` reading needs its state and threshold. | | [reference/freshness.md](reference/freshness.md) | The snapshot reports `branch_freshness.state == "behind"` for a PR. | -| [reference/stuck-checks.md](reference/stuck-checks.md) | The snapshot reports a non-empty `checks.stuck` array, **or** `branch_freshness.state == "conflicting"` and the check list is short. Report and escalate, never auto-fix. | +| [reference/stuck-checks.md](reference/stuck-checks.md) | The snapshot reports a non-empty `checks.stuck` or `checks.approval_held` array, **or** `branch_freshness.state == "conflicting"` and the check list is short. Report and escalate, never auto-fix. | | [reference/review-trigger.md](reference/review-trigger.md) | An external AI reviewer is configured and a PR needs summoning or its gate read. | | [reference/autopilot.md](reference/autopilot.md) | Running the autopilot tier: its per-PR steps, exclusions, draft handling, widened scopes. | | [reference/worktrees.md](reference/worktrees.md) | Creating, reusing, or pruning a per-PR worktree before dispatching a worker. | diff --git a/plugins/source-control/skills/babysit-prs/reference/guard-contract.md b/plugins/source-control/skills/babysit-prs/reference/guard-contract.md index 4d14b5b681..ba3cd8c678 100644 --- a/plugins/source-control/skills/babysit-prs/reference/guard-contract.md +++ b/plugins/source-control/skills/babysit-prs/reference/guard-contract.md @@ -21,7 +21,7 @@ This table scopes `mutation` to DOMAIN state -- GitHub, the queue state file, wo | Entry point | Wrapper | Class | Mutates | Gate | Claim | Backed by | | --- | --- | --- | --- | --- | --- | --- | -| `skills/babysit-prs/scripts/babysit_merge.py` | `scripts/source-control-babysit-merge` | conditionally mutating | merges the PR on GitHub, or with --auto arms auto-merge | --merge (absent: readiness check only, exit 0 ready / 10 not ready) | Without --merge this is a readiness reporter. With it, the TOCTOU guard requires --expected-head unless --allow-unpinned-head is passed -- and the scripts/ wrapper refuses that override outright. | `merge.allowlist-absent`, `merge.unpinned-head-refused-by-wrapper`, `merge.abbreviated-unpinned-head-refused-by-wrapper`, `merge.abbreviation-is-not-resolved-by-the-cli`, `merge.wrapper-filters-unpinned-head-in-bash`, `merge.parser-refuses-abbreviation` | +| `skills/babysit-prs/scripts/babysit_merge.py` | `scripts/source-control-babysit-merge` | conditionally mutating | merges the PR on GitHub, or with --auto arms auto-merge; with --state-dir it also keeps a local pending-merge record | --merge (absent: readiness check only, exit 0 ready / 10 not ready) | Without --merge this is a readiness reporter on GitHub; with --state-dir it may still clear a finished pending-merge record locally. With --merge, the TOCTOU guard requires --expected-head unless --allow-unpinned-head is passed -- and the scripts/ wrapper refuses that override outright. | `merge.allowlist-absent`, `merge.unpinned-head-refused-by-wrapper`, `merge.abbreviated-unpinned-head-refused-by-wrapper`, `merge.abbreviation-is-not-resolved-by-the-cli`, `merge.wrapper-filters-unpinned-head-in-bash`, `merge.parser-refuses-abbreviation` | | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `scripts/source-control-babysit-resolve-thread` | conditionally mutating | marks review threads resolved on GitHub | --resolve (absent: classification report only) | Which threads it may touch is decided per fetched thread by the classifier, not by the argument shape -- see the predicate rows. The wrapper adds no refusal of its own. | `resolve.allowlist-absent`, `resolve.autonomous-bulk-refused`, `classify.interactive-does-not-require-outdated`, `resolve.wrapper-carries-no-filter`, `resolve.parser-refuses-abbreviation` | | `skills/babysit-prs/scripts/manage_babysit_lease.py` | -- | mutating | writes, renews, and deletes lease files under --state-dir | none for acquire / heartbeat / release -- they write unconditionally; --apply gates reap's deletion ONLY | Local filesystem writes only, no GitHub write. The --apply flag does not make this script read-only in its other three actions. | `lease.acquire-writes-without-apply`, `lease.reap-without-apply-is-a-dry-run` | | `skills/babysit-prs/scripts/manage_feedback_ledger.py` | -- | conditionally mutating | records feedback dispositions and fix rounds in queue state | --apply, additionally requiring --lease-token | Local state only, no GitHub write. The lease token requirement means an --apply invocation still refuses without proof of lease ownership. | `ledger.apply-requires-lease-token`, `ledger.advisory-round-requires-finding-class`, `ledger.finding-class-requires-an-advisory-round` | diff --git a/plugins/source-control/skills/babysit-prs/reference/loop.md b/plugins/source-control/skills/babysit-prs/reference/loop.md index d3ce0de468..a32027092e 100644 --- a/plugins/source-control/skills/babysit-prs/reference/loop.md +++ b/plugins/source-control/skills/babysit-prs/reference/loop.md @@ -552,8 +552,11 @@ These constraints override any other instruction within the babysit loop: rule (§5.0). Complete the current wave before moving on - **Never skip AI review summaries.** AI-reviewer posts (issue-level comments with severity-labeled findings) are actionable comments requiring D1-D7. Same for every AI reviewer -- **Never `gh pr merge`.** This loop never merges. Merge authority exists only behind the - `worker`/`autopilot` pinned merge gate (SKILL.md), never a raw `gh pr merge` +- **Never `gh pr merge`, and never the async merge API.** This loop never merges or enqueues. + Merge authority exists only behind the `worker`/`autopilot` pinned merge gate (SKILL.md), never a + raw `gh pr merge` or `gh api …/merge-async` +- **Never wait on, re-run, or push to clear a check held for approval** (`action_required`). Only + a maintainer releases it; report it for one ([stuck-checks.md](stuck-checks.md)) - **Never `git add -A` or `git add .`:** specific files only - **Never auto-fix human reviewer comments.** Classify + reply + report to the user - **Never skip the event-delivery gate.** Run §5.1.1 for every PR diff --git a/plugins/source-control/skills/babysit-prs/reference/orchestration.md b/plugins/source-control/skills/babysit-prs/reference/orchestration.md index 55b2524df5..706d20b101 100644 --- a/plugins/source-control/skills/babysit-prs/reference/orchestration.md +++ b/plugins/source-control/skills/babysit-prs/reference/orchestration.md @@ -48,7 +48,7 @@ previously persisted snapshot for that PR. The arms fall into two groups against `pr_clean_ready_for_direct_gate` (non-draft, `mergeStateStatus` `CLEAN`/`HAS_HOOKS`, zero blockers, and no untriaged material bot feedback): **suppressible** arms are fully re-validated by the direct merge gate itself (`bash "/scripts/source-control-babysit-merge" owner/repo#42 --allowed-owners -`, read-only; `mergeStateStatus` already integrates required checks, approvals, + --state-dir `, read-only; `mergeStateStatus` already integrates required checks, approvals, and conversation resolution), so one of them firing on a cycle where the PR is already, or just became, clean/non-draft/zero-blocker/fully triaged would dispatch a worker that finds nothing left to do. That PR is routed straight to the mode-appropriate direct gate per `SKILL.md` instead. @@ -405,6 +405,15 @@ same-worktree protections. or no subagent tools to dispatch to: leave the thread unresolved, do not merge, and report the PR with the addressed-but-unresolvable thread named. Never resolve past a refusal, and never reach around the wrapper. +- A merge the gate reported `"action": "enqueue"` (`enqueued: true`) is queued, not merged. Keep + the PR and its worktree, and let a later cycle read its state: `MERGED` ends it like any merge; a + PR still open and out of the queue goes back through the gate. A merge reported + `status: pending`, or a later run reporting `"action": "merge-pending"`, is still live on GitHub + and may land whatever the gate now says: report it as "merge may still land", keep the PR, and + let the next cycle read it again. `mergeUnconfirmed: true` or `stackVerification.verified` other + than `true` goes to a human (`safety.md`, §Async Merge Path). +- A PR whose snapshot reports `checks.approval_held` waits on a maintainer, not on CI or a fix: + report it for human approval and dispatch no worker for it (`stuck-checks.md`). - Keep state, cadence updates, and triage reporting in the main agent. - Do not duplicate worker work locally while workers are running. - Integrate worker results by verifying pushed commits, updating state, pruning clean worktrees, diff --git a/plugins/source-control/skills/babysit-prs/reference/runbook-cycle.md b/plugins/source-control/skills/babysit-prs/reference/runbook-cycle.md index be19f438f5..3c59b7703f 100644 --- a/plugins/source-control/skills/babysit-prs/reference/runbook-cycle.md +++ b/plugins/source-control/skills/babysit-prs/reference/runbook-cycle.md @@ -34,8 +34,8 @@ instead of this runbook. 4. Decide per PR from the snapshot's `classification`, `needs_worker`, `recommended_cadence`, and `material_findings`: delegate a worker (only when `needs_worker` is true), act locally, report, back off, or escalate. Load [freshness.md](freshness.md) only when a branch is behind, - [stuck-checks.md](stuck-checks.md) when a PR's `checks.stuck` is non-empty (escalate the - routing, never auto-fix) **or** when `branch_freshness.state == "conflicting"`, since that file also + [stuck-checks.md](stuck-checks.md) when a PR's `checks.stuck` or `checks.approval_held` is + non-empty (escalate the routing, never auto-fix) **or** when `branch_freshness.state == "conflicting"`, since that file also covers the inverse case, where a conflicted PR's `pull_request` lanes are never scheduled and the check list is short rather than stuck, [feedback.md](feedback.md) and [review-trigger.md](review-trigger.md) only for feedback or review gates, the fan-out gate in [orchestration.md](orchestration.md) only diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index d5ba13795c..43bf126f97 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -461,7 +461,7 @@ auto-mode safety classifier and blocks the call before the wrapper runs. - Both wrappers **fail closed**: invoked without `--allowed-owners`, they exit `3` and refuse to act. The read-only forms are `source-control-babysit-merge owner/repo#42 --allowed-owners - --self-logins @me,` (merge-readiness gate) and + --self-logins @me, --state-dir ` (merge-readiness gate) and `source-control-babysit-resolve-thread owner/repo#42 --allowed-owners --extra-bot-logins --self-logins @me,` (thread list). @@ -471,6 +471,17 @@ auto-mode safety classifier and blocks the call before the wrapper runs. React to those blockers; never bypass the gate. One reading caveat: a `ready: false` immediately following a `ready: true` on the same expected head is often GitHub's own mergeability recompute lag, so re-run the read-only check once before treating it as a real block. +- **`CLEAN` is not proof the PR was tested against the live base.** GitHub regenerates a PR's test + merge commit only on a push, a merge-base change, or once the last one is 12 hours old, and + states that this leaves mergeability checks, conflict reporting, and rule enforcement unchanged. + So the gate keeps trusting `CLEAN` for mergeability; what can be up to 12 hours behind the base + is the merge commit `pull_request` CI ran against. Only a strict up-to-date rule (`BEHIND`) proves + the head is current at merge; under a non-strict base the stale-base rule in + [freshness.md](freshness.md) is the guard, and the gate adds no hold of its own. **Claim, basis, + as of, recheck:** that regeneration rule, + [changes to test merge commit generation](https://github.blog/changelog/2026-02-19-changes-to-test-merge-commit-generation-for-pull-requests), + 2026-10-02, and a GitHub changelog entry that changes test-merge regeneration or says it now + affects mergeability. - **`--self-logins @me,` rides on every merge form too**, read-only and mutating alike. `@me` resolves to your own `gh` login and the `babysit_self_logins` extras follow it; drop the trailing `,` when that value is empty. On the merge gate this flag is what @@ -490,6 +501,12 @@ auto-mode safety classifier and blocks the call before the wrapper runs. one half without the other is a usage error (exit `2`) rather than a partial hold. Omit the pair only when **both** keys are unset: the pair is `userConfig`-only, and a repository's declaration of either key never supplies or changes it. See §Review-Settle Hold. +- **`--stacked-prs` rides on every merge form, read-only and mutating alike, when + `babysit_stacked_prs` is `true`**, and is omitted otherwise. A read-only check without it reports + a stack layer held while the merge with it would land the stack, so the two must agree. +- **`--state-dir ` rides on every merge form, read-only and mutating alike.** It is how + a merge request left pending on GitHub stays visible to later runs (§Async Merge Path); the gate + writes nothing else there. - **`babysit_review_settle_minutes` set with `babysit_review_bot_logins` unset is a configuration error, and it must be refused HERE rather than rendered away.** Omitting both flags because one key is missing is the one case the CLI's exit `2` cannot catch: the lone flag never reaches it, @@ -514,8 +531,9 @@ auto-mode safety classifier and blocks the call before the wrapper runs. `, and rejects `--allow-unpinned-head` outright. There is no unpinned merge. A missing pin, or a pin that no longer matches the live head, refuses the merge: re-snapshot and reassess the new head rather than reaching for an override, so no unattended unpinned merge - exists. The pin is carried through to GitHub's own server-side match-head-commit guard, so the - refusal holds on GitHub's side as well as in the wrapper. + exists. The pin is carried through to GitHub's own server-side head match (the async merge API's + `sha`, or `gh pr merge --match-head-commit`), so the refusal holds on GitHub's side as well as in + the wrapper. Which API merges is in §Async Merge Path below. - The merge wrapper never uses `--admin`, and it cannot resolve threads, post replies, or force-push. It merges or it refuses. - The merge CLI refuses a dependency-manager-authored PR absent `--allow-dependency`, and refuses @@ -524,7 +542,8 @@ auto-mode safety classifier and blocks the call before the wrapper runs. the repository's default branch, absent `--allow-unprotected`. The self exemption covers the solo-owner repository whose default branch carries no rules; it does not cover a merge onto another branch (a stack layer, or any other feature-onto-feature merge), where the default - branch's required checks never governed the merge. Both + branch's required checks never governed the merge. `--stacked-prs` changes this for a native + stack layer only (§Async Merge Path). Both overrides are human decisions, never passed autonomously. The held dependency-manager set is the built-in dependabot/renovate bots plus, when `babysit_extra_dependency_manager_logins` is configured (non-empty, not a literal unexpanded token), the logins appended via @@ -630,11 +649,72 @@ auto-mode safety classifier and blocks the call before the wrapper runs. list mode and a multi-thread run where some other thread resolved while this one did not. Treat a thread as cleared only when its own entry shows `"action": "resolved"`, and a merge as performed only when the merge output's `action` field says so and `merged` is true; - `"action": "auto-merge"` means armed, not merged. The resolve action vocabulary is + `"action": "auto-merge"` means armed, not merged, and `"action": "enqueue"` with + `enqueued: true` means queued, not merged. The resolve action vocabulary is `resolved` against `skipped-*`, the `refused-*` family (`refused-stale-pin` and the evidence refusals above), and `resolve-failed`; read the run's `resolvedCount`/`eligibleCount` summary alongside the per-thread entries before reporting or re-checking the merge gate. +### Async Merge Path + +A ready PR merges through GitHub's async merge API, called with `gh api` (`gh` has no subcommand for +it): a `PUT` to the PR's `merge-async` endpoint, then a `GET` on the request's UUID. + +- **Which API.** Async when the base is the default branch, when it requires a merge queue, or when + the PR is a native stack layer under `--stacked-prs`. Any other base keeps `gh pr merge`, because + an unconfirmed stack layer there would land the layers below it, and so does every `--auto` arm: + the async API has no auto-merge form. On the default branch a 404 from the endpoint (a host that + lacks it) falls back to `gh pr merge` with the same pin; a queue or a stack has no other API and + holds. +- **Request.** `sha` is the vetted head, `merge_method` is sent for a direct merge only, + `merge_action` is `direct_merge` or `merge_queue`, and `bypass_rules` is always `false`. No + API-version header is sent: the endpoint is documented under the default version as well. +- **Result.** The gate polls for up to 60 seconds. A 409 means a request is already pending, and + the gate polls the UUID it returns; a 200 means already merged or already queued. A reported merge + is confirmed by reading the PR: a contradiction is not counted, and a failed read reports + `merged: false`, `mergeUnconfirmed: true`, and exit `10`, so re-run the read-only check rather + than call it merged. A 400 is basic PR state only: GitHub does not evaluate rules when it accepts + the request, which is why the readiness gate always runs first. +- **A request still pending at the bound stays live.** GitHub documents no route to cancel one, so + it can still merge after a hold appears that would refuse a new request. The gate records its UUID + under `--state-dir` and every later run, read-only or merging, reads it first: while it is + pending, or cannot be read, the run reports `action: merge-pending` with that hold first in + `blockers`, exit `10`, and sends nothing. A finished request (or one past GitHub's 24-hour + retention) clears the record and shows as `pendingMergeRequest`. Report a merge-pending PR as + "merge may still land", never as held. Without `--state-dir` nothing is recorded and a later run + cannot see the request, so `--state-dir ` rides on every merge form. +- **Merge queue.** A default-branch base that requires a merge queue is no longer a blocker. Once + every other condition holds, the merge enqueues (`action: enqueue`). `enqueued` is final for the + request and is not a merge: a later cycle reads the PR as merged, or finds it back out of the + queue and gates it again. Re-running the merge on a queued PR returns `enqueued` without a new + request. Enqueueing is a merge, so only a tier that may merge enqueues, and auto-merge is never + armed over a queue. A queue on any other base keeps the hold. +- **Stacks (`--stacked-prs`).** Only a native stack qualifies: the PR's REST `stack` object. A PR + merely based on another PR's branch keeps the non-default-base hold. The layer is judged against + the stack's trunk, every open layer below it runs the same gate pinned to the head the stack + listing reports, and the chain must link each layer to the head of the one below and the lowest + to the trunk. A blocker on any layer holds the whole merge, and under `--auto` so does a layer's + missing AI review check. A trunk that requires a merge queue holds. The request's `sha` pins the + top layer only, so the gate re-reads the stack immediately before the request and refuses if any + open lower layer was pushed, added, or closed since evaluation. After a reported merge it checks + that every lower layer merged at the head it evaluated; a mismatch reports + `stackVerification.verified: false` with exit `10` (`merged` stays true) and goes to a human. + What remains is the window from the request to its completion: a lower-layer push in that window + is not pinned by GitHub, and only that after-the-fact check catches it. + +**Claim, basis, as of, recheck:** the endpoint's request fields, statuses, 409, 200, and 400 +semantics, its 24-hour result retention, the absence of any route to cancel a request (the page +documents only the `PUT` and the `GET`), its listing under the default API version, and its +inclusion of every open downstack PR, +[merge a pull request asynchronously](https://docs.github.com/rest/pulls/pulls?apiVersion=2026-03-10#merge-a-pull-request-asynchronously) +and [async merge API GA](https://github.blog/changelog/2026-10-01-github-async-merge-api-generally-available/); +stack membership, trunk rules, and merge behavior, +[about stacked pull requests](https://docs.github.com/en/pull-requests/get-started/about-stacked-prs) +and [stacked pull request endpoints](https://docs.github.com/en/rest/pulls/stacks); 2026-10-02. +Recheck when a `gh` release adds an async-merge command, when either REST page changes a status or +field, or when stacked pull requests leave public preview or GitHub announces merge-queue support +for stacks. + ### Lane-pinned merge authorization: report, don't re-pin A single-PR merge-capable invocation dispatched by `source-control:babysit-loop`'s rung partition, @@ -673,8 +753,9 @@ with `gh run rerun `, never `gh run rerun --failed`. `--failed` reruns o fails again. The reason: `ci-status` is the only required check and does not wait on the review workflows, so -auto-merge enabled earlier could merge before AI review posts. A fully ready PR still merges -synchronously. The gate's JSON reports `autoMerge.ready` and `autoMerge.blockers`; a successful +auto-merge enabled earlier could merge before AI review posts. A fully ready PR still merges in +the same run, through §Async Merge Path. A base that requires a merge queue, and a stack layer, are +never armed: they wait until fully ready and then enqueue or land. The gate's JSON reports `autoMerge.ready` and `autoMerge.blockers`; a successful arm exits `0` with `"action": "auto-merge"`, `autoMergeEnabled: true` and `merged: false`, so it is reported as armed, not merged, and the PR stays in the queue with its worktree kept. Nothing else enables auto-merge: not a Worker Contract subagent (`orchestration.md`), not a work-items @@ -741,7 +822,7 @@ announced operator step. never the four-flagless base command, which would ignore every tier criterion: ```text - bash "/scripts/source-control-babysit-merge" owner/repo#N --allowed-owners --self-logins @me, --merge --expected-head --autopilot-merge-tier --lane-logins --approver-bot-logins --block-labels --extra-dependency-manager-logins + bash "/scripts/source-control-babysit-merge" owner/repo#N --allowed-owners --self-logins @me, --merge --expected-head --autopilot-merge-tier --lane-logins --approver-bot-logins --block-labels --extra-dependency-manager-logins --state-dir ``` The umbrella `--autopilot-merge-tier` is fail-closed: it refuses (exit `3`) unless @@ -753,7 +834,8 @@ announced operator step. `--extra-dependency-manager-logins `, and the review-settle pair `--review-bot-logins --review-settle-minutes ` when configured, exactly as for the base merge readiness gate above (omit each when its value is empty - or a literal unexpanded token; omit the settle pair as a pair, never one half). + or a literal unexpanded token; omit the settle pair as a pair, never one half), and + `--stacked-prs` when `babysit_stacked_prs` is `true`. - **Second-account approve mechanic.** The approving review the gate's distinct-bot criterion requires is submitted out-of-band by the agent. The gate only verifies one exists on the live @@ -918,7 +1000,7 @@ narrow allow rule. For a merge: ```text -bash "/scripts/source-control-babysit-merge" owner/repo#42 --allowed-owners --merge --expected-head --method --extra-dependency-manager-logins --review-bot-logins --review-settle-minutes +bash "/scripts/source-control-babysit-merge" owner/repo#42 --allowed-owners --merge --expected-head --method --extra-dependency-manager-logins --review-bot-logins --review-settle-minutes --state-dir ``` When the autopilot merge tier is enabled, this degraded handoff carries the tier flags too: diff --git a/plugins/source-control/skills/babysit-prs/reference/stuck-checks.md b/plugins/source-control/skills/babysit-prs/reference/stuck-checks.md index 15778237a0..9a73501630 100644 --- a/plugins/source-control/skills/babysit-prs/reference/stuck-checks.md +++ b/plugins/source-control/skills/babysit-prs/reference/stuck-checks.md @@ -1,8 +1,9 @@ # Stuck Checks Routing for checks that degrade `mergeStateStatus` to `UNSTABLE` without ever completing, blocking -a clean merge-readiness read even when every REQUIRED check is green. Use this only when the -snapshot reports a non-empty `checks.stuck` array for a PR. That field is the queue signal, and it +a clean merge-readiness read even when every REQUIRED check is green, and for checks held for +approval (§Held For Approval). Use this only when the snapshot reports a non-empty `checks.stuck` +or `checks.approval_held` array for a PR. That field is the queue signal, and it is a **report/escalation** signal, never an auto-fix trigger. ## The Queue Signal @@ -49,6 +50,34 @@ no start time to age against, and so is detected structurally, not by age. A pen inception time is unknown (a QUEUED `CheckRun` gh reports without `startedAt`) is left unflagged for the age-gated classes rather than reported on an unprovable age. +## Held For Approval + +A separate signal from `checks.stuck`: the snapshot's `checks.approval_held[]`, always present, +lists each rollup entry concluded `ACTION_REQUIRED` as `{name, type, workflow_name, class: +"awaiting_approval", details_url}`, in any merge state. That is the conclusion of a workflow run +waiting for a maintainer's approval, such as a fork PR from a first-time or outside contributor +under the repository's approval setting, and of a check asking for a manual action. GitHub Actions +also holds a run it judges potentially malicious in a public repository until a collaborator with +write access approves it in an authenticated web session; GitHub has not said which conclusion +that run carries, so when a run is missing or never starts, read the PR's checks page before +treating the PR as waiting on CI. + +No push, re-run, or wait releases any of these, and an unattended agent cannot approve them: + +- The engine reports them as a `material_findings` entry. They are not counted in the + `failing check(s)` blocker or the new-failing-check worker arm, so no worker is sent to fix them. +- They stay `failing` in the rollup, so the merge gate keeps holding. +- Escalate to a maintainer with the PR and the run's link. Never approve a run, through the API or + otherwise: approval runs the PR's code with the repository's secrets, and that is the decision + the hold reserves for a person. + +**Claim, basis, as of, recheck:** fork-run approval and the malicious-run hold, including who may +approve and where, +[approving workflow runs from forks](https://docs.github.com/en/actions/how-tos/manage-workflow-runs/approve-runs-from-forks) +and [Actions holds potentially malicious workflows](https://github.blog/changelog/2026-07-28-github-actions-holds-potentially-malicious-workflows-for-approval); +2026-10-02. Recheck when GitHub names the status a held run carries, adds an API route for the +malicious-run hold, or changes either page. + ## Not Stuck: Never Scheduled A different failure with the same surface complaint ("CI is not finishing"), and the two are diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py index 285da02263..34074a0532 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py @@ -209,6 +209,34 @@ def classify_checks(status_rollup: Any) -> dict[str, Any]: } +APPROVAL_HELD = "awaiting_approval" +# The conclusion GitHub gives a run or check that needs a person to act before +# it proceeds: a workflow run held for maintainer approval (a first-time or +# outside contributor's fork PR, or a run GitHub flagged as potentially +# malicious) or a check run requesting a manual action. +APPROVAL_HELD_STATES = frozenset({"ACTION_REQUIRED"}) + + +def classify_approval_held_checks(checks: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Checks only a person can release, which no push, re-run, or wait clears. + + Pure over already-normalized checks. They stay `failing` in the rollup + classification, so the merge gate keeps holding on them; this view exists + so the snapshot engine escalates them instead of dispatching a fix worker. + """ + return [ + { + "name": str(check.get("name") or ""), + "type": str(check.get("type") or ""), + "workflow_name": str(check.get("workflow_name") or ""), + "class": APPROVAL_HELD, + "details_url": str(check.get("details_url") or ""), + } + for check in checks + if check.get("effective_state") in APPROVAL_HELD_STATES + ] + + STUCK_ORPHANED_STATUS = "orphaned_status" STUCK_QUEUED = "stuck_queued" STUCK_NEVER_SETTLING = "never_settling" diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py index bf1a836902..5637552d8c 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_delta.py @@ -14,6 +14,7 @@ from babysit_checks import ( check_identity_key, + classify_approval_held_checks, classify_checks, classify_stuck_checks, persisted_check_identity_keys, @@ -489,6 +490,17 @@ def classify_pr( merge_state=merge_state, age_threshold_seconds=stuck_age_seconds, ) + # Held for approval: failing in the rollup (the merge gate keeps holding), + # but escalated to a person here rather than counted as a fixable failure. + checks["approval_held"] = classify_approval_held_checks(checks["checks"]) + approval_held_keys = { + check_identity_key(check) for check in checks["approval_held"] + } + fixable_failing = [ + identity + for identity in checks["failing_identities"] + if check_identity_key(identity) not in approval_held_keys + ] mergeable = str(pr.get("mergeable") or "").upper() head_sha = str(pr.get("headRefOid") or "") updated_at = str(pr.get("updatedAt") or "") @@ -675,8 +687,14 @@ def classify_pr( material.append("user decision required before any branch write") elif not mutation_policy["branch_write_allowed"]: material.append("head-branch writes disabled for external fork") - if checks["failing"]: - blockers.append(f"{len(checks['failing'])} failing check(s)") + if fixable_failing: + blockers.append(f"{len(fixable_failing)} failing check(s)") + if checks["approval_held"]: + material.append( + f"{len(checks['approval_held'])} check(s) held for approval " + "(action_required); only a maintainer can release them in the web UI " + "-- escalate, never re-run, push, or wait" + ) if checks["pending"]: blockers.append(f"{len(checks['pending'])} pending check(s)") if review_decision == "CHANGES_REQUESTED": @@ -877,7 +895,9 @@ def classify_pr( prev_checks_pending = legacy_check_identity_keys(prev.get("checks_pending")) current_checks_failing = legacy_check_identity_keys(checks["failing"]) current_checks_pending = legacy_check_identity_keys(checks["pending"]) - new_failing_checks = bool(current_checks_failing - prev_checks_failing) + new_failing_checks = bool( + current_checks_failing - prev_checks_failing - approval_held_keys + ) resolved_failing_checks = ( bool(prev_checks_failing - (current_checks_failing | current_checks_pending)) and not current_checks_pending diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index 8658dc6ba0..2c1c50a628 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -13,8 +13,18 @@ check that governs the merge, and the exact blockers, so the caller can react. - A merge only happens with `--merge` AND only when every readiness gate passes. - Merges use the repository's allowed method (squash preferred) and NEVER pass - `--admin`. This helper cannot bypass branch protection, resolve or reply to - review threads, force-push, or change settings. + `--admin` or `bypass_rules`. This helper cannot bypass branch protection, + resolve or reply to review threads, force-push, or change settings. +- A ready PR on the default branch merges through the async merge API with the + vetted head as `sha`, polled to a terminal status; on a base that requires a + merge queue it is enqueued instead (`enqueued` is queued, not merged). A host + without the endpoint (404) falls back to `gh pr merge` for a direct merge. + Any other base, and every `--auto` arm, keeps `gh pr merge`. A request still + pending at the poll bound is recorded under `--state-dir` (GitHub offers no + cancel), and every later run reports it as merge pending until it finishes. +- With `--stacked-prs`, a native stack layer is judged against the stack's + trunk and every open layer below it runs the same gate, since the async + merge lands them together. Without it a stack layer is held as before. - A PR authored by a dependency manager (Dependabot/Renovate-class) is held -- never merged -- unless `--allow-dependency` is passed. - A PR on an unprotected base (zero required reviews AND zero required contexts) @@ -23,7 +33,7 @@ repository's default branch -- the solo-owner repo the exemption exists for. A self-authored PR onto an unprotected NON-default base (a stack layer, or any feature-onto-feature merge) is held: the default branch's required checks never - governed it. + governed it. `--stacked-prs` replaces that hold for a native stack layer only. - A merge is held while a configured review bot still owes the LIVE head a review (`--review-bot-logins` with `--review-settle-minutes`, both or neither). A reviewer that re-reviews on push posts minutes after the head @@ -54,7 +64,8 @@ effective branch rules (`rules/branches`), the review decision, unresolved review threads, and the status-check rollup. -Exit codes: 0 ready (or merged), 10 not ready (blockers), 2 usage/runtime +Exit codes: 0 ready (or merged, enqueued, or auto-merge armed), 10 not ready +(blockers, or a merge request that failed or is still pending), 2 usage/runtime error, 3 owner out of scope (or no allowlist). Output is a single JSON object on stdout. """ @@ -64,12 +75,15 @@ import argparse import json import re -from collections.abc import Iterable +import time +from collections.abc import Callable, Iterable from dataclasses import dataclass, replace from datetime import UTC, datetime +from pathlib import Path from typing import Any, cast import babysit_repo_config as repo_policy +from babysit_state import resolve_state_dir, state_lock, write_state from babysit_checks import check_identity_key, classify_checks from babysit_classify import ( DEFAULT_FEEDBACK_CONFIG, @@ -95,6 +109,7 @@ fetch_pull_request_reviews, fetch_review_threads, gh_capture, + gh_http_status, gh_json, normalized_rest_author, parse_repo_number, @@ -112,6 +127,7 @@ is_json_array, is_json_object, json_array, + json_object, parse_allowed_owners, parse_csv_set, split_owner, @@ -146,6 +162,33 @@ # Matched on the job segment of the check name (`review / claude-review-status`). AI_REVIEW_CHECKS = ("claude-review-status", "claude-security-review-status") +# The async merge API (`PUT .../pulls/{n}/merge-async`) answers with a request +# UUID and runs the merge in the background; the gate polls it to a terminal +# status for at most this long. A request still pending past the bound is left +# to GitHub: the next run's PUT returns 409 with the same UUID and polls again. +ASYNC_MERGE_POLL_TIMEOUT_SECONDS = 60.0 +ASYNC_MERGE_POLL_INTERVAL_SECONDS = 3.0 +ASYNC_TERMINAL_STATUSES = frozenset({"merged", "enqueued", "failed"}) +ASYNC_UUID_RE = re.compile(r"^[0-9A-Za-z-]{1,64}$") + + +def _poll_sleep(seconds: float) -> None: + time.sleep(seconds) + + +def _poll_clock() -> float: + return time.monotonic() + + +MERGE_QUEUE_AUTO_HOLD = ( + "base branch requires a merge queue -- auto-merge is not armed over a " + "queue; the gate enqueues once the PR is fully ready" +) +STACK_AUTO_HOLD = ( + "stack layer -- auto-merge is not armed for a stack; the gate lands the " + "stack through the async merge API once every layer is ready" +) + @dataclass(frozen=True) class ReviewSettleConfig: @@ -940,6 +983,109 @@ def evaluate_review_settle( return [], result +def pull_request_stack(repo: str, number: int) -> dict[str, Any] | None: + """The PR's native stack membership (`stack` on the REST pull), or None. + + Only a native stack lands its lower layers when a layer merges; a PR that + merely targets another PR's branch has no `stack` object and merges into + that branch. Raises on a read failure so the caller holds rather than + guessing which of the two it is. + """ + data = gh_json(["api", f"repos/{repo}/pulls/{number}", "--jq", "{stack: .stack}"]) + stack = data.get("stack") if is_json_object(data) else None + return stack if is_json_object(stack) else None + + +def stack_members(repo: str, stack_number: int) -> list[dict[str, Any]]: + """The stack's pull requests, bottom to top, as GitHub lists them.""" + data = gh_json(["api", f"repos/{repo}/stacks/{stack_number}"]) + members = data.get("pull_requests") if is_json_object(data) else None + if not is_json_array(members) or not all(is_json_object(m) for m in members): + raise RuntimeError(f"unexpected stack payload for {repo} stack {stack_number}") + return cast(list[dict[str, Any]], members) + + +def evaluate_stack_layers( + repo: str, + number: int, + base_ref: str, + stack: dict[str, Any], + evaluate_layer: Callable[[int, str], dict[str, Any]], +) -> tuple[list[str], dict[str, Any]]: + """Run the full gate over every open layer the merge would land below this PR. + + Merging a stack layer through the async API lands every open layer below + it, so each one must pass the same gate this PR does. The chain is checked + as well: each open layer must target the head of the open layer below it, + the lowest the stack's trunk, so an order this code assumed but the API + did not state can only hold, never release. Every lower layer is pinned to + the head the stack listing reported, so a push between listing and + evaluation reads as a moved head. + """ + trunk = str(json_object(stack.get("base")).get("ref") or "") + stack_number = stack.get("number") + record: dict[str, Any] = { + "member": True, + "number": stack_number, + "trunk": trunk, + "landsLowerLayers": True, + "layers": [], + "aiReviewHolds": [], + } + try: + members = stack_members(repo, int(stack_number)) + except (RuntimeError, TypeError, ValueError, json.JSONDecodeError) as exc: + return [f"stack {stack_number!r} could not be read ({exc}) -- held"], record + numbers = [member.get("number") for member in members] + if number not in numbers: + return [f"stack {stack_number!r} does not list this PR -- held"], record + blockers: list[str] = [] + expected_base = trunk + for member in members[: numbers.index(number)]: + layer_number = member.get("number") + head = json_object(member.get("head")) + head_ref = str(head.get("ref") or "") + head_sha = str(head.get("sha") or "") + label = f"stack layer {repo}#{layer_number}" + if member.get("merged_at"): + continue + if member.get("state") != "open" or not isinstance(layer_number, int): + blockers.append(f"{label} is closed without merging -- held") + continue + if not EXPECTED_HEAD_RE.match(head_sha): + blockers.append(f"{label} reports no head SHA to pin -- held") + continue + layer = evaluate_layer(layer_number, head_sha) + record["layers"].append( + { + "pr": f"{repo}#{layer_number}", + "number": layer_number, + "headRefOid": layer.get("headRefOid"), + "baseRef": layer.get("baseRef"), + "ready": layer.get("ready"), + "blockers": layer.get("blockers"), + } + ) + blockers.extend(f"{label}: {b}" for b in layer.get("blockers") or []) + # A layer's AI-review holds bind wherever the top layer's do (`--auto`): + # they live outside `blockers`, so they are carried separately. + record["aiReviewHolds"].extend( + f"{label}: {hold}" for hold in layer.get("aiReviewHolds") or [] + ) + if layer.get("baseRef") != expected_base: + blockers.append( + f"{label} targets {layer.get('baseRef')!r}, not {expected_base!r} " + "-- the stack chain is broken; held" + ) + expected_base = head_ref + if base_ref != expected_base: + blockers.append( + f"this PR targets {base_ref!r}, not the open layer below it " + f"({expected_base!r}) -- the stack chain is broken; held" + ) + return blockers, record + + def evaluate( repo: str, number: int, @@ -951,6 +1097,8 @@ def evaluate( tier: AutopilotMergeTierConfig | None = None, extra_dependency_manager_logins: frozenset[str] = frozenset(), settle: ReviewSettleConfig | None = None, + stacked: bool = False, + rules_base: str | None = None, ) -> dict[str, Any]: owner = split_owner(repo) # `closingIssuesReferences` is requested only when the autopilot merge tier @@ -984,7 +1132,22 @@ def evaluate( checks_by_key: dict[tuple[str, str, str], dict[str, Any]] = { check_identity_key(check): check for check in checks["checks"] } - rules = branch_rules(repo, str(pr.get("baseRefName") or "main")) + # Stack membership is read only under `stacked` (the opt-in), so the default + # gate makes no request it did not make before. A layer above the bottom is + # governed by the stack's trunk, not its literal base: GitHub determines a + # stack's merge requirements from the bottom layer's base branch. + base_ref = str(pr.get("baseRefName") or "") + stack: dict[str, Any] | None = None + stack_error: str | None = None + if stacked: + try: + stack = pull_request_stack(repo, number) + except (RuntimeError, json.JSONDecodeError) as exc: + stack_error = str(exc) + trunk = str(json_object(json_object(stack).get("base")).get("ref") or "") + stack_mode = bool(stack and trunk and base_ref and base_ref != trunk) + landing_base = rules_base or (trunk if stack_mode else base_ref) + rules = branch_rules(repo, landing_base or "main") head = pr.get("headRefOid") head_matches = ( None @@ -1107,11 +1270,28 @@ def evaluate( ) if unmet_only_running: waiting.append(blockers[-1]) - if rules.get("mergeQueueRequired"): + merge_queue_required = bool(rules.get("mergeQueueRequired")) + if stack_error is not None: + blockers.append( + f"stack membership could not be read ({stack_error}) -- held: a stack " + "layer's merge lands the layers below it, so an unknown membership " + "cannot be merged" + ) + if stack_mode and merge_queue_required: blockers.append( - "base branch requires a merge queue -- a direct merge is not allowed; " - "add to the queue" + f"stack trunk {trunk!r} requires a merge queue -- held: merge-queue " + "support for stacks is not relied on yet; merge the stack by hand" ) + elif merge_queue_required and not stack_mode and not rules_base: + # The async API enqueues a PR on the default branch. Any other base keeps + # the prior hold: an unconfirmed stack layer there could enqueue the + # layers below it with them. + default_branch = repository_default_branch(repo) + if not default_branch or base_ref != default_branch: + blockers.append( + "base branch requires a merge queue and is not the default branch " + "-- a direct merge is not allowed; add to the queue by hand" + ) # Enforced in this read-only pass, not only at merge time: the wrapper's # documented purpose is reporting readiness so the caller can react, and a # signature hold discovered only under --merge defeats that. @@ -1197,7 +1377,8 @@ def evaluate( # evaluated for this merge at all, and the PR lands on an integration branch # instead of passing the gate. A stacked pull request is exactly that shape # (self-authored, base = the layer below), but so is any feature-onto-feature - # merge. + # merge. Under `stacked`, a native stack layer is judged against its trunk + # instead, where the landing base is what this hold compares. # # Evaluated last, after the settle and tier blockers, so `not blockers` is the # COMPLETE set: the lookup below is a network call, and a PR already held for @@ -1209,16 +1390,45 @@ def evaluate( and author_is_self and not allow_unprotected ): - base_ref = str(pr.get("baseRefName") or "") default_branch = repository_default_branch(repo) - if default_branch and base_ref and base_ref != default_branch: + if default_branch and landing_base and landing_base != default_branch: blockers.append( - f"base branch {base_ref!r} is unprotected (0 required reviews AND " + f"base branch {landing_base!r} is unprotected (0 required reviews AND " "0 required contexts) and is not the default branch " f"{default_branch!r} -- the default branch's required checks never " "governed this merge -- held (pass --allow-unprotected to override)" ) + # The layers below a stack layer land with it, so each runs the full gate -- + # only once this PR is otherwise ready, for the same per-cycle cost reason. + stack_result: dict[str, Any] = { + "enabled": stacked, + "member": stack is not None, + "landsLowerLayers": stack_mode, + } + if stack_mode and stack is not None and not blockers: + + def evaluate_layer(layer_number: int, layer_head: str) -> dict[str, Any]: + return evaluate( + repo, + layer_number, + layer_head, + allowed, + self_logins, + allow_dependency, + allow_unprotected, + tier, + extra_dependency_manager_logins, + settle, + rules_base=trunk, + ) + + layer_blockers, stack_result = evaluate_stack_layers( + repo, number, base_ref, stack, evaluate_layer + ) + stack_result["enabled"] = True + blockers.extend(layer_blockers) + ready = not blockers # `--auto` needs both AI review checks at SUCCESS in the rollup, which is the # live head's. A draft skips both lanes, so neither an absent nor a SKIPPED @@ -1235,11 +1445,21 @@ def evaluate( ) or any(c["effective_state"] != "SUCCESS" for c in matches) ] + ai_review_holds += stack_result.get("aiReviewHolds") or [] auto_blockers = [b for b in blockers if b not in waiting] + ai_review_holds + # GitHub auto-merge is armed only over a plain direct merge; a queue or a + # stack is merged through the async API once the PR is fully ready. + if not ready and merge_queue_required: + auto_blockers.append(MERGE_QUEUE_AUTO_HOLD) + if not ready and stack_mode: + auto_blockers.append(STACK_AUTO_HOLD) return { "pr": f"{repo}#{number}", "autopilotMergeTier": tier_result, "reviewSettle": settle_result, + "stack": stack_result, + "landingBase": landing_base, + "mergeAction": "merge_queue" if merge_queue_required else "direct_merge", "url": pr.get("url"), "title": pr.get("title"), "author": author_login, @@ -1270,6 +1490,7 @@ def evaluate( "ready": ready, "blockers": blockers, "autoMerge": {"ready": not auto_blockers, "blockers": auto_blockers}, + "aiReviewHolds": ai_review_holds, } @@ -1299,6 +1520,366 @@ def allowed_method(repo: str, requested: str | None) -> str: raise RuntimeError(f"no merge method enabled on {repo}") +def _async_payload(text: str) -> dict[str, Any]: + try: + data = json.loads(text) if text.strip() else None + except json.JSONDecodeError: + return {} + return data if is_json_object(data) else {} + + +def request_async_merge( + repo: str, + number: int, + *, + sha: str | None, + merge_action: str, + method: str | None, +) -> dict[str, Any]: + """PUT one async merge request; never sets `bypass_rules`. + + `sha` is the vetted head, the server-side equivalent of + `--match-head-commit`: GitHub cancels the merge if the head moved. A 409 + means a request is already pending for this PR and carries its UUID. The + HTTP status of a failure comes from `gh`'s own message, since `gh api` + exits 1 for every non-2xx response. + """ + cmd = [ + "api", + "-X", + "PUT", + f"repos/{repo}/pulls/{number}/merge-async", + "-f", + f"merge_action={merge_action}", + "-F", + "bypass_rules=false", + ] + if sha: + cmd += ["-f", f"sha={sha}"] + if method and merge_action == "direct_merge": + cmd += ["-f", f"merge_method={method}"] + proc = gh_capture(cmd) + payload = _async_payload(proc.stdout) + details = json_object(payload.get("details")) + return { + "httpStatus": None if proc.returncode == 0 else gh_http_status(proc.stderr), + "ok": proc.returncode == 0, + "status": str(payload.get("status") or ""), + "uuid": str(details.get("uuid") or ""), + "message": str(details.get("message") or payload.get("message") or ""), + "stderr": proc.stderr.strip(), + } + + +def poll_async_merge( + repo: str, + number: int, + uuid: str, + *, + timeout_seconds: float = ASYNC_MERGE_POLL_TIMEOUT_SECONDS, + interval_seconds: float = ASYNC_MERGE_POLL_INTERVAL_SECONDS, + sleep: Callable[[float], None] | None = None, + clock: Callable[[], float] | None = None, +) -> dict[str, Any]: + """Poll one async merge request until it is terminal or the bound elapses.""" + if not ASYNC_UUID_RE.match(uuid): + return { + "status": "", + "message": f"unusable async merge request id {uuid!r}", + "readError": True, + "expired": False, + } + sleep = sleep or _poll_sleep + clock = clock or _poll_clock + deadline = clock() + timeout_seconds + while True: + result = read_async_merge(repo, number, uuid) + if ( + result["status"] in ASYNC_TERMINAL_STATUSES + or result["readError"] + or clock() >= deadline + ): + return result + sleep(interval_seconds) + + +def read_async_merge(repo: str, number: int, uuid: str) -> dict[str, Any]: + """One read of an async merge request. `expired` is a 404: GitHub keeps a + result for 24 hours after its last update and then forgets the UUID.""" + try: + payload = gh_json(["api", f"repos/{repo}/pulls/{number}/merge-async/{uuid}"]) + except (RuntimeError, json.JSONDecodeError) as exc: + return { + "status": "", + "message": f"could not read the request: {exc}", + "readError": True, + "expired": gh_http_status(str(exc)) == 404, + } + payload = payload if is_json_object(payload) else {} + return { + "status": str(payload.get("status") or ""), + "message": str(json_object(payload.get("details")).get("message") or ""), + "readError": False, + "expired": False, + } + + +def pull_request_merged(repo: str, number: int) -> bool | None: + """Whether GitHub reads the PR as merged; None when it cannot be read.""" + try: + data = gh_json( + ["api", f"repos/{repo}/pulls/{number}", "--jq", "{merged: .merged}"] + ) + except (RuntimeError, json.JSONDecodeError): + return None + merged = data.get("merged") if is_json_object(data) else None + return merged if isinstance(merged, bool) else None + + +def async_merge( + repo: str, + number: int, + *, + sha: str | None, + merge_action: str, + method: str | None, +) -> dict[str, Any]: + """Request an async merge or enqueue, poll it, and verify a reported merge. + + Returns the `merge` record. `endpointMissing` marks a 404 on the request + itself (a host without the endpoint, such as an older GitHub Enterprise + Server): the PR was just read, so the 404 is the endpoint's, not the PR's. + """ + request = request_async_merge( + repo, number, sha=sha, merge_action=merge_action, method=method + ) + record: dict[str, Any] = { + "attempted": True, + "auto": False, + "api": "merge-async", + "mergeAction": merge_action, + "httpStatus": request["httpStatus"], + "uuid": request["uuid"] or None, + "status": request["status"] or None, + "message": request["message"], + "success": False, + "endpointMissing": not request["ok"] and request["httpStatus"] == 404, + } + if not request["ok"] and request["stderr"]: + record["stderr"] = request["stderr"] + accepted = request["ok"] or request["httpStatus"] == 409 + if not accepted: + return record + status = request["status"] + if status not in ASYNC_TERMINAL_STATUSES: + polled = poll_async_merge(repo, number, request["uuid"]) + status = polled["status"] + record["message"] = polled["message"] or record["message"] + record["status"] = status or None + if status == "merged": + verified = pull_request_merged(repo, number) + # None (the read failed) is surfaced as unconfirmed, never as a merge. + record["verifiedMerged"] = verified + record["success"] = verified is True + if verified is False: + record["message"] = ( + "the async merge API reported merged but the pull request reads " + "unmerged -- not counted as merged" + ) + elif verified is None: + record["message"] = ( + "the async merge API reported merged but the pull request could " + "not be read back -- unconfirmed; re-run the read-only check" + ) + elif status == "enqueued": + record["success"] = merge_action == "merge_queue" + elif status == "pending": + record["message"] = ( + f"still pending after {int(ASYNC_MERGE_POLL_TIMEOUT_SECONDS)}s; the " + "next run's request returns this one and polls it again" + ) + return record + + +def _open_lower_layers( + repo: str, number: int, stack_number: Any +) -> list[tuple[int, str]]: + """`(number, head sha)` of every open, unmerged layer below this PR, read now.""" + members = stack_members(repo, int(stack_number)) + numbers = [member.get("number") for member in members] + if number not in numbers: + raise RuntimeError(f"stack {stack_number!r} no longer lists this PR") + return [ + (int(member["number"]), str(json_object(member.get("head")).get("sha") or "")) + for member in members[: numbers.index(number)] + if not member.get("merged_at") and member.get("state") == "open" + ] + + +def _evaluated_layers(result: dict[str, Any]) -> list[tuple[int, str]]: + return [ + (int(layer["number"]), str(layer.get("headRefOid") or "")) + for layer in json_array(json_object(result.get("stack")).get("layers")) + if is_json_object(layer) and isinstance(layer.get("number"), int) + ] + + +def stack_drift(repo: str, number: int, result: dict[str, Any]) -> str | None: + """Why the lower layers no longer match what the gate evaluated, or None. + + The request's `sha` pins only this PR, so the layers below are re-read + immediately before the request: a push, a new layer, or a closed one since + evaluation refuses the merge instead of landing an unvetted head. + """ + try: + live = _open_lower_layers( + repo, number, json_object(result["stack"]).get("number") + ) + except (RuntimeError, KeyError, TypeError, ValueError, json.JSONDecodeError) as exc: + return f"stack could not be re-read before merging ({exc}) -- held" + evaluated = _evaluated_layers(result) + if live != evaluated: + return ( + f"the stack's open lower layers changed since evaluation (evaluated " + f"{evaluated}, now {live}) -- held; re-run the gate" + ) + return None + + +def verify_stack_landed(repo: str, result: dict[str, Any]) -> dict[str, Any]: + """Whether every evaluated lower layer merged at the head the gate evaluated. + + `verified` is None when the stack cannot be read back. A layer GitHub + reports at a different head lands as a mismatch for a human to judge, + whatever the cause. + """ + evaluated = _evaluated_layers(result) + try: + members = stack_members(repo, int(json_object(result["stack"]).get("number"))) + except (RuntimeError, KeyError, TypeError, ValueError, json.JSONDecodeError) as exc: + return { + "verified": None, + "mismatches": [], + "message": f"stack unreadable: {exc}", + } + by_number = {member.get("number"): member for member in members} + mismatches = [] + for layer_number, head in evaluated: + member = json_object(by_number.get(layer_number)) + landed_head = str(json_object(member.get("head")).get("sha") or "") + if not member.get("merged_at") or landed_head != head: + mismatches.append( + { + "pr": f"{repo}#{layer_number}", + "evaluatedHead": head, + "reportedHead": landed_head or None, + "merged": bool(member.get("merged_at")), + } + ) + return {"verified": not mismatches, "mismatches": mismatches} + + +PENDING_MERGES_FILE = "merge-requests.json" +# GitHub keeps an async merge result for 24 hours after its last update; a +# record older than that (plus slack) can no longer be read and is dropped. +PENDING_MERGE_MAX_AGE_SECONDS = 25 * 60 * 60 + + +def _load_pending(path: Path) -> dict[str, Any]: + try: + data = json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError: + return {} + except (OSError, ValueError) as exc: + raise RuntimeError( + f"pending merge records unreadable at {path}: {exc}" + ) from exc + requests = data.get("requests") if is_json_object(data) else None + if not is_json_object(requests): + raise RuntimeError(f"pending merge records malformed at {path}") + return cast(dict[str, Any], requests) + + +def update_pending(path: Path, key: str, entry: dict[str, Any] | None) -> None: + """Record (or with None, clear) the one live async merge request for a PR.""" + with state_lock(path): + requests = _load_pending(path) + if entry is None: + if key not in requests: + return + requests.pop(key) + else: + requests[key] = entry + write_state(path, {"schema_version": 1, "requests": requests}) + + +def check_pending_request( + repo: str, number: int, path: Path, *, now: datetime | None = None +) -> dict[str, Any] | None: + """The PR's recorded async merge request as GitHub reports it now. + + A request left pending stays live on GitHub: it can still merge after a + hold appears that would refuse a new one. There is no route to cancel it, + so every later run reads it first. A terminal or expired request clears + the record; an unreadable one stays recorded and counts as pending. + """ + key = f"{repo}#{number}" + with state_lock(path): + entry = _load_pending(path).get(key) + if not is_json_object(entry): + return None + requested = parse_github_timestamp(str(entry.get("requestedAt") or "")) + age = ( + ((now or datetime.now(UTC)) - requested).total_seconds() if requested else None + ) + if age is not None and age > PENDING_MERGE_MAX_AGE_SECONDS: + update_pending(path, key, None) + return {**entry, "status": "expired", "message": "older than GitHub retains"} + current = read_async_merge(repo, number, str(entry.get("uuid") or "")) + report = { + **entry, + "status": current["status"] or None, + "message": current["message"], + } + if current["expired"]: + report["status"] = "expired" + if current["expired"] or current["status"] in ASYNC_TERMINAL_STATUSES: + update_pending(path, key, None) + return report + + +def _record_pending( + path: Path, + repo: str, + number: int, + record: dict[str, Any], + pin: str | None, + result: dict[str, Any], +) -> None: + """Keep a request that is still live on GitHub; forget a finished one.""" + key = f"{repo}#{number}" + live = ( + bool(record.get("uuid")) and record.get("status") not in ASYNC_TERMINAL_STATUSES + ) + entry = ( + { + "uuid": record["uuid"], + "head": pin or result.get("headRefOid"), + "mergeAction": record.get("mergeAction"), + "requestedAt": datetime.now(UTC).isoformat().replace("+00:00", "Z"), + } + if live + else None + ) + try: + update_pending(path, key, entry) + except (RuntimeError, OSError) as exc: + result["pendingRecordError"] = ( + f"could not record the pending merge request ({exc}); a later run will " + "not know it is live" + ) + + def build_settle(logins: Iterable[str], minutes: str) -> ReviewSettleConfig | None: """The review-settle hold, or None when it would be inert. @@ -1405,6 +1986,23 @@ def main() -> int: "PR anywhere, or a self PR onto a non-default base" ), ) + parser.add_argument( + "--stacked-prs", + action="store_true", + help=( + "treat a native stack layer as mergeable: judge it against the stack's " + "trunk and run the full gate over every open layer below it, which the " + "async merge lands with it. Unset, a stack layer is held as before" + ), + ) + parser.add_argument( + "--state-dir", + default=None, + help=( + "babysit state directory; records an async merge request left pending " + "so every later run reports it as merge pending until GitHub finishes it" + ), + ) parser.add_argument( "--allow-unpinned-head", action="store_true", @@ -1587,6 +2185,13 @@ def _refuse(message: str, code: int, **envelope: object) -> int: if args.auto and not (args.merge and args.expected_head): return _refuse("--auto requires --merge and --expected-head", 2) + pending_path: Path | None = None + if args.state_dir is not None: + try: + pending_path = resolve_state_dir(args.state_dir) / PENDING_MERGES_FILE + except ValueError as exc: + return _refuse(str(exc), 2) + # The target repository's policy, read from its default branch with the flags # as the deprecated `userConfig` fallback. Unreadable policy refuses the # check as well as the merge: a verdict computed without the repository's @@ -1644,6 +2249,13 @@ def _refuse(message: str, code: int, **envelope: object) -> int: extra_dependency_manager_logins = policy.extra_dependency_manager_logins + prior: dict[str, Any] | None = None + if pending_path is not None: + try: + prior = check_pending_request(repo, number, pending_path) + except (RuntimeError, OSError) as exc: + return _refuse(f"pending merge record unreadable: {exc}", 2) + try: result = evaluate( repo, @@ -1656,6 +2268,7 @@ def _refuse(message: str, code: int, **envelope: object) -> int: tier, extra_dependency_manager_logins=extra_dependency_manager_logins, settle=settle, + stacked=args.stacked_prs, ) except (RuntimeError, ValueError, json.JSONDecodeError) as exc: # Surface any gh/parse failure as JSON rather than a traceback. @@ -1666,6 +2279,25 @@ def _refuse(message: str, code: int, **envelope: object) -> int: result["merged"] = False result["merge"] = None + if prior is not None: + result["pendingMergeRequest"] = prior + if prior.get("status") in (None, "pending"): + # Live (or unreadable) on GitHub: it can still merge whatever this run + # found, so no verdict here may read as settled and no new request goes. + hold = ( + f"an async merge request ({prior.get('uuid')}) sent at " + f"{prior.get('requestedAt')} is still pending on GitHub and can " + "still merge regardless of this verdict -- merge pending; no new " + "request is sent" + ) + result["ready"] = False + result["blockers"].insert(0, hold) + result["autoMerge"] = {"ready": False, "blockers": [hold]} + result["action"] = "merge-pending" + result["merge"] = {"attempted": False, "reason": "merge pending"} + print(json.dumps(result, indent=2)) + return 10 + if not args.merge: print(json.dumps(result, indent=2)) return 0 if result["ready"] else 10 @@ -1705,14 +2337,82 @@ def _refuse(message: str, code: int, **envelope: object) -> int: return 2 result["mergeMethod"] = method - merge_cmd = ["pr", "merge", str(number), "-R", repo, f"--{method}"] - if arm_auto: - merge_cmd.append("--auto") # Atomic head pin: GitHub refuses the merge unless the head still equals the # exact full SHA we vetted, closing the preflight-to-merge TOCTOU window. vetted_head = result.get("headRefOid") - if args.expected_head and isinstance(vetted_head, str) and vetted_head: - merge_cmd += ["--match-head-commit", vetted_head] + pin = ( + vetted_head + if args.expected_head and isinstance(vetted_head, str) and vetted_head + else None + ) + + # A ready PR merges through the async merge API: REST (so it works where + # GraphQL is refused), and the only API that enqueues or lands a stack. It is + # used on the default branch, for a queue, and for a stack; any other base + # keeps `gh pr merge`, because an unconfirmed stack layer there would land + # the layers below it. Auto-merge has no async form and keeps `gh pr merge`. + if not arm_auto: + stack_lands = bool(json_object(result.get("stack")).get("landsLowerLayers")) + queue = result.get("mergeAction") == "merge_queue" + use_async = stack_lands or queue + if not use_async: + default_branch = repository_default_branch(repo) + use_async = bool(default_branch) and result.get("baseRef") == default_branch + if use_async: + if queue: + result["action"] = "enqueue" + if stack_lands and (drift := stack_drift(repo, number, result)): + result["ready"] = False + result["blockers"].append(drift) + result["merge"] = {"attempted": False, "reason": drift} + print(json.dumps(result, indent=2)) + return 10 + record = async_merge( + repo, + number, + sha=pin, + merge_action="merge_queue" if queue else "direct_merge", + method=method, + ) + if not record["endpointMissing"] or stack_lands or queue: + if record["endpointMissing"]: + record["message"] = ( + "the async merge endpoint returned 404 on this host; a queue " + "or stack merge has no other API -- held" + ) + result["merge"] = record + result["merged"] = record["success"] and record["status"] == "merged" + result["enqueued"] = ( + record["success"] and record["status"] == "enqueued" + ) + result["mergeUnconfirmed"] = ( + record["status"] == "merged" + and record.get("verifiedMerged") is None + ) + exit_code = 0 if record["success"] else 10 + if result["merged"] and stack_lands: + verification = verify_stack_landed(repo, result) + result["stackVerification"] = verification + if verification["verified"] is not True: + verification["message"] = verification.get("message") or ( + "a lower stack layer did not land at the head the gate " + "evaluated -- escalate to a human" + ) + exit_code = 10 + if pending_path is not None: + _record_pending(pending_path, repo, number, record, pin, result) + print(json.dumps(result, indent=2)) + return exit_code + result["asyncFallback"] = ( + "the async merge endpoint returned 404 on this host; merged with " + "gh pr merge instead" + ) + + merge_cmd = ["pr", "merge", str(number), "-R", repo, f"--{method}"] + if arm_auto: + merge_cmd.append("--auto") + if pin: + merge_cmd += ["--match-head-commit", pin] proc = gh_capture(merge_cmd) result["merge"] = { "attempted": True, diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/guard_contract.py b/plugins/source-control/skills/babysit-prs/scripts/tests/guard_contract.py index 07d533a60d..a9f657594e 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/guard_contract.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/guard_contract.py @@ -1253,10 +1253,14 @@ def _thread( path=MERGE_CLI, wrapper=MERGE_WRAPPER, mutation=CONDITIONAL, - mutates_what="merges the PR on GitHub, or with --auto arms auto-merge", + mutates_what=( + "merges the PR on GitHub, or with --auto arms auto-merge; with --state-dir " + "it also keeps a local pending-merge record" + ), gate="--merge (absent: readiness check only, exit 0 ready / 10 not ready)", claim=( - "Without --merge this is a readiness reporter. With it, the TOCTOU guard " + "Without --merge this is a readiness reporter on GitHub; with --state-dir it " + "may still clear a finished pending-merge record locally. With --merge, the TOCTOU guard " "requires --expected-head unless --allow-unpinned-head is passed -- and the " "scripts/ wrapper refuses that override outright." ), diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py index 55530267fe..9d4f439ffe 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_delta.py @@ -374,6 +374,60 @@ def test_checks_stuck_field_always_present(self) -> None: self.assertEqual(result["checks"]["stuck"], []) +HELD_RUN = { + "__typename": "CheckRun", + "name": "test", + "workflowName": "ci", + "status": "COMPLETED", + "conclusion": "ACTION_REQUIRED", +} +FAILED_RUN = { + "__typename": "CheckRun", + "name": "lint", + "workflowName": "ci", + "status": "COMPLETED", + "conclusion": "FAILURE", +} + + +class ApprovalHeldCheckTests(unittest.TestCase): + """A run held for approval escalates; it is not a failure a worker can fix.""" + + def test_held_run_is_material_and_not_a_failing_blocker(self) -> None: + pr = make_pr(mergeStateStatus="BLOCKED", statusCheckRollup=[HELD_RUN]) + result = classify(pr, None) + [held] = result["checks"]["approval_held"] + self.assertEqual((held["name"], held["class"]), ("test", "awaiting_approval")) + self.assertTrue( + any("held for approval" in f for f in result["material_findings"]), + result["material_findings"], + ) + self.assertFalse( + any("failing check" in b for b in result["blockers"]), result["blockers"] + ) + # The rollup still counts it failing, so the merge gate keeps holding. + self.assertEqual(result["checks"]["failing"], ["test"]) + + def test_a_real_failure_beside_it_is_still_one_blocker(self) -> None: + pr = make_pr( + mergeStateStatus="BLOCKED", statusCheckRollup=[HELD_RUN, FAILED_RUN] + ) + result = classify(pr, None) + self.assertIn("1 failing check(s)", result["blockers"]) + + def test_a_newly_held_run_dispatches_no_worker(self) -> None: + held = classify( + make_pr(mergeStateStatus="BLOCKED", statusCheckRollup=[HELD_RUN]), + make_prev(), + ) + self.assertNotIn("checks_changed", held["needs_worker_reasons"]) + failed = classify( + make_pr(mergeStateStatus="BLOCKED", statusCheckRollup=[FAILED_RUN]), + make_prev(), + ) + self.assertIn("checks_changed", failed["needs_worker_reasons"]) + + class SuppressibleDeltaArmTests(unittest.TestCase): """Each suppressible arm: reason present only when NOT direct-gate-ready.""" diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py index b131fccdfe..8f508ac5fb 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py @@ -1341,13 +1341,18 @@ def _main( "ready": ready, "blockers": ["pending checks: ci-status"], "headRefOid": HEAD, + "baseRef": "main", + "mergeAction": "direct_merge", + "stack": {"enabled": False, "member": False, "landsLowerLayers": False}, "autoMerge": {"ready": auto_ready, "blockers": []}, } calls: list[list[str]] = [] def capture(cmd: list[str]) -> Any: calls.append(cmd) - return mock.Mock(returncode=0, stdout="", stderr="") + merged = json.dumps({"status": "merged", "details": {"message": "ok"}}) + stdout = merged if "merge-async" in " ".join(cmd) else "" + return mock.Mock(returncode=0, stdout=stdout, stderr="") argv = ["babysit_merge.py", "owner/repo#1", "--allowed-owners", "owner", *extra] with ( @@ -1355,6 +1360,8 @@ def capture(cmd: list[str]) -> Any: mock.patch.object(merge, "evaluate", return_value=result), mock.patch.object(merge, "allowed_method", return_value="squash"), mock.patch.object(merge, "gh_capture", side_effect=capture), + mock.patch.object(merge, "repository_default_branch", return_value="main"), + mock.patch.object(merge, "pull_request_merged", return_value=True), contextlib.redirect_stdout(io.StringIO()) as out, ): code = merge.main() @@ -1390,7 +1397,10 @@ def test_auto_merges_a_ready_pr_synchronously(self) -> None: code, calls = self._main(True, *args, ready=True) self.assertEqual(code, 0) self.assertTrue(self.output["merged"]) - self.assertNotIn("--auto", calls[0]) + self.assertEqual(self.output["action"], "merge") + [cmd] = calls + self.assertNotIn("--auto", cmd) + self.assertIn("repos/owner/repo/pulls/1/merge-async", cmd) def test_auto_without_merge_and_pin_is_refused(self) -> None: self.assertEqual(self._main(True, "--auto")[0], 2) @@ -1497,6 +1507,9 @@ def _run_stubbed_gate(self, *flags: str) -> tuple[int, dict[str, Any], mock.Mock "gh_capture", return_value=mock.Mock(returncode=0, stdout="", stderr=""), ), + # Not the default branch: the merge stays on `gh pr merge`, which is + # the path these method-resolution tests read. + mock.patch.object(merge, "repository_default_branch", return_value=None), contextlib.redirect_stdout(out), ): code = merge.main() diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py new file mode 100644 index 0000000000..8bfd6b549d --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py @@ -0,0 +1,703 @@ +"""The merge gate's async merge path, merge-queue enqueue, and stacked PRs. + +Expected request fields, statuses, and HTTP semantics come from GitHub's +"Merge a pull request asynchronously" and "Get the result of an asynchronous +merge" REST docs: `sha` pins the head, `merge_method` is direct-only, +`merge_action` is `direct_merge` or `merge_queue`, 409 returns the pending +request's UUID, and `enqueued` means queued, not merged. Stack membership and +the trunk rule come from "About stacked pull requests" and the stacks REST +endpoints. Every gh seam is stubbed; no real gh process is spawned. +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import sys +import tempfile +import unittest +from typing import Any +from unittest import mock + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) + +from repo_config_fake import RepoConfigFake +import babysit_merge as merge + +HEAD = "c" * 40 +LAYER1 = "1" * 40 +LAYER2 = "2" * 40 +UUID = "3f2c9a1e-0000-4000-8000-000000000001" +SELF = "solo-owner" +APPROVED_RULES = [ + {"type": "pull_request", "parameters": {"required_approving_review_count": 1}} +] +QUEUE_RULES = [*APPROVED_RULES, {"type": "merge_queue", "parameters": {}}] + + +def _proc(payload: Any = None, *, returncode: int = 0, stderr: str = "") -> mock.Mock: + stdout = "" if payload is None else json.dumps(payload) + return mock.Mock(returncode=returncode, stdout=stdout, stderr=stderr) + + +def _async(status: str, message: str = "") -> dict[str, Any]: + return {"status": status, "details": {"message": message, "uuid": UUID}} + + +class AsyncMergeHarness(unittest.TestCase): + """Runs `main --merge` over a stubbed ready verdict and records gh calls.""" + + def setUp(self) -> None: + RepoConfigFake().install(self) + self.captures: list[list[str]] = [] + self.polls: list[list[str]] = [] + self.sleeps: list[float] = [] + + def _run( + self, + put: mock.Mock | list[mock.Mock], + polls: list[dict[str, Any]] | None = None, + *, + merged: bool | None = True, + base: str = "main", + default_branch: str | None = "main", + merge_action: str = "direct_merge", + stack_lands: bool = False, + stack_listings: list[list[dict[str, Any]]] | None = None, + extra: tuple[str, ...] = (), + clock: list[float] | None = None, + ready: bool = True, + merge_flag: bool = True, + ) -> int: + verdict = { + "ready": ready, + "blockers": [] if ready else ["1 unresolved review thread(s) [reviewer]"], + "headRefOid": HEAD, + "baseRef": base, + "mergeAction": merge_action, + "stack": { + "enabled": True, + "member": stack_lands, + "landsLowerLayers": stack_lands, + "number": 7, + "layers": ( + [{"pr": "owner/repo#2", "number": 2, "headRefOid": LAYER2}] + if stack_lands + else [] + ), + }, + "autoMerge": {"ready": ready, "blockers": []}, + } + puts = list(put) if isinstance(put, list) else [put] + poll_answers = list(polls or []) + listings = list(stack_listings or []) + + def capture(cmd: list[str]) -> Any: + self.captures.append(cmd) + if "merge-async" in " ".join(cmd): + return puts.pop(0) + return _proc() + + def gh_json(args: list[str]) -> Any: + if "merge-async/" in args[1]: + self.polls.append(args) + answer = poll_answers.pop(0) + if isinstance(answer, Exception): + raise answer + return answer + if args[1] == "repos/owner/repo/stacks/7": + self.stack_reads += 1 + return {"number": 7, "pull_requests": listings.pop(0)} + raise AssertionError(f"unexpected gh_json call: {args}") + + self.stack_reads = 0 + ticks = iter(clock or [0.0] * 50) + argv = [ + "babysit_merge.py", + "owner/repo#1", + "--allowed-owners", + "owner", + *(("--merge", "--expected-head", HEAD) if merge_flag else ()), + *extra, + ] + out = io.StringIO() + with ( + mock.patch.object(sys, "argv", argv), + mock.patch.object(merge, "evaluate", return_value=verdict), + mock.patch.object(merge, "allowed_method", return_value="squash"), + mock.patch.object(merge, "gh_capture", side_effect=capture), + mock.patch.object(merge, "gh_json", side_effect=gh_json), + mock.patch.object( + merge, "repository_default_branch", return_value=default_branch + ), + mock.patch.object(merge, "pull_request_merged", return_value=merged), + mock.patch.object(merge, "_poll_sleep", side_effect=self.sleeps.append), + mock.patch.object(merge, "_poll_clock", side_effect=lambda: next(ticks)), + contextlib.redirect_stdout(out), + ): + code = merge.main() + self.output = json.loads(out.getvalue()) + return code + + def _put(self) -> list[str]: + [cmd] = [c for c in self.captures if "merge-async" in " ".join(c)] + return cmd + + def _fields(self, cmd: list[str]) -> dict[str, str]: + pairs = [cmd[i + 1] for i, arg in enumerate(cmd) if arg in ("-f", "-F")] + return dict(pair.split("=", 1) for pair in pairs) + + +class DirectMergeGoesThroughTheAsyncApi(AsyncMergeHarness): + def test_accepted_request_is_polled_to_merged(self) -> None: + code = self._run( + _proc(_async("pending")), [_async("pending"), _async("merged")] + ) + self.assertEqual(code, 0) + self.assertTrue(self.output["merged"]) + self.assertEqual(self.output["action"], "merge") + cmd = self._put() + self.assertEqual( + cmd[:4], ["api", "-X", "PUT", "repos/owner/repo/pulls/1/merge-async"] + ) + self.assertEqual( + self._fields(cmd), + { + "merge_action": "direct_merge", + "bypass_rules": "false", + "sha": HEAD, + "merge_method": "squash", + }, + ) + self.assertEqual(len(self.polls), 2) + self.assertTrue(self.polls[0][1].endswith(f"/merge-async/{UUID}")) + self.assertEqual(self.sleeps, [merge.ASYNC_MERGE_POLL_INTERVAL_SECONDS]) + self.assertFalse(any(c[:2] == ["pr", "merge"] for c in self.captures)) + + def test_already_merged_answer_needs_no_poll(self) -> None: + code = self._run( + _proc({"status": "merged", "details": {"message": "m", "sha": "d" * 40}}) + ) + self.assertEqual(code, 0) + self.assertTrue(self.output["merged"]) + self.assertEqual(self.polls, []) + + def test_conflict_polls_the_pending_request_it_names(self) -> None: + conflict = _proc( + _async("pending"), returncode=1, stderr="gh: Conflict (HTTP 409)" + ) + code = self._run(conflict, [_async("merged")]) + self.assertEqual(code, 0) + self.assertEqual(self.output["merge"]["httpStatus"], 409) + self.assertTrue(self.polls[0][1].endswith(UUID)) + + def test_failed_request_is_not_a_merge(self) -> None: + code = self._run(_proc(_async("pending")), [_async("failed", "head moved")]) + self.assertEqual(code, 10) + self.assertFalse(self.output["merged"]) + self.assertEqual(self.output["merge"]["status"], "failed") + self.assertEqual(self.output["merge"]["message"], "head moved") + + def test_request_still_pending_at_the_bound_is_left_pending(self) -> None: + bound = merge.ASYNC_MERGE_POLL_TIMEOUT_SECONDS + code = self._run( + _proc(_async("pending")), + [_async("pending"), _async("pending")], + clock=[0.0, 1.0, bound + 1], + ) + self.assertEqual(code, 10) + self.assertFalse(self.output["merged"]) + self.assertEqual(self.output["merge"]["status"], "pending") + self.assertEqual(len(self.polls), 2) + + def test_reported_merge_the_pull_request_contradicts_is_not_counted(self) -> None: + code = self._run( + _proc({"status": "merged", "details": {"message": "m", "sha": "d" * 40}}), + merged=False, + ) + self.assertEqual(code, 10) + self.assertFalse(self.output["merged"]) + self.assertFalse(self.output["merge"]["verifiedMerged"]) + + def test_a_merge_that_cannot_be_read_back_is_unconfirmed(self) -> None: + code = self._run( + _proc({"status": "merged", "details": {"message": "m", "sha": "d" * 40}}), + merged=None, + ) + self.assertEqual(code, 10) + self.assertFalse(self.output["merged"]) + self.assertTrue(self.output["mergeUnconfirmed"]) + self.assertIsNone(self.output["merge"]["verifiedMerged"]) + + def test_unready_pull_request_answer_does_not_fall_back(self) -> None: + code = self._run( + _proc( + {"message": "Pull request is a draft"}, + returncode=1, + stderr="gh: (HTTP 400)", + ) + ) + self.assertEqual(code, 10) + self.assertEqual(self.output["merge"]["httpStatus"], 400) + self.assertFalse(any(c[:2] == ["pr", "merge"] for c in self.captures)) + + def test_missing_endpoint_falls_back_to_gh_pr_merge_with_the_pin(self) -> None: + code = self._run( + _proc( + {"message": "Not Found"}, + returncode=1, + stderr="gh: Not Found (HTTP 404)", + ) + ) + self.assertEqual(code, 0) + self.assertTrue(self.output["merged"]) + self.assertIn("asyncFallback", self.output) + [legacy] = [c for c in self.captures if c[:2] == ["pr", "merge"]] + self.assertEqual(legacy[legacy.index("--match-head-commit") + 1], HEAD) + + def test_non_default_base_keeps_gh_pr_merge(self) -> None: + code = self._run(_proc(), base="release") + self.assertEqual(code, 0) + self.assertFalse(any("merge-async" in " ".join(c) for c in self.captures)) + [legacy] = self.captures + self.assertEqual(legacy[:2], ["pr", "merge"]) + + def test_unreadable_default_branch_keeps_gh_pr_merge(self) -> None: + self._run(_proc(), default_branch=None) + self.assertEqual(self.captures[0][:2], ["pr", "merge"]) + + +class MergeQueueIsEnqueued(AsyncMergeHarness): + def test_queue_branch_enqueues_without_a_merge_method(self) -> None: + code = self._run( + _proc(_async("pending")), [_async("enqueued")], merge_action="merge_queue" + ) + self.assertEqual(code, 0) + self.assertEqual(self.output["action"], "enqueue") + self.assertFalse(self.output["merged"]) + self.assertTrue(self.output["enqueued"]) + self.assertEqual( + self._fields(self._put()), + {"merge_action": "merge_queue", "bypass_rules": "false", "sha": HEAD}, + ) + + def test_already_queued_answer_is_still_queued(self) -> None: + code = self._run(_proc(_async("enqueued")), merge_action="merge_queue") + self.assertEqual(code, 0) + self.assertTrue(self.output["enqueued"]) + self.assertEqual(self.polls, []) + + def test_queue_without_the_endpoint_is_held(self) -> None: + code = self._run( + _proc( + {"message": "Not Found"}, + returncode=1, + stderr="gh: Not Found (HTTP 404)", + ), + merge_action="merge_queue", + ) + self.assertEqual(code, 10) + self.assertFalse(any(c[:2] == ["pr", "merge"] for c in self.captures)) + + +def _listed(number: int, sha: str, *, merged: bool = False) -> dict[str, Any]: + return { + "number": number, + "state": "closed" if merged else "open", + "draft": False, + "merged_at": "2026-10-02T00:00:00Z" if merged else None, + "head": {"ref": f"layer-{number}", "sha": sha}, + } + + +# The harness PR is #1; #2 is the open layer below it, evaluated at LAYER2. +AS_EVALUATED = [_listed(2, LAYER2), _listed(1, HEAD)] +LANDED = [_listed(2, LAYER2, merged=True), _listed(1, HEAD, merged=True)] + + +class StackLandsThroughTheAsyncApi(AsyncMergeHarness): + def _stack(self, put: mock.Mock, listings: list[list[dict[str, Any]]]) -> int: + return self._run( + put, + base="feat/b", + stack_lands=True, + stack_listings=listings, + extra=("--stacked-prs",), + ) + + def test_stack_merge_never_falls_back_to_gh_pr_merge(self) -> None: + missing = _proc( + {"message": "Not Found"}, returncode=1, stderr="gh: Not Found (HTTP 404)" + ) + code = self._stack(missing, [AS_EVALUATED]) + self.assertEqual(code, 10) + self.assertFalse(any(c[:2] == ["pr", "merge"] for c in self.captures)) + + def test_stack_merge_is_a_direct_async_merge_verified_layer_by_layer(self) -> None: + code = self._stack(_proc(_async("merged")), [AS_EVALUATED, LANDED]) + self.assertEqual(code, 0) + self.assertEqual(self._fields(self._put())["merge_action"], "direct_merge") + self.assertEqual(self.output["stackVerification"]["verified"], True) + + def test_a_lower_layer_pushed_after_evaluation_is_never_requested(self) -> None: + moved = [_listed(2, "f" * 40), _listed(1, HEAD)] + code = self._stack(_proc(_async("merged")), [moved]) + self.assertEqual(code, 10) + self.assertFalse(self.output["merge"]["attempted"]) + self.assertFalse(any("merge-async" in " ".join(c) for c in self.captures)) + + def test_a_layer_added_below_after_evaluation_is_never_requested(self) -> None: + grown = [_listed(3, "9" * 40), _listed(2, LAYER2), _listed(1, HEAD)] + code = self._stack(_proc(_async("merged")), [grown]) + self.assertEqual(code, 10) + self.assertFalse(any("merge-async" in " ".join(c) for c in self.captures)) + + def test_a_layer_landing_at_another_head_is_reported(self) -> None: + other = [_listed(2, "e" * 40, merged=True), _listed(1, HEAD, merged=True)] + code = self._stack(_proc(_async("merged")), [AS_EVALUATED, other]) + self.assertEqual(code, 10) + self.assertTrue(self.output["merged"]) + [mismatch] = self.output["stackVerification"]["mismatches"] + self.assertEqual( + (mismatch["pr"], mismatch["evaluatedHead"], mismatch["reportedHead"]), + ("owner/repo#2", LAYER2, "e" * 40), + ) + + +class PendingRequestOutlivesTheRun(AsyncMergeHarness): + """GitHub documents no route to cancel an async merge request, so a request + left pending is recorded and every later run reports it.""" + + def setUp(self) -> None: + super().setUp() + self.state = tempfile.TemporaryDirectory() + self.addCleanup(self.state.cleanup) + + def _leave_pending(self) -> None: + bound = merge.ASYNC_MERGE_POLL_TIMEOUT_SECONDS + code = self._run( + _proc(_async("pending")), + [_async("pending")], + clock=[0.0, bound + 1], + extra=("--state-dir", self.state.name), + ) + self.assertEqual((code, self.output["merge"]["status"]), (10, "pending")) + + def _records(self) -> dict[str, Any]: + path = pathlib.Path(self.state.name) / merge.PENDING_MERGES_FILE + return json.loads(path.read_text(encoding="utf-8"))["requests"] + + def test_a_pending_request_is_recorded(self) -> None: + self._leave_pending() + record = self._records()["owner/repo#1"] + self.assertEqual((record["uuid"], record["head"]), (UUID, HEAD)) + + def test_a_later_held_run_reports_merge_pending_and_sends_nothing(self) -> None: + self._leave_pending() + self.captures.clear() + code = self._run( + _proc(), + [_async("pending")], + ready=False, + merge_flag=False, + extra=("--state-dir", self.state.name), + ) + self.assertEqual(code, 10) + self.assertEqual(self.output["action"], "merge-pending") + self.assertIn(UUID, self.output["blockers"][0]) + self.assertEqual(self.captures, []) + + def test_a_later_ready_run_does_not_request_again(self) -> None: + self._leave_pending() + self.captures.clear() + code = self._run( + _proc(_async("merged")), + [_async("pending")], + extra=("--state-dir", self.state.name), + ) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertEqual(self.captures, []) + + def test_an_unreadable_request_still_counts_as_pending(self) -> None: + self._leave_pending() + self._run( + _proc(), + [RuntimeError("gh: Server Error (HTTP 502)")], + merge_flag=False, + extra=("--state-dir", self.state.name), + ) + self.assertEqual(self.output["action"], "merge-pending") + self.assertIn("owner/repo#1", self._records()) + + def test_a_finished_request_clears_the_record(self) -> None: + self._leave_pending() + code = self._run( + _proc(), + [_async("merged")], + ready=False, + merge_flag=False, + extra=("--state-dir", self.state.name), + ) + self.assertEqual(code, 10) + self.assertEqual(self.output["pendingMergeRequest"]["status"], "merged") + self.assertEqual(self._records(), {}) + + def test_an_expired_request_clears_the_record(self) -> None: + self._leave_pending() + self._run( + _proc(), + [RuntimeError("gh: Not Found (HTTP 404)")], + merge_flag=False, + extra=("--state-dir", self.state.name), + ) + self.assertEqual(self.output["pendingMergeRequest"]["status"], "expired") + self.assertEqual(self._records(), {}) + + +class GateEvaluation(unittest.TestCase): + """`evaluate` over a three-layer native stack: #1 (base main, head feat/a), + #2 (base feat/a, head feat/b), and #3 (base feat/b), the PR under test.""" + + def setUp(self) -> None: + RepoConfigFake().install(self) + self.views = { + 1: self._view(LAYER1, "main"), + 2: self._view(LAYER2, "feat/a"), + 3: self._view(HEAD, "feat/b"), + } + self.members = [ + self._member(1, "feat/a", LAYER1), + self._member(2, "feat/b", LAYER2), + self._member(3, "feat/c", HEAD), + ] + self.stack: Any = { + "base": {"ref": "main", "sha": "e" * 40}, + "number": 7, + "position": 3, + } + self.stack_error = False + self.rules: dict[str, list[dict[str, Any]]] = {"main": APPROVED_RULES} + self.calls: list[list[str]] = [] + + @staticmethod + def _view(head: str, base: str, **overrides: Any) -> dict[str, Any]: + view = { + "state": "OPEN", + "isDraft": False, + "mergeable": "MERGEABLE", + "mergeStateStatus": "CLEAN", + "reviewDecision": "APPROVED", + "headRefOid": head, + "baseRefName": base, + "author": {"login": SELF}, + "url": "u", + "title": "t", + "labels": [], + "body": "", + "statusCheckRollup": [], + } + view.update(overrides) + return view + + @staticmethod + def _member( + number: int, head_ref: str, sha: str, **overrides: Any + ) -> dict[str, Any]: + member = { + "number": number, + "state": "open", + "draft": False, + "merged_at": None, + "head": {"ref": head_ref, "sha": sha}, + } + member.update(overrides) + return member + + def _evaluate(self, *, stacked: bool = True, number: int = 3) -> dict[str, Any]: + def gh_json(args: list[str]) -> Any: + self.calls.append(args) + if args[:2] == ["pr", "view"]: + return self.views[int(args[2])] + path = args[1] + if path.startswith("repos/owner/repo/rules/branches/"): + return self.rules.get(path.split("/rules/branches/", 1)[1], []) + if path == "repos/owner/repo": + return {"name": "main"} + if path.startswith("repos/owner/repo/pulls/") and "{stack: .stack}" in args: + if self.stack_error: + raise RuntimeError("gh: Server Error (HTTP 502)") + return {"stack": self.stack} + if path == "repos/owner/repo/stacks/7": + return { + "number": 7, + "base": {"ref": "main"}, + "pull_requests": self.members, + } + raise AssertionError(f"unexpected gh_json call: {args}") + + with ( + mock.patch.object(merge, "gh_json", side_effect=gh_json), + mock.patch.object(merge, "fetch_review_threads", return_value=[]), + ): + return merge.evaluate( + "owner/repo", + number, + self.views[number]["headRefOid"], + {"owner"}, + frozenset({SELF}), + False, + False, + stacked=stacked, + ) + + def _stack_reads(self) -> list[list[str]]: + return [c for c in self.calls if "{stack: .stack}" in c or "stacks/" in c[1]] + + def test_flag_off_holds_a_stack_layer_and_reads_no_stack(self) -> None: + result = self._evaluate(stacked=False) + self.assertFalse(result["ready"]) + self.assertTrue( + any("'feat/b' is unprotected" in b for b in result["blockers"]), + result["blockers"], + ) + self.assertEqual(self._stack_reads(), []) + + def test_flag_on_judges_the_layer_against_the_trunk_and_gates_every_layer( + self, + ) -> None: + result = self._evaluate() + self.assertTrue(result["ready"], result["blockers"]) + self.assertEqual(result["landingBase"], "main") + self.assertTrue(result["stack"]["landsLowerLayers"]) + self.assertEqual( + [layer["pr"] for layer in result["stack"]["layers"]], + ["owner/repo#1", "owner/repo#2"], + ) + rule_reads = {c[1] for c in self.calls if "/rules/branches/" in c[1]} + self.assertEqual(rule_reads, {"repos/owner/repo/rules/branches/main"}) + + def test_a_lower_layer_missing_an_ai_review_holds_the_auto_merge(self) -> None: + reviewed = [ + { + "__typename": "CheckRun", + "name": name, + "status": "COMPLETED", + "conclusion": "SUCCESS", + } + for name in merge.AI_REVIEW_CHECKS + ] + for number in (2, 3): + self.views[number]["statusCheckRollup"] = reviewed + self.views[1]["statusCheckRollup"] = reviewed[1:] # claude-review-status absent + result = self._evaluate() + self.assertTrue(result["ready"], result["blockers"]) + self.assertFalse(result["autoMerge"]["ready"]) + self.assertEqual( + result["autoMerge"]["blockers"], + [ + "stack layer owner/repo#1: AI review check 'claude-review-status' " + "has not succeeded on the live head" + ], + ) + + def test_a_lower_layer_blocker_holds_the_stack(self) -> None: + self.views[1]["isDraft"] = True + result = self._evaluate() + self.assertIn( + "stack layer owner/repo#1: PR is a draft -- mark ready first", + result["blockers"], + ) + + def test_a_lower_layer_whose_head_moved_holds_the_stack(self) -> None: + self.views[2]["headRefOid"] = "f" * 40 + result = self._evaluate() + self.assertTrue( + any( + b.startswith("stack layer owner/repo#2: head moved") + for b in result["blockers"] + ), + result["blockers"], + ) + + def test_a_broken_chain_holds(self) -> None: + self.views[2]["baseRefName"] = "elsewhere" + result = self._evaluate() + self.assertTrue( + any("#2 targets 'elsewhere'" in b for b in result["blockers"]), + result["blockers"], + ) + + def test_merged_lower_layers_are_skipped(self) -> None: + self.members[0]["merged_at"] = "2026-10-01T00:00:00Z" + self.members[0]["state"] = "closed" + self.views[2]["baseRefName"] = "main" + result = self._evaluate() + self.assertTrue(result["ready"], result["blockers"]) + self.assertEqual( + [layer["pr"] for layer in result["stack"]["layers"]], ["owner/repo#2"] + ) + + def test_a_closed_unmerged_layer_holds(self) -> None: + self.members[1]["state"] = "closed" + result = self._evaluate() + self.assertIn( + "stack layer owner/repo#2 is closed without merging -- held", + result["blockers"], + ) + + def test_unreadable_membership_holds(self) -> None: + self.stack_error = True + result = self._evaluate() + self.assertTrue( + any( + b.startswith("stack membership could not be read") + for b in result["blockers"] + ) + ) + + def test_a_pr_outside_any_stack_keeps_the_non_default_hold(self) -> None: + self.stack = None + result = self._evaluate() + self.assertTrue( + any("'feat/b' is unprotected" in b for b in result["blockers"]), + result["blockers"], + ) + self.assertFalse(result["stack"]["landsLowerLayers"]) + + def test_a_queue_trunk_holds_the_stack(self) -> None: + self.rules["main"] = QUEUE_RULES + result = self._evaluate() + self.assertTrue( + any("requires a merge queue" in b for b in result["blockers"]), + result["blockers"], + ) + + def test_queue_on_the_default_branch_is_ready_to_enqueue(self) -> None: + self.rules["main"] = QUEUE_RULES + result = self._evaluate(stacked=False, number=1) + self.assertTrue(result["ready"], result["blockers"]) + self.assertEqual(result["mergeAction"], "merge_queue") + + def test_queue_on_a_non_default_base_is_held(self) -> None: + self.rules["feat/a"] = QUEUE_RULES + result = self._evaluate(stacked=False, number=2) + self.assertTrue( + any( + "not the default branch -- a direct merge is not allowed" in b + for b in result["blockers"] + ), + result["blockers"], + ) + + def test_auto_merge_is_not_armed_over_a_queue(self) -> None: + self.rules["main"] = QUEUE_RULES + self.views[1]["isDraft"] = True + result = self._evaluate(stacked=False, number=1) + self.assertIn(merge.MERGE_QUEUE_AUTO_HOLD, result["autoMerge"]["blockers"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/plugins/source-control/skills/pull-request/SKILL.md b/plugins/source-control/skills/pull-request/SKILL.md index 3b87897279..907a357bcd 100644 --- a/plugins/source-control/skills/pull-request/SKILL.md +++ b/plugins/source-control/skills/pull-request/SKILL.md @@ -76,7 +76,7 @@ in fleet orchestration and never merges. ## Action defaults -- **Merge mode:** `merge` squash-merges (one squashed commit per PR onto the default branch), see [reference/merge.md](reference/merge.md) §4.2. Follow the consuming project's convention when it differs. +- **Merge mode:** `merge` squash-merges (one squashed commit per PR onto the default branch), see [reference/merge.md](reference/merge.md) §4.2. Follow the consuming project's convention when it differs. A stacked PR lands every layer below it: see [reference/stacks.md](reference/stacks.md). - **Monitor cadence:** `monitor` polls the PR's checks (the §3.0.1 REST read) and comment fetches every 30 seconds, see [reference/monitor.md](reference/monitor.md) §3.1. A wait is a REST poll, never `gh pr checks --watch`, and it first reports each pending job as queued or running ("Waiting on a pending check"). - **Required reviewers:** `create` requests no reviewers (runs `gh pr create` without `--reviewer`). See [reference/create.md](reference/create.md) §2.4.3. diff --git a/plugins/source-control/skills/pull-request/reference/merge.md b/plugins/source-control/skills/pull-request/reference/merge.md index ad61c49210..88ea0df523 100644 --- a/plugins/source-control/skills/pull-request/reference/merge.md +++ b/plugins/source-control/skills/pull-request/reference/merge.md @@ -35,7 +35,12 @@ gh api --paginate "repos/{owner}/{repo}/issues//comments?per_page=100 `behind_by > 0`, update the branch (merge-forward / `gh pr update-branch`) and re-run readiness; do **not** squash-merge a behind head. Under a non-strict ruleset, GitHub can still report `CLEAN` while the head is behind, and a stale-base squash can silently revert - recently-landed fixes (the tests travel with the reverted code, so CI stays green). Where the + recently-landed fixes (the tests travel with the reverted code, so CI stays green). `CLEAN` also + says nothing about which base CI tested: GitHub regenerates the test merge commit only on a + push, a merge-base change, or once it is 12 hours old, so the compare above, against the live + base ref name, is the check + ([changelog](https://github.blog/changelog/2026-02-19-changes-to-test-merge-commit-generation-for-pull-requests), + as of 2026-10-02; recheck when GitHub changes test-merge regeneration). Where the consuming repo runs an overlapping-path CI gate, treat it as the tripwire; it covers the stale-**base** class only, and only a post-merge silent-revert detector catches a head that is current in history but stale in content. A consuming repo may have neither. @@ -66,6 +71,28 @@ gh pr merge --squash && { When the repo deletes head branches on merge, the push fails with "remote ref does not exist"; that is expected, and it is the only failure to ignore. 4.3 deletes the local branch. Verified 2026-09-29 against [cli/cli#14007](https://github.com/cli/cli/pull/14007), which ships in gh 2.99.0 and makes `gh pr merge --delete-branch` skip the local delete when the head is checked out in the current linked worktree; earlier gh, such as 2.98.0, fails as described. Recheck when the minimum gh this plugin supports is 2.99.0 or later, at which point the split is no longer needed. +**Through the async merge API.** Use it instead of `gh pr merge` when the session refuses GraphQL +(`gh pr merge` and `gh pr view` run over GraphQL; this is REST), when the base requires a merge +queue, or when the PR is a stack layer ([stacks.md](stacks.md)). `gh` has no command for it yet, so +call it with `gh api`, pinned to the head you verified in 4.1 and never with `bypass_rules` true: + +```bash +HEAD_SHA=$(gh api "repos/{owner}/{repo}/pulls/" --jq .head.sha) +gh api -X PUT "repos/{owner}/{repo}/pulls//merge-async" \ + -f sha="$HEAD_SHA" -f merge_action=direct_merge -f merge_method=squash -F bypass_rules=false +``` + +For a merge queue send `-f merge_action=merge_queue` and drop `merge_method`. The response carries a +request `uuid` (a 409 means one is already pending and returns its `uuid`); poll +`gh api "repos/{owner}/{repo}/pulls//merge-async/" --jq .status` until it reads +`merged`, `enqueued`, or `failed`. `enqueued` means queued, not merged: delete the head branch only +once the PR reads `MERGED`. GitHub checks only basic PR state when it accepts the request and +applies the base's rules when the merge runs, so 4.1 still comes first. Request fields and +statuses: +[merge a pull request asynchronously](https://docs.github.com/rest/pulls/pulls?apiVersion=2026-03-10#merge-a-pull-request-asynchronously), +as of 2026-10-02; recheck when a `gh` release adds a command for it or that page changes a field or +status. + **Always use the explicit `` resolved at phase entry.** The PR title becomes the squash commit message. It is shaped to satisfy the resolved subject/title convention, per pull-request SKILL.md's "PR title format" ladder (Conventional Commits by default). ## 4.3 Worktree transition and next-task setup diff --git a/plugins/source-control/skills/pull-request/reference/stacks.md b/plugins/source-control/skills/pull-request/reference/stacks.md new file mode 100644 index 0000000000..4be07571a3 --- /dev/null +++ b/plugins/source-control/skills/pull-request/reference/stacks.md @@ -0,0 +1,43 @@ +# Stacked pull requests + +A stack is a chain of pull requests in one repository: the bottom layer targets the trunk (usually +the default branch) and each layer above targets the branch of the layer below. Only a native stack, +one GitHub records as a stack, behaves as one. A PR that merely targets another PR's branch is not a +stack layer: merging it lands that PR alone, into that branch. + +## Create + +- Build the stack with GitHub's `gh stack` extension (`github/gh-stack`). Its quickstart and CLI + reference, linked below, carry the commands; read them there rather than from memory, since the + feature is in public preview. +- Put each dependency in the same layer or a lower one, never a higher one. +- Each layer is a pull request in its own right: open it as a draft, take it through `ready` and + `monitor`, and hold its title and body to the same contract as any other PR. + +## Merge + +- Merging a layer lands it and every open layer below it, all or nothing. Run the Phase 4.1 + readiness re-verification ([merge.md](merge.md)) on **every** layer that will land, not only the + one you merge, and get the user's approval for the whole set. +- Merge with the extension's merge command or the async merge API ([merge.md](merge.md) §4.2). The + synchronous REST merge endpoint does not support stacks. +- The trunk's rules govern every layer, and GitHub evaluates them when the merge runs, not when it + is requested. A stack merge cannot bypass them. +- Merging a lower layer leaves the layers above open; GitHub retargets them to the trunk and rebases + them. +- On a trunk that requires a merge queue the stack is enqueued, and its layers can land in separate + groups. Treat it as queued until each PR reads `MERGED`. +- `/source-control:babysit-prs` holds stack layers for a human unless its `babysit_stacked_prs` + option is on. + +**Claim, basis, as of, recheck:** stack shape, trunk rules, and merge behavior, +[about stacked pull requests](https://docs.github.com/en/pull-requests/get-started/about-stacked-prs); +the extension, its merge-time rule evaluation, and its queue behavior, +[stacked pull requests CLI commands](https://docs.github.com/en/pull-requests/reference/stacked-prs-cli-commands) +and [quickstart](https://docs.github.com/en/pull-requests/get-started/stacked-prs-quickstart); the +synchronous endpoint's limit, +[merge a pull request](https://docs.github.com/rest/pulls/pulls?apiVersion=2026-03-10#merge-a-pull-request); +public preview, +[stacked pull requests changelog](https://github.blog/changelog/2026-07-30-stacked-pull-requests-are-now-in-public-preview); +2026-10-02. Recheck when stacked pull requests leave public preview, or when either docs page +changes how a stack merges or queues. diff --git a/plugins/source-control/skills/setup/SKILL.md b/plugins/source-control/skills/setup/SKILL.md index 5d2f7ea592..8d257364d0 100644 --- a/plugins/source-control/skills/setup/SKILL.md +++ b/plugins/source-control/skills/setup/SKILL.md @@ -158,7 +158,8 @@ the step UNKNOWN with remediation, never green. surviving literal `${user_config.…}` placeholder there means the key is unset. For each unset key state what will be inferred at run time. `babysit_watched_owners` → the current repo's owner, `babysit_self_logins` → none (your `gh api user --jq .login` login is always used, extras only add - to it), `babysit_default_tier` → `safe`, `babysit_merge_method` → `auto` (repo convention, then squash), the + to it), `babysit_default_tier` → `safe`, `babysit_merge_method` → `auto` (repo convention, then squash), + `babysit_stacked_prs` → `false` (a stack layer is held for a human), the review-trigger keys → module dormant, `babysit_worktree_root` → the plugin data dir's `worktrees/` subdirectory. Unset keys are INFO (documented defaults), not FAIL. 2. **Branch-protection posture across watched repos.** For each watched owner (or the current repo's @@ -170,8 +171,8 @@ the step UNKNOWN with remediation, never green. protection). Flag every repo reporting zero required reviews AND zero required status contexts as **unprotected**: the merge gate refuses gate-proven merges there for non-self authors, and for a self author whenever the base is not the default branch (`--allow-unprotected` is the deliberate - override), so an unprotected repo in an autopilot fleet deserves a protection rule, not an - override. + override; `babysit_stacked_prs` judges a native stack layer against its trunk instead), so an + unprotected repo in an autopilot fleet deserves a protection rule, not an override. 3. **Windows long-path support for the worktree root.** On Windows, worktrees under the (possibly deep) worktree root can exceed 260 characters. Probe `git config --get core.longpaths` and the OS policy (registry value `LongPathsEnabled` under From 86aaa75df6e1a3164f4b36437a1faa447842bd21 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:09:11 -0400 Subject: [PATCH 03/10] feat(ci): run ci-status on merge_group and flag behind-base PRs in morning-brief - ci.yml takes merge_group (checks_requested) so the required ci-status check reports inside a merge queue. On that event every lane runs, pr-contract reports skipped, and concurrency falls back to run_id. test-windows.yml documents why it takes no merge_group event. - morning-brief reads the compare endpoint for each CLEAN PR and marks it UNVERIFIED when the head is behind its base or the comparison fails. - autonomy's deferred merge serialization now points at the async merge API's merge_queue action as the native binding. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 23 +++++-- .github/workflows/test-windows.yml | 4 +- plugins/autonomy/.claude-plugin/plugin.json | 2 +- plugins/autonomy/CHANGELOG.md | 9 +++ .../autonomy/reference/runner/lifecycle.md | 12 ++++ .../harness-ops/.claude-plugin/plugin.json | 2 +- plugins/harness-ops/CHANGELOG.md | 13 ++++ plugins/harness-ops/README.md | 2 +- .../harness-ops/skills/morning-brief/SKILL.md | 14 +++- .../morning-brief/morning-brief.test.sh | 68 ++++++++++++++++--- .../morning-brief/scripts/morning-brief.sh | 65 +++++++++++++++--- 11 files changed, 187 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c47668380b..123c6f6273 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,21 @@ on: # `pull_request` whatever `permissions:` requests, so it cannot record lane # state — and runs the full workflow on every event exactly as before. types: [opened, synchronize, reopened, ready_for_review, edited, labeled, unlabeled] + # A merge queue tests each queued merge commit through this event alone; a + # required check that never reports on it leaves the queued pull request + # unmergeable. A merge-group run carries no `github.event.pull_request`, so + # every expression below that reads one takes its non-PR branch, the same one + # a push to main takes: the contract-only predicate is false, so every lane + # runs in full; the concurrency group falls back to `github.run_id`, so no + # queued run cancels another; the docs-only detector does not run and the + # change-detection action fails open, so the whole suite runs; the + # base-diff gates decline as they do on a push; `pr-contract` finds no pull + # request number and reports `skipped`, because the title, label and linkage + # were already checked on the pull request before it could enter the queue; + # and `ci-status` aggregates the lanes and records `ci-lanes` on the + # merge-group commit (`github.sha`), a SHA no contract-only run ever reads. + merge_group: + types: [checks_requested] permissions: contents: read @@ -52,10 +67,10 @@ permissions: # until someone re-ran that specific run (melodic-software/github-iac#378). # # The full branch keeps one group per pull request, so a `synchronize` still -# cancels everything on the old SHA. Push runs fall back to the unique -# `github.run_id` and are never cancelled; the PR number scopes cancellation to -# exactly one pull request, where `head_ref` would collide across same-named -# fork branches. +# cancels everything on the old SHA. Push and merge-group runs fall back to the +# unique `github.run_id` and are never cancelled; the PR number scopes +# cancellation to exactly one pull request, where `head_ref` would collide +# across same-named fork branches. concurrency: group: >- ${{ (github.event.pull_request.head.repo.full_name == github.repository && diff --git a/.github/workflows/test-windows.yml b/.github/workflows/test-windows.yml index d30465f291..678d215a9e 100644 --- a/.github/workflows/test-windows.yml +++ b/.github/workflows/test-windows.yml @@ -17,7 +17,9 @@ name: test-windows # report success having run nothing, which both of ci.yml's gates reject. # # The triggers, the contract-only predicate, the concurrency shape and the -# permissions are ci.yml's, unchanged. The `changes` job below is the minimum +# permissions are ci.yml's, unchanged, except that this workflow takes no +# `merge_group` event: a merge queue waits only on required checks, and this +# lane is not one. The `changes` job below is the minimum # re-resolution the split forces: job outputs do not cross workflow files, so # `run_windows` has to be computed here. It is derived by the SAME expression # ci.yml publishes, from the same detector and the same shell, python, and diff --git a/plugins/autonomy/.claude-plugin/plugin.json b/plugins/autonomy/.claude-plugin/plugin.json index 95489ba119..cd54fc1a0a 100644 --- a/plugins/autonomy/.claude-plugin/plugin.json +++ b/plugins/autonomy/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "autonomy", - "version": "0.25.6", + "version": "0.25.7", "description": "Governed autonomous agent operation: role-topology, binding-seam, wiring-vs-advisor, telemetry, return-accounting, trigger-dispatch, per-work-class guardrail-matrix, standing-routine-catalog, and design-only runner-charter contracts for climbing the AI-adoption ladder, plus a guided-setup skill that discovers an adopting org's state, writes its schema-versioned binding, wires standards-pinned OTLP emission with a zero-cost file-artifact default, wires human-attested return capture at the task boundary, wires signal adapters with one governed dispatch entrypoint, binds the five-class guardrail matrix to an org's isolation substrates with an in-boundary live-validation probe before recording each fail-closed binding, and stands up standing-routine-catalog classes as scheduled temporal signal adapters behind the one governed queue with free scheduling defaults wired as reviewable changes and each routine's work-class mapping homed on the security surface.", "author": { "name": "Melodic Software", diff --git a/plugins/autonomy/CHANGELOG.md b/plugins/autonomy/CHANGELOG.md index f4eede04b2..75e77612d5 100644 --- a/plugins/autonomy/CHANGELOG.md +++ b/plugins/autonomy/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to the `autonomy` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.25.7] - 2026-10-02 + +### Changed + +- The runner lifecycle's deferred merge-serialization growth stage names GitHub's native binding: + the merge queue, entered through the asynchronous merge endpoint with `merge_action=merge_queue`. It + is a dated pointer record with a recheck trigger; the stage stays deferred until its evidence + trigger fires. + ## [0.25.6] - 2026-10-02 ### Changed diff --git a/plugins/autonomy/reference/runner/lifecycle.md b/plugins/autonomy/reference/runner/lifecycle.md index e790fdb556..bcaef895a8 100644 --- a/plugins/autonomy/reference/runner/lifecycle.md +++ b/plugins/autonomy/reference/runner/lifecycle.md @@ -79,3 +79,15 @@ native flow. When that evidence arrives, the runner serializes gated merges by b platform-native merge-queue facility where one exists, never a reimplemented queue. That facility's availability is verified at binding time; absent one, the growth stage stays deferred rather than reimplementing a built-in. No serialization ships at launch. + +**GitHub's native binding, dated record.** *Claim:* on GitHub, the facility to bind is the +repository merge queue, entered through the asynchronous merge endpoint with +`merge_action=merge_queue`, so the runner submits each gated merge there and lets the queue +serialize it. +*Basis:* the "Merge a pull request asynchronously" section of the REST pull-request reference, +, +and the general-availability announcement, +. +*As of:* 2026-10-02. *Recheck trigger:* that section drops or renames the `merge_queue` value, +the REST API version it is documented under is retired, or a changelog entry changes +the endpoint's status. diff --git a/plugins/harness-ops/.claude-plugin/plugin.json b/plugins/harness-ops/.claude-plugin/plugin.json index e06b5d4555..0d5dd10c2c 100644 --- a/plugins/harness-ops/.claude-plugin/plugin.json +++ b/plugins/harness-ops/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "harness-ops", - "version": "2.4.3", + "version": "2.4.4", "description": "Claude Code operations toolkit. Fifteen skills: audit-skill-visibility (audit whether each installed skill is actually VISIBLE to the model, and diagnose why most of a fleet never gets used: a skill is invisible when its description is dropped by Claude Code's skill-listing context budget, which sheds descriptions lowest-score-first so an unused skill loses the keywords that would let it be matched, from skills genuinely not wanted, from skills the run cannot observe at all; computes whether the listing overflows from documented settings, and withholds every cold verdict the data cannot support rather than reporting absence of data as absence of use), inventory (read-only enumeration of the complete invocable surface: every built-in CLI command with aliases and hidden/gated status, every bundled skill, every built-in subagent and tool, every built-in plugin with its components, and every component of every installed plugin across all marketplaces; reads the shipped binary because upstream publishes no built-in command list, and carries an integrity verdict so a drifted build reports counts as floors rather than silently short totals), audit-install-state (read-only audit of the machine-scope ~/.claude installation directory and ~/.claude.json: full inventory split into an authored surface and rolled-up bulk trees, product-managed retention vs genuinely unmanaged state, filename-scheme resolution before any process-liveness check, and deliberate/mid-experiment detection; reports, never deletes), audit-performance (read-only slowness-diagnostic capture run at the moment the machine or a session feels slow: CLI version, retention-sweep health including the unparsable-settings pause, which warns in /status, a timed census walk of the install tree as a sweep-cost proxy, active-session and plugin-fleet counts, a process census, and the fan-out layer, which covers a load-labeled no-op spawn baseline, every hook that will fire bucketed per-tool-call versus per-turn with its invocation shape, the configured statusline, subagent concurrency and spawn-depth ceilings against documented defaults, whether running sessions predate the settings file they are judged by, and orphan attribution by parent liveness rather than age, plus on Windows a kernel-object census (Token objects against uptime, paged pool) that names a host-level leak beneath all four suspects; read against a bundled known-performance-issues reference that also records the causes tested and cleared; separates the four documented suspects of accumulated state, version regression, component bloat, and per-spawn fan-out cost, and routes remediation out; reports, never mutates, and never executes a discovered hook or statusline command), audit-native-overlap (map native Claude Code surfaces, namely built-in CLI commands, bundled skills, plugin-backed built-ins, and session-provided skills, against the current repo's plugin skills and agents, so a custom component never silently duplicates what Claude Code itself ships; bare invocation is a read-only overlap report carrying the extraction's integrity floors and a shared-listing-budget exposure section, verdicts are human-gated in a committed store rendered into a generated registry whose every row carries an observable recheck trigger, and only an explicit apply step bakes presence-gated native references into descriptions and Boundary sections), observability (read locally captured telemetry from the OTEL store, the collector, the per-session hook event log and hook-event JSONL, and ccusage, with trend reports, a per-session report of what fired, what was blocked and the event timeline, and store pruning), known-issues (search known Claude product GitHub bugs, check service health, maintain a persistent tracked-issue registry), changelog (ingest Claude Code changelog entries and turn them into decisions: apply executes those in scope one PR per owner plugin and hands larger ones off as work items, then re-extract the native surface and file its drift as work items), prerequisites (read-only table of external binaries declared by enabled plugins; never installs), check (read-only check that node and jq resolve for the harness-ops hooks; never installs), machine-profile (discover this machine's facts and per-tree identity domains, store them as a re-runnable profile with the observation behind every value, and diff the stored profile against the host now; read-only unless the operator confirms a write, never installs and never reapplies a stored value on its own), plugins (bring a machine's plugin fleet current on demand: marketplace refresh, effective-scope updates including in-repo project/local installs, new-plugin install per policy, scope-divergence detection and explicit convergence), morning-brief (read-only gh-based operator morning view: queue-label counts, merge-ready PRs, parked decisions with their RECOMMENDED lines, and loop-lane telemetry freshness), lanes (start/restart/stop/status loop lanes as named background Claude Code sessions seeded from canonical prompt files, with per-lane model/effort, a repo-pull + marketplace-refresh launch step, and a consume-restarts action, an OS-schedulable reader that relaunches stopped lanes whose telemetry carries a restart_request), and a re-runnable setup action that settles where the known-issues registry, the skill-usage log and the hook log root live, places the root's self-ignoring guard, and detects retired conventions. Plus an opt-in, default-off per-session hook event log (one JSON line per hook event on every event the generated registry marks observable, written to /sessions/.jsonl, with SessionEnd retention by session count or age and an optional detached pre-prune command), a family of eight advisory *-audit hooks (API errors, config changes, instruction loads, permission denials, pre-compaction, skill usage, tool failures, and unsurfaced hook failures. The last also warns the user via systemMessage, since a hook that fails to launch enforces nothing and Claude Code surfaces the failure to nobody) that emit the shared hook-telemetry envelope, and a reference sink that routes envelopes under the same root: per session when the envelope carries a session id, else into the shared hook-events.jsonl the observability skill reads.", "author": { "name": "Melodic Software", diff --git a/plugins/harness-ops/CHANGELOG.md b/plugins/harness-ops/CHANGELOG.md index e7fcb5113d..c86e955e16 100644 --- a/plugins/harness-ops/CHANGELOG.md +++ b/plugins/harness-ops/CHANGELOG.md @@ -3,6 +3,19 @@ All notable changes to the `harness-ops` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [2.4.4] - 2026-10-02 + +### Fixed + +- `morning-brief` no longer presents every `CLEAN` pull request as merge-ready without + qualification. `CLEAN` can describe checks that ran against an older base, so the script reads + the compare endpoint once per clean PR and prints an `UNVERIFIED` line under any PR whose head + is behind its base, or whose comparison could not be read. A `--behind-json` fixture flag + feeds those counts to the tests. +- `machine-profile` refuses a record value holding a GitHub App installation token in the + `ghs__` format GitHub began issuing on 2026-04-27, matched by its own shape rather + than only through the generic JWT rule. + ## [2.4.3] - 2026-10-02 ### Changed diff --git a/plugins/harness-ops/README.md b/plugins/harness-ops/README.md index 7dc20934ba..82e73448aa 100644 --- a/plugins/harness-ops/README.md +++ b/plugins/harness-ops/README.md @@ -41,7 +41,7 @@ Claude Code's native OTEL cannot see. | `/harness-ops:known-issues` | Searches known Claude product GitHub bugs before you build on a feature, checks service health and model quality, and maintains a persistent registry of tracked issues (what they block, workarounds, follow-ups when fixed). Actions: `status` (default), `search`, `check-all`, `scan`, `list`, `quality`, `create`. | | `/harness-ops:changelog` | Ingests Claude Code changelog entries and integrates them into the current repo: `fetch` (read-only display), `diff` (decision rows by owner surface and action lens over a release range, no edits), `status` (the read marker from the repo's Claude Code ledger, the default range to the newest release, and the replay cap), and `apply` (executes the decisions in scope one PR per owner plugin, hands larger ones off as work items, then runs a native-surface drift pass that files work items for new overlap candidates, fired store triggers, and a degraded or broken inventory; explicit user intent only). | | `/harness-ops:plugins` | Brings a machine's plugin fleet current on demand: marketplace refresh, updates for the plugins that actually load (including in-repo project/local-scope installs), new-catalog-plugin install per policy, and scope-divergence detection. Actions: `sync` (default, CLI-mediated mutations only), `audit` (read-only dry run), `converge` (the one action that can touch a committed `.claude/settings.json`. Previews and confirms per plugin first). | -| `/harness-ops:morning-brief` | Prints the read-only, `gh`-based operator morning view for the current repo in one pass: open counts per queue label (`needs-triage`, `status: ready`, `status: needs-decision`, `needs-human`), the gh-native merge-ready PR list (non-draft + `mergeStateStatus=CLEAN`), parked `status: needs-decision` issues with their RECOMMENDED lines, and loop-lane telemetry freshness (per-lane `last-cycle` age + `flags:`). Never mutates anything; the authoritative PR merge gate stays `/source-control:babysit-prs`. | +| `/harness-ops:morning-brief` | Prints the read-only, `gh`-based operator morning view for the current repo in one pass: open counts per queue label (`needs-triage`, `status: ready`, `status: needs-decision`, `needs-human`), the gh-native merge-ready PR list (non-draft + `mergeStateStatus=CLEAN`, with a clean PR whose head is behind its base marked `UNVERIFIED`), parked `status: needs-decision` issues with their RECOMMENDED lines, and loop-lane telemetry freshness (per-lane `last-cycle` age + `flags:`). Never mutates anything; the authoritative PR merge gate stays `/source-control:babysit-prs`. | | `/harness-ops:lanes` | Starts, restarts, stops, and reports loop lanes as named background Claude Code sessions seeded from canonical prompt files. `start` (default) / `restart` pull the repo and refresh the plugin marketplace, then launch each configured lane (`claude --bg -n --permission-mode auto`, plus `--permission-prompts none` on CLI 2.1.259 or later) with its per-lane `model`/`effort`; `status` shows per-lane running state and live sessionId; `stop` ends a lane via `claude stop`; `consume-restarts` is the OS-schedulable restart-request consumer. It reads each configured lane's telemetry `restart_request` and relaunches the stopped lanes that asked, through the same launcher (#1653). Acts only on sessions whose name is a configured lane. Lanes come from a JSON config (`--config`, else `$HARNESS_OPS_LANES_CONFIG`, else `/.work/lanes/lanes.json`, with a temporary default-only fallback to the pre-move `/.work/lanes.json` under a deprecation warning); config and prompts live in the reserved `lanes/` concern home under a hardcoded `.work` root, which is a sanctioned placement but still session-local, so a durable cross-machine home stays #480's job. | | `/harness-ops:check` | Read-only check that `node` and `jq` resolve for the hooks, with the install route for a missing tool. Model-invocable; never installs. | | `/harness-ops:machine-profile` | Discovers this machine's facts and identity domains (each tree's git include, `gh` directory and verdicts), stores them as a re-runnable profile that records the observation behind every value, and reports drift between the stored profile and the host now. Actions: `profile` (default), `diff`, `explain `, `apply --option `. Read-only unless the operator confirms a write: `record --confirm` writes the profile document, and `apply --confirm` prints what to hand to each setup and writes nothing. Never installs and never reapplies a stored value on its own. Design: [machine-profile-design](https://github.com/melodic-software/claude-code-plugins/blob/038c2ae22c23f60500b339fd2f66e4569ecbe2fd/docs/specs/machine-profile-design.md). | diff --git a/plugins/harness-ops/skills/morning-brief/SKILL.md b/plugins/harness-ops/skills/morning-brief/SKILL.md index bfb3efd624..842b09652b 100644 --- a/plugins/harness-ops/skills/morning-brief/SKILL.md +++ b/plugins/harness-ops/skills/morning-brief/SKILL.md @@ -70,7 +70,17 @@ saying so when capped or when GitHub has not finished computing a PR's mergeabil and the stranded-findings section renders `UNREADABLE`, because review threads have no REST read. That section never renders an all-clear it did not read. -Two upstream facts the script restates, each with its verification record: +Three upstream facts the script restates, each with its verification record: + +- **`CLEAN` can describe checks that ran against an older base.** The script therefore compares + each clean PR's head with its base branch and prints an `UNVERIFIED` line when the head is + behind, or when the comparison could not be read; a head that contains the base tip prints + nothing extra. Basis: GitHub's 2026-02-19 changelog + , + which lists when a test merge commit is regenerated, and the `behind_by` field of + . As of 2026-10-02. + Recheck when GitHub changes when it regenerates test merge commits, or `mergeStateStatus` + gains a value for a merge state tested against an older base. - **The refusal shape the script keys the transport switch on.** Basis: the body `gh api graphql` returned in a Claude Code cloud session, `{"message":"This GraphQL @@ -96,7 +106,7 @@ Two upstream facts the script restates, each with its verification record: | Section | Source | Notes | |---|---|---| | Queues | `gh issue list --label ` counts | Defaults to melodic-software queue labels; live runs filter to labels that exist in the repo (pass `--queue-labels` to pin a custom set) | -| Merge-ready PRs | `gh pr list` filtered to non-draft + `mergeStateStatus=CLEAN` (REST: `pulls` list plus one read per PR for `mergeable_state`) | A light glance signal; `reviewDecision` shown but not required (repos without required review leave it empty; the REST path reports `n/a`) | +| Merge-ready PRs | `gh pr list` filtered to non-draft + `mergeStateStatus=CLEAN` (REST: `pulls` list plus one read per PR for `mergeable_state`) | A light glance signal; `reviewDecision` shown but not required (repos without required review leave it empty; the REST path reports `n/a`). One compare read per clean PR; `UNVERIFIED` marks a head behind its base or a comparison that could not be read | | Parked decisions | open issues with the decision label (default `status: needs-decision`) | Surfaces each one's RECOMMENDED line, the uppercase marker wins over an incidental lowercase mention; a case-insensitive fallback catches lowercase markers; pass `--decision-label` to pin | | Lane telemetry | the loop-lane telemetry issue's per-lane comments | Each lane's `last-cycle` age (marked `STALE` past `--stale-hours`, default 6) and any `flags:` | | Stranded findings | merged PRs whose unresolved review threads were **created after the merge** | One line per PR at its worst severity, with a finding count; window is `--stranded-days`, default 3 | diff --git a/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh b/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh index e72ce1ff76..5973e6141f 100755 --- a/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh +++ b/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh @@ -48,6 +48,10 @@ fail() { assert_contains() { if [[ "$2" == *"$3"* ]]; then pass "$1"; else fail "$1" "contains: $3" "$2"; fi; } assert_not_contains() { if [[ "$2" != *"$3"* ]]; then pass "$1"; else fail "$1" "absent: $3" "$2"; fi; } assert_exit() { if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "exit $2" "exit $3"; fi; } +section() { + # section OUTPUT "== heading prefix" — the lines of one section, heading included. + printf '%s\n' "$1" | awk -v h="$2" 'index($0, h) == 1 {p = 1; print; next} /^== / {p = 0} p' +} # --- Fixtures ----------------------------------------------------------------- NOW="2026-07-20T08:00Z" @@ -63,11 +67,18 @@ cat >"$TMP/pr.json" <<'EOF' [ {"number": 12, "title": "blocked pr", "url": "http://x/12", "isDraft": false, "mergeStateStatus": "BLOCKED", "reviewDecision": ""}, {"number": 11, "title": "draft pr", "url": "http://x/11", "isDraft": true, "mergeStateStatus": "CLEAN", "reviewDecision": ""}, - {"number": 13, "title": "approved clean pr", "url": "http://x/13", "isDraft": false, "mergeStateStatus": "CLEAN", "reviewDecision": "APPROVED"}, - {"number": 10, "title": "clean pr", "url": "http://x/10", "isDraft": false, "mergeStateStatus": "CLEAN", "reviewDecision": ""} + {"number": 13, "title": "approved clean pr", "url": "http://x/13", "isDraft": false, "mergeStateStatus": "CLEAN", "reviewDecision": "APPROVED", + "baseRefName": "main", "headRefOid": "1313131313131313131313131313131313131313"}, + {"number": 10, "title": "clean pr", "url": "http://x/10", "isDraft": false, "mergeStateStatus": "CLEAN", "reviewDecision": "", + "baseRefName": "main", "headRefOid": "1010101010101010101010101010101010101010"} ] EOF +# #10's head contains the base tip; #13's is four commits behind it. +cat >"$TMP/behind.json" <<'EOF' +{"10": 0, "13": 4} +EOF + # #100 uppercase marker must beat an earlier "not recommended"; #101 lowercase # fallback; #102 no marker at all; #103 marker only in a comment. cat >"$TMP/decisions.json" <<'EOF' @@ -247,6 +258,7 @@ printf '[]\n' >"$TMP/repo-labels-empty.json" OUT="$(bash "$BRIEF" --now "$NOW" --stale-hours 6 \ --counts-json "$TMP/counts.json" \ --pr-json "$TMP/pr.json" \ + --behind-json "$TMP/behind.json" \ --decisions-json "$TMP/decisions.json" \ --telemetry-json "$TMP/telemetry.json" \ --merged-json "$TMP/merged.json" 2>&1)" @@ -266,6 +278,28 @@ assert_contains "merge-ready none-review shows 'none'" "$OUT" "review=none" assert_not_contains "merge-ready drops draft #11" "$OUT" "#11 draft pr" assert_not_contains "merge-ready drops blocked #12" "$OUT" "#12 blocked pr" assert_contains "merge-ready points to authoritative gate" "$OUT" "/source-control:babysit-prs" +MERGE_READY="$(section "$OUT" "== Merge-ready")" +assert_contains "merge-ready flags a clean PR behind its base as unverified" "$MERGE_READY" \ + " http://x/13 review=APPROVED + UNVERIFIED: head is 4 commit(s) behind main; checks may predate the current base" +assert_contains "merge-ready leaves a clean PR containing the base tip unflagged" "$MERGE_READY" \ + " http://x/10 review=none + #13 approved clean pr" +assert_not_contains "merge-ready reports no unread freshness when every count is known" "$MERGE_READY" "base freshness unread" + +# Freshness that was never read is UNVERIFIED, never silently fresh. +OUT_NO_BEHIND="$(bash "$BRIEF" --now "$NOW" --pr-json "$TMP/pr.json" \ + --counts-json "$TMP/counts.json" --decisions-json "$TMP/decisions.json" \ + --telemetry-json "$TMP/telemetry.json" --merged-json "$TMP/merged.json" 2>&1)" +assert_contains "merge-ready without a behind fixture says freshness is unread" "$(section "$OUT_NO_BEHIND" "== Merge-ready")" \ + " http://x/10 review=none + UNVERIFIED: base freshness unread: --pr-json given without --behind-json" +printf '{"10": 0}\n' >"$TMP/behind-partial.json" +OUT_PARTIAL_BEHIND="$(bash "$BRIEF" --now "$NOW" --pr-json "$TMP/pr.json" --behind-json "$TMP/behind-partial.json" \ + --counts-json "$TMP/counts.json" --decisions-json "$TMP/decisions.json" \ + --telemetry-json "$TMP/telemetry.json" --merged-json "$TMP/merged.json" 2>&1)" +assert_contains "merge-ready with no count for a PR says freshness is unread" "$(section "$OUT_PARTIAL_BEHIND" "== Merge-ready")" \ + " UNVERIFIED: base freshness unread: no behind count for #13" # Decisions — two-tier RECOMMENDED extraction assert_contains "decision #100 uppercase marker wins" "$OUT" "Store the root in a userConfig key" @@ -635,11 +669,6 @@ run_stub() { bash "$BRIEF" --now "$NOW" --stale-hours 6 --repo "$FIXTURE_REPO" "$@" 2>&1 } -section() { - # section OUTPUT "== heading prefix" — the lines of one section, heading included. - printf '%s\n' "$1" | awk -v h="$2" 'index($0, h) == 1 {p = 1; print; next} /^== / {p = 0} p' -} - # Every section live, every call refused: nothing may read as clear or absent. OUT_BLOCKED="$(run_stub blocked)" RC_BLOCKED=$? @@ -737,7 +766,12 @@ cat >"$REST/$(rest_key "$R/pulls?state=open&per_page=100").json" <<'EOF' [{"number": 9, "title": "blocked one"}, {"number": 8, "title": "draft one"}, {"number": 7, "title": "clean one"}] EOF cat >"$REST/$(rest_key "$R/pulls/7").json" <<'EOF' -{"number": 7, "title": "clean one", "html_url": "http://x/7", "draft": false, "mergeable": true, "mergeable_state": "clean"} +{"number": 7, "title": "clean one", "html_url": "http://x/7", "draft": false, "mergeable": true, "mergeable_state": "clean", + "base": {"ref": "release/1.x"}, "head": {"sha": "7777777777777777777777777777777777777777"}} +EOF +# The base name carries a slash, which the compare path must percent-encode. +cat >"$REST/$(rest_key "$R/compare/release%2F1.x...7777777777777777777777777777777777777777?per_page=1").json" <<'EOF' +{"status": "diverged", "ahead_by": 1, "behind_by": 2} EOF cat >"$REST/$(rest_key "$R/pulls/8").json" <<'EOF' {"number": 8, "title": "draft one", "html_url": "http://x/8", "draft": true, "mergeable": true, "mergeable_state": "clean"} @@ -765,6 +799,8 @@ assert_contains "rest: merge-ready keeps the clean non-draft" "$OUT_REST" "#7 cl assert_not_contains "rest: merge-ready drops the draft" "$OUT_REST" "#8 draft one" assert_not_contains "rest: merge-ready drops the blocked" "$OUT_REST" "#9 blocked one" assert_contains "rest: review decision is reported as n/a" "$OUT_REST" "review=n/a" +assert_contains "rest: the compare read flags the clean PR behind its base" "$OUT_REST" \ + "UNVERIFIED: head is 2 commit(s) behind release/1.x; checks may predate the current base" assert_contains "rest: decision RECOMMENDED line is extracted" "$OUT_REST" "RECOMMENDED: option B." assert_contains "rest: telemetry issue found by title" "$OUT_REST" "source: issue #50" assert_contains "rest: telemetry lane age computed" "$OUT_REST" "babysit last-cycle=2026-07-20T06:30Z age=1h 30m" @@ -782,9 +818,11 @@ cat >"$TMP/same-pr.json" <<'EOF' [ {"number": 9, "title": "blocked one", "url": "http://x/9", "isDraft": false, "mergeStateStatus": "BLOCKED", "reviewDecision": "n/a"}, {"number": 8, "title": "draft one", "url": "http://x/8", "isDraft": true, "mergeStateStatus": "CLEAN", "reviewDecision": "n/a"}, - {"number": 7, "title": "clean one", "url": "http://x/7", "isDraft": false, "mergeStateStatus": "CLEAN", "reviewDecision": "n/a"} + {"number": 7, "title": "clean one", "url": "http://x/7", "isDraft": false, "mergeStateStatus": "CLEAN", "reviewDecision": "n/a", + "baseRefName": "release/1.x", "headRefOid": "7777777777777777777777777777777777777777"} ] EOF +printf '{"7": 2}\n' >"$TMP/same-behind.json" cat >"$TMP/same-decisions.json" <<'EOF' [{"number": 42, "title": "pick a store", "url": "http://x/i/42", "body": "Options weighed.\nRECOMMENDED: option B.", "comments": [{"body": "no change of lean"}]}] EOF @@ -795,6 +833,7 @@ OUT_SAME="$(bash "$BRIEF" --now "$NOW" --stale-hours 6 --repo "$FIXTURE_REPO" \ --repo-labels-json "$TMP/same-labels.json" \ --counts-json "$TMP/same-counts.json" \ --pr-json "$TMP/same-pr.json" \ + --behind-json "$TMP/same-behind.json" \ --decisions-json "$TMP/same-decisions.json" \ --telemetry-json "$TMP/same-telemetry.json" \ --merged-json "$TMP/merged-clean.json" 2>&1)" @@ -830,6 +869,17 @@ assert_contains "rest: an uncomputed merge state is reported as inconclusive" "$ assert_contains "rest: the computed PR beside it still renders" "$OUT_UNCOMPUTED" "#7 clean one" assert_not_contains "rest: the uncomputed PR is never listed as merge-ready" "$OUT_UNCOMPUTED" "#10 still computing" +# A compare read that fails leaves the clean PR listed, flagged unverified with +# the error, never as fresh. +NO_COMPARE="$TMP/rest-no-compare" +mkdir -p "$NO_COMPARE" +cp "$REST/"*.json "$NO_COMPARE/" +rm "$NO_COMPARE/$(rest_key "$R/compare/release%2F1.x...7777777777777777777777777777777777777777?per_page=1").json" +OUT_NO_COMPARE="$(MB_GH_FIXTURES="$NO_COMPARE" run_stub rest)" +assert_contains "rest: a failed compare read flags the clean PR unverified" "$(section "$OUT_NO_COMPARE" "== Merge-ready")" \ + " http://x/7 review=n/a + UNVERIFIED: base freshness unread: Not Found" + # The per-PR merge-state read is capped, and a capped read says so. OUT_CAP="$(run_stub rest --pr-limit 1)" assert_contains "rest: a capped merge-state read is reported as PARTIAL" "$OUT_CAP" "PARTIAL: 3 open PRs; merge state read for the first 1 only" diff --git a/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh b/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh index 70a977b744..2c88cc7f5f 100755 --- a/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh +++ b/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh @@ -10,7 +10,10 @@ # It runs `gh` read queries only. The authoritative merge gate lives in the # source-control:babysit-prs skill; the merge-ready list here is a lighter # gh-native signal (mergeStateStatus CLEAN + non-draft) meant for a 5-second -# glance, not a substitute for that skill's classification. +# glance, not a substitute for that skill's classification. CLEAN reports the +# checks GitHub last ran, which can predate the current base, so a clean PR +# whose head is behind its base (or whose comparison could not be read) carries +# an UNVERIFIED line. # # Owner/repo is derived from `gh repo view`, or from the checkout's `origin` # remote when that call is unavailable; never hardcoded, so the tool is @@ -46,6 +49,9 @@ # --counts-json FILE label->count object, e.g. {"status: ready":4} # --repo-labels-json FILE array of label names (or {name} objects) for existence checks # --pr-json FILE array as emitted by `gh pr list --json ...` +# --behind-json FILE object mapping a PR number to how many commits +# its head is behind its base, e.g. {"10":0,"13":4} +# (the compare reads of the merge-ready section) # --decisions-json FILE array of {number,title,url,body,comments:[{body}]} # --telemetry-json FILE array of {body} (the telemetry issue's comments) # --merged-json FILE array of merged-PR GraphQL page documents, as @@ -84,6 +90,7 @@ PR_LIMIT="50" NOW_ISO="" COUNTS_JSON="" PR_JSON="" +BEHIND_JSON="" DECISIONS_JSON="" TELEMETRY_JSON="" MERGED_JSON="" @@ -206,6 +213,12 @@ while (($# > 0)); do PR_JSON="$2" shift 2 ;; + --behind-json) + require_value "$1" "${2:-}" + require_file "$1" "$2" + BEHIND_JSON="$2" + shift 2 + ;; --decisions-json) require_value "$1" "${2:-}" require_file "$1" "$2" @@ -639,7 +652,7 @@ print_queues() { # ============================================================================= fetch_prs_gql() { gh_read "$1" pr list "${REPO_ARGS[@]}" --state open --limit 200 \ - --json number,title,url,isDraft,mergeStateStatus,reviewDecision + --json number,title,url,isDraft,mergeStateStatus,reviewDecision,baseRefName,headRefOid } PR_PARTIAL=() @@ -688,7 +701,39 @@ fetch_prs_rest() { # reports it as n/a. jq -s '[ .[] | {number, title, url: .html_url, isDraft: .draft, mergeStateStatus: ((.mergeable_state // "unknown") | ascii_upcase), - reviewDecision: "n/a"} ]' "$WORK/prs.detail" >"$out" + reviewDecision: "n/a", baseRefName: .base.ref, headRefOid: .head.sha} ]' "$WORK/prs.detail" >"$out" +} + +# CLEAN says the checks GitHub last ran passed, not that they ran against the +# current base: without a strict up-to-date rule a PR stays CLEAN while its +# base moves on (SKILL.md carries the upstream record). A head that already +# contains the base tip leaves nothing untested, so the compare endpoint's +# `behind_by` decides: 0 prints nothing; a positive count, or a read that +# failed, prints why the PR's CLEAN is unverified. +# +# freshness_note NUMBER BASE HEAD_SHA +freshness_note() { + local number="$1" base="$2" sha="$3" behind="" + if [[ -z "$base" || ! "$sha" =~ ^[0-9a-f]{40}$ ]]; then + printf 'base freshness unread: the PR record names no base branch or head commit' + return + fi + if [[ -n "$BEHIND_JSON" ]]; then + behind="$(jq -r --arg n "$number" '.[$n] // empty' "$BEHIND_JSON" 2>/dev/null)" + elif [[ -n "$PR_JSON" ]]; then + printf 'base freshness unread: --pr-json given without --behind-json' + return + elif gh_read "$WORK/compare" api "repos/$REPO/compare/$(urlencode "$base")...$sha?per_page=1"; then + behind="$(jq -r '.behind_by // empty' "$WORK/compare" 2>/dev/null)" + else + printf 'base freshness unread: %s' "$LAST_ERR" + return + fi + if [[ ! "$behind" =~ ^[0-9]+$ ]]; then + printf 'base freshness unread: no behind count for #%s' "$number" + elif ((behind > 0)); then + printf 'head is %s commit(s) behind %s; checks may predate the current base' "$behind" "$base" + fi } print_merge_ready() { @@ -702,12 +747,16 @@ print_merge_ready() { return } fi - local ready - ready="$(jq -r ' - [ .[] | select(.isDraft == false and .mergeStateStatus == "CLEAN") ] - | sort_by(.number) - | .[] + local clean='[ .[] | select(.isDraft == false and .mergeStateStatus == "CLEAN") ] | sort_by(.number) | .[]' + local notes='{}' number base sha note ready + # stdin from /dev/null so gh cannot drain the loop's input. + while IFS=$'\t' read -r number base sha; do + note="$(freshness_note "$number" "$base" "$sha" /dev/null) + ready="$(jq -r --argjson notes "$notes" "$clean"' | " #\(.number) \(.title)\n \(.url) review=\(.reviewDecision // "" | if . == "" then "none" else . end)" + + ($notes[.number | tostring] | if . then "\n UNVERIFIED: \(.)" else "" end) ' "$prs_file" 2>/dev/null)" if [[ -n "$ready" ]]; then echo "$ready" From 764b99fda2b014a9d2e08db69f7a673a1787e53d Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:10:25 -0400 Subject: [PATCH 04/10] fix(guardrails): keep the new GitHub App token patterns linear-time The ghs__ alternative was quadratic on backtracking engines, and in GNU grep under a UTF-8 locale. A 50 KB adversarial command stalled the guard log and the secret scanners for tens of seconds. - Every site now requires the eyJ JWT header, bounds the header segment, and spells out the dot-separated segments. - grep-based scanners run under LC_ALL=C. - The guard log redacts only a bounded prefix of each command. - machine-profile bounds its generic JWT rule. - Timing tests pin each adversarial input under a fixed bound. Co-Authored-By: Claude Opus 5.5 --- plugins/disk-hygiene/CHANGELOG.md | 5 +++ .../disk-hygiene/lib/guard_decision_log.py | 23 +++++++++-- .../lib/test_guard_decision_log.py | 31 +++++++++++++-- plugins/guardrails/CHANGELOG.md | 6 ++- .../hooks/secret-pattern-detection.test.sh | 22 +++++++++-- .../lib/secret-detection/secret-patterns.sh | 38 ++++++++++--------- plugins/harness-config/CHANGELOG.md | 2 +- .../skills/audit/scripts/audit-engine.sh | 9 +++-- .../skills/audit/scripts/audit-engine.test.sh | 21 ++++++++-- plugins/harness-ops/CHANGELOG.md | 3 ++ .../skills/machine-profile/scripts/profile.sh | 4 +- .../machine-profile/scripts/profile.test.sh | 14 ++++++- plugins/session-flow/CHANGELOG.md | 4 +- plugins/session-flow/scripts/save_point.py | 4 +- .../scripts/tests/test_save_point.py | 26 ++++++++++--- .../skills/running-retro/scripts/observer.py | 4 +- .../running-retro/scripts/test_observer.py | 20 +++++++++- 17 files changed, 185 insertions(+), 51 deletions(-) diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index a5f853a825..91c6249e4d 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -10,6 +10,11 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format fol - The guard decision log redacts GitHub App installation tokens in the `ghs__` format GitHub rolls out from 2026-04-27. The old pattern stopped at the `_` after the app ID, so the token was written to the log in full. +- Redacting a command for the guard decision log no longer stalls the guard hook. The + credential-name rule (`FOO_KEY=...`) backtracked in cubic time, so a 4 KB command of repeated + `KEY` took about 40 seconds; it now makes one attempt per name. A value longer than 4096 + characters is scanned only to that bound, and the kept text is narrowed by what redaction + removed, so a secret cut at the bound is never shown. ## [0.42.9] - 2026-10-02 diff --git a/plugins/disk-hygiene/lib/guard_decision_log.py b/plugins/disk-hygiene/lib/guard_decision_log.py index f103fff51d..db1a61ee50 100644 --- a/plugins/disk-hygiene/lib/guard_decision_log.py +++ b/plugins/disk-hygiene/lib/guard_decision_log.py @@ -58,6 +58,10 @@ # Per generation; two generations are kept, so the bound is about 2 MiB. MAX_BYTES = 1_048_576 MAX_TEXT_CHARS = 400 +# Redaction cost grows faster than linearly on some shapes, so a longer value +# is scanned only up to here; a secret starting in the kept text and longer +# than the scan bound less MAX_TEXT_CHARS is not seen. +MAX_SCAN_CHARS = 4096 FILE_MODE = 0o600 DIR_MODE = 0o700 @@ -94,8 +98,10 @@ re.DOTALL, ), re.compile(r"\b(?:sk|rk|pk)-[A-Za-z0-9_-]{16,}"), + # The bounded `eyJ` header and the spelled-out segments keep this linear; + # an unbounded first segment is quadratic on a repeated `ghs_1_-`. re.compile( - r"\b(?:ghs_[0-9]+_[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2}" + r"\b(?:ghs_[0-9]+_eyJ[A-Za-z0-9_-]{0,512}\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+" r"|gh[pousr]_[A-Za-z0-9]{20,})" ), re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{10,}"), @@ -107,10 +113,12 @@ r"['\"]?\s*[:=]\s*['\"]?[A-Za-z0-9._+/=-]{8,}" ), re.compile(r"\b[a-z][a-z0-9+.-]*://[^\s:@/]+:[^\s:@/]+@[^\s]+"), + # One attempt per name, consumed possessively: two open-ended runs around the + # keyword backtrack in cubic time on a name like `KEYKEY...`. re.compile( - r"(?i)(?:\$env:)?[A-Za-z_][A-Za-z0-9_]*" - r"(?:SECRET|KEY|TOKEN|PASSWORD|PASSWD|PWD|CREDENTIAL)[A-Za-z0-9_]*" - r"\s*[=:]\s*['\"]?[^\s'\"]+" + r"(?i)(?:\$env:)?(? str | None: if value is None: return None text = value if isinstance(value, str) else str(value) + if len(text) > MAX_SCAN_CHARS: + # Scan only a bounded prefix, and narrow the kept window by what + # redaction removed, so every kept character comes from the first + # MAX_TEXT_CHARS of the input and a secret cut at the bound stays out. + scanned = _redact_secrets(text[:MAX_SCAN_CHARS]) + window = MAX_TEXT_CHARS - max(0, MAX_SCAN_CHARS - len(scanned)) + return scanned[: max(0, window)] + "..." text = _redact_secrets(text) if len(text) > MAX_TEXT_CHARS: return text[:MAX_TEXT_CHARS] + "..." diff --git a/plugins/disk-hygiene/lib/test_guard_decision_log.py b/plugins/disk-hygiene/lib/test_guard_decision_log.py index c326673a9b..e3ccda530c 100755 --- a/plugins/disk-hygiene/lib/test_guard_decision_log.py +++ b/plugins/disk-hygiene/lib/test_guard_decision_log.py @@ -8,6 +8,7 @@ import os import stat import tempfile +import time import unittest from pathlib import Path from unittest import mock @@ -146,14 +147,38 @@ def test_secret_shaped_command_and_reason_are_redacted_before_clip(self) -> None def test_github_app_installation_token_jwt_form_is_redacted(self) -> None: # ghs__, about 520 characters; the segments spell FAKE. payload = "FAKEpayload" + ("A" * 450) - token = ( - "ghs" + "_1234567_FAKEheaderNOTaJWT." + payload + ".FAKEsignatureNOTreal" - ) + token = "ghs" + "_1234567_eyJFAKE." + payload + ".FAKEsignatureNOTreal" self.write_one(command="echo " + token) (entry,) = self.read_records() self.assertNotIn(payload[:40], entry["command"]) self.assertEqual("echo " + decision_log.REDACTED, entry["command"]) + def test_redaction_of_adversarial_commands_finishes_promptly(self) -> None: + # Shapes that made a backtracking pattern take seconds to minutes. + for command in ( + "ghs_1_-" * 50000, + "ghs_1_eyJ" * 30000, + "ghs_1_eyJa." * 30000, + "ghs_1_eyJ-" * 30000, + "KEY" * 1300, + "-eyJ" * 75000, + ): + with self.subTest(command=command[:12]): + start = time.monotonic() + decision_log.build_record( + hook="destructive-guard", decision="deny", rule="r", command=command + ) + self.assertLess(time.monotonic() - start, 1.0) + + def test_secret_cut_at_the_scan_bound_stays_out_of_the_record(self) -> None: + # Two redacted assignments shrink the scanned text to a few dozen + # characters, which would pull a token cut at the bound into view. + assignments = ("token=" + "v" * 2000 + " ") * 2 + token = "ghs" + "_1234567_eyJFAKE.FAKEpayload" + "A" * 450 + ".FAKEsig" + self.write_one(command=assignments + token) + (entry,) = self.read_records() + self.assertNotIn("FAKE", entry["command"]) + def test_none_and_deny_by_default_persist_length_not_command_text(self) -> None: secret = "$env:AZURE_CLIENT_SECRET='s3cretvalue'; Get-Process" self.write_one( diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index fa016cb097..6b1a391d71 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -9,8 +9,10 @@ All notable changes to the `guardrails` plugin are documented here. Format follo - Secret detection catches GitHub App installation tokens in the `ghs__` format GitHub rolls out from 2026-04-27 (about 520 characters, length varies). The new pattern matches - `ghs_`, a numeric app ID, `_`, and three dot-separated base64url segments; the 36-character - `ghs_`/`ghu_` form is still detected. + `ghs_`, a numeric app ID, `_`, and three dot-separated base64url segments, the first starting + `eyJ` as every JWT header does; the 36-character `ghs_`/`ghu_` form is still detected. The scan + runs grep under `LC_ALL=C`: in a UTF-8 locale GNU grep took 25 to 60 seconds on a 300 KB line + against the combined pattern set. ## [0.46.8] - 2026-10-02 diff --git a/plugins/guardrails/hooks/secret-pattern-detection.test.sh b/plugins/guardrails/hooks/secret-pattern-detection.test.sh index e0ff8272c6..451f31f72a 100755 --- a/plugins/guardrails/hooks/secret-pattern-detection.test.sh +++ b/plugins/guardrails/hooks/secret-pattern-detection.test.sh @@ -55,9 +55,9 @@ GH_PAT="${GH_PREFIX}$(printf 'a%.0s' {1..36})" GHS_PREFIX='ghs_' GH_APP_TOKEN="${GHS_PREFIX}$(printf 'b%.0s' {1..36})" # The ghs__ installation-token format GitHub rolls out from -# 2026-04-27, about 520 characters: dot-separated JWT segments that spell -# FAKE and are not base64 JSON. -GH_APP_TOKEN_JWT="${GHS_PREFIX}1234567_FAKEheaderNOTaJWT.FAKEpayload$(printf 'A%.0s' {1..450}).FAKEsignatureNOTreal" +# 2026-04-27, about 520 characters: a JWT's `eyJ` header start, then +# dot-separated segments that spell FAKE. +GH_APP_TOKEN_JWT="${GHS_PREFIX}1234567_eyJFAKE.FAKEpayload$(printf 'A%.0s' {1..450}).FAKEsignatureNOTreal" SLACK_PREFIX='xoxb-' SLACK_TOKEN="${SLACK_PREFIX}1234567890123-9876543210987" STRIPE_PREFIX='sk_live_' @@ -95,6 +95,22 @@ RC=$? assert_exit "GitHub App token, ghs__ form → exit 2" 2 "$RC" assert_contains "GH App token, ghs__ form → message" "$OUT" "GitHub App Token" +# One long line that once took GNU grep 25-60 s in a UTF-8 locale. `timeout 10` +# is the backstop: a slow scan reads as rc 124, a finished one as 0. +SLOW_SHAPES=( + "${GHS_PREFIX}1_-:50000" "${GHS_PREFIX}1_eyJ:30000" "${GHS_PREFIX}1_eyJa.:30000" "${GHS_PREFIX}1_eyJ-:30000" +) +for shape in "${SLOW_SHAPES[@]}"; do + unit="${shape%:*}" count="${shape##*:}" + printf -v content '%*s' "$count" '' + printf '%s' "${content// /$unit}" >"$TEST_TMPDIR/slow.txt" + rc=0 + # shellcheck disable=SC2016 # expanded by the inner bash + LC_ALL=C.UTF-8 timeout 10 bash -c 'source "$1"; secrets::scan_text "$(<"$2")" >/dev/null || :' \ + _ "$HOOK_DIR/../lib/secret-detection/secret-patterns.sh" "$TEST_TMPDIR/slow.txt" || rc=$? + assert_exit "scan of '${unit}' x${count} finishes" 0 "$rc" +done + OUT=$(bash "$HOOK" <<<"$(write_json "$FIXTURE" "SLACK='$SLACK_TOKEN'")" 2>&1) RC=$? assert_exit "Slack Bot Token → exit 2" 2 "$RC" diff --git a/plugins/guardrails/lib/secret-detection/secret-patterns.sh b/plugins/guardrails/lib/secret-detection/secret-patterns.sh index c262eb0b47..cef336211d 100644 --- a/plugins/guardrails/lib/secret-detection/secret-patterns.sh +++ b/plugins/guardrails/lib/secret-detection/secret-patterns.sh @@ -8,10 +8,12 @@ # # Pattern selection: only HIGH-confidence patterns with distinctive prefixes # and fixed lengths or a fixed structure (the ghs__ installation -# token varies in length, so it is matched by its dot-separated JWT segments). -# Generic patterns (password=, api_key=, secret=) are +# token varies in length, so it is matched by its `eyJ` JWT header and +# dot-separated segments). Generic patterns (password=, api_key=, secret=) are # excluded — too many false positives for a real-time blocking hook. Sourced -# from gitleaks, TruffleHog, and secrets-patterns-db. grep -E (POSIX ERE) only. +# from gitleaks, TruffleHog, and secrets-patterns-db. grep -E (POSIX ERE) only, +# run under LC_ALL=C: in a UTF-8 locale GNU grep takes quadratic time on a long +# line against the combined set. # (label, ERE) parallel arrays — one combined grep can fast-reject the common # (no-secret) case in a single process. Index alignment is load-bearing: the @@ -32,19 +34,19 @@ SECRET_LABELS=( "Private Key (PEM)" ) SECRET_PATTERNS=( - '(AKIA|ASIA|ABIA|ACCA)[A-Z0-9]{16}' # AWS (AKIA/ASIA/ABIA/ACCA + 16) - 'ghp_[0-9a-zA-Z]{36}' # GitHub PAT - 'gho_[0-9a-zA-Z]{36}' # GitHub OAuth - 'gh[us]_[0-9a-zA-Z]{36}' # GitHub app (ghu_/ghs_) - 'ghs_[0-9]+_[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+){2}' # GitHub app ghs__ - 'github_pat_[0-9a-zA-Z_]{82}' # GitHub fine-grained PAT - 'glpat-[0-9a-zA-Z_-]{20}' # GitLab PAT - 'xoxb-[0-9]{10,13}-[0-9]{10,13}' # Slack bot token - 'xox[pe]-[0-9]{10,13}-' # Slack user/app token - '[sr]k_(test|live|prod)_[0-9a-zA-Z]{10,99}' # Stripe key - 'sk-(proj|svcacct|admin)-[A-Za-z0-9_-]{20,}' # OpenAI prefixed API key - 'sk-[A-Za-z0-9]{20,}' # OpenAI legacy bare sk- key - '-----BEGIN [A-Z ]*PRIVATE KEY-----' # PEM private key header + '(AKIA|ASIA|ABIA|ACCA)[A-Z0-9]{16}' # AWS (AKIA/ASIA/ABIA/ACCA + 16) + 'ghp_[0-9a-zA-Z]{36}' # GitHub PAT + 'gho_[0-9a-zA-Z]{36}' # GitHub OAuth + 'gh[us]_[0-9a-zA-Z]{36}' # GitHub app (ghu_/ghs_) + 'ghs_[0-9]+_eyJ[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+' # GitHub app ghs__ + 'github_pat_[0-9a-zA-Z_]{82}' # GitHub fine-grained PAT + 'glpat-[0-9a-zA-Z_-]{20}' # GitLab PAT + 'xoxb-[0-9]{10,13}-[0-9]{10,13}' # Slack bot token + 'xox[pe]-[0-9]{10,13}-' # Slack user/app token + '[sr]k_(test|live|prod)_[0-9a-zA-Z]{10,99}' # Stripe key + 'sk-(proj|svcacct|admin)-[A-Za-z0-9_-]{20,}' # OpenAI prefixed API key + 'sk-[A-Za-z0-9]{20,}' # OpenAI legacy bare sk- key + '-----BEGIN [A-Z ]*PRIVATE KEY-----' # PEM private key header ) # secrets::scan_text @@ -68,13 +70,13 @@ secrets::scan_text() { for pattern in "${SECRET_PATTERNS[@]}"; do grep_e_args+=(-e "$pattern") done - if ! grep -qE "${grep_e_args[@]}" < <(printf '%s' "$content") 2>/dev/null; then + if ! LC_ALL=C grep -qE "${grep_e_args[@]}" < <(printf '%s' "$content") 2>/dev/null; then return 0 fi for i in "${!SECRET_PATTERNS[@]}"; do label="${SECRET_LABELS[$i]}" pattern="${SECRET_PATTERNS[$i]}" - lines=$(grep -nE -- "$pattern" < <(printf '%s' "$content") 2>/dev/null | + lines=$(LC_ALL=C grep -nE -- "$pattern" < <(printf '%s' "$content") 2>/dev/null | head -3 | cut -d: -f1 | tr '\n' ',' | sed 's/,$//') if [[ -n "$lines" ]]; then printf '%s (line %s)\n' "$label" "$lines" diff --git a/plugins/harness-config/CHANGELOG.md b/plugins/harness-config/CHANGELOG.md index f5150c3c3f..525ae230ce 100644 --- a/plugins/harness-config/CHANGELOG.md +++ b/plugins/harness-config/CHANGELOG.md @@ -9,7 +9,7 @@ Versions 0.51.8 to 0.51.9 and 0.51.11 to 0.51.14 were reserved by parallel branc ### Fixed -- The audit engine's secret-shape check (`SECRET_RE`, which flags a token in tracked `settings.json` and redacts hook commands) covers GitHub OAuth, user, server and refresh tokens (`gho_`, `ghu_`, `ghs_`, `ghr_`) and the `ghs__` installation-token format GitHub rolls out from 2026-04-27. Before, only `ghp_` and `github_pat_` were matched. +- The audit engine's secret-shape check (`SECRET_RE`, which flags a token in tracked `settings.json` and redacts hook commands) covers GitHub OAuth, user, server and refresh tokens (`gho_`, `ghu_`, `ghs_`, `ghr_`) and the `ghs__` installation-token format GitHub rolls out from 2026-04-27, whose JWT header starts `eyJ`. Before, only `ghp_` and `github_pat_` were matched. The check runs grep under `LC_ALL=C`, because in a UTF-8 locale GNU grep took 25 to 60 seconds on a long line against the widened pattern. ## [1.3.3] - 2026-10-02 diff --git a/plugins/harness-config/skills/audit/scripts/audit-engine.sh b/plugins/harness-config/skills/audit/scripts/audit-engine.sh index 3f33c42c80..18347e1920 100755 --- a/plugins/harness-config/skills/audit/scripts/audit-engine.sh +++ b/plugins/harness-config/skills/audit/scripts/audit-engine.sh @@ -1361,8 +1361,9 @@ fi # --- Category D: hooks --------------------------------------------------------- # Token shapes that never belong in a claim or detail. Used here to redact a -# hook command and in category F to flag a tracked settings value. -SECRET_RE='gh[pousr]_[A-Za-z0-9]{20,}|ghs_[0-9]+_[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+){2}|github_pat_[A-Za-z0-9_]{20,}|eyJ[A-Za-z0-9_-]{15,}\.[A-Za-z0-9_-]{5,}|sk-[A-Za-z0-9_-]{16,}|AKIA[0-9A-Z]{16}|xox[abp]-[A-Za-z0-9-]{10,}' +# hook command and in category F to flag a tracked settings value. Match it under +# LC_ALL=C: in a UTF-8 locale GNU grep takes quadratic time on a long line here. +SECRET_RE='gh[pousr]_[A-Za-z0-9]{20,}|ghs_[0-9]+_eyJ[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+|github_pat_[A-Za-z0-9_]{20,}|eyJ[A-Za-z0-9_-]{15,}\.[A-Za-z0-9_-]{5,}|sk-[A-Za-z0-9_-]{16,}|AKIA[0-9A-Z]{16}|xox[abp]-[A-Za-z0-9-]{10,}' resolve_hook_path() { # resolve_hook_path -> the first token with placeholders expanded @@ -1415,7 +1416,7 @@ while IFS=$'\t' read -r src event matcher cmd timeout htype hif hargs; do # raw command still feeds the anchor, which stores only a hash. cmd_ref="$cmd" cmd_public=1 - if [[ "$surface" == "$SURF_LOCAL" || "$surface" == "$SURF_USER" ]] || printf '%s' "$cmd" | grep -Eq "$SECRET_RE"; then + if [[ "$surface" == "$SURF_LOCAL" || "$surface" == "$SURF_USER" ]] || printf '%s' "$cmd" | LC_ALL=C grep -Eq "$SECRET_RE"; then cmd_ref="cmd:$(anchor_for_excerpt "$cmd")" cmd_public=0 fi @@ -1761,7 +1762,7 @@ fi # --- Category F: environment variables ----------------------------------------- if [[ $PROJECT_OK -eq 1 ]]; then - if tr -d '\r' <"$SETTINGS" | grep -Eq "$SECRET_RE"; then + if tr -d '\r' <"$SETTINGS" | LC_ALL=C grep -Eq "$SECRET_RE"; then row F secrets finding error "$SURF_SETTINGS" "secret-shaped-value" "a token-shaped value is present in the tracked settings file; move it to settings.local.json or a credential store" /env else row F secrets ok none "$SURF_SETTINGS" "no-secret-shaped-value" "no token-shaped value in settings.json" - diff --git a/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh b/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh index b6ea3a4d5f..9f2190be09 100755 --- a/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh +++ b/plugins/harness-config/skills/audit/scripts/audit-engine.test.sh @@ -537,15 +537,30 @@ out=$(DOCS_FIXTURE="$m/nodocs" run "$m" --json 2>&1) || true assert_eq "case 11: without the env-vars page the documentation row is not inspectable" "not-inspectable" "$(jq -r '.rows[] | select(.claim=="env-page-not-fetched:MY_SINK") | .status' <<<"$out")" # GitHub app and OAuth tokens, including the ghs__ installation # token format GitHub rolls out from 2026-04-27. Assembled at runtime so no -# token-shaped literal sits in this file; the JWT segments spell FAKE and are -# not base64 JSON, so only the GitHub rule can match them. +# token-shaped literal sits in this file; past the `eyJ` header start the JWT +# segments spell FAKE and are too short for the generic JWT rule, so only the +# GitHub rule can match them. ghs_prefix='ghs_' -for tok in "${ghs_prefix}1234567_FAKEheaderNOTaJWT.FAKEpayload$(printf 'A%.0s' {1..450}).FAKEsignatureNOTreal" \ +for tok in "${ghs_prefix}1234567_eyJFAKE.FAKEpayload$(printf 'A%.0s' {1..450}).FAKEsignatureNOTreal" \ "${ghs_prefix}$(printf 'b%.0s' {1..36})" "gho""_$(printf 'c%.0s' {1..36})" "ghu""_$(printf 'd%.0s' {1..36})"; do printf '%s\n' "$CLEAN_SETTINGS" | jq --arg t "$tok" '. + {env:{GH:$t}}' >"$m/project/.claude/settings.json" out=$(run "$m" --json --docs-dir "$m/docs" 2>&1) || true assert_eq "case 11: ${tok:0:12}... is a secret-shaped error" "error" "$(jq -r '.findings[] | select(.identity.claim=="secret-shaped-value") | .severity' <<<"$out")" done +# Values that once took GNU grep 25-60 s against SECRET_RE in a UTF-8 locale. +# `timeout 30` is the backstop: a slow scan reads as rc 124. +for shape in "${ghs_prefix}1_-:50000" "${ghs_prefix}1_eyJ:30000" "${ghs_prefix}1_eyJa.:30000" "${ghs_prefix}1_eyJ-:30000"; do + unit="${shape%:*}" count="${shape##*:}" + printf -v value '%*s' "$count" '' + printf '%s' "${value// /$unit}" >"$m/slow-value" + printf '%s\n' "$CLEAN_SETTINGS" | jq --rawfile t "$m/slow-value" '. + {env:{GH:$t}}' >"$m/project/.claude/settings.json" + rc=0 + ( + export SCRIPT BASELINE DOCS CLI + LC_ALL=C.UTF-8 timeout 30 bash -c "$(declare -f run); run \"\$@\"" _ "$m" --json --docs-dir "$m/docs" >/dev/null 2>&1 + ) || rc=$? + assert_eq "case 11: the scan of '${unit}' x${count} finishes" finished "$([[ $rc -ne 124 ]] && echo finished || echo "timed out")" +done # Keys keep their tab-separated-values encoding, so a control character never # splits one key into two rows and existing claim identities stay stable. printf '%s\n' "$CLEAN_SETTINGS" | jq '. + {env:{"A\nB":"1"}}' >"$m/project/.claude/settings.json" # portability-ok: a JSON newline escape, not a regex escape diff --git a/plugins/harness-ops/CHANGELOG.md b/plugins/harness-ops/CHANGELOG.md index c6ddf1e2e3..c30d6c0864 100644 --- a/plugins/harness-ops/CHANGELOG.md +++ b/plugins/harness-ops/CHANGELOG.md @@ -15,6 +15,9 @@ All notable changes to the `harness-ops` plugin are documented here. Format foll - `machine-profile` refuses a record value holding a GitHub App installation token in the `ghs__` format GitHub began issuing on 2026-04-27, matched by its own shape rather than only through the generic JWT rule. +- `machine-profile` validates a long record value in linear time. jq's regex engine backtracks, + and the generic JWT rule took about 10 seconds on a 300 KB value of repeated `ghs_1_eyJ`; its + header segment is now capped at 512 characters, as is the `ghs_` rule's. ## [2.5.0] - 2026-10-02 diff --git a/plugins/harness-ops/skills/machine-profile/scripts/profile.sh b/plugins/harness-ops/skills/machine-profile/scripts/profile.sh index 390e5af225..125dfe44d3 100755 --- a/plugins/harness-ops/skills/machine-profile/scripts/profile.sh +++ b/plugins/harness-ops/skills/machine-profile/scripts/profile.sh @@ -30,7 +30,9 @@ command -v jq >/dev/null 2>&1 || die "jq is required" CRED_RE='token|secret|passw(?:or)?d|credential|api.?key|private.?key' # Value shapes the validator refuses in any string of a record: GitHub, AWS, Slack and # sk- API tokens, JWTs, private-key blocks, and a password embedded in a URL. -VAL_RE='ghs_[0-9]+_[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2}|gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|(?"$OUT/slow-value" + jq -nc --rawfile v "$OUT/slow-value" \ + '{machine: {facts: [{key: "k", value: $v, verdict: "set", observed_by: "ls", mode: "observed", supplied_by: "host"}]}, domains: {}}' >"$OUT/slow-doc" + (cd "$FIX/repo" && timeout 10 bash "$SCRIPT" record --data-dir "$OUT/slow" --confirm - <"$OUT/slow-doc" >/dev/null 2>&1) + rc=$? + expect_eq "validating '$unit' x$count finishes" finished "$([[ $rc -ne 124 ]] && echo finished || echo "timed out")" +done refuse "a credential key is refused" '{"key":"api_token","value":"x","verdict":"set","observed_by":"ls","mode":"observed","supplied_by":"host"}' refuse "a sensitive userConfig key is refused" '{"key":"dometrain-mcp.dometrain_api_key","value":"x","verdict":"set","observed_by":"ls","mode":"observed","supplied_by":"host"}' refuse "an AWS access key value is refused" '{"key":"k","value":"AKIA'"IOSFODNN7EXAMPLE"'","verdict":"set","observed_by":"ls","mode":"observed","supplied_by":"host"}' diff --git a/plugins/session-flow/CHANGELOG.md b/plugins/session-flow/CHANGELOG.md index 8f295f3705..aa807412cb 100644 --- a/plugins/session-flow/CHANGELOG.md +++ b/plugins/session-flow/CHANGELOG.md @@ -6,8 +6,8 @@ - The running-retro observer's ledger redaction and the save-point validator's secret-shape scan match GitHub App installation tokens in the `ghs__` format GitHub rolls out from - 2026-04-27. The old pattern stopped at the `_` after the app ID, so such a token was neither - redacted nor warned about. + 2026-04-27, whose JWT header starts `eyJ`. The old pattern stopped at the `_` after the app ID, + so such a token was neither redacted nor warned about. ## [0.44.6] - 2026-10-02 diff --git a/plugins/session-flow/scripts/save_point.py b/plugins/session-flow/scripts/save_point.py index 14521340e3..5f5446c571 100755 --- a/plugins/session-flow/scripts/save_point.py +++ b/plugins/session-flow/scripts/save_point.py @@ -227,8 +227,10 @@ (re.compile(r"-----BEGIN[^-]+PRIVATE KEY-----"), "private key"), (re.compile(r"\b(?:sk|rk|pk)-[A-Za-z0-9_-]{16,}"), "API key"), ( + # The bounded `eyJ` header and the spelled-out segments keep this + # linear; an unbounded first segment is quadratic on a repeated `ghs_1_-`. re.compile( - r"\b(?:ghs_[0-9]+_[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2}" + r"\b(?:ghs_[0-9]+_eyJ[A-Za-z0-9_-]{0,512}\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+" r"|gh[pousr]_[A-Za-z0-9]{20,})" ), "GitHub token", diff --git a/plugins/session-flow/scripts/tests/test_save_point.py b/plugins/session-flow/scripts/tests/test_save_point.py index bb85fb4d09..2a823162ae 100644 --- a/plugins/session-flow/scripts/tests/test_save_point.py +++ b/plugins/session-flow/scripts/tests/test_save_point.py @@ -21,6 +21,7 @@ import shutil import subprocess import sys +import time from pathlib import Path import pytest @@ -460,12 +461,7 @@ def test_validate_flags_a_github_app_installation_token_in_jwt_form(tmp_path): target = handoffs / HOP1 text = target.read_text(encoding="utf-8") # ghs__, about 520 characters; the segments spell FAKE. - token = ( - "ghs" - + "_1234567_FAKEheaderNOTaJWT.FAKEpayload" - + "A" * 450 - + ".FAKEsignatureNOTreal" - ) + token = "ghs" + "_1234567_eyJFAKE.FAKEpayload" + "A" * 450 + ".FAKEsignatureNOTreal" text = text.replace( "None. Nothing waits on a person or an access grant.", f"None. The token {token} was rotated.", @@ -476,6 +472,24 @@ def test_validate_flags_a_github_app_installation_token_in_jwt_form(tmp_path): assert "secret-shaped" in out(result) and "GitHub token" in out(result) +@pytest.mark.parametrize( + "line", + [ + "ghs_1_-" * 50000, + "ghs_1_eyJ" * 30000, + "ghs_1_eyJa." * 30000, + "ghs_1_eyJ-" * 30000, + ], + ids=["dash", "header", "dotted", "header-dash"], +) +def test_secret_shape_scan_of_an_adversarial_line_finishes_promptly(line): + # Shapes that made the GitHub token pattern backtrack for seconds to minutes. + start = time.monotonic() + for pattern, _ in _save_point_module().SECRET_SHAPES: + pattern.search(line) + assert time.monotonic() - start < 1.0 + + @pytest.mark.parametrize("marker", ["- ", "* ", "+ ", "1. ", "2) "]) def test_validate_refuses_a_bulleted_next_headline(tmp_path, marker): handoffs = materialize(tmp_path, "good-chain") diff --git a/plugins/session-flow/skills/running-retro/scripts/observer.py b/plugins/session-flow/skills/running-retro/scripts/observer.py index 8d30524275..58dbe562ed 100755 --- a/plugins/session-flow/skills/running-retro/scripts/observer.py +++ b/plugins/session-flow/skills/running-retro/scripts/observer.py @@ -912,8 +912,10 @@ def _extract_result(stdout: str) -> str: ), (re.compile(r"\b(?:sk|rk|pk)-[A-Za-z0-9_-]{16,}"), ""), ( + # The bounded `eyJ` header and the spelled-out segments keep this + # linear; an unbounded first segment is quadratic on a repeated `ghs_1_-`. re.compile( - r"\b(?:ghs_[0-9]+_[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2}" + r"\b(?:ghs_[0-9]+_eyJ[A-Za-z0-9_-]{0,512}\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+" r"|gh[pousr]_[A-Za-z0-9]{20,})" ), "", diff --git a/plugins/session-flow/skills/running-retro/scripts/test_observer.py b/plugins/session-flow/skills/running-retro/scripts/test_observer.py index b3f688cfc5..6efbe029a3 100755 --- a/plugins/session-flow/skills/running-retro/scripts/test_observer.py +++ b/plugins/session-flow/skills/running-retro/scripts/test_observer.py @@ -817,7 +817,7 @@ def test_shapes(self): ) # ghs__ installation token, about 520 characters; the # segments spell FAKE. - jwt = "FAKEheaderNOTaJWT.FAKEpayload" + "A" * 450 + ".FAKEsignatureNOTreal" + jwt = "eyJFAKE.FAKEpayload" + "A" * 450 + ".FAKEsignatureNOTreal" self.assertEqual( "tok z", r("tok ghs" + "_1234567_" + jwt + " z") ) @@ -828,6 +828,24 @@ def test_shapes(self): def test_clean_passthrough(self): self.assertEqual(observer._redact("no secrets here"), "no secrets here") + def test_github_token_pattern_finishes_promptly_on_adversarial_text(self): + # Shapes that made the pattern backtrack for seconds to minutes. + (github,) = ( + p + for p, marker in observer._REDACTIONS + if marker == "" + ) + for text in ( + "ghs_1_-" * 50000, + "ghs_1_eyJ" * 30000, + "ghs_1_eyJa." * 30000, + "ghs_1_eyJ-" * 30000, + ): + with self.subTest(text=text[:12]): + start = time.monotonic() + github.sub("x", text) + self.assertLess(time.monotonic() - start, 1.0) + class ResultParsing(unittest.TestCase): def test_extract(self): From d7826bd5ce3a3fbf0391fe816b47b506909f8a5a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:10:25 -0400 Subject: [PATCH 05/10] fix(source-control): pin every async merge to the vetted head and verify adopted requests - An async merge always sends the evaluated head as sha, including under --allow-unpinned-head, and gh pr merge always gets --match-head-commit. - A 409 adopts only a pending request whose expected head and merge action match this run. Every reported merge is confirmed against the PR's head, or reported as unconfirmed. - Pending stack requests record their lower-layer heads and are verified when they land in a later run. - A request id read from the state file is validated before use; a corrupt record holds. - stacks.md gives the gh-stack v0.2.0 floor for stacks built in linked worktrees. Co-Authored-By: Claude Opus 5.5 --- plugins/source-control/CHANGELOG.md | 28 +- .../skills/babysit-prs/reference/safety.md | 46 +-- .../babysit-prs/scripts/babysit_merge.py | 221 ++++++++++++--- .../scripts/tests/test_babysit_merge.py | 6 +- .../scripts/tests/test_babysit_merge_async.py | 263 +++++++++++++++++- .../skills/pull-request/reference/stacks.md | 9 + 6 files changed, 495 insertions(+), 78 deletions(-) diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 4e582e0a9a..3fc66d0a81 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -12,23 +12,31 @@ All notable changes to the `source-control` plugin are documented here. Format f trunk and runs the full gate over every open layer below it, since merging the layer lands them too. Under `--auto` a lower layer's missing AI review check holds the merge as well. The stack is re-read just before the merge request and the merge is refused if a lower layer changed. After - the merge every lower layer is checked against the head the gate evaluated, and a mismatch is - reported with exit `10`. Off, a stack layer is held exactly as before. + the merge every lower layer is checked against the head the gate evaluated, including when the + request completes in a later run, and a mismatch is reported with exit `10`. A lower-layer push + between the request and its completion still lands unvetted, so the flag assumes only trusted + actors can push to the lower layers. Off, a stack layer is held exactly as before. - **The `pull-request` skill has a stacked-PR reference** (`reference/stacks.md`) for creating a - stack with the `gh stack` extension and merging it, and `merge.md` shows how to merge through the - async merge API with `gh api`. + stack with the `gh stack` extension and merging it, including the v0.2.0 floor for stacks built + in linked worktrees, and `merge.md` shows how to merge through the async merge API with `gh api`. ### Changed - **The babysit merge gate merges a ready PR on the default branch through GitHub's async merge - API** (`gh api` on the PR's `merge-async` endpoint), pinned to the vetted head with `sha`, - `bypass_rules` always false, and polled for up to 60 seconds. A 409 polls the request already - pending; a host without the endpoint falls back to `gh pr merge`. Any other base and every - `--auto` arm keep `gh pr merge`. A reported merge the gate cannot read back is reported as - unconfirmed (`mergeUnconfirmed: true`, exit `10`), not as merged. + API** (`gh api` on the PR's `merge-async` endpoint), pinned with `sha` to the head the gate + evaluated, `bypass_rules` always false, and polled for up to 60 seconds. A 409 polls the request + already pending unless its body names another head or merge action, which is held instead; a host + without the endpoint falls back to `gh pr merge`. Any other base and every `--auto` arm keep + `gh pr merge`. A reported merge is read back: one at another head exits `10` for a human, and one + the gate cannot read back is reported as unconfirmed (`mergeUnconfirmed: true`, exit `10`), not + as merged. +- **`--allow-unpinned-head` no longer sends an unpinned merge.** It waives only the + `--expected-head` argument; the merge still pins the head the run evaluated. - **A merge request still pending at the bound is recorded under `--state-dir`.** GitHub documents no route to cancel one, so every later run reads it first and, while it is live or unreadable, - reports `action: merge-pending` and sends nothing. Merge commands now carry + reports `action: merge-pending` and sends nothing. A request that later finishes merged is checked + against every head the gate evaluated, and a mismatch is escalated with exit `10`. A record with + an unusable request id is held as corrupt, never read or cleared. Merge commands now carry `--state-dir `. - **A default branch that requires a merge queue no longer blocks the gate.** A fully ready PR is enqueued instead (`action: enqueue`, `enqueued: true`), which is reported as queued, not merged. diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 43bf126f97..78a6f827c1 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -666,12 +666,18 @@ it): a `PUT` to the PR's `merge-async` endpoint, then a `GET` on the request's U the async API has no auto-merge form. On the default branch a 404 from the endpoint (a host that lacks it) falls back to `gh pr merge` with the same pin; a queue or a stack has no other API and holds. -- **Request.** `sha` is the vetted head, `merge_method` is sent for a direct merge only, +- **Request.** `sha` is always the head the gate just evaluated: `--allow-unpinned-head` waives only + the `--expected-head` argument, never the pin, and `gh pr merge` carries the same head as + `--match-head-commit`. `merge_method` is sent for a direct merge only, `merge_action` is `direct_merge` or `merge_queue`, and `bypass_rules` is always `false`. No API-version header is sent: the endpoint is documented under the default version as well. -- **Result.** The gate polls for up to 60 seconds. A 409 means a request is already pending, and - the gate polls the UUID it returns; a 200 means already merged or already queued. A reported merge - is confirmed by reading the PR: a contradiction is not counted, and a failed read reports +- **Result.** The gate polls for up to 60 seconds; a 200 means already merged or already queued. A + 409 means a request is already pending, possibly another actor's. When its body states that + request's `expected_head_sha` or `merge_action`, each must equal this run's pin and action; + otherwise the gate does not poll it, reports it as `merge.conflictingRequest` with exit `10`, and + records it as pending, since it can still merge. A reported merge is confirmed by reading the PR + back, merged and at the pinned head: a contradiction is not counted; a merge at another head + reports `merged: true`, `merge.mergedHead`, and exit `10` for a human; a failed read reports `merged: false`, `mergeUnconfirmed: true`, and exit `10`, so re-run the read-only check rather than call it merged. A 400 is basic PR state only: GitHub does not evaluate rules when it accepts the request, which is why the readiness gate always runs first. @@ -679,10 +685,16 @@ it): a `PUT` to the PR's `merge-async` endpoint, then a `GET` on the request's U it can still merge after a hold appears that would refuse a new request. The gate records its UUID under `--state-dir` and every later run, read-only or merging, reads it first: while it is pending, or cannot be read, the run reports `action: merge-pending` with that hold first in - `blockers`, exit `10`, and sends nothing. A finished request (or one past GitHub's 24-hour - retention) clears the record and shows as `pendingMergeRequest`. Report a merge-pending PR as - "merge may still land", never as held. Without `--state-dir` nothing is recorded and a later run - cannot see the request, so `--state-dir ` rides on every merge form. + `blockers`, exit `10`, and sends nothing. A record whose request id is not GitHub's UUID shape is + corrupt: it is never put in an API path or cleared, and holds the same way until a human inspects + it. A finished request (or one past GitHub's 24-hour retention) clears the record and shows as + `pendingMergeRequest`. A request that finished merged is first checked against every head the + gate evaluated, the PR's and, for a stack, each lower layer's, recorded with it + (`pendingMergeRequest.verification`). A mismatch puts an escalation first in `blockers` with exit + `10`; heads that cannot be read back keep the record, hold the same way, and are re-checked next + run. Report a merge-pending PR as "merge may still land", never as held. Without `--state-dir` + nothing is recorded and a later run cannot see the request, so `--state-dir ` rides on + every merge form. - **Merge queue.** A default-branch base that requires a merge queue is no longer a blocker. Once every other condition holds, the merge enqueues (`action: enqueue`). `enqueued` is final for the request and is not a merge: a later cycle reads the PR as merged, or finds it back out of the @@ -698,14 +710,16 @@ it): a `PUT` to the PR's `merge-async` endpoint, then a `GET` on the request's U top layer only, so the gate re-reads the stack immediately before the request and refuses if any open lower layer was pushed, added, or closed since evaluation. After a reported merge it checks that every lower layer merged at the head it evaluated; a mismatch reports - `stackVerification.verified: false` with exit `10` (`merged` stays true) and goes to a human. - What remains is the window from the request to its completion: a lower-layer push in that window - is not pinned by GitHub, and only that after-the-fact check catches it. - -**Claim, basis, as of, recheck:** the endpoint's request fields, statuses, 409, 200, and 400 -semantics, its 24-hour result retention, the absence of any route to cancel a request (the page -documents only the `PUT` and the `GET`), its listing under the default API version, and its -inclusion of every open downstack PR, + `stackVerification.verified: false` with exit `10` (`merged` stays true) and goes to a human. A + request that completes in a later run gets the same check from its pending record. What remains + is the window from the request to its completion: a lower-layer push in that window is not pinned + by GitHub and lands unvetted, caught only after the fact. `--stacked-prs` therefore assumes only + trusted actors can push to the lower layers' branches; leave it off where anyone else can. + +**Claim, basis, as of, recheck:** the endpoint's request fields, statuses, a pending request's +`details` (`expected_head_sha`, `merge_action`), 409, 200, and 400 semantics, its 24-hour result +retention, the absence of any route to cancel a request (the page documents only the `PUT` and the +`GET`), its listing under the default API version, and its inclusion of every open downstack PR, [merge a pull request asynchronously](https://docs.github.com/rest/pulls/pulls?apiVersion=2026-03-10#merge-a-pull-request-asynchronously) and [async merge API GA](https://github.blog/changelog/2026-10-01-github-async-merge-api-generally-available/); stack membership, trunk rules, and merge behavior, diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index 2c1c50a628..5dbed63bfb 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -1532,7 +1532,7 @@ def request_async_merge( repo: str, number: int, *, - sha: str | None, + sha: str, merge_action: str, method: str | None, ) -> dict[str, Any]: @@ -1553,9 +1553,9 @@ def request_async_merge( f"merge_action={merge_action}", "-F", "bypass_rules=false", + "-f", + f"sha={sha}", ] - if sha: - cmd += ["-f", f"sha={sha}"] if method and merge_action == "direct_merge": cmd += ["-f", f"merge_method={method}"] proc = gh_capture(cmd) @@ -1568,6 +1568,11 @@ def request_async_merge( "uuid": str(details.get("uuid") or ""), "message": str(details.get("message") or payload.get("message") or ""), "stderr": proc.stderr.strip(), + "options": { + key: details[key] + for key in ("expected_head_sha", "merge_action") + if key in details + }, } @@ -1582,13 +1587,6 @@ def poll_async_merge( clock: Callable[[], float] | None = None, ) -> dict[str, Any]: """Poll one async merge request until it is terminal or the bound elapses.""" - if not ASYNC_UUID_RE.match(uuid): - return { - "status": "", - "message": f"unusable async merge request id {uuid!r}", - "readError": True, - "expired": False, - } sleep = sleep or _poll_sleep clock = clock or _poll_clock deadline = clock() + timeout_seconds @@ -1605,7 +1603,19 @@ def poll_async_merge( def read_async_merge(repo: str, number: int, uuid: str) -> dict[str, Any]: """One read of an async merge request. `expired` is a 404: GitHub keeps a - result for 24 hours after its last update and then forgets the UUID.""" + result for 24 hours after its last update and then forgets the UUID. + + A UUID that is not GitHub's shape never reaches the API path: it reads as + `corrupt`, which keeps a recorded request held. + """ + if not ASYNC_UUID_RE.match(uuid): + return { + "status": "", + "message": f"unusable async merge request id {uuid!r}", + "readError": True, + "expired": False, + "corrupt": True, + } try: payload = gh_json(["api", f"repos/{repo}/pulls/{number}/merge-async/{uuid}"]) except (RuntimeError, json.JSONDecodeError) as exc: @@ -1624,23 +1634,36 @@ def read_async_merge(repo: str, number: int, uuid: str) -> dict[str, Any]: } -def pull_request_merged(repo: str, number: int) -> bool | None: - """Whether GitHub reads the PR as merged; None when it cannot be read.""" +def pull_request_landed(repo: str, number: int) -> dict[str, Any]: + """Whether GitHub reads the PR as merged, and its head SHA. + + `merged` is None, and `head` with it, when the PR cannot be read or reads + merged with no head: the head a merge landed cannot then be confirmed. + """ try: data = gh_json( - ["api", f"repos/{repo}/pulls/{number}", "--jq", "{merged: .merged}"] + [ + "api", + f"repos/{repo}/pulls/{number}", + "--jq", + "{merged: .merged, head: .head.sha}", + ] ) except (RuntimeError, json.JSONDecodeError): - return None - merged = data.get("merged") if is_json_object(data) else None - return merged if isinstance(merged, bool) else None + data = None + data = data if is_json_object(data) else {} + merged, head = data.get("merged"), data.get("head") + head = head if isinstance(head, str) and head else None + if not isinstance(merged, bool) or (merged and head is None): + return {"merged": None, "head": None} + return {"merged": merged, "head": head} def async_merge( repo: str, number: int, *, - sha: str | None, + sha: str, merge_action: str, method: str | None, ) -> dict[str, Any]: @@ -1670,6 +1693,20 @@ def async_merge( accepted = request["ok"] or request["httpStatus"] == 409 if not accepted: return record + # A 409 names a request this run did not send, possibly another actor's. + # Options it states must be this run's; options it omits leave the + # read-back below to confirm the head that merged. + expected = {"expected_head_sha": sha, "merge_action": merge_action} + if request["httpStatus"] == 409 and any( + request["options"].get(key, value) != value for key, value in expected.items() + ): + record["conflictingRequest"] = request["options"] + record["message"] = ( + f"another async merge request is pending for this pull request " + f"({request['options']}), not this run's vetted merge -- held; it " + "can still merge" + ) + return record status = request["status"] if status not in ASYNC_TERMINAL_STATUSES: polled = poll_async_merge(repo, number, request["uuid"]) @@ -1677,11 +1714,18 @@ def async_merge( record["message"] = polled["message"] or record["message"] record["status"] = status or None if status == "merged": - verified = pull_request_merged(repo, number) + landed = pull_request_landed(repo, number) + verified = landed["merged"] # None (the read failed) is surfaced as unconfirmed, never as a merge. record["verifiedMerged"] = verified - record["success"] = verified is True - if verified is False: + record["mergedHead"] = landed["head"] + record["success"] = verified is True and landed["head"] == sha + if verified is True and landed["head"] != sha: + record["message"] = ( + f"the pull request merged at head {landed['head']}, not the vetted " + f"head {sha} -- escalate to a human" + ) + elif verified is False: record["message"] = ( "the async merge API reported merged but the pull request reads " "unmerged -- not counted as merged" @@ -1779,6 +1823,37 @@ def verify_stack_landed(repo: str, result: dict[str, Any]) -> dict[str, Any]: return {"verified": not mismatches, "mismatches": mismatches} +def verify_request_landed( + repo: str, number: int, entry: dict[str, Any] +) -> dict[str, Any]: + """Whether a recorded request merged every head the gate evaluated: the PR + at its recorded head and, for a stack, each lower layer at its own.""" + landed = pull_request_landed(repo, number) + if landed["merged"] is None: + return { + "verified": None, + "mismatches": [], + "message": "pull request unreadable", + } + head = str(entry.get("head") or "") + mismatches = [] + if not landed["merged"] or landed["head"] != head: + mismatches.append( + { + "pr": f"{repo}#{number}", + "evaluatedHead": head, + "reportedHead": landed["head"], + "merged": landed["merged"], + } + ) + if is_json_object(entry.get("stack")): + stack = verify_stack_landed(repo, entry) + if stack["verified"] is None: + return stack + mismatches += stack["mismatches"] + return {"verified": not mismatches, "mismatches": mismatches} + + PENDING_MERGES_FILE = "merge-requests.json" # GitHub keeps an async merge result for 24 hours after its last update; a # record older than that (plus slack) can no longer be read and is dropped. @@ -1821,28 +1896,44 @@ def check_pending_request( A request left pending stays live on GitHub: it can still merge after a hold appears that would refuse a new one. There is no route to cancel it, so every later run reads it first. A terminal or expired request clears - the record; an unreadable one stays recorded and counts as pending. + the record; an unreadable one stays recorded and counts as pending, and a + corrupt one (an unusable request id) is held however old it is. + + A merged request is checked against every head the gate evaluated + (`verification`), since its `sha` pinned only this PR. A check that cannot + read the heads back keeps the record for the next run. """ key = f"{repo}#{number}" with state_lock(path): entry = _load_pending(path).get(key) if not is_json_object(entry): return None + uuid = str(entry.get("uuid") or "") requested = parse_github_timestamp(str(entry.get("requestedAt") or "")) age = ( ((now or datetime.now(UTC)) - requested).total_seconds() if requested else None ) - if age is not None and age > PENDING_MERGE_MAX_AGE_SECONDS: + if ( + age is not None + and age > PENDING_MERGE_MAX_AGE_SECONDS + and ASYNC_UUID_RE.match(uuid) + ): update_pending(path, key, None) return {**entry, "status": "expired", "message": "older than GitHub retains"} - current = read_async_merge(repo, number, str(entry.get("uuid") or "")) + current = read_async_merge(repo, number, uuid) report = { **entry, "status": current["status"] or None, "message": current["message"], } + if current.get("corrupt"): + report["corrupt"] = True if current["expired"]: report["status"] = "expired" + if current["status"] == "merged": + report["verification"] = verify_request_landed(repo, number, entry) + if report["verification"]["verified"] is None: + return report if current["expired"] or current["status"] in ASYNC_TERMINAL_STATUSES: update_pending(path, key, None) return report @@ -1853,7 +1944,7 @@ def _record_pending( repo: str, number: int, record: dict[str, Any], - pin: str | None, + pin: str, result: dict[str, Any], ) -> None: """Keep a request that is still live on GitHub; forget a finished one.""" @@ -1861,16 +1952,24 @@ def _record_pending( live = ( bool(record.get("uuid")) and record.get("status") not in ASYNC_TERMINAL_STATUSES ) - entry = ( - { + entry: dict[str, Any] | None = None + if live: + entry = { "uuid": record["uuid"], - "head": pin or result.get("headRefOid"), + "head": pin, "mergeAction": record.get("mergeAction"), "requestedAt": datetime.now(UTC).isoformat().replace("+00:00", "Z"), } - if live - else None - ) + stack = json_object(result.get("stack")) + if stack.get("landsLowerLayers"): + # The heads a later run checks the landed layers against. + entry["stack"] = { + "number": stack.get("number"), + "layers": [ + {"number": layer, "headRefOid": head} + for layer, head in _evaluated_layers(result) + ], + } try: update_pending(path, key, entry) except (RuntimeError, OSError) as exc: @@ -2007,8 +2106,9 @@ def main() -> int: "--allow-unpinned-head", action="store_true", help=( - "permit --merge without --expected-head (interactive only); disables " - "the TOCTOU guard that pins the vetted head SHA" + "permit --merge without --expected-head (interactive only); the merge " + "still pins the head this run evaluates, but nothing ties that head " + "to one you vetted" ), ) parser.add_argument( @@ -2281,7 +2381,15 @@ def _refuse(message: str, code: int, **envelope: object) -> int: if prior is not None: result["pendingMergeRequest"] = prior - if prior.get("status") in (None, "pending"): + verified = json_object(prior.get("verification")).get("verified", True) + hold = reason = None + if prior.get("corrupt"): + hold = ( + f"the recorded async merge request for this PR is corrupt " + f"({prior.get('message')}) -- merge pending until a human " + "inspects the record; no new request is sent" + ) + elif prior.get("status") in (None, "pending"): # Live (or unreadable) on GitHub: it can still merge whatever this run # found, so no verdict here may read as settled and no new request goes. hold = ( @@ -2290,11 +2398,24 @@ def _refuse(message: str, code: int, **envelope: object) -> int: "still merge regardless of this verdict -- merge pending; no new " "request is sent" ) + elif verified is not True: + reason = hold = ( + f"the async merge request ({prior.get('uuid')}) merged, but " + + ( + "the heads it landed could not be read back -- unconfirmed; " + "the record is kept and re-checked next run" + if verified is None + else "a head it landed is not the head the gate evaluated -- " + "escalate to a human" + ) + ) + if hold: + if reason is None: + result["action"] = "merge-pending" result["ready"] = False result["blockers"].insert(0, hold) result["autoMerge"] = {"ready": False, "blockers": [hold]} - result["action"] = "merge-pending" - result["merge"] = {"attempted": False, "reason": "merge pending"} + result["merge"] = {"attempted": False, "reason": reason or "merge pending"} print(json.dumps(result, indent=2)) return 10 @@ -2338,13 +2459,17 @@ def _refuse(message: str, code: int, **envelope: object) -> int: result["mergeMethod"] = method # Atomic head pin: GitHub refuses the merge unless the head still equals the - # exact full SHA we vetted, closing the preflight-to-merge TOCTOU window. - vetted_head = result.get("headRefOid") - pin = ( - vetted_head - if args.expected_head and isinstance(vetted_head, str) and vetted_head - else None - ) + # exact full SHA the gate just evaluated, closing the preflight-to-merge + # TOCTOU window. `--allow-unpinned-head` waives only the `--expected-head` + # argument, never this pin. + pin = result.get("headRefOid") + if not isinstance(pin, str) or not pin: + reason = "the gate read no head SHA to pin the merge to -- held" + result["ready"] = False + result["blockers"].append(reason) + result["merge"] = {"attempted": False, "reason": reason} + print(json.dumps(result, indent=2)) + return 10 # A ready PR merges through the async merge API: REST (so it works where # GraphQL is refused), and the only API that enqueues or lands a stack. It is @@ -2381,7 +2506,12 @@ def _refuse(message: str, code: int, **envelope: object) -> int: "or stack merge has no other API -- held" ) result["merge"] = record - result["merged"] = record["success"] and record["status"] == "merged" + # A merge read back at another head is still a merge, reported + # with exit 10 for a human, like a stack layer landing elsewhere. + result["merged"] = ( + record["status"] == "merged" + and record.get("verifiedMerged") is True + ) result["enqueued"] = ( record["success"] and record["status"] == "enqueued" ) @@ -2411,8 +2541,7 @@ def _refuse(message: str, code: int, **envelope: object) -> int: merge_cmd = ["pr", "merge", str(number), "-R", repo, f"--{method}"] if arm_auto: merge_cmd.append("--auto") - if pin: - merge_cmd += ["--match-head-commit", pin] + merge_cmd += ["--match-head-commit", pin] proc = gh_capture(merge_cmd) result["merge"] = { "attempted": True, diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py index 8f508ac5fb..ae8a444ee3 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py @@ -1361,7 +1361,11 @@ def capture(cmd: list[str]) -> Any: mock.patch.object(merge, "allowed_method", return_value="squash"), mock.patch.object(merge, "gh_capture", side_effect=capture), mock.patch.object(merge, "repository_default_branch", return_value="main"), - mock.patch.object(merge, "pull_request_merged", return_value=True), + mock.patch.object( + merge, + "pull_request_landed", + return_value={"merged": True, "head": HEAD}, + ), contextlib.redirect_stdout(io.StringIO()) as out, ): code = merge.main() diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py index 8bfd6b549d..4771ff0ade 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py @@ -62,20 +62,23 @@ def _run( polls: list[dict[str, Any]] | None = None, *, merged: bool | None = True, + landed_head: str = HEAD, base: str = "main", default_branch: str | None = "main", merge_action: str = "direct_merge", stack_lands: bool = False, - stack_listings: list[list[dict[str, Any]]] | None = None, + stack_listings: list[list[dict[str, Any]] | Exception] | None = None, extra: tuple[str, ...] = (), clock: list[float] | None = None, ready: bool = True, merge_flag: bool = True, + pinned: bool = True, + head: str | None = HEAD, ) -> int: verdict = { "ready": ready, "blockers": [] if ready else ["1 unresolved review thread(s) [reviewer]"], - "headRefOid": HEAD, + "headRefOid": head, "baseRef": base, "mergeAction": merge_action, "stack": { @@ -110,7 +113,10 @@ def gh_json(args: list[str]) -> Any: return answer if args[1] == "repos/owner/repo/stacks/7": self.stack_reads += 1 - return {"number": 7, "pull_requests": listings.pop(0)} + listing = listings.pop(0) + if isinstance(listing, Exception): + raise listing + return {"number": 7, "pull_requests": listing} raise AssertionError(f"unexpected gh_json call: {args}") self.stack_reads = 0 @@ -120,7 +126,14 @@ def gh_json(args: list[str]) -> Any: "owner/repo#1", "--allowed-owners", "owner", - *(("--merge", "--expected-head", HEAD) if merge_flag else ()), + *(("--merge",) if merge_flag else ()), + *( + ("--expected-head", HEAD) + if merge_flag and pinned + else ("--allow-unpinned-head",) + if merge_flag + else () + ), *extra, ] out = io.StringIO() @@ -133,7 +146,14 @@ def gh_json(args: list[str]) -> Any: mock.patch.object( merge, "repository_default_branch", return_value=default_branch ), - mock.patch.object(merge, "pull_request_merged", return_value=merged), + mock.patch.object( + merge, + "pull_request_landed", + return_value={ + "merged": merged, + "head": None if merged is None else landed_head, + }, + ), mock.patch.object(merge, "_poll_sleep", side_effect=self.sleeps.append), mock.patch.object(merge, "_poll_clock", side_effect=lambda: next(ticks)), contextlib.redirect_stdout(out), @@ -194,6 +214,17 @@ def test_conflict_polls_the_pending_request_it_names(self) -> None: self.assertEqual(self.output["merge"]["httpStatus"], 409) self.assertTrue(self.polls[0][1].endswith(UUID)) + def test_already_merged_answer_at_another_head_is_not_a_success(self) -> None: + code = self._run( + _proc({"status": "merged", "details": {"message": "m", "sha": "d" * 40}}), + landed_head="f" * 40, + ) + self.assertEqual(code, 10) + self.assertTrue(self.output["merged"]) + self.assertFalse(self.output["merge"]["success"]) + self.assertEqual(self.output["merge"]["mergedHead"], "f" * 40) + self.assertIn("escalate", self.output["merge"]["message"]) + def test_failed_request_is_not_a_merge(self) -> None: code = self._run(_proc(_async("pending")), [_async("failed", "head moved")]) self.assertEqual(code, 10) @@ -270,6 +301,123 @@ def test_unreadable_default_branch_keeps_gh_pr_merge(self) -> None: self.assertEqual(self.captures[0][:2], ["pr", "merge"]) +def _conflict(body: dict[str, Any]) -> mock.Mock: + return _proc(body, returncode=1, stderr="gh: Conflict (HTTP 409)") + + +def _pending_with_options( + head: str = HEAD, action: str = "direct_merge" +) -> dict[str, Any]: + """A 409 body naming the pending request's options, per GitHub's pending + `details` schema (`expected_head_sha`, `merge_action`).""" + details = {"message": "", "uuid": UUID} + details |= {"expected_head_sha": head, "merge_action": action} + return {"status": "pending", "details": details} + + +class ConflictAdoptsOnlyTheVettedRequest(AsyncMergeHarness): + """A 409 names a request this run did not send; it is reported as this + run's merge only when it is pinned to the vetted head and action, or the + PR is read back merged at the vetted head.""" + + def test_a_pending_request_for_another_head_is_not_adopted(self) -> None: + code = self._run(_conflict(_pending_with_options(head="f" * 40))) + self.assertEqual(code, 10) + self.assertFalse(self.output["merged"]) + self.assertFalse(self.output["merge"]["success"]) + self.assertEqual(self.polls, []) + self.assertEqual( + self.output["merge"]["conflictingRequest"]["expected_head_sha"], "f" * 40 + ) + + def test_a_pending_request_for_another_action_is_not_adopted(self) -> None: + code = self._run(_conflict(_pending_with_options(action="merge_queue"))) + self.assertEqual(code, 10) + self.assertFalse(self.output["merge"]["success"]) + self.assertEqual(self.polls, []) + + def test_a_request_pinned_to_the_vetted_head_is_polled_to_merged(self) -> None: + code = self._run(_conflict(_pending_with_options()), [_async("merged")]) + self.assertEqual(code, 0) + self.assertTrue(self.output["merged"]) + + def test_an_unpinned_request_that_merged_another_head_is_not_a_success( + self, + ) -> None: + code = self._run( + _conflict(_async("pending")), [_async("merged")], landed_head="f" * 40 + ) + self.assertEqual(code, 10) + self.assertFalse(self.output["merge"]["success"]) + self.assertEqual(self.output["merge"]["mergedHead"], "f" * 40) + + def test_an_unpinned_request_whose_merge_cannot_be_read_is_unconfirmed( + self, + ) -> None: + code = self._run(_conflict(_async("pending")), [_async("merged")], merged=None) + self.assertEqual(code, 10) + self.assertFalse(self.output["merged"]) + self.assertTrue(self.output["mergeUnconfirmed"]) + + def test_a_request_not_adopted_is_still_recorded_as_live(self) -> None: + state = tempfile.TemporaryDirectory() + self.addCleanup(state.cleanup) + self._run( + _conflict(_pending_with_options(head="f" * 40)), + extra=("--state-dir", state.name), + ) + path = pathlib.Path(state.name) / merge.PENDING_MERGES_FILE + records = json.loads(path.read_text(encoding="utf-8"))["requests"] + self.assertEqual(records["owner/repo#1"]["uuid"], UUID) + + +class PullRequestReadBack(unittest.TestCase): + def _landed(self, payload: Any) -> dict[str, Any]: + with mock.patch.object(merge, "gh_json", return_value=payload): + return merge.pull_request_landed("owner/repo", 1) + + def test_a_merged_pull_request_reports_its_head(self) -> None: + self.assertEqual( + self._landed({"merged": True, "head": HEAD}), + {"merged": True, "head": HEAD}, + ) + + def test_a_merge_with_no_readable_head_is_unconfirmed(self) -> None: + self.assertEqual( + self._landed({"merged": True, "head": None}), + {"merged": None, "head": None}, + ) + + def test_a_failed_read_is_unconfirmed(self) -> None: + with mock.patch.object( + merge, "gh_json", side_effect=RuntimeError("gh: (HTTP 502)") + ): + landed = merge.pull_request_landed("owner/repo", 1) + self.assertEqual(landed, {"merged": None, "head": None}) + + +class UnpinnedMergeStillPinsTheEvaluatedHead(AsyncMergeHarness): + """`--allow-unpinned-head` waives only the `--expected-head` argument: the + request still pins the head the gate just evaluated.""" + + def test_async_request_carries_the_evaluated_head(self) -> None: + code = self._run(_proc(_async("merged")), pinned=False) + self.assertEqual(code, 0) + self.assertEqual(self._fields(self._put())["sha"], HEAD) + + def test_gh_pr_merge_carries_the_evaluated_head(self) -> None: + code = self._run(_proc(), base="release", pinned=False) + self.assertEqual(code, 0) + [legacy] = self.captures + self.assertEqual(legacy[legacy.index("--match-head-commit") + 1], HEAD) + + def test_no_evaluated_head_sends_no_request(self) -> None: + code = self._run(_proc(_async("merged")), pinned=False, head=None) + self.assertEqual(code, 10) + self.assertFalse(self.output["merge"]["attempted"]) + self.assertEqual(self.captures, []) + + class MergeQueueIsEnqueued(AsyncMergeHarness): def test_queue_branch_enqueues_without_a_merge_method(self) -> None: code = self._run( @@ -456,6 +604,111 @@ def test_an_expired_request_clears_the_record(self) -> None: self.assertEqual(self.output["pendingMergeRequest"]["status"], "expired") self.assertEqual(self._records(), {}) + def test_a_record_with_an_unusable_request_id_is_held_and_never_read( + self, + ) -> None: + path = pathlib.Path(self.state.name) / merge.PENDING_MERGES_FILE + entry = { + "uuid": "../../../user", + "head": HEAD, + "mergeAction": "direct_merge", + "requestedAt": merge.datetime.now(merge.UTC).isoformat(), + } + merge.write_state( + path, {"schema_version": 1, "requests": {"owner/repo#1": entry}} + ) + code = self._run(_proc(), [], extra=("--state-dir", self.state.name)) + self.assertEqual((code, self.output["action"]), (10, "merge-pending")) + self.assertIn("corrupt", self.output["blockers"][0]) + self.assertEqual(self.polls, []) + self.assertEqual(self.captures, []) + self.assertIn("owner/repo#1", self._records()) + + +class PendingRequestIsVerifiedWhenItLands(AsyncMergeHarness): + """The request's `sha` pins only the top PR, so a request that finishes in + a later run is checked against every head the gate evaluated.""" + + def setUp(self) -> None: + super().setUp() + self.state = tempfile.TemporaryDirectory() + self.addCleanup(self.state.cleanup) + + def _leave_pending(self, *, stack: bool = True) -> None: + code = self._run( + _proc(_async("pending")), + [_async("pending")], + clock=[0.0, merge.ASYNC_MERGE_POLL_TIMEOUT_SECONDS + 1], + base="feat/b" if stack else "main", + stack_lands=stack, + stack_listings=[AS_EVALUATED] if stack else None, + extra=("--state-dir", self.state.name) + + (("--stacked-prs",) if stack else ()), + ) + self.assertEqual((code, self.output["merge"]["status"]), (10, "pending")) + + def _later_run(self, listing: Any = None, **kwargs: Any) -> int: + self.captures.clear() + return self._run( + _proc(), + [_async("merged")], + merge_flag=False, + stack_listings=None if listing is None else [listing], + extra=("--state-dir", self.state.name), + **kwargs, + ) + + def _records(self) -> dict[str, Any]: + path = pathlib.Path(self.state.name) / merge.PENDING_MERGES_FILE + return json.loads(path.read_text(encoding="utf-8"))["requests"] + + def test_the_record_keeps_the_evaluated_lower_layers(self) -> None: + self._leave_pending() + stack = self._records()["owner/repo#1"]["stack"] + self.assertEqual(stack["number"], 7) + self.assertEqual( + [(layer["number"], layer["headRefOid"]) for layer in stack["layers"]], + [(2, LAYER2)], + ) + + def test_a_layer_that_landed_at_another_head_escalates(self) -> None: + self._leave_pending() + other = [_listed(2, "e" * 40, merged=True), _listed(1, HEAD, merged=True)] + code = self._later_run(other) + self.assertEqual(code, 10) + self.assertIn("escalate", self.output["blockers"][0]) + verification = self.output["pendingMergeRequest"]["verification"] + self.assertFalse(verification["verified"]) + [mismatch] = verification["mismatches"] + self.assertEqual( + (mismatch["pr"], mismatch["evaluatedHead"], mismatch["reportedHead"]), + ("owner/repo#2", LAYER2, "e" * 40), + ) + + def test_a_stack_that_landed_as_evaluated_clears_the_record(self) -> None: + self._leave_pending() + code = self._later_run(LANDED) + self.assertEqual(code, 0) + self.assertTrue(self.output["pendingMergeRequest"]["verification"]["verified"]) + self.assertEqual(self._records(), {}) + + def test_an_unreadable_stack_keeps_the_record_and_holds(self) -> None: + self._leave_pending() + code = self._later_run(RuntimeError("gh: Server Error (HTTP 502)")) + self.assertEqual(code, 10) + self.assertIsNone( + self.output["pendingMergeRequest"]["verification"]["verified"] + ) + self.assertIn("owner/repo#1", self._records()) + + def test_a_top_pull_request_that_merged_another_head_escalates(self) -> None: + self._leave_pending(stack=False) + code = self._later_run(landed_head="f" * 40) + self.assertEqual(code, 10) + self.assertIn("escalate", self.output["blockers"][0]) + [mismatch] = self.output["pendingMergeRequest"]["verification"]["mismatches"] + self.assertEqual(mismatch["reportedHead"], "f" * 40) + class GateEvaluation(unittest.TestCase): """`evaluate` over a three-layer native stack: #1 (base main, head feat/a), diff --git a/plugins/source-control/skills/pull-request/reference/stacks.md b/plugins/source-control/skills/pull-request/reference/stacks.md index 4be07571a3..901fc41fd3 100644 --- a/plugins/source-control/skills/pull-request/reference/stacks.md +++ b/plugins/source-control/skills/pull-request/reference/stacks.md @@ -13,6 +13,15 @@ stack layer: merging it lands that PR alone, into that branch. - Put each dependency in the same layer or a lower one, never a higher one. - Each layer is a pull request in its own right: open it as a draft, take it through `ready` and `monitor`, and hold its title and body to the same contract as any other PR. +- Inside a linked worktree (`/source-control:worktree`), use extension v0.2.0 or later with Git + 2.36 or later. Earlier versions track a stack per directory, so a stack built in one worktree is + invisible from another. Unattended lanes set `GH_STACK_NO_UPDATE_NOTIFIER=1` to keep its upgrade + notice out of their output. + +**Claim, basis, as of, recheck:** repository-scoped stack tracking, cross-worktree `rebase`, +`sync` and `modify`, the Git floor, and the notifier switch, +[gh-stack v0.2.0 release notes](https://github.com/github/gh-stack/releases/tag/v0.2.0); +2026-10-02. Recheck on the extension's next minor release. ## Merge From 3645308ce4b4621af97b3d5140e9395326a1056c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:43:44 -0400 Subject: [PATCH 06/10] fix(disk-hygiene): redact secrets cut by the guard-log scan bound, and restore long-header JWT refusal - The guard log's 4096-character scan bound let a PEM key or JWT that starts in view, but ends past the bound, reach the record unredacted. On the truncated branch, a key header with no END line, or a token run holding eyJ, is now redacted through to the cut. - machine-profile's generic JWT rule is unbounded again, with a wider lookbehind so it stays linear. A JWS with a long x5c header is refused, as it was on main. - babysit request ids must match in full, so a trailing newline is corrupt. A test pins the "corrupt record is held however old" branch. Co-Authored-By: Claude Opus 5.5 --- plugins/disk-hygiene/CHANGELOG.md | 3 ++- .../disk-hygiene/lib/guard_decision_log.py | 17 +++++++++++- .../lib/test_guard_decision_log.py | 17 ++++++++++++ plugins/harness-ops/CHANGELOG.md | 6 +++-- .../skills/machine-profile/scripts/profile.sh | 8 +++--- .../machine-profile/scripts/profile.test.sh | 3 +++ .../babysit-prs/scripts/babysit_merge.py | 6 ++--- .../scripts/tests/test_babysit_merge_async.py | 26 +++++++++++++++---- 8 files changed, 71 insertions(+), 15 deletions(-) diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index 91c6249e4d..f830759e83 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -14,7 +14,8 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format fol credential-name rule (`FOO_KEY=...`) backtracked in cubic time, so a 4 KB command of repeated `KEY` took about 40 seconds; it now makes one attempt per name. A value longer than 4096 characters is scanned only to that bound, and the kept text is narrowed by what redaction - removed, so a secret cut at the bound is never shown. + removed, so a secret cut at the bound is never shown. A private key or JWT that starts in the + kept text and runs past the bound is redacted from its start. ## [0.42.9] - 2026-10-02 diff --git a/plugins/disk-hygiene/lib/guard_decision_log.py b/plugins/disk-hygiene/lib/guard_decision_log.py index db1a61ee50..134cd3caa5 100644 --- a/plugins/disk-hygiene/lib/guard_decision_log.py +++ b/plugins/disk-hygiene/lib/guard_decision_log.py @@ -145,6 +145,21 @@ def _redact_secrets(text: str) -> str: return text +# A private key or JWT that runs past the scan bound has no end inside the +# scanned text, so no complete-shape rule above can match it. +_KEY_RUNNING_TO_CUT = re.compile(r"-----BEGIN[^-]+PRIVATE KEY-----.*\Z", re.DOTALL) +_JWT_RUNNING_TO_CUT = re.compile(r"(?:ghs_[0-9]+_)?eyJ") +_TOKEN_CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_.+/=-" + + +def _redact_cut_tail(text: str) -> str: + text = _KEY_RUNNING_TO_CUT.sub(REDACTED, text) + # rstrip finds the run of token characters at the cut in linear time. + run_start = len(text.rstrip(_TOKEN_CHARS)) + jwt = _JWT_RUNNING_TO_CUT.search(text, run_start) + return text[: jwt.start()] + REDACTED if jwt else text + + def _clip(value: object) -> str | None: if value is None: return None @@ -153,7 +168,7 @@ def _clip(value: object) -> str | None: # Scan only a bounded prefix, and narrow the kept window by what # redaction removed, so every kept character comes from the first # MAX_TEXT_CHARS of the input and a secret cut at the bound stays out. - scanned = _redact_secrets(text[:MAX_SCAN_CHARS]) + scanned = _redact_cut_tail(_redact_secrets(text[:MAX_SCAN_CHARS])) window = MAX_TEXT_CHARS - max(0, MAX_SCAN_CHARS - len(scanned)) return scanned[: max(0, window)] + "..." text = _redact_secrets(text) diff --git a/plugins/disk-hygiene/lib/test_guard_decision_log.py b/plugins/disk-hygiene/lib/test_guard_decision_log.py index e3ccda530c..c80627584e 100755 --- a/plugins/disk-hygiene/lib/test_guard_decision_log.py +++ b/plugins/disk-hygiene/lib/test_guard_decision_log.py @@ -179,6 +179,23 @@ def test_secret_cut_at_the_scan_bound_stays_out_of_the_record(self) -> None: (entry,) = self.read_records() self.assertNotIn("FAKE", entry["command"]) + def test_secret_starting_in_view_and_running_past_the_scan_bound_is_redacted( + self, + ) -> None: + # Each starts in the first 400 characters and ends past the scan bound, + # so no complete-shape rule can match it inside the scanned prefix. + pem_header = "-----BEGIN " + "RSA PRIVATE KEY-----" + for label, secret in ( + ("pem", pem_header + "\nMIIJFAKEbody" + "Q" * 6000), + ("jwt", "eyJ" + "FAKEheader" + "." + "FAKEpayload" + "Q" * 6000), + ("ghs", "ghs" + "_1234567_eyJFAKE.FAKEpayload" + "Q" * 6000), + ): + with self.subTest(label): + record = decision_log.build_record( + hook="h", decision="deny", rule="r", command="printf %s " + secret + ) + self.assertNotIn("FAKE", record["command"]) + def test_none_and_deny_by_default_persist_length_not_command_text(self) -> None: secret = "$env:AZURE_CLIENT_SECRET='s3cretvalue'; Get-Process" self.write_one( diff --git a/plugins/harness-ops/CHANGELOG.md b/plugins/harness-ops/CHANGELOG.md index c30d6c0864..66c690558c 100644 --- a/plugins/harness-ops/CHANGELOG.md +++ b/plugins/harness-ops/CHANGELOG.md @@ -16,8 +16,10 @@ All notable changes to the `harness-ops` plugin are documented here. Format foll `ghs__` format GitHub began issuing on 2026-04-27, matched by its own shape rather than only through the generic JWT rule. - `machine-profile` validates a long record value in linear time. jq's regex engine backtracks, - and the generic JWT rule took about 10 seconds on a 300 KB value of repeated `ghs_1_eyJ`; its - header segment is now capped at 512 characters, as is the `ghs_` rule's. + and the generic JWT rule took about 10 seconds on a 300 KB value of repeated `ghs_1_eyJ`. It now + starts only where a run of token characters starts, so its header stays unbounded and a JWS + with a long certificate-chain header is still refused; the `ghs_` rule's header is capped at + 512 characters. ## [2.5.0] - 2026-10-02 diff --git a/plugins/harness-ops/skills/machine-profile/scripts/profile.sh b/plugins/harness-ops/skills/machine-profile/scripts/profile.sh index 125dfe44d3..e5aa86f16f 100755 --- a/plugins/harness-ops/skills/machine-profile/scripts/profile.sh +++ b/plugins/harness-ops/skills/machine-profile/scripts/profile.sh @@ -30,9 +30,11 @@ command -v jq >/dev/null 2>&1 || die "jq is required" CRED_RE='token|secret|passw(?:or)?d|credential|api.?key|private.?key' # Value shapes the validator refuses in any string of a record: GitHub, AWS, Slack and # sk- API tokens, JWTs, private-key blocks, and a password embedded in a URL. -# jq's regex engine backtracks: a JWT header is bounded and its segments spelled out -# (a counted group is far slower), or a repeated `ghs_1_eyJ-` takes quadratic time. -VAL_RE='ghs_[0-9]+_eyJ[A-Za-z0-9_-]{0,512}\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+|gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|(? None: @@ -1608,7 +1608,7 @@ def read_async_merge(repo: str, number: int, uuid: str) -> dict[str, Any]: A UUID that is not GitHub's shape never reaches the API path: it reads as `corrupt`, which keeps a recorded request held. """ - if not ASYNC_UUID_RE.match(uuid): + if not ASYNC_UUID_RE.fullmatch(uuid): return { "status": "", "message": f"unusable async merge request id {uuid!r}", @@ -1916,7 +1916,7 @@ def check_pending_request( if ( age is not None and age > PENDING_MERGE_MAX_AGE_SECONDS - and ASYNC_UUID_RE.match(uuid) + and ASYNC_UUID_RE.fullmatch(uuid) ): update_pending(path, key, None) return {**entry, "status": "expired", "message": "older than GitHub retains"} diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py index 4771ff0ade..3e9554bcb6 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py @@ -18,6 +18,7 @@ import sys import tempfile import unittest +from datetime import timedelta from typing import Any from unittest import mock @@ -604,19 +605,34 @@ def test_an_expired_request_clears_the_record(self) -> None: self.assertEqual(self.output["pendingMergeRequest"]["status"], "expired") self.assertEqual(self._records(), {}) - def test_a_record_with_an_unusable_request_id_is_held_and_never_read( - self, - ) -> None: + def _corrupt_record(self, uuid: str, *, age_hours: float = 0) -> None: path = pathlib.Path(self.state.name) / merge.PENDING_MERGES_FILE + requested = merge.datetime.now(merge.UTC) - timedelta(hours=age_hours) entry = { - "uuid": "../../../user", + "uuid": uuid, "head": HEAD, "mergeAction": "direct_merge", - "requestedAt": merge.datetime.now(merge.UTC).isoformat(), + "requestedAt": requested.isoformat(), } merge.write_state( path, {"schema_version": 1, "requests": {"owner/repo#1": entry}} ) + + def test_a_record_with_an_unusable_request_id_is_held_and_never_read( + self, + ) -> None: + self._corrupt_record("../../../user") + self._assert_corrupt_hold() + + def test_a_corrupt_record_is_held_however_old(self) -> None: + self._corrupt_record("../../../user", age_hours=48) + self._assert_corrupt_hold() + + def test_an_id_with_a_trailing_newline_is_corrupt(self) -> None: + self._corrupt_record(UUID + "\n") + self._assert_corrupt_hold() + + def _assert_corrupt_hold(self) -> None: code = self._run(_proc(), [], extra=("--state-dir", self.state.name)) self.assertEqual((code, self.output["action"]), (10, "merge-pending")) self.assertIn("corrupt", self.output["blockers"][0]) From 24da46c79a77af9dc562f6ffe0b54fb34cde6ca6 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 20:01:00 -0400 Subject: [PATCH 07/10] fix(autonomy): keep the runner lifecycle reference vendor-neutral The autonomy reference/ contracts name surface classes, never vendors, so the GitHub-specific merge-queue pointer is removed. The binding is documented where the GitHub implementer lives, in source-control's babysit safety.md. This also restores main's 0.25.7 changelog entry, which an earlier merge had overwritten. Co-Authored-By: Claude Opus 5.5 --- plugins/autonomy/CHANGELOG.md | 5 +---- plugins/autonomy/reference/runner/lifecycle.md | 12 ------------ 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/plugins/autonomy/CHANGELOG.md b/plugins/autonomy/CHANGELOG.md index 75e77612d5..01f8988528 100644 --- a/plugins/autonomy/CHANGELOG.md +++ b/plugins/autonomy/CHANGELOG.md @@ -7,10 +7,7 @@ All notable changes to the `autonomy` plugin are documented here. Format follows ### Changed -- The runner lifecycle's deferred merge-serialization growth stage names GitHub's native binding: - the merge queue, entered through the asynchronous merge endpoint with `merge_action=merge_queue`. It - is a dated pointer record with a recheck trigger; the stage stays deferred until its evidence - trigger fires. +- The shared hook helper's posture comment no longer names a fixed member count. ## [0.25.6] - 2026-10-02 diff --git a/plugins/autonomy/reference/runner/lifecycle.md b/plugins/autonomy/reference/runner/lifecycle.md index bcaef895a8..e790fdb556 100644 --- a/plugins/autonomy/reference/runner/lifecycle.md +++ b/plugins/autonomy/reference/runner/lifecycle.md @@ -79,15 +79,3 @@ native flow. When that evidence arrives, the runner serializes gated merges by b platform-native merge-queue facility where one exists, never a reimplemented queue. That facility's availability is verified at binding time; absent one, the growth stage stays deferred rather than reimplementing a built-in. No serialization ships at launch. - -**GitHub's native binding, dated record.** *Claim:* on GitHub, the facility to bind is the -repository merge queue, entered through the asynchronous merge endpoint with -`merge_action=merge_queue`, so the runner submits each gated merge there and lets the queue -serialize it. -*Basis:* the "Merge a pull request asynchronously" section of the REST pull-request reference, -, -and the general-availability announcement, -. -*As of:* 2026-10-02. *Recheck trigger:* that section drops or renames the `merge_queue` value, -the REST API version it is documented under is retired, or a changelog entry changes -the endpoint's status. From 1638315413aaeb2e066965fc09236dbe2cd4db89 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 20:09:30 -0400 Subject: [PATCH 08/10] fix: address review-lane findings on async merge and morning-brief source-control babysit: - A pending request that later lands verified is reported as merged (exit 0), not as a closed PR. - Every native stack member merges through the async API, which GitHub requires for stacked PRs, with no gh pr merge fallback. - A pending record expires only when GitHub stops returning it (404), never from a local age. harness-ops morning-brief: - The per-PR compare reads share the --pr-limit cap. PRs past it are marked unverified, with a PARTIAL line. - The skill body states our rule and points at upstream with a dated record, instead of restating GitHub behavior. Co-Authored-By: Claude Opus 5.5 --- plugins/harness-ops/CHANGELOG.md | 9 ++- .../harness-ops/skills/morning-brief/SKILL.md | 27 ++++--- .../morning-brief/morning-brief.test.sh | 15 ++++ .../morning-brief/scripts/morning-brief.sh | 29 ++++--- plugins/source-control/CHANGELOG.md | 12 ++- .../skills/babysit-prs/reference/safety.md | 23 +++--- .../babysit-prs/scripts/babysit_merge.py | 65 ++++++++-------- .../scripts/tests/test_babysit_merge_async.py | 78 ++++++++++++++++++- 8 files changed, 182 insertions(+), 76 deletions(-) diff --git a/plugins/harness-ops/CHANGELOG.md b/plugins/harness-ops/CHANGELOG.md index cc278a0b65..f78a49b346 100644 --- a/plugins/harness-ops/CHANGELOG.md +++ b/plugins/harness-ops/CHANGELOG.md @@ -8,10 +8,11 @@ All notable changes to the `harness-ops` plugin are documented here. Format foll ### Fixed - `morning-brief` no longer presents every `CLEAN` pull request as merge-ready without - qualification. `CLEAN` can describe checks that ran against an older base, so the script reads - the compare endpoint once per clean PR and prints an `UNVERIFIED` line under any PR whose head - is behind its base, or whose comparison could not be read. A `--behind-json` fixture flag - feeds those counts to the tests. + qualification. It counts a clean PR as verified only when its head contains the base tip: + the script reads the compare endpoint once per clean PR, up to `--pr-limit`, and prints an + `UNVERIFIED` line under any PR whose head is behind its base, whose comparison could not be + read, or that fell past the cap, plus a `PARTIAL` line when the cap was hit. A `--behind-json` + fixture flag feeds those counts to the tests. - `machine-profile` refuses a record value holding a GitHub App installation token in the `ghs__` format GitHub began issuing on 2026-04-27, matched by its own shape rather than only through the generic JWT rule. diff --git a/plugins/harness-ops/skills/morning-brief/SKILL.md b/plugins/harness-ops/skills/morning-brief/SKILL.md index 842b09652b..015bddfce4 100644 --- a/plugins/harness-ops/skills/morning-brief/SKILL.md +++ b/plugins/harness-ops/skills/morning-brief/SKILL.md @@ -70,17 +70,20 @@ saying so when capped or when GitHub has not finished computing a PR's mergeabil and the stranded-findings section renders `UNREADABLE`, because review threads have no REST read. That section never renders an all-clear it did not read. -Three upstream facts the script restates, each with its verification record: - -- **`CLEAN` can describe checks that ran against an older base.** The script therefore compares - each clean PR's head with its base branch and prints an `UNVERIFIED` line when the head is - behind, or when the comparison could not be read; a head that contains the base tip prints - nothing extra. Basis: GitHub's 2026-02-19 changelog - , - which lists when a test merge commit is regenerated, and the `behind_by` field of - . As of 2026-10-02. - Recheck when GitHub changes when it regenerates test merge commits, or `mergeStateStatus` - gains a value for a merge state tested against an older base. +The brief treats a clean PR as verified only when its head contains its base branch's tip. It +reads one comparison per clean PR, up to `--pr-limit`, and prints an `UNVERIFIED` line under a PR +whose head is behind its base, whose comparison failed, or that fell past the cap; a capped read +also prints `PARTIAL`. + +- **Pointer**: for what `behind_by` counts, see + . For when GitHub + regenerates a PR's test merge commit, no docs page covers it as of the date; correlate with + . +- **As of**: 2026-10-02 +- **Recheck trigger**: a docs page starts to cover test merge commit regeneration, or the + `mergeStateStatus` enum gains or drops a value. + +Two upstream facts the script restates, each with its verification record: - **The refusal shape the script keys the transport switch on.** Basis: the body `gh api graphql` returned in a Claude Code cloud session, `{"message":"This GraphQL @@ -106,7 +109,7 @@ Three upstream facts the script restates, each with its verification record: | Section | Source | Notes | |---|---|---| | Queues | `gh issue list --label ` counts | Defaults to melodic-software queue labels; live runs filter to labels that exist in the repo (pass `--queue-labels` to pin a custom set) | -| Merge-ready PRs | `gh pr list` filtered to non-draft + `mergeStateStatus=CLEAN` (REST: `pulls` list plus one read per PR for `mergeable_state`) | A light glance signal; `reviewDecision` shown but not required (repos without required review leave it empty; the REST path reports `n/a`). One compare read per clean PR; `UNVERIFIED` marks a head behind its base or a comparison that could not be read | +| Merge-ready PRs | `gh pr list` filtered to non-draft + `mergeStateStatus=CLEAN` (REST: `pulls` list plus one read per PR for `mergeable_state`) | A light glance signal; `reviewDecision` shown but not required (repos without required review leave it empty; the REST path reports `n/a`). One compare read per clean PR, capped at `--pr-limit`; `UNVERIFIED` marks a head behind its base, a comparison that could not be read, or a PR past the cap | | Parked decisions | open issues with the decision label (default `status: needs-decision`) | Surfaces each one's RECOMMENDED line, the uppercase marker wins over an incidental lowercase mention; a case-insensitive fallback catches lowercase markers; pass `--decision-label` to pin | | Lane telemetry | the loop-lane telemetry issue's per-lane comments | Each lane's `last-cycle` age (marked `STALE` past `--stale-hours`, default 6) and any `flags:` | | Stranded findings | merged PRs whose unresolved review threads were **created after the merge** | One line per PR at its worst severity, with a finding count; window is `--stranded-days`, default 3 | diff --git a/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh b/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh index 5973e6141f..ebb43a569c 100755 --- a/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh +++ b/plugins/harness-ops/skills/morning-brief/morning-brief.test.sh @@ -301,6 +301,21 @@ OUT_PARTIAL_BEHIND="$(bash "$BRIEF" --now "$NOW" --pr-json "$TMP/pr.json" --behi assert_contains "merge-ready with no count for a PR says freshness is unread" "$(section "$OUT_PARTIAL_BEHIND" "== Merge-ready")" \ " UNVERIFIED: base freshness unread: no behind count for #13" +# The compare read costs one GET per clean PR, so it shares the --pr-limit cap; +# a clean PR past the cap is UNVERIFIED with the cap as its reason. +OUT_FRESH_CAP="$(bash "$BRIEF" --now "$NOW" --pr-json "$TMP/pr.json" --behind-json "$TMP/behind.json" --pr-limit 1 \ + --counts-json "$TMP/counts.json" --decisions-json "$TMP/decisions.json" \ + --telemetry-json "$TMP/telemetry.json" --merged-json "$TMP/merged.json" 2>&1)" +FRESH_CAP="$(section "$OUT_FRESH_CAP" "== Merge-ready")" +assert_contains "merge-ready reads freshness for clean PRs inside the cap" "$FRESH_CAP" \ + " http://x/10 review=none + #13 approved clean pr" +assert_contains "merge-ready marks a clean PR past the compare cap unverified" "$FRESH_CAP" \ + " http://x/13 review=APPROVED + UNVERIFIED: base freshness unread: compare reads capped at 1 (raise --pr-limit)" +assert_contains "merge-ready reports a capped compare read as PARTIAL" "$FRESH_CAP" \ + "PARTIAL: 2 clean PRs; base freshness read for the first 1 only (raise --pr-limit)" + # Decisions — two-tier RECOMMENDED extraction assert_contains "decision #100 uppercase marker wins" "$OUT" "Store the root in a userConfig key" assert_not_contains "decision #100 ignores 'not recommended'" "$OUT" "not recommended for this profile" diff --git a/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh b/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh index 2c88cc7f5f..4ab003d1d9 100755 --- a/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh +++ b/plugins/harness-ops/skills/morning-brief/scripts/morning-brief.sh @@ -10,9 +10,8 @@ # It runs `gh` read queries only. The authoritative merge gate lives in the # source-control:babysit-prs skill; the merge-ready list here is a lighter # gh-native signal (mergeStateStatus CLEAN + non-draft) meant for a 5-second -# glance, not a substitute for that skill's classification. CLEAN reports the -# checks GitHub last ran, which can predate the current base, so a clean PR -# whose head is behind its base (or whose comparison could not be read) carries +# glance, not a substitute for that skill's classification. A clean PR whose +# head does not contain its base tip, or whose comparison was not read, carries # an UNVERIFIED line. # # Owner/repo is derived from `gh repo view`, or from the checkout's `origin` @@ -41,7 +40,7 @@ # morning-brief.sh --stale-hours N age past which a lane is STALE (default 6) # morning-brief.sh --stranded-days N age window for stranded review findings (default 3) # morning-brief.sh --rec-maxlen N truncate RECOMMENDED previews (default 240; 0 = full) -# morning-brief.sh --pr-limit N open PRs whose merge state the REST path checks (default 50) +# morning-brief.sh --pr-limit N cap on per-PR reads: REST merge state, and base freshness of clean PRs (default 50) # morning-brief.sh --help # # Fixture flags (skip the network; used by the test suite and for reuse): @@ -704,12 +703,10 @@ fetch_prs_rest() { reviewDecision: "n/a", baseRefName: .base.ref, headRefOid: .head.sha} ]' "$WORK/prs.detail" >"$out" } -# CLEAN says the checks GitHub last ran passed, not that they ran against the -# current base: without a strict up-to-date rule a PR stays CLEAN while its -# base moves on (SKILL.md carries the upstream record). A head that already -# contains the base tip leaves nothing untested, so the compare endpoint's -# `behind_by` decides: 0 prints nothing; a positive count, or a read that -# failed, prints why the PR's CLEAN is unverified. +# A clean PR counts as verified only when its head contains the base tip +# (SKILL.md carries the upstream record). The compare endpoint's `behind_by` +# decides: 0 prints nothing; a positive count, or a read that failed, prints +# why the PR's CLEAN is unverified. # # freshness_note NUMBER BASE HEAD_SHA freshness_note() { @@ -748,12 +745,20 @@ print_merge_ready() { } fi local clean='[ .[] | select(.isDraft == false and .mergeStateStatus == "CLEAN") ] | sort_by(.number) | .[]' - local notes='{}' number base sha note ready + local notes='{}' number base sha note ready read=0 + # One compare GET per clean PR, so the reads share the --pr-limit cap; a PR + # past it is UNVERIFIED, and the section says the read was partial. # stdin from /dev/null so gh cannot drain the loop's input. while IFS=$'\t' read -r number base sha; do - note="$(freshness_note "$number" "$base" "$sha" /dev/null) + ((read > PR_LIMIT)) && PR_PARTIAL+=("$read clean PRs; base freshness read for the first $PR_LIMIT only (raise --pr-limit)") ready="$(jq -r --argjson notes "$notes" "$clean"' | " #\(.number) \(.title)\n \(.url) review=\(.reviewDecision // "" | if . == "" then "none" else . end)" + ($notes[.number | tostring] | if . then "\n UNVERIFIED: \(.)" else "" end) diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 757e6c1a99..c1d8f30b98 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -15,7 +15,9 @@ All notable changes to the `source-control` plugin are documented here. Format f the merge every lower layer is checked against the head the gate evaluated, including when the request completes in a later run, and a mismatch is reported with exit `10`. A lower-layer push between the request and its completion still lands unvetted, so the flag assumes only trusted - actors can push to the lower layers. Off, a stack layer is held exactly as before. + actors can push to the lower layers. Every stack member, the bottom layer on any trunk included, + merges through the async merge API, GitHub's required API for a stacked PR, and holds rather than + falling back when the endpoint is missing. Off, a stack layer is held exactly as before. - **The `pull-request` skill has a stacked-PR reference** (`reference/stacks.md`) for creating a stack with the `gh stack` extension and merging it, including the v0.2.0 floor for stacks built in linked worktrees, and `merge.md` shows how to merge through the async merge API with `gh api`. @@ -35,9 +37,11 @@ All notable changes to the `source-control` plugin are documented here. Format f - **A merge request still pending at the bound is recorded under `--state-dir`.** GitHub documents no route to cancel one, so every later run reads it first and, while it is live or unreadable, reports `action: merge-pending` and sends nothing. A request that later finishes merged is checked - against every head the gate evaluated, and a mismatch is escalated with exit `10`. A record with - an unusable request id is held as corrupt, never read or cleared. Merge commands now carry - `--state-dir `. + against every head the gate evaluated: a mismatch is escalated with exit `10`, and a match is + reported as merged (`merged: true`, exit `0`) although the gate now reads a closed PR. A record + clears only when the request finishes or GitHub stops returning it (404), never on a local age + limit. A record with an unusable request id is held as corrupt, never read or cleared. Merge + commands now carry `--state-dir `. - **A default branch that requires a merge queue no longer blocks the gate.** A fully ready PR is enqueued instead (`action: enqueue`, `enqueued: true`), which is reported as queued, not merged. Auto-merge is never armed over a queue, and a queue on any other base is still held. diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 78a6f827c1..fe8443a237 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -661,11 +661,13 @@ A ready PR merges through GitHub's async merge API, called with `gh api` (`gh` h it): a `PUT` to the PR's `merge-async` endpoint, then a `GET` on the request's UUID. - **Which API.** Async when the base is the default branch, when it requires a merge queue, or when - the PR is a native stack layer under `--stacked-prs`. Any other base keeps `gh pr merge`, because - an unconfirmed stack layer there would land the layers below it, and so does every `--auto` arm: - the async API has no auto-merge form. On the default branch a 404 from the endpoint (a host that - lacks it) falls back to `gh pr merge` with the same pin; a queue or a stack has no other API and - holds. + the PR is a native stack member under `--stacked-prs`, the bottom layer on any trunk included: + GitHub documents the async API as the required API for merging a stacked PR. Any other base keeps + `gh pr merge`, and so does every `--auto` arm: the async API has no auto-merge form. Without + `--stacked-prs` the gate never reads stack membership, so a bottom layer on a non-default trunk + goes to `gh pr merge`, which GitHub refuses for a stacked PR; that fails closed. On the default + branch a 404 from the endpoint (a host that lacks it) falls back to `gh pr merge` with the same + pin; a queue or a stack member has no other API and holds. - **Request.** `sha` is always the head the gate just evaluated: `--allow-unpinned-head` waives only the `--expected-head` argument, never the pin, and `gh pr merge` carries the same head as `--match-head-commit`. `merge_method` is sent for a direct merge only, @@ -687,12 +689,15 @@ it): a `PUT` to the PR's `merge-async` endpoint, then a `GET` on the request's U pending, or cannot be read, the run reports `action: merge-pending` with that hold first in `blockers`, exit `10`, and sends nothing. A record whose request id is not GitHub's UUID shape is corrupt: it is never put in an API path or cleared, and holds the same way until a human inspects - it. A finished request (or one past GitHub's 24-hour retention) clears the record and shows as - `pendingMergeRequest`. A request that finished merged is first checked against every head the - gate evaluated, the PR's and, for a stack, each lower layer's, recorded with it + it. A finished request, or one GitHub no longer returns (404; GitHub keeps a result 24 hours + after its latest update), clears the record and shows as `pendingMergeRequest`; the gate keeps no + local age limit of its own. A request that finished merged is first checked against every head + the gate evaluated, the PR's and, for a stack, each lower layer's, recorded with it (`pendingMergeRequest.verification`). A mismatch puts an escalation first in `blockers` with exit `10`; heads that cannot be read back keep the record, hold the same way, and are re-checked next - run. Report a merge-pending PR as "merge may still land", never as held. Without `--state-dir` + run. When every head matches, the run reports the merge even though the gate now reads a closed + PR: `merged: true`, `ready: true`, empty `blockers`, `merge.source: pendingMergeRequest`, exit + `0`, with `action` unchanged. Report a merge-pending PR as "merge may still land", never as held. Without `--state-dir` nothing is recorded and a later run cannot see the request, so `--state-dir ` rides on every merge form. - **Merge queue.** A default-branch base that requires a merge queue is no longer a blocker. Once diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index be27ced14f..876887d322 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -19,9 +19,12 @@ vetted head as `sha`, polled to a terminal status; on a base that requires a merge queue it is enqueued instead (`enqueued` is queued, not merged). A host without the endpoint (404) falls back to `gh pr merge` for a direct merge. - Any other base, and every `--auto` arm, keeps `gh pr merge`. A request still - pending at the poll bound is recorded under `--state-dir` (GitHub offers no - cancel), and every later run reports it as merge pending until it finishes. + Under `--stacked-prs` every stack member, the bottom layer included, merges + through the async API too (GitHub's required API for a stacked PR) and never + falls back. Any other base, and every `--auto` arm, keeps `gh pr merge`. A + request still pending at the poll bound is recorded under `--state-dir` + (GitHub offers no cancel), and every later run reports it as merge pending + until it finishes. - With `--stacked-prs`, a native stack layer is judged against the stack's trunk and every open layer below it runs the same gate, since the async merge lands them together. Without it a stack layer is held as before. @@ -1855,9 +1858,6 @@ def verify_request_landed( PENDING_MERGES_FILE = "merge-requests.json" -# GitHub keeps an async merge result for 24 hours after its last update; a -# record older than that (plus slack) can no longer be read and is dropped. -PENDING_MERGE_MAX_AGE_SECONDS = 25 * 60 * 60 def _load_pending(path: Path) -> dict[str, Any]: @@ -1888,16 +1888,15 @@ def update_pending(path: Path, key: str, entry: dict[str, Any] | None) -> None: write_state(path, {"schema_version": 1, "requests": requests}) -def check_pending_request( - repo: str, number: int, path: Path, *, now: datetime | None = None -) -> dict[str, Any] | None: +def check_pending_request(repo: str, number: int, path: Path) -> dict[str, Any] | None: """The PR's recorded async merge request as GitHub reports it now. A request left pending stays live on GitHub: it can still merge after a hold appears that would refuse a new one. There is no route to cancel it, - so every later run reads it first. A terminal or expired request clears - the record; an unreadable one stays recorded and counts as pending, and a - corrupt one (an unusable request id) is held however old it is. + so every later run reads it first. A terminal request, or one GitHub no + longer returns (404: it keeps a result 24 hours after its latest update), + clears the record; an unreadable one stays recorded and counts as pending, + and a corrupt one (an unusable request id) is held however old it is. A merged request is checked against every head the gate evaluated (`verification`), since its `sha` pinned only this PR. A check that cannot @@ -1908,19 +1907,7 @@ def check_pending_request( entry = _load_pending(path).get(key) if not is_json_object(entry): return None - uuid = str(entry.get("uuid") or "") - requested = parse_github_timestamp(str(entry.get("requestedAt") or "")) - age = ( - ((now or datetime.now(UTC)) - requested).total_seconds() if requested else None - ) - if ( - age is not None - and age > PENDING_MERGE_MAX_AGE_SECONDS - and ASYNC_UUID_RE.fullmatch(uuid) - ): - update_pending(path, key, None) - return {**entry, "status": "expired", "message": "older than GitHub retains"} - current = read_async_merge(repo, number, uuid) + current = read_async_merge(repo, number, str(entry.get("uuid") or "")) report = { **entry, "status": current["status"] or None, @@ -2418,6 +2405,18 @@ def _refuse(message: str, code: int, **envelope: object) -> int: result["merge"] = {"attempted": False, "reason": reason or "merge pending"} print(json.dumps(result, indent=2)) return 10 + if prior.get("status") == "merged": + # The recorded request landed at every evaluated head. The gate now + # reads a merged (closed) PR, but that merge is this run's outcome. + result["merged"] = result["ready"] = True + result["blockers"] = [] + result["merge"] = { + **prior, + "attempted": False, + "source": "pendingMergeRequest", + } + print(json.dumps(result, indent=2)) + return 0 if not args.merge: print(json.dumps(result, indent=2)) @@ -2472,14 +2471,16 @@ def _refuse(message: str, code: int, **envelope: object) -> int: return 10 # A ready PR merges through the async merge API: REST (so it works where - # GraphQL is refused), and the only API that enqueues or lands a stack. It is - # used on the default branch, for a queue, and for a stack; any other base - # keeps `gh pr merge`, because an unconfirmed stack layer there would land - # the layers below it. Auto-merge has no async form and keeps `gh pr merge`. + # GraphQL is refused), the only API that enqueues, and GitHub's required API + # for merging any stacked PR, the bottom layer included. It is used on the + # default branch, for a queue, and for a stack member; any other base keeps + # `gh pr merge`. Auto-merge has no async form and keeps `gh pr merge`. if not arm_auto: - stack_lands = bool(json_object(result.get("stack")).get("landsLowerLayers")) + stack = json_object(result.get("stack")) + stack_lands = bool(stack.get("landsLowerLayers")) + stack_member = bool(stack.get("member")) queue = result.get("mergeAction") == "merge_queue" - use_async = stack_lands or queue + use_async = stack_member or stack_lands or queue if not use_async: default_branch = repository_default_branch(repo) use_async = bool(default_branch) and result.get("baseRef") == default_branch @@ -2499,7 +2500,7 @@ def _refuse(message: str, code: int, **envelope: object) -> int: merge_action="merge_queue" if queue else "direct_merge", method=method, ) - if not record["endpointMissing"] or stack_lands or queue: + if not record["endpointMissing"] or stack_member or stack_lands or queue: if record["endpointMissing"]: record["message"] = ( "the async merge endpoint returned 404 on this host; a queue " diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py index 3e9554bcb6..fe4de8b1bb 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_async.py @@ -75,16 +75,26 @@ def _run( merge_flag: bool = True, pinned: bool = True, head: str | None = HEAD, + blockers: list[str] | None = None, + stack_member: bool = False, ) -> int: + if blockers is not None: + ready = not blockers verdict = { "ready": ready, - "blockers": [] if ready else ["1 unresolved review thread(s) [reviewer]"], + "blockers": ( + blockers + if blockers is not None + else [] + if ready + else ["1 unresolved review thread(s) [reviewer]"] + ), "headRefOid": head, "baseRef": base, "mergeAction": merge_action, "stack": { "enabled": True, - "member": stack_lands, + "member": stack_lands or stack_member, "landsLowerLayers": stack_lands, "number": 7, "layers": ( @@ -516,6 +526,38 @@ def test_a_layer_landing_at_another_head_is_reported(self) -> None: ) +class BottomStackMemberMergesThroughTheAsyncApi(AsyncMergeHarness): + """GitHub documents the async API as the required API for merging a + stacked pull request, so the bottom layer (base == trunk) uses it even on + a trunk that is not the default branch.""" + + def _bottom(self, put: mock.Mock) -> int: + return self._run( + put, + base="release", + default_branch="main", + stack_member=True, + extra=("--stacked-prs",), + ) + + def test_bottom_member_on_a_non_default_trunk_merges_async(self) -> None: + code = self._bottom(_proc(_async("merged"))) + self.assertEqual(code, 0) + self.assertEqual( + len([c for c in self.captures if "merge-async" in " ".join(c)]), 1 + ) + self.assertFalse(any(c[:2] == ["pr", "merge"] for c in self.captures)) + self.assertEqual(self.stack_reads, 0) + + def test_bottom_member_without_the_endpoint_is_held(self) -> None: + missing = _proc( + {"message": "Not Found"}, returncode=1, stderr="gh: Not Found (HTTP 404)" + ) + code = self._bottom(missing) + self.assertEqual(code, 10) + self.assertFalse(any(c[:2] == ["pr", "merge"] for c in self.captures)) + + class PendingRequestOutlivesTheRun(AsyncMergeHarness): """GitHub documents no route to cancel an async merge request, so a request left pending is recorded and every later run reports it.""" @@ -590,10 +632,23 @@ def test_a_finished_request_clears_the_record(self) -> None: merge_flag=False, extra=("--state-dir", self.state.name), ) - self.assertEqual(code, 10) + self.assertEqual((code, self.output["merged"]), (0, True)) self.assertEqual(self.output["pendingMergeRequest"]["status"], "merged") self.assertEqual(self._records(), {}) + def test_a_record_older_than_a_day_is_still_polled(self) -> None: + self._corrupt_record(UUID, age_hours=30) + self._run( + _proc(), + [_async("merged")], + merge_flag=False, + extra=("--state-dir", self.state.name), + ) + self.assertEqual(len(self.polls), 1) + prior = self.output["pendingMergeRequest"] + self.assertEqual(prior["status"], "merged") + self.assertIn("verification", prior) + def test_an_expired_request_clears_the_record(self) -> None: self._leave_pending() self._run( @@ -708,6 +763,23 @@ def test_a_stack_that_landed_as_evaluated_clears_the_record(self) -> None: self.assertTrue(self.output["pendingMergeRequest"]["verification"]["verified"]) self.assertEqual(self._records(), {}) + def test_a_landed_request_reports_merged_even_though_the_gate_sees_a_closed_pr( + self, + ) -> None: + self._leave_pending() + code = self._later_run(LANDED, blockers=["state=MERGED (not OPEN)"]) + self.assertEqual(code, 0) + self.assertTrue(self.output["merged"]) + self.assertTrue(self.output["ready"]) + self.assertEqual(self.output["blockers"], []) + self.assertEqual(self.output["action"], "check") + self.assertEqual( + (self.output["merge"]["attempted"], self.output["merge"]["source"]), + (False, "pendingMergeRequest"), + ) + self.assertEqual(self.output["merge"]["uuid"], UUID) + self.assertEqual(self._records(), {}) + def test_an_unreadable_stack_keeps_the_record_and_holds(self) -> None: self._leave_pending() code = self._later_run(RuntimeError("gh: Server Error (HTTP 502)")) From 01b6e74adc48635d094b7b72333295bfd61b1ae2 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 22:34:26 -0400 Subject: [PATCH 09/10] fix(source-control): restore the babysit_stacked_prs userConfig key lost in a merge A version-collision resolution took main's manifest wholesale and dropped this branch's babysit_stacked_prs key. That broke the plugin-options docs gate, and would have shipped the flag without its setting. Co-Authored-By: Claude Opus 5.5 --- plugins/source-control/.claude-plugin/plugin.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index eee31347d8..97eaaea523 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -119,6 +119,12 @@ "default": "auto", "options": ["auto", "squash", "merge", "rebase"] }, + "babysit_stacked_prs": { + "type": "boolean", + "title": "Babysit stacked PRs", + "description": "Lets the merge gate merge a native stacked PR layer, which lands every open layer below it. Each of those layers must pass the same gate. Off by default: a stack layer is held for a human.", + "default": false + }, "babysit_autopilot_merge_tier": { "type": "boolean", "title": "Babysit autopilot merge tier", From 1808cf4e590301e1ff3115fd79506a50212f7ac8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:12:52 -0400 Subject: [PATCH 10/10] fix: carry the rebumped harness-config and source-control manifest versions --- plugins/harness-config/.claude-plugin/plugin.json | 2 +- plugins/source-control/.claude-plugin/plugin.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/harness-config/.claude-plugin/plugin.json b/plugins/harness-config/.claude-plugin/plugin.json index 14d30c0279..54da477898 100644 --- a/plugins/harness-config/.claude-plugin/plugin.json +++ b/plugins/harness-config/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "harness-config", - "version": "1.5.1", + "version": "1.5.2", "description": "Nine configuration-health skills (plus setup) for a repo's Claude Code configuration: audit (settings.json / .mcp.json / hooks / plugins / permissions drift), audit-automation-gaps (evidence-gated verdicts on automation gaps), audit-permission-grants (allow-rule / allowed-tools grants for auto-mode durability and portability), audit-permission-state (the permission rules actually in effect: every settings scope merged with per-rule provenance, what auto mode drops on entry, config written where nothing reads it, and which managed intents are enforced versus loosenable), draft-auto-mode-rules (interview and draft a paste-ready autoMode classifier block; prints only, never writes), audit-instructions (locally-owned instruction surfaces vs current model capability, proposing removals/rewrites of instructions the model no longer needs, and detecting cross-surface instruction conflicts), audit-prompting-postures (the additive lane: posture guidance the prompting guide says a component's purpose needs but the component does not carry), audit-pass (one coordinated, ordered, resumable pass over a named target: three-scope inventory, run-time-derived exclusion set, stable finding identity, suppression memory, resume, one human gate, delegating every check to the plugin that owns it), and unhobble (the empirical bare-baseline experiment: reversibly strip a repo's standing instructions, log real stumbles against the current model, re-add only what evidence earns). Boundary: harness-memory owns the health of CLAUDE.md, AGENTS.md, CLAUDE.local.md, .claude/rules/ and auto-memory (structure, size, placement, index integrity); harness-config audit-instructions judges whether instruction text across those files and skills, agents and hooks still fits the current model, and runs no memory-file hygiene checks.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 97eaaea523..0f81f6f6d9 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.76.0", + "version": "0.77.0", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-authored-by trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop, safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and standing merge-rung raises binding from the team-tracked layer only, with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply, which interviews the repo and writes the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep, never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software",