feat(#5633): add --platform=cloudflare to mint enroll/unenroll - #6137
feat(#5633): add --platform=cloudflare to mint enroll/unenroll#6137fullsend-ai-coder[bot] wants to merge 3 commits into
Conversation
|
🤖 Finished Review · ✅ Success · Started 9:20 AM UTC · Completed 9:40 AM UTC Commit: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
Resolved prior findings
Previous runReviewFindingsMedium
Low
Resolved prior findings
Previous run (2)ReviewFindingsLow
Resolved prior findings
Previous run (3)ReviewFindingsLow
Resolved prior findings
Previous run (4)ReviewFindingsMedium
Low
Resolved prior findings
Labels: PR primarily modifies Go production code and tests in the mint and CF provisioner packages. Previous run (5)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (6)ReviewFindingsMedium
Low
Resolved prior findings
Previous run (7)ReviewFindingsMedium
Low
Previous run (8)ReviewFindingsMedium
Low
Previous run (9)ReviewFindingsMedium
Low
Labels: PR modifies mint CLI enrollment commands and CF provisioner Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
component/mint
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 9:59 AM UTC · Completed 10:13 AM UTC Commit: |
🔧 Fix agent — iteration 1 (human-triggered)Addressed 10 of 12 review findings. Fixed the version metadata stamping bug, added early-return for no-op remove operations, renamed CF-suffix functions for consistency, added warnIrrelevantEnrollFlags, added missing tests, and updated three documentation files with Cloudflare mode content. Disagreed with 2 findings: skills/mint-enroll/SKILL.md is a protected path, and the race condition is an accepted limitation matching GCP. Fixed (10):
Disagreed (2):
Tests: passedNext steps:
|
- Skip writeVersionTS when Version/Commit are empty to avoid wiping Worker version metadata during enroll/unenroll - Return early from RemoveOrgFromWorker/RemoveRepoFromWorker when the target entity is not present, avoiding unnecessary redeploy - Document why RegisterRepoInWorker adds owner to ALLOWED_ORGS (diverges from GCP path by design) - Rename CF-suffix helpers to Cloudflare for naming consistency with the dispatch functions (runMintEnrollCloudflare, etc.) - Add warnIrrelevantEnrollFlags to enroll/unenroll commands - Add tests for remove operations when entity is not enrolled - Update docs/cli/mint.md with Cloudflare mode sections and flags - Update mint-administration.md with CF enrollment/unenrollment docs - Update operations.md to reflect cross-platform enroll/unenroll Addresses review feedback on #6137
|
🤖 Review · ❌ Terminated · Started 10:14 AM UTC · Ended 10:30 AM UTC Commit: |
Site previewPreview: https://a2f7925b-site.fullsend-ai.workers.dev Commit: |
Superseded by updated review
fullsend-ai-review
Bot
added
the
requires-manual-review
|
🤖 Finished Review · ✅ Success · Started 10:14 AM UTC · Completed 10:30 AM UTC Commit: |
|
/fs-fix when enrolling a repo in per-repo mode is should nit be added the ALLOWED_ORGS only to PER_REPO_WIF_REPOS , this has notihg to do with the dedicated WIF privider. |
|
🤖 Finished Fix · ✅ Success · Started 11:21 AM UTC · Completed 11:31 AM UTC Commit: |
🔧 Fix agent — iteration 2 (human-triggered)Per-repo Cloudflare enrollment no longer adds the owner org to ALLOWED_ORGS. Only PER_REPO_WIF_REPOS is updated, matching the GCP per-repo enrollment behavior. Updated provisioner logic, CLI help text, dry-run output, documentation, and tests. Fixed (5):
Tests: passedNext steps:
|
RegisterRepoInWorker no longer adds the owner org to ALLOWED_ORGS. Per-repo enrollment is independent of org-level enrollment on both GCP and Cloudflare — only PER_REPO_WIF_REPOS is updated. Updated CLI help text, dry-run output, docs, and tests to reflect that per-repo enrollment does not touch ALLOWED_ORGS. Addresses review feedback on #6137
|
🤖 Finished Review · ✅ Success · Started 11:33 AM UTC · Completed 11:50 AM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/fs-fix Replace the enroll/unenroll durable-var update path so it does not require local Worker sources or Problem
Required approach: version clone + binding update (no local sources)Follow the same pattern as
Auth: support wrangler OAuth, not only
|
|
🤖 Finished Fix · ✅ Success · Started 6:26 AM UTC · Completed 6:45 AM UTC Commit: |
🔧 Fix agent — iteration 3 (human-triggered)Replaced the enroll/unenroll var-update path to use the Cloudflare Versions API instead of wrangler deploy. The new UpdateVars method on WranglerRunner clones the deployed version's modules from the API, creates a new version with updated plain_text bindings (preserving secrets and other bindings via keep_bindings), and deploys to 100% traffic. Also added OAuth auth support via wrangler auth token fallback. All 8 findings from the human instruction addressed. Fixed (8):
Tests: passed
Decision points
Next steps:
|
…unenroll Replace the updateDurableVars implementation so enroll/unenroll no longer requires local Worker sources, WASM build artifacts, or wrangler deploy. The new path uses the Cloudflare Versions API to: 1. Fetch the currently deployed Worker's module content 2. Create a new version with updated plain_text bindings and keep_bindings for secrets/KV/DO/service bindings 3. Deploy the new version to 100% traffic Also adds ResolveCloudflareAPITokenFn which falls back to `wrangler auth token` when CLOUDFLARE_API_TOKEN is unset, enabling enroll/unenroll via wrangler OAuth sessions. Addresses human review feedback on #6137
|
🤖 Finished Review · ✅ Success · Started 6:46 AM UTC · Completed 7:02 AM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
|
/fs-fix Manual CF enroll/unenroll testing on a live worker ( Fix (high): public-mode detection in CF enroll/unenroll In Expected behavior after fix:
Update Fix (high):
Fix (medium, optional): deploy footgun
Fix (low, optional): no-op UX No-op enroll/unenroll still prints "Enrollment/Unenrollment complete" — consider "already enrolled" / "not enrolled" when vars unchanged. |
|
🤖 Finished Fix · ✅ Success · Started 12:12 PM UTC · Completed 12:23 PM UTC Commit: |
…oll/unenroll Replace IsPublicMint(ALLOWED_ORGS) with IsPublicMintRepos(PER_REPO_WIF_REPOS) in all four CF enroll/unenroll methods. On Cloudflare, public mode is PER_REPO_WIF_REPOS=* (set by mint deploy --public); ALLOWED_ORGS is the per-org/tight mode variable and has no public wildcard on CF. Also fix resolveCloudflareAPIToken to extract the last non-empty line from wrangler auth token output, which may include banner lines that corrupt the Authorization header value. Addresses review feedback on #6137
🔧 Fix agent — iteration 7 (human-triggered)Fixed 3 of 5 findings (2 high-priority). Replaced all four IsPublicMint(ALLOWED_ORGS) guards with IsPublicMintRepos(PER_REPO_WIF_REPOS) in CF enroll/unenroll methods. Fixed resolveCloudflareAPIToken to extract last non-empty line from wrangler output. Updated and expanded test coverage. Disagreed with 2 optional findings (deploy footgun and no-op UX) as out of scope. Fixed (3):
Disagreed (2):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 12:24 PM UTC · Completed 12:41 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/fs-fix Before addressing review items, squash all commits on this branch into a single commit and rebase onto Then fix all remaining relevant review findings on the squashed result (skip persona-table scope-creep in input-validation
nondeterministic-output
code-duplication
stale-docs
Tests
Skip
|
|
🤖 Finished Fix · ✅ Success · Started 6:26 PM UTC · Completed 6:46 PM UTC Commit: |
Adds --platform=cloudflare support to mint enroll and mint unenroll commands, enabling org and per-repo enrollment against durable Cloudflare Workers. Mirrors the existing platform flag on mint deploy and mint delete. Changes: - WranglerRunner interface: Added GetVars, HasPreviewVersions, UpdateVars - CF Provisioner: Added EnsureOrgInWorker, RemoveOrgFromWorker, RegisterRepoInWorker, RemoveRepoFromWorker methods - CLI enroll/unenroll: Added --platform, --worker-name, --preview flags with platform routing - Help text documents CF credentials and mutable-vs-preview model - Uses PER_REPO_WIF_REPOS for public-mode detection on CF paths Closes #5633
- Validate repo slug (gcf.ValidateRepoSlug) and reject PlaceholderOrg
in runMintEnrollRepoCloudflare and runMintUnenrollRepoCloudflare,
matching the GCP repo enroll/unenroll validation
- Validate Cloudflare account ID format (^[a-f0-9]{32}$) in
ResolveCloudflareAuth before using in API URLs
- Sort binding names in createVersionWithVars for deterministic
version metadata
- Extract shared CF enroll/unenroll preamble into prepareCFEnrollContext
helper (defaults effectiveName, constructs wrangler + provisioner,
verifies Worker exists, warns on preview versions)
- Add CF platform triage step and enrollment subsection to
skills/mint-enroll/SKILL.md
- Add CF credential footnote to docs/guides/dev/cli-internals.md
Command Decomposition table
- Add tests for ValidateAccountID, repo slug validation on CF paths,
PlaceholderOrg rejection, and deterministic binding order
Addresses review feedback on #6137
🔧 Fix agent — iteration 8 (human-triggered)Addressed all 8 human-instructed findings: squashed+rebased branch, added input validation (repo slug, PlaceholderOrg, account ID format), deterministic binding sort, extracted shared CF preamble helper, updated SKILL.md and cli-internals.md docs. Skipped 3 explicitly marked out-of-scope items (operations.md persona labels, ETag concurrency, deploy UX). Fixed (8):
Disagreed (3):
Tests: passed Decision points
Next steps:
|
978210c to
8f5ee5e
Compare
|
🤖 Finished Review · ✅ Success · Started 6:52 PM UTC · Completed 7:09 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-fix Two small fixes from the latest review on
Skip squash, protected-path, persona table, and other open items. |
|
🤖 Finished Fix · ✅ Success · Started 7:28 PM UTC · Completed 7:36 PM UTC Commit: |
- Include orphaned version ID in UpdateVars deploy error message for easier debugging when createVersionWithVars succeeds but deploy fails - Add "Enroll serially" / "Unenroll serially" callouts to the Cloudflare sections of docs/cli/mint.md, consistent with mint-administration.md Addresses review feedback on #6137
🔧 Fix agent — iteration 3 (human-triggered)Addressed both review findings: (1) included orphaned version ID in UpdateVars deploy error message for debugging, (2) added race-condition serial-operation notes to Cloudflare enroll/unenroll sections in docs/cli/mint.md. Fixed (2):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 7:37 PM UTC · Completed 7:55 PM UTC Commit: |
| // EnsureOrgInWorker adds an org to the durable Worker's ALLOWED_ORGS. | ||
| // If the org is already present (case-insensitive), this is a no-op. | ||
| // The Worker must already exist (deployed via 'mint deploy'). | ||
| func (p *Provisioner) EnsureOrgInWorker(ctx context.Context, org string) error { |
There was a problem hiding this comment.
[low] race-condition
The enroll/unenroll Cloudflare path uses a read-modify-write cycle (GetVars → merge → UpdateVars) without concurrency control. Two concurrent commands targeting the same Worker will race, and one change may be silently lost. This is an accepted CLI limitation matching the GCP path's behavior, documented in the CLI help text and the Enrollment ordering section.
| | GCP Admin (Mint) | `fullsend mint remove-role <role>` | Remove a role from the mint (deletes PEM secret by default) | | ||
| | GCP Admin (Mint) | `fullsend mint enroll <org\|owner/repo>` | Register an org or repo in the mint (does not grant Agent Platform access — use `inference provision`) | | ||
| | GCP Admin (Mint) | `fullsend mint unenroll <org\|owner/repo>` | Remove an org or repo from the mint | | ||
| | Mint Admin | `fullsend mint enroll <org\|owner/repo>` | Register an org or repo in the mint — supports `--platform=gcp` (default) and `--platform=cloudflare` | |
There was a problem hiding this comment.
[low] scope-creep
The PR renames the persona for mint enroll/unenroll from 'GCP Admin (Mint)' to 'Mint Admin' while adjacent rows (deploy, delete, add-role, remove-role, status) still use 'GCP Admin (Mint)', creating a visual inconsistency within the same table.
Suggested fix: Either rename all multi-platform mint commands to 'Mint Admin' for consistency, or keep the original label.
| // warnIrrelevantEnrollFlags prints a warning for each flag that was explicitly | ||
| // set but belongs to a different platform than the one being used. This is the | ||
| // enroll/unenroll counterpart to warnIrrelevantFlags (used by deploy/delete). | ||
| func warnIrrelevantEnrollFlags(cmd *cobra.Command, platform string) { |
There was a problem hiding this comment.
[low] naming-consistency
Function name warnIrrelevantEnrollFlags follows a slightly different pattern than the existing warnIrrelevantFlags. These are separate functions because they have different flag sets (deploy/delete vs enroll/unenroll), so the naming is defensible, but the suffix style differs.
Suggested fix: Consider consolidating into a single parameterized helper, or accept the current naming as reflecting the distinct flag sets.
1 participant
Summary
Adds
--platform=cloudflaresupport tomint enrollandmint unenrollcommands, enabling org and per-repo enrollment against durable Cloudflare Workers. This mirrors the existing platform flag onmint deployandmint delete.Related Issue
Closes #5633
Changes
internal/dispatch/cf/provisioner.go): AddedGetVars(reads Worker vars via CF API) andHasPreviewVersions(detects preview aliases viawrangler versions list)internal/dispatch/cf/provisioner.go): AddedEnsureOrgInWorker,RemoveOrgFromWorker,RegisterRepoInWorker,RemoveRepoFromWorkermethods that read-modify-write Worker vars via a durable redeploy with--keep-varsinternal/cli/mint.go): Added--platform(gcp|cloudflare),--worker-name, and--preview(rejected) flags with platform routinginternal/cli/mint.go): Same flag additions with platform routing, including confirmation prompts for CF path--previewrejection semantics--previewon enroll/unenroll is rejected with a clear error pointing tomint deployTesting
--platform=cloudflareon enroll/unenroll (dry-run, success, preview rejection, worker-not-found, preview warning, unsupported platform)go vet,go build ./...passCloses #5633
Post-script verification
agent/5633-cf-enroll-unenroll)55bd9f61ce57f9f2151a29d59a04d42b664110cf..HEAD)