Skip to content

ci: skip macOS shards for docs site changes#1671

Open
Yuxin-Qiao wants to merge 2 commits into
steipete:mainfrom
Yuxin-Qiao:codex/ci-macos-risk-buckets
Open

ci: skip macOS shards for docs site changes#1671
Yuxin-Qiao wants to merge 2 commits into
steipete:mainfrom
Yuxin-Qiao:codex/ci-macos-risk-buckets

Conversation

@Yuxin-Qiao

@Yuxin-Qiao Yuxin-Qiao commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • extend the macOS path gate from Markdown-only changes to docs site static changes already covered by portable Linux checks
  • keep contributor/runtime configuration contracts, unknown docs code, source, scripts, workflows, packages, and tests on macOS
  • print the first path/reason when macOS shards are required so queued checks are easier to explain

Safety

  • docs/site.js remains covered by node --check in lint-linux
  • docs links and site locale structure remain covered by portable checks
  • unknown docs code like docs/custom-tool.js still requires macOS

Validation

  • bash -n Scripts/ci_macos_test_gate.sh
  • bash -n Scripts/test_ci_path_gate.sh
  • ./Scripts/test_ci_path_gate.sh
  • git diff --check
  • ./Scripts/lint.sh lint-linux ran portable checks and SwiftLint with 0 violations; local sandbox still returns nonzero when a tool tries to write a macOS plist cache outside the workspace after linting

Behavior proof

@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 20, 2026, 12:20 AM ET / 04:20 UTC.

Summary
The PR updates the macOS CI path gate and its tests so selected docs site files and one-level docs image assets skip macOS Swift shards while unknown docs code and contract files still require them.

Reproducibility: yes. for the CI behavior: current main requires macOS for docs site code, while the PR allow-lists selected docs site paths. The linked proof run demonstrates a docs/site.css-only change skipping swift-test-macos while the aggregate job passes.

Review metrics: 2 noteworthy metrics.

  • CI gate diff: 3 files changed, 28 additions, 7 deletions. The touched surface is small and limited to CI gate scripts and fixtures, so review centers on automation policy.
  • Skip allow-list: 9 named docs site files plus 6 one-level docs image extensions. Maintainers should notice exactly how far the macOS shard skip expands before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Have a maintainer explicitly accept the expanded macOS skip policy for docs site files and one-level docs image assets.

Risk before merge

  • [P1] Merging intentionally lets docs/site.js, CSS/HTML/site metadata, and one-level docs image assets bypass macOS Swift shards when Linux portable checks pass; this is a CI branch-protection policy choice, not only a script implementation detail.

Maintainer options:

  1. Accept the expanded docs-site skip
    Maintainers can merge if they agree the listed docs site files and one-level docs image assets are sufficiently covered by portable Linux checks without macOS Swift shards.
  2. Narrow the allow-list before merge
    If maintainers want stricter branch protection, keep fewer docs site paths in the skip list and continue requiring macOS for anything with uncertain coverage.

Next step before merge

  • No automated repair is needed; the remaining action is maintainer acceptance of the expanded CI skip policy.

Security
Cleared: The diff changes existing CI shell scripts and tests only, with no new action refs, dependencies, permissions, secret handling, or downloaded executable path.

Review details

Best possible solution:

Merge after maintainers accept the expanded docs-site skip policy and exact-head required checks pass; otherwise narrow the allow-list while preserving fail-closed handling for unknown docs code and contract files.

Do we have a high-confidence way to reproduce the issue?

Yes for the CI behavior: current main requires macOS for docs site code, while the PR allow-lists selected docs site paths. The linked proof run demonstrates a docs/site.css-only change skipping swift-test-macos while the aggregate job passes.

Is this the best way to solve the issue?

Yes, if maintainers accept the CI policy change. The implementation is narrow, keeps fail-closed behavior for unknown docs code and contract files, and adds focused gate fixtures plus updated aggregate wording.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0f85e05f1c3d.

Label changes

Label changes:

  • add P2: This is a bounded CI improvement with meaningful automation coverage implications but no direct app runtime regression.
  • add merge-risk: 🚨 automation: The diff changes which pull request file sets request macOS Swift shards, affecting CI and branch-protection coverage after merge.

Label justifications:

  • P2: This is a bounded CI improvement with meaningful automation coverage implications but no direct app runtime regression.
  • merge-risk: 🚨 automation: The diff changes which pull request file sets request macOS Swift shards, affecting CI and branch-protection coverage after merge.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): Sufficient linked artifact: the proof PR changes only docs/site.css and its Actions run shows macOS skipped while portable checks and the aggregate job pass.
  • proof: sufficient: Contributor real behavior proof is sufficient. Sufficient linked artifact: the proof PR changes only docs/site.css and its Actions run shows macOS skipped while portable checks and the aggregate job pass.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was present and fully read; its guidance to prefer focused CLI checks for CLI-testable behavior applies to this CI gate review. (AGENTS.md:1, 0f85e05f1c3d)
  • Current main gate behavior: Current main skips Markdown except contract files and requires macOS for other paths, including docs site JavaScript before this PR. (Scripts/ci_macos_test_gate.sh:21, 0f85e05f1c3d)
  • Workflow wiring: The CI workflow derives macOS shard execution from the changes job output and verifies the lint/changes/macOS result tuple in the aggregate job. (.github/workflows/ci.yml:61, 0f85e05f1c3d)
  • Portable docs coverage: lint-linux runs the CI path-gate test, documentation link checks, site locale checks, and node syntax checking for docs/site.js. (Scripts/lint.sh:49, 0f85e05f1c3d)
  • Live PR diff: The PR allow-lists exact docs site files and one-level docs image extensions, records the first macOS-required reason, and keeps unknown docs code on macOS. (Scripts/ci_macos_test_gate.sh:32, c7a347405ed0)
  • Gate fixtures: The PR adds focused fixtures for docs site allow-listing, image assets, unknown docs code, mixed config changes, and docs-to-site renames. (Scripts/test_ci_path_gate.sh:31, c7a347405ed0)

Likely related people:

  • Yuxin-Qiao: Authored the merged docs-only macOS gate and several recent Linux/macOS CI changes in the same area, so they are strongly connected to the behavior under review. (role: recent CI path-gate contributor; confidence: high; commits: 669d1e9b08ee, 10dd11fb7e67, 5e38f6beb98d; files: Scripts/ci_macos_test_gate.sh, Scripts/test_ci_path_gate.sh, Scripts/ci_verify_test_jobs.sh)
  • steipete: Co-authored the merged path-gate work and authored recent CI sharding and macOS test-flow commits around the affected workflow. (role: adjacent CI workflow contributor; confidence: high; commits: 669d1e9b08ee, 63e473f7123d, 3f5cf401111d; files: .github/workflows/ci.yml, Scripts/lint.sh, Scripts/test.sh)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@Yuxin-Qiao

Copy link
Copy Markdown
Contributor Author

Proof PR for the new path gate: Yuxin-Qiao#4

It changes only docs/site.css on top of this PR branch. Observed checks from run https://github.com/Yuxin-Qiao/CodexBar/actions/runs/27858321694:

  • changes: pass
  • lint: pass
  • lint-build-test: pass
  • swift-test-macos: skipping

The remaining Linux CLI build jobs were still pending when checked, but the macOS queue behavior is already verified: docs site-only changes no longer request macOS Swift shards.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 20, 2026
@Yuxin-Qiao

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 20, 2026
@Yuxin-Qiao

Copy link
Copy Markdown
Contributor Author

Addressed the stale aggregate wording in c7a34740:

  • Scripts/ci_verify_test_jobs.sh now says docs/site-only changes for the false:skipped success path
  • bash -n Scripts/ci_verify_test_jobs.sh
  • bash -n Scripts/test_ci_path_gate.sh
  • ./Scripts/test_ci_path_gate.sh
  • ./Scripts/ci_verify_test_jobs.sh success success false skipped now prints the updated docs/site-only message
  • ./Scripts/ci_verify_test_jobs.sh success success true success still passes

New exact-head CI is running at https://github.com/steipete/CodexBar/actions/runs/27859863170; macOS shards are currently queued as expected because this PR still changes Scripts/.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jun 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant