Skip to content

ci: harden R2 canary cleanup and coverage - #13342

Merged
teamleaderleo merged 9 commits into
manaflow-ai:mainfrom
teamleaderleo:chore/r2-canary-cleanup-retry
Sep 23, 2026
Merged

teamleaderleo merged 9 commits into
manaflow-ai:mainfrom
teamleaderleo:chore/r2-canary-cleanup-retry

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The canary cleanup deleted its R2 artifact once. A transient remote-delete failure could leave the bucket non-empty, so bucket deletion failed and the per-run bucket remained allocated until its lifecycle rule expired. The canary job also had too little timeout margin, cleanup treated an absent secret as an error after partial deploys, and transport checks did not run on pushes that changed .github/workflows/ci.yml alone.

Change

  • Retry the exact canary artifact deletion up to four times with short backoff before bucket cleanup.
  • Treat only Cloudflare HTTP 404 responses as an idempotent absent-secret cleanup; preserve other failures.
  • Increase the isolated canary job budget from 15 to 20 minutes.
  • Include .github/workflows/ci.yml in the main push path filter.
  • Add regression coverage for cleanup semantics, workflow coverage, timeout budget, and artifact-delete retries.

Validation

  • python3 tests/test_ci_r2_canary.py (20 tests)
  • git diff --check

@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 73dfb9ca-d143-455d-aa0c-8da741aeaac5

📥 Commits

Reviewing files that changed from the base of the PR and between 3f542e8 and 9fd4f91.

📒 Files selected for processing (3)
  • .github/workflows/ci-artifact-canary.yml
  • scripts/ci/r2-canary-cloudflare.py
  • tests/test_ci_r2_canary.py
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • 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.

@greptile-apps

greptile-apps Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the only remaining concern is a non-blocking gap in the exact artifact-key regression assertion.

Summary

This PR hardens the R2 canary cleanup path and expands its regression coverage.

  • Retries remote artifact deletion four times with incremental backoff before bucket removal.
  • Treats only Cloudflare 404 responses as successful idempotent secret cleanup.
  • Increases the canary timeout and runs transport checks when the main CI workflow changes.
  • Adds executable coverage for retry count, backoff, and terminal status.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Canary cleanup starts] --> B[Delete artifact]
  B -->|Success| C[Delete empty bucket]
  B -->|Failure and attempts remain| D[Back off]
  D --> B
  B -->|Fourth failure| E[Fail artifact cleanup]
  E --> C
  C --> F[Delete worker secrets]
  F -->|HTTP 404| G[Treat secret as already absent]
  F -->|Other error| H[Preserve cleanup failure]
Loading

Reviews (3) · Last reviewed commit: "test: exercise canary cleanup retries"

Comment thread tests/test_ci_r2_canary.py
@teamleaderleo teamleaderleo changed the title ci: retry canary artifact cleanup before bucket removal ci: harden R2 canary cleanup and coverage Sep 21, 2026
@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 21, 2026 08:30
@cursor

cursor Bot commented Sep 22, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

teamleaderleo and others added 8 commits September 22, 2026 06:40
The push-path trigger named ci.yml, but manaflow-ai#13405 moved the macOS jobs that
consume these artifacts into ci-macos.yml, so pushes that changed the
consuming lane no longer re-validated transport. Point the trigger and
its test at ci-macos.yml.

Also drop a duplicate definition of
test_workflow_retries_remote_artifact_delete_before_bucket_cleanup. The
later definition shadowed the earlier one, so the shorter version never
ran; the surviving copy is the superset that also executes the wrangler
stub.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Resolved the conflicts in 9fd4f91. The branch carried obsolete upstream history; the merge preserves current main and the four-file canary change, rather than restoring unrelated old code. One trigger change was already on main, leaving three changed files. All 20 canary tests, both workflow lint checks, and independent review pass. GitHub confirms the updated head is mergeable. Hosted native failures still require separate validation; no R2 deployment or activation was performed.

@teamleaderleo
teamleaderleo merged commit 6e0ad4f into manaflow-ai:main Sep 23, 2026
55 checks passed
@teamleaderleo
teamleaderleo deleted the chore/r2-canary-cleanup-retry branch September 23, 2026 11:22
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