Skip to content

feat(cli): add mint sub-command for standalone GCP mint management#1261

Open
waynesun09 wants to merge 1 commit into
mainfrom
mint-subcommand
Open

feat(cli): add mint sub-command for standalone GCP mint management#1261
waynesun09 wants to merge 1 commit into
mainfrom
mint-subcommand

Conversation

@waynesun09
Copy link
Copy Markdown
Contributor

Summary

  • Adds fullsend mint sub-command tree with 4 commands: deploy, enroll, unenroll, status
  • Extracts all mint-related operations from admin install so GCP admins can manage token mint infrastructure without needing GitHub credentials
  • deploy provisions the Cloud Function and supporting GCP infrastructure (SA, WIF, function deploy)
  • enroll performs full enrollment: PEM copy, env var merge (ALLOWED_ORGS, ROLE_APP_IDS), and per-repo WIF provider creation
  • unenroll reverses enrollment with confirmation prompt; defaults to disable-only (explicit flags for permanent deletion)
  • status provides read-only health check with enrolled orgs, PEM status, and WIF state
  • Adds 8 new Provisioner methods for unenroll operations (RemoveOrgFromMint, RemoveRepoFromMint, DisablePEMSecrets, DeletePEMSecrets, DisableWIFProvider, DeleteWIFProvider, etc.)
  • Extends GCFClient interface with 4 new GCP operations

Test plan

  • go test ./internal/cli/... passes
  • go test ./internal/dispatch/gcf/... passes (296 new lines of provisioner tests)
  • go vet clean
  • Verify fullsend mint --help shows all 4 sub-commands
  • Verify fullsend mint deploy --help shows all flags
  • Verify fullsend mint enroll --help handles both org and owner/repo args
  • Verify fullsend mint unenroll requires confirmation
  • Verify no GitHub token required for any command
  • Verify role aliasing (fix → coder) works in enroll
  • E2E: fullsend mint deploy <org> --project=<proj> deploys Cloud Function
  • E2E: fullsend mint enroll <org> --project=<proj> copies PEMs and updates env vars
  • E2E: fullsend mint status --project=<proj> shows mint health

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 2026

Site preview

Preview: https://c1153a90-site.fullsend-ai.workers.dev

Commit: 521bae03e7bf31244a1234c746de2cb8bafc695d

@fullsend-ai-review
Copy link
Copy Markdown

fullsend-ai-review Bot commented May 20, 2026

Review

Findings

Medium

  • [correctness] internal/cli/mint.go:733 — Unenroll org flow does not revoke the roles/aiplatform.user IAM binding granted during enrollment (GrantOrgVertexAIAccess). While the WIF condition update prevents new authentication, the stale IAM binding persists. The GCFClient interface currently lacks a RemoveProjectIAMBinding method, so this cannot be fixed without expanding the interface. Risk is limited since the IAM binding is inert without WIF authentication, but stale bindings accumulate and would auto-activate if the org is re-enrolled.
    Remediation: Add a RemoveProjectIAMBinding method to GCFClient and call it during org unenroll, or document this as a known limitation with a manual cleanup step.

  • [documentation-currency] docs/guides/dev/cli-internals.md — The CLI command tree diagram (lines 5–25) lists admin, run, scan, post-review, post-comment but does not include the new mint sub-command tree. Users and developers reading this doc will have an incomplete picture of the CLI surface.
    Remediation: Update the command tree to include mint { deploy, enroll, unenroll, status }.

  • [documentation-currency] docs/guides/admin/installation.md — This guide documents mint deployment exclusively as part of admin install (lines 52–89). With the new standalone mint deploy/enroll commands, the guide should explain when to use admin install --mint-project vs fullsend mint deploy + fullsend mint enroll separately.
    Remediation: Add a section explaining the standalone mint workflow and its relationship to the integrated install flow.

Low

  • [correctness] internal/dispatch/gcf/provisioner.go:1771EnsureOrgInWIFCondition and RemoveOrgFromWIFCondition use read-modify-write without locking, as documented in their WARNING comments. Concurrent enrollment/unenrollment of different orgs could lose updates. This is an inherent limitation of the GCP API pattern but worth tracking for future hardening (e.g., compare-and-swap via function generation numbers).

  • [correctness] internal/cli/mint.go:471runMintEnrollRepo calls provisioner.ProvisionWIF(ctx) to create a per-repo WIF provider, but the Provisioner is constructed without AgentAppIDs populated (only ProjectID, Region, GitHubOrgs, Repo). ProvisionWIF calls ensureWIFPoolAndProvider which needs AgentAppIDs for the org-level provider merge path. Verify that per-repo mode (where cfg.Repo != "") skips that code path and goes directly to the per-repo provider creation branch.

Info

  • [style] internal/cli/mint.go:583 — The --yolo flag name is colorful but consistent with the codebase's informal style. No action needed.

  • [correctness] internal/dispatch/gcf/gcp.go:226 — The CreateWIFProvider conflict-handling refactor (_ = undeleteWIFProviderUpdateWIFProviderenableWIFProvider) is cleaner than the original dual-UpdateWIFProvider pattern. The ignored undelete error is safe because the subsequent update will fail if the provider is genuinely unreachable.

Previous run

Review

Findings

Medium

  • [correctness] internal/cli/mint.go:733 — Unenroll org flow does not revoke the roles/aiplatform.user IAM binding granted during enrollment (GrantOrgVertexAIAccess). While the WIF condition update prevents new authentication, the stale IAM binding persists. The GCFClient interface currently lacks a RemoveProjectIAMBinding method, so this cannot be fixed without expanding the interface. Risk is limited since the IAM binding is inert without WIF authentication, but stale bindings accumulate and would auto-activate if the org is re-enrolled.
    Remediation: Add a RemoveProjectIAMBinding method to GCFClient and call it during org unenroll, or document this as a known limitation with a manual cleanup step.

  • [documentation-currency] docs/guides/dev/cli-internals.md — The CLI command tree diagram (lines 5–25) lists admin, run, scan, post-review, post-comment but does not include the new mint sub-command tree. Users and developers reading this doc will have an incomplete picture of the CLI surface.
    Remediation: Update the command tree to include mint { deploy, enroll, unenroll, status }.

  • [documentation-currency] docs/guides/admin/installation.md — This guide documents mint deployment exclusively as part of admin install (lines 52–89). With the new standalone mint deploy/enroll commands, the guide should explain when to use admin install --mint-project vs fullsend mint deploy + fullsend mint enroll separately.
    Remediation: Add a section explaining the standalone mint workflow and its relationship to the integrated install flow.

Low

  • [correctness] internal/dispatch/gcf/provisioner.go:1771EnsureOrgInWIFCondition and RemoveOrgFromWIFCondition use read-modify-write without locking, as documented in their WARNING comments. Concurrent enrollment/unenrollment of different orgs could lose updates. This is an inherent limitation of the GCP API pattern but worth tracking for future hardening (e.g., compare-and-swap via function generation numbers).

  • [correctness] internal/cli/mint.go:471runMintEnrollRepo calls provisioner.ProvisionWIF(ctx) to create a per-repo WIF provider, but the Provisioner is constructed without AgentAppIDs populated (only ProjectID, Region, GitHubOrgs, Repo). ProvisionWIF calls ensureWIFPoolAndProvider which needs AgentAppIDs for the org-level provider merge path. Verify that per-repo mode (where cfg.Repo != "") skips that code path and goes directly to the per-repo provider creation branch.

Info

  • [style] internal/cli/mint.go:583 — The --yolo flag name is colorful but consistent with the codebase's informal style. No action needed.

  • [correctness] internal/dispatch/gcf/gcp.go:226 — The CreateWIFProvider conflict-handling refactor (_ = undeleteWIFProviderUpdateWIFProviderenableWIFProvider) is cleaner than the original dual-UpdateWIFProvider pattern. The ignored undelete error is safe because the subsequent update will fail if the provider is genuinely unreachable.

Previous run (2)

Review

Findings

Medium

  • [documentation-currency] docs/guides/dev/cli-internals.md:7 — The CLI Command Tree section shows the fullsend command structure without the new mint subcommand tree (deploy, enroll, unenroll, status). The documented tree is now incomplete and will mislead developers looking for mint-related code paths.
    Remediation: Add the mint subtree to the command tree diagram, including all four subcommands and their argument patterns.

  • [documentation-currency] docs/guides/admin/installation.md:52 — The installation guide documents mint provisioning exclusively as part of fullsend admin install (via --mint-project, --skip-mint-deploy, etc.) without mentioning that mint infrastructure can now be managed independently via fullsend mint deploy/enroll/unenroll/status. Admins managing multi-org mints — the primary use case for the new commands — won't discover the standalone workflow.
    Remediation: Add a section (or cross-reference) explaining that GCP admins can use fullsend mint subcommands for standalone mint management without GitHub credentials, and clarify the relationship between admin install (which bundles mint provisioning) and the new standalone commands.

Info

  • [race-condition] internal/dispatch/gcf/provisioner.goEnsureOrgInWIFCondition and RemoveOrgFromWIFCondition use read-modify-write without locking, as documented in their WARNING comments. This is acceptable for CLI usage (single operator at a time) but could cause lost updates if multiple admins run enrollment concurrently against the same mint. No action required — the existing inline warnings are appropriate.
Previous run (3)

Review

Findings

Medium

  • [documentation-currency] docs/guides/dev/cli-internals.md:7 — The CLI Command Tree section shows the fullsend command structure without the new mint subcommand tree (deploy, enroll, unenroll, status). The documented tree is now incomplete and will mislead developers looking for mint-related code paths.
    Remediation: Add the mint subtree to the command tree diagram, including all four subcommands and their argument patterns.

  • [documentation-currency] docs/guides/admin/installation.md:52 — The installation guide documents mint provisioning exclusively as part of fullsend admin install (via --mint-project, --skip-mint-deploy, etc.) without mentioning that mint infrastructure can now be managed independently via fullsend mint deploy/enroll/unenroll/status. Admins managing multi-org mints — the primary use case for the new commands — won't discover the standalone workflow.
    Remediation: Add a section (or cross-reference) explaining that GCP admins can use fullsend mint subcommands for standalone mint management without GitHub credentials, and clarify the relationship between admin install (which bundles mint provisioning) and the new standalone commands.

Info

  • [race-condition] internal/dispatch/gcf/provisioner.goEnsureOrgInWIFCondition and RemoveOrgFromWIFCondition use read-modify-write without locking, as documented in their WARNING comments. This is acceptable for CLI usage (single operator at a time) but could cause lost updates if multiple admins run enrollment concurrently against the same mint. No action required — the existing inline warnings are appropriate.
Previous run (4)

Review

Findings

Low

  • [correctness] internal/dispatch/gcf/provisioner.go — Several new functions lack unit tests: stripPlaceholderOrg, stripPlaceholderRoleAppIDs, EnsureOrgInWIFCondition, RemoveOrgFromWIFCondition. The WIF condition functions contain important logic (placeholder stripping, empty-list fallback to PlaceholderOrg) that should be covered. The provisioner-level tests for RemoveOrgFromMint and RemoveRepoFromMint are solid, and the CLI tests cover flag validation and role aliasing well.
    Remediation: Add unit tests for the untested helper functions and WIF condition update methods, particularly edge cases like empty org lists and placeholder handling.

  • [correctness] internal/cli/mint.go:632confirmUnenroll is untested. While the function is straightforward, it handles terminal detection and user input parsing which are easy to get wrong in edge cases (e.g., EOF on stdin, trailing whitespace).
    Remediation: Add a test using a bufio.Reader wrapping a strings.Reader for the happy path and mismatch/EOF paths. The function already accepts a reader parameter making it testable.

Info

  • [correctness] internal/dispatch/gcf/provisioner.go:1177,1231EnsureOrgInWIFCondition and RemoveOrgFromWIFCondition have documented race conditions (read-modify-write without locking). Acceptable for interactive CLI operations that are unlikely to run concurrently, but worth noting if these methods are ever called from automated pipelines.

  • [style] internal/cli/admin.go — The additional validateOrgName checks (max 39 chars, no consecutive hyphens) are good hardening that aligns with GitHub's actual org name rules. These apply to the existing admin install flow too, which is correct since GitHub already enforces these constraints.

Previous run (5)

Review

Findings

Medium

  • [documentation-currency] docs/guides/dev/cli-internals.md:7-25 — The CLI Command Tree section does not include the new mint subcommand tree (deploy, enroll, unenroll, status). This doc is the authoritative CLI structure reference and is now stale.
    Remediation: Add the mint subtree to the CLI Command Tree:
    ├── mint            # Manage token mint infrastructure (GCP-only)
    │   ├── deploy      # Deploy or update the Cloud Function
    │   ├── enroll      # Enroll an org or repo in the mint
    │   ├── unenroll    # Remove an org or repo from the mint
    │   └── status      # Show mint state and health
    

Low

  • [style] internal/cli/mint.go:556 — The --yolo flag name is informal. Standard CLI conventions use --yes or --force for confirmation bypass. This is a minor readability issue for operators unfamiliar with the convention.
    Remediation: Consider renaming to --yes (or add --yes as an alias) to match common CLI patterns (e.g., apt-get -y, terraform -auto-approve).

  • [correctness] internal/cli/mint.go:660-673 — In runMintUnenrollOrg, the dry-run path exits before discovering enrolled roles (step 1 discovers the mint but doesn't extract roles). The dry-run output says "Would remove {org}" but doesn't list which roles would be affected, unlike the enroll dry-run which itemizes roles. Minor UX inconsistency.
    Remediation: Move role discovery before the dry-run exit so the dry-run can list affected roles.

Info

  • [documentation-currency] docs/guides/admin/installation.md — The installation guide documents fullsend admin install as the primary workflow for GCP mint management. Now that fullsend mint provides standalone GCP operations, a cross-reference or note about when to use mint vs admin install would help administrators choose the right command. Not blocking — the existing docs aren't wrong, just incomplete.

  • [correctness] internal/dispatch/gcf/provisioner.goEnsureOrgInWIFCondition and RemoveOrgFromWIFCondition both document a read-modify-write race ("concurrent calls may race"). This matches the existing pattern used elsewhere in the provisioner, and the CLI's sequential execution model makes races unlikely in practice, but it's worth noting for future concurrent use.

Previous run (6)

Review

Findings

Medium

  • [documentation-currency] docs/guides/dev/cli-internals.md:7-25 — The CLI command tree is now incomplete. The new mint sub-command tree (deploy, enroll, unenroll, status) is not listed. This doc is a reference for developers and should reflect the actual command structure.
    Remediation: Add the mint sub-command tree to the CLI command tree diagram, between admin and run.

Low

  • [documentation-currency] docs/guides/admin/installation.md — The installation guide documents mint provisioning exclusively as part of fullsend admin install. Now that standalone fullsend mint commands exist as an alternative path (particularly for GCP admins who don't need GitHub credentials), this guide could mention the alternative and link to a fullsend mint usage section.
    Remediation: Add a brief note in the mint-related sections pointing to fullsend mint as an alternative for GCP-only operators.

Info

  • [intent-alignment] internal/cli/admin.go — The PR description states this "extracts all mint-related operations from admin install," but the admin install path is unchanged — this PR is purely additive. Both admin install and mint commands now provide mint management. This is architecturally fine but the description could be clearer that admin install retains full mint functionality.
Previous run (7)

Review

Findings

Medium

  • [documentation-currency] docs/guides/dev/cli-internals.md:7 — The CLI Command Tree section does not include the new mint subcommand tree (mint deploy, mint enroll, mint unenroll, mint status). This doc will be misleading for developers trying to understand the CLI structure.
    Remediation: Add the mint subtree to the CLI Command Tree diagram, after admin.

  • [documentation-currency] docs/guides/admin/installation.md:90 — The Multi-org setup section documents using fullsend admin install with --mint-url and --skip-mint-check for subsequent orgs. With the new fullsend mint enroll command, this section should mention the standalone enrollment path as an alternative, or note that mint management can now be done independently of admin install.
    Remediation: Add a note or cross-reference explaining that fullsend mint enroll is now available for enrolling additional orgs without GitHub credentials.

Low

  • [correctness] internal/cli/mint.go:593confirmUnenroll takes a *bufio.Reader and an isTerminal bool, but the terminal check happens before reading — if stdin is not a terminal, the function errors immediately. The function signature accepting a reader that is never used in the non-terminal path is slightly misleading but not incorrect. The callers correctly pass os.Stdin for both the reader and the terminal check.

  • [correctness] internal/cli/mint.go:149 — In newMintDeployCmd, the AgentAppIDs map uses gcf.PlaceholderOrg as key with value "0". This placeholder is a valid design choice for deploy-time scaffolding, but the value "0" is a magic number. Consider defining a constant alongside PlaceholderOrg for clarity.

  • [style] internal/cli/mint.go:534 — The --yolo flag name is informal for a CLI that handles production GCP infrastructure. While it works, --yes or --force would be more conventional and less surprising in audit logs.

  • [correctness] internal/dispatch/gcf/provisioner.go (stripPlaceholderRoleAppIDs) — If json.Marshal fails (which would only happen with truly unusual map contents), the error is silently swallowed with out, _ := json.Marshal(m). This is acceptable given that the input was just successfully unmarshaled, but a comment noting why the error is safe to ignore would improve clarity.

Previous run (8)

Review

Findings

Medium

  • [documentation-currency] docs/guides/dev/cli-internals.md:7 — The CLI command tree diagram does not include the new mint subcommand tree (deploy, enroll, unenroll, status). Readers relying on this diagram for an overview of available commands will not discover the mint management commands.
    Remediation: Add a mint branch to the ASCII command tree with its four subcommands.

Low

  • [documentation-currency] docs/guides/admin/installation.md — The installation guide documents mint provisioning exclusively through fullsend admin install --mint-project flags. With the new standalone fullsend mint deploy/enroll workflow, the guide should mention this alternative path for GCP admins who lack GitHub credentials.
    Remediation: Add a section or note explaining that fullsend mint commands can be used independently for mint infrastructure management.

  • [correctness] internal/cli/mint.go:461resolveEnrollAppIDs with explicit JSON (--role-app-ids) does not validate that role keys in the JSON map match rolePattern. Invalid role names (uppercase, special chars) would be written to ROLE_APP_IDS without validation, unlike the --roles flag path which validates via parseAgentRoles.
    Remediation: Validate each key in the explicit JSON map against rolePattern or resolveRole before accepting it.

Previous run (9)

Review

Findings

Low

  • [documentation-currency] docs/guides/dev/cli-internals.md — The CLI command tree diagram does not include the new mint subtree (deploy, enroll, unenroll, status). This makes the diagram incomplete for anyone using it as a reference for the CLI's command structure.
    Remediation: Add the mint subtree with its four subcommands to the CLI command tree in cli-internals.md.

Info

  • [correctness] internal/dispatch/gcf/gcp.go:283DisableWIFProvider does not handle HTTP 404 (provider not found), unlike its sibling DeleteWIFProvider which returns nil on 404. If a provider has already been deleted and a user runs unenroll with the default (disable-only) behavior, the call will fail with an opaque error. This is consistent with the existing UpdateWIFProvider pattern so it is not a bug, but adding 404 tolerance would improve the operator experience for idempotent cleanup.

  • [correctness] internal/cli/mint.go:125 — The deploy command uses GitHubOrgs: []string{"placeholder"} and AgentAppIDs: map[string]string{"placeholder": "0"} to satisfy Provision's requirement for at least one org. The comment explains the intent, but "placeholder" will appear in the function's ALLOWED_ORGS and ROLE_APP_IDS env vars until a real enroll overwrites them. This is harmless (app ID "0" won't resolve to a real app) but could confuse operators inspecting env vars via mint status before enrolling any org.

  • [style] internal/cli/mint.go:528 — The --yolo flag name is informal. Most Go CLIs use --yes or --force for confirmation bypass. This is a minor style preference and does not affect correctness.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label May 20, 2026
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels May 20, 2026
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels May 20, 2026
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels May 20, 2026
Comment thread internal/cli/mint.go
}
if org == gcf.PlaceholderOrg {
return fmt.Errorf("cannot unenroll reserved placeholder org %q", org)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] correctness

confirmUnenroll is untested. It handles terminal detection and user input parsing which are easy to get wrong in edge cases.

Suggested fix: Add tests using bufio.Reader wrapping strings.Reader for happy path, mismatch, and EOF paths.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels May 20, 2026
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed ready-for-merge All reviewers approved — ready to merge labels May 20, 2026
@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label May 20, 2026
@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label May 20, 2026
Add `fullsend mint` command tree with four sub-commands that operate
on GCP mint infrastructure without requiring a GitHub token:

- `mint deploy`: deploy/update the mint Cloud Function
- `mint enroll <org|owner/repo>`: enroll org or repo in mint
- `mint unenroll <org|owner/repo>`: remove org or repo from mint
- `mint status [org]`: read-only health check of mint state

Also adds new Provisioner methods for unenroll operations:
RemoveOrgFromMint, RemoveRepoFromMint, DisablePEMSecrets,
DeletePEMSecrets, DisableWIFProvider, DeleteWIFProvider.

Extends GCFClient interface with DisableSecretVersion, DeleteSecret,
DisableWIFProvider, and DeleteWIFProvider methods.

Signed-off-by: Wayne Sun <[email protected]>
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels May 20, 2026
@waynesun09 waynesun09 marked this pull request as ready for review May 20, 2026 20:37
@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels May 20, 2026
Copy link
Copy Markdown
Contributor

@ralphbean ralphbean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The make test tests need to pass. e2e can continue to fail until it is fixed on main, but let's not regress make test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants