Repository navigation
fix(automation): queue version bumps across PR closures - #97
extremeheat wants to merge 4 commits into
Conversation
|
cc @u9g |
extremeheat
left a comment
There was a problem hiding this comment.
Github API related queries may belong in gh-helpers
| const fs = require('fs') | ||
| const { runQueue, writeState } = require('./bump-state') | ||
|
|
||
| const exec = (...args) => { |
There was a problem hiding this comment.
Rename to git for clarity if it only works on git
There was a problem hiding this comment.
Renamed exec to git, including all call sites, in 65015c0.
| async function listBumpPulls (repository) { | ||
| const pulls = [] | ||
| for (let page = 1; ; page++) { | ||
| const params = new URLSearchParams({ state: 'all', base: 'main', head: `${repository.split('/')[0]}:bump`, per_page: '100', page: String(page) }) | ||
| const response = await fetch(`https://api.github.com/repos/${repository}/pulls?${params}`, { | ||
| headers: { Authorization: `Bearer ${process.env.GITHUB_TOKEN}`, Accept: 'application/vnd.github+json' } | ||
| }) | ||
| if (!response.ok) throw new Error(`Cannot list bump PRs: ${response.status}`) | ||
| const batch = await response.json() | ||
| pulls.push(...batch.filter(pr => isBumpBranch(pr, repository))) | ||
| if (batch.length < 100) return pulls | ||
| } | ||
| } |
There was a problem hiding this comment.
Seems like this should be in gh-helpers
There was a problem hiding this comment.
Moved pagination and API access into gh.listPullRequests({ state, head, base }) in extremeheat/gh-helpers#58, with types, mock support, documentation, and four transport-level tests. This PR now uses the helper's normalized results and temporarily pins its exact commit until it is released. Verified the installed helper against this repository's live PR history.
| console.error('Usage: node bump.js <newVersion> (or a bump PR closed event)') | ||
| process.exitCode = 1 | ||
| } | ||
| module.exports(args, { reply: console.log }) |
There was a problem hiding this comment.
or a bump PR closed event
Can't github action workflow run with relevant flag from start without script having to infer what trigger it?
There was a problem hiding this comment.
Done in 65015c0. The workflow explicitly runs bump.js "$MC_VERSION" for a request or bump.js --advance for closure/queue continuation. Removed event-file parsing and trigger inference from the script. A manual dispatch with an empty version also advances the saved queue.
| function legacyVersion (pr) { | ||
| const version = /boilerplate for Minecraft version ([\w.-]+)\./.exec(pr.body)?.[1] || | ||
| /^🎈 Add version ([\w.-]+)$/.exec(pr.title)?.[1] | ||
| if (!version) throw new Error(`Cannot determine the version of bump PR #${pr.number}`) | ||
| return version | ||
| } |
There was a problem hiding this comment.
Why legacy? If it's just for special case current, why not migrate the current to the new?
There was a problem hiding this comment.
Removed the legacy parser in 65015c0. Initialized #77 once with WF_STATE for 26.2 and pending 26.3 (minecraft-data PR #1300 / issue #1299), preserving its existing description. Old closed PRs without state are ignored. Once this workflow is merged, a single manual dispatch with an empty version advances #77's saved queue. The read-only advance simulation passes.
There was a problem hiding this comment.
🔵 Needs a closer look
An unresolved merge-versus-checkout race and shared-branch handoff behavior require correction and end-to-end verification before human approval.
1 open issue
What changed in this PR
This PR replaces dropped version requests with a persistent queue, preserving sequential generator updates so later versions inherit earlier merged fixes.
Changes:
- Persist pending requests and handoff metadata in bump PR descriptions.
- Advance the queue on merge or closure, with duplicate handling and recovery.
- Add queue tests, CI execution, updated dependencies, and usage documentation.
| File | Description |
|---|---|
| README.md | Documents queue behavior, migration, and retries. |
| package.json | Adds the test command and updates gh-helpers. |
| .github/workflows/handle-mcdata-update.yml | Handles closures and serializes queue operations. |
| .github/workflows/build.yml | Runs automation tests in CI. |
| .github/commands/bump.js | Connects queue processing to GitHub and version scaffolding. |
| .github/commands/bump-state.test.js | Tests ordering, deduplication, advancement, and recovery. |
| .github/commands/bump-state.js | Implements persistent queue state and successor handoffs. |
Review effort: Balanced
| require('../../tools/newVersion')(next.version) | ||
| git('checkout', '-B', 'bump') |
There was a problem hiding this comment.
Right, checkout should happen from head of repo (after relevant GH action trigger) and then create a new local branch (any name but ideally something different). Then force push to remote "bump" branch. That removes any race issue
There was a problem hiding this comment.
Fixed in 2b37146 using the separate-local-branch approach. After the queue observes closure, it fetches origin/main and checks out local bump-next from that fetched head. It reloads versions.json and removes newly supported queued versions before scaffolding. The scaffold runs in a new process so neither the manifest nor tool is cached from the initial checkout, then pushes HEAD:refs/heads/bump --force.
Added a local Git integration test that clones the old main, advances the remote to simulate the predecessor merging, and deliberately caches the old tool/manifest. The actual scaffold, commit, and push preserve the predecessor's fixes and version entry and use the updated tool. Additional tests cover newly supported queue entries and fetch failures. All 22 tests and StandardJS checks pass.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
is commands/ actually the right encapsulation for this workflow ?
is the dynamic file writes on test idiomatic?
are these tests necessary and meaningful?
| const api = { | ||
| prepare: async () => supportedVersions, | ||
| list: async () => structuredClone(pulls), | ||
| get: async number => structuredClone(pulls.find(pr => pr.number === number)), | ||
| update: async (number, body) => { | ||
| updates.push(number) | ||
| pulls.find(pr => pr.number === number).body = body | ||
| }, | ||
| create: async (next, state) => { | ||
| const number = Math.max(0, ...pulls.map(pr => pr.number)) + 1 | ||
| const pr = { number, title: `🎈 Add version ${next.version}`, body: writeState('New PR.', state), state: 'open' } | ||
| pulls.push(pr) | ||
| created.push({ ...next, number }) | ||
| return { number } | ||
| } | ||
| } |

Change and reason
When a version request arrives while a
bumpPR is open, the bot currently drops it after posting a comment. This preserves one active bump but requires a manual rerun after it closes.Persist pending requests in a
<!-- <WF_STATE>…</WF_STATE> -->JSON block in the active PR body, alongside a readable queue. Merging or closing that PR creates the next queued version from the latestmain, carries the remaining queue forward, and links predecessor and successor PRs. Closing without merging skips that version.For example, 26.4 and 26.5 wait behind 26.3. Merging 26.3 opens 26.4; closing 26.4 without merging opens 26.5 from the implementation on
main. Duplicate deliveries do not recreate handled versions, including skipped ones.Fixes #96. Related to #94: this retains sequential generator updates so later versions inherit earlier fixes.
Scope
Automation, tests, and documentation only; no Minecraft extraction code or generated data changes.
queue: max.main, create localbump-next, and reload its manifest before choosing the successor. Run the refreshed scaffold tool in a new process and force-pushHEAD:refs/heads/bump.mainusing the existing PAT; restrict closure handling to the repository's balloon-markedbumpPRs targetingmain.npm test.Validation
Installed published
gh-helpers@1.3.0and verified the queue's required API methods are available.npm test: all 22 tests pass, covering multiple releases, ordering, duplicates, merge/close advancement, explicit migration, stateless old PRs, empty queues, and failures/retries before and after PR creation../node_modules/.bin/standard --stdin < <file>for each of the four changed automation JavaScript files: passed. Stdin avoids the linter's default hidden-directory exclusion.Parsed both changed workflow YAML files with
js-yaml: passed.git diff --check: passed.Local Git integration test reproduces
mainadvancing after workflow checkout with the old manifest/tool already cached. The actual scaffold/commit/push preserves the merged predecessor's fixes and version entry, uses the updated tool, and leaves the existing localbumpbranch untouched. Additional tests cover refreshed versions consuming part/all of the queue and failed fetches retaining requests.Read-only advance simulation using the installed gh-helpers dependency and live migrated 🎈 Support 26.2 and fix various data generation issues on other versions #77: recovered the saved 26.3 request, preserving minecraft-data PR #1300 and issue #1299 and linking it to predecessor 🎈 Support 26.2 and fix various data generation issues on other versions #77. Creation and handoff writes were stubbed.
Live workflow dispatch and actual PR creation remain to be verified after deployment. Gradle generation was not run because this changes automation only.
Compatibility
The existing
bumpbranch and version/PR/issue dispatch inputs remain in use. An empty version explicitly advances the saved queue. The workflow invokesbump.js <version>orbump.js --advance; the script does not infer its trigger from GitHub event payloads. Keep the hidden JSON block when editing descriptions. Old closed PRs without state are ignored; an open PR without state requires explicit initialization.GitHub PR listing now uses
gh.listPullRequests, added in extremeheat/gh-helpers#58. The dependency uses the publishedgh-helpers@^1.3.0release. No recurring minecraft-data lookup is required.#77 has been explicitly initialized with
WF_STATEfor 26.2 and a pending 26.3 request (upstream PR, issue). After merging, runhandle-mcdata-updateonce with the version empty to advance this saved queue. Future requests are persisted and advanced automatically.Checklist