diff --git a/.changes/pr-pipeline/feat-address-feedback-lane-escalation-ffa0d57f.md b/.changes/pr-pipeline/feat-address-feedback-lane-escalation-ffa0d57f.md new file mode 100644 index 0000000000..d2a2a6b04a --- /dev/null +++ b/.changes/pr-pipeline/feat-address-feedback-lane-escalation-ffa0d57f.md @@ -0,0 +1,7 @@ +--- +bump: minor +--- + +### Changed + +- `address-feedback` works a pull request carrying the human-gated label when a lane escalation (pr-unblock or fix-ci) put the label on and no review-lane hold names the current head or is open; a review-lane hold, or anything ambiguous, still limits it to threads a trusted person with write access asked it to fix. The label stays on: `clear-escalations` removes it only while no address-feedback round has been recorded since it went on, so once a round runs a person removes it. To stop the lane working such a PR, add `do-not-merge`. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index b36a852b18..83dfc4b83a 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -64,14 +64,25 @@ import { pathToFileURL } from "node:url"; import { createGitHub, paginate } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { humanGatedLabelFrom } from "../pr-unblock/address-feedback.mjs"; +import { + CLEARED, + ESCALATION, + escalationsSince, + foreignMarker, + isApp, + labelHistory, + laneHold, + lines, + openReviewHold, + PAIRED_MS, + RESUME_KEY, + same, +} from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; -const ESCALATION = ""; -const RESUME_KEY = /^$/; const REASON = /^- Reason: `([a-z-]+)`$/m; const clearedMarker = (reason, head) => ``; -const CLEARED = /^$/; // babysit_merge.py's verifier-refusal escalation, reasons comment and // shared-copy escalation: the marker on the first or second line, the next // line naming `at head ```. @@ -81,39 +92,18 @@ const PR_MERGE_MARKERS = Object.freeze([ "", ]); const AT_HEAD = / at head `([^`]*)`/g; -// A review lane's hold comment opens a line with this marker. The lanes post -// as claude[bot], or as github-actions[bot] with the lane's footer as the last -// non-blank line (address-feedback.mjs's lane comment rule). -const REVIEW_HOLD = ""; +export { ROUND_MARKER }; // github-actions[bot], the author of the review lanes' reviewed-head markers. export const MARKER_AUTHOR_ID = 41898282; const REVIEWED_HEAD = @@ -422,8 +424,18 @@ export async function select({ api.listThreads(pr), ]); // The human-gated label holds the PR except for threads a trusted person - // with write access asked the lane to fix. - const gated = (pull.labels ?? []).some((label) => label.name === humanGatedLabel); + // with write access asked the lane to fix, unless a lane escalation put it + // on and no review lane holds the PR (gate-source.mjs). The label stays on. + const labeled = (pull.labels ?? []).some((label) => label.name === humanGatedLabel); + const escalated = + labeled && + gateSource({ + timeline: [...commentItems(issueComments), ...(await api.listLabelEvents(pr))], + label: humanGatedLabel, + head: headSha, + isLanesApp: (user) => user?.id === appId && user?.type === "Bot", + }).source === "lane-escalation"; + const gated = labeled && !escalated; const fixers = await reviewFixers({ reviewComments, trustedHumans, permission: api.getPermission }); const kept = keepThreads({ threads, @@ -481,7 +493,10 @@ export async function select({ return { decision: "proceed", rounds, - fixRequested: gated, + // True drops the human-gated label from the write runner's holds: every + // kept thread carries a write-access fix request, or a lane escalation + // put the label on. + fixRequested: labeled, threads: worked, file: { version: 1, repository, pr, head_sha: headSha, threads: worked }, }; @@ -592,6 +607,7 @@ export function createGitHubApi({ token, repository, apiUrl, graphqlUrl, fetch = const repo = `${apiUrl}/repos/${owner}/${name}`; return { getPull: async (pr) => (await request(`${repo}/pulls/${pr}`)).data, + listLabelEvents: (pr) => listLabelEvents({ graphql, repository, number: pr }), // Metadata read, which every token holds. A 404 (not a collaborator) is none. async getPermission(login) { const url = `${repo}/collaborators/${encodeURIComponent(login)}/permission`; @@ -831,7 +847,8 @@ export async function main(argv, env, { api, log = (line) => console.log(line) } } output("proceed", String(result.decision === "proceed")); // The write runner drops the human-gated label from the hold list only - // when this is true: every kept thread carries a write-access fix request. + // when this is true: every kept thread carries a write-access fix request, + // or a lane escalation put the label on. output("fix-requested", String(result.fixRequested === true)); output("skip-reason", result.decision === "skip" ? result.skipReason : result.decision === "escalate" ? "awaiting-human" : ""); output("escalation-reason", result.escalationReason ?? ""); diff --git a/.github/scripts/pr-unblock/address-feedback.test.mjs b/.github/scripts/pr-unblock/address-feedback.test.mjs index acac42d8b6..177b2ff6aa 100644 --- a/.github/scripts/pr-unblock/address-feedback.test.mjs +++ b/.github/scripts/pr-unblock/address-feedback.test.mjs @@ -53,6 +53,10 @@ function fakeApi(overrides = {}) { listReviews: async () => overrides.reviews ?? [{ id: 10, user: { id: REVIEWER }, commit_id: HEAD }], listReviewComments: async () => overrides.reviewComments ?? [comment(100, REVIEWER)], listThreads: async () => overrides.threads ?? [thread("PRRT_1", [100])], + async listLabelEvents() { + overrides.labelEventReads?.push(1); + return overrides.labelEvents ?? []; + }, listPullCommits: async () => Object.keys(overrides.checks ?? { [HEAD]: [] }).map((sha) => ({ sha })), listCheckRuns: async (sha) => (overrides.checks ?? {})[sha] ?? [], createIssueComment: async (pr, body) => posted.push({ pr, body }), @@ -238,6 +242,52 @@ for (const [label, overrides] of [ }); } +// Where needs-human came from (gate-source.mjs): a lane escalation is the +// lanes App's label, then its marked comment; a review-lane hold is the review +// lane's hold comment. +const APP_USER = { login: "melodic-automation-lanes[bot]", id: APP, type: "Bot" }; +const LANE_ESCALATION = { + id: 2, + user: APP_USER, + body: `\n\nThe pr-unblock lane stopped.`, + created_at: "2026-10-18T10:00:05Z", + updated_at: "2026-10-18T10:00:05Z", +}; +const REVIEW_HOLD = { + id: 3, + user: { login: "github-actions[bot]", id: ACTIONS_BOT, type: "Bot" }, + body: `\nHeld.\n\nclaude-lane: claude-review`, + created_at: "2026-10-18T09:00:00Z", + updated_at: "2026-10-18T09:00:00Z", +}; +const APP_LABELED = [{ event: "labeled", actor: APP_USER, label: { name: "needs-human" }, created_at: "2026-10-18T10:00:00Z" }]; +const reviewedHead = { id: 1, user: { id: ACTIONS_BOT }, body: marker("claude-review", HEAD) }; + +test("on a needs-human PR a lane escalation gated, a reviewer-bot thread proceeds and releases the label hold", async () => { + const result = await run(fakeApi({ labels: ["needs-human"], issueComments: [reviewedHead, LANE_ESCALATION], labelEvents: APP_LABELED })); + assert.equal(result.decision, "proceed"); + assert.equal(result.fixRequested, true); + assert.equal(result.file.threads[0].root_comment_id, 100); +}); + +for (const [label, issueComments] of [ + ["a review-lane hold", [reviewedHead, REVIEW_HOLD]], + ["a lane escalation beside a review-lane hold", [reviewedHead, REVIEW_HOLD, LANE_ESCALATION]], + ["an escalation marker with no resume-key line", [reviewedHead, { ...LANE_ESCALATION, body: "\nStopped." }]], +]) { + test(`on a needs-human PR, a reviewer-bot thread under ${label} holds the PR (awaiting-human)`, async () => { + const result = await run(fakeApi({ labels: ["needs-human"], issueComments, labelEvents: APP_LABELED })); + assert.equal(result.decision, "skip"); + assert.equal(result.skipReason, "awaiting-human"); + }); +} + +test("without needs-human no label event is read", async () => { + const labelEventReads = []; + assert.equal((await run(fakeApi({ labelEventReads }))).decision, "proceed"); + assert.deepEqual(labelEventReads, []); +}); + test("without needs-human nothing changes and no permission is read", async () => { const api = fakeApi(escalated()); const result = await run(api); diff --git a/.github/scripts/pr-unblock/feedback-dispatch.mjs b/.github/scripts/pr-unblock/feedback-dispatch.mjs index b283682da4..4c8d4aafd3 100644 --- a/.github/scripts/pr-unblock/feedback-dispatch.mjs +++ b/.github/scripts/pr-unblock/feedback-dispatch.mjs @@ -22,8 +22,10 @@ // every comment is the reviewer bot's, a trusted person with write access // asked the lane to fix it (isFixRequest), or a trusted person answered the // lane's escalation (address-feedback.mjs threadAuthors); on a PR carrying -// the human-gated label, only a thread where a trusted person with write -// access asked the lane to fix it (isFixRequest); +// the human-gated label, the same when a lane escalation put the label on +// and no review lane holds the PR (gate-source.mjs gateSource), and +// otherwise only a thread where a trusted person with write access asked +// the lane to fix it (isFixRequest); // - that head has no pr-unblock / address-feedback check run yet (one the // usage limit stopped stops counting once its limit resets, and one that // started before the head's newest pr-unblock / publish-feedback check run @@ -60,6 +62,7 @@ import { pathToFileURL } from "node:url"; import { commitPulls } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { fixRequesters, humanGatedLabelFrom, threadAuthors, trustedHumansFrom } from "./address-feedback.mjs"; +import { commentItems, gateSource, listLabelEvents } from "./gate-source.mjs"; import { emptyRecord, isHandled, markHandled, readRecord, writeRecord } from "./dispatch-record.mjs"; import { anyHandles, limitResetsAt, permanentlyHandles } from "./limit-stop.mjs"; import { budgetedFetch } from "./request-budget.mjs"; @@ -227,8 +230,19 @@ export async function dispatchFeedback({ } const threads = await api.listReviewThreads(number); // A human-gated PR dispatches only for a thread a trusted person with - // write access asked the lane to fix. - const gated = (pull.labels ?? []).some((label) => label.name === humanGatedLabel); + // write access asked the lane to fix, unless a lane escalation put the + // label on and no review lane holds the PR. The label stays on either way: + // clear-escalations.mjs owns taking it off. + const labeled = (pull.labels ?? []).some((label) => label.name === humanGatedLabel); + const gate = labeled + ? gateSource({ + timeline: [...commentItems(comments), ...(await api.listLabelEvents(number))], + label: humanGatedLabel, + head, + isLanesApp: (user) => user?.id === appSenderId && user?.type === "Bot", + }) + : null; + const gated = labeled && gate.source !== "lane-escalation"; const fixers = await fixRequesters({ comments: threads.flatMap((thread) => thread.comments.nodes.map((node) => ({ authorId: node.author?.databaseId, login: node.author?.login, body: node.body })), @@ -240,7 +254,7 @@ export async function dispatchFeedback({ if (!hasWorkableThread(threads, { reviewerId: settings.reviewerId, appId: appSenderId, trustedHumans, requesters, fixers })) { return skip( gated - ? `#${number} is labeled ${humanGatedLabel} and has no thread a trusted person with write access asked the lane to fix` + ? `#${number} is labeled ${humanGatedLabel} (${gate.source}: ${gate.reason}) and has no thread a trusted person with write access asked the lane to fix` : `#${number} has no unresolved thread left by the reviewer bot alone, asked to be fixed, or answered by a trusted person`, ); } @@ -393,6 +407,7 @@ export function createGitHubApi({ token, repository, apiUrl, graphqlUrl, fetch = } throw new Error(`#${number} has more than 5000 comments`); }, + listLabelEvents: (number) => listLabelEvents({ graphql, repository, number }), async listReviewThreads(number) { const threads = []; let after = null; diff --git a/.github/scripts/pr-unblock/feedback-dispatch.test.mjs b/.github/scripts/pr-unblock/feedback-dispatch.test.mjs index d4943d19db..b0b3242201 100644 --- a/.github/scripts/pr-unblock/feedback-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/feedback-dispatch.test.mjs @@ -87,8 +87,9 @@ function fakeApi({ workflowRuns = [], workflowRunId = 38779200001, permissions = { "kyle-sexton": "admin" }, + labelEvents = [], } = {}) { - const calls = { dispatch: [], permission: [] }; + const calls = { dispatch: [], permission: [], labelEvents: 0 }; return { calls, // The event's workflow_run, which the script takes in place of a read. @@ -110,6 +111,10 @@ function fakeApi({ }, listIssueComments: async () => comments, listReviewThreads: async () => threads, + async listLabelEvents() { + calls.labelEvents += 1; + return labelEvents; + }, async getPermission(login) { calls.permission.push(login); return permissions[login] ?? "none"; @@ -783,6 +788,89 @@ for (const [label, options] of [ }); } +// Where needs-human came from (gate-source.mjs). A lane escalation: +// report-escalation.sh's label, then its marked comment, by the lanes App. A +// review-lane hold: the review lane's label and hold comment. +const APP_USER = { login: "melodic-automation-lanes[bot]", id: APP_ID, type: "Bot" }; +const ACTIONS_USER = { ...ACTIONS_BOT, type: "Bot" }; +const RESUME = ``; +const escalationComment = ({ body = `\n${RESUME}\nThe pr-unblock lane stopped.\n\n- Reason: \`ci-fix-round-cap\``, user = APP_USER, at = "2026-10-18T10:00:05Z" } = {}) => ({ + user, + body, + created_at: at, + updated_at: at, +}); +const holdComment = ({ sha = HEAD, at = "2026-10-18T09:00:00Z" } = {}) => ({ + user: ACTIONS_USER, + body: `\nChanges a trust gate.\n\n\`claude-lane: claude-security-review\``, + created_at: at, + updated_at: at, +}); +const labelEvent = (event, actor, at) => ({ event, actor, label: { name: "needs-human" }, created_at: at }); +const appLabeled = labelEvent("labeled", APP_USER, "2026-10-18T10:00:00Z"); +const reviewed = [marker("claude-review"), marker("claude-security-review")]; + +test("a needs-human PR a lane escalation gated dispatches for a reviewer-bot thread", async () => { + const api = fakeApi({ pulls: humanGated(), comments: [...reviewed, escalationComment()], labelEvents: [appLabeled] }); + const { result, lines } = await decide(api); + assert.equal(result?.number, 42, lines.join("\n")); + assert.equal(api.calls.dispatch.length, 1); +}); + +for (const [label, source, options] of [ + ["a review-lane hold on the current head", "review-hold", { + comments: [...reviewed, holdComment()], + labelEvents: [labelEvent("labeled", ACTIONS_USER, "2026-10-18T09:00:00Z")], + }], + ["a lane escalation beside a review-lane hold on the current head", "review-hold", { + comments: [...reviewed, holdComment({ at: "2026-10-18T10:30:00Z" }), escalationComment()], + labelEvents: [appLabeled], + }], + ["a lane escalation beside an open review-lane hold on an older head", "review-hold", { + comments: [...reviewed, holdComment({ sha: OLD_HEAD }), escalationComment()], + labelEvents: [appLabeled], + }], + ["an escalation marker with no resume-key line", "unknown", { + comments: [...reviewed, escalationComment({ body: "\nThe pr-unblock lane stopped." })], + labelEvents: [appLabeled], + }], + ["a lane escalation beside an escalation marker out of place", "unknown", { + comments: [...reviewed, escalationComment(), escalationComment({ body: `Note.\n\n${RESUME}`, at: "2026-10-18T10:05:00Z" })], + labelEvents: [appLabeled], + }], + ["a lane escalation beside a hold marker whose head does not parse", "unknown", { + comments: [...reviewed, holdComment({ sha: "bbbb" }), escalationComment()], + labelEvents: [appLabeled], + }], + ["an escalation marker someone other than the lanes App wrote", "unknown", { + comments: [...reviewed, escalationComment({ user: { login: "kyle-sexton", id: KYLE.databaseId, type: "User" } })], + labelEvents: [appLabeled], + }], + ["a lane escalation under a label a person added", "unknown", { + comments: [...reviewed, escalationComment()], + labelEvents: [labelEvent("labeled", { login: "kyle-sexton", id: KYLE.databaseId, type: "User" }, "2026-10-18T10:00:00Z")], + }], + ["a lane escalation older than the last label add", "unknown", { + comments: [...reviewed, escalationComment({ at: "2026-10-18T09:00:00Z" })], + labelEvents: [appLabeled], + }], +]) { + test(`a needs-human PR dispatches nothing for a reviewer-bot thread under ${label} (${source})`, async () => { + const api = fakeApi({ pulls: humanGated(), ...options }); + const { result, lines } = await decide(api); + assert.equal(result, null, lines.join("\n")); + assert.deepEqual(api.calls.dispatch, []); + // The skip names why the label does not admit the PR. + assert.ok(lines.at(-1).includes(`is labeled needs-human (${source}: `), lines.at(-1)); + }); +} + +test("a PR without needs-human reads no label events", async () => { + const api = fakeApi(); + assert.equal((await decide(api)).result.number, 42); + assert.equal(api.calls.labelEvents, 0); +}); + test("without needs-human a trusted reply dispatches with no permission read", async () => { const api = fakeApi({ threads: [decidedThread({ author: KYLE_LOGIN })] }); assert.equal((await decide(api)).result.number, 42); diff --git a/.github/scripts/pr-unblock/gate-source.mjs b/.github/scripts/pr-unblock/gate-source.mjs new file mode 100644 index 0000000000..23a99e181e --- /dev/null +++ b/.github/scripts/pr-unblock/gate-source.mjs @@ -0,0 +1,182 @@ +// gate-source.mjs: where a pull request's human-gated label came from, read +// from its timeline. clear-escalations.mjs (pr-merge) uses the pieces to decide +// whether the label may come off; feedback-dispatch.mjs and address-feedback.mjs +// (pr-unblock) use gateSource to decide whether address-feedback works the PR +// while the label stays on. +// +// Two writers put the label on: +// - a lane escalation: the lanes App's report-escalation.sh (pr-unblock, +// fix-ci) adds the label, then posts a comment whose first line is +// ESCALATION and whose second is the resume-key line; +// - a review-lane hold: a review lane's status job adds the label and posts a +// `` comment, +// as claude[bot], or as github-actions[bot] ending in the lane's footer. +// +// Timeline items take the REST timeline shape: `event` ("commented", +// "labeled", "unlabeled"), `user` or `actor` ({ login, id, type }), `body`, +// `label`, `created_at`, `updated_at`. Text written by anyone but those +// authors is never read. + +export const ESCALATION = ""; +export const RESUME_KEY = /^$/; +// Any escalation marker, of any lane: one that is not a whole ESCALATION +// comment is unparsable here. +const ANY_ESCALATION = ""; +const MODEL_RUN = /^$/; +export const CLEARED = /^$/; +// A review lane's hold comment opens a line with this marker. +const REVIEW_HOLD = "$/; +const LANE_FOOTER = /^`?claude-lane: (?:claude-review|claude-security-review)`?$/; +// report-escalation.sh writes the label, then the comment, retrying the +// comment for up to 15 seconds; a minute covers that (judgment). +export const PAIRED_MS = 60_000; + +export const lines = (body) => String(body ?? "").split("\n").map((line) => line.replace(/\r$/, "")); +export const same = (a, b) => String(a ?? "").toLowerCase() === String(b).toLowerCase(); +export const isApp = (user, bot) => user?.login === bot && user?.type === "Bot"; +// Written or edited at or after `since`; an unparsable time counts as since. +const writtenSince = (item, since) => + !(Date.parse(item.created_at) < since) || !(Date.parse(item.updated_at ?? item.created_at) < since); + +// The REVIEW_HOLD marker line of a review lane's hold comment, or undefined. +export function laneHold(item) { + const body = lines(item.body).map((line) => line.trim()); + const marker = body.find((line) => line.startsWith(REVIEW_HOLD)); + const lane = + isApp(item.user, "claude[bot]") || + (isApp(item.user, "github-actions[bot]") && LANE_FOOTER.test(body.findLast((line) => line !== "") ?? "")); + return item.event === "commented" && lane ? marker : undefined; +} + +// Issue comments (REST) as timeline items. +export const commentItems = (comments) => comments.map((comment) => ({ event: "commented", ...comment })); + +// The newest `labeled` event for `label`, when it was, and when the label was +// last removed at or before it (-Infinity when never). +export function labelHistory(timeline, label) { + const labeled = timeline.filter((item) => item.event === "labeled" && same(item.label?.name, label)).at(-1); + const since = Date.parse(labeled?.created_at ?? ""); + const lifted = Math.max( + -Infinity, + ...timeline + .filter((item) => item.event === "unlabeled" && same(item.label?.name, label)) + .map((item) => Date.parse(item.created_at)) + .filter((time) => time <= since), + ); + return { labeled, since, lifted }; +} + +// A review lane's hold comment written or edited since the label was last +// removed (ever, when it never was): a person removing the label lifts it. +export function openReviewHold(timeline, lifted) { + return timeline.find( + (item) => laneHold(item) !== undefined && !(Math.max(Date.parse(item.created_at), Date.parse(item.updated_at ?? item.created_at)) < lifted), + ); +} + +// The lanes App's escalation comments written at or after `since`. +export function escalationsSince(appComments, since) { + return appComments + .map((item) => ({ body: lines(item.body), at: Date.parse(item.created_at) })) + .filter(({ body, at }) => body[0] === ESCALATION && RESUME_KEY.test(body[1] ?? "") && at >= since); +} + +// The first marker line on a lanes App comment written or edited at or after +// `since` that `known` does not accept and is not clear-escalations' own. +export function foreignMarker(appComments, since, known) { + return appComments + .filter((item) => writtenSince(item, since)) + .flatMap((item) => lines(item.body).filter((line) => line.includes("\n\nStopped.`, at); +const hold = (sha, at) => + commented(ACTIONS, `\nHeld.\n\nclaude-lane: claude-review`, at); +const source = (timeline) => gateSource({ timeline, label: "needs-human", head: HEAD, isLanesApp }).source; + +test("a hold on an older head a person lifted by unlabeling leaves a later lane escalation workable", () => { + const timeline = [ + hold(OLD, "2026-10-18T08:00:00Z"), + label("unlabeled", OWNER, "2026-10-18T09:00:00Z"), + label("labeled", APP, "2026-10-18T10:00:00Z"), + escalation(), + ]; + assert.equal(source(timeline), "lane-escalation"); +}); + +test("a hold on the current head holds even after a person unlabeled it", () => { + const timeline = [ + hold(HEAD, "2026-10-18T08:00:00Z"), + label("unlabeled", OWNER, "2026-10-18T09:00:00Z"), + label("labeled", APP, "2026-10-18T10:00:00Z"), + escalation(), + ]; + assert.equal(source(timeline), "review-hold"); +}); + +test("a hold marker text a person wrote is not read", () => { + const timeline = [commented(OWNER, ``, "2026-10-18T10:01:00Z"), label("labeled", APP, "2026-10-18T10:00:00Z"), escalation()]; + assert.equal(source(timeline), "lane-escalation"); +}); + +test("another lane's marker on a lanes App comment since the label is unknown", () => { + const timeline = [label("labeled", APP, "2026-10-18T10:00:00Z"), escalation(), commented(APP, "\nRisk.", "2026-10-18T10:02:00Z")]; + assert.equal(source(timeline), "unknown"); +}); + +test("address-feedback's own round records since the escalation leave it workable", () => { + const timeline = [ + label("labeled", APP, "2026-10-18T10:00:00Z"), + escalation(), + commented(APP, "\n\nRound 1.", "2026-10-18T10:30:00Z"), + commented(APP, "\nRound 2.", "2026-10-18T11:30:00Z"), + ]; + assert.equal(source(timeline), "lane-escalation"); +}); + +test("an escalation more than a minute after the label is unknown", () => { + assert.equal(source([label("labeled", APP, "2026-10-18T10:00:00Z"), escalation("2026-10-18T10:05:00Z")]), "unknown"); +}); + +test("listLabelEvents maps GraphQL label events to timeline items across pages", async () => { + const pages = [ + { + nodes: [{ __typename: "LabeledEvent", createdAt: "2026-10-18T10:00:00Z", label: { name: "needs-human" }, actor: { __typename: "Bot", login: APP.login, databaseId: APP.id } }], + pageInfo: { hasNextPage: true, endCursor: "c1" }, + }, + { + nodes: [{ __typename: "UnlabeledEvent", createdAt: "2026-10-18T11:00:00Z", label: { name: "needs-human" }, actor: { __typename: "User", login: OWNER.login, databaseId: OWNER.id } }], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + ]; + const asked = []; + const graphql = async (query, variables) => { + asked.push(variables); + return { repository: { pullRequest: { timelineItems: pages[asked.length - 1] } } }; + }; + const events = await listLabelEvents({ graphql, repository: "o/r", number: 7 }); + assert.deepEqual(events, [ + label("labeled", APP, "2026-10-18T10:00:00Z"), + label("unlabeled", OWNER, "2026-10-18T11:00:00Z"), + ]); + assert.deepEqual(asked, [ + { owner: "o", name: "r", number: 7, after: null }, + { owner: "o", name: "r", number: 7, after: "c1" }, + ]); +}); diff --git a/.github/workflows/pr-run-activity-write.yml b/.github/workflows/pr-run-activity-write.yml index 1a1457095f..2b86999198 100644 --- a/.github/workflows/pr-run-activity-write.yml +++ b/.github/workflows/pr-run-activity-write.yml @@ -306,7 +306,8 @@ on: apply-patch with patch-prepare review-threads: the select job's fix-requested output. `true` leaves the human-gated label out of the holds, since that job kept only threads a trusted person with - write access asked the lane to fix. + write access asked the lane to fix, or a lane escalation put the + label on. required: false type: string default: "" @@ -1085,7 +1086,8 @@ jobs: # human-gated label is left out only for apply-patch under the # review-threads row, when the caller's select job kept nothing but # threads a trusted person with write access asked the lane to fix on a - # PR carrying it, unless labels-absent lists it. + # PR carrying it, or a lane escalation put it on, unless labels-absent + # lists it. - name: Read the hold labels from the base id: hold-label if: >- diff --git a/.github/workflows/pr-unblock.yml b/.github/workflows/pr-unblock.yml index a7f97468d1..4d76903091 100644 --- a/.github/workflows/pr-unblock.yml +++ b/.github/workflows/pr-unblock.yml @@ -413,7 +413,8 @@ jobs: patch-threads-attempt: ${{ needs.address-feedback-select.outputs.context-attempt }} patch-threads-sha256: ${{ needs.address-feedback-select.outputs.threads-sha256 }} # true drops the human-gated label from the holds: the select job kept - # only threads a trusted person with write access asked to fix. + # only threads a trusted person with write access asked to fix, or a + # lane escalation put the label on (gate-source.mjs). patch-fix-requested: ${{ needs.address-feedback-select.outputs.fix-requested }} reply-attempt: ${{ needs.address-feedback-subscription.result != 'skipped' && needs.address-feedback-subscription.outputs.scan-reply-attempt || needs.address-feedback-subscription.result == 'skipped' && needs.address-feedback.outputs.scan-reply-attempt || '' }} diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 019f057889..f659ed195c 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -155,7 +155,14 @@ PR, it carries no `do-not-merge`, both lanes' newest reviewed-head markers name the head, an unresolved thread exists that `address-feedback` would keep (left only by the reviewer bot, one the lane never answered where a fix request, below, follows the reviewer bot's finding, or answered by a trusted human after the lane escalated it; on a PR carrying the -human-gated label, only one whose answer is a fix request), the head has no +human-gated label, only one whose answer is a fix request, unless a lane escalation put the label +on: the lanes App added it last, its `work-items:escalation lane=pr-unblock` comment (pr-unblock +or fix-ci) followed within a minute, no other marker is on its comments since, and no review-lane +`claude-lane-hold` names the current head or is open, per +`.github/scripts/pr-unblock/gate-source.mjs`; anything ambiguous keeps the fix-request rule, and +the label stays on, and `clear-escalations.mjs` removes it only while no address-feedback round +record is on the lanes App's comments since it went on; to stop the lane working such a PR, a +person adds `do-not-merge`, since re-adding a label already on records no event), the head has no `pr-unblock / address-feedback` check run (one that started before the head's newest `pr-unblock / publish-feedback` check run stops counting when that run skipped `no-model-turn`: the round handled nothing, and the round cap still bounds the retries; a limit stop keeps its