Repository navigation
feat(code-tidying): add upkeep settings, cap and per-merge detection #12614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
1007424
feat(code-tidying): add upkeep settings, per-merge detection and the …
kyle-sexton 0a703f3
fix(code-tidying): read the upkeep gate's sweep history to the window…
kyle-sexton 794cea0
fix(code-tidying): limit the upkeep backstop to areas with unswept churn
kyle-sexton bc5cbed
Merge branch 'main' into feat/12475-code-tidying-upkeep
kyle-sexton 7c51995
Merge remote-tracking branch 'origin/main' into feat/12475-code-tidyi…
kyle-sexton 2190a87
fix(code-tidying): keep a sweep's merge out of churn and overdue chur…
kyle-sexton 7a1da0b
fix(code-tidying): keep the upkeep gate planning on a full sweep list
kyle-sexton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
14 changes: 14 additions & 0 deletions
14
.changes/code-tidying/feat-12475-upkeep-settings-5d2e8c17.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| --- | ||
| bump: minor | ||
| --- | ||
|
|
||
| ### Added | ||
|
|
||
| - Upkeep settings `upkeep_cadence`, `upkeep_backstop` and `upkeep_max_open` in | ||
| `<home>/code-tidying.yaml` (defaults `daily`, `weekly`, 3), read by | ||
| `skills/tidy/scripts/upkeep-setting.sh`. See `reference/config.md`. | ||
|
|
||
| ### Changed | ||
|
|
||
| - The `/code-tidying:tidy` backlog throttle takes its limit from `upkeep_max_open`, and | ||
| `open-pr-count.sh` reports the limit and the layer that supplied it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "name": "maintenance-code-tidying-upkeep", | ||
| "private": true, | ||
| "scripts": { | ||
| "test": "node --test" | ||
| } | ||
| } |
212 changes: 212 additions & 0 deletions
212
.github/scripts/maintenance-code-tidying-upkeep/upkeep.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,212 @@ | ||
| // Code-tidying upkeep, no model. Two commands: | ||
| // | ||
| // detect --before <sha> --after <sha> [--root <dir>] | ||
| // Per merge: names the areas a pushed range changed, on stdout and in the | ||
| // job summary. Opens nothing. | ||
| // gate [--root <dir>] | ||
| // Prints the batch as JSON: whether it may run and which areas to sweep, | ||
| // with `/code-tidying:tidy <glob>` opening one PR per area on the | ||
| // chore/tidy-adhoc-<slug>-YYYY-MM-DD branch that skill names. Exit 1 when | ||
| // a read fails: nothing is planned on a count it could not read. | ||
| // | ||
| // The gate derives churn from git log, so a push run GitHub lost drops no area | ||
| // (dependabot-fragments.yml records that those happen). | ||
| import { execFileSync } from "node:child_process"; | ||
| import { appendFileSync } from "node:fs"; | ||
| import { dirname, join } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
|
|
||
| const HERE = dirname(fileURLToPath(import.meta.url)); | ||
| const TIDY_SCRIPTS = join(HERE, "../../../plugins/code-tidying/skills/tidy/scripts"); | ||
| const DAY_MS = 86_400_000; | ||
| const WINDOW_DAYS = { daily: 1, weekly: 7, monthly: 30 }; | ||
| const PULL_LIST_LIMIT = 1000; | ||
| const NEVER_SWEPT_DAYS = 90; | ||
| const SAFE_AREA = /^[A-Za-z0-9_][A-Za-z0-9._-]*(\/[A-Za-z0-9._-]+)?$/; | ||
| const ADHOC_BRANCH = /^chore\/tidy-adhoc-(.+)-\d{4}-\d{2}-\d{2}$/; | ||
|
|
||
| const dayOf = (ms) => Math.floor(ms / DAY_MS); | ||
|
|
||
| // A path's area: plugins/<name>, docs/<dir>, else the first segment. A path under | ||
| // a dot directory is left out: tidy's HARD list covers agent and CI config. A | ||
| // file directly under plugins/ or docs/, or at the root, has no subtree of its | ||
| // own, so its glob stops at that level. | ||
| export function areaOf(path) { | ||
| const parts = path.split("/"); | ||
| if (parts[0].startsWith(".")) return null; | ||
| let found; | ||
| if (parts.length === 1) { | ||
| found = { area: "root", glob: "*" }; | ||
| } else if ((parts[0] === "plugins" || parts[0] === "docs") && parts.length > 2) { | ||
| const area = `${parts[0]}/${parts[1]}`; | ||
| found = { area, glob: `${area}/**` }; | ||
| } else if (parts[0] === "plugins" || parts[0] === "docs") { | ||
| found = { area: parts[0], glob: `${parts[0]}/*` }; | ||
| } else { | ||
| found = { area: parts[0], glob: `${parts[0]}/**` }; | ||
| } | ||
| return SAFE_AREA.test(found.area) ? found : null; | ||
| } | ||
|
|
||
| // The <slug> tidy's ad hoc branch carries: the glob's literal directory prefix in kebab case. | ||
| export const slugOf = (area) => area.toLowerCase().replace(/[^a-z0-9]+/g, "-"); | ||
|
|
||
| export function detectAreas(paths) { | ||
| const names = paths.map((path) => areaOf(path)?.area).filter(Boolean); | ||
| return [...new Set(names)].sort(); | ||
| } | ||
|
|
||
| // pulls: { headRefName, createdAt, state, mergeCommit }. Slug to the newest sweep's time, | ||
| // the slugs with one open, and each sweep's own merge commit, which is not churn. | ||
| function sweepsOf(pulls) { | ||
| const last = new Map(); | ||
| const open = new Set(); | ||
| const merges = new Set(); | ||
| for (const pull of pulls) { | ||
| const slug = ADHOC_BRANCH.exec(pull.headRefName)?.[1]; | ||
| if (!slug) continue; | ||
| last.set(slug, Math.max(last.get(slug) ?? 0, Date.parse(pull.createdAt))); | ||
| if (pull.state === "OPEN") open.add(slug); | ||
| if (pull.mergeCommit?.oid) merges.add(pull.mergeCommit.oid); | ||
| } | ||
| return { last, open, merges }; | ||
| } | ||
|
|
||
| // How far back the gate reads commits: to the oldest known sweep, so an area's | ||
| // churn since its sweep is seen however long the cap held it, and at least | ||
| // NEVER_SWEPT_DAYS for an area with no sweep on record. | ||
| export function lookbackStart(pulls, now) { | ||
| const sweeps = pulls.filter((pull) => ADHOC_BRANCH.test(pull.headRefName)).map((pull) => Date.parse(pull.createdAt)); | ||
| return Math.min(now - NEVER_SWEPT_DAYS * DAY_MS, ...sweeps); | ||
| } | ||
|
|
||
| // paths: every tracked file. commits: { sha, at (ms), paths } back to lookbackStart. | ||
| // pulls: every sweep. Only an area with a commit after its | ||
| // last sweep is eligible. It is overdue once that oldest unswept commit is past | ||
| // the backstop window; overdue areas go first, then the oldest unswept. | ||
| export function plan({ paths, commits, pulls, cadence, backstop, limit, open, throttled, now }) { | ||
| const stop = (reason) => ({ proceed: false, reason, areas: [] }); | ||
| const windowDays = WINDOW_DAYS[backstop]; | ||
|
|
||
| const { last, open: openSlugs, merges } = sweepsOf(pulls); | ||
| const newest = Math.max(0, ...last.values()); | ||
| if (newest > 0 && dayOf(now) - dayOf(newest) < WINDOW_DAYS[cadence]) return stop("cadence"); | ||
| const slots = Math.max(0, limit - open); | ||
| if (throttled || slots === 0) return stop("cap"); | ||
|
|
||
| const rows = new Map(); | ||
| for (const path of paths) { | ||
| const found = areaOf(path); | ||
| if (!found || rows.has(found.area)) continue; | ||
| const slug = slugOf(found.area); | ||
| const swept = last.get(slug) ?? 0; | ||
| rows.set(found.area, { ...found, slug, swept, unswept: Infinity }); | ||
| } | ||
| for (const commit of commits) { | ||
| if (merges.has(commit.sha)) continue; | ||
| for (const path of commit.paths) { | ||
| const row = rows.get(areaOf(path)?.area); | ||
| if (row && commit.at > row.swept) row.unswept = Math.min(row.unswept, commit.at); | ||
| } | ||
| } | ||
|
|
||
| const overdue = (row) => dayOf(now) - dayOf(row.unswept) >= windowDays; | ||
| const eligible = [...rows.values()].filter((row) => row.unswept !== Infinity && !openSlugs.has(row.slug)); | ||
| eligible.sort((a, b) => Number(overdue(b)) - Number(overdue(a)) || a.unswept - b.unswept || a.area.localeCompare(b.area)); | ||
| const areas = eligible.slice(0, slots).map((row) => ({ area: row.area, glob: row.glob, slug: row.slug, overdue: overdue(row) })); | ||
| return { proceed: areas.length > 0, reason: areas.length > 0 ? "due" : "nothing-due", areas }; | ||
| } | ||
|
|
||
| export function parseCount(text) { | ||
| const open = /^Open tidy PR count: (\d+)( or more)?$/m.exec(text); | ||
| const limit = /^Throttle limit: (\d+),/m.exec(text); | ||
| const active = /^Throttle active: (yes|no)$/m.exec(text); | ||
| if (!open || !limit || !active) throw new Error("the open tidy PR count is unknown"); | ||
| return { open: Number(open[1]), limit: Number(limit[1]), throttled: active[1] === "yes" }; | ||
| } | ||
|
|
||
| // `\0<sha> <unix seconds>` before each commit's paths. | ||
| export function parseLog(text) { | ||
| return text | ||
| .split("\0") | ||
| .slice(1) | ||
| .map((chunk) => { | ||
| const [head, ...paths] = chunk.split("\n").filter(Boolean); | ||
| const [sha, at] = head.split(" "); | ||
| return { sha, at: Number(at) * 1000, paths }; | ||
| }); | ||
| } | ||
|
|
||
| function run(root, file, args, env = {}) { | ||
| return execFileSync(file, args, { | ||
| cwd: root, | ||
| encoding: "utf8", | ||
| maxBuffer: 512 * 1024 * 1024, | ||
| env: { ...process.env, CLAUDE_PROJECT_DIR: root, ...env }, | ||
| stdio: ["ignore", "pipe", "pipe"], | ||
| }); | ||
| } | ||
|
|
||
| function setting(root, key) { | ||
| const [value, source] = run(root, "bash", [join(TIDY_SCRIPTS, "upkeep-setting.sh"), key]).trimEnd().split("\t"); | ||
| return { value, source }; | ||
| } | ||
|
|
||
| function gate(root, now) { | ||
| const cadence = setting(root, "upkeep_cadence"); | ||
| const backstop = setting(root, "upkeep_backstop"); | ||
| const count = parseCount(run(root, "bash", [join(TIDY_SCRIPTS, "open-pr-count.sh")])); | ||
| // The sweeps, by branch prefix: listing all pull requests reaches about 3 days per 1000 here. | ||
| // A full list holds the newest; lookbackStart reads commits back to its oldest, and every one of | ||
| // those is after the last sweep of an area the list has lost, so that area is judged correctly. | ||
| const sweeps = JSON.parse(run(root, "gh", ["pr", "list", "--state", "all", "--limit", String(PULL_LIST_LIMIT), "--search", "head:chore/tidy-adhoc-", "--json", "headRefName,createdAt,state,mergeCommit"])); | ||
| const openPulls = JSON.parse(run(root, "gh", ["pr", "list", "--state", "open", "--limit", String(PULL_LIST_LIMIT), "--json", "headRefName,createdAt,state"])); | ||
| const since = new Date(lookbackStart(sweeps, now)).toISOString(); | ||
| const commits = parseLog(run(root, "git", ["log", `--since=${since}`, "--no-renames", "--name-only", "--format=%x00%H %ct"])); | ||
| const paths = run(root, "git", ["ls-files"]).split("\n").filter(Boolean); | ||
| const result = plan({ paths, commits, pulls: [...sweeps, ...openPulls], cadence: cadence.value, backstop: backstop.value, ...count, now }); | ||
| return { ...result, settings: { upkeep_cadence: cadence, upkeep_backstop: backstop, upkeep_max_open: count.limit }, open: count.open }; | ||
| } | ||
|
|
||
| function detect(root, before, after) { | ||
| const sha = /^[0-9a-f]{40}$/; | ||
| if (!sha.test(before) || !sha.test(after) || /^0+$/.test(before)) { | ||
| console.log("::warning::the pushed range has no readable start; the daily gate's git log still covers it"); | ||
| return; | ||
| } | ||
| let changed; | ||
| try { | ||
| changed = run(root, "git", ["diff", "--name-only", "--no-renames", before, after]); | ||
| } catch { | ||
| console.log("::warning::the pushed range did not diff; the daily gate's git log still covers it"); | ||
| return; | ||
| } | ||
| const areas = detectAreas(changed.split("\n").filter(Boolean)); | ||
| console.log(areas.join("\n")); | ||
| if (process.env.GITHUB_STEP_SUMMARY) { | ||
| appendFileSync(process.env.GITHUB_STEP_SUMMARY, `Upkeep areas changed: ${areas.join(", ") || "none"}\n`); | ||
| } | ||
| } | ||
|
|
||
| function option(args, name, fallback) { | ||
| const at = args.indexOf(name); | ||
| return at === -1 ? fallback : args[at + 1]; | ||
| } | ||
|
|
||
| if (process.argv[1] === fileURLToPath(import.meta.url)) { | ||
| const [command, ...args] = process.argv.slice(2); | ||
| const root = option(args, "--root", process.cwd()); | ||
| if (command === "detect") { | ||
| detect(root, option(args, "--before", ""), option(args, "--after", "")); | ||
| } else if (command === "gate") { | ||
| try { | ||
| console.log(JSON.stringify(gate(root, Date.now()), null, 2)); | ||
| } catch (error) { | ||
|
kyle-sexton marked this conversation as resolved.
Resolved
|
||
| console.error(`upkeep gate: ${error.message.split("\n")[0]}`); | ||
| process.exit(1); | ||
| } | ||
| } else { | ||
| console.error("usage: upkeep.mjs detect --before <sha> --after <sha> [--root <dir>] | gate [--root <dir>]"); | ||
| process.exit(2); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.