diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 74211a081..74e357e4b 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -1277,7 +1277,10 @@ func runEnableRepos(ctx context.Context, client forge.Client, printer *ui.Printe } // Determine which repos to enable. + // We always need the full org repo list (for validation or discovery), + // so fetch it once and reuse for org variable visibility sync later. var reposToEnable []string + var allOrgRepos []forge.Repository if all { // Get all org repos by calling ListOrgRepos. // Note: disable --all iterates cfg.Repos instead of calling ListOrgRepos. @@ -1285,13 +1288,13 @@ func runEnableRepos(ctx context.Context, client forge.Client, printer *ui.Printe // while disable --all operates on previously configured repos (which may have // been deleted from the org but still need unenrollment PRs for cleanup). printer.StepStart("Discovering all organization repositories") - allRepos, err := client.ListOrgRepos(ctx, org) + allOrgRepos, err = client.ListOrgRepos(ctx, org) if err != nil { printer.StepFail("Failed to list organization repositories") printer.StepInfo("Hint: verify your token has 'repo' scope with: gh auth refresh -s repo") return fmt.Errorf("listing org repos: %w", err) } - for _, r := range allRepos { + for _, r := range allOrgRepos { if r.Name != forge.ConfigRepoName { reposToEnable = append(reposToEnable, r.Name) } @@ -1304,7 +1307,7 @@ func runEnableRepos(ctx context.Context, client forge.Client, printer *ui.Printe // one API call per repo (O(n) → O(1) API calls). printer.StepStart("Validating repository names") - allOrgRepos, err := client.ListOrgRepos(ctx, org) + allOrgRepos, err = client.ListOrgRepos(ctx, org) if err != nil { printer.StepFail("Failed to list organization repositories") printer.StepInfo("Hint: verify your token has 'repo' scope with: gh auth refresh -s repo") @@ -1366,6 +1369,14 @@ func runEnableRepos(ctx context.Context, client forge.Client, printer *ui.Printe return err } + // Sync org variable visibility so newly enrolled repos can read + // dispatch variables like FULLSEND_MINT_URL. Without this, the + // shim workflow in the new repo fails because the org variable + // has "selected" visibility that doesn't include the new repo. + if cfg.Dispatch.Mode == "oidc-mint" { + syncOrgVariableVisibility(ctx, client, printer, org, cfg, allOrgRepos) + } + printer.Blank() printer.Summary("Repositories enabled", []string{ fmt.Sprintf("Organization: %s", org), @@ -1376,6 +1387,60 @@ func runEnableRepos(ctx context.Context, client forge.Client, printer *ui.Printe return nil } +// dispatchOrgVariableNames returns the org-level variable names managed by the +// dispatch layer. This is kept in sync with the gcf.Provisioner.OrgVariableNames() +// method. We avoid importing the gcf package here to keep the CLI layer thin; +// adding a variable to the dispatcher requires updating this list. +var dispatchOrgVariableNames = []string{"FULLSEND_MINT_URL"} + +// syncOrgVariableVisibility updates the "selected" repository list for each +// dispatch org variable so that all currently enrolled repos (plus the config +// repo) can read them. This is best-effort: failures are logged as warnings +// but do not fail the enable command, because the repo-maintenance workflow +// can reconcile this later. +func syncOrgVariableVisibility(ctx context.Context, client forge.Client, printer *ui.Printer, org string, cfg *config.OrgConfig, allOrgRepos []forge.Repository) { + // Build a name→ID lookup from the discovered org repos. + repoIDByName := make(map[string]int64, len(allOrgRepos)) + for _, r := range allOrgRepos { + repoIDByName[r.Name] = r.ID + } + + // Collect IDs for all enabled repos. + enrolledRepoIDs := collectEnrolledRepoIDs(allOrgRepos, cfg.EnabledRepos()) + + // Ensure the config repo (.fullsend) is included — it needs access + // to dispatch variables for its own workflows. + configRepo, err := client.GetRepo(ctx, org, forge.ConfigRepoName) + if err == nil && configRepo != nil { + seen := make(map[int64]bool, len(enrolledRepoIDs)) + for _, id := range enrolledRepoIDs { + seen[id] = true + } + if !seen[configRepo.ID] { + enrolledRepoIDs = append(enrolledRepoIDs, configRepo.ID) + } + } + + for _, varName := range dispatchOrgVariableNames { + exists, checkErr := client.OrgVariableExists(ctx, org, varName) + if checkErr != nil { + printer.StepWarn(fmt.Sprintf("could not check org variable %s: %v", varName, checkErr)) + continue + } + if !exists { + // Variable not yet created (e.g. mint not provisioned yet). + continue + } + + printer.StepStart(fmt.Sprintf("Updating %s visibility for enrolled repos", varName)) + if setErr := client.SetOrgVariableRepos(ctx, org, varName, enrolledRepoIDs); setErr != nil { + printer.StepWarn(fmt.Sprintf("failed to update %s visibility: %v", varName, setErr)) + } else { + printer.StepDone(fmt.Sprintf("Updated %s visibility (%d repos)", varName, len(enrolledRepoIDs))) + } + } +} + // runDisableRepos disables the specified repositories from fullsend enrollment. func runDisableRepos(ctx context.Context, client forge.Client, printer *ui.Printer, org string, repos []string, all bool, yolo bool) error { printer.Banner() diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index c18841562..b379f8409 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -512,6 +512,85 @@ func TestRunEnableRepos_CommitMessageFormat(t *testing.T) { assert.Contains(t, client.CreatedFiles[0].Message, "chore: enable 2 repositories") } +func TestRunEnableRepos_UpdatesOrgVariableVisibility(t *testing.T) { + // Setup: initial config with repo A enabled, repo B disabled. + // Dispatch mode is oidc-mint. Org variable FULLSEND_MINT_URL exists. + cfg := setupTestConfig(map[string]bool{ + "web-app": true, + "api": false, + }) + cfg.Dispatch.Mode = "oidc-mint" + + client := setupTestClient("testorg", cfg, []string{"web-app", "api"}) + // Assign repo IDs so we can verify they appear in the variable visibility. + for i := range client.Repos { + switch client.Repos[i].Name { + case ".fullsend": + client.Repos[i].ID = 100 + case "web-app": + client.Repos[i].ID = 200 + case "api": + client.Repos[i].ID = 300 + } + } + // Pre-populate the org variable so it "exists". + client.OrgVariables = map[string]bool{"testorg/FULLSEND_MINT_URL": true} + client.OrgVariableValues = map[string]string{"testorg/FULLSEND_MINT_URL": "https://mint.example.com"} + + printer := ui.New(&discardWriter{}) + + // Action: enable repo "api". + err := runEnableRepos(context.Background(), client, printer, "testorg", []string{"api"}, false, true) + require.NoError(t, err) + + // Assert: SetOrgVariableRepos was called with both enrolled repo IDs + // plus the config repo (.fullsend). + require.Contains(t, client.OrgVariableRepoIDs, "testorg/FULLSEND_MINT_URL") + repoIDs := client.OrgVariableRepoIDs["testorg/FULLSEND_MINT_URL"] + assert.Contains(t, repoIDs, int64(100), "config repo ID should be included") + assert.Contains(t, repoIDs, int64(200), "web-app repo ID should be included") + assert.Contains(t, repoIDs, int64(300), "api repo ID should be included") +} + +func TestRunEnableRepos_SkipsVariableSyncWhenNotOIDCMint(t *testing.T) { + // When dispatch mode is not oidc-mint, variable sync should be skipped. + cfg := setupTestConfig(map[string]bool{ + "web-app": false, + }) + // No dispatch mode set (empty string). + + client := setupTestClient("testorg", cfg, []string{"web-app"}) + client.OrgVariables = map[string]bool{"testorg/FULLSEND_MINT_URL": true} + + printer := ui.New(&discardWriter{}) + + err := runEnableRepos(context.Background(), client, printer, "testorg", []string{"web-app"}, false, true) + require.NoError(t, err) + + // SetOrgVariableRepos should not have been called. + assert.Nil(t, client.OrgVariableRepoIDs) +} + +func TestRunEnableRepos_SkipsVariableSyncWhenVariableNotExists(t *testing.T) { + // When the org variable doesn't exist yet (mint not provisioned), + // sync should skip gracefully. + cfg := setupTestConfig(map[string]bool{ + "web-app": false, + }) + cfg.Dispatch.Mode = "oidc-mint" + + client := setupTestClient("testorg", cfg, []string{"web-app"}) + // No OrgVariables set — FULLSEND_MINT_URL doesn't exist. + + printer := ui.New(&discardWriter{}) + + err := runEnableRepos(context.Background(), client, printer, "testorg", []string{"web-app"}, false, true) + require.NoError(t, err) + + // SetOrgVariableRepos should not have been called. + assert.Nil(t, client.OrgVariableRepoIDs) +} + // Business logic tests for runDisableRepos func TestRunDisableRepos_DisableSingleRepo(t *testing.T) { diff --git a/internal/forge/fake.go b/internal/forge/fake.go index a754f1cf6..6cb38e2a7 100644 --- a/internal/forge/fake.go +++ b/internal/forge/fake.go @@ -109,8 +109,9 @@ type FakeClient struct { OrgSecretRepoIDs map[string][]int64 // key: "org/name" → repo IDs // Org-level variable state - OrgVariables map[string]bool // key: "org/name" - OrgVariableValues map[string]string // key: "org/name" → value + OrgVariables map[string]bool // key: "org/name" + OrgVariableValues map[string]string // key: "org/name" → value + OrgVariableRepoIDs map[string][]int64 // key: "org/name" → repo IDs // Error injection: key is method name, value is error to return. Errors map[string]error @@ -936,6 +937,11 @@ func (f *FakeClient) CreateOrUpdateOrgVariable(_ context.Context, org, name, val f.OrgVariableValues = make(map[string]string) } f.OrgVariableValues[org+"/"+name] = value + + if f.OrgVariableRepoIDs == nil { + f.OrgVariableRepoIDs = make(map[string][]int64) + } + f.OrgVariableRepoIDs[org+"/"+name] = selectedRepoIDs return nil } @@ -953,6 +959,35 @@ func (f *FakeClient) OrgVariableExists(_ context.Context, org, name string) (boo return f.OrgVariables[org+"/"+name], nil } +func (f *FakeClient) SetOrgVariableRepos(_ context.Context, org, name string, repoIDs []int64) error { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("SetOrgVariableRepos"); e != nil { + return e + } + + if f.OrgVariableRepoIDs == nil { + f.OrgVariableRepoIDs = make(map[string][]int64) + } + f.OrgVariableRepoIDs[org+"/"+name] = repoIDs + return nil +} + +func (f *FakeClient) GetOrgVariableRepos(_ context.Context, org, name string) ([]int64, error) { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("GetOrgVariableRepos"); e != nil { + return nil, e + } + + if f.OrgVariableRepoIDs == nil { + return nil, nil + } + return f.OrgVariableRepoIDs[org+"/"+name], nil +} + func (f *FakeClient) DeleteOrgVariable(_ context.Context, org, name string) error { f.mu.Lock() defer f.mu.Unlock() diff --git a/internal/forge/fake_test.go b/internal/forge/fake_test.go index 6ea573120..8da76a826 100644 --- a/internal/forge/fake_test.go +++ b/internal/forge/fake_test.go @@ -457,6 +457,13 @@ func TestFakeClient_ErrorInjection(t *testing.T) { {"DeleteOrgVariable", func(fc *FakeClient) error { return fc.DeleteOrgVariable(ctx, "o", "n") }}, + {"SetOrgVariableRepos", func(fc *FakeClient) error { + return fc.SetOrgVariableRepos(ctx, "o", "n", nil) + }}, + {"GetOrgVariableRepos", func(fc *FakeClient) error { + _, err := fc.GetOrgVariableRepos(ctx, "o", "n") + return err + }}, } for _, m := range methods { @@ -522,6 +529,8 @@ func TestFakeClient_ThreadSafety(t *testing.T) { _ = fc.CreateOrUpdateOrgVariable(ctx, "o", "n", "v", []int64{1}) _, _ = fc.OrgVariableExists(ctx, "o", "var") _ = fc.DeleteOrgVariable(ctx, "o", "n") + _ = fc.SetOrgVariableRepos(ctx, "o", "n", []int64{1, 2}) + _, _ = fc.GetOrgVariableRepos(ctx, "o", "n") }(i) } diff --git a/internal/forge/forge.go b/internal/forge/forge.go index f2290ad45..ba80582a7 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -178,6 +178,10 @@ type Client interface { CreateOrUpdateOrgVariable(ctx context.Context, org, name, value string, selectedRepoIDs []int64) error OrgVariableExists(ctx context.Context, org, name string) (bool, error) DeleteOrgVariable(ctx context.Context, org, name string) error + SetOrgVariableRepos(ctx context.Context, org, name string, repoIDs []int64) error + // GetOrgVariableRepos returns the list of repository IDs that have access + // to the given org-level variable. + GetOrgVariableRepos(ctx context.Context, org, name string) ([]int64, error) // CI/Workflow operations GetLatestWorkflowRun(ctx context.Context, owner, repo, workflowFile string) (*WorkflowRun, error) diff --git a/internal/forge/github/github.go b/internal/forge/github/github.go index 58d0a25bd..6fd29b88f 100644 --- a/internal/forge/github/github.go +++ b/internal/forge/github/github.go @@ -1756,6 +1756,47 @@ func (c *LiveClient) DeleteOrgVariable(ctx context.Context, org, name string) er return &APIError{StatusCode: resp.StatusCode, Message: "unexpected status deleting org variable"} } +// SetOrgVariableRepos sets the list of repositories that can access an org variable. +func (c *LiveClient) SetOrgVariableRepos(ctx context.Context, org, name string, repoIDs []int64) error { + if repoIDs == nil { + repoIDs = []int64{} + } + payload := map[string]any{ + "selected_repository_ids": repoIDs, + } + + resp, err := c.put(ctx, fmt.Sprintf("/orgs/%s/actions/variables/%s/repositories", org, name), payload) + if err != nil { + return fmt.Errorf("set org variable repos for %s: %w", name, err) + } + resp.Body.Close() + return nil +} + +// GetOrgVariableRepos returns the repository IDs that have access to an org variable. +func (c *LiveClient) GetOrgVariableRepos(ctx context.Context, org, name string) ([]int64, error) { + resp, err := c.get(ctx, fmt.Sprintf("/orgs/%s/actions/variables/%s/repositories", org, name)) + if err != nil { + return nil, fmt.Errorf("get org variable repos for %s: %w", name, err) + } + defer resp.Body.Close() + + var result struct { + Repositories []struct { + ID int64 `json:"id"` + } `json:"repositories"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("decode org variable repos for %s: %w", name, err) + } + + ids := make([]int64, len(result.Repositories)) + for i, r := range result.Repositories { + ids[i] = r.ID + } + return ids, nil +} + // isNotFound checks whether an error is a 404 API error. func isNotFound(err error) bool { var apiErr *APIError