Skip to content

feat(cli): add inference sub-command for standalone WIF provisioning#1258

Open
waynesun09 wants to merge 4 commits into
mainfrom
inference-subcommand
Open

feat(cli): add inference sub-command for standalone WIF provisioning#1258
waynesun09 wants to merge 4 commits into
mainfrom
inference-subcommand

Conversation

@waynesun09
Copy link
Copy Markdown
Contributor

@waynesun09 waynesun09 commented May 20, 2026

Summary

  • Adds fullsend inference sub-command tree with 2 commands: provision and status
  • Extracts inference WIF provisioning from the monolithic admin install so GCP admins can provision Vertex AI WIF infrastructure without needing GitHub credentials
  • provision creates WIF pool/provider and prints the resource name for handoff to fullsend admin install
  • status combines health check + config display with --format support (text/json/env)

Test plan

  • go test ./internal/cli/... passes (28 tests)
  • go vet ./internal/cli/... clean
  • Verify fullsend inference --help shows both sub-commands
  • Verify fullsend inference provision --help shows all flags
  • Verify fullsend inference status --help shows --format flag
  • Verify no GitHub token is required for either command
  • E2E: fullsend inference provision <org> --project=<proj> creates WIF infra and prints provider name
  • E2E: fullsend inference status <org> --project=<proj> --format=env prints config values

Extract inference WIF provisioning from the monolithic admin install
command into a standalone `fullsend inference` sub-command tree that
only requires GCP access (no GitHub token or mint project needed).

New commands:
- `fullsend inference provision-wif <org|owner/repo>` — creates WIF
  pool, provider, and IAM bindings for Vertex AI inference. Supports
  both org-scoped and repo-scoped modes with --dry-run preview.
- `fullsend inference status <org|owner/repo>` — checks WIF health
  and prints config values with --format support (text, json, env)
  for handoff to the GitHub admin.

This enables a split-admin workflow where the GCP inference admin
provisions WIF independently and hands off the provider resource name
to the GitHub admin who runs `fullsend github setup`.

Signed-off-by: Wayne Sun <[email protected]>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 2026

Site preview

Preview: https://75e7eef8-site.fullsend-ai.workers.dev

Commit: 18b20c89a8ca5a4809c49f70ad33424d8107c074

@fullsend-ai-review
Copy link
Copy Markdown

fullsend-ai-review Bot commented May 20, 2026

Review

Findings

Low

  • [documentation-currency] docs/guides/dev/cli-internals.md:7 — The CLI command tree does not include the new inference subcommand with its provision and status children. Developers consulting this doc for the full command surface will not see the new commands. The CLI --help output is correct, but the doc should be updated for completeness.
    Remediation: Add inference (with provision and status children) to the command tree in docs/guides/dev/cli-internals.md between admin and run.
Previous run

Review

Findings

Low

  • [documentation-currency] docs/guides/dev/cli-internals.md:5-25 — The CLI Command Tree diagram does not include the new inference subcommand tree (inference, provision, status). This diagram is the canonical reference for the CLI structure and should be updated to reflect the new commands.
    Remediation: Add the inference subtree to the command tree diagram between admin and run.

Info

  • [style] internal/cli/inference.go:339formatStatusJSON mixes domain keys (FULLSEND_GCP_PROJECT_ID, FULLSEND_GCP_WIF_PROVIDER) with metadata keys (status, details) in a map[string]interface{}. A dedicated struct would make the JSON contract more explicit and prevent accidental key collisions. Not blocking — the current approach works.
Previous run (2)

Review

Findings

Low

  • [documentation-currency] docs/guides/dev/cli-internals.md:7-25 — The CLI Command Tree diagram does not include the new inference subcommand tree (inference, provision, status). This diagram is the canonical reference for the CLI structure and should be updated to reflect the new commands.
    Remediation: Add the inference subtree to the command tree diagram between admin and run.

Info

  • [style] internal/cli/inference.go:339formatStatusJSON mixes domain keys (FULLSEND_GCP_PROJECT_ID, FULLSEND_GCP_WIF_PROVIDER) with metadata keys (status, details) in a map[string]interface{}. A dedicated struct would make the JSON contract more explicit and prevent accidental key collisions. Not blocking — the current approach works.
Previous run (3)

Review

Findings

Medium

  • [correctness] internal/cli/inference.go:64 — The --region flag (line 100) is documented as "WIF location (default: global)" but the value is never passed to gcf.Config in runInferenceProvisionWIF (line 155). WIF pools are always created at locations/global regardless of this flag. The value is only used in the printed next-step command (--inference-region=%s), making the flag misleading. Similarly, runInferenceStatus (line 245) hardcodes locations/global in the WIF provider path but displays the user-provided --region in output. If the intent is to capture the inference region for handoff to fullsend github setup, the flag help text should say "GCP region for inference (passed to --inference-region in setup)" rather than "WIF location."
    Remediation: Either (a) rename/redocument the flag as a pass-through for --inference-region and not a WIF location control, or (b) remove it from provision-wif since WIF is always global, and keep it only on status for config display.

Low

  • [correctness] internal/cli/inference.go:275 — Org condition validation uses strings.Contains(condition, org) which can produce false-positive matches when the org name is a substring of another identifier in the condition (e.g., org "ac" matching a condition containing "acme"). Consider using an exact match against the expected condition string, consistent with the repo-scoped check on line 270.
    Remediation: Replace strings.Contains(condition, org) with an exact match: condition == fmt.Sprintf("assertion.repository_owner == '%s'", strings.ToLower(org)).

  • [documentation-currency] docs/guides/dev/cli-internals.md:5 — The CLI Command Tree section does not include the new inference command group (provision-wif, status). The tree should be updated to reflect the new sub-commands.

  • [documentation-currency] docs/guides/admin/installation.md:333 — The "Advanced: pre-configure WIF" section documents manual gcloud commands for WIF setup but does not mention fullsend inference provision-wif as a supported alternative. Users may not discover the new command.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label May 20, 2026
- Fix condition mismatch not setting unhealthy status (tracked via
  conditionOK bool with proper org exact/substring validation)
- Remove misleading --region flag (WIF is always locations/global)
- Add --pool/--provider flags to status command (was hardcoded)
- Remove false service account claim from dry-run output
- Lowercase org/repo in condition output to match provisioner behavior
- Fix redundant SplitN in repo-scoped dry-run
- Reject --provider in repo-scoped mode for both provision and status
- Use cmd.OutOrStdout() instead of os.Stdout for testability
- Conditionally include config keys in JSON/env output only when healthy
- Add FULLSEND_INFERENCE_STATUS to env output format
- Remove non-existent FULLSEND_GCP_REGION from all output formats
- Rename provision-wif to provision (shorter, unambiguous)

Signed-off-by: Wayne Sun <[email protected]>
Comment thread internal/cli/inference.go
printer := ui.New(cmd.OutOrStdout())
printer.Banner()
printer.Blank()
printer.Header("Inference Status")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[info] style

formatStatusJSON mixes domain keys (FULLSEND_GCP_PROJECT_ID, FULLSEND_GCP_WIF_PROVIDER) with metadata keys (status, details) in a map[string]interface{}. A dedicated struct would make the JSON contract more explicit.

@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
Fix incorrect handoff command reference (fullsend github setup →
fullsend admin install), gate JSON/env config key output on WIFProvider
presence instead of status string, and tighten multi-org condition
matching to also require repository_owner in the attribute condition.

Signed-off-by: Wayne Sun <[email protected]>
Comment thread internal/cli/inference.go
printer := ui.New(cmd.OutOrStdout())
printer.Banner()
printer.Blank()
printer.Header("Inference Status")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[info] style

formatStatusJSON mixes domain keys (FULLSEND_GCP_PROJECT_ID, FULLSEND_GCP_WIF_PROVIDER) with metadata keys (status, details) in a map[string]interface{}. A dedicated struct would make the JSON contract more explicit.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 20, 2026
Add early project ID validation using the same regex pattern as the gcf
provisioner. This gives users a clear error message instead of an opaque
API failure when an invalid project ID is passed to inference provision
or status.

Signed-off-by: Wayne Sun <[email protected]>
@waynesun09 waynesun09 marked this pull request as ready for review May 20, 2026 19:55
@waynesun09 waynesun09 requested a review from ggallen May 20, 2026 19:55
@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants