Skip to content

feat(code-tidying): run upkeep as a daily batch of at most 3 PRs - #12674

Merged
kyle-sexton merged 21 commits into
mainfrom
feat/12475-upkeep-daily-batch
Nov 3, 2026
Merged

kyle-sexton merged 21 commits into
mainfrom
feat/12475-upkeep-daily-batch

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Closes #12475

session: lane-2

Summary

Adds the upkeep batch: the model job that opens one chore/tidy-adhoc-<slug>-<date> pull request per due area. #12614 shipped the settings, the cap, detection and the gate. This renames the lane verb-first to maintenance-tidy-code-upkeep, as the vocabulary requires (melodic-software/standards#945).

Fix

  • Batch. Gate, then model, scan, push and publish jobs, following upstream-apply-changelog.yml. The kill switch is read before the model step, and the model job holds no write token.
  • Token. The broker accepts this caller, limited to a branch push or a PR open (melodic-software/azure-iac#112, docs: surface the local development loop from the README #113, both deployed).
  • Scan key. The scan job is the third holder of SCAN_CARRIER_PRIVATE_KEY, under the second holder's limits, on the owner's decision. Pinned in callers.test.mjs.
  • Cap. push-branches reads upkeep_max_open again after the model job, before its first new branch, so no branch is left without a pull request. A sweep past the cap goes unrecorded and stays due. That job gains pull-requests: read and counts with its read-only token; the scan job, as the key holder, stays contents: read.
  • Re-runs. push reuses a branch at the planned tree, and stops on one holding another tree with no pull request. publish skips a branch that already has a pull request.
  • Clean sweeps. A sweep that opens no PR is recorded in the tidy-code-upkeep-sweeps artifact (90 days) with the swept commit's time, so that area isn't picked again until it changes. The gate trusts only records from this repository's main. Basis: uploading needs no write token.
  • Merging. Upkeep PRs are App-opened, so the lanes adopt them and pr-merge merges them under its usual gates (docs/conventions/pr-pipeline.yaml, adoption). The README and tidy/SKILL.md said a person always merges; both now say the repository's merge lane or a person.

Verification

  • Upkeep and batch tests: 49 pass, 0 fail. check-artifact-retention.sh: 80 uploads match. test_floor_paths.py: 28 pass, with the floor pins updated for the new workflow row. Static check-affected: 14 passed, 1 skipped.
  • Left out: no live run yet, so scan, push, publish and the workflow YAML are not exercised end to end. A workflow_dispatch after merge is the first live run. Whether an upkeep PR goes through pr-merge is unproven until then.
  • Left out: the full local --tests, per check-affected --tests overruns its budget and stalls the ready flip #12639; CI runs the suites.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn

kyle-sexton and others added 15 commits November 2, 2026 21:13
…batch gate

Settings upkeep_cadence, upkeep_backstop and upkeep_max_open live in
<home>/code-tidying.yaml and resolve through one reader. The tidy throttle
takes its limit from upkeep_max_open. A push job names the areas each merge
changed. upkeep.mjs gate plans the daily batch from git log, the open tidy PR
count and earlier ad hoc sweeps; no workflow runs it yet.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
…'s edge

A listing of 1000 pull requests here reaches back about three days, so a
limit-based read stopped every run short of the weekly window. The gate now
pages newest first until it passes the window, and reads open pull requests
separately so an old open sweep still counts.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
An area nobody changed is never eligible, since 108 areas cannot fit the
sweeps a cap of 3 allows in a week. An area is overdue once its oldest unswept
commit passes the backstop window, and overdue areas go first. The gate reads
commits and pull requests back twice the window.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
…n in view

A squash-merged sweep's own commit no longer counts as churn in its area. The
gate reads every sweep by branch prefix and reads commits back to the oldest
sweep, at least 90 days, so overdue churn no longer leaves a fixed window. A
sweep list at the listing limit stops the batch with a reason. The throttle
test clears the inherited git environment.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
The sweep search returns at most 1000 results, so stopping on a full list would
have stopped the gate for good within a year. The oldest listed sweep is the
lookback floor instead, and an area the list has lost is still judged correctly.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
The maintenance-code-tidying-upkeep workflow gains the daily batch on schedule
and dispatch: gate, model, scan, push-branches and publish, in the shape of
upstream-apply-changelog. The model runs on the automations subscription token
with a read-only token. The scan job is the scan-carrier key's third holder,
by the owner's decision on #12475, and records a clean sweep as a workflow
artifact, which needs no write token, so a clean area is not picked again until
it changes.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
…daily-batch

# Conflicts:
#	.github/scripts/maintenance-code-tidying-upkeep/upkeep.mjs
…de-upkeep

The vocabulary needs a verb first, and code is not a verb. The sweep record
artifact becomes tidy-code-upkeep-sweeps and the vocabulary function
tidy-code-upkeep.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 04:13
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T04:16:08.447372Z a445421 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a445421f9a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

The sweep record took its time from the artifact's creation, so a commit that
landed on main while the model swept the fixed base counted as swept. The scan
job now writes the base commit's committer time, and the gate drops a record
without a valid one.

Refs: #12475

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 2 inline finding(s) on a445421f9a05b66bcd9a47630a3aa4377b391804; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 7dea65701103cbcf07b4fd0a57db06e752f057e6: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #12674, incremental from 923608c7 to 7dea6570

Scope: I read every hunk of both files in this push: .github/scripts/maintenance-tidy-code-upkeep/batch.mjs and batch.test.mjs. I used the full diff and the head file only for context.

Findings: 0

Earlier findings, both fixed:

Not a finding: two edge cases remain, and neither one meets the bar:

  • A reused branch gets a slot even when slots is already 0. That can only push the count past the cap if another tidy PR opened between the failed run and the re-run.
  • A reused branch that already has its PR is counted twice: once in open and once by pushStep. This only means fewer new branches get pushed; the cap still holds.

Closing issue #12475: the "No more than 3 upkeep PRs are open at once" item now covers the re-run path. The "daily batch" item holds again because the cadence is daily. Nothing else in scope changed.

Not verified (author-claimed): the test results in the PR body (49 passing), and that the workflow works end to end. The PR says it has had no live run yet.

Other checks:

  • Manifests: no plugin.json or marketplace.json changes in this push.
  • Settled threads: none re-raised.
  • Prior findings: none repeated. Both inline findings were fixed, and their threads are resolved.
  • No embedded instructions in the PR context, the settled threads or the prior findings.

Spend (not a finding): this push doesn't change what the batch spends. That's still one model run per due batch on the automations subscription token.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on a445421f9a05b66bcd9a47630a3aa4377b391804; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 7dea65701103cbcf07b4fd0a57db06e752f057e6: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12674, incremental from 923608c7 to 7dea6570

I found no security issues in this push. There are no CRITICAL, IMPORTANT or SUGGESTION findings.

critical-findings: 0

What I read. Every hunk of both files on the incremental list:

  • .github/scripts/maintenance-tidy-code-upkeep/batch.mjs
  • .github/scripts/maintenance-tidy-code-upkeep/batch.test.mjs

Neither file matches a trusted-path glob. This is a partial review by design: it covers only what changed since 923608c7. That earlier head was reviewed in full and had no findings.

What changed. The push adds pushStep(existing, slots) and uses it in the push loop. Before, a branch the run re-used (existing.head set) never used up one of the open-PR cap's slots. Now it does. A branch that already has its pull request still uses none, and a new branch is pushed only while a slot is left. The change only narrows how many PRs a re-run can open.

What it leaves unchanged:

  • every guard before the step: staysInArea, changesHoldPath, and the branch and tree match in findExisting
  • createRef without overwrite
  • the scope of the lanes token

No untrusted input reaches the new code that did not reach it before, and it adds no new token, actor, or path for untrusted text.

Prior findings and settled threads.

  • My only earlier inline finding, r4450905752, was fixed at 923608c7. This push does not touch upkeep.mjs, so it stays fixed.
  • The settled thread about the re-run cap is the bug this push fixes. That thread raised no security point, and nothing here reopens it.

Spend. None. The cap now limits more strictly how many upkeep PRs, and so how many review runs on them, a re-run can open.

Embedded text. None found in the PR context, settled threads or prior findings.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 2 inline finding(s) on 923608c768ea33f728b8029541fb63cdff38881c; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread docs/conventions/code-tidying.yaml Resolved
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit ec5fa52 Nov 3, 2026
70 checks passed
@kyle-sexton
kyle-sexton deleted the feat/12475-upkeep-daily-batch branch November 3, 2026 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

code-tidying: run upkeep as a daily batch of at most 3 PRs

1 participant