Skip to content

Commit 48eac1a

Browse files
authored
Merge pull request #6960 from fullsend-ai/agent/6958-wif-provider-repos-install
feat(#6958): add --inference-wif-provider to repos install, drop --inference-project-number
2 parents 2a10349 + 865355a commit 48eac1a

6 files changed

Lines changed: 250 additions & 51 deletions

File tree

‎docs/cli/repos.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ When repos are specified as positional arguments, only those repos are processed
110110
| `--roles` | `triage,coder,review,fix,retro,prioritize` | Agent roles to install |
111111
| `--direct` | `false` | Push scaffold directly to default branch (skip PR) |
112112
| `--inference-project` | | GCP project ID for inference (written as `FULLSEND_GCP_PROJECT_ID` secret) |
113-
| `--inference-project-number` | | Numeric GCP project number for WIF provider computation (auto-derived from `--inference-project` when omitted) |
113+
| `--inference-wif-provider` | | Full WIF provider resource name (`projects/{number}/locations/global/workloadIdentityPools/{pool}/providers/{id}`); uses this provider for all repos instead of deriving per-repo providers. Project number is embedded in the path, so no auto-derivation is needed. |
114114
| `--forge` | | Forge type for new repos (`github` or `gitlab`). Required when adding repos not already in the manifest; inferred from existing platform sections when unambiguous. |
115115
| `--force` | `false` | Allow scaffold ref downgrades |
116116
| `--inference-region` | | Per-repo GCP inference region override (default: global when `--inference-project` is set; install-time only, not stored in the manifest) |

‎docs/guides/dev/cli-internals.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ fullsend
6868
│ │ ├── --roles <list> # Agent roles (default: triage,coder,review,fix,retro,prioritize)
6969
│ │ ├── --direct # Push scaffold to default branch (skip PR)
7070
│ │ ├── --inference-project <id> # GCP project ID for inference (install-time only)
71-
│ │ ├── --inference-project-number <num> # Numeric GCP project number for WIF (auto-derived; install-time only)
71+
│ │ ├── --inference-wif-provider <path> # Full WIF provider resource name (uses verbatim; skips per-repo derivation)
7272
│ │ ├── --forge <type> # Forge type for new repos (github or gitlab)
7373
│ │ ├── --inference-region <region> # Per-repo GCP inference region override
7474
│ │ ├── --fullsend-ref <ref> # Per-repo fullsend workflow ref override

‎internal/cli/repos.go‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,8 @@ type reposInstallConfig struct {
423423

424424
// GCP credentials (install-time only)
425425
inferenceProject string
426-
inferenceProjectNumber string
426+
inferenceWIFProvider string
427+
inferenceProjectNumber string // auto-derived from --inference-project; not a CLI flag
427428
inferenceRegion string
428429

429430
// GitLab-specific
@@ -478,7 +479,7 @@ GCP infrastructure (WIF, mint) must be provisioned separately via
478479
cmd.Flags().BoolVar(&opts.force, "force", false, "allow scaffold ref downgrades")
479480
cmd.Flags().StringVar(&opts.forge, "forge", "", "forge type for repos not yet in the manifest (github or gitlab)")
480481
cmd.Flags().StringVar(&opts.inferenceProject, "inference-project", "", "GCP project ID for inference")
481-
cmd.Flags().StringVar(&opts.inferenceProjectNumber, "inference-project-number", "", "numeric GCP project number (auto-derived from --inference-project when omitted)")
482+
cmd.Flags().StringVar(&opts.inferenceWIFProvider, "inference-wif-provider", "", "full WIF provider resource name (projects/{number}/locations/global/workloadIdentityPools/{pool}/providers/{id}); uses this provider for all repos instead of deriving per-repo providers")
482483
cmd.Flags().StringVar(&opts.inferenceRegion, "inference-region", "", "GCP region for inference (default: global)")
483484
cmd.Flags().StringVar(&opts.fullsendRef, "fullsend-ref", "", "per-repo fullsend workflow ref override")
484485
cmd.Flags().StringVar(&opts.mintURL, "mint-url", "", "per-repo mint URL override")
@@ -496,8 +497,10 @@ func runReposInstall(ctx context.Context, opts *reposInstallConfig) error {
496497
if opts.inferenceProject != "" && !repos.IsValidGCPProjectID(opts.inferenceProject) {
497498
return fmt.Errorf("--inference-project %q is not a valid GCP project ID (must be 6-30 lowercase letters, digits, hyphens; start with a letter, no trailing hyphen)", opts.inferenceProject)
498499
}
499-
if opts.inferenceProjectNumber != "" && !repos.IsNumeric(opts.inferenceProjectNumber) {
500-
return fmt.Errorf("--inference-project-number must be numeric, got %q", opts.inferenceProjectNumber)
500+
if opts.inferenceWIFProvider != "" {
501+
if err := validateWIFProvider(opts.inferenceWIFProvider); err != nil {
502+
return err
503+
}
501504
}
502505
if opts.forge != "" && !repos.IsValidForge(opts.forge) {
503506
return fmt.Errorf("--forge: %q is not a valid forge platform (valid: %s, %s)", opts.forge, repos.ForgeGitHub, repos.ForgeGitLab)
@@ -520,9 +523,11 @@ func runReposInstall(ctx context.Context, opts *reposInstallConfig) error {
520523
opts.inferenceRegion = "global"
521524
}
522525

523-
// Derive --inference-project-number from --inference-project via
524-
// the GCP Resource Manager API when not explicitly provided.
525-
if opts.inferenceProject != "" && opts.inferenceProjectNumber == "" {
526+
// When --inference-wif-provider is not set, derive the project number
527+
// from --inference-project via the GCP Resource Manager API so that
528+
// per-repo WIF provider paths can be constructed in converge.go.
529+
// Skip when the project number is already populated (internal use).
530+
if opts.inferenceProject != "" && opts.inferenceWIFProvider == "" && opts.inferenceProjectNumber == "" {
526531
var projectNumber string
527532
var lookupErr error
528533
if opts.testProjectNumberFn != nil {
@@ -532,7 +537,7 @@ func runReposInstall(ctx context.Context, opts *reposInstallConfig) error {
532537
projectNumber, lookupErr = gcpClient.GetProjectNumber(ctx, opts.inferenceProject)
533538
}
534539
if lookupErr != nil {
535-
return fmt.Errorf("deriving project number from %q: %w (use --inference-project-number to specify it manually)", opts.inferenceProject, lookupErr)
540+
return fmt.Errorf("deriving project number from %q: %w (use --inference-wif-provider to specify the full WIF provider path)", opts.inferenceProject, lookupErr)
536541
}
537542
opts.inferenceProjectNumber = projectNumber
538543
printer.StepDone(fmt.Sprintf("Derived project number %s from project %s", projectNumber, opts.inferenceProject))
@@ -727,6 +732,7 @@ func runReposInstall(ctx context.Context, opts *reposInstallConfig) error {
727732
InferenceProject: opts.inferenceProject,
728733
InferenceProjectNumber: opts.inferenceProjectNumber,
729734
InferenceRegion: opts.inferenceRegion,
735+
WIFProvider: opts.inferenceWIFProvider,
730736
ReviewAppClientID: reviewAppClientID,
731737
}
732738

‎internal/cli/repos_test.go‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,12 +1156,18 @@ func TestReposInstallCmd_ForgeFlag(t *testing.T) {
11561156

11571157
func TestReposInstallCmd_PerRepoOverrideFlags(t *testing.T) {
11581158
cmd := newReposInstallCmd()
1159-
for _, name := range []string{"inference-region", "fullsend-ref", "mint-url", "allowed-remote-resources"} {
1159+
for _, name := range []string{"inference-region", "inference-wif-provider", "fullsend-ref", "mint-url", "allowed-remote-resources"} {
11601160
f := cmd.Flags().Lookup(name)
11611161
require.NotNil(t, f, "expected --%s flag", name)
11621162
}
11631163
}
11641164

1165+
func TestReposInstallCmd_NoInferenceProjectNumberFlag(t *testing.T) {
1166+
cmd := newReposInstallCmd()
1167+
f := cmd.Flags().Lookup("inference-project-number")
1168+
assert.Nil(t, f, "--inference-project-number flag should be removed")
1169+
}
1170+
11651171
func TestRunReposInstall_AddsNewReposToManifest(t *testing.T) {
11661172
manifestPath := writeTestManifest(t, testManifestYAML)
11671173
fc := newInstallFakeClient("acme/api", "acme/web")
@@ -1383,16 +1389,16 @@ func TestRunReposInstall_InvalidInferenceProject(t *testing.T) {
13831389
assert.Contains(t, err.Error(), "--inference-project")
13841390
}
13851391

1386-
func TestRunReposInstall_InvalidInferenceProjectNumber(t *testing.T) {
1392+
func TestRunReposInstall_InvalidInferenceWIFProvider(t *testing.T) {
13871393
manifestPath := writeTestManifest(t, testManifestYAML)
13881394
err := runReposInstall(context.Background(), &reposInstallConfig{
1389-
manifest: manifestPath,
1390-
concurrency: 4,
1391-
inferenceProjectNumber: "not-a-number",
1392-
testClient: newInstallFakeClient(),
1395+
manifest: manifestPath,
1396+
concurrency: 4,
1397+
inferenceWIFProvider: "not-a-valid-provider",
1398+
testClient: newInstallFakeClient(),
13931399
})
13941400
require.Error(t, err)
1395-
assert.Contains(t, err.Error(), "--inference-project-number must be numeric")
1401+
assert.Contains(t, err.Error(), "--inference-wif-provider")
13961402
}
13971403

13981404
func TestRunReposInstall_DerivesProjectNumber(t *testing.T) {
@@ -1429,28 +1435,27 @@ func TestRunReposInstall_DerivesProjectNumber(t *testing.T) {
14291435
"inference region should default to global")
14301436
}
14311437

1432-
func TestRunReposInstall_ExplicitProjectNumberSkipsLookup(t *testing.T) {
1438+
func TestRunReposInstall_WIFProviderSkipsProjectNumberLookup(t *testing.T) {
14331439
manifestPath := writeTestManifest(t, testManifestYAML)
14341440
fc := newInstallFakeClient("acme/api")
14351441

14361442
lookupCalled := false
14371443
err := runReposInstall(context.Background(), &reposInstallConfig{
1438-
manifest: manifestPath,
1439-
concurrency: 4,
1440-
roles: []string{"triage"},
1441-
direct: true,
1442-
inferenceProject: "inf-proj",
1443-
inferenceProjectNumber: "111222333",
1444-
inferenceRegion: "us-central1",
1445-
testClient: fc,
1444+
manifest: manifestPath,
1445+
concurrency: 4,
1446+
roles: []string{"triage"},
1447+
direct: true,
1448+
inferenceProject: "inf-proj",
1449+
inferenceWIFProvider: "projects/123456789/locations/global/workloadIdentityPools/fullsend-inference/providers/github-oidc",
1450+
testClient: fc,
14461451
testProjectNumberFn: func(_ context.Context, _ string) (string, error) {
14471452
lookupCalled = true
14481453
return "999", nil
14491454
},
14501455
})
14511456
require.NoError(t, err)
14521457
assert.False(t, lookupCalled,
1453-
"project number lookup should be skipped when --inference-project-number is explicit")
1458+
"project number lookup should be skipped when --inference-wif-provider is set")
14541459
}
14551460

14561461
func TestRunReposInstall_DefaultsInferenceRegion(t *testing.T) {
@@ -1493,7 +1498,7 @@ func TestRunReposInstall_ProjectNumberLookupError(t *testing.T) {
14931498
require.Error(t, err)
14941499
assert.Contains(t, err.Error(), "deriving project number")
14951500
assert.Contains(t, err.Error(), "API unavailable")
1496-
assert.Contains(t, err.Error(), "--inference-project-number")
1501+
assert.Contains(t, err.Error(), "--inference-wif-provider")
14971502
}
14981503

14991504
func TestRunReposInstall_PerRepoOverrideFlags_Applied(t *testing.T) {

‎internal/repos/converge.go‎

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,18 @@ type ConvergeConfig struct {
4141

4242
// InferenceProject is the GCP project ID for inference.
4343
InferenceProject string
44-
// InferenceProjectNumber is the numeric GCP project number.
44+
// InferenceProjectNumber is the numeric GCP project number,
45+
// auto-derived from InferenceProject when WIFProvider is not set.
4546
InferenceProjectNumber string
4647
// InferenceRegion is the GCP region for inference.
4748
InferenceRegion string
4849

50+
// WIFProvider, when set, is used as the WIF provider resource name
51+
// for all repos instead of constructing per-repo provider IDs via
52+
// BuildRepoProviderID. This supports org-scoped WIF providers
53+
// (e.g., assertion.repository_owner == 'acme').
54+
WIFProvider string
55+
4956
// ReviewAppClientID is the OAuth client ID of the review agent's
5057
// GitHub App.
5158
ReviewAppClientID string
@@ -222,34 +229,52 @@ func Converge(ctx context.Context, cfg ConvergeConfig,
222229
return &ConvergeBatchResult{}, nil
223230
}
224231

225-
// Validate inference flags.
226-
inferenceFlags := []struct{ name, val string }{
227-
{"--inference-project", cfg.InferenceProject},
228-
{"--inference-project-number", cfg.InferenceProjectNumber},
229-
{"--inference-region", cfg.InferenceRegion},
230-
}
231-
var inferenceSet, inferenceMissing []string
232-
for _, f := range inferenceFlags {
233-
if f.val != "" {
234-
inferenceSet = append(inferenceSet, f.name)
235-
} else {
236-
inferenceMissing = append(inferenceMissing, f.name)
232+
// Validate inference flags. When --inference-wif-provider is set,
233+
// --inference-project-number is not required (the project number is
234+
// embedded in the provider path).
235+
if cfg.WIFProvider != "" {
236+
// --inference-project and --inference-region are required
237+
// alongside --inference-wif-provider because the secret-writing
238+
// paths gate on InferenceProject to decide whether to write
239+
// FULLSEND_GCP_PROJECT_ID and FULLSEND_GCP_WIF_PROVIDER.
240+
if cfg.InferenceProject == "" {
241+
return nil, fmt.Errorf("--inference-project is required when --inference-wif-provider is set")
237242
}
238-
}
239-
if len(inferenceSet) > 0 && len(inferenceMissing) > 0 {
240-
return nil, fmt.Errorf("incomplete inference flags: %s set but %s missing — all three are required when any is specified",
241-
strings.Join(inferenceSet, ", "), strings.Join(inferenceMissing, ", "))
242-
}
243-
244-
if cfg.InferenceProject != "" {
245243
if !IsValidGCPProjectID(cfg.InferenceProject) {
246244
return nil, fmt.Errorf("--inference-project %q is not a valid GCP project ID (must be 6-30 lowercase letters, digits, hyphens; start with a letter)", cfg.InferenceProject)
247245
}
248246
if !IsValidGCPRegion(cfg.InferenceRegion) {
249247
return nil, fmt.Errorf("--inference-region %q is not a valid GCP region (must be lowercase letters, digits, hyphens; start with a letter)", cfg.InferenceRegion)
250248
}
251-
if !IsNumeric(cfg.InferenceProjectNumber) {
252-
return nil, fmt.Errorf("--inference-project-number must be numeric, got %q", cfg.InferenceProjectNumber)
249+
} else {
250+
inferenceFlags := []struct{ name, val string }{
251+
{"--inference-project", cfg.InferenceProject},
252+
{"--inference-project-number", cfg.InferenceProjectNumber},
253+
{"--inference-region", cfg.InferenceRegion},
254+
}
255+
var inferenceSet, inferenceMissing []string
256+
for _, f := range inferenceFlags {
257+
if f.val != "" {
258+
inferenceSet = append(inferenceSet, f.name)
259+
} else {
260+
inferenceMissing = append(inferenceMissing, f.name)
261+
}
262+
}
263+
if len(inferenceSet) > 0 && len(inferenceMissing) > 0 {
264+
return nil, fmt.Errorf("incomplete inference flags: %s set but %s missing — all three are required when any is specified",
265+
strings.Join(inferenceSet, ", "), strings.Join(inferenceMissing, ", "))
266+
}
267+
268+
if cfg.InferenceProject != "" {
269+
if !IsValidGCPProjectID(cfg.InferenceProject) {
270+
return nil, fmt.Errorf("--inference-project %q is not a valid GCP project ID (must be 6-30 lowercase letters, digits, hyphens; start with a letter)", cfg.InferenceProject)
271+
}
272+
if !IsValidGCPRegion(cfg.InferenceRegion) {
273+
return nil, fmt.Errorf("--inference-region %q is not a valid GCP region (must be lowercase letters, digits, hyphens; start with a letter)", cfg.InferenceRegion)
274+
}
275+
if !IsNumeric(cfg.InferenceProjectNumber) {
276+
return nil, fmt.Errorf("--inference-project-number must be numeric, got %q", cfg.InferenceProjectNumber)
277+
}
253278
}
254279
}
255280

@@ -351,6 +376,10 @@ func Converge(ctx context.Context, cfg ConvergeConfig,
351376
var wif string
352377
if !hasSecrets {
353378
switch {
379+
case cfg.WIFProvider != "":
380+
// Explicit WIF provider — use it verbatim for all repos.
381+
// No per-repo derivation or collision check needed.
382+
wif = cfg.WIFProvider
354383
case d.resolved.Forge == ForgeGitHub && cfg.InferenceProjectNumber != "":
355384
providerID := mintcore.BuildRepoProviderID(d.repo.Owner, d.repo.Repo)
356385
wif = fmt.Sprintf("projects/%s/locations/global/workloadIdentityPools/%s/providers/%s",
@@ -378,7 +407,7 @@ func Converge(ctx context.Context, cfg ConvergeConfig,
378407
}
379408

380409
// Validate inference flags for repos without existing secrets.
381-
if cfg.InferenceProject == "" {
410+
if cfg.InferenceProject == "" && cfg.WIFProvider == "" {
382411
repoFullName := d.repo.Owner + "/" + d.repo.Repo
383412
result.Results[i] = ConvergeResult{
384413
Owner: d.repo.Owner,

0 commit comments

Comments
 (0)