chore(lessons): promote both pending lessons, one as a gate and one as a rule

.claude/pending-lessons.md

@@ -1,15 +1,12 @@
-# Pending lessons (staging — not loaded into context)
+# Pending lessons (staging, not loaded into context)
 
 Reviewed and promoted via `/lesson`. Delete each entry once promoted to a rule/hook or dropped.
 
-## 2026-07-14 — sweep-merge can race a re-triggered review on a BEHIND PR → merges past CHANGES_REQUESTED
-- Trigger: `tools/merge-sweep-cov.sh` merging any PR that is BEHIND main (require-up-to-date) and therefore needs an update-branch. Hit once on orbit-api #403 (a HIGH backend-contract finding shipped to main + deployed before the re-review landed; the fix went to the orphaned head branch, not main).
-- Type: checkable (the sweep script can enforce this deterministically).
-- Proposed home: a guard inside `tools/merge-sweep-cov.sh` — after its update-branch step, re-poll `gh pr view <n> --json reviewDecision` until the re-triggered `review` check reaches a terminal state, and BLOCK the merge unless it re-settles to APPROVED (never merge on the pre-update APPROVED snapshot). Secondary signal to detect a past occurrence: the PR's head branch survives deletion (a post-merge push re-created it) = an orphaned fix that never reached main — scan for surviving head branches after a sweep.
-- Draft: In the sweep, sequence = update-branch → wait-for-checks-terminal (INCLUDING `review`) → re-read reviewDecision → if APPROVED and required checks green (or coverage-only), merge; else abort + report. Do not read reviewDecision once before the update-branch and reuse it.
-- Interim operational guard (until promoted): for every BEHIND-PR sweep tonight, after merge re-check `reviewDecision` + whether the head branch still exists; if flipped/orphaned, fix-forward onto main.
+Queue is empty.
 
 ## Graduated
 
 - 2026-07-08 "don't offer optional next-steps" + 2026-07-09 proactivity failures (assume/ask/optional/improvise) → merged as one class and graduated to the global **proactivity guard** (`~/.claude/hooks/proactivity-reminder.mjs` UserPromptSubmit re-injection + `~/.claude/hooks/proactivity-guard.mjs` Stop class-gate). See `project_proactivity_guard` memory. Cleared 2026-07-09.
 - 2026-07-14 "opencode + Zen" is the opencode Zen gateway, NOT Z.ai → promoted to the `feedback_opencode_zen_not_zai` memory and to the `OpenCode Go plus Zen over OpenRouter` ADR in the brain vault, which carries the full naming trap + the pricing rationale. Cleared 2026-07-16.
+- 2026-07-14 sweep-merge races a re-triggered review on a BEHIND PR and merges past CHANGES_REQUESTED (orbit-api #403) → graduated to a **gate**, not prose: `tools/merge-sweep-cov.sh` and `tools/merge-sweep.sh` now block every merge path until the `review` check on the CURRENT head SHA settles, re-read `reviewDecision` after it does, and scan merged PRs' head branches at end of sweep, exiting 1 on a re-created (orphaned) branch. Cleared 2026-07-24.
+- 2026-07-24 background subagents idle on phantom "background waiters" while babysitting CI → promoted to `.claude/skills/orchestrate/SKILL.md`, "Delegation discipline → Waiting is foreground work, on both sides": the subagent-side foreground-poll contract plus the parent-side rule that "standing by" is not progress. Cleared 2026-07-24.

.claude/skills/orchestrate/SKILL.md

@@ -92,3 +92,19 @@ landed clean. So:
   (SendMessage), never into the main session.
 - The main session keeps only: decisions, small verification reads, user
   checkpoints, and cross-repo sequencing.
+
+### Waiting is foreground work, on both sides
+
+A stopped agent receives no notifications, so a background waiter it armed can never wake
+it: ending a turn with the goal unmet records the task as idle, and only a human nudge
+restarts it. Measured 2026-07-24: three agents in one day (the ui merge-chain, the Phase 3
+deletion, the orchestrate-skill fix) each ended their turn on "the monitor will notify me"
+with no live background child. Two of the three prompts already carried a warning against
+exactly that, so the subagent-side half alone does not hold. Both halves are the rule:
+
+- **In a prompt whose task includes waiting on CI or a review:** poll in the FOREGROUND,
+  sleep 60 to 120s per loop, inside your own turn. End the turn only on the goal state or a
+  genuinely unfixable blocker, and say which one.
+- **On any completion notification whose result reads "waiting", "standing by", or "monitor
+  armed":** read the real PR/CI state yourself and send the agent back to work with it.
+  Standing by is not progress.

tools/README.md

@@ -15,8 +15,8 @@ Read `CONVENTIONS.md` before adding one. Use the `/make-tool` skill to scaffold
 | Tool | What it does | Usage |
 |---|---|---|
 | `agent-review.sh` / `agent-review.ps1` | Cross-model second opinion (GLM-5.2 via opencode) on one claim or review finding. Thin wrapper over `.claude/skills/second-opinion/second-opinion.mjs`; prints one line of JSON (`AGREE` / `DISAGREE` / `UNSURE`, or a graceful `UNAVAILABLE`). | `agent-review --claim "<claim>"` or `agent-review < dossier.txt`; `--help` for options |
-| `merge-sweep.sh` | Require-up-to-date server-side merge sweep: per PR, update-branch then poll `mergeStateStatus` until it is decidable and squash-merge. Skips on a failed required check or timeout. | `bash merge-sweep.sh <repo> <pr...>` |
-| `merge-sweep-cov.sh` | Coverage-aware merge sweep: like `merge-sweep.sh`, but admin-overrides a SonarCloud failure that is solely new-code coverage (verified from the check-run summary), and skips anything more. | `bash merge-sweep-cov.sh <repo> <pr...>` |
+| `merge-sweep.sh` | Require-up-to-date server-side merge sweep: per PR, update-branch then poll `mergeStateStatus` until it is decidable and squash-merge. Skips on a failed required check or timeout. Blocks every merge until the `review` check on the CURRENT head SHA settles (fail-closed: an unreadable workflow list keeps the wait on), so an update-branch cannot merge on a stale APPROVED, and exits 1 on a merged PR whose head branch tip moved past the merged SHA. | `bash merge-sweep.sh <repo> <pr...>` (or `--help`) |
+| `merge-sweep-cov.sh` | Coverage-aware merge sweep: like `merge-sweep.sh` (same review-staleness guard and orphaned-head scan), but admin-overrides a SonarCloud failure that is solely new-code coverage (verified from the check-run summary), and skips anything more. | `bash merge-sweep-cov.sh <repo> <pr...>` (or `--help`) |
 | `rollup.sh` | Thin cross-repo CI/nightly health roll-up: reads the latest `main` run of each tracked quality gate across all three Orbit repos and prints ONE consolidated verdict (exit `0` green / `1` red / `2` tool-error). Reads run conclusions only; runs and audits nothing. Backs the `/rollup` skill and `.github/workflows/rollup.yml`. | `bash rollup.sh` (or `--help`) |
 | `surface-manifest.mjs` | Derives the visual-surface inventory from the codebase (web routes, the multi-view Today root, overlays) into `.claude/manifests/surfaces.json`, expanded to one cell per surface x theme x locale. Emits **no status field** on purpose. Survives until the #539 Linear project completes (REBUILD.md D39), then folds into `arch-map.mjs`. | `npm run surfaces:manifest` |
 | `capture-surfaces.mjs` | Playwright capture of one screenshot per manifest cell into `.artifacts/surfaces/`, against a running local stack. Reports every surface it cannot reach generically instead of skipping it. The evidence-gate screenshot mechanism (D7). | `ORBIT_AUTH_TOKEN=... npm run surfaces:capture` |

tools/dash-baseline.json

@@ -160,7 +160,5 @@
   "packages/shared/src/utils/plural.ts": 1,
   "sonar-project.properties": 13,
   "TESTING.md": 7,
-  "tools/CONVENTIONS.md": 2,
-  "tools/merge-sweep-cov.sh": 3,
-  "tools/merge-sweep.sh": 1
+  "tools/CONVENTIONS.md": 2
 }

tools/merge-sweep-cov.sh

@@ -9,9 +9,69 @@
 #         never a Bug/Vuln/Hotspot/Smell/Duplication/rating drop) -> admin squash-merge
 #         (coverage debt repaid in the Sonar burn-down; rubber-stamp tests are banned).
 #       * Sonar FAILURE on anything more -> SKIP (needs a real fix).
-# Never touches the local working tree. Usage: merge-sweep-cov.sh <repo> <pr...>
-repo="$1"; shift
-gate() { # prints  MS \t REVIEW \t NONSONAR_FAILED \t SONARSTATE \t SHA
+# WHY the review-staleness guard below: an update-branch rewrites the head SHA and re-triggers the
+# `review` check, but GitHub keeps the PRE-update APPROVED reviewDecision while that re-review runs,
+# so a sweep that merges on a decidable merge state can ship past a CHANGES_REQUESTED that lands
+# seconds later. That happened on https://github.com/thomasluizon/orbit-api/pull/403: a HIGH
+# backend-contract finding reached main and deployed, and the fix went to the orphaned head branch.
+# Never touches the local working tree.
+set -u
+
+REVIEW_WORKFLOW_PATH=".github/workflows/claude-review.yml"
+REVIEW_CHECK_NAME="review"
+
+usage() {
+  cat <<EOF
+Coverage-aware merge sweep: squash-merge each APPROVED, green PR server-side.
+
+Usage: merge-sweep-cov.sh <owner/repo> <pr-number>...
+       merge-sweep-cov.sh --help
+
+Per PR it update-branches, polls until the merge state is decidable, then merges:
+  Sonar SUCCESS or absent           -> squash merge
+  Sonar FAILURE, new-code coverage  -> admin squash merge (coverage-only override)
+  Sonar FAILURE, anything else      -> SKIP
+
+It refuses to merge while the \`$REVIEW_CHECK_NAME\` check for the CURRENT head SHA is still
+running, and re-reads reviewDecision after that check settles, so a pre-update APPROVED can
+never carry a merge. Only a workflow lookup that succeeds and shows no $REVIEW_WORKFLOW_PATH
+skips that wait; a failed lookup keeps the guard on.
+
+After the sweep it re-checks every merged PR's head branch. A branch whose tip moved past the
+SHA that was merged carries a post-merge commit that never reached main.
+
+Output (stdout): one MERGED/SKIP/FAIL-ADMIN line per PR, then any ORPHANED-HEAD lines, then
+COV-SWEEP-DONE.
+Exit codes: 0 every merged head verified clean; 1 at least one orphaned head branch; 2 bad usage;
+3 a head branch could not be verified (unknown is not a clean pass).
+EOF
+}
+
+case "${1:-}" in
+  -h | --help)
+    usage
+    exit 0
+    ;;
+esac
+if [ "$#" -lt 2 ]; then
+  usage >&2
+  exit 2
+fi
+repo="$1"
+shift
+
+# Fails CLOSED: only a lookup that SUCCEEDS and positively shows no review workflow turns the wait
+# off, so an auth/rate-limit/network hiccup costs a slower sweep rather than the guard itself.
+review_required=1
+if workflow_paths=$(gh api "repos/$repo/actions/workflows" --paginate --jq '.workflows[].path' 2>/dev/null); then
+  printf '%s\n' "$workflow_paths" | grep -qx "$REVIEW_WORKFLOW_PATH" || review_required=""
+else
+  echo "WARN: could not list $repo workflows; assuming the $REVIEW_CHECK_NAME check is required" >&2
+fi
+
+merged_heads=""
+
+gate() { # prints  MS \t REVIEW \t NONSONAR_FAILED \t SONARSTATE \t SHA \t REVIEWCHECK
   gh pr view "$1" --repo "$repo" --json mergeStateStatus,reviewDecision,statusCheckRollup,headRefOid 2>/dev/null | node -e "
     let s='';process.stdin.on('data',d=>s+=d).on('end',()=>{
       try{
@@ -22,37 +82,118 @@ gate() { # prints  MS \t REVIEW \t NONSONAR_FAILED \t SONARSTATE \t SHA
         const nonSonar=failed.filter(c=>(c.name||c.context)!=='SonarCloud Code Analysis').map(c=>c.name||c.context);
         const sonar=rows.find(c=>(c.name||c.context)==='SonarCloud Code Analysis')||{};
         const sonarState=(sonar.conclusion||sonar.state||'NONE').toUpperCase();
-        process.stdout.write([(d.mergeStateStatus||'?'),(d.reviewDecision||'?'),(nonSonar.join(',')||'NONE'),sonarState,(d.headRefOid||'')].join('\t'));
-      }catch(e){process.stdout.write('ERR\tERR\tERR\tERR\t');}
+        const review=rows.find(c=>(c.name||c.context)==='$REVIEW_CHECK_NAME');
+        const reviewSettled=!!review&&(!!review.conclusion||(review.status||'').toUpperCase()==='COMPLETED');
+        const reviewCheck=!review?'ABSENT':(reviewSettled?'SETTLED':'RUNNING');
+        process.stdout.write([(d.mergeStateStatus||'?'),(d.reviewDecision||'?'),(nonSonar.join(',')||'NONE'),sonarState,(d.headRefOid||''),reviewCheck].join('\t'));
+      }catch(e){process.stdout.write('ERR\tERR\tERR\tERR\t\tERR');}
     })"
 }
+
+squash_merge() { # <pr> <head-sha> <label> [extra gh pr merge flags...]
+  local pr="$1" head_sha="$2" label="$3"
+  shift 3
+  local branch
+  branch=$(gh pr view "$pr" --repo "$repo" --json headRefName --jq .headRefName 2>/dev/null)
+  if gh pr merge "$pr" --repo "$repo" --squash --delete-branch "$@" >/dev/null 2>&1; then
+    echo "MERGED #$pr ($label)"
+    # `^` is illegal in a refname, so it cannot collide with a branch name.
+    merged_heads="$merged_heads $pr^$branch^$head_sha"
+    return 0
+  fi
+  return 1
+}
+
 for n in "$@"; do
   gh pr update-branch "$n" --repo "$repo" >/dev/null 2>&1
   done_pr=""
-  for i in $(seq 1 45); do   # ~15 min per PR
-    IFS=$'\t' read -r ms rev nonsonar sonar sha < <(gate "$n")
-    if [ "$rev" != "APPROVED" ]; then echo "SKIP #$n review=$rev"; done_pr=1; break; fi
-    if [ "$nonsonar" != "NONE" ] && [ "$nonsonar" != "ERR" ]; then echo "SKIP #$n FAILED(non-sonar)=[$nonsonar]"; done_pr=1; break; fi
-    if [ "$ms" = "DIRTY" ]; then echo "SKIP #$n DIRTY (conflict)"; done_pr=1; break; fi
-    if [ "$ms" = "BEHIND" ]; then gh pr update-branch "$n" --repo "$repo" >/dev/null 2>&1; sleep 20; continue; fi
+  block_reason="no decidable merge state"
+  for i in $(seq 1 45); do # ~15 min per PR
+    IFS=$'\t' read -r ms rev nonsonar sonar sha reviewcheck < <(gate "$n")
+    if [ "$rev" != "APPROVED" ]; then
+      echo "SKIP #$n review=$rev"
+      done_pr=1
+      break
+    fi
+    if [ "$nonsonar" != "NONE" ] && [ "$nonsonar" != "ERR" ]; then
+      echo "SKIP #$n FAILED(non-sonar)=[$nonsonar]"
+      done_pr=1
+      break
+    fi
+    if [ "$ms" = "DIRTY" ]; then
+      echo "SKIP #$n DIRTY (conflict)"
+      done_pr=1
+      break
+    fi
+    if [ "$ms" = "BEHIND" ]; then
+      block_reason="still BEHIND main after update-branch"
+      gh pr update-branch "$n" --repo "$repo" >/dev/null 2>&1
+      sleep 20
+      continue
+    fi
+    # The APPROVED read above is PR-level and survives the update-branch, so it can predate this
+    # head SHA. Nothing below may merge until this SHA's own review has settled.
+    if [ -n "$review_required" ] && [ "$reviewcheck" != "SETTLED" ]; then
+      block_reason="the $REVIEW_CHECK_NAME check on head $sha never settled (state=$reviewcheck), so the APPROVED is stale"
+      sleep 20
+      continue
+    fi
     # Non-Sonar failures already ruled out above; a Sonar FAILURE here is the SOLE blocker
-    # (ms is typically BLOCKED, since SonarCloud Code Analysis is a REQUIRED check) — handle it
-    # regardless of ms so a coverage-only PR doesn't loop to timeout.
+    # (ms is typically BLOCKED, since SonarCloud Code Analysis is a REQUIRED check), so handle it
+    # regardless of ms and a coverage-only PR does not loop to timeout.
     if [ "$sonar" = "FAILURE" ]; then
       summary=$(gh api "repos/$repo/commits/$sha/check-runs" --jq '.check_runs[] | select(.name=="SonarCloud Code Analysis") | .output.summary' 2>/dev/null)
       if printf '%s' "$summary" | grep -qi "Coverage on New Code" && ! printf '%s' "$summary" | grep -qiE "New Bugs|Bugs |Vulnerabilit|Security Hotspots|Security Rating|Code Smell|Duplicat|Maintainability Rating|Reliability Rating"; then
-        if gh pr merge "$n" --repo "$repo" --squash --admin --delete-branch >/dev/null 2>&1; then echo "MERGED #$n (admin: coverage-only override)"; else echo "FAIL-ADMIN #$n"; fi
+        squash_merge "$n" "$sha" "admin: coverage-only override" --admin || echo "FAIL-ADMIN #$n"
       else
-        echo "SKIP #$n Sonar fails on MORE than coverage — needs a real fix"
+        echo "SKIP #$n Sonar fails on MORE than coverage, needs a real fix"
       fi
-      done_pr=1; break
+      done_pr=1
+      break
     fi
     if { [ "$ms" = "CLEAN" ] || [ "$ms" = "UNSTABLE" ]; } && { [ "$sonar" = "SUCCESS" ] || [ "$sonar" = "NONE" ]; }; then
-      if gh pr merge "$n" --repo "$repo" --squash --delete-branch >/dev/null 2>&1; then echo "MERGED #$n (clean)"; done_pr=1; break; fi
+      if squash_merge "$n" "$sha" "clean"; then
+        done_pr=1
+        break
+      fi
     fi
-    # ms=BLOCKED/UNKNOWN with Sonar not-yet-failed → required checks still settling — wait
+    # ms=BLOCKED/UNKNOWN with Sonar not-yet-failed: required checks still settling, wait
+    block_reason="no decidable merge state (ms=$ms)"
     sleep 20
   done
-  [ -z "$done_pr" ] && echo "SKIP #$n (timeout waiting for a decidable merge state)"
+  [ -z "$done_pr" ] && echo "SKIP #$n (timeout: $block_reason)"
+done
+
+# A head branch that merely survived --delete-branch is benign; only a tip that MOVED past the SHA
+# that was merged proves a post-merge commit that never reached main. The GraphQL ref lookup exits 0
+# with an EMPTY oid for a deleted branch, so a non-zero exit is unambiguously "could not verify",
+# which is reported and counted, never silently read as clean.
+branch_tip() { # <branch>; stdout: tip SHA, or empty when the ref is confirmed absent
+  gh api graphql \
+    -f query='query($o:String!,$n:String!,$q:String!){repository(owner:$o,name:$n){ref(qualifiedName:$q){target{oid}}}}' \
+    -F o="${repo%%/*}" -F n="${repo##*/}" -F q="refs/heads/$1" \
+    --jq '.data.repository.ref.target.oid // ""' 2>/dev/null
+}
+
+orphans=0
+unverified=0
+for entry in $merged_heads; do
+  pr="${entry%%^*}"
+  rest="${entry#*^}"
+  branch="${rest%^*}"
+  merged_sha="${rest##*^}"
+  [ -n "$branch" ] || continue
+  if ! tip=$(branch_tip "$branch"); then
+    echo "WARN: could not verify branch $branch for #$pr; orphan status unknown" >&2
+    unverified=$((unverified + 1))
+    continue
+  fi
+  if [ -n "$tip" ] && [ "$tip" != "$merged_sha" ]; then
+    echo "ORPHANED-HEAD #$pr $branch tip=$tip (moved past the merged $merged_sha, so those commits are NOT on main)"
+    orphans=$((orphans + 1))
+  fi
 done
+
 echo "COV-SWEEP-DONE"
+[ "$orphans" -eq 0 ] || exit 1
+[ "$unverified" -eq 0 ] || exit 3