diff --git a/.github/scripts/agent-merge-janitor.sh b/.github/scripts/agent-merge-janitor.sh index dca53bb..991aef2 100755 --- a/.github/scripts/agent-merge-janitor.sh +++ b/.github/scripts/agent-merge-janitor.sh @@ -5,8 +5,8 @@ # the low-risk auto-merge lane (task_1785685546659). Complements # dependabot-janitor.sh, which only covers Dependabot PRs on -mcp/node- repos. # -# THIS SCRIPT NEVER MERGES. Per the design's trust-model finding: the -# wyre-agent-fleet App credential (used to post the GO-signal review) is +# THIS SCRIPT DOES NOT MERGE BY DEFAULT. Per the design's trust-model finding: +# the wyre-agent-fleet App credential (used to post the GO-signal review) is # fleet-shared today, not scoped to a distinct reviewer identity — # task_1784224475661 Step 3 (real per-agent GitHub identity) is not yet # built, and mintInstallationToken() has no caller-identity concept at all. @@ -14,8 +14,19 @@ # process with fleet-level GitHub-App-secret access reviewed this at this # exact commit — but not yet a genuine reviewer-vs-author separation # guarantee. Until Aaron closes that gap (Infisical ACL scoping, or Step 3), -# a human keeps the final merge click. This script's job is to do the -# re-verification and produce a trustworthy backlog, nothing more. +# ENABLE_MERGE must stay false and a human keeps the final merge click. +# +# 2026-08-21 (murph, boss-directed, per Aaron's "get shit merged" relayed +# through boss): the merge-execution code path below exists and is +# functional, but is gated behind ENABLE_MERGE (default false) — a SEPARATE +# flag from DRY_RUN, so this can be code-complete and warden-reviewed +# without being live. Do not flip ENABLE_MERGE to true in any persistent +# workflow/cron config until boss confirms Aaron has explicitly answered the +# credential-scoping question (option A or B in +# orgs/wyre/agents/murph/memory/auto-merge-lane-design-2026-08-02.md) — not +# assumed answered by a general "get things merged" directive. This script's +# job absent that answer is still just re-verification + a trustworthy +# backlog. # # GO-signal mechanism: the reviewing agent, after doing real per-PR # diligence (same discipline as the manual Task-1 merges: CI green, @@ -38,24 +49,53 @@ # ORG GitHub org (default: wyre-technology) # REPOS space-separated repo list (default: "cortextos conduit") # DRY_RUN if "true" (default), classify + report only — never touches -# labels or posts comments, even on a failed re-verify. +# labels, comments, or merges, even on a failed re-verify. +# Overrides ENABLE_MERGE unconditionally. +# ENABLE_MERGE if "true" (default "false"), a PR that reaches full pass +# gets actually merged (squash, --match-head-commit) instead of +# only reported as eligible. Has no effect while DRY_RUN=true. +# SEE THE GATING NOTE ABOVE — do not set true in any persistent +# config without boss's explicit confirmation. # BACKLOG_FILE path to write the human-readable summary (default: # agent-merge-backlog.md) # # Requires: gh CLI authenticated (GH_TOKEN) with pull_requests:write on the # repos in scope, to read PR reviews/files and (in live mode) remove a stale -# label + post an explanation comment. Never posts approvals itself — that -# is the reviewing agent's job, done by hand, under their own diligence. +# label, post an explanation comment, or merge. Never posts approvals itself +# — that is the reviewing agent's job, done by hand, under their own +# diligence, before this script ever sees the PR. set -uo pipefail +# Case/whitespace-insensitive truthy check for env-var flags. Warden's review +# of the ENABLE_MERGE addition (2026-08-21): an exact `== "true"` string +# match on DRY_RUN is fine while DRY_RUN only gated labels/comments, but now +# that the same flag gates real merges, a stray "True"/"TRUE"/trailing-space +# value silently falling through to the false branch is a real risk, not a +# cosmetic one — it would leave ENABLE_MERGE's forced-off override +# un-triggered. Applied to both flags for consistency, not just the one that +# was flagged. +is_true() { + local v + v="$(tr '[:upper:]' '[:lower:]' <<<"${1:-}" | tr -d '[:space:]')" + [[ "$v" == "true" ]] +} + ORG="${ORG:-wyre-technology}" REPOS="${REPOS:-cortextos conduit}" DRY_RUN="${DRY_RUN:-true}" +ENABLE_MERGE="${ENABLE_MERGE:-false}" BACKLOG_FILE="${BACKLOG_FILE:-agent-merge-backlog.md}" LABEL="auto-merge-ready" +# DRY_RUN is the master safety switch: it must fully disable merging +# regardless of how ENABLE_MERGE is set, so a config that sets both isn't +# ambiguous about which one wins. +if is_true "$DRY_RUN"; then + ENABLE_MERGE="false" +fi + work="$(mktemp -d)" -for cat in eligible excluded no_go stale_go red pending conflicts no_ci errors; do : > "$work/$cat"; done +for cat in eligible merged merge_failed excluded no_go stale_go red pending conflicts no_ci errors; do : > "$work/$cat"; done # --------------------------------------------------------------------------- # Exclusion path patterns (deny-by-default). Grep -E, case-insensitive where @@ -185,6 +225,54 @@ latest_bot_approval() { | jq -r '[.[] | select(.state=="APPROVED") | select(.user.type=="Bot")] | sort_by(.submitted_at) | last | select(. != null) | "\(.user.login) \(.commit_id)"' } +# Any other OPEN PR whose base branch is this PR's head branch — i.e. +# something is stacked on it. Task 1's #25→#62 lesson: deleting a branch out +# from under a stacked PR breaks it silently. Prints the first hit's +# "repo#number" or nothing if the branch is safe to delete. +stacked_pr() { + local repo="$1" branch="$2" + local out rc + out="$(gh pr list -R "$ORG/$repo" --state open --base "$branch" --json number --jq '.[0].number // empty' 2>&1)"; rc=$? + if [[ $rc -ne 0 ]]; then + # Fail closed, matching ci_status/check_test_tamper's own convention in + # this file: an API error must not read the same as "genuinely zero + # stacked PRs," or the branch gets deleted anyway on a lookup failure — + # the exact #25->#62 bug, just moved one function over. Any non-empty + # return here is treated by the caller as "something's stacked, keep + # the branch," so this fails safe without changing do_merge()'s logic. + echo "UNKNOWN (gh pr list failed: $out)" + return + fi + echo "$out" +} + +# Merge a fully-verified PR. Re-verifies nothing itself — the caller has +# already re-checked CI/mergeable/exclusions/GO-freshness at $head_sha +# immediately before calling this, and --match-head-commit closes the +# remaining TOCTOU window atomically, server-side, for free (warden's fix: +# re-checking harder in bash can't close a race between "last check" and +# "the merge API call," but GitHub's own head-commit match can). +# Prints a one-line outcome to stdout; caller routes it to the right bucket. +do_merge() { + local n="$1" repo="$2" head_sha="$3" branch="$4" + local merge_out merge_rc + merge_out="$(gh pr merge "$n" -R "$ORG/$repo" --squash --match-head-commit "$head_sha" 2>&1)"; merge_rc=$? + if [[ $merge_rc -ne 0 ]]; then + echo "merge failed: $merge_out" + return 1 + fi + # Merged. Delete the branch only if nothing else is stacked on it. + local stacked + stacked="$(stacked_pr "$repo" "$branch")" + if [[ -n "$stacked" ]]; then + echo "merged (branch kept — $repo#$stacked is stacked on $branch)" + return 0 + fi + gh api -X DELETE "repos/$ORG/$repo/git/refs/heads/$branch" >/dev/null 2>&1 || true + echo "merged (branch deleted)" + return 0 +} + # --------------------------------------------------------------------------- # Main scan # @@ -199,7 +287,7 @@ scan_one_repo() { local repo="$1" prs="$2" [[ "$(jq 'length' <<<"$prs")" == "0" ]] && return - while IFS=$'\t' read -r num title author is_draft mergeable _merge_state head_sha; do + while IFS=$'\t' read -r num title author is_draft mergeable _merge_state head_sha head_branch; do [[ -z "$num" ]] && continue label_line="$repo #$num — $title (@$author)" @@ -244,30 +332,43 @@ scan_one_repo() { go_login="${go%% *}"; go_sha="${go##* }" if [[ "$go_sha" != "$head_sha" ]]; then echo "$label_line -- bot review ($go_login) is pinned to $go_sha, current head is $head_sha (stale — new commit pushed since review)" >>"$work/stale_go" - if [[ "$DRY_RUN" != "true" ]]; then + if ! is_true "$DRY_RUN"; then gh pr edit "$num" -R "$ORG/$repo" --remove-label "$LABEL" >/dev/null 2>&1 || true gh pr comment "$num" -R "$ORG/$repo" -b "Auto-merge janitor: removing \`$LABEL\` — the bot review is pinned to $go_sha but the current head is $head_sha. A new push invalidates the prior GO signal; re-review needed at the current commit." >/dev/null 2>&1 || true fi continue fi - # Full pass. Still human-click-gated (per design) -- report only, never merge. - echo "$label_line -- reviewed by $go_login at $go_sha, CI green, mergeable, clean of all exclusions" >>"$work/eligible" - done < <(jq -r '.[] | "\(.number)\t\(.title)\t\(.author.login)\t\(.isDraft)\t\(.mergeable)\t\(.mergeStateStatus)\t\(.headRefOid)"' <<<"$prs") + # Full pass. Everything above was re-verified fresh, at $head_sha, in + # this same run -- nothing here is trusted from an earlier pass. + pass_desc="reviewed by $go_login at $go_sha, CI green, mergeable, clean of all exclusions" + if is_true "$ENABLE_MERGE"; then + merge_result="$(do_merge "$num" "$repo" "$head_sha" "$head_branch")"; merge_rc=$? + if [[ $merge_rc -eq 0 ]]; then + echo "$label_line -- $pass_desc -- $merge_result" >>"$work/merged" + else + echo "$label_line -- $pass_desc -- $merge_result" >>"$work/merge_failed" + fi + else + # Still human-click-gated (per design, or ENABLE_MERGE not yet set) -- + # report only, never merge. + echo "$label_line -- $pass_desc" >>"$work/eligible" + fi + done < <(jq -r '.[] | "\(.number)\t\(.title)\t\(.author.login)\t\(.isDraft)\t\(.mergeable)\t\(.mergeStateStatus)\t\(.headRefOid)\t\(.headRefName)"' <<<"$prs") } if [[ -n "${PR_OVERRIDE:-}" ]]; then for pair in $PR_OVERRIDE; do repo="${pair%%:*}"; num="${pair##*:}" one="$(gh pr view "$num" -R "$ORG/$repo" \ - --json number,title,author,isDraft,mergeable,mergeStateStatus,headRefOid 2>/dev/null)" \ + --json number,title,author,isDraft,mergeable,mergeStateStatus,headRefOid,headRefName 2>/dev/null)" \ || { echo "$repo #$num: pr view failed" >>"$work/errors"; continue; } scan_one_repo "$repo" "[$one]" done else for repo in $REPOS; do prs="$(gh pr list -R "$ORG/$repo" --state open --label "$LABEL" \ - --json number,title,author,isDraft,mergeable,mergeStateStatus,headRefOid,reviewDecision 2>/dev/null)" \ + --json number,title,author,isDraft,mergeable,mergeStateStatus,headRefOid,headRefName,reviewDecision 2>/dev/null)" \ || { echo "$repo: pr list failed" >>"$work/errors"; continue; } scan_one_repo "$repo" "$prs" done @@ -280,9 +381,15 @@ section() { local t="$1" f="$2"; echo "### $t ($(count "$f"))"; [[ -s "$work/$f" { echo "## 🤖 Agent Merge Janitor — $(date -u +%Y-%m-%d\ %H:%MZ)" echo "- Repos scanned: $REPOS" - echo "- Human click still required to actually merge (see task_1785685546659 design — reviewer/author separation not yet guaranteed)." - [[ "$DRY_RUN" == "true" ]] && echo "- **DRY RUN** (no labels/comments touched)" + if is_true "$ENABLE_MERGE"; then + echo "- **ENABLE_MERGE=true — this run merges eligible PRs automatically.** Reviewer/author separation is not yet guaranteed (see task_1785685546659 design); this should only be true with boss's explicit confirmation that Aaron answered the credential-scoping question." + else + echo "- Human click still required to actually merge (see task_1785685546659 design — reviewer/author separation not yet guaranteed)." + fi + is_true "$DRY_RUN" && echo "- **DRY RUN** (no labels/comments/merges touched)" echo + section "✅ Merged" merged + section "💥 Merge attempted, failed" merge_failed section "✅ Ready — human, please merge" eligible section "🚫 Excluded (path/size/test-tamper)" excluded section "⏳ Stale GO — re-review needed" stale_go