Repository navigation
ci: make unit-ci compile, and fail when its unit tests skip - #14008
Conversation
unit-ci clears suite-coverage on the strength of `app-host unit tests`, but it left the build-input reuse steps live. The usual way to use the label is to add it after a first push, and that rerun finds an earlier run that compiled the same inputs, sets compile_admitted, and skips compile admission. App-host runs only behind an admission that succeeded, so it skipped too, and macOS status did not require it outside the full suite. The PR came out green having run no tests. Skip reuse under unit-ci the way the full suite already does, and make macOS status require app-host under unit-ci, so any other route to a skip fails instead of passing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
teamleaderleo
left a comment
There was a problem hiding this comment.
Self-review. This is the fix posted on #13996 and pushed there as 6aad6c102f, cherry-picked onto 3c58b03f04 without changes. It changes three lines of workflow logic and adds one test.
Things I checked that aren't in the body:
ghosttykit_releaseand thecompile_admittedoutput both readsteps.unchanged_inputs.outputs.compile_admitted. When those steps skip, they fall back exactly as they already do underfull_suite.- Forcing a compile under
unit-cicosts one admission build per labeled run. The whole point of the label is to run tests against this run's product, so there's nothing cheaper that's still correct.
— Ophelia g1 🍄
Run: run_cmux_main_red_triage_app_host_census_and_pr_review_20260923_07d8d17b
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughUnit-ci runs now compile the app host without reusing an earlier build. The macOS status check also requires ChangesUnit CI Requirements
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Unit-ci runs must compile and pass the app-host unit tests; no actionable merge risk remains after normal checks. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3ca19ad fix: honor the tab index when inserting a Cloud mirror terminal (manaflow-ai#13998) c42548e test(settings): enforce that advertised cmux.json paths are actually supported (manaflow-ai#13963) 49bd8be ci: make unit-ci compile, and fail when its unit tests skip (manaflow-ai#14008) 3c58b03 ci: add a unit-ci tier between compile-only and the full suite (manaflow-ai#13996) c3dd613 fix(ci): name recorded failures when the app host restarts mid-run (manaflow-ai#14000) db5d212 docs: record how to read CI cost measurements (manaflow-ai#13971) 157c67f ci: pin the paid-overflow gate's fallbacks and name the Tart catch (manaflow-ai#13994) # Conflicts: # .github/workflows/ci-guards.yml # .github/workflows/ci-macos.yml # .github/workflows/ci.yml
Takes manaflow-ai#14008: macOS status requires app-host under unit-ci alongside this branch's CLI admission and cli-product-tests rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in main through the base branch, including manaflow-ai#14008. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
#13996 merged at
a088ac12fb, a minute before the fix from its review (comment) was pushed. That fix is not on main.As merged,
unit-cican turn a PR fully green without any app-host test running, and it happens in the ordinary way people use the label:cmuxTests/PR is pushed. Compile admission passes, andsuite-coveragerefuses the run.unit-ciis added and a fresh run starts.full_suiteis stillfalse, so the build-input reuse steps run. They find the earlier run that compiled the same inputs, andcompile_admitted=true.macos-compile-admissionskips.app-host-unit-testsruns only after an admission that succeeded in this run, so it skips too.macOS statusrequires app-host only underfull_suite, so the skip passes. The label has already clearedsuite-coverage.ci-statusgoes green.Before #13996,
suite-coverageat least refused that run.Change
ci.yml: both reuse steps now skip underunit_suite, as they already do under the full suite. The existing block comment gave the reason ("the shards need the product"), and it now coversunit-ci.ci-macos.ymlmacOS status:app-host-unit-testsis now required whenmacos and (full_suite or unit_suite). Any other route to a skip fails instead of passing. A caller that predates the input leaves it empty, which reads as not routed.Testing
test_a_unit_ci_run_cannot_pass_with_the_unit_tests_skippedfails on main. With each half of the fix reverted in turn, it still fails, so it catches both.tests/test_ci_change_areas.pyandtests/test_ci_product_publication.pypass.ci-guards.ymlcommands pass. The failure istest_ghostty_zig_version_sync.sh, because the ghostty submodule isn't checked out locally.actionlint: nothing beyond the SC2129 notes already on main.Demo Video
Not applicable. This changes CI routing only.
Checklist
— Ophelia g1 🍄
Run: run_cmux_main_red_triage_app_host_census_and_pr_review_20260923_07d8d17b
🤖 Generated with Claude Code
Summary by cubic
Fixes the CI hole where adding the
unit-cilabel could turn a PR fully green with no app-host unit tests run.unit_suite, matching full-suite behavior, so app-host tests always run behind a compile admission that succeeded in the same run.app-host-unit-testsunderunit_suite, so a skip fails instead of passing.Written for commit c94998d. Summary will update on new commits.
Summary by CodeRabbit