ci: reduce pull request runner usage - #3261
Conversation
The repository now lives at apache/maka, so ASF Infra reads .asf.yaml from the default branch. Declare the repository metadata, merge policy, and branch protection that were previously unset, and route GitHub activity to commits@maka.apache.org so project activity is archived on the mailing list. Required status checks are omitted for now. On main today the CI jobs are gated on a path-filter job and report as skipped when unaffected, which GitHub treats as an unmet required context. #3261 collapses them into a single always-running CI / test gate that can be required once it lands. Generated-by: Claude Code
a7b2d7f to
d8f3d1a
Compare
The repository now lives at apache/maka, so ASF Infra reads .asf.yaml from the default branch. Declare the repository metadata, merge policy, and branch protection that were previously unset, and route GitHub activity to commits@maka.apache.org so project activity is archived on the mailing list. Every field is checked against the strictyaml schema in apache/infrastructure-asfyaml, which rejects the whole file on an unknown key. del_branch_on_merge uses the current github.pull_requests location rather than the legacy top-level one, which raises once any other pull_requests setting is added. Required status checks are omitted for now. On main today the CI jobs are gated on a path-filter job and report as skipped when unaffected, which GitHub treats as an unmet required context. #3261 collapses them into a single always-running CI / test gate that can be required once it lands. Generated-by: Claude Code
c4683a2 to
a2b7add
Compare
|
The consolidation itself looks sound, but there is one blocking integration issue before this leaves draft. This PR removes the standalone
Please align #3262 with the final workflow—most simply, require only I reviewed the consolidated test plan, Windows path filters, and CLI release path. I don’t see another issue that should block this PR. Removing the CLI validation |
ASF committers have no GitHub admin UI, so .asf.yaml is the only self-service path to repository settings. main currently has no branch protection at all and the repository carries no topics, so declare both here, along with the merge policy and Dependabot alerts that are already in effect. Require the test check. This has to merge after #3261, which folds lint, format, build, tsc and knip into the test job and removes the separate typecheck job. On main today test does not depend on typecheck, so requiring test alone leaves those checks unenforced, and requiring typecheck as well would block #3261 from ever satisfying it. Validated through the two-pass parse asfyaml actually performs: dirty_load, then as_yaml() and a schema-checked reparse. A folded description scalar is sensitive to that round trip. A comment indented at the same level is pulled into the scalar and silently appended to the repository description, and PyYAML does not reproduce this. Generated-by: Claude Code
|
Thanks for catching this. #3262 has since been updated to require only With that integration issue resolved and the checks green, I’m marking this PR ready for review. Posted with Codex assistance. |
a2b7add to
3698e29
Compare
Generated-by: Codex
3698e29 to
65aa623
Compare
M4n5ter
left a comment
There was a problem hiding this comment.
Re-reviewed the latest revision. The required-check mismatch is resolved, and I didn’t find any new issues on my side. LGTM.
|
This change qualifies for the repository's self-merge fast path: it is limited to CI configuration, addresses an active repository-wide blocker, has passed the required check, and is straightforward to revert. M4n5ter independently reviewed and approved the final revision. I have reviewed the final diff and commit message and am choosing the fast path as the human contributor of record. Any regression or subsequent community objection will be handled by reverting or adjusting the change. |
ASF committers have no GitHub admin UI, so .asf.yaml is the only self-service path to repository settings. main currently has no branch protection at all and the repository carries no topics, so declare both here, along with the merge policy and Dependabot alerts that are already in effect. Require the test check. Since #3261 landed, ci.yml runs a single test job on every pull request covering lint, format, build, tsc, knip, the selected workspace suites, e2e and Storybook, which makes it a complete and stable required context. Validated through the two-pass parse asfyaml actually performs: dirty_load, then as_yaml() and a schema-checked reparse. A folded description scalar is sensitive to that round trip. A comment indented at the same level is pulled into the scalar and silently appended to the repository description, and PyYAML does not reproduce this. Generated-by: Claude Code
The recovery lane went back to an unconditional pull request trigger and became a second required context in #3789, which puts a three-minute Windows job on every pull request whether or not it can observe the change. That reverses the automatic-runner-start reduction agreed in #3261, and it does not scale: the platform matrix is heading for four Node versions across Linux, Apple x64, Apple silicon and Windows, where one channel must not gate every other pull request. Give the lane the shape the other conditional Windows lane already uses. A path pre-filter names the storage, runtime and Runtime Host source trees whose crash and owner-death authorities its steps execute, and a nightly run covers the transitive edits a static list cannot match. Drop windows_recovery from the ASF-managed required contexts, because a required context behind a path filter never reports and would freeze every pull request instead. Across the last 30 merged pull requests the filter selects 17 and skips 13. Generated-by: Claude Opus
Reverts the in-job planner selection and restores the paths filter, with the review findings from both approaches folded in. The in-job version kept windows_recovery required, which is only possible if the check reports on every pull request, which is only possible if the lane starts a Windows runner on every pull request. It cut the time an unrelated diff spent there but not the runner start, and #3261 measured starts. Windows is one of four platforms this project is heading for, and the other three hold no required gate today: runtime-host-owner-platform covers macOS and Windows, release-windows-check and windows-sandbox-w0 cover Windows, and none of them blocks a merge. A required gate for one platform out of four is the asymmetry being removed here, not a guarantee being given up. What replaces the blocking check is the unfiltered main push, which observes the merged result immediately and attributes a regression to one commit, plus the nightly for the transitive edits a static list cannot match. The concurrency key deliberately gives each main push its own group so no merge supersedes another's evidence. The filter covers the workspaces whose gates the steps execute, every workspace they depend on, and the manifests, project files, patches and scripts the unconditional install and clean steps consume. Its contract test derives the workspace set from the dist paths the steps run and widens it with loadWorkspaceGraph, the same graph the planner selects with, so this does not introduce a second definition of the dependency edges. The workflow scanners read the comment-stripped `on:` block through the existing triggerBlock helper and accept the quoting YAML allows, so documenting a trigger or requoting an entry no longer reports a filter that is plainly there as missing. Generated-by: Claude Opus
Summary
Reduce automatic pull request runner starts to one stable core gate.
testjob.workflow_call; it no longer starts automatically for pull requests.mainpushes. Themainrun validates the exact merged delta because this repository does not currently require pull requests to stay up to date with the base branch.For this PR's own CI-sensitive diff, the automatic path changes from nine jobs to
CI / testonly. The tradeoff is less platform evidence on every pull request and less lane-level parallelism; the retained installed-package smoke covers the common CLI packaging failure mode without another runner.This does not add a new daily or weekly tier. The existing Windows nightly remains in place.
Fixes #3259
Verification
node --test --test-concurrency=1 scripts/ci-test-plan.test.mjs scripts/release-cli-workflow-policy.test.mjs(22 tests)npm run windows:inventory(3 tests; generated inventory current)actionlint .github/workflows/ci.yml .github/workflows/windows-baseline.yml .github/workflows/windows-recovery.yml .github/workflows/cli-package-validation.ymlnpm run format:checkgit diff --checknpm --userconfig=/dev/null run release:cli:pack && npm --userconfig=/dev/null run release:cli:smoke(clean isolated build, offline installed-package validation passed)Repository-wide tests and the Electron E2E suite were not run locally. The changed workflow will provide the selected CI evidence.
Discussion and rollout
The
dev@maka.apache.orgdiscussion remains open, and this PR is ready for concrete review. Do not merge while there are unresolved objections. If there are no objections after the stated 72-hour window, it can proceed under lazy consensus after human review of the final diff.The stable required check remains
CI / test. Live verification found no branch protection or repository ruleset that requires the removed platform job names.AI use
Select exactly one:
Tool(s) and scope: Codex analyzed runner usage, implemented and validated the workflow changes, and performed independent deep reviews. Claude Opus performed separate adversarial reviews. I reviewed the final diff, the validation evidence, and each accepted or rejected finding.
Checklist
Does this PR entail a change in behavior?