fix(plugins): publish plugin bundles to R2 by default#74
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
corvus-plugins-edge | 4d4a314 | Feb 24 2026, 11:55 AM |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request changes the plugin publishing workflow to upload immutable artifacts and mutable metadata to Cloudflare R2 (primary path), adds catalog integrity verification and public Worker smoke-checks, and makes the legacy Cloudflare Pages catalog UI deployment optional; docs and workflow inputs/validation were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
actor GH as GitHub Actions
participant R2 as Cloudflare R2
participant Verify as Verification Job
participant Worker as Public Worker
participant Pages as Cloudflare Pages
GH->>R2: Upload artifact, signature, certificate, manifest
GH->>R2: Upload catalog.json & revocations.json (atomic)
GH->>Verify: Request catalog integrity check
Verify->>R2: Fetch catalog.json & revocations.json
R2-->>Verify: Return assets
Verify-->>GH: Validation result
GH->>Worker: Smoke-check public Worker endpoints (R2-backed URLs)
Worker-->>GH: Accessibility result
alt deploy_cloudflare_pages == true
GH->>Pages: Build & deploy legacy UI
Pages-->>GH: Deployment result
end
GH->>GH: Upload build artifacts for traceability
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-24 to 2026-02-24 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/publish-plugins.yml (2)
368-385: Catalog assembled from local git checkout, not from R2 — sequential plugin publishes can lose entriesThe bundle step reads
clients/web/apps/plugins/public/catalog.jsonfrom the checked-out repo. With R2 now the source of truth, any plugin published after the last repo-side commit will not be included in this base, and its entry will be silently dropped when the next plugin is published.
concurrency: cancel-in-progress: trueprevents overlapping runs but not back-to-back runs from different commits (or the same commit for multiple plugin tags).The fix is to download the current
catalog/catalog.jsonfrom R2 as the base before merging in the new entry. Pnpm/Node isn't available yet at this step, but the Cloudflare REST API can be hit directly withcurlto read the object, or pnpm/Node setup could be moved earlier in the job.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-plugins.yml around lines 368 - 385, The current logic uses the repo's clients/web/apps/plugins/public/catalog.json (source_catalog) as the base which can miss entries already published to R2; change the base-loading code to first attempt fetching the canonical catalog from R2 (e.g., curl the Cloudflare R2 REST URL for catalog/catalog.json) and parse that JSON into catalog_payload; if the R2 fetch fails or returns non-JSON fall back to reading source_catalog or to {"plugins": []}; then continue using existing_plugins, filtered, and catalog_out as before to merge the new plugin_entry and write dist_dir / "catalog.json". Ensure the R2 fetch is done before building filtered so sequential publishes use R2 as the source of truth.
554-572: Install wrangler once instead of spawning it 6 times in the upload stepThe
wrangler_put()function is called 6 times (lines 565–572), and with 2 morer2 object getcalls in the verify step plus 1pages deploycall, that's 9pnpm dlxinvocations. Each one bootstraps a new Node.js process and re-validates the package cache. Installing wrangler once and reusing the binary is significantly faster.♻️ Suggested approach
Add a dedicated install step right after the Node.js setup step, then replace all
pnpm dlx wrangler@4.44.0calls with justwrangler:+ - name: 📦 Install wrangler + run: pnpm add -g wrangler@4.44.0Then in the upload step's
wrangler_puthelper:- pnpm dlx wrangler@4.44.0 r2 object put "${CLOUDFLARE_R2_BUCKET}/${key}" \ + wrangler r2 object put "${CLOUDFLARE_R2_BUCKET}/${key}" \ --file "$file" \ --content-type "$content_type"Apply the same substitution to the
r2 object getcalls in the verify step and thepages deploycall in the Pages deployment step.Additionally,
wrangler@4.44.0is pinned but the current latest release is4.65.0. The intervening releases (4.45.0, 4.56.0, 4.57.0) include R2 binding provisioning features and snapshot-expiration API improvements. Upgrading would bring incremental improvements, though the corer2 object put/getcommands remain compatible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-plugins.yml around lines 554 - 572, Replace repeated 'pnpm dlx wrangler@4.44.0' invocations by installing wrangler once and reusing the binary: add an install step after Node setup that runs 'pnpm dlx -y wrangler@4.65.0 --no-install' or the equivalent to place a persistent 'wrangler' binary on PATH, then update the wrangler_put() helper and all other call sites (the 'wrangler_put' function body, all calls to wrangler_put, the 'r2 object get' verify calls, and the 'pages deploy' call) to invoke 'wrangler' directly instead of 'pnpm dlx wrangler@4.44.0'; also bump the pinned version to 4.65.0 to pick up recent R2 improvements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish-plugins.yml:
- Around line 592-637: The inline Python verification reads TMP_CATALOG_PATH and
validates catalog.json but never validates the downloaded revocations.json;
extend the same Python block to load and parse the revocations file
(tmp_dir/revocations.json or a new env var like TMP_REVOCATIONS_PATH), ensure it
is valid JSON and has the expected top-level structure (e.g., a "revocations"
key that is a list or the schema your pipeline expects), and call SystemExit
with a descriptive error if the file is malformed or missing required fields;
update the inline script variables and validation logic near the existing
catalog checks (look for the python block and uses of TMP_CATALOG_PATH, tmp_dir,
and "revocations.json").
In `@clients/web/apps/docs/src/content/docs/es/guides/plugins.md`:
- Around line 159-161: Fix inconsistent spacing around the label/code span in
the repository variable bullet list by ensuring each item uses the same pattern
"Label: `CODE`" with a single space after the colon; update the items that
mention CLOUDFLARE_R2_BUCKET_NAME and CLOUDFLARE_PAGES_PROJECT_NAME so both have
exactly one space after the colon before the backtick-wrapped variable name to
match the other list entries.
- Around line 130-134: Fix the minor Markdown formatting: remove the extra
leading whitespace before the parenthetical continuation in the "8. Sube
artefactos..." list item so the continuation aligns with the item text, ensure
there is a blank line (or proper newline) separating "9. Verifica integridad del
catálogo..." and "10. Opcionalmente..." so the numbered items render separately,
and delete the stray space immediately before the backtick in the "(
`deploy_cloudflare_pages=true` )" phrase so the code span is contiguous.
---
Nitpick comments:
In @.github/workflows/publish-plugins.yml:
- Around line 368-385: The current logic uses the repo's
clients/web/apps/plugins/public/catalog.json (source_catalog) as the base which
can miss entries already published to R2; change the base-loading code to first
attempt fetching the canonical catalog from R2 (e.g., curl the Cloudflare R2
REST URL for catalog/catalog.json) and parse that JSON into catalog_payload; if
the R2 fetch fails or returns non-JSON fall back to reading source_catalog or to
{"plugins": []}; then continue using existing_plugins, filtered, and catalog_out
as before to merge the new plugin_entry and write dist_dir / "catalog.json".
Ensure the R2 fetch is done before building filtered so sequential publishes use
R2 as the source of truth.
- Around line 554-572: Replace repeated 'pnpm dlx wrangler@4.44.0' invocations
by installing wrangler once and reusing the binary: add an install step after
Node setup that runs 'pnpm dlx -y wrangler@4.65.0 --no-install' or the
equivalent to place a persistent 'wrangler' binary on PATH, then update the
wrangler_put() helper and all other call sites (the 'wrangler_put' function
body, all calls to wrangler_put, the 'r2 object get' verify calls, and the
'pages deploy' call) to invoke 'wrangler' directly instead of 'pnpm dlx
wrangler@4.44.0'; also bump the pinned version to 4.65.0 to pick up recent R2
improvements.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/README.md.github/workflows/publish-plugins.ymlclients/web/apps/docs/src/content/docs/en/guides/plugins.mdclients/web/apps/docs/src/content/docs/es/guides/plugins.md
Deploying corvus-plugins with
|
| Latest commit: |
4d4a314
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f7988219.corvus-plugins.pages.dev |
| Branch Preview URL: | https://fix-plugins-r2-publish-defau.corvus-plugins.pages.dev |
Deploying corvus with
|
| Latest commit: |
4d4a314
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://10c21f74.corvus-42x.pages.dev |
| Branch Preview URL: | https://fix-plugins-r2-publish-defau.corvus-42x.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/publish-plugins.yml (1)
633-669:⚠️ Potential issue | 🟡 MinorAlso validate
revocations.updated_at.You parse
revocations.json, butupdated_atis never validated. A malformed file could pass the check while still missing required metadata.✅ Proposed addition
if "revoked" not in revocations: raise SystemExit("revocations payload missing required key: revoked") if not isinstance(revocations.get("revoked"), list): raise SystemExit("revocations/revoked must be a list") + if not revocations.get("updated_at"): + raise SystemExit("revocations payload missing required key: updated_at")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-plugins.yml around lines 633 - 669, The revocations JSON currently isn't validating the updated_at field; after checking revocations is a dict and validating "revoked" is a list, add a check that revocations contains the "updated_at" key and that its value is a valid timestamp (e.g., non-empty string or integer) — use revocations.get("updated_at") to fetch it and raise SystemExit with a clear message like "revocations payload missing required key: updated_at" or "revocations/updated_at must be a valid timestamp" if the validation fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish-plugins.yml:
- Around line 371-390: The remote catalog fetch currently falls back silently to
the local repo copy on any error; change the logic around
catalog_base_url/remote_catalog_url/catalog_payload so that if
urllib.request.urlopen raises urllib.error.HTTPError you only accept the local
fallback when the HTTP status is 404 (resource genuinely missing) or when an
explicit env var like ALLOW_LOCAL_CATALOG_FALLBACK is set; for all other errors
(URLError, JSONDecodeError when decoding the remote payload, or non-404
HTTPError) exit/raise a non-zero failure so the workflow fails fast and does not
overwrite R2 with stale source_catalog; keep the existing local-source parsing
(source_catalog.read_text and json.loads) only for the allowed 404 or
explicit-fallback cases.
---
Duplicate comments:
In @.github/workflows/publish-plugins.yml:
- Around line 633-669: The revocations JSON currently isn't validating the
updated_at field; after checking revocations is a dict and validating "revoked"
is a list, add a check that revocations contains the "updated_at" key and that
its value is a valid timestamp (e.g., non-empty string or integer) — use
revocations.get("updated_at") to fetch it and raise SystemExit with a clear
message like "revocations payload missing required key: updated_at" or
"revocations/updated_at must be a valid timestamp" if the validation fails.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish-plugins.ymlclients/web/apps/docs/src/content/docs/es/guides/plugins.md
1 participant
This pull request updates the plugin publishing workflow to prioritize Cloudflare R2 for artifact and metadata storage, adds integrity verification steps, and makes Cloudflare Pages deployment optional and clearly separated as a legacy feature. Documentation in both English and Spanish is revised to reflect these changes and new required variables.
Workflow and deployment changes:
cloudflare_r2_bucket_name/CLOUDFLARE_R2_BUCKET_NAME) are introduced, and the Pages project variable is now optional. [1] [2] [3]Documentation updates:
.github/workflows/README.mdis revised to explain the new order of operations, required secrets/vars, and the distinction between R2 publishing and optional Pages deployment.Summary by CodeRabbit
New Features
Documentation