ci: require Linux matrix success in aggregate checks - #3219
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 automation
|
Codex review: needs maintainer review before merge. Reviewed August 26, 2026, 7:03 PM ET / 23:03 UTC. ClawSweeper reviewWhat this changesThe PR makes the aggregate CI check require a successful glibc Linux matrix, expands result-state verification, and stabilizes related portable and macOS test fixtures. Merge readinessKeep open pending completion of the exact-head Linux and macOS CI jobs; source review found no actionable patch defect. Likely related people: steipete (high confidence; current workflow and test contributor). Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar’s GitHub Actions workflow combines lint, change detection, platform tests, and Linux builds into an aggregate branch-protection check. The changed verifier consumes those job results and decides whether the aggregate check succeeds. flowchart LR
A[Source changes] --> B[CI path detection]
B --> C[Linux glibc matrix]
B --> D[macOS and musl jobs]
C --> E[Aggregate CI verifier]
D --> E
E --> F[Branch protection check]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge only after the exact PR head shows successful aggregate, Linux x64/ARM, and macOS results, preserving the fail-closed aggregate contract. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: the base aggregate lacked the Linux matrix dependency/result, while the focused contract script exercises the relevant failing result states. Is this the best way to solve the issue? Yes: adding the existing matrix as an aggregate dependency and requiring its explicit success is the narrowest way to prevent a false-green aggregate check. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4e1f7a3ef362. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles) |
|
Merged as abe8f9180a75. Local main fast-forwarded and is clean. The aggregate now waits for the regular Linux matrix and rejects every result other than success. The existing portable localhost-session test runs in the shared macOS/Linux target and holds its synthetic loopback port until all requests finish. No provider networking, branch protection, release artifacts, or production scroll behavior changed. Validation:
The initial full suite exposed existing host-dependent coarse CGEvent conversion: some synthetic events produced zero NSEvent deltas. A separate test-only follow-up now supplies and verifies deterministic NSEvent inputs, isolates settings/account state, and avoids whole-controller assertion dumps. The CoreGraphics trigger is not diagnosed. The first suite failed on its built-in retry; the final suite above passed without retries. CI start was delayed after the follow-up push, so the same PR was reopened without changing the commit. One same-head run was superseded/cancelled; the watcher later recovered from a local network error. Neither was a test failure. Final exact-head CI at Related #2243 remains open: a passing retry of the earlier ARM/glibc dispatch crash is not root-cause proof for that issue's x64/musl production crash. No real provider account, saved credentials, administrator installer execution, or app relaunch was used. |
1 participant
Summary
Make the existing
lint-build-testaggregate wait for and validate the glibc Linux build/test matrix. Previously it could report success while a regular Linux job failed. The check name, matrix fail-fast setting, macOS/musl path rules, draft behavior and branch-protection settings are unchanged.The verifier now requires an explicit successful Linux result. Tests cover failure, cancellation, skipped, empty, missing and unknown results across the existing valid path combinations, preserve the older failure cases, and verify the real workflow dependency and argument wiring.
Move the existing portable
AntigravityLocalhostSessionLifetimeTestsintoCodexBarLinuxTests, which runs on macOS and Linux, instead of leaving this Linux-related regression only in the macOS app target. Keep its synthetic loopback socket bound but non-listening until all concurrent requests finish, preventing port reuse during the test. Document the CI contract.Full-suite validation also exposed a host-dependent Overview scroll test fixture: synthetic coarse CGEvents sometimes converted to zero NSEvent scroll deltas. A separate test-only follow-up now supplies deterministic NSEvent properties, validates those inputs, isolates settings/account state, and preserves all navigation assertions. Boolean assertions avoid dumping controller/store graphs. Production scroll behavior is unchanged; the underlying CoreGraphics trigger is not diagnosed or claimed fixed.
Related context: #2243. This does not identify or fix the cause of the intermittent Linux dispatch crash, change provider networking, upgrade toolchains, or add blanket test serialization/retries. The observed ARM/glibc test failure passed on one exact-job retry; that is not root-cause proof.
Verification
bash -n Scripts/ci_verify_test_jobs.sh Scripts/test_ci_path_gate.shpassed../Scripts/test_ci_path_gate.shpassed, including result matrix and workflow-wiring checks.CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter AntigravityLocalhostSessionLifetimeTestspassed both moved tests on macOS. Initial-head Linux x64/ARM CI also passed the moved suite; final-head CI remains required.make checkpassed with zero violations; the moved file also received explicit formatting/lint checks.make teststopped at group 68 on three existingStatusMenuOverviewScrollTestscases, repeated on the harness's group retry. The unchanged focused run reproduced them; safe probes observed zero coarse-event deltas and later normal values. The test-only follow-up passed the focused 10-test suite (including 12 fixture input combinations), the exact 115-test/11-suite group,make check, and independent review without retries.make testpassed all 933 selections in 78 groups: every group first-pass, zero failures, retries or timeouts, 799.2 seconds. Run with test Keychain access suppressed and the allow override unset.9582144959ef1e479d0edfa3023a6892a3e78117passed: lint, Linux x64/ARM builds/tests/smoke checks, both macOS shards and provider-engine goldens, and the aggregate. Musl was correctly skipped by the unchanged path gate. GitHub initially did not create a run after the follow-up push; reopening the same PR retriggered CI without changing the commit. An earlier same-head run was superseded/cancelled, not a test failure; the final watcher recovered from a local network error.No real provider account, saved credential, app restart, or external network probe was used. The loopback test uses only its reserved synthetic local port. No changelog entry is needed for this internal CI/test-only change.