Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/actions/resolve-config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ an `issues` event cannot be decided and fails the step.

The file at `output-path` holds `version`, `source` (`base-path`, `config-path`), `lane` (name,
stage, enabled, and every slot resolved), `selected` (the requested activity's slot, or null),
`loop` and `merge`. Every default is applied: `scope` `diff`, `enabled` true, `loop` 2 and 3,
`loop`, `limits` and `merge`. Every default is applied: `scope` `diff`, `enabled` true, `loop` 2 and 3,
the six `limits` (5, 1, 168, 5, 3, 3; [README, Limits](../../../docs/conventions/pr-pipeline/README.md#limits)),
`merge.rung` `off`, `stack-landing` `manual`. A skill slot carries `models`: its profile name and
that profile's `model`, `effort` (null when unset), `subagent-model` (`inherit` when unset) and
`max-turns`, from its activity's route, else `models.default-profile`, else, with no `models`
Expand Down
88 changes: 88 additions & 0 deletions .github/actions/resolve-config/dispatch-limits.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// pr-unblock-dispatch.yml's limits read through the real resolver: dispatch.mjs
// takes `limits` from the base's pr-pipeline.yaml, so a set
// `limits.limit-stops-per-head` changes which heads it dispatches again, and a
// config the resolver refuses fails the run before anything is dispatched.
// Lives here because it needs resolve-config's dependencies.
import assert from "node:assert/strict";
import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { test } from "node:test";
import { fileURLToPath } from "node:url";

import { main } from "../../scripts/pr-unblock/dispatch.mjs";

const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../../..");
const LIVE = readFileSync(join(ROOT, "docs/conventions/pr-pipeline.yaml"), "utf8");
const HEAD = "a".repeat(40);
// A resolve-conflicts run the usage limit stopped six hours ago with no stated
// reset: past the default five-hour fallback, so it no longer holds the head.
const STOPPED = {
name: "pr-unblock / resolve-conflicts",
status: "completed",
conclusion: "failure",
completed_at: "2026-10-17T06:00:00Z",
output: { title: "Failed", summary: "lane=pr-unblock activity=resolve-conflicts class=rate-limit\n\nThe activity step failed." },
};

function baseWith(text) {
const base = mkdtempSync(join(tmpdir(), "dispatch-limits-base-"));
for (const file of ["docs/conventions/pr-pipeline/pr-pipeline.schema.json", ".github/standards/github-actions-conventions/vocabulary.json"]) {
mkdirSync(dirname(join(base, file)), { recursive: true });
cpSync(join(ROOT, file), join(base, file));
}
writeFileSync(join(base, "docs/conventions/pr-pipeline.yaml"), text);
return base;
}

const api = (dispatched) => ({
listOpenPulls: async () => ({
nodes: [{ number: 40, isDraft: false, isCrossRepository: false, mergeable: "CONFLICTING", baseRefName: "main", headRefOid: HEAD }],
pageInfo: { hasNextPage: false, endCursor: null },
}),
getPulls: async () => [],
listCheckRuns: async () => [STOPPED],
listWorkflowRuns: async () => [],
dispatchWorkflow: async ({ inputs }) => {
dispatched.push(inputs["pr-number"]);
return { workflowRunId: 9040 };
},
});

async function run(text) {
const base = baseWith(text);
const output = join(base, "output");
writeFileSync(output, "");
const dispatched = [];
try {
const code = await main(
{ GITHUB_TOKEN: "t", GITHUB_REPOSITORY: "o/r", GITHUB_RUN_ID: "4242", GITHUB_OUTPUT: output, BASE_PATH: base },
{ api: api(dispatched), sleep: async () => {}, log: () => {}, now: Date.parse("2026-10-17T12:00:00Z") },
);
return { code, dispatched };
} finally {
rmSync(base, { recursive: true, force: true });
}
}

test("the base config's limits.limit-stops-per-head decides whether a reset limit stop still holds the head", async () => {
assert.deepEqual(await run(LIVE), { code: 0, dispatched: ["40"] });
assert.deepEqual(await run(`${LIVE}\nlimits:\n limit-stops-per-head: 1\n`), { code: 0, dispatched: [] });
});

test("a base config the resolver refuses fails the run before any dispatch", async () => {
const dispatched = [];
const base = baseWith(`${LIVE}\nlimits:\n limit-stops-per-head: 0\n`);
try {
await assert.rejects(
main(
{ GITHUB_TOKEN: "t", GITHUB_REPOSITORY: "o/r", GITHUB_RUN_ID: "4242", GITHUB_OUTPUT: join(base, "output"), BASE_PATH: base },
{ api: api(dispatched), sleep: async () => {}, log: () => {} },
),
/fails the schema/,
);
assert.deepEqual(dispatched, []);
} finally {
rmSync(base, { recursive: true, force: true });
}
});
9 changes: 9 additions & 0 deletions .github/actions/resolve-config/resolve-config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
modelSettings,
neededFacts,
resolve,
resolveLimits,
resolveRoute,
} from "./resolve.mjs";

Expand Down Expand Up @@ -264,6 +265,14 @@ export async function resolveFromBase({ env, github, sleep = wait, event, absent
return resolved;
}

// The base config's resolved `limits`, for a script that resolves no lane
// slot; a config the resolver refuses throws.
export function limitsFromBase(env) {
const basePath = env.BASE_PATH || DEFAULT_BASE_PATH;
const configPath = env.CONFIG_PATH || DEFAULT_CONFIG_PATH;
return resolveLimits(filesUnderBase(basePath, configPath), { configPath });
}

// A run that is no lane activity resolves only its model route.
function resolveRouteFromBase(env) {
if (env.LANE) {
Expand Down
19 changes: 19 additions & 0 deletions .github/actions/resolve-config/resolve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,24 @@ function resolveMerge(merge = {}, configPath) {
};
}

// The `limits` leaf with every default applied, in hours and counts; the lane
// scripts' copy of these defaults is
// .github/scripts/work-resolve-issue/limits.mjs.
const limitsOf = (limits = {}) => ({
"fallback-reset": limits["fallback-reset"] ?? 5,
"reset-clamp-min": limits["reset-clamp-min"] ?? 1,
"reset-clamp-max": limits["reset-clamp-max"] ?? 168,
"restarts-per-run": limits["restarts-per-run"] ?? 5,
"limit-stops-per-head": limits["limit-stops-per-head"] ?? 3,
"limit-stops-per-issue": limits["limit-stops-per-issue"] ?? 3,
});

// The resolved limits alone, after the same file checks as resolve(), for a
// run that resolves no lane slot (pr-unblock-dispatch.yml).
export function resolveLimits(files, { configPath }) {
return limitsOf(loadConfig(files, { configPath }).limits);
}

export function resolve(
files,
{ lane, activity, configPath, basePath, facts },
Expand All @@ -735,6 +753,7 @@ export function resolve(
"no-progress": doc.loop?.["no-progress"] ?? 3,
"dependency-review-rounds": doc.loop?.["dependency-review-rounds"] ?? 2,
},
limits: limitsOf(doc.limits),
merge: resolveMerge(doc.merge, configPath),
};
}
32 changes: 32 additions & 0 deletions .github/actions/resolve-config/resolve.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,38 @@ test("the denied config path is the one the run read", () => {

// Defaults

// The values the lanes must keep with no limits leaf, as the request for the
// leaf states them: a 5-hour fallback reset, a 1-hour to 7-day clamp, 5
// restarts per sweep, 3 limit stops per head and per issue.
const LIMITS = {
"fallback-reset": 5,
"reset-clamp-min": 1,
"reset-clamp-max": 168,
"restarts-per-run": 5,
"limit-stops-per-head": 3,
"limit-stops-per-issue": 3,
};

test("limits: a config with no limits leaf resolves today's six values", () => {
assert.deepEqual(run(fixture("valid.yaml")).limits, LIMITS);
});

test("limits: a set leaf overrides its default and leaves the others", () => {
const config = edited((doc) => {
doc.limits = { "fallback-reset": 2, "limit-stops-per-issue": 4 };
});
assert.deepEqual(run(config).limits, { ...LIMITS, "fallback-reset": 2, "limit-stops-per-issue": 4 });
});

test("limits: an unknown key, a value that is not an integer, or one out of its range is rejected", () => {
for (const limits of [{ "fallback-resets": 5 }, { "fallback-reset": 0 }, { "restarts-per-run": -1 }, { "reset-clamp-max": 1.5 }, { "limit-stops-per-head": "3" }, { "limit-stops-per-head": 11 }, { "limit-stops-per-issue": 11 }, { "restarts-per-run": 51 }, { "fallback-reset": 721 }, { "reset-clamp-max": 721 }]) {
const config = edited((doc) => {
doc.limits = limits;
});
assertRejected(() => run(config), "invalid-config");
}
});

test("defaults are materialized: loop 2/3/2, merge off and manual, slot scope diff and enabled", () => {
const resolved = run(fixture("valid.yaml"), { activity: "run-tests" });
assert.deepEqual(resolved.loop, { "review-rounds": 2, "no-progress": 3, "dependency-review-rounds": 2 });
Expand Down
18 changes: 16 additions & 2 deletions .github/scripts/pr-unblock/dispatch.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export async function dispatchConflicting({
unknownRetries = DEFAULTS.unknownRetries,
unknownDelayMs = DEFAULTS.unknownDelayMs,
now = Date.now(),
limits,
}) {
const listed = await listAll(api, base);
const candidates = listed.filter((node) => isCandidate(node, base));
Expand Down Expand Up @@ -137,7 +138,7 @@ export async function dispatchConflicting({
let handled;
try {
const existing = await api.listCheckRuns({ sha: node.headRefOid, checkName, appId });
handled = anyHandles(existing, { now });
handled = anyHandles(existing, { now, limits });
} catch (error) {
failed += 1;
log(
Expand Down Expand Up @@ -297,6 +298,16 @@ export function createGitHubApi({
};
}

// The base config's resolved limits, through the base checkout's
// resolve-config; the workflow installs its dependencies before this runs. A
// config the resolver refuses throws, so nothing is dispatched.
export async function loadBaseLimits(env) {
const { limitsFromBase } = await import(
new URL("../../actions/resolve-config/resolve-config.mjs", import.meta.url).href
);
return limitsFromBase({ BASE_PATH: env.BASE_PATH || ".base" });
}

// Writes the dispatched run ids, lowest PR first, as the step output run-ids,
// which the workflow records as an artifact on this run.
export async function main(
Expand All @@ -305,6 +316,8 @@ export async function main(
api,
sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)),
log = (line) => console.log(line),
loadLimits = loadBaseLimits,
now,
} = {},
) {
for (const key of ["GITHUB_TOKEN", "GITHUB_REPOSITORY", "GITHUB_RUN_ID", "GITHUB_OUTPUT"]) {
Expand All @@ -320,7 +333,8 @@ export async function main(
apiUrl: env.GITHUB_API_URL ?? "https://api.github.com",
graphqlUrl: env.GITHUB_GRAPHQL_URL ?? "https://api.github.com/graphql",
});
const conflicts = await dispatchConflicting({ api: client, runId: env.GITHUB_RUN_ID, sleep, log });
const limits = await loadLimits(env);
const conflicts = await dispatchConflicting({ api: client, runId: env.GITHUB_RUN_ID, sleep, log, limits, now });
const reviews = await dispatchDependencyReviews({
api: client,
runId: env.GITHUB_RUN_ID,
Expand Down
15 changes: 14 additions & 1 deletion .github/scripts/pr-unblock/dispatch.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { tmpdir } from "node:os";
import { join } from "node:path";
import { test } from "node:test";

import { LIMIT_DEFAULTS } from "../work-resolve-issue/limits.mjs";
import { createGitHubApi, dispatchConflicting, main } from "./dispatch.mjs";

const CHECK_NAME = "pr-unblock / resolve-conflicts";
Expand Down Expand Up @@ -291,6 +292,18 @@ test("a limit-stopped run counts as handled only until its limit resets", async
assert.deepEqual(dispatchedNumbers(api), ["21", "23"]);
});

test("the resolved limits set how many reset limit stops hold a head", async () => {
const now = Date.parse("2026-10-17T12:00:00Z");
const stopped = pr(28);
const apiOptions = { pages: [[stopped]], checked: new Map([[stopped.headRefOid, [failedRun("2026-10-17T06:00:00Z")]]]) };
const defaults = harness(apiOptions, { now });
await defaults.run();
assert.deepEqual(dispatchedNumbers(defaults.api), ["28"]);
const one = harness(apiOptions, { now, limits: { ...LIMIT_DEFAULTS, "limit-stops-per-head": 1 } });
await one.run();
assert.deepEqual(dispatchedNumbers(one.api), []);
});

test("skips a PR while a resolve-conflicts run for it is queued or going", async () => {
for (const status of ["queued", "in_progress", "waiting", "pending"]) {
const { api, lines, run } = harness({
Expand Down Expand Up @@ -471,7 +484,7 @@ async function runMain(apiOptions) {
GITHUB_RUN_ID: "4242",
GITHUB_OUTPUT: output,
},
{ api: fakeApi(apiOptions), sleep: async () => {}, log: () => {} },
{ api: fakeApi(apiOptions), sleep: async () => {}, log: () => {}, loadLimits: async () => LIMIT_DEFAULTS },
);
return { code, output: readFileSync(output, "utf8") };
} finally {
Expand Down
8 changes: 4 additions & 4 deletions .github/scripts/pr-unblock/feedback-dispatch.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,6 @@ export async function dispatchFeedback({
if (!hasWorkableThread(threads, { reviewerId: settings.reviewerId, appId: appSenderId, trustedHumans })) {
return skip(`#${number} has no unresolved thread left by the reviewer bot alone or answered by a trusted person`);
}
const existing = await api.listCheckRuns({ sha: head, checkName: settings.checkName, appId: settings.appId });
if (anyHandles(existing, { now })) {
return skip(`#${number}: head ${head} already has a ${settings.checkName} check run`);
}
// Both review lanes finish on one head, and their dispatchers queue in one
// concurrency group. The first one's run posts no check until it ends, so
// the second skips while a run of this activity for this PR, created after
Expand All @@ -184,6 +180,10 @@ export async function dispatchFeedback({
if (resolved.selected?.applies !== true) {
return skip(`#${number}: ${settings.activity} does not apply (${resolved.selected?.["skip-reason"] ?? "unknown"})`);
}
const existing = await api.listCheckRuns({ sha: head, checkName: settings.checkName, appId: settings.appId });
if (anyHandles(existing, { now, limits: resolved.limits })) {
return skip(`#${number}: head ${head} already has a ${settings.checkName} check run`);
}
const { workflowRunId } = await api.dispatchWorkflow({
workflow: settings.workflow,
ref: `refs/heads/${settings.base}`,
Expand Down
35 changes: 16 additions & 19 deletions .github/scripts/pr-unblock/limit-stop.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// dispatch.mjs and feedback-dispatch.mjs. A check run handles the head unless
// it is the gate's completed neutral skip for an untrusted trigger, or a run
// the usage limit stopped whose limit has reset: the lane's next trigger then
// dispatches it again, at most MAX_LIMIT_STOPS times on one head.
// dispatches it again, at most `limits.limit-stops-per-head` times on one head.
//
// A limit stop is read only from a failed run's lane record, which
// report-check-run writes from runner-computed outputs. The record is the first
Expand All @@ -11,16 +11,14 @@
// max-turns, models and profile in that order, each optional. A failed run with no such line is
// no limit stop.

// When a limit stop names no `resets=` time, it is taken to reset this long
// after the run ended: the shortest plan limit is a five-hour session window
// (https://support.claude.com/en/articles/11145838).
// Hard-coded until pr-pipeline.yaml has a limits.fallback-reset leaf:
// https://github.com/melodic-software/claude-code-plugins/issues/7764
export const FALLBACK_RESET_MS = 5 * 3_600_000;
const HOUR_MS = 3_600_000;
// Limit stops on one head after which the dispatchers stop re-dispatching it
// (judgment); the next push makes a new head.
export const MAX_LIMIT_STOPS = 3;
// `limits` is pr-pipeline.yaml's resolved limits leaf. When a limit stop names
// no `resets=` time, it is taken to reset `fallback-reset` hours after the run
// ended: the shortest plan limit is a five-hour session window
// (https://support.claude.com/en/articles/11145838). After
// `limit-stops-per-head` stops the dispatchers stop re-dispatching the head;
// the next push makes a new one.
import { LIMIT_DEFAULTS, clampReset } from "../work-resolve-issue/limits.mjs";

const ISO = String.raw`\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(?::\d{2}(?:\.\d+)?)?Z`;
const NAME = "[a-z][a-z0-9]*(?:-[a-z0-9]+)*";
const MODEL = String.raw`[A-Za-z0-9][A-Za-z0-9._[\]-]{0,63}`;
Expand All @@ -42,8 +40,8 @@ function summaryRecord(run) {

// When the limit that stopped the failed `run` resets (epoch ms), or null when
// its lane record does not mark it a limit stop. The stated reset is held
// between one hour and seven days after the run ended.
export function limitResetsAt(run) {
// within the limits' clamp after the run ended.
export function limitResetsAt(run, limits = LIMIT_DEFAULTS) {
const ended = Date.parse(run.completed_at ?? "");
if (!Number.isFinite(ended) || run.conclusion !== "failure") {
return null;
Expand All @@ -52,8 +50,7 @@ export function limitResetsAt(run) {
if (summary?.failureClass !== "rate-limit") {
return null;
}
const resets = summary.resets ? Date.parse(summary.resets) : ended + FALLBACK_RESET_MS;
return Math.min(Math.max(resets, ended + HOUR_MS), ended + 7 * 24 * HOUR_MS);
return clampReset(summary.resets ? Date.parse(summary.resets) : null, ended, limits);
}

const untrustedSkip = (run) =>
Expand All @@ -62,8 +59,8 @@ const untrustedSkip = (run) =>
String(run.output?.title ?? "").startsWith("Skipped: untrusted-trigger");

// Whether `runs` handle the head: any run that is not an untrusted skip or a
// reset limit stop does, and so do MAX_LIMIT_STOPS limit stops.
export function anyHandles(runs, { now }) {
// reset limit stop does, and so do `limit-stops-per-head` limit stops.
export function anyHandles(runs, { now, limits = LIMIT_DEFAULTS }) {
let stops = 0;
for (const run of runs) {
if (untrustedSkip(run)) {
Expand All @@ -72,11 +69,11 @@ export function anyHandles(runs, { now }) {
if (run.status !== "completed" || run.conclusion !== "failure") {
return true;
}
const resets = limitResetsAt(run);
const resets = limitResetsAt(run, limits);
if (resets === null || now < resets) {
return true;
}
stops += 1;
}
return stops >= MAX_LIMIT_STOPS;
return stops >= limits["limit-stops-per-head"];
}
Loading
Loading