canary autocut: scope bump signals to watched-path commits, paginate the range, and persist failed promotions

[don-petry]

Split out of review feedback on PR #1021 (implements #1019). Both findings are valid and accepted; they are deferred because each needs an interface change, and CodeRabbit itself labelled them 🏗️ Heavy lift. #1021 already fixes the two defects that would have made it a non-fix (compare truncation in _gh_changed_files, and a silent promote-all).

1. _autocut_range_signals — bump signals are unscoped and the range is unpaginated

_autocut_range_signals() {
  local host="$1" base="$2" head="$3" json out
  json="$(gh api "repos/$host/compare/$base...$head" 2>/dev/null)" || return 1
  out="$(jq -r ' … (.commits | map(.commit.message // "")) as $msgs
    | { b: any($msgs[]; test("^\\w+(\\([^)]*\\))?!:") …), f: any($msgs[]; test("^feat…")) } … ')"

Two defects in one function:

• Unscoped. It scans every commit message in the range. An unrelated docs: feat!: … commit therefore forces a major bump on an otherwise script-only cut. Bump level drives the channel/ring pin surface, so a spurious major seeds a fresh v<newMAJOR>-next (per #657 F4) and consumers pinned to the old major stop receiving updates entirely — a silent-non-delivery outcome, the same class #1019 exists to remove.
• Unpaginated. Same root cause as the _gh_changed_files truncation bug fixed in #1021: the compare response caps .commits (250) and .files (300) and sets .truncated. Signals beyond the cap are invisible, so a genuine BREAKING CHANGE: in a long range can be missed and ship as a patch — the more dangerous direction.

Why it wasn't fixed in #1021: scoping requires knowing which commits touch a watched path, i.e. per-commit file lists, and the function's (host, base, head) signature carries no agent — so it cannot resolve _watched_paths. That is an interface change plus an API-cost decision (one call per commit), not a local edit.

Acceptance criteria:

• Bump signals are derived only from commits that touch a watched path for the agent being cut.
• Truncated ranges are handled explicitly — enumerate commits rather than trusting one capped response.
• Fail-safe direction is stated and tested: an unresolvable range must not silently downgrade a breaking change to a patch.
• Tests: unrelated feat! outside watched paths does not raise the bump; a BREAKING CHANGE: in a watched-path commit beyond the compare cap is detected.

2. cmd_promote_all — failed promotions are logged but not persisted

#1021 now emits an aggregated ::warning:: naming every failed agent, so a green sweep no longer looks clean. But the failure is still only in the run log: CANARY_PROMOTIONS_LOG records successful moves only, and sync-issues tracks BLOCKED gate states — not failed tag writes (a permission or API rejection). So a repeatedly-failing tag write leaves no durable artifact and no trend.

Acceptance criteria:

• Failed promotions are persisted alongside successes (extend CANARY_PROMOTIONS_LOG with an outcome field, or a sibling failure log).
• A tag write that fails on N consecutive scheduled runs raises or updates a blocker issue, so it escalates like a gate block rather than scrolling past.
• Distinguish a gate-blocked promotion (expected, already tracked) from a failed tag write (unexpected) — conflating them is what made this invisible.

Context

Found on #1021, whose own history is the argument for both: dev-lead posted detailed replies claiming fixes for the truncation and summary defects — naming functions and citing new bats tests — and made none of them (#1567). The fixes in #1021 are hand-applied steering commits (a700662, 55fe2e8). Please verify these by diffing the head, not by reading a reply.

Related: #1019, petry-projects/.github-private#1592, #1567, #657.

[don-petry]

Sharpening the scoping requirement — an "any watched hit in range" guard is vacuous

From the pr-review analysis on PR #1021 (which caught a false resolution claiming this was already fixed):

in the autocut flow the fallback only fires after a watched path already changed, so "any watched hit in range" is vacuously true; the inflation case (watched fix: commit + unrelated non-watched feat: commit in the same range) still bumps minor.

That is the trap in this issue, and it is worth stating explicitly because the obvious implementation walks straight into it.

_autocut_agent only reaches the range-signal path after any_watched_change has already returned 1. So a guard of the form "did any watched path change in this range?" is always true at that point and filters nothing. The bump would still be inflated by an unrelated commit.

The requirement is per-commit scoping, not per-range:

• Determine, for each commit in the range, whether that commit touches a watched path.
• Derive breaking / feat signals only from the subset of commits that do.
• A range containing a watched fix: commit and a non-watched docs: feat!: commit must yield patch, not major.

Suggested acceptance test (the case a range-level guard passes and a commit-level guard fails):

range = [ commit A: "fix(engine): …"      touches scripts/engine.sh      (watched)
          commit B: "feat!: new docs nav"  touches docs/nav.md            (NOT watched) ]
expected bump = patch      # not minor, not major

Combined with the pagination requirement already in this issue (the compare response caps .commits at 250 and sets .truncated), the signal derivation needs both: the full commit list, and per-commit file attribution.

For the record, the false resolution on #1021 claimed _autocut_detect_bump called watched_hits with a covering bats test; verified at head e1e5c74 — watched_hits occurrences inside _autocut_detect_bump: 0; matching tests: 0. Tracked as petry-projects/.github-private#1567 (fix merged).

[don-petry]

Dev-Lead Implementation Plan

Issue: #1023 — canary autocut: scope bump signals to watched-path commits, paginate the range, and persist failed promotions

Note (verify-by-diffing-head, per the issue): on main the bump-signal function is
_autocut_commit_signals(host, reusable, next_commit, mainsha) using the
commits?path= API (already path-scoped to the single reusable), not the
compare-based _autocut_range_signals the review quoted. The ACs are implemented
against the code actually on head.

Scope

Defect 1 — scope autocut bump signals to the agent's watched paths, paginate the
commit range (never trust one capped response), and fail safe to major (loudly), never
silently to patch, when the range is unresolvable. Defect 2 — persist failed tag-writes
in a sibling log distinct from gate-blocks, and escalate a write that fails on N consecutive
scheduled runs to a durable, needs-human/dev-lead blocker issue.

Implementation Checklist

• scripts/canary-rollout.sh — add _watched_paths <agent> (reusable + .agents[a].autocut.watched_paths[])
• scripts/canary-rollout.sh — add _commit_date <host> <sha> + rework _autocut_commit_signals to (agent, host, next_commit, mainsha): paginated, since-windowed, watched-path-scoped; rc 0=resolved / 1=fetch-error (→patch) / 3=range-unresolvable (→major)
• scripts/canary-rollout.sh — _autocut_detect_bump: on rc 3 fail safe to major with a ::warning:: (not patch)
• scripts/canary-rollout.sh — _log_promotion_failure + call it when cmd_promote's _gh_move_tag fails (writes CANARY_PROMOTIONS_FAILED_LOG; a gate-block returns before the move, so it is never logged here)
• scripts/canary-rollout.sh — cmd_sync_promotion_failures + sync-promotion-failures dispatch: upsert a marker-keyed per-agent issue tracking the consecutive-failure streak; escalate at threshold; reset/close on a successful write
• scripts/lib/canary-rollout.sh — pure helpers promotion_failure_next_count, promotion_failure_should_escalate
• .github/workflows/canary-rollout.yml — set CANARY_PROMOTIONS_FAILED_LOG + run sync-promotion-failures after promote-all (read standards/ci-standards.md first)

Tests to Write (tests/canary_rollout.bats)

• pure: promotion_failure_next_count (failed→+1, ok→reset, non-numeric→0) and promotion_failure_should_escalate
• autocut: an unrelated feat! touching a non-watched path does not raise the bump
• autocut: a BREAKING CHANGE: in a watched-path commit beyond the first page (cap) is detected → major
• autocut: an unresolvable range fails safe to major with a warning (never patch)
• promote: a failed _gh_move_tag writes the failure log (agent/ring/cand/host/reason); a gate-block does not
• sync-promotion-failures: opens/updates the streak issue; escalates (needs-human) at threshold; closes on a successful write

Test Command

./node_modules/.bin/bats tests/canary_rollout.bats (CI: bats --print-output-on-failure tests/canary_rollout.bats + shellcheck --severity=warning -x)

[don-petry]

Dev-Lead: Implementation Complete

Implemented against the code actually on main (verified by diffing head, as the issue asked): the
bump-signal function is _autocut_commit_signals using the commits?path= API — not the
compare-based _autocut_range_signals the review quoted — so the ACs are met against that code.

Plan Execution

Defect 1 — scope + paginate + fail-safe direction

• _watched_paths <agent> — reusable path + optional .agents[a].autocut.watched_paths[], de-duped
• _commit_date <host> <sha> — bounds the range with a since=<boundary date> window
• Reworked _autocut_commit_signals(agent, host, next_commit, mainsha): scoped to watched-path commits, paginated (never one capped response), boundary excluded; rc 0=resolved / 1=fetch-error / 3=range-unresolvable
• _autocut_detect_bump: rc 3 fails safe to MAJOR with a ::warning:: (never silently to patch); rc 1 keeps the existing patch fail-safe (transient error, knob can still force)

Defect 2 — persist + escalate failed tag writes, distinct from gate-blocks

• _log_promotion_failure + hook in cmd_promote's post-move error path → sibling CANARY_PROMOTIONS_FAILED_LOG (a gate-block returns before the move, so it is never logged here)
• cmd_sync_promotion_failures + sync-promotion-failures dispatch: per-agent marker-keyed tracking issue; consecutive-failure streak escalates to needs-human+dev-lead at the threshold; auto-closes when a write recovers
• Pure cores promotion_failure_next_count / promotion_failure_should_escalate in the lib (unit-tested, no I/O)
• .github/workflows/canary-rollout.yml: set CANARY_PROMOTIONS_FAILED_LOG on the run step + a new scheduled always() step running sync-promotion-failures (read standards/ci-standards.md first; no new actions, uses the existing App token)

Test Results

./node_modules/.bin/bats tests/canary_rollout.bats → 273 passing, 0 failing (257 baseline + 16 new).
shellcheck --severity=warning -x scripts/canary-rollout.sh scripts/lib/canary-rollout.sh → clean.
bash .dev-lead/scripts/dev-lead-lint.sh → all checks passed.

ok 268 autocut — an unrelated feat! OUTSIDE the watched paths does NOT raise the bump (#1023)
ok 269 autocut — a breaking feat! on a CONFIGURED extra watched path IS detected → major (#1023)
ok 270 autocut — a BREAKING CHANGE in a watched-path commit BEYOND the page cap IS detected → major (#1023)
ok 271 autocut — an UNRESOLVABLE range fails safe to MAJOR (loudly), never silently to patch (#1023)
ok 128 a failed promote tag-write is persisted to the failure log (agent/ring/cand/host/reason) (#1023)
ok 129 a gate-BLOCKED promotion writes NOTHING to the failure log (#1023)
ok 148 sync-promotion-failures opens a tracking issue on the FIRST failure — not yet escalated (#1023)
ok 149 sync-promotion-failures escalates (needs-human + dev-lead) at the Nth consecutive failure (#1023)
ok 150 sync-promotion-failures auto-closes the tracking issue when the write recovers (#1023)
ok 27-31 promotion_failure_next_count / _should_escalate pure cores

Files Changed

• scripts/canary-rollout.sh — _watched_paths, _commit_date, reworked _autocut_commit_signals (scoped+paginated+3-way fail-safe), _autocut_detect_bump major-on-unresolvable, _log_promotion_failure, cmd_sync_promotion_failures + dispatch, two config knobs
• scripts/lib/canary-rollout.sh — pure promotion_failure_next_count / promotion_failure_should_escalate
• tests/canary_rollout.bats — 16 new tests (4 scoping/pagination/fail-safe, 2 promote-log, 5 escalation, 5 pure) + two stubs
• .github/workflows/canary-rollout.yml — failed-log env + scheduled escalation step

Acceptance Criteria

Defect 1: ✅ signals only from watched-path commits · ✅ truncated ranges enumerated (paginated + since) rather than trusting one capped response · ✅ fail-safe direction stated + tested (unresolvable ⇒ major, never a silent patch downgrade) · ✅ tests for both the unscoped-feat! and beyond-the-cap cases.
Defect 2: ✅ failures persisted alongside successes (sibling log) · ✅ N-consecutive-run escalation raises/updates a blocker issue · ✅ gate-block vs failed-write kept distinct (only real write rejections are logged/escalated).

Notes

• Chose a sibling failure log over adding an outcome column to CANARY_PROMOTIONS_LOG — additive/non-breaking to the existing 4-column deployment consumer.
• Fail-safe philosophy is deliberately split: a transient fetch error stays patch (matches the existing pinned test; a real break can be forced with the autocut.bump knob), while an unresolvable/truncated range escalates to major loudly — the direction the issue calls out as the more dangerous to get wrong.
• Escalation threshold is CANARY_PROMOTION_FAILURE_ESCALATE_AFTER (default 2 ≈ 8h at the 4h cadence); page ceiling is CANARY_MAX_COMMIT_PAGES (default 50 = 5000 commits).
• No follow-ups required.