[pdillinger]

Summary:
CI cost reduction and cleanup in the GitHub Actions configuration:

• Move build-macos-cmake (four test shards on macos-15-xlarge) from pr-jobs to nightly. These jobs are among the most expensive per run and rarely catch anything not caught elsewhere. PRs still run build-macos (a build without tests) and the macOS Java jobs. The job is unchanged except for using nightly's owner check (it can no longer be selected through pr-jobs' ONLY_JOB; use workflow_dispatch on nightly instead).

• Remove the post-steps composite action and its 36 call sites. It was invoked without an if: condition, so GitHub skipped it whenever an earlier step had failed, meaning it only ever ran after successful jobs. Its failure-oriented steps (copying t/ test logs, uploading core dumps) therefore never did anything. On success it uploaded the gnu_parallel job log (misleadingly named "db-log-file") and gtest XML results, neither of which is consumed by anything, and the macOS Java jobs that call it produce neither. Failing test output is already printed in the job console by make check. The GTEST_OUTPUT setting in pre-steps existed only to produce the uploaded XML, so it is removed too. Rather than fixing post-steps to run on failure, this change drops it because what it would collect is redundant with the console output.

• Bonus: the pre-steps diagnostics step now logs OS and CPU details alongside the existing ulimits, to help diagnose failures that depend on the runner environment.

Test Plan:
CI on this PR validates the workflow YAML (make check-workflow-yaml in check-format-and-targets) and exercises pre-steps and the remaining jobs without post-steps. The relocated build-macos-cmake won't run before merge; parsing the workflows confirmed the nightly job is identical to the removed pr-jobs job apart from its owner-check condition. A manual workflow_dispatch of nightly after merge can confirm it runs.

[meta-codesync]

@pdillinger has imported this pull request. If you are a Meta employee, you can view this in D128353844.

[github-actions]

✅ clang-tidy: No findings on changed lines

Completed in 0.0s.

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 5507648

---

Summary

Clean CI cleanup PR. The three changes (move macOS CMake tests to nightly, remove dead post-steps action, add system diagnostics to pre-steps) are well-motivated and correctly implemented. The build-macos-cmake job is faithfully transplanted to nightly. The post-steps removal is safe given the action's inability to fire on failure. The pre-steps diagnostics addition is harmless and useful.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🟡 MEDIUM

M1. Residual post-steps references in candidate workflows — nightly-candidate.yml:19, pr-jobs-candidate.yml:17,47

• Issue: The diff removes post-steps from these candidate workflow files. However, the post-steps/action.yml file itself is also deleted. After merge, any workflow file that still references post-steps will fail at the uses: step because the action no longer exists. The candidate workflows (nightly-candidate.yml, pr-jobs-candidate.yml) are workflow_dispatch only and gated on repository_owner == 'meta', so the blast radius is limited to manual dispatch on the meta fork. This appears correctly handled in the diff — all three candidate references are removed. No issue if the diff is applied cleanly.
• Root cause: N/A — this is a verification note, not a bug. All call sites are covered.
• Suggested fix: None needed.

M2. build-format-compatible in nightly still has post-steps in the on-disk file — nightly.yml:29

• Issue: The diff removes the post-steps reference from build-format-compatible in nightly.yml. However, the on-disk file at the time of this review still has it (line 29). This is expected if the diff has not been applied yet. Verify this removal is included in the final merge.
• Root cause: Review is against the diff, not the applied state.
• Suggested fix: None — just confirming the diff covers this.

🟢 LOW / NIT

L1. grep used directly in pre-steps diagnostic — pre-steps/action.yml:14

• Issue: The new diagnostics step uses grep -m1 '^microcode' /proc/cpuinfo || true. This is fine for a diagnostic step (not data processing), but note that grep is the system grep, not rg. Since this is a shell step in CI (not a code search tool invocation), this is perfectly appropriate.
• Suggested fix: None needed.

L2. No if: always() on the diagnostics step — pre-steps/action.yml:5

• Issue: The diagnostics step lacks if: always(). If an earlier step in pre-steps fails (e.g., lld install), this step won't run. However, since this step comes first in the sequence (before lld install and env setup), and uses || true throughout, this is not practically an issue.
• Suggested fix: None needed — step ordering handles this.

L3. build-macos-cmake in nightly lacks ccache teardown if: always() consistency — nightly.yml:169

• Issue: The migrated build-macos-cmake job in nightly includes teardown-ccache with if: always(), which is correct and consistent with the original in pr-jobs.
• Suggested fix: None — this is a positive observation.

Cross-Component Analysis

This PR changes only CI workflow configuration (GitHub Actions YAML). No C++ code, no database logic, no API surfaces are affected. The DB mutex, concurrency, serialization, and performance perspectives are not applicable.

Aspect	Affected?	Notes
C++ code	No	Pure CI config change
Public API	No	No API changes
DB mutex	No	No runtime code
Performance	No	CI cost reduction only
Backwards compat	No	No behavioral changes
Serialization	No	No format changes

Positive Observations

• The PR description clearly explains why post-steps was ineffective (no if: condition meant GitHub skipped it on failure, so its failure-oriented steps never executed).
• The GTEST_OUTPUT removal is correctly scoped — it only existed to feed the XML upload in post-steps, so removing both together is clean.
• The build-macos-cmake migration preserves the job definition exactly (same runner, same xcode version, same sharding, same ccache config), only changing the trigger condition from pr-jobs' ONLY_JOB check to nightly's repository_owner check, which is appropriate.
• The new pre-steps diagnostics (uname -a, lscpu/sysctl) will be genuinely useful for debugging runner-environment-dependent failures.

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/ci_review_prompt.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase