diff --git a/.github/actions/resolve-config/README.md b/.github/actions/resolve-config/README.md index 632dc99b61..807a49ab18 100644 --- a/.github/actions/resolve-config/README.md +++ b/.github/actions/resolve-config/README.md @@ -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` diff --git a/.github/actions/resolve-config/dispatch-limits.test.mjs b/.github/actions/resolve-config/dispatch-limits.test.mjs new file mode 100644 index 0000000000..88c4a73927 --- /dev/null +++ b/.github/actions/resolve-config/dispatch-limits.test.mjs @@ -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 }); + } +}); diff --git a/.github/actions/resolve-config/resolve-config.mjs b/.github/actions/resolve-config/resolve-config.mjs index 5bccba77c9..374e61c0bb 100644 --- a/.github/actions/resolve-config/resolve-config.mjs +++ b/.github/actions/resolve-config/resolve-config.mjs @@ -20,6 +20,7 @@ import { modelSettings, neededFacts, resolve, + resolveLimits, resolveRoute, } from "./resolve.mjs"; @@ -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) { diff --git a/.github/actions/resolve-config/resolve.mjs b/.github/actions/resolve-config/resolve.mjs index 83f2ed649f..8e95969b83 100644 --- a/.github/actions/resolve-config/resolve.mjs +++ b/.github/actions/resolve-config/resolve.mjs @@ -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 }, @@ -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), }; } diff --git a/.github/actions/resolve-config/resolve.test.mjs b/.github/actions/resolve-config/resolve.test.mjs index 20c1200132..be4c84c2d6 100644 --- a/.github/actions/resolve-config/resolve.test.mjs +++ b/.github/actions/resolve-config/resolve.test.mjs @@ -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 }); diff --git a/.github/scripts/pr-unblock/dispatch.mjs b/.github/scripts/pr-unblock/dispatch.mjs index 6d0bb3ac66..428bca2fb7 100644 --- a/.github/scripts/pr-unblock/dispatch.mjs +++ b/.github/scripts/pr-unblock/dispatch.mjs @@ -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)); @@ -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( @@ -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( @@ -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"]) { @@ -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, diff --git a/.github/scripts/pr-unblock/dispatch.test.mjs b/.github/scripts/pr-unblock/dispatch.test.mjs index 9b79f95392..3fece59387 100644 --- a/.github/scripts/pr-unblock/dispatch.test.mjs +++ b/.github/scripts/pr-unblock/dispatch.test.mjs @@ -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"; @@ -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({ @@ -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 { diff --git a/.github/scripts/pr-unblock/feedback-dispatch.mjs b/.github/scripts/pr-unblock/feedback-dispatch.mjs index e8a991f9eb..8a83a84466 100644 --- a/.github/scripts/pr-unblock/feedback-dispatch.mjs +++ b/.github/scripts/pr-unblock/feedback-dispatch.mjs @@ -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 @@ -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}`, diff --git a/.github/scripts/pr-unblock/limit-stop.mjs b/.github/scripts/pr-unblock/limit-stop.mjs index e59d127332..ace628e53a 100644 --- a/.github/scripts/pr-unblock/limit-stop.mjs +++ b/.github/scripts/pr-unblock/limit-stop.mjs @@ -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 @@ -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}`; @@ -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; @@ -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) => @@ -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)) { @@ -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"]; } diff --git a/.github/scripts/pr-unblock/limit-stop.test.mjs b/.github/scripts/pr-unblock/limit-stop.test.mjs index 693f09bcf0..fbb7e133a0 100644 --- a/.github/scripts/pr-unblock/limit-stop.test.mjs +++ b/.github/scripts/pr-unblock/limit-stop.test.mjs @@ -75,6 +75,28 @@ test("a limit stop handles the head until its reset", () => { assert.equal(anyHandles([posted(LIMIT_LINE)], { now: Date.parse("2026-10-17T13:00:00Z") }), false); }); +// A resolved `limits` leaf set away from every default (hours and counts). +const SET = { + "fallback-reset": 2, + "reset-clamp-min": 3, + "reset-clamp-max": 24, + "restarts-per-run": 5, + "limit-stops-per-head": 1, + "limit-stops-per-issue": 3, +}; + +test("the resolved limits set the fallback reset and the clamp", () => { + assert.equal(limitResetsAt(posted(LIMIT), SET), ended + 3 * HOUR); + assert.equal(limitResetsAt(posted(LIMIT), { ...SET, "reset-clamp-min": 1 }), ended + 2 * HOUR); + assert.equal(limitResetsAt(posted(`${LIMIT} resets=2027-01-01T00:00:00Z`), SET), ended + 24 * HOUR); +}); + +test("the resolved limits set how many reset limit stops handle a head", () => { + const now = ended + 30 * HOUR; + assert.equal(anyHandles([posted(LIMIT, { id: 1 })], { now }), false); + assert.equal(anyHandles([posted(LIMIT, { id: 1 })], { now, limits: SET }), true); +}); + test("a failed run with no summary record handles the head, however long ago it ended", () => { assert.equal(anyHandles([run()], { now: ended + 30 * 24 * HOUR }), true); assert.equal(anyHandles([posted(LIMIT, { id: 1 }), run(2)], { now: ended + 6 * HOUR }), true); diff --git a/.github/scripts/work-resolve-issue/limits.mjs b/.github/scripts/work-resolve-issue/limits.mjs new file mode 100644 index 0000000000..6f376a2fcf --- /dev/null +++ b/.github/scripts/work-resolve-issue/limits.mjs @@ -0,0 +1,44 @@ +// The `limits` leaf of pr-pipeline.yaml (pr-pipeline.schema.json) as the lane +// scripts read it from resolve-config's output: its defaults, in hours and +// counts, and the reset clamp the lanes share. It imports no package and sits +// beside report-outcome.mjs, which runs from a copy of this directory alone. +import { readFileSync } from "node:fs"; + +const HOUR_MS = 3_600_000; + +export const LIMIT_DEFAULTS = Object.freeze({ + "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, +}); + +// Every leaf, each set positive integer over its default. +export const resolveLimits = (limits) => + Object.fromEntries( + Object.entries(LIMIT_DEFAULTS).map(([key, value]) => { + const set = limits?.[key]; + return [key, Number.isInteger(set) && set > 0 ? set : value]; + }), + ); + +// The limits in resolve-config's output file; the defaults when it is absent +// or unreadable. +export function readLimits(path) { + try { + return resolveLimits(JSON.parse(readFileSync(path, "utf8"))?.limits); + } catch { + return resolveLimits(undefined); + } +} + +// When a limit stop at `from` may restart: its stated reset (else the +// fallback), held between the clamp's bounds after the stop, so a reset in the +// past cannot start a tight restart loop and one far off cannot park the work. +export const clampReset = (resets, from, limits = LIMIT_DEFAULTS) => + Math.min( + Math.max(resets ?? from + limits["fallback-reset"] * HOUR_MS, from + limits["reset-clamp-min"] * HOUR_MS), + from + limits["reset-clamp-max"] * HOUR_MS, + ); diff --git a/.github/scripts/work-resolve-issue/report-outcome.mjs b/.github/scripts/work-resolve-issue/report-outcome.mjs index 7e66c17ae5..0937a55dfd 100755 --- a/.github/scripts/work-resolve-issue/report-outcome.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.mjs @@ -53,7 +53,8 @@ // (runner-written issue, branch and tip; model output) // TRACKER_CONFIG base .work-item-tracker.json, for the human-gated label // RESOLVED_CONFIG resolve-config's output for this run (default -// $RUNNER_TEMP/resolved-config.json), for the denied paths +// $RUNNER_TEMP/resolved-config.json), for the denied +// paths and the limits // LANES_BOT_LOGIN the lanes App bot login, the only author of an open PR // a rerun reuses (default melodic-automation-lanes[bot]) // TRUSTED_ACTORS_PATH --find-prior: trusted-actors.json from the default @@ -65,6 +66,8 @@ import { join, posix } from "node:path"; import process from "node:process"; import { pathToFileURL } from "node:url"; +import { LIMIT_DEFAULTS, clampReset, readLimits } from "./limits.mjs"; + export const HOLD_LINE = "Hold: a human merges this"; const EXITS = new Set(["done", "pushback", "stopped"]); // The subject allows no `#`, `/` or `GH-`, so the squash commit title @@ -119,20 +122,11 @@ const MAX_ITEM = 300; // hide a hold path. const COMPARE_FILES_CAP = 300; -// When a limit stop names no reset time, it is taken to reset this long after -// the run: 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; - -// When a limit stop at `from` may restart: its stated reset (else the -// fallback), held between one hour and seven days after the stop, so a reset -// in the past cannot start a tight restart loop and one far off cannot park -// the issue. -export const clampReset = (resets, from) => - Math.min(Math.max(resets ?? from + FALLBACK_RESET_MS, from + HOUR_MS), from + 7 * 24 * HOUR_MS); +// A limit stop with no reset time is taken to reset `limits.fallback-reset` +// hours after the run: the shortest plan limit is a five-hour session window +// (https://support.claude.com/en/articles/11145838). work-unblock takes +// clampReset from here. +export { clampReset }; const ISO = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2}(\.\d+)?)?Z$/; const MARKER = /^/; @@ -450,6 +444,7 @@ export function decide({ humanLabel = "needs-human", runUrl = "", limit = null, + limits = LIMIT_DEFAULTS, now = Date.now(), runId = "", }) { @@ -458,7 +453,7 @@ export function decide({ // code the run executed can forge the record, so it must not hide a stop // or pushback the skill reported from the person it goes to. if (limit && outcome.unread) { - const resets = new Date(clampReset(limit.resets, now)).toISOString().replace(/\.\d+Z$/, "Z"); + const resets = new Date(clampReset(limit.resets, now, limits)).toISOString().replace(/\.\d+Z$/, "Z"); return limitedPlan({ issueNumber, branch, resets, runId, runUrl }); } if (outcome.exit === "done" && !branch) { @@ -850,7 +845,8 @@ export async function main({ env = process.env, fetch = globalThis.fetch, now = ); }; const changedFiles = outcome?.exit === "done" && branch ? await compare() : []; - const holdPaths = readHoldPaths(env.RESOLVED_CONFIG || join(env.RUNNER_TEMP ?? "", "resolved-config.json")); + const resolvedPath = env.RESOLVED_CONFIG || join(env.RUNNER_TEMP ?? "", "resolved-config.json"); + const holdPaths = readHoldPaths(resolvedPath); // A reuse whose outcome cannot be read never takes the limit record: the // skipped run hit no limit, and a limited marker would loop restarts. Its // file list is only text, so a failed compare never blocks the PR. @@ -877,6 +873,7 @@ export async function main({ env = process.env, fetch = globalThis.fetch, now = humanLabel: humanLabelFrom(env.TRACKER_CONFIG ?? ".base/.work-item-tracker.json"), runUrl, limit, + limits: readLimits(resolvedPath), now, runId: /^[1-9][0-9]{0,14}$/.test(env.GITHUB_RUN_ID ?? "") ? env.GITHUB_RUN_ID : "", }); diff --git a/.github/scripts/work-resolve-issue/report-outcome.test.mjs b/.github/scripts/work-resolve-issue/report-outcome.test.mjs index 3d2ea534da..09d3b681a8 100644 --- a/.github/scripts/work-resolve-issue/report-outcome.test.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.test.mjs @@ -783,6 +783,25 @@ test("main on a limit stop comments the marker and touches no label", async () = }); }); +test("main takes the fallback reset and clamp from the resolved config's limits", async () => { + const resolved = JSON.stringify({ limits: { "fallback-reset": 2, "reset-clamp-min": 1, "reset-clamp-max": 3 } }); + for (const [lane, expected] of [ + [JSON.stringify({ branch: "", class: "rate-limit" }), "2026-10-17T14:00:00Z"], + [LIMITED_LANE, "2026-10-17T15:00:00Z"], + ]) { + await withAsyncDir({ "lane.json": lane, "resolved.json": resolved }, async (dir) => { + const { fetch, calls } = fakeFetch([ + ["GET", /\/issues\/77$/, ...ISSUE.slice(1)], + ["GET", /\/issues\/77\/comments$/, 200, []], + ["POST", /\/issues\/77\/comments$/, 201, {}], + ]); + assert.equal(await main({ env: { ...env(dir), RESOLVED_CONFIG: join(dir, "resolved.json") }, fetch, now: LIMIT_NOW }), 0); + const posted = calls.filter((c) => c.method === "POST"); + assert.ok(posted[0].body.body.startsWith(``), posted[0].body.body); + }); + } +}); + test("main posts the limited marker once per run", async () => { await withDir({ "lane.json": LIMITED_LANE }, async (dir) => { const marker = ""; diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index e6b27fb486..ca530f1b62 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -19,7 +19,7 @@ // issue whose newest lanes-App outcome marker is `limited`, past its reset, // with no agent-ready label added since (so no lane run started after the // stop), and no hold label gets agent-ready removed and -// added again by the App, which starts the lane. At most RESTARTS_PER_RUN +// added again by the App, which starts the lane. At most limits.restarts-per-run // issues per run, longest-waiting first. // // It re-fires the same way an open agent-ready issue whose newest @@ -40,7 +40,7 @@ // again re-runs it. Open, it waits: report-outcome put needs-human on the // issue of a Refs PR, and this lifts that hold only when that PR merges, // only while its refs-hold marker and the App's needs-human label are the -// newest. Each hand-off also counts against RESTARTS_PER_RUN. +// newest. Each hand-off also counts against limits.restarts-per-run. // // Environment: // GH_TOKEN the lane App token (issues: write) @@ -61,6 +61,8 @@ // and actor of a restart this trusts (default // melodic-automation-lanes[bot]) // GITHUB_API_URL +// RESOLVED_CONFIG resolve-config's output (default +// $RUNNER_TEMP/resolved-config.json), for the limits import { readFileSync } from "node:fs"; import process from "node:process"; import { pathToFileURL } from "node:url"; @@ -68,6 +70,7 @@ import { pathToFileURL } from "node:url"; import { createGitHub, loadTrustedList, paginate } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { writeOutputs } from "../../actions/check-kill-switch/check-kill-switch.mjs"; import { clampReset, isLaneBranch, parseOutcomeMarker, refsHoldMarker } from "../work-resolve-issue/report-outcome.mjs"; +import { readLimits } from "../work-resolve-issue/limits.mjs"; const LABEL = "agent-ready"; const HUMAN_LABEL = "needs-human"; @@ -82,12 +85,6 @@ const BLOB = /^[0-9a-f]{40}$/; const REFIRED = /^/; const handedMarker = (pr) => ``; const DEFAULT_BOT = "melodic-automation-lanes[bot]"; -// Hard-coded until pr-pipeline.yaml has a limits.restarts-per-run leaf: -// https://github.com/melodic-software/claude-code-plugins/issues/7764 -export const RESTARTS_PER_RUN = 5; -// Limit stops since a person last added agent-ready before the issue goes to -// a person instead of another restart (judgment). -export const MAX_LIMIT_STOPS = 3; // A person decides these; the lane never starts on them. const HOLDS = new Set(["needs-owner-decision", "status: needs-decision", "needs-human"]); const NUMBER = /^[1-9][0-9]{0,9}$/; @@ -302,7 +299,7 @@ async function labelUnblocked({ github, repo, closed, sweep, humans, bot }) { return failed; } -// The hand-off to a person after MAX_LIMIT_STOPS limit stops. Its marker lets +// The hand-off to a person after limits.limit-stops-per-issue limit stops. Its marker lets // a later sweep see the comment was posted. const ESCALATED = ""; export const escalationComment = (stops) => @@ -458,7 +455,7 @@ async function refireRefusal({ github, actions, repo, number, lastAdd, addedAt, // Why an agent-ready issue is not restarted now (after a limit stop) or // re-fired (after a failed run), or null with the time it has waited since. // Every fact is read fresh from the API. -async function restartRefusal({ github, actions, repo, issue, bot, humans, now, workflowKey }) { +async function restartRefusal({ github, actions, repo, issue, bot, humans, now, workflowKey, limits }) { const base = `/repos/${repo}/issues/${issue.number}`; const labels = (issue.labels ?? []).map((label) => (typeof label === "string" ? label : label.name)); const holds = labels.map((label) => String(label).toLowerCase()).filter((label) => HOLDS.has(label)); @@ -514,14 +511,14 @@ async function restartRefusal({ github, actions, repo, issue, bot, humans, now, .filter((event) => isLabel(event, "labeled") && event.actor?.login !== bot) .reduce((latest, event) => Math.max(latest, Date.parse(event.created_at)), Number.NEGATIVE_INFINITY); const stops = markers.filter((item) => item.outcome === "limited" && item.at > since).length; - if (stops >= MAX_LIMIT_STOPS) { + if (stops >= limits["limit-stops-per-issue"]) { const explained = comments.some( (comment) => comment.user?.login === bot && String(comment.body ?? "").startsWith(ESCALATED) && Date.parse(comment.created_at) > since, ); return { reason: `${stops} limit stops since a person last added ${LABEL}`, escalate: stops, explained }; } - const resets = clampReset(marker.resets, marker.at); + const resets = clampReset(marker.resets, marker.at, limits); if (now < resets) { return { reason: `waiting for the limit to reset at ${new Date(resets).toISOString()}` }; } @@ -530,7 +527,7 @@ async function restartRefusal({ github, actions, repo, issue, bot, humans, now, // Restarts, re-fires or hands off up to `cap` issues, longest-waiting first. // Returns how many reads or writes failed. -async function restartLimited({ github, actions, repo, bot, humans, now, cap = RESTARTS_PER_RUN }) { +async function restartLimited({ github, actions, repo, bot, humans, now, limits, cap = limits["restarts-per-run"] }) { let failed = 0; const ready = []; // Main's blob SHAs of the lane workflow files, read once per run. @@ -567,6 +564,7 @@ async function restartLimited({ github, actions, repo, bot, humans, now, cap = R humans, now, workflowKey, + limits, }); const { reason, escalate, explained } = verdict; if (escalate) { @@ -682,7 +680,8 @@ export async function main({ env = process.env, github, actions, now = Date.now( let failed = await labelUnblocked({ github, repo, closed, sweep, humans, bot }); if (sweep) { actions ??= env.ACTIONS_TOKEN ? createGitHub({ token: env.ACTIONS_TOKEN, apiUrl: env.GITHUB_API_URL || undefined }) : null; - failed += await restartLimited({ github, actions, repo, bot, humans, now }); + const limits = readLimits(env.RESOLVED_CONFIG || `${env.RUNNER_TEMP ?? ""}/resolved-config.json`); + failed += await restartLimited({ github, actions, repo, bot, humans, now, limits }); } return failed === 0 ? 0 : 1; } catch (error) { diff --git a/.github/scripts/work-unblock/label-unblocked.test.mjs b/.github/scripts/work-unblock/label-unblocked.test.mjs index 5361e28907..2012397f29 100644 --- a/.github/scripts/work-unblock/label-unblocked.test.mjs +++ b/.github/scripts/work-unblock/label-unblocked.test.mjs @@ -871,6 +871,38 @@ test("a third limit stop since a person added agent-ready adds needs-human with assert.deepEqual(writes(reset.calls), restarted(30)); }); +// The sweep reads the resolved config's `limits` (hours and counts) from the +// file resolve-config wrote. +const withLimits = (limits) => { + const path = join(mkdtempSync(join(tmpdir(), "work-unblock-limits-")), "resolved-config.json"); + writeFileSync(path, JSON.stringify({ limits })); + return { RESOLVED_CONFIG: path }; +}; + +test("the resolved limits set the sweep's restarts per run, limit stops per issue and fallback reset", async () => { + const numbers = [31, 32, 33, 34]; + const many = world({ + openReady: numbers.map((n) => ready(n)), + comments: Object.fromEntries(numbers.map((n) => [n, [note(), limitedNote(`2026-10-16T0${n - 30}:00:00Z`, "2026-10-16T00:30:00Z")]])), + events: Object.fromEntries(numbers.map((n) => [n, [added("2026-10-16T00:00:00Z")]])), + }); + assert.equal(await sweep({ github: many.github }, withLimits({ "restarts-per-run": 2 })), 0); + assert.deepEqual(writes(many.calls), [31, 32].flatMap(restarted)); + + const stops = [limitedNote("2026-10-16T03:00:00Z", "2026-10-16T02:30:00Z"), limitedNote("2026-10-16T11:00:00Z", "2026-10-16T06:00:00Z")]; + const second = stopped({ comments: [note(), ...stops] }); + assert.equal(await sweep({ github: second.github }, withLimits({ "limit-stops-per-issue": 2 })), 0); + assert.deepEqual(writes(second.calls).slice(1), [`POST /repos/${REPO}/issues/30/labels {"labels":["needs-human"]}`]); + + // No stated reset, stopped at 06:00: five hours has passed at 12:00, seven has not. + const bare = stopped({ resets: "" }); + assert.equal(await sweep({ github: bare.github }), 0); + assert.deepEqual(writes(bare.calls), restarted(30)); + const later = stopped({ resets: "" }); + assert.equal(await sweep({ github: later.github }, withLimits({ "fallback-reset": 7 })), 0); + assert.deepEqual(writes(later.calls), []); +}); + // After the DELETE, a failed re-add is tried once more; a second failure is an // ::error naming the issue and fails the run. test("a failed re-add after the removal is retried once, then reported", async (t) => { diff --git a/.github/workflows/pr-run-activity-write.yml b/.github/workflows/pr-run-activity-write.yml index 41a71f750c..4c15b4cc53 100644 --- a/.github/workflows/pr-run-activity-write.yml +++ b/.github/workflows/pr-run-activity-write.yml @@ -278,8 +278,10 @@ jobs: reuse-branch: ${{ steps.issue-prior.outputs.reuse-branch }} # The lane record, from steps that read the execution file after the # skill. A forged limit stop can only restart the lane after a reset - # limit-stop.mjs and report-outcome clamp to at least an hour away, at - # most MAX_LIMIT_STOPS times. + # limit-stop.mjs and report-outcome clamp to at least + # `limits.reset-clamp-min` hours away, at + # most `limits.limit-stops-per-head` (a PR) or + # `limits.limit-stops-per-issue` (an issue) times. lane-class: ${{ steps.lane-record.outputs.failure-class }} lane-resets: ${{ steps.lane-record.outputs.resets-at }} lane-turns: ${{ steps.lane-record.outcome == 'success' && steps.act-turns.outputs.turns || '' }} diff --git a/.github/workflows/pr-unblock-dispatch.yml b/.github/workflows/pr-unblock-dispatch.yml index b1f9a276dc..49bb30ba6e 100644 --- a/.github/workflows/pr-unblock-dispatch.yml +++ b/.github/workflows/pr-unblock-dispatch.yml @@ -96,6 +96,14 @@ jobs: echo "::error::${REASON:-untrusted-actor}: this run was not started by a listed trusted human" exit 1 + # dispatch.mjs reads the base config's `limits` through the base's + # resolve-config, which needs its dependencies. + - name: Install resolve-config's dependencies + if: >- + steps.kill.outputs.proceed == 'true' + && (steps.gate.outcome == 'skipped' || steps.gate.outputs.proceed == 'true') + run: npm ci --ignore-scripts --omit=dev --prefix .base/.github/actions/resolve-config + - name: Dispatch pr-unblock for conflicting PRs and review-tier Dependabot PRs id: dispatch if: >- diff --git a/docs/conventions/pr-pipeline/CHANGELOG.md b/docs/conventions/pr-pipeline/CHANGELOG.md index ebf0575e60..66f027bc1a 100644 --- a/docs/conventions/pr-pipeline/CHANGELOG.md +++ b/docs/conventions/pr-pipeline/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog for the PR pipeline convention +## 2.8.0 - 2026-10-17 + +An optional `limits` block sets the limit-stop restart values: `fallback-reset`, +`reset-clamp-min` and `reset-clamp-max` (hours), `restarts-per-run`, `limit-stops-per-head` and +`limit-stops-per-issue` ([README.md, Limits](README.md#limits)). Each defaults to the value the +lanes used before, and resolve-config emits all six. `version` stays 2. + ## 2.7.0 - 2026-10-17 `resolve-issue` lists skill-quality's `check-skill.sh` and `check-evals-quality.sh` as diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 5145a012f3..d62876fdb1 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -14,6 +14,7 @@ - [The lane sweep](#the-lane-sweep) - [Models](#models) - [Loop caps](#loop-caps) +- [Limits](#limits) - [Planning lane work](#planning-lane-work) - [Changing this config](#changing-this-config) - [Outside this repository](#outside-this-repository) @@ -859,6 +860,26 @@ escalates. The address-feedback activity hands a PR to a person once `loop.revie rounds have run. The default is 2; this repository sets 3. Separately, after `loop.no-progress` lane runs in a row that change nothing, the PR escalates. +## Limits + +The optional `limits` block sets when the lanes restart work the usage limit stopped. Durations +are whole hours, at most 720; stop caps are at most 10 and `restarts-per-run` at most 50, so a +typo cannot let a forged limit stop restart a lane without bound. resolve-config emits all six +with defaults. + +| Key | Default | What it sets | +|---|---|---| +| `fallback-reset` | 5 | Hours after a limit stop that names no reset time before the limit counts as reset. | +| `reset-clamp-min` | 1 | Fewest hours after a stop before a restart, whatever reset the stop names. | +| `reset-clamp-max` | 168 | Most hours after a stop before a restart, whatever reset the stop names. | +| `restarts-per-run` | 5 | Issues one work-unblock sweep restarts or re-fires. | +| `limit-stops-per-head` | 3 | Limit stops on one PR head after which pr-unblock stops dispatching it again. | +| `limit-stops-per-issue` | 3 | Limit stops since a person last added `agent-ready` before work-unblock hands the issue to a person. | + +report-outcome and the work-unblock sweep read them from resolve-config's output; +pr-feedback-dispatch and pr-unblock-dispatch resolve the base config themselves, and a config the +resolver refuses fails their run before any dispatch. This repository sets no `limits` block. + ## Planning lane work Size the process to the change: diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index 02a75b5373..9e13ccd0dd 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -54,6 +54,55 @@ } } }, + "limits": { + "type": "object", + "additionalProperties": false, + "description": "When the lanes restart work the usage limit stopped, and when they stop restarting it. Durations are whole hours.", + "properties": { + "fallback-reset": { + "type": "integer", + "minimum": 1, + "maximum": 720, + "default": 5, + "description": "Hours after a limit stop that names no reset time before its limit is taken to have reset." + }, + "reset-clamp-min": { + "type": "integer", + "minimum": 1, + "maximum": 720, + "default": 1, + "description": "Fewest hours after a limit stop before a restart, whatever reset the stop names." + }, + "reset-clamp-max": { + "type": "integer", + "minimum": 1, + "maximum": 720, + "default": 168, + "description": "Most hours after a limit stop before a restart, whatever reset the stop names." + }, + "restarts-per-run": { + "type": "integer", + "minimum": 1, + "maximum": 50, + "default": 5, + "description": "Issues one work-unblock sweep restarts or re-fires." + }, + "limit-stops-per-head": { + "type": "integer", + "minimum": 1, + "maximum": 10, + "default": 3, + "description": "Limit stops on one PR head after which the pr-unblock dispatchers stop dispatching it again." + }, + "limit-stops-per-issue": { + "type": "integer", + "minimum": 1, + "maximum": 10, + "default": 3, + "description": "Limit stops since a person last added agent-ready after which work-unblock hands the issue to a person." + } + } + }, "merge": { "$ref": "#/$defs/merge" }, "models": { "$ref": "#/$defs/models" } },