From 625b51dc0b0b5d82eebcbe15738a89b99beeb440 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 17 Aug 2026 00:33:01 +0000 Subject: [PATCH 1/3] feat(#6269): derive inference project number in repos install repos install now only requires --inference-project. The project number is auto-derived via the GCP Resource Manager API (matching admin install behavior), and --inference-region defaults to "global". --inference-project-number remains available as an optional override for environments where the API lookup is not possible. Changes: - internal/cli/repos.go: default region to "global" when project is set, derive project number via GetProjectNumber() when not explicit - internal/repos/batch_install.go: update validation comment to reflect CLI-level defaults - internal/cli/repos_test.go: add tests for derivation, defaulting, explicit override, and lookup error handling Closes #6269 --- internal/cli/repos.go | 33 +++++++++++-- internal/cli/repos_test.go | 87 +++++++++++++++++++++++++++++++++ internal/repos/batch_install.go | 10 ++-- 3 files changed, 122 insertions(+), 8 deletions(-) diff --git a/internal/cli/repos.go b/internal/cli/repos.go index d6656bdf6a..8065a5b9af 100644 --- a/internal/cli/repos.go +++ b/internal/cli/repos.go @@ -426,7 +426,8 @@ type reposInstallConfig struct { allowedRemoteResources []string // Test overrides - testClient forge.Client + testClient forge.Client + testProjectNumberFn func(ctx context.Context, projectID string) (string, error) } func newReposInstallCmd() *cobra.Command { @@ -466,9 +467,9 @@ GCP infrastructure (WIF, mint) must be provisioned separately via cmd.Flags().BoolVar(&opts.direct, "direct", false, "push scaffold directly to default branch (skip PR)") cmd.Flags().BoolVar(&opts.force, "force", false, "allow scaffold ref downgrades") cmd.Flags().StringVar(&opts.forge, "forge", "", "forge type for repos not yet in the manifest (github or gitlab)") - cmd.Flags().StringVar(&opts.inferenceProject, "inference-project", "", "GCP project ID for inference; requires all three --inference-* flags") - cmd.Flags().StringVar(&opts.inferenceProjectNumber, "inference-project-number", "", "numeric GCP project number; requires all three --inference-* flags") - cmd.Flags().StringVar(&opts.inferenceRegion, "inference-region", "", "GCP region for inference; requires all three --inference-* flags") + cmd.Flags().StringVar(&opts.inferenceProject, "inference-project", "", "GCP project ID for inference") + cmd.Flags().StringVar(&opts.inferenceProjectNumber, "inference-project-number", "", "numeric GCP project number (auto-derived from --inference-project when omitted)") + cmd.Flags().StringVar(&opts.inferenceRegion, "inference-region", "", "GCP region for inference (default: global)") cmd.Flags().StringVar(&opts.fullsendRef, "fullsend-ref", "", "per-repo fullsend workflow ref override") cmd.Flags().StringVar(&opts.mintURL, "mint-url", "", "per-repo mint URL override") cmd.Flags().StringSliceVar(&opts.allowedRemoteResources, "allowed-remote-resources", nil, "per-repo allowed remote resources override") @@ -502,6 +503,30 @@ func runReposInstall(ctx context.Context, opts *reposInstallConfig) error { printer := ui.New(os.Stdout) + // Default --inference-region to "global" (matching admin install) + // when --inference-project is set but --inference-region is not. + if opts.inferenceProject != "" && opts.inferenceRegion == "" { + opts.inferenceRegion = "global" + } + + // Derive --inference-project-number from --inference-project via + // the GCP Resource Manager API when not explicitly provided. + if opts.inferenceProject != "" && opts.inferenceProjectNumber == "" { + var projectNumber string + var lookupErr error + if opts.testProjectNumberFn != nil { + projectNumber, lookupErr = opts.testProjectNumberFn(ctx, opts.inferenceProject) + } else { + gcpClient := gcf.NewLiveGCFClient(opts.inferenceProject) + projectNumber, lookupErr = gcpClient.GetProjectNumber(ctx, opts.inferenceProject) + } + if lookupErr != nil { + return fmt.Errorf("deriving project number from %q: %w (use --inference-project-number to specify it manually)", opts.inferenceProject, lookupErr) + } + opts.inferenceProjectNumber = projectNumber + printer.StepDone(fmt.Sprintf("Derived project number %s from project %s", projectNumber, opts.inferenceProject)) + } + printer.StepStart("Loading manifest") manifest, err := repos.LoadManifest(ctx, opts.manifest) if err != nil { diff --git a/internal/cli/repos_test.go b/internal/cli/repos_test.go index 2fed624a52..ec63eda738 100644 --- a/internal/cli/repos_test.go +++ b/internal/cli/repos_test.go @@ -1397,6 +1397,93 @@ func TestRunReposInstall_InvalidInferenceProjectNumber(t *testing.T) { assert.Contains(t, err.Error(), "--inference-project-number must be numeric") } +func TestRunReposInstall_DerivesProjectNumber(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstallFakeClient("acme/api") + + err := runReposInstall(context.Background(), &reposInstallConfig{ + manifest: manifestPath, + concurrency: 4, + roles: []string{"triage"}, + direct: true, + inferenceProject: "inf-proj", + // No inferenceProjectNumber — should be auto-derived. + // No inferenceRegion — should default to "global". + testClient: fc, + testProjectNumberFn: func(_ context.Context, projectID string) (string, error) { + if projectID != "inf-proj" { + t.Errorf("expected project ID inf-proj, got %s", projectID) + } + return "987654321", nil + }, + }) + require.NoError(t, err) +} + +func TestRunReposInstall_ExplicitProjectNumberSkipsLookup(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstallFakeClient("acme/api") + + lookupCalled := false + err := runReposInstall(context.Background(), &reposInstallConfig{ + manifest: manifestPath, + concurrency: 4, + roles: []string{"triage"}, + direct: true, + inferenceProject: "inf-proj", + inferenceProjectNumber: "111222333", + inferenceRegion: "us-central1", + testClient: fc, + testProjectNumberFn: func(_ context.Context, _ string) (string, error) { + lookupCalled = true + return "999", nil + }, + }) + require.NoError(t, err) + assert.False(t, lookupCalled, + "project number lookup should be skipped when --inference-project-number is explicit") +} + +func TestRunReposInstall_DefaultsInferenceRegion(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstallFakeClient("acme/api") + + err := runReposInstall(context.Background(), &reposInstallConfig{ + manifest: manifestPath, + concurrency: 4, + roles: []string{"triage"}, + direct: true, + inferenceProject: "inf-proj", + // inferenceRegion left empty — should default to "global". + testClient: fc, + testProjectNumberFn: func(_ context.Context, _ string) (string, error) { + return "123456789", nil + }, + }) + require.NoError(t, err) +} + +func TestRunReposInstall_ProjectNumberLookupError(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstallFakeClient("acme/api") + + err := runReposInstall(context.Background(), &reposInstallConfig{ + manifest: manifestPath, + concurrency: 4, + roles: []string{"triage"}, + direct: true, + inferenceProject: "inf-proj", + testClient: fc, + testProjectNumberFn: func(_ context.Context, _ string) (string, error) { + return "", errors.New("API unavailable") + }, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "deriving project number") + assert.Contains(t, err.Error(), "API unavailable") + assert.Contains(t, err.Error(), "--inference-project-number") +} + func TestRunReposInstall_PerRepoOverrideFlags_Applied(t *testing.T) { manifestPath := writeTestManifest(t, testManifestYAML) fc := newInstallFakeClient("acme/api", "acme/web") diff --git a/internal/repos/batch_install.go b/internal/repos/batch_install.go index b640d25efa..5e428aefdd 100644 --- a/internal/repos/batch_install.go +++ b/internal/repos/batch_install.go @@ -257,10 +257,12 @@ func BatchInstall(ctx context.Context, cfg BatchInstallConfig, return result, nil } - // Inference flags are all-or-nothing: if any one of - // --inference-project, --inference-project-number, or - // --inference-region is set, all three are required. Fail fast - // before per-repo validation. + // Inference flags validation: all three must be present when any + // is set. The CLI layer defaults --inference-region to "global" + // and auto-derives --inference-project-number from the project + // ID, so users only need to pass --inference-project. This + // validation acts as a safety net for callers that bypass the + // CLI (e.g. tests calling BatchInstall directly). inferenceFlags := []struct{ name, val string }{ {"--inference-project", cfg.InferenceProject}, {"--inference-project-number", cfg.InferenceProjectNumber}, From 3d8bb5bac76806a9cc91a842f6a262743fbf5941 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 17 Aug 2026 01:05:34 +0000 Subject: [PATCH 2/3] fix: address review feedback on PR #6271 - Update docs/cli/repos.md inference flag descriptions to reflect auto-derivation behavior: --inference-project no longer implies mutual dependency, --inference-project-number documents auto-derivation, --inference-region documents global default. - Strengthen TestRunReposInstall_DerivesProjectNumber and TestRunReposInstall_DefaultsInferenceRegion with direct assertions on opts fields to verify derived values reach the BatchInstall layer without relying on all-or-nothing validation as an indirect check. Addresses review feedback on #6271 --- docs/cli/repos.md | 6 +++--- internal/cli/repos_test.go | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/docs/cli/repos.md b/docs/cli/repos.md index 1b81a65d04..3c3c8b9863 100644 --- a/docs/cli/repos.md +++ b/docs/cli/repos.md @@ -110,11 +110,11 @@ When repos are specified as positional arguments, only those repos are processed | `--concurrency` | `4` | Max parallel operations (1-32) | | `--roles` | `triage,coder,review,fix,retro,prioritize` | Agent roles to install | | `--direct` | `false` | Push scaffold directly to default branch (skip PR) | -| `--inference-project` | | GCP project ID for inference (written as `FULLSEND_GCP_PROJECT_ID` secret; required when any inference flag is set) | -| `--inference-project-number` | | Numeric GCP project number for WIF provider computation (required when any inference flag is set) | +| `--inference-project` | | GCP project ID for inference (written as `FULLSEND_GCP_PROJECT_ID` secret) | +| `--inference-project-number` | | Numeric GCP project number for WIF provider computation (auto-derived from `--inference-project` when omitted) | | `--forge` | | Forge type for new repos (`github` or `gitlab`). Required when adding repos not already in the manifest; falls back to `defaults.forge` if set. | | `--force` | `false` | Allow scaffold ref downgrades | -| `--inference-region` | | Per-repo GCP inference region override (install-time only, not stored in the manifest) | +| `--inference-region` | | Per-repo GCP inference region override (default: global when `--inference-project` is set; install-time only, not stored in the manifest) | | `--fullsend-ref` | | Per-repo fullsend workflow ref override | | `--mint-url` | | Per-repo mint URL override | | `--allowed-remote-resources` | | Per-repo allowed remote resources override | diff --git a/internal/cli/repos_test.go b/internal/cli/repos_test.go index ec63eda738..c6f7240fa7 100644 --- a/internal/cli/repos_test.go +++ b/internal/cli/repos_test.go @@ -1401,7 +1401,7 @@ func TestRunReposInstall_DerivesProjectNumber(t *testing.T) { manifestPath := writeTestManifest(t, testManifestYAML) fc := newInstallFakeClient("acme/api") - err := runReposInstall(context.Background(), &reposInstallConfig{ + opts := &reposInstallConfig{ manifest: manifestPath, concurrency: 4, roles: []string{"triage"}, @@ -1416,8 +1416,15 @@ func TestRunReposInstall_DerivesProjectNumber(t *testing.T) { } return "987654321", nil }, - }) + } + err := runReposInstall(context.Background(), opts) require.NoError(t, err) + + // Verify derived values reached the BatchInstall layer via opts. + assert.Equal(t, "987654321", opts.inferenceProjectNumber, + "project number should be auto-derived from testProjectNumberFn") + assert.Equal(t, "global", opts.inferenceRegion, + "inference region should default to global") } func TestRunReposInstall_ExplicitProjectNumberSkipsLookup(t *testing.T) { @@ -1448,7 +1455,7 @@ func TestRunReposInstall_DefaultsInferenceRegion(t *testing.T) { manifestPath := writeTestManifest(t, testManifestYAML) fc := newInstallFakeClient("acme/api") - err := runReposInstall(context.Background(), &reposInstallConfig{ + opts := &reposInstallConfig{ manifest: manifestPath, concurrency: 4, roles: []string{"triage"}, @@ -1459,8 +1466,11 @@ func TestRunReposInstall_DefaultsInferenceRegion(t *testing.T) { testProjectNumberFn: func(_ context.Context, _ string) (string, error) { return "123456789", nil }, - }) + } + err := runReposInstall(context.Background(), opts) require.NoError(t, err) + assert.Equal(t, "global", opts.inferenceRegion, + "inference region should default to global when --inference-project is set") } func TestRunReposInstall_ProjectNumberLookupError(t *testing.T) { From 9547dc24764b3434fb1f80477934e4aabe4dc27c Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 17 Aug 2026 01:40:45 +0000 Subject: [PATCH 3/3] fix: update stale docs and clarify test assertion for PR #6271 - Remove --inference-project-number from GitLab install example in operations.md since it is now auto-derived - Add "(auto-derived)" to --inference-project-number description in cli-internals.md - Clarify test comment explaining why opts-level assertion is sufficient for verifying project number derivation Addresses review feedback on #6271 --- docs/guides/dev/cli-internals.md | 2 +- docs/guides/getting-started/operations.md | 3 +-- internal/cli/repos_test.go | 6 +++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index afc5b745e4..c5d2dc1648 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -55,7 +55,7 @@ fullsend │ │ ├── --roles # Agent roles (default: triage,coder,review,fix,retro,prioritize) │ │ ├── --direct # Push scaffold to default branch (skip PR) │ │ ├── --inference-project # GCP project ID for inference (install-time only) -│ │ ├── --inference-project-number # Numeric GCP project number for WIF (install-time only) +│ │ ├── --inference-project-number # Numeric GCP project number for WIF (auto-derived; install-time only) │ │ ├── --forge # Forge type for new repos (github or gitlab) │ │ ├── --inference-region # Per-repo GCP inference region override │ │ ├── --fullsend-ref # Per-repo fullsend workflow ref override diff --git a/docs/guides/getting-started/operations.md b/docs/guides/getting-started/operations.md index 2866e42fc7..1e7c67b5f5 100644 --- a/docs/guides/getting-started/operations.md +++ b/docs/guides/getting-started/operations.md @@ -32,8 +32,7 @@ For GitLab repos, re-run `repos install` with updated values to converge configu ```bash fullsend repos install -f repos.yaml "$OWNER/$REPO" \ - --inference-project "" \ - --inference-project-number "" + --inference-project "" ``` | Key | Storage Type | Description | Example value | diff --git a/internal/cli/repos_test.go b/internal/cli/repos_test.go index c6f7240fa7..9a69a8c46a 100644 --- a/internal/cli/repos_test.go +++ b/internal/cli/repos_test.go @@ -1420,7 +1420,11 @@ func TestRunReposInstall_DerivesProjectNumber(t *testing.T) { err := runReposInstall(context.Background(), opts) require.NoError(t, err) - // Verify derived values reached the BatchInstall layer via opts. + // Verify derived values. runReposInstall sets these on opts before + // constructing BatchInstallConfig (which copies them verbatim), so + // asserting here confirms the derivation logic. The require.NoError + // above also provides indirect coverage: BatchInstall's all-or-nothing + // validation would fail if the values were missing or empty. assert.Equal(t, "987654321", opts.inferenceProjectNumber, "project number should be auto-derived from testProjectNumberFn") assert.Equal(t, "global", opts.inferenceRegion,