Skip to content

ci: ignore a GitHub API error in the stale-run check - #14603

Merged
teamleaderleo merged 3 commits into
mainfrom
ci/stale-run-check-api-errors
Sep 25, 2026
Merged

teamleaderleo merged 3 commits into
mainfrom
ci/stale-run-check-api-errors

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Why

#14486's compile admission failed in its first step (job 108058643725):

Refusing stale PR run: run head cf20bda571...; current head {
	"message": "API rate limit exceeded for installation. ...

gh api --jq prints an API error body on stdout and exits non-zero. The step's || true kept that body as the "current head", so any rate limit or outage made the run refuse itself as stale. The same pattern is in remote-daemon.yml.

What

Both "Reject stale pull request rerun" steps treat anything but a 40-hex SHA as an unresolved head. That path already exists: it prints "Could not resolve run/PR head identity" and continues with normal CI.

The step is in NON_PRODUCT_RECIPE_STEPS, so product identity and seeds are unchanged.

Testing

Two commits: 103ed32d1b4 adds test_stale_run_check_ignores_github_api_errors, which runs both steps against a fake gh (API error, current head, newer head). It fails on that commit with the error above and passes on 75284a9b0af. actionlint, test_ci_fork_runner_routing.py, test_ci_guard_workflow_structure.py and test_ci_app_host_identity.sh pass.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes the stale-run check reading a GitHub API error as the current PR head, so a rate limit or outage no longer makes compile admission refuse its own run as stale. The step now only accepts a 40-hex SHA as a head; any other output falls through to the existing normal-CI path.

  • Applies the same guard to the "Reject stale pull request rerun" steps in ci-macos.yml and remote-daemon.yml.
  • Adds a test covering API error, current head, and newer head cases against a fake gh, and updates the existing remote-daemon stale-run test to use real 40-hex SHAs.

Written for commit be62096. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved stale pull request checks: invalid or unavailable identity data no longer incorrectly marks a run as stale, allowing normal CI to continue.
    • Stale status is determined by comparing valid commit identifiers.

teamleaderleo and others added 2 commits September 25, 2026 08:19
On a rate limit `gh api --jq` prints the error body on stdout and exits
non-zero. The step's `|| true` kept that body as the current PR head, so
#14486's compile admission refused its own current run as stale (job
108058643725). This test fails until the step ignores non-SHA output.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`gh api --jq` prints an API error body on stdout, and `|| true` kept it, so
a rate limit made compile admission (and the remote-daemon admission) refuse
the PR's current run as stale. Treat anything but a 40-hex SHA as an
unresolved head, which already continues with normal CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4a3a0cc8-f8db-4810-b6e9-d38f323c8bc5

📥 Commits

Reviewing files that changed from the base of the PR and between c0aaac7 and be62096.

📒 Files selected for processing (3)
  • .github/workflows/ci-macos.yml
  • .github/workflows/remote-daemon.yml
  • tests/test_ci_change_areas.py

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Both CI workflows now accept API-returned head values only when they match a 40-character lowercase hexadecimal SHA. Regression tests cover API failures, matching heads, and differing heads.

Changes

Stale rerun identity checks

Layer / File(s) Summary
Validate head SHA and test stale-rerun outcomes
.github/workflows/ci-macos.yml, .github/workflows/remote-daemon.yml, tests/test_ci_change_areas.py
Both workflows clear API values that do not match the SHA format. Tests check that API failures and matching SHAs pass, while a different SHA is treated as stale. Remote-daemon fixtures use 40-character SHAs.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to be620

No actionable issue is identified that would hold up merging the stale-rerun checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to be620

API errors will no longer incorrectly stop current CI runs, but they can also let an older run proceed when its head cannot be checked. Confirmed stale heads are still rejected, and no new privilege or verified security exploit was identified.

Retained concerns

  • Low · security · inferred: When the current-head lookup returns a non-SHA error body, an older PR run can now pass admission and execute downstream CI work. This widens the existing fail-open case for unresolved identities; it does not establish an attacker-controlled bypass or new privilege.
Security review details

Security Blast Radius

  • inferred — The conditional exposure is CI execution for an older PR run when head lookup fails with non-SHA output: remote-daemon tests or macOS compile and dependent jobs can proceed. The evidence does not establish access beyond the authority those jobs already have.

Security Findings and Attack Paths

  • inferred — A stale run plus an API response containing an error body is the changed admission path: the body is discarded, so the gate succeeds without comparing heads. Whether an actor can independently arrange that lookup failure, or turn the resulting CI execution into a security compromise, is not established.

Trust Boundaries and Controls

  • observed — The gates still reject confirmed differing heads. Admission dependencies, PR-run cancellation, and fork-specific runner routing provide independent limits, although they do not resolve an unknown head at the gate.

Resilience and Maintainability Implications

  • inferred — Clearing malformed API output avoids treating a rate-limit body as a different commit, but preserves an indeterminate admission result across retries: each attempt depends on whether the lookup resolves at that time. The tests establish script exit behavior, not outcomes under live API failure or concurrent runs.

Hardening Proposals

  • proposed — If rejecting stale execution is a security requirement rather than a best-effort resource guard, retry identity lookups with a bound and give unresolved identity an explicit non-admission outcome. That would be a policy change from the pre-existing fail-open behavior.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing GitHub API errors from being treated as stale-run identities.
Description check ✅ Passed The description explains the problem, resulting behavior, affected workflows, and testing. It does not use the template's exact Summary heading and omits the checklist, but it is otherwise complete an…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The pull request changes only stale-run validation in two GitHub Actions workflows and related tests. It does not change Cloud terminal creation, cmux-tui transport, manual renderers, PTY readin…
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes only two GitHub Actions YAML workflows and one Python test. The authoritative diff contains no Swift files and no Swift declarations, actor-isolation annotations, models…
Cmux Swift Blocking Runtime ✅ Passed The pull request changes only two GitHub Actions YAML workflows and one Python test file. The authoritative diff contains no Swift files or production Swift code. The added shell validation is CI sync…
Cmux Browser Automation Off-Main ✅ Passed PASS: The PR changes only GitHub workflow stale-run SHA validation and its Python regression tests. The diff adds no browser.* socket command, WebKit/AppKit access, mainActor/processV2Command routing,…
Cmux Expensive Synchronous Load ✅ Passed The pull request changes only two GitHub workflow YAML files and one Python test file. The authoritative diff contains no production Swift changes and no agent-history, transcript, JSONL, directory-sc…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only GitHub Actions YAML files and a Python test file. It does not change production Swift, TypeScript, or JavaScript code, so the cache-substitution correctness conditi…
Cmux No Hacky Sleeps ✅ Passed The pull request adds SHA validation to GitHub Actions workflow shell steps and adds deterministic test scaffolding. The rule explicitly excludes GitHub Actions workflow YAML, and the test changes int…
Cmux Algorithmic Complexity ✅ Passed The production diff adds only fixed-length SHA regex checks to two shell admission steps. These checks operate on scalar API outputs and add no loops, collection scans, sorting, filtering, joins, or s…
Cmux Swift Concurrency ✅ Passed The pull request changes only two GitHub Actions YAML workflows and one Python test file. The diff adds shell SHA validation and regression tests; it introduces no cmux-owned Swift code, Dispatch queu…
Cmux Swift @Concurrent ✅ Passed The pull request changes only GitHub workflow YAML and Python test code. It introduces no Swift files, Swift declarations, or Swift call sites, so the @concurrent rule does not apply.
Cmux Swift Package Boundaries ✅ Passed The pull request changes only two GitHub workflow files and one Python test. The authoritative diff contains no Swift or SwiftPM files and introduces no production Swift logic. Therefore, the Swift pa…
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only two workflow files and one Python test. The diff contains no Package.swift, Package.resolved, .gitignore, cmux.xcodeproj, SwiftPM, or package-reference changes. Therefore, …
Cmux Swift Logging ✅ Passed PASS: The pull request changes only GitHub workflow YAML and Python tests; it adds no production Swift code or Swift logging. The added shell echo statements are CI output, and the test harness outp…
Cmux User-Facing Error Privacy ✅ Passed PASS. The diff changes only two GitHub Actions admission workflows and a regression test. The added output is internal CI diagnostics, with no concrete path to a cmux end user. The API error body is d…
Cmux Full Internationalization ✅ Passed PASS: The PR changes only GitHub Actions workflow checks and Python regression tests. The added text is operational CI output and test documentation, not production Swift, web UI, metadata, API respon…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only two YAML workflows and one Python test. The authoritative diff contains no Swift or SwiftUI files and no SwiftUI state or layout constructs. Therefore, the SwiftUI …
Cmux Architecture Rethink ✅ Passed The pull request changes only two GitHub Actions workflow files and a Python test file. It introduces no Swift code or Swift lifecycle changes, and it does not introduce sleeps, polling, locks, observ…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The pull request changes only GitHub workflow YAML and Python tests. The diff contains no Swift changes and no user-visible auxiliary window code. The cmux auxiliary-window close-shortcut rule is not …
Cmux Source Artifacts ✅ Passed The pull request changes only two GitHub workflow configuration files and one hand-written Python test file. The diff adds SHA validation to existing CI checks and regression coverage for API-error be…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only two workflow YAML files and one Python test file. It does not change any Swift file under a production Sources/ path, so the custom check is not applicable.
Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

The stale-run check now treats anything but a 40-hex SHA as an unresolved
head, so the placeholder heads "head" and "newer-head" read as unresolved
and the stale case passed. Use 40-hex values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 25, 2026 13:03
@teamleaderleo
teamleaderleo merged commit 99c4404 into main Sep 25, 2026
127 of 149 checks passed
@teamleaderleo
teamleaderleo deleted the ci/stale-run-check-api-errors branch September 25, 2026 14:41
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for be62096b6e: every check was green at merge (17 verified; 14 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 25, 2026
74b3778 test: restore manaflow-ai#14406's sidebar AX walk assertion lost in the manaflow-ai#14408 squash (manaflow-ai#14593)
3b14475 ci: run swift-package-tests on owned minis when the run builds no Release helper (manaflow-ai#14411)
2a40caa Handle WebAuthn assertions without user handles (manaflow-ai#9060)
6cdb469 Match upload rules on HostName when a broker rewrites the host (manaflow-ai#11477)
265bef2 fix: hide browser affordances while the browser is disabled (manaflow-ai#10866) (manaflow-ai#13023)
99c4404 ci: ignore a GitHub API error in the stale-run check (manaflow-ai#14603)
ceae537 test(cloud): bind the first workspace receipt before discovery (manaflow-ai#14618)

# Conflicts:
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci.yml
#	.github/workflows/remote-daemon.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant