review: un-fixable PR framing is a COMMENT, not a blocking RC (#369) #371

Merged

joshtronic merged 2 commits from fix/369-unfixable-framing-comment into master

2026-07-09 22:56:41 +00:00

igor commented

2026-07-09 04:27:41 +00:00

Collaborator

Addresses #369 at the root: the reviewer should not have issued a blocking RC on un-fixable pipeline framing.

This PR (the prevention half): broadens the directive's existing "machine-generated count → COMMENT" rule to cover any pipeline-generated title/description framing the author can't edit from inside the PR (the ergo-decedo case), and names the auto-merge deadlock as the explicit consequence. Low-risk (directive-only), make test green.

Recommended follow-up — the deterministic backstop (I did NOT rush it): the directive is a soft prompt (the model flagged the root cause last time and blocked anyway). To guarantee no deadlock, lib/automerge.sh:353 should stop treating a shadow RC as blocking once it's been escalated to the human. Safe design:

Add review_set_escalated <key> <head_sha> (mirrors review_set_rework_rounds) that records .review[key].escalated_sha, called at both escalation sites (tick.sh:2818 rc_rounds≥3, tick.sh:3652 no-commit rework).
In automerge, when verdict==REQUEST_CHANGES: allow the merge only if escalated_sha == current head sha AND the human has approved (already required). Tying it to the exact escalated diff means a new sha re-blocks normally — no stale-flag leak.
Cover it in bin/test-automerge.sh.

I held off building that tonight because it's merge-gating on a self-deploying harness and deserves careful sha-context wiring + tests, not an overnight rush. Happy to build it in a focused pass on your go.

Note: the currently-wedged igor.bot#307 needs a manual nudge (clear its .review entry or merge past it) — this fix prevents recurrence, it doesn't retroactively unwedge.

Addresses #369 at the root: the reviewer should not have issued a blocking RC on un-fixable pipeline framing. **This PR (the prevention half):** broadens the directive's existing "machine-generated count → COMMENT" rule to cover *any* pipeline-generated title/description framing the author can't edit from inside the PR (the ergo-decedo case), and names the auto-merge deadlock as the explicit consequence. Low-risk (directive-only), `make test` green. **Recommended follow-up — the deterministic backstop (I did NOT rush it):** the directive is a soft prompt (the model flagged the root cause last time and blocked anyway). To *guarantee* no deadlock, `lib/automerge.sh:353` should stop treating a shadow RC as blocking once it's been **escalated to the human**. Safe design: - Add `review_set_escalated <key> <head_sha>` (mirrors `review_set_rework_rounds`) that records `.review[key].escalated_sha`, called at both escalation sites (`tick.sh:2818` rc_rounds≥3, `tick.sh:3652` no-commit rework). - In automerge, when `verdict==REQUEST_CHANGES`: allow the merge **only if** `escalated_sha == current head sha` AND the human has approved (already required). Tying it to the exact escalated diff means a *new* sha re-blocks normally — no stale-flag leak. - Cover it in `bin/test-automerge.sh`. I held off building that tonight because it's merge-gating on a self-deploying harness and deserves careful sha-context wiring + tests, not an overnight rush. Happy to build it in a focused pass on your go. **Note:** the currently-wedged `igor.bot#307` needs a manual nudge (clear its `.review` entry or merge past it) — this fix prevents recurrence, it doesn't retroactively unwedge.

igor added 1 commit

2026-07-09 04:27:41 +00:00

review: unfixable PR framing is a COMMENT, not a blocking RC

Lint / check-sync (push) Successful in 5s

Details

Lint / check-sync (pull_request) Successful in 4s

Details

9178bb1f66

Part of #369. The directive already downgraded a machine-generated *count*
mismatch to COMMENT, but scoped it narrowly to counts. The ergo-decedo deadlock
was pipeline-generated title/description *framing* the author couldn't edit from
inside the PR -- not covered, so the reviewer issued a blocking REQUEST_CHANGES,
which spun a no-op rework loop, escalated, and then deadlocked auto-merge on the
already-approved PR (refusing to merge past the live RC, every tick).

Generalize the rule: whenever the only remaining defect is pipeline-generated
framing the author provably can't fix from inside the PR, return COMMENT and
name the deadlock consequence explicitly. Block only if the code/data in the
diff is actually wrong.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

igor commented

2026-07-09 04:29:29 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for 9178bb1f: success

Directive-only change: adds one bullet to bin/lib/review-directive.md generalizing the existing "machine-generated count → COMMENT" rule to any pipeline-generated PR framing the author can't edit from inside the PR. Verdict: APPROVE.

Scope: single 10-line addition to the review directive, nothing else touched. Well under the size bar and focused on #369.
Honesty: description accurately reflects the diff. It explicitly states the deterministic backstop (lib/automerge.sh, tick.sh escalation wiring, bin/test-automerge.sh) was not built here and defers it — no checkbox claims work that isn't present. Good.
Correctness of the rule: the new bullet is tightly scoped — it fires only when the ONLY remaining defect is un-editable pipeline framing, and it preserves the hard block for cases where "the CODE or DATA in the diff is actually wrong." It does not weaken any security/merge gate; it narrows one false-block case that otherwise deadlocks auto-merge on an already-approved PR. Consistent with the adjacent count rule.
CI: success. No test changes are meaningful for a prose directive edit.

Notes for the human (non-blocking):

The justification ("refuses to merge past a live RC, every tick, forever") describes automerge.sh behavior I can't verify from this diff. The directive change stands on its own regardless, but the follow-up backstop the author proposes is the real fix — worth a decision on whether to greenlight that focused pass.
The wedged igor.bot#307 still needs the manual nudge called out in the description; this PR is prevention only.

_{Independent review by the harness on claude-opus-4-8 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `APPROVE` _(automated)_ CI for `9178bb1f`: **success** Directive-only change: adds one bullet to `bin/lib/review-directive.md` generalizing the existing "machine-generated count → COMMENT" rule to any pipeline-generated PR framing the author can't edit from inside the PR. Verdict: APPROVE. - **Scope**: single 10-line addition to the review directive, nothing else touched. Well under the size bar and focused on #369. - **Honesty**: description accurately reflects the diff. It explicitly states the deterministic backstop (`lib/automerge.sh`, `tick.sh` escalation wiring, `bin/test-automerge.sh`) was *not* built here and defers it — no checkbox claims work that isn't present. Good. - **Correctness of the rule**: the new bullet is tightly scoped — it fires only when the ONLY remaining defect is un-editable pipeline framing, and it preserves the hard block for cases where "the CODE or DATA in the diff is actually wrong." It does not weaken any security/merge gate; it narrows one false-block case that otherwise deadlocks auto-merge on an already-approved PR. Consistent with the adjacent count rule. - **CI**: `success`. No test changes are meaningful for a prose directive edit. Notes for the human (non-blocking): - The justification ("refuses to merge past a live RC, every tick, forever") describes `automerge.sh` behavior I can't verify from this diff. The directive change stands on its own regardless, but the follow-up backstop the author proposes is the real fix — worth a decision on whether to greenlight that focused pass. - The wedged `igor.bot#307` still needs the manual nudge called out in the description; this PR is prevention only. --- <sub>Independent review by the harness on `claude-opus-4-8` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-09 04:29:30 +00:00

27 seconds

igor requested review from joshtronic

2026-07-09 04:29:30 +00:00