Repository navigation
ci: trim PR checks and disable standalone test workflows - #127
Merged
Merged
Conversation
Keep code_paths, the fork ownership guard, preflight (lint, typecheck, skill guide and localization catalog verification, unit planning), unit tests and cross-version wire. Drop the remaining jobs and preflight ratchets, and the verify entries and code_paths outputs only they read.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
They assert jobs and preflight steps the trimmed pr.yml no longer runs. Declared as deleted permanent exceptions so syncs keep them out.
Drop linux-package-fixture-cache's upstream test (deleted exception) and the fork-owned assertions that read pr.yml's package and shell_contracts jobs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ELI5
This fork ran a large stack of automated checks on every pull request, plus about 25 separate test workflows on schedules and pushes. Most of that guards upstream behavior the fork doesn't change. This PR cuts the PR checks down to the ones that catch real fork breakage, and turns off the standalone test workflows.
What Changed
PR Checks (
.github/workflows/pr.yml)Before, a code PR ran 15 jobs. After, it runs these 7:
code_paths(change detection)fork_ownership_guardpreflighttestandunit_selection_evidencecross-version-wireverify(the required "PR checks passed" gate)I removed these jobs:
git_compatibility,codex_index_heal_contract,xterm_patch_sync,shell_contracts,mobile_web_app,managed_hook_node18,package,e2e,layout_oracleandterminal_ime_native. The orcad browser contract insidetest(browser_contract) is gone too.preflightnow runs:typecheck:citestreads the planI dropped every ratchet and contract check from preflight. That covers the ones in the plan, plus the unnamed ones that went with them: process-host imports, main-sync-spawn, focus-read, layout-writer, deprecated runtime-owner, path-literal, worktree-id composer, e2e typecheck and legacy relay fence. It also covers the locked audit tools check (React Doctor's tooling) and the mobile dependency install that only the changed-code quality gate used.
verifynow needs only the kept jobs, and its env block andcheck_jobcalls match. Thefork_ownership_guardhard check is unchanged. I also removed thecode_pathsoutputs that nothing reads anymore, along with the E2E spec filter step that produced them.Dependencies I kept on purpose
testskips its shards (run_shards: mobile_ci_tools != 'true') and relies on this step. Without the step, those PRs would run no unit tests at all.layout_oracleis gone. It wasn't in the keep list. It runs E2E specs throughe2e.yml, which is now disabled.Deleted upstream contract tests
Thirteen upstream
config/scripts/*.test.mjsfiles assert the job graph and preflight steps this PR removes, and 54 of their tests failed against the trimmed pr.yml. I deleted them and declared each one inconfig/fork-ownership.jsonas a deleted permanent exception, so the next sync won't bring them back. The fork already does this for the tests of the removed Windows PR jobs.Two fork-owned tests also read the removed jobs, so I edited them in place.
run-sandboxed-test-shards.test.mjsnow checks the sandbox shell lane's own spec list instead of comparing it with pr.yml'sshell_contractsjob. Inci-dependency-download-cache.test.mjsI dropped the one test that matched release-cut's cache against the removedpackagejob.Two of these files carried fork seams, so I removed those seams and their residual budgets from the manifest. I also deleted
fork-ownership-guard/pr-workflow-contract.mjs, becausepr-ready-check-reuse.test.mjswas the only file that imported it.This drops some assertions that still passed, most of them
pr-code-change-scope's classifier tests. It also means 27 unit files that the sharded run excludes now run in no gating job: the shell contract, Git compatibility and Codex contract suites.ci-unit-exclusion-coveragewas the test that flagged this.Disabled standalone workflows
These 27 are disabled through
gh workflow disable. No files were edited or deleted, so the next upstream sync won't put anything back.terminal-perfwas already disabled before this change.ci-closed-pr-caches("Clean obsolete PR work") isn't a test. It cancels a PR's superseded runs. It asks for a Blacksmith runner the fork doesn't have, so every run since 2026-10-19 19:02Z sat queued as a pendingcleancheck. I cancelled the 7 queued runs.Still enabled:
node-server-tests.yml.release-cut.ymland the daily, hourly and adhoc mac build workflows call it as a reusable workflow. GitHub's docs don't say whether disabling a workflow blocksworkflow_call, so I didn't risk breaking releases.To re-enable everything:
Why
Most of the removed jobs exercise upstream subsystems: real Git versions, pinned Codex binaries, xterm patches, Linux packaging, E2E. The fork rarely touches any of them, and they cost runner time and red-check noise on every PR. Lint, typecheck, unit tests, the wire-compat journeys and the ownership guard still catch the breakage fork changes actually cause.
I edited pr.yml directly because it's already a permanent fork exception in
config/fork-ownership.json, so the next sync keeps these changes. The only manifest change covers the deleted contract tests. For the standalone workflows I usedgh workflow disableinstead of editing them, because they're upstream-owned and a sync would restore any edit or deletion. Disabling also reverses with one command.Linked Issue
N/A, fork maintenance.
Visual Proof
N/A. This is a CI workflow change with no app behavior change.
Testing
Parsed the trimmed pr.yml with python3 (actionlint isn't installed here). Every
needs.Xreference resolves to a remaining job, everywait:names a remaining step, and every remainingcode_pathsoutput has a reader (exceptreused_run_id, which had none before this change either).Ran all 32 test files that read pr.yml on the remote test sandbox. The 12 deleted files account for every failure caused by this change.
required-pr-check-namealso failed there, but only because Bun's postinstall didn't run in the sandbox.node .github/scripts/check-fork-ownership.mjs origin/main HEADpasses.This PR runs the trimmed pr.yml itself, so its own checks exercise the new graph.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
The change is workflow-only. Several upstream
config/scripts/*.test.mjssuites parse pr.yml, and this PR'stestjob runs them.AI Disclosure
Claude (claude-opus-5-5) via Claude Code.
Review
Agent skill upstream boundary
Notes
No app runtime change. That covers security, cross-platform, SSH, mobile and backwards compatibility. Coverage on those paths is lower now: Windows, SSH and packaging regressions will surface later, in release builds or user reports.
Other open PRs still run main's old pr.yml until this merges, and its
layout_oraclejob calls the now-disablede2e.yml. If GitHub won't run a disabled workflow when another workflow calls it, those PRs'verifywill fail until they pick up this change.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)