Repository navigation
pr-pipeline: harden the lane-escalation gate for address-feedback #9419
Description
Follow-ups deferred from #9417, which lets address-feedback work a needs-human PR when a lane escalation put the label on. All fail safe or are low severity; the owner chose to harden later.
1. Re-check the gate source before the write runner pushes
select decides the gate once. When fix-requested=true, pr-run-activity-write.yml (the "Read the hold labels from the base" step) drops the human-gated label from its holds, and nothing in that workflow reads claude-lane-hold comments. A review-lane hold posted on the same head during the model turn, or a person removing and re-adding the label, is missed and the patch is pushed. ci-status still blocks the merge while the label is on.
Proposed fix: pass the gate kind (fix-request or lane-escalation) instead of a bare fix-requested flag, and re-run gateSource on a fresh read of comments and label events in the apply-patch hold step when it is lane-escalation.
2. Limit which escalation reasons ungate address-feedback
gateSource (.github/scripts/pr-unblock/gate-source.mjs) never reads the escalation comment's - Reason: line, so every pr-unblock reason ungates, including feedback-disputed, workflow-staged and unexpected-path-staged (report-escalation.sh). Today do-not-merge is the documented human re-gate. Proposed: an allowlist of reasons (for example ci-fix-*, the round cap), with any other reason returning unknown.
3. Let clear-escalations clear after an address-feedback round
clear-escalations.mjs (pr-unblock known set) treats <!-- pr-unblock:address-feedback-round --> and the model-run line as foreign markers, so once a round is recorded the label stays until a person removes it. Proposed: add both lines to its pr-unblock known set, as gateSource already does.
4. Count only trusted humans as lifting a review-lane hold
labelHistory in gate-source.mjs treats any actor's unlabeled event as lifting older holds. ci-status (pr-require-checks.yml, untrusted-lift step) accepts a lift only from trusted-actors.json and also counts a review bot's labeled event as a hold. Proposed: match ci-status on both points.
5. An unparsable hold marker disables the feature on that PR permanently
gateSource checks every hold on the timeline against criteria=[a-z-]+(?:,[a-z-]+)* before it applies the lifted or current-head logic, so one malformed hold, even an old lifted one, returns unknown for good. Confirm the criteria grammar against the producer (ci-workflows pr-review-cli.yml), or check parseability only for holds that are open or name the current head.
6. Confirm the order of GraphQL timelineItems
labelHistory relies on .at(-1) being the newest label event, which assumes timelineItems returns events oldest first. That assumption has not been checked against GitHub's docs. Confirm it, or sort by createdAt.
Activity
github-actions commentedon Oct 23, 2026
Proposed work class: work-class: scoped
Basis: automated intake triage; the lanes App applied this class.
Size: Medium
Complexity: Medium
Uncertainty: Known
Flags: none
Blocked by: none
#9419 (comment)
That decision settles the open questions. Items 1, 3, 4, 5 and 6 go ahead as proposed, and item 2 is closed as already done on main. The issue stays open for the five remaining acceptance criteria, which are bounded changes to the gate, clear-escalations and their tests. The work needs no new decision.
Automated triage, applied by the lanes App.
kyle-sexton commentedon Oct 26, 2026
Decision recorded. The owner answered in the Issue Triage session (2026-10-25), accepting the recommended answers below:
Question asked: Do the hardening now as the body proposes for items 1, 3, 4, 5 and 6, and close item 2 as done on main?
Standing rule: the owner choice the body records:
No intake refinement exists; triage handed the issue to a maintainer. Open items are the body's six follow-ups, each re-checked on origin/main. Items 1, 2 and 4 touch the gate that lets a lane work past
needs-human, so each is a security call.Accepted: Do it as proposed: pass the gate kind instead of a bare
fix-requestedflag and re-rungateSourceon a fresh read in the apply-patch hold step. Still open: the hold step drops the human-gated label whenFIX_REQUESTEDis true, with no re-check. Basis: origin/main.github/workflows/pr-run-activity-write.yml:1167-1196.Accepted: Close it as done.
laneEscalationAllowsnow reads every escalation's- Reason:line: an unlisted reason holds every activity, and an activity's own reason holds it. Basis:.github/scripts/pr-unblock/gate-source.mjs:145-161,183-203(added after this issue by fix(pr-pipeline): unstick fixers held by lane escalations and one-shot heads #9561, commit adcf682). This narrows the body's proposal:workflow-stagedstill lets address-feedback work, since it maps to resolve-conflicts.Accepted: Do it as proposed: add the round marker and the model-run line to clear-escalations' pr-unblock known set. Still open. Basis:
.github/scripts/pr-merge/clear-escalations.mjs:219lacks them;gate-source.mjs:132has them.Accepted: Do it as proposed: match ci-status on trusted actors and on a review bot's
labeledevent. Still open. Basis:gate-source.mjs:60-71counts any actor'sunlabeledevent.Accepted: Check parseability only for holds that are open or name the current head. Still open. Basis:
gate-source.mjs:107-111; judgment on the choice.timelineItems.Accepted: Sort by
createdAtinlabelHistory, which removes the dependency on the order. Basis:gate-source.mjs:61uses.at(-1); the API order is UNVERIFIED this pass; judgment.Acceptance criteria:
gateSourceon a fresh read when the gate kind islane-escalation, with a test case.clear-escalations.mjsaccepts the address-feedback round marker and model-run line in its pr-unblock known set, with a test case.labelHistorycounts anunlabeledevent as a lift only from a trusted actor, and a review bot'slabeledevent as a hold, matching ci-status, with test cases.gateSourcereturnunknown, with a test case.labelHistorypicks the newest label event bycreatedAt, not by list position.melodic-automation-lanes commentedon Oct 26, 2026
Refinement. A read-only run explored this repository, searched its issues and pull requests, and read the allowed docs to draft what follows. On an issue a listed human filed, it counts as the issue's spec once posted, while unedited; it is still model text, so check it.
Intended outcome: Harden the address-feedback lane-escalation gate for items 1, 3, 4, 5 and 6 as the owner's decision accepted; item 2 is already done on main.
Related work
Open questions
RECOMMENDED: Close it if that change covers this issue; otherwise record what is still left to do. Basis: #9561: already did item 2 (reason allowlist in laneEscalationAllows); only item 2 is covered
Sources read
Automated refinement by the lanes App. Answer with a comment whose first line is
**Decision recorded.**.kyle-sexton commentedon Oct 26, 2026
Released for re-triage under the owner's 2026-10-26 rule: a person is involved only for "1. A change in original intent/vision 2. Something comes up during implementation ... that alters intent/vision or the primary design 3. If there is a HIGH/CRITICAL security issue". None applies here, so the hold label came off and intake triage runs again now.
To hold this issue again, add
needs-human(orstatus: needs-decision) and name which of the three applies.