diff --git a/docs/ADRs/0057-repos-management.md b/docs/ADRs/0057-repos-management.md index d80ada26c7..5e3f9bfb6c 100644 --- a/docs/ADRs/0057-repos-management.md +++ b/docs/ADRs/0057-repos-management.md @@ -126,7 +126,13 @@ and implementation details are in the ## Implementation status -`repos install` (batch install with WIF serialization) is being implemented in PR #3033. -Remaining subcommands (`repos init`, `repos status`, `repos sync`, -`repos upgrade`, `repos upgrade-mint`, `repos remove`) are tracked in the +Implemented subcommands: + +- `repos init` — PR #3033 +- `repos install` — PR #3033 +- `repos status` — PR #4079 +- `repos add`, `repos remove`, `repos uninstall` — PR #4081 + +Remaining subcommands (`repos sync`, `repos diff`, `repos upgrade`, +`repos upgrade-mint`) are tracked in the [repos management plan](../plans/repos-management.md). diff --git a/docs/cli/repos.md b/docs/cli/repos.md index b13eaf413f..bd6d1a5f05 100644 --- a/docs/cli/repos.md +++ b/docs/cli/repos.md @@ -11,7 +11,10 @@ Manage per-repo installations across multiple orgs via a declarative `repos.yaml | Command | Description | |---------|-------------| | `fullsend repos init ` | Generate a repos.yaml manifest by discovering existing installations | -| `fullsend repos install` | Install fullsend on uninstalled manifest repos | +| `fullsend repos install [repos...]` | Install fullsend on uninstalled manifest repos | +| `fullsend repos add ` | Add repo entries to a repos.yaml manifest | +| `fullsend repos remove ` | Remove repo entries from a repos.yaml manifest | +| `fullsend repos uninstall ` | Tear down fullsend from specific repos | | `fullsend repos status` | Compare manifest against actual repo state | ## `repos init` @@ -73,17 +76,18 @@ Runs in three phases: ```bash fullsend repos install -f repos.yaml fullsend repos install --dry-run -fullsend repos install --repo acme/api --repo acme/web -fullsend repos install --direct --concurrency 8 +fullsend repos install acme/api acme/web +fullsend repos install "acme/*" --direct --concurrency 8 ``` +When repos are specified as positional arguments, only those repos are installed. Glob patterns (e.g. `acme/*`) are matched against manifest entries. When no repos are specified, all manifest repos are installed. + ### Flags | Flag | Default | Description | |------|---------|-------------| | `-f`, `--manifest` | `repos.yaml` | Path or URL to repos.yaml manifest | | `--dry-run` | `false` | Preview what would be installed without making changes | -| `--repo` | (all) | Install specific repos only (repeatable) | | `--skip-mint-check` | `false` | Skip mint URL discovery and org registration (EnsureOrgInMint). Use when orgs are already registered in the mint. | | `--concurrency` | `4` | Max parallel operations (1-32) | | `--roles` | `triage,coder,review,fix,retro,prioritize` | Agent roles to install | @@ -106,7 +110,7 @@ fullsend repos install -f repos.yaml --dry-run Install specific repos (orgs already registered): ```bash -fullsend repos install --repo acme/api --repo acme/web --skip-mint-check +fullsend repos install acme/api acme/web --skip-mint-check ``` > **Note:** Without `--skip-mint-check`, `repos install` will register any new @@ -121,8 +125,8 @@ Read-only comparison of the `repos.yaml` manifest against actual forge state. Re ```bash fullsend repos status fullsend repos status -f path/to/repos.yaml -fullsend repos status --repo owner/repo1 --repo owner/repo2 -fullsend repos status --json +fullsend repos status --repo acme/api --repo acme/web +fullsend repos status --repo "acme/*" --json ``` ### Flags @@ -130,8 +134,8 @@ fullsend repos status --json | Flag | Short | Default | Description | |------|-------|---------|-------------| | `--manifest` | `-f` | `repos.yaml` | Path or HTTPS URL to manifest file | +| `--repo` | | | Filter to specific repos (repeatable, supports globs) | | `--json` | | `false` | Emit JSON output instead of table | -| `--repo` | | | Filter to specific repos (repeatable) | | `--concurrency` | | `8` | Max parallel API calls | ### Output @@ -153,6 +157,85 @@ The command returns a non-zero exit code when any repo has drift, is not install Requires a GitHub token via `GH_TOKEN`, `GITHUB_TOKEN`, or `gh auth token`. +## `repos add` + +Add one or more repo entries to the `repos.yaml` manifest file, editing it in place. Use `--install` to also install fullsend on the added repos after updating the manifest. + +```bash +fullsend repos add acme/new-api acme/new-web +fullsend repos add acme/new-api --install --direct +fullsend repos add acme/new-api --dry-run +``` + +### Flags + +| Flag | Default | Description | +|------|---------|-------------| +| `-f`, `--manifest` | `repos.yaml` | Path to repos.yaml manifest | +| `--dry-run` | `false` | Preview what would be added without making changes | +| `--install` | `false` | Also install fullsend on the added repos | +| `--concurrency` | `4` | Max parallel operations (1-32, used with `--install`) | +| `--direct` | `false` | Push scaffold directly to default branch (used with `--install`) | +| `--roles` | default roles | Agent roles to install (used with `--install`) | + +Duplicate entries are silently skipped. Glob patterns (e.g. `acme/*`) are allowed as manifest entries. + +> **Note:** With `--install`, the manifest is updated before installation begins. +> If installation fails for some repos, those entries remain in the manifest as +> desired state. Run `fullsend repos status` to identify repos that need +> re-installation, then `fullsend repos install ` to retry. + +## `repos remove` + +Remove one or more repo entries from the `repos.yaml` manifest file, editing it in place. When multiple repos are targeted (via globs or explicit bulk lists), the command prompts for confirmation unless `--yes` is set. + +Use `--uninstall` to tear down fullsend from the repos before removing them from the manifest (deletes workflow, variables, secrets, and WIF). + +```bash +fullsend repos remove acme/old-api +fullsend repos remove "acme/*" --yes +fullsend repos remove acme/old-api --uninstall +fullsend repos remove acme/old-api --uninstall --skip-wif-cleanup +``` + +### Flags + +| Flag | Default | Description | +|------|---------|-------------| +| `-f`, `--manifest` | `repos.yaml` | Path to repos.yaml manifest | +| `--dry-run` | `false` | Preview what would be removed without making changes | +| `--uninstall` | `false` | Tear down fullsend from repos before removing from manifest | +| `--yes` | `false` | Skip confirmation prompt when multiple repos are targeted | +| `--skip-wif-cleanup` | `false` | Skip GCP WIF provider deletion (only with `--uninstall`) | +| `--concurrency` | `4` | Max parallel operations (1-32, used with `--uninstall`) | + +## `repos uninstall` + +Tear down fullsend from the specified repos by deleting workflow files, variables, secrets, and WIF infrastructure. Does **not** modify `repos.yaml` — use `repos remove` for that. + +When multiple repos are targeted (via globs or explicit bulk lists), the command prompts for confirmation unless `--yes` is set. + +Runs in two phases: +1. **Parallel per-repo cleanup** — delete workflow, variables, secrets (concurrent) +2. **Sequential WIF deregistration** — deregister from mint and delete WIF provider + +```bash +fullsend repos uninstall acme/old-api +fullsend repos uninstall "acme/*" --yes +fullsend repos uninstall acme/old-api --skip-wif-cleanup +fullsend repos uninstall acme/old-api --dry-run +``` + +### Flags + +| Flag | Default | Description | +|------|---------|-------------| +| `-f`, `--manifest` | `repos.yaml` | Path to repos.yaml manifest | +| `--dry-run` | `false` | Preview what would be uninstalled without making changes | +| `--yes` | `false` | Skip confirmation prompt when multiple repos are targeted | +| `--skip-wif-cleanup` | `false` | Skip GCP WIF provider deletion | +| `--concurrency` | `4` | Max parallel operations (1-32) | + ## See also - [Getting Started](../guides/getting-started/) — Standard per-repo installation diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index f7a9ee3498..63e8c5bded 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -48,14 +48,33 @@ fullsend │ │ ├── --inference-project # Default GCP project for inference │ │ ├── --force # Overwrite output file if it exists │ │ └── --concurrency # Max parallel API calls (default: 8) -│ ├── install # Install fullsend on uninstalled manifest repos +│ ├── install [repos...] # Install fullsend on uninstalled manifest repos │ │ ├── -f, --manifest # Path or URL to repos.yaml (default: repos.yaml) │ │ ├── --dry-run # Preview without making changes -│ │ ├── --repo # Install specific repos only (repeatable) │ │ ├── --skip-mint-check # Skip org registration in mint │ │ ├── --concurrency # Max parallel operations (1-32, default: 4) │ │ ├── --roles # Agent roles (default: triage,coder,review,fix,retro,prioritize) │ │ └── --direct # Push scaffold to default branch (skip PR) +│ ├── add # Add repo entries to manifest +│ │ ├── -f, --manifest # Path to repos.yaml (default: repos.yaml) +│ │ ├── --dry-run # Preview without making changes +│ │ ├── --install # Also install fullsend on the added repos +│ │ ├── --concurrency # Max parallel operations (1-32, default: 4) +│ │ ├── --direct # Push scaffold to default branch (skip PR) +│ │ └── --roles # Agent roles to install (used with --install) +│ ├── remove # Remove repo entries from manifest +│ │ ├── -f, --manifest # Path to repos.yaml (default: repos.yaml) +│ │ ├── --dry-run # Preview without making changes +│ │ ├── --uninstall # Tear down fullsend before removing +│ │ ├── --yes # Skip confirmation for glob patterns +│ │ ├── --skip-wif-cleanup # Skip GCP WIF provider deletion +│ │ └── --concurrency # Max parallel operations (1-32, default: 4) +│ ├── uninstall # Tear down fullsend from repos +│ │ ├── -f, --manifest # Path to repos.yaml (default: repos.yaml) +│ │ ├── --dry-run # Preview without making changes +│ │ ├── --yes # Skip confirmation for glob patterns +│ │ ├── --skip-wif-cleanup # Skip GCP WIF provider deletion +│ │ └── --concurrency # Max parallel operations (1-32, default: 4) │ └── status # Compare manifest against actual repo state ├── agent # Manage agent registrations in config │ ├── add # Register an agent (URL auto-pinned) diff --git a/docs/guides/getting-started/operations.md b/docs/guides/getting-started/operations.md index c69c8ef313..d9b225713c 100644 --- a/docs/guides/getting-started/operations.md +++ b/docs/guides/getting-started/operations.md @@ -72,7 +72,10 @@ For organizations that separate GCP and GitHub responsibilities across teams, fu | GCP Admin (Mint) | `fullsend mint status` | Inspect mint state and PEM health | | Fleet Admin | `fullsend repos init ` | Generate a `repos.yaml` manifest by discovering existing installations | -| Platform Admin | `fullsend repos install -f repos.yaml` | Bulk-install fullsend on repos from a declarative manifest (parallel discovery → sequential WIF → parallel scaffold) | +| Platform Admin | `fullsend repos install [repos...]` | Bulk-install fullsend on repos from a declarative manifest (parallel discovery → sequential WIF → parallel scaffold) | +| Fleet Admin | `fullsend repos add ` | Add repo entries to `repos.yaml` manifest (with optional `--install`) | +| Fleet Admin | `fullsend repos remove ` | Remove repo entries from `repos.yaml` manifest (with optional `--uninstall`) | +| Platform Admin | `fullsend repos uninstall ` | Tear down fullsend from repos (workflow, variables, secrets, WIF) without modifying manifest | | Fleet Admin | `fullsend repos status` | Compare `repos.yaml` manifest against actual per-repo state (drift detection) | | Developer | `fullsend agent add ` | Register an agent in config (URL auto-pinned to commit SHA) | diff --git a/docs/plans/repos-management.md b/docs/plans/repos-management.md index 0e9e1d3c3f..0f8604cacc 100644 --- a/docs/plans/repos-management.md +++ b/docs/plans/repos-management.md @@ -9,7 +9,7 @@ per-repo installations across multiple orgs. The work is structured as 8 PRs across two phases. Phase 1 (PRs 1–4) builds the foundation: extracting reusable install logic, the manifest parser, a new forge method, and read-only status. Phase 2 (PRs 5–8) -adds write operations: bulk install, sync/diff, upgrade, and remove. +adds write operations: bulk install, sync/diff, upgrade, add/remove/uninstall. --- @@ -181,7 +181,7 @@ Installs fullsend on repos not yet installed. Three-phase execution: Concurrent `repos install` and `fullsend github setup` targeting the same repo are unsafe — no distributed lock is held. -Supports `--dry-run`, `--repo` (filter), `--concurrency`. +Supports `--dry-run`, positional args (filter, supports globs), `--concurrency`. #### `fullsend repos diff` @@ -244,19 +244,40 @@ is behind the target fullsend ref. The `/health` endpoint must be extended to include a `version` field (currently only returns `{"status":"ok"}`). +#### `fullsend repos add` + +Adds repo entries to the `repos.yaml` manifest. Supports glob patterns +(e.g. `acme/*`) and validates repo name format (`owner/repo`). Skips +duplicates. With `--install`, also installs fullsend on the added repos. + +Supports `--dry-run`, `--install`, `--concurrency`, `--direct`. + #### `fullsend repos remove` -Removes fullsend from specific repos. Requires explicit repo names — -no glob expansion, to prevent accidental bulk deletion. +Removes repo entries from the `repos.yaml` manifest. Glob patterns are +matched against manifest entries and prompt for confirmation unless +`--yes` is set. + +With `--uninstall`, tears down fullsend from the matched repos before +removing them from the manifest (deletes workflow, variables, secrets, +and WIF infrastructure). + +Supports `--dry-run`, `--uninstall`, `--yes`, `--skip-wif-cleanup`, +`--concurrency`. + +#### `fullsend repos uninstall` + +Tears down fullsend from specific repos without modifying the manifest. +Glob patterns are matched against manifest entries. For each repo: deletes workflow file, variables, secrets, deregisters from mint's `PER_REPO_WIF_REPOS` (sequential), deletes WIF provider. -Does **not** remove repos from the manifest (operator edits manually). -Does **not** remove `.fullsend/` — it contains user-authored config -that may be version-controlled independently. +Does **not** remove repos from the manifest — use `repos remove` for +that. Does **not** remove `.fullsend/` — it contains user-authored +config that may be version-controlled independently. -Supports `--dry-run`, `--skip-wif-cleanup`, `--concurrency`. +Supports `--dry-run`, `--yes`, `--skip-wif-cleanup`, `--concurrency`. ### Version management @@ -314,7 +335,7 @@ should be proposed in its own ADR when pursued. PRs 1, 2, 3 ─────────> PR 5 (repos install) PRs 2, 3 ────> PR 4 (repos status) ──┬──> PR 6 (sync/diff) └──> PR 7 (upgrade) -PRs 1, 3 ─────────> PR 8 (remove) +PRs 1, 3 ─────────> PR 8 (add/remove/uninstall) ``` PRs 1, 2, 3 are independent and can be developed in parallel. @@ -326,7 +347,8 @@ PR 7 depends on PR 4 (reuses `extractWorkflowRef()` for reading current refs from workflow files). PR 8 depends on PRs 1 and 3 (reuses install types + `DeleteRepoVariable`/`DeleteRepoSecret`) and can be developed in -parallel with PRs 4–7. +parallel with PRs 4–7. Implements three commands: `repos add`, +`repos remove`, and `repos uninstall`. The `repos init` command is covered by a [separate implementation plan](repos-init.md) and can be developed @@ -845,7 +867,7 @@ Flags: - `--manifest` / `-f` (string, default `repos.yaml`): path or URL. - `--dry-run` (bool). -- `--repo` (string, repeatable): install specific repos only. +- Positional args: install specific repos only (supports globs). - `--skip-app-setup` (bool). - `--skip-mint-check` (bool). - `--concurrency` (int, default 4): max parallel scaffold writes. @@ -1156,9 +1178,9 @@ tests. Mint compatibility tested with a fake HTTP server. --- -### PR 8: `fullsend repos remove` (uninstall) +### PR 8: repos management (add, remove, uninstall) -**Scope:** New CLI command. Deletes infrastructure. +**Scope:** Three new CLI commands for managing per-repo installations. **Depends on:** PR 1 (reuses types), PR 3 (`DeleteRepoVariable`, `DeleteRepoSecret`). @@ -1167,21 +1189,33 @@ Can be developed in parallel with PRs 4–7. #### `internal/cli/repos.go` (modify) -Add `newReposRemoveCmd()`. +Add `newReposAddCmd()`, `newReposRemoveCmd()`, `newReposUninstallCmd()`. -Flags: +`repos add` — adds repo entries to the manifest: +- Positional args: repos to add (supports globs). +- `--manifest` / `-f`. +- `--dry-run`. +- `--install`: also install fullsend on added repos. +- `--concurrency`, `--direct` (used with `--install`). +`repos remove` — removes repo entries from the manifest: +- Positional args: repos to remove (supports globs). +- `--manifest` / `-f`. +- `--dry-run`. +- `--uninstall`: tear down fullsend before removing from manifest. +- `--yes`: skip confirmation for glob patterns. +- `--skip-wif-cleanup`, `--concurrency` (used with `--uninstall`). + +`repos uninstall` — tears down fullsend without modifying manifest: +- Positional args: repos to uninstall (supports globs). - `--manifest` / `-f`: used to resolve mint config for WIF cleanup. -- `--repo` (repeatable, **required**): repos to remove. - `--dry-run`. +- `--yes`: skip confirmation for glob patterns. - `--skip-wif-cleanup`: skip GCP WIF provider deletion and mint deregistration. - `--concurrency` (int, default 4): max parallel Phase 1 cleanup operations. -No glob expansion. `--repo` requires exact `owner/repo` values to -prevent accidental bulk removal. - #### `internal/repos/remove.go` (new) ```go diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 9d894d3585..3abf13b2e0 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -1175,6 +1175,18 @@ func (a *gcfProvisionerAdapter) DeletePerRepoWIF(ctx context.Context, repo strin return a.provisioner.RemoveRepoFromMint(ctx, repo) } +func (a *gcfProvisionerAdapter) DeleteWIFProvider(ctx context.Context, repo string) error { + if a.provisioner == nil { + return fmt.Errorf("WIF provisioner not configured") + } + parts := strings.SplitN(repo, "/", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return fmt.Errorf("invalid repo format %q: expected owner/repo", repo) + } + providerID := mintcore.BuildRepoProviderID(strings.ToLower(parts[0]), strings.ToLower(parts[1])) + return a.provisioner.DeleteWIFProvider(ctx, providerID) +} + // applyPerRepoScaffold commits scaffold files to the repo's default branch // and configures the repository variables and secrets needed for fullsend. func applyPerRepoScaffold(ctx context.Context, client forge.Client, printer *ui.Printer, diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index fcd1adf86d..88a8b038ca 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -2139,6 +2139,7 @@ func (p *testWIFProvisioner) ProvisionWIF(_ context.Context) (string, error) { func (p *testWIFProvisioner) RegisterPerRepoWIF(_ context.Context, _ string) error { return nil } func (p *testWIFProvisioner) EnsureOrgInMint(_ context.Context, _, _ string) error { return nil } func (p *testWIFProvisioner) DeletePerRepoWIF(_ context.Context, _ string) error { return nil } +func (p *testWIFProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { return nil } func perRepoTestBase() perRepoInstallConfig { return perRepoInstallConfig{ @@ -3218,6 +3219,27 @@ func TestGCFWIFAdapter_DeletePerRepoWIF_NilProvisioner(t *testing.T) { assert.Contains(t, err.Error(), "not configured") } +func TestGCFWIFAdapter_DeleteWIFProvider_NilProvisioner(t *testing.T) { + adapter := &gcfProvisionerAdapter{provisioner: nil} + err := adapter.DeleteWIFProvider(context.Background(), "acme/widget") + require.Error(t, err) + assert.Contains(t, err.Error(), "not configured") +} + +func TestGCFWIFAdapter_DeleteWIFProvider_Success(t *testing.T) { + fakeClient := gcf.NewFakeGCFClient() + prov := gcf.NewProvisioner(gcf.Config{ + ProjectID: "test-project", + GitHubOrgs: []string{"acme"}, + Repo: "acme/widget", + WIFPoolName: "fullsend-pool", + }, fakeClient) + adapter := &gcfProvisionerAdapter{provisioner: prov} + + err := adapter.DeleteWIFProvider(context.Background(), "acme/widget") + require.NoError(t, err) +} + func TestGCFWIFAdapter_ProvisionWIF_Success(t *testing.T) { fakeClient := gcf.NewFakeGCFClient() prov := gcf.NewProvisioner(gcf.Config{ diff --git a/internal/cli/repos.go b/internal/cli/repos.go index a332ccb7b6..50bce64bf8 100644 --- a/internal/cli/repos.go +++ b/internal/cli/repos.go @@ -1,6 +1,7 @@ package cli import ( + "bufio" "context" "encoding/json" "errors" @@ -16,6 +17,7 @@ import ( "github.com/fullsend-ai/fullsend/internal/repos" "github.com/fullsend-ai/fullsend/internal/ui" "github.com/spf13/cobra" + "golang.org/x/term" ) func newReposCmd() *cobra.Command { @@ -28,7 +30,10 @@ The repos subcommand group provides bulk operations for platform administrators managing fullsend across many repositories and organizations.`, } cmd.AddCommand(newReposInitCmd()) + cmd.AddCommand(newReposAddCmd()) + cmd.AddCommand(newReposRemoveCmd()) cmd.AddCommand(newReposInstallCmd()) + cmd.AddCommand(newReposUninstallCmd()) cmd.AddCommand(newReposStatusCmd()) return cmd } @@ -318,22 +323,27 @@ func newReposInstallCmd() *cobra.Command { opts := &reposInstallConfig{} cmd := &cobra.Command{ - Use: "install", + Use: "install [repos...]", Short: "Install fullsend on repos defined in a manifest", Long: `Install fullsend on repos not yet installed, as defined in a repos.yaml manifest. +When repos are specified as positional arguments, only those repos are installed. +Glob patterns (e.g. "acme/*") are matched against manifest entries. +When no repos are specified, all manifest repos are installed. + Runs in three phases: 1. Parallel discovery: check which repos are already installed 2. Sequential WIF: provision WIF infrastructure per repo (not concurrent-safe) 3. Parallel scaffold: commit scaffold files and write variables/secrets`, + Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { + opts.repoFilter = args return runReposInstall(cmd.Context(), opts) }, } cmd.Flags().StringVarP(&opts.manifest, "manifest", "f", "repos.yaml", "path or URL to repos.yaml manifest") cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "preview what would be installed without making changes") - cmd.Flags().StringArrayVar(&opts.repoFilter, "repo", nil, "install specific repos only (repeatable)") cmd.Flags().BoolVar(&opts.skipMintCheck, "skip-mint-check", false, "skip mint URL discovery and org registration (EnsureOrgInMint)") cmd.Flags().IntVar(&opts.concurrency, "concurrency", 4, "max parallel operations (1-32)") cmd.Flags().StringSliceVar(&opts.roles, "roles", config.PerRepoDefaultRoles(), "agent roles to install") @@ -462,6 +472,505 @@ func runReposInstall(ctx context.Context, opts *reposInstallConfig) error { return nil } +// reposAddConfig holds flag values for repos add. +type reposAddConfig struct { + manifest string + dryRun bool + install bool + concurrency int + direct bool + roles []string + + testClient forge.Client + testProvisioner repos.WIFProvisioner +} + +func newReposAddCmd() *cobra.Command { + opts := &reposAddConfig{} + + cmd := &cobra.Command{ + Use: "add ", + Short: "Add repo entries to a repos.yaml manifest", + Long: `Add one or more repo entries to the repos.yaml manifest file, editing it +in place. + +Use --install to also install fullsend on the added repos after updating +the manifest.`, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runReposAdd(cmd.Context(), opts, args) + }, + } + + cmd.Flags().StringVarP(&opts.manifest, "manifest", "f", "repos.yaml", "path to repos.yaml manifest") + cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "preview what would be added without making changes") + cmd.Flags().BoolVar(&opts.install, "install", false, "also install fullsend on the added repos") + cmd.Flags().IntVar(&opts.concurrency, "concurrency", 4, "max parallel operations (1-32)") + cmd.Flags().BoolVar(&opts.direct, "direct", false, "push scaffold directly to default branch (skip PR)") + cmd.Flags().StringSliceVar(&opts.roles, "roles", config.PerRepoDefaultRoles(), "agent roles to install (used with --install)") + + return cmd +} + +func runReposAdd(ctx context.Context, opts *reposAddConfig, repoArgs []string) error { + if opts.install && (opts.concurrency < 1 || opts.concurrency > 32) { + return fmt.Errorf("--concurrency must be between 1 and 32, got %d", opts.concurrency) + } + + printer := ui.New(os.Stdout) + + printer.StepStart("Loading manifest") + manifest, err := repos.LoadManifest(ctx, opts.manifest) + if err != nil { + return fmt.Errorf("loading manifest: %w", err) + } + if err := manifest.Validate(); err != nil { + return fmt.Errorf("manifest validation failed: %w", err) + } + printer.StepDone(fmt.Sprintf("Loaded manifest with %d repo entries", len(manifest.Repos))) + + var client forge.Client + if opts.testClient != nil { + client = opts.testClient + } else { + token, tokenErr := resolveToken() + if tokenErr != nil { + return tokenErr + } + client = newGitHubLiveClient(token) + } + + entries := make([]repos.RepoEntry, len(repoArgs)) + for i, r := range repoArgs { + entries[i] = repos.RepoEntry{Repo: r} + } + + progressFn := func(repo, phase, msg string) { + switch phase { + case "done", "manifest": + printer.StepDone(fmt.Sprintf("[%s] %s", repo, msg)) + default: + printer.StepInfo(fmt.Sprintf("[%s] %s", repo, msg)) + } + } + + result, _, err := repos.AddToManifest(ctx, repos.ManifestEditConfig{ + Manifest: manifest, + ManifestPath: opts.manifest, + DryRun: opts.dryRun, + }, entries, client, progressFn) + if err != nil { + return err + } + + printer.Blank() + printer.StepDone(fmt.Sprintf("Add complete: %d added, %d skipped", len(result.Added), len(result.Skipped))) + + if opts.install && len(result.Added) > 0 && !opts.dryRun { + printer.Blank() + printer.StepStart("Installing fullsend on added repos") + installOpts := &reposInstallConfig{ + manifest: opts.manifest, + repoFilter: result.Added, + concurrency: opts.concurrency, + direct: opts.direct, + roles: opts.roles, + testClient: opts.testClient, + testProvisioner: opts.testProvisioner, + } + if err := runReposInstall(ctx, installOpts); err != nil { + return err + } + } + + return nil +} + +// reposRemoveConfig holds flag values for repos remove. +type reposRemoveConfig struct { + manifest string + dryRun bool + uninstall bool + yes bool + skipWIFCleanup bool + concurrency int + + testClient forge.Client + testProvisioner repos.WIFProvisioner +} + +func newReposRemoveCmd() *cobra.Command { + opts := &reposRemoveConfig{} + + cmd := &cobra.Command{ + Use: "remove ", + Short: "Remove repo entries from a repos.yaml manifest", + Long: `Remove one or more repo entries from the repos.yaml manifest file, +editing it in place. + +Glob patterns (e.g. "acme/*") are matched against manifest entries and +prompt for confirmation unless --yes is set. + +Use --uninstall to tear down fullsend from the repos before removing them +from the manifest (deletes workflow, variables, secrets, and WIF).`, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runReposRemove(cmd.Context(), opts, args) + }, + } + + cmd.Flags().StringVarP(&opts.manifest, "manifest", "f", "repos.yaml", "path to repos.yaml manifest") + cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "preview what would be removed without making changes") + cmd.Flags().BoolVar(&opts.uninstall, "uninstall", false, "tear down fullsend from repos before removing from manifest") + cmd.Flags().BoolVar(&opts.yes, "yes", false, "skip confirmation prompt when multiple repos are targeted") + cmd.Flags().BoolVar(&opts.skipWIFCleanup, "skip-wif-cleanup", false, "skip GCP WIF provider deletion (only with --uninstall)") + cmd.Flags().IntVar(&opts.concurrency, "concurrency", 4, "max parallel operations (1-32)") + + return cmd +} + +func runReposRemove(ctx context.Context, opts *reposRemoveConfig, repoArgs []string) error { + if opts.uninstall && (opts.concurrency < 1 || opts.concurrency > 32) { + return fmt.Errorf("--concurrency must be between 1 and 32, got %d", opts.concurrency) + } + + printer := ui.New(os.Stdout) + var uninstallFailed int + + printer.StepStart("Loading manifest") + manifest, err := repos.LoadManifest(ctx, opts.manifest) + if err != nil { + return fmt.Errorf("loading manifest: %w", err) + } + if err := manifest.Validate(); err != nil { + return fmt.Errorf("manifest validation failed: %w", err) + } + printer.StepDone(fmt.Sprintf("Loaded manifest with %d repo entries", len(manifest.Repos))) + + if !opts.yes && !opts.dryRun { + action := "remove" + if opts.uninstall { + action = "remove and uninstall" + } + if err := confirmBulkAction(printer, action, repoArgs, manifest, os.Stdin); err != nil { + return err + } + } + + if opts.uninstall { + matched, matchErr := repos.MatchManifestRepos(manifest, repoArgs) + if matchErr != nil { + return matchErr + } + var concreteRepos []string + for _, r := range matched { + if strings.ContainsAny(r, "*?[") { + printer.StepInfo(fmt.Sprintf("[%s] Skipping glob manifest entry (use concrete repo names to uninstall)", r)) + continue + } + concreteRepos = append(concreteRepos, r) + } + if len(concreteRepos) > 0 { + printer.Blank() + if opts.dryRun { + printer.StepStart("Previewing uninstall for repos") + } else { + printer.StepStart("Uninstalling fullsend from repos before removing from manifest") + } + + var client forge.Client + if opts.testClient != nil { + client = opts.testClient + } else if !opts.dryRun { + token, tokenErr := resolveToken() + if tokenErr != nil { + return tokenErr + } + client = newGitHubLiveClient(token) + } + + uninstallCfg := repos.UninstallConfig{ + Manifest: manifest, + Repos: concreteRepos, + DryRun: opts.dryRun, + SkipWIFCleanup: opts.skipWIFCleanup, + MaxConcurrency: opts.concurrency, + } + provFactory := buildProvisionerFactory(opts.testProvisioner, opts.skipWIFCleanup) + progressFn := func(repo, phase, msg string) { + switch phase { + case "done", "wif": + printer.StepDone(fmt.Sprintf("[%s] %s", repo, msg)) + default: + printer.StepInfo(fmt.Sprintf("[%s] %s", repo, msg)) + } + } + results, uninstallErr := repos.Uninstall(ctx, uninstallCfg, client, provFactory, progressFn) + if uninstallErr != nil { + return uninstallErr + } + var succeededRepos []string + for _, r := range results { + if r.Success { + succeededRepos = append(succeededRepos, r.Owner+"/"+r.Repo) + } else { + uninstallFailed++ + printer.StepInfo(fmt.Sprintf(" FAILED: %s/%s — %v", r.Owner, r.Repo, r.Error)) + } + } + if uninstallFailed > 0 { + if len(succeededRepos) > 0 { + repoArgs = succeededRepos + } else { + return fmt.Errorf("%d repos failed to uninstall, manifest unchanged", uninstallFailed) + } + } + } + } + + progressFn := func(repo, phase, msg string) { + switch phase { + case "done", "manifest": + printer.StepDone(fmt.Sprintf("[%s] %s", repo, msg)) + default: + printer.StepInfo(fmt.Sprintf("[%s] %s", repo, msg)) + } + } + + result, _, err := repos.RemoveFromManifest(repos.ManifestEditConfig{ + Manifest: manifest, + ManifestPath: opts.manifest, + DryRun: opts.dryRun, + }, repoArgs, progressFn) + if err != nil { + return err + } + + printer.Blank() + printer.StepDone(fmt.Sprintf("Remove complete: %d removed, %d skipped", len(result.Removed), len(result.Skipped))) + + if opts.uninstall && uninstallFailed > 0 { + return fmt.Errorf("%d repos failed to uninstall (successfully uninstalled repos were removed from manifest)", uninstallFailed) + } + return nil +} + +// reposUninstallConfig holds flag values for repos uninstall. +type reposUninstallConfig struct { + manifest string + dryRun bool + yes bool + skipWIFCleanup bool + concurrency int + + testClient forge.Client + testProvisioner repos.WIFProvisioner +} + +func newReposUninstallCmd() *cobra.Command { + opts := &reposUninstallConfig{} + + cmd := &cobra.Command{ + Use: "uninstall ", + Short: "Tear down fullsend from specific repos", + Long: `Tear down fullsend from the specified repos by deleting workflow files, +variables, secrets, and WIF infrastructure. + +Does NOT modify repos.yaml — use "repos remove" for that. + +Glob patterns (e.g. "acme/*") are matched against manifest entries and +prompt for confirmation unless --yes is set.`, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runReposUninstall(cmd.Context(), opts, args) + }, + } + + cmd.Flags().StringVarP(&opts.manifest, "manifest", "f", "repos.yaml", "path or URL to repos.yaml manifest") + cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "preview what would be uninstalled without making changes") + cmd.Flags().BoolVar(&opts.yes, "yes", false, "skip confirmation prompt when multiple repos are targeted") + cmd.Flags().BoolVar(&opts.skipWIFCleanup, "skip-wif-cleanup", false, "skip GCP WIF provider deletion and mint deregistration") + cmd.Flags().IntVar(&opts.concurrency, "concurrency", 4, "max parallel operations (1-32)") + + return cmd +} + +func runReposUninstall(ctx context.Context, opts *reposUninstallConfig, repoArgs []string) error { + if opts.concurrency < 1 || opts.concurrency > 32 { + return fmt.Errorf("--concurrency must be between 1 and 32, got %d", opts.concurrency) + } + + printer := ui.New(os.Stdout) + + printer.StepStart("Loading manifest") + manifest, err := repos.LoadManifest(ctx, opts.manifest) + if err != nil { + return fmt.Errorf("loading manifest: %w", err) + } + if err := manifest.Validate(); err != nil { + return fmt.Errorf("manifest validation failed: %w", err) + } + printer.StepDone(fmt.Sprintf("Loaded manifest with %d repo entries", len(manifest.Repos))) + + matched, matchErr := repos.MatchManifestRepos(manifest, repoArgs) + if matchErr != nil { + return matchErr + } + if len(matched) == 0 { + printer.StepInfo("No manifest entries matched the given patterns") + return nil + } + + var concreteRepos []string + for _, r := range matched { + if strings.ContainsAny(r, "*?[") { + printer.StepInfo(fmt.Sprintf("[%s] Skipping glob manifest entry (use concrete repo names to uninstall)", r)) + continue + } + concreteRepos = append(concreteRepos, r) + } + if len(concreteRepos) == 0 { + printer.StepInfo("All matched entries are glob patterns — no concrete repos to uninstall") + return nil + } + + if !opts.yes && !opts.dryRun { + if err := confirmBulkAction(printer, "uninstall", repoArgs, manifest, os.Stdin); err != nil { + return err + } + } + + var client forge.Client + if opts.testClient != nil { + client = opts.testClient + } else { + token, tokenErr := resolveToken() + if tokenErr != nil { + return tokenErr + } + client = newGitHubLiveClient(token) + } + + provFactory := buildProvisionerFactory(opts.testProvisioner, opts.skipWIFCleanup) + + cfg := repos.UninstallConfig{ + Manifest: manifest, + Repos: concreteRepos, + DryRun: opts.dryRun, + SkipWIFCleanup: opts.skipWIFCleanup, + MaxConcurrency: opts.concurrency, + } + + progressFn := func(repo, phase, msg string) { + switch phase { + case "done", "wif": + printer.StepDone(fmt.Sprintf("[%s] %s", repo, msg)) + default: + printer.StepInfo(fmt.Sprintf("[%s] %s", repo, msg)) + } + } + + printer.Blank() + if opts.dryRun { + printer.StepStart("Dry-run: previewing uninstall") + } else { + printer.StepStart("Uninstalling fullsend from repos") + } + + results, err := repos.Uninstall(ctx, cfg, client, provFactory, progressFn) + if err != nil { + return err + } + + var succeeded, failed int + for _, r := range results { + if r.Success { + succeeded++ + } else { + failed++ + } + } + + printer.Blank() + printer.StepDone(fmt.Sprintf("Uninstall complete: %d uninstalled, %d failed", succeeded, failed)) + + for _, r := range results { + if !r.Success { + printer.StepInfo(fmt.Sprintf(" FAILED: %s/%s — %v", r.Owner, r.Repo, r.Error)) + } + } + + if failed > 0 { + return fmt.Errorf("%d repos failed to uninstall", failed) + } + return nil +} + +// confirmBulkAction prompts for confirmation when a destructive action targets +// multiple repos — either through glob expansion or an explicit bulk list. +func confirmBulkAction(printer *ui.Printer, action string, patterns []string, manifest *repos.Manifest, stdin *os.File) error { + matched, err := repos.MatchManifestRepos(manifest, patterns) + if err != nil { + return err + } + if len(matched) <= 1 { + return nil + } + + if !term.IsTerminal(int(stdin.Fd())) { + return fmt.Errorf("stdin is not a terminal; use --yes to skip confirmation") + } + + printer.StepWarn(fmt.Sprintf("This will %s %d repos:", action, len(matched))) + for _, r := range matched { + printer.StepInfo(" " + r) + } + printer.StepInfo("Continue? [y/N]") + + reader := bufio.NewReader(stdin) + line, err := reader.ReadString('\n') + if err != nil { + return fmt.Errorf("reading confirmation: %w", err) + } + if strings.TrimSpace(strings.ToLower(line)) != "y" { + return fmt.Errorf("aborted") + } + return nil +} + +// buildProvisionerFactory creates a ProvisionerFactory for uninstall operations. +// When skipWIF is true or testProv is non-nil, shortcuts are used. +func buildProvisionerFactory(testProv repos.WIFProvisioner, skipWIF bool) repos.ProvisionerFactory { + if skipWIF { + return nil + } + return func(resolved repos.ResolvedConfig) repos.WIFProvisioner { + if testProv != nil { + return testProv + } + mintProv := &gcfProvisionerAdapter{ + provisioner: gcf.NewProvisioner(gcf.Config{ + ProjectID: resolved.MintProject, + Region: resolved.MintRegion, + GitHubOrgs: []string{resolved.Owner}, + Repo: resolved.Owner + "/" + resolved.Repo, + WIFPoolName: gcf.DefaultInferencePool, + MintURL: resolved.MintURL, + }, gcf.NewLiveGCFClient(resolved.MintProject)), + } + wifProv := &gcfProvisionerAdapter{ + provisioner: gcf.NewProvisioner(gcf.Config{ + ProjectID: resolved.InferenceProject, + Region: resolved.InferenceRegion, + GitHubOrgs: []string{resolved.Owner}, + Repo: resolved.Owner + "/" + resolved.Repo, + WIFPoolName: gcf.DefaultInferencePool, + }, gcf.NewLiveGCFClient(resolved.InferenceProject)), + } + return &splitProjectAdapter{mint: mintProv, inference: wifProv} + } +} + // splitProjectAdapter routes WIFProvisioner methods to the correct GCP project: // ProvisionWIF targets the inference project (IAM resources) while mint // operations target the mint project (Cloud Function env vars). @@ -487,5 +996,15 @@ func (s *splitProjectAdapter) EnsureOrgInMint(ctx context.Context, expectedURL s } func (s *splitProjectAdapter) DeletePerRepoWIF(ctx context.Context, repo string) error { - return s.mint.DeletePerRepoWIF(ctx, repo) + if err := s.mint.DeletePerRepoWIF(ctx, repo); err != nil { + return fmt.Errorf("deregistering from mint: %w", err) + } + if err := s.inference.DeleteWIFProvider(ctx, repo); err != nil { + return fmt.Errorf("deleting WIF provider for %s (mint deregistration already succeeded — re-run is safe): %w", repo, err) + } + return nil +} + +func (s *splitProjectAdapter) DeleteWIFProvider(ctx context.Context, repo string) error { + return s.inference.DeleteWIFProvider(ctx, repo) } diff --git a/internal/cli/repos_test.go b/internal/cli/repos_test.go index b9fee7830c..c82ba2e158 100644 --- a/internal/cli/repos_test.go +++ b/internal/cli/repos_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "context" + "fmt" "os" "path/filepath" "strings" @@ -10,6 +11,7 @@ import ( "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/repos" + "github.com/fullsend-ai/fullsend/internal/ui" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,7 +23,10 @@ func TestReposCommand_HasSubcommands(t *testing.T) { names[sub.Name()] = true } assert.True(t, names["init"], "expected init subcommand") + assert.True(t, names["add"], "expected add subcommand") + assert.True(t, names["remove"], "expected remove subcommand") assert.True(t, names["install"], "expected install subcommand") + assert.True(t, names["uninstall"], "expected uninstall subcommand") assert.True(t, names["status"], "expected status subcommand") } @@ -488,6 +493,11 @@ func (p *trackingProvisioner) DeletePerRepoWIF(_ context.Context, _ string) erro return nil } +func (p *trackingProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + p.calls = append(p.calls, "DeleteWIFProvider") + return nil +} + func TestSplitProjectAdapter_MethodRouting(t *testing.T) { mint := &trackingProvisioner{label: "mint"} inference := &trackingProvisioner{label: "inference"} @@ -504,10 +514,11 @@ func TestSplitProjectAdapter_MethodRouting(t *testing.T) { require.NoError(t, adapter.RegisterPerRepoWIF(ctx, "o/r")) require.NoError(t, adapter.EnsureOrgInMint(ctx, "url", "org")) + require.NoError(t, adapter.DeletePerRepoWIF(ctx, "o/r")) assert.Equal(t, []string{"DiscoverMint", "RegisterPerRepoWIF", "EnsureOrgInMint", "DeletePerRepoWIF"}, mint.calls) - assert.Equal(t, []string{"ProvisionWIF"}, inference.calls) + assert.Equal(t, []string{"ProvisionWIF", "DeleteWIFProvider"}, inference.calls) } func writeTestManifest(t *testing.T, content string) string { @@ -641,3 +652,588 @@ repos: require.Error(t, err) assert.Contains(t, err.Error(), "failed to install") } + +// --- repos add --- + +func TestReposAddCmd_Flags(t *testing.T) { + cmd := newReposAddCmd() + + manifestFlag := cmd.Flags().Lookup("manifest") + require.NotNil(t, manifestFlag) + assert.Equal(t, "repos.yaml", manifestFlag.DefValue) + + dryRunFlag := cmd.Flags().Lookup("dry-run") + require.NotNil(t, dryRunFlag) + + installFlag := cmd.Flags().Lookup("install") + require.NotNil(t, installFlag) + + shorthand := cmd.Flags().ShorthandLookup("f") + require.NotNil(t, shorthand, "expected -f shorthand for --manifest") +} + +func TestReposAddCmd_RequiresArgs(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"repos", "add"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "requires at least 1 arg") +} + +func TestRunReposAdd_Basic(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := forge.NewFakeClient() + + err := runReposAdd(context.Background(), &reposAddConfig{ + manifest: manifestPath, + testClient: fc, + }, []string{"acme/web"}) + require.NoError(t, err) + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 2, len(m.Repos)) +} + +func TestRunReposAdd_Duplicate(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := forge.NewFakeClient() + + err := runReposAdd(context.Background(), &reposAddConfig{ + manifest: manifestPath, + testClient: fc, + }, []string{"acme/api"}) + require.NoError(t, err) + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 1, len(m.Repos)) +} + +func TestRunReposAdd_DryRun(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := forge.NewFakeClient() + + err := runReposAdd(context.Background(), &reposAddConfig{ + manifest: manifestPath, + testClient: fc, + dryRun: true, + }, []string{"acme/web"}) + require.NoError(t, err) + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 1, len(m.Repos), "dry-run should not modify manifest") +} + +func TestRunReposAdd_InvalidManifest(t *testing.T) { + err := runReposAdd(context.Background(), &reposAddConfig{ + manifest: "/nonexistent/repos.yaml", + }, []string{"acme/web"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "loading manifest") +} + +// --- repos remove --- + +func TestReposRemoveCmd_Flags(t *testing.T) { + cmd := newReposRemoveCmd() + + manifestFlag := cmd.Flags().Lookup("manifest") + require.NotNil(t, manifestFlag) + + dryRunFlag := cmd.Flags().Lookup("dry-run") + require.NotNil(t, dryRunFlag) + + uninstallFlag := cmd.Flags().Lookup("uninstall") + require.NotNil(t, uninstallFlag) + + yesFlag := cmd.Flags().Lookup("yes") + require.NotNil(t, yesFlag) + + skipWIFFlag := cmd.Flags().Lookup("skip-wif-cleanup") + require.NotNil(t, skipWIFFlag) +} + +func TestReposRemoveCmd_RequiresArgs(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"repos", "remove"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "requires at least 1 arg") +} + +func TestRunReposRemove_Basic(t *testing.T) { + yaml := `version: 1 +mint: + url: https://mint.example.com + project: mint-proj + region: us-central1 +defaults: + inference_project: inf-proj + inference_region: us-central1 +repos: + - repo: acme/api + - repo: acme/web +` + manifestPath := writeTestManifest(t, yaml) + + err := runReposRemove(context.Background(), &reposRemoveConfig{ + manifest: manifestPath, + yes: true, + }, []string{"acme/api"}) + require.NoError(t, err) + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 1, len(m.Repos)) + assert.Equal(t, "acme/web", m.Repos[0].Repo) +} + +func TestRunReposRemove_NotFound(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + + err := runReposRemove(context.Background(), &reposRemoveConfig{ + manifest: manifestPath, + yes: true, + }, []string{"acme/nonexistent"}) + require.NoError(t, err) +} + +func TestRunReposRemove_DryRun(t *testing.T) { + yaml := `version: 1 +mint: + url: https://mint.example.com + project: mint-proj + region: us-central1 +defaults: + inference_project: inf-proj + inference_region: us-central1 +repos: + - repo: acme/api + - repo: acme/web +` + manifestPath := writeTestManifest(t, yaml) + + err := runReposRemove(context.Background(), &reposRemoveConfig{ + manifest: manifestPath, + dryRun: true, + }, []string{"acme/api"}) + require.NoError(t, err) + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 2, len(m.Repos), "dry-run should not modify manifest") +} + +func TestRunReposRemove_InvalidManifest(t *testing.T) { + err := runReposRemove(context.Background(), &reposRemoveConfig{ + manifest: "/nonexistent/repos.yaml", + yes: true, + }, []string{"acme/api"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "loading manifest") +} + +// --- repos uninstall --- + +func TestReposUninstallCmd_Flags(t *testing.T) { + cmd := newReposUninstallCmd() + + manifestFlag := cmd.Flags().Lookup("manifest") + require.NotNil(t, manifestFlag) + + dryRunFlag := cmd.Flags().Lookup("dry-run") + require.NotNil(t, dryRunFlag) + + yesFlag := cmd.Flags().Lookup("yes") + require.NotNil(t, yesFlag) + + skipWIFFlag := cmd.Flags().Lookup("skip-wif-cleanup") + require.NotNil(t, skipWIFFlag) + + concurrencyFlag := cmd.Flags().Lookup("concurrency") + require.NotNil(t, concurrencyFlag) +} + +func TestReposUninstallCmd_RequiresArgs(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"repos", "uninstall"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "requires at least 1 arg") +} + +func newInstalledFakeClientCLI(repoNames ...string) *forge.FakeClient { + fc := forge.NewFakeClient() + fc.InstallationToken = true + for _, r := range repoNames { + parts := strings.SplitN(r, "/", 2) + fc.Repos = append(fc.Repos, forge.Repository{ + FullName: r, + Name: parts[1], + DefaultBranch: "main", + }) + fc.VariableValues[r+"/FULLSEND_PER_REPO_INSTALL"] = "true" + fc.VariableValues[r+"/FULLSEND_MINT_URL"] = "https://mint.example.com" + fc.VariableValues[r+"/FULLSEND_GCP_REGION"] = "us-central1" + fc.Secrets[r+"/FULLSEND_GCP_PROJECT_ID"] = true + fc.Secrets[r+"/FULLSEND_GCP_WIF_PROVIDER"] = true + fc.FileContents[r+"/.github/workflows/fullsend.yml"] = []byte("uses: fullsend-ai/fullsend/.github/workflows/dispatch.yml@v1") + } + return fc +} + +func TestRunReposUninstall_DryRun(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstalledFakeClientCLI("acme/api") + prov := &trackingProvisioner{label: "test"} + + err := runReposUninstall(context.Background(), &reposUninstallConfig{ + manifest: manifestPath, + dryRun: true, + yes: true, + concurrency: 4, + testClient: fc, + testProvisioner: prov, + }, []string{"acme/api"}) + require.NoError(t, err) + assert.Empty(t, prov.calls, "dry-run should not call provisioner") +} + +func TestRunReposUninstall_Success(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstalledFakeClientCLI("acme/api") + prov := &trackingProvisioner{label: "test"} + + err := runReposUninstall(context.Background(), &reposUninstallConfig{ + manifest: manifestPath, + yes: true, + concurrency: 4, + testClient: fc, + testProvisioner: prov, + }, []string{"acme/api"}) + require.NoError(t, err) + assert.Contains(t, prov.calls, "DeletePerRepoWIF") +} + +func TestRunReposUninstall_NoMatch(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstalledFakeClientCLI("acme/api") + + err := runReposUninstall(context.Background(), &reposUninstallConfig{ + manifest: manifestPath, + yes: true, + concurrency: 4, + testClient: fc, + }, []string{"other/nonexistent"}) + require.NoError(t, err) +} + +func TestRunReposUninstall_ConcurrencyValidation(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstalledFakeClientCLI("acme/api") + + err := runReposUninstall(context.Background(), &reposUninstallConfig{ + manifest: manifestPath, + yes: true, + concurrency: 0, + testClient: fc, + }, []string{"acme/api"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "--concurrency must be between 1 and 32") +} + +func TestRunReposUninstall_InvalidManifest(t *testing.T) { + err := runReposUninstall(context.Background(), &reposUninstallConfig{ + manifest: "/nonexistent/repos.yaml", + yes: true, + concurrency: 4, + }, []string{"acme/api"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "loading manifest") +} + +func TestRunReposUninstall_SkipWIF(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstalledFakeClientCLI("acme/api") + prov := &trackingProvisioner{label: "test"} + + err := runReposUninstall(context.Background(), &reposUninstallConfig{ + manifest: manifestPath, + yes: true, + skipWIFCleanup: true, + concurrency: 4, + testClient: fc, + testProvisioner: prov, + }, []string{"acme/api"}) + require.NoError(t, err) + assert.NotContains(t, prov.calls, "DeletePerRepoWIF") +} + +// --- repos install positional args --- + +func TestReposInstallCmd_PositionalArgs(t *testing.T) { + cmd := newReposInstallCmd() + + repoFlag := cmd.Flags().Lookup("repo") + assert.Nil(t, repoFlag, "--repo flag should be removed, use positional args") +} + +func TestRunReposInstall_WithFilter(t *testing.T) { + yaml := `version: 1 +mint: + url: https://mint.example.com + project: mint-proj + region: us-central1 +defaults: + inference_project: inf-proj + inference_region: us-central1 + fullsend_ref: v1.0.0 +repos: + - repo: acme/api + - repo: acme/web +` + manifestPath := writeTestManifest(t, yaml) + fc := newInstallFakeClient("acme/api", "acme/web") + prov := &trackingProvisioner{label: "test"} + + err := runReposInstall(context.Background(), &reposInstallConfig{ + manifest: manifestPath, + concurrency: 4, + repoFilter: []string{"acme/api"}, + roles: []string{"triage"}, + direct: true, + testClient: fc, + testProvisioner: prov, + }) + require.NoError(t, err) +} + +// --- repos remove with uninstall --- + +func TestRunReposRemove_WithUninstall(t *testing.T) { + yaml := `version: 1 +mint: + url: https://mint.example.com + project: mint-proj + region: us-central1 +defaults: + inference_project: inf-proj + inference_region: us-central1 +repos: + - repo: acme/api + - repo: acme/web +` + manifestPath := writeTestManifest(t, yaml) + fc := newInstalledFakeClientCLI("acme/api", "acme/web") + prov := &trackingProvisioner{label: "test"} + + err := runReposRemove(context.Background(), &reposRemoveConfig{ + manifest: manifestPath, + uninstall: true, + yes: true, + concurrency: 4, + testClient: fc, + testProvisioner: prov, + }, []string{"acme/api"}) + require.NoError(t, err) + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 1, len(m.Repos), "acme/api should be removed from manifest") + assert.Equal(t, "acme/web", m.Repos[0].Repo) + assert.Contains(t, prov.calls, "DeletePerRepoWIF") +} + +func TestRunReposRemove_WithUninstall_PartialFailure(t *testing.T) { + yaml := `version: 1 +mint: + url: https://mint.example.com + project: mint-proj + region: us-central1 +defaults: + inference_project: inf-proj + inference_region: us-central1 +repos: + - repo: acme/api + - repo: acme/web +` + manifestPath := writeTestManifest(t, yaml) + fc := newInstalledFakeClientCLI("acme/api", "acme/web") + fc.DeleteFilesErrors = map[string]error{ + "acme/web": fmt.Errorf("permission denied"), + } + prov := &trackingProvisioner{label: "test"} + + err := runReposRemove(context.Background(), &reposRemoveConfig{ + manifest: manifestPath, + uninstall: true, + yes: true, + concurrency: 1, + testClient: fc, + testProvisioner: prov, + }, []string{"acme/api", "acme/web"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "1 repos failed to uninstall") + assert.Contains(t, err.Error(), "successfully uninstalled repos were removed from manifest") + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 1, len(m.Repos), "only failed repo should remain") + assert.Equal(t, "acme/web", m.Repos[0].Repo) +} + +func TestRunReposRemove_ConcurrencyValidation(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + + err := runReposRemove(context.Background(), &reposRemoveConfig{ + manifest: manifestPath, + uninstall: true, + concurrency: 0, + yes: true, + }, []string{"acme/api"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "--concurrency must be between 1 and 32") +} + +// --- repos install flag tests --- + +func TestReposInstallCmd_Flags(t *testing.T) { + cmd := newReposInstallCmd() + + manifestFlag := cmd.Flags().Lookup("manifest") + require.NotNil(t, manifestFlag) + assert.Equal(t, "repos.yaml", manifestFlag.DefValue) + + dryRunFlag := cmd.Flags().Lookup("dry-run") + require.NotNil(t, dryRunFlag) + + concurrencyFlag := cmd.Flags().Lookup("concurrency") + require.NotNil(t, concurrencyFlag) + assert.Equal(t, "4", concurrencyFlag.DefValue) + + directFlag := cmd.Flags().Lookup("direct") + require.NotNil(t, directFlag) + + shorthand := cmd.Flags().ShorthandLookup("f") + require.NotNil(t, shorthand, "expected -f shorthand for --manifest") +} + +func TestReposAddCmd_ManifestShortFlag(t *testing.T) { + cmd := newReposAddCmd() + shorthand := cmd.Flags().ShorthandLookup("f") + require.NotNil(t, shorthand, "expected -f shorthand for --manifest") +} + +func TestReposRemoveCmd_ManifestShortFlag(t *testing.T) { + cmd := newReposRemoveCmd() + shorthand := cmd.Flags().ShorthandLookup("f") + require.NotNil(t, shorthand, "expected -f shorthand for --manifest") +} + +func TestReposUninstallCmd_ManifestShortFlag(t *testing.T) { + cmd := newReposUninstallCmd() + shorthand := cmd.Flags().ShorthandLookup("f") + require.NotNil(t, shorthand, "expected -f shorthand for --manifest") +} + +// --- confirmBulkAction --- + +func TestConfirmBulkAction_SingleRepo(t *testing.T) { + manifest := &repos.Manifest{ + Repos: []repos.RepoEntry{{Repo: "acme/api"}}, + } + err := confirmBulkAction(nil, "remove", []string{"acme/api"}, manifest, nil) + require.NoError(t, err) +} + +func TestConfirmBulkAction_GlobNoMatch(t *testing.T) { + manifest := &repos.Manifest{ + Repos: []repos.RepoEntry{{Repo: "other/repo"}}, + } + printer := ui.New(os.Stdout) + err := confirmBulkAction(printer, "remove", []string{"acme/*"}, manifest, nil) + require.NoError(t, err) +} + +func TestConfirmBulkAction_GlobMultiMatch(t *testing.T) { + manifest := &repos.Manifest{ + Repos: []repos.RepoEntry{{Repo: "acme/api"}, {Repo: "acme/web"}}, + } + printer := ui.New(os.Stdout) + + r, w, _ := os.Pipe() + w.Close() + + err := confirmBulkAction(printer, "remove", []string{"acme/*"}, manifest, r) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a terminal") +} + +func TestConfirmBulkAction_ExplicitBulkList(t *testing.T) { + manifest := &repos.Manifest{ + Repos: []repos.RepoEntry{{Repo: "acme/api"}, {Repo: "acme/web"}}, + } + printer := ui.New(os.Stdout) + + r, w, _ := os.Pipe() + w.Close() + + err := confirmBulkAction(printer, "remove", []string{"acme/api", "acme/web"}, manifest, r) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a terminal") +} + +func TestConfirmBulkAction_GlobSingleMatch(t *testing.T) { + manifest := &repos.Manifest{ + Repos: []repos.RepoEntry{{Repo: "acme/api"}}, + } + err := confirmBulkAction(nil, "remove", []string{"acme/*"}, manifest, nil) + require.NoError(t, err) +} + +// --- repos add with install --- + +func TestRunReposAdd_WithInstall(t *testing.T) { + manifestPath := writeTestManifest(t, testManifestYAML) + fc := newInstallFakeClient("acme/api", "acme/web") + prov := &trackingProvisioner{label: "test"} + + err := runReposAdd(context.Background(), &reposAddConfig{ + manifest: manifestPath, + install: true, + concurrency: 4, + direct: true, + testClient: fc, + testProvisioner: prov, + }, []string{"acme/web"}) + require.NoError(t, err) + + m, loadErr := repos.LoadManifest(context.Background(), manifestPath) + require.NoError(t, loadErr) + assert.Equal(t, 2, len(m.Repos)) +} + +// --- buildProvisionerFactory --- + +func TestBuildProvisionerFactory_SkipWIF(t *testing.T) { + factory := buildProvisionerFactory(nil, true) + assert.Nil(t, factory) +} + +func TestBuildProvisionerFactory_WithTestProv(t *testing.T) { + prov := &trackingProvisioner{label: "test"} + factory := buildProvisionerFactory(prov, false) + require.NotNil(t, factory) + + result := factory(repos.ResolvedConfig{ + Owner: "acme", + Repo: "api", + MintProject: "mint-proj", + MintRegion: "us-central1", + InferenceProject: "inf-proj", + InferenceRegion: "us-central1", + }) + assert.Equal(t, prov, result) +} diff --git a/internal/forge/fake.go b/internal/forge/fake.go index ef328c3b60..608d4a52be 100644 --- a/internal/forge/fake.go +++ b/internal/forge/fake.go @@ -190,6 +190,10 @@ type FakeClient struct { // Key is "owner/repo", checked before the generic Errors map. CreateBranchErrors map[string]error + // DeleteFilesErrors injects per-repo errors for DeleteFiles. + // Key is "owner/repo", checked before the generic Errors map. + DeleteFilesErrors map[string]error + // Issue comments for ListIssueComments / UpdateIssueComment. IssueComments map[string][]IssueComment // key: "owner/repo/number" OpenIssues map[string][]Issue // key: "owner/repo" @@ -555,6 +559,9 @@ func (f *FakeClient) DeleteFiles(_ context.Context, owner, repo, message string, f.mu.Lock() defer f.mu.Unlock() + if e, ok := f.DeleteFilesErrors[owner+"/"+repo]; ok { + return 0, e + } if e := f.err("DeleteFiles"); e != nil { return 0, e } diff --git a/internal/repos/batch_install.go b/internal/repos/batch_install.go index 613d443ed9..4386196184 100644 --- a/internal/repos/batch_install.go +++ b/internal/repos/batch_install.go @@ -78,7 +78,11 @@ func BatchInstall(ctx context.Context, cfg BatchInstallConfig, } if len(cfg.RepoFilter) > 0 { - repos = filterRepos(repos, cfg.RepoFilter) + var filterErr error + repos, filterErr = filterRepos(repos, cfg.RepoFilter) + if filterErr != nil { + return nil, filterErr + } } if len(repos) == 0 { return &BatchInstallResult{}, nil diff --git a/internal/repos/batch_install_test.go b/internal/repos/batch_install_test.go index eb92d2f28a..ed9086562e 100644 --- a/internal/repos/batch_install_test.go +++ b/internal/repos/batch_install_test.go @@ -77,6 +77,10 @@ func (f *batchFakeProvisioner) DeletePerRepoWIF(_ context.Context, repo string) return f.deleteErr } +func (f *batchFakeProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} + // perRepoProvisioner tracks calls per repo and supports per-repo error injection. type perRepoProvisioner struct { mu sync.Mutex @@ -139,6 +143,10 @@ func (p *perRepoProvisioner) DeletePerRepoWIF(_ context.Context, _ string) error return nil } +func (p *perRepoProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} + func newBatchManifest(repos ...string) *Manifest { entries := make([]RepoEntry, len(repos)) for i, r := range repos { @@ -1252,6 +1260,10 @@ func (c *cancellingOrgMintProvisioner) DeletePerRepoWIF(ctx context.Context, rep return c.inner.DeletePerRepoWIF(ctx, repo) } +func (c *cancellingOrgMintProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} + // cancellingProvisioner cancels a context after N ProvisionWIF calls. type cancellingProvisioner struct { inner *batchFakeProvisioner @@ -1284,6 +1296,10 @@ func (c *cancellingProvisioner) DeletePerRepoWIF(ctx context.Context, repo strin return c.inner.DeletePerRepoWIF(ctx, repo) } +func (c *cancellingProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} + // trackingOrgProvisioner delegates to an inner provisioner but tracks org calls. type trackingOrgProvisioner struct { inner *batchFakeProvisioner @@ -1313,3 +1329,7 @@ func (t *trackingOrgProvisioner) EnsureOrgInMint(ctx context.Context, url string func (t *trackingOrgProvisioner) DeletePerRepoWIF(ctx context.Context, repo string) error { return t.inner.DeletePerRepoWIF(ctx, repo) } + +func (t *trackingOrgProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} diff --git a/internal/repos/init.go b/internal/repos/init.go index 9a376411d0..17659367bf 100644 --- a/internal/repos/init.go +++ b/internal/repos/init.go @@ -325,28 +325,23 @@ func discoverRepo(ctx context.Context, client forge.Client, fullName := owner + "/" + repo progress(fullName, "discover", "reading variables") - vars, err := client.ListRepoVariables(ctx, owner, repo) - if err != nil { - return DiscoveredRepo{}, fmt.Errorf("listing variables for %s: %w", fullName, err) + state, err := ProbeRepoState(ctx, client, owner, repo) + if err != nil && !state.Installed { + return DiscoveredRepo{}, err } - // Check for per-repo installation. - if vars[forge.PerRepoGuardVar] == "true" { + if state.Installed { progress(fullName, "discover", "per-repo installation detected") - ref, err := readWorkflowRef(ctx, client, owner, repo) - if err != nil { - return DiscoveredRepo{}, err - } - if ref != "" { - progress(fullName, "discover", fmt.Sprintf("ref: %s", ref)) + if state.FullsendRef != "" { + progress(fullName, "discover", fmt.Sprintf("ref: %s", state.FullsendRef)) } return DiscoveredRepo{ Owner: owner, Repo: repo, Source: "per-repo", - MintURL: vars["FULLSEND_MINT_URL"], - InferenceRegion: vars["FULLSEND_GCP_REGION"], - FullsendRef: ref, + MintURL: state.MintURL, + InferenceRegion: state.InferenceRegion, + FullsendRef: state.FullsendRef, }, nil } diff --git a/internal/repos/install.go b/internal/repos/install.go index 6bfb409be4..eedbfd57e6 100644 --- a/internal/repos/install.go +++ b/internal/repos/install.go @@ -119,6 +119,10 @@ type WIFProvisioner interface { // DeletePerRepoWIF removes a repo from per-repo WIF registration. DeletePerRepoWIF(ctx context.Context, repo string) error + + // DeleteWIFProvider deletes the WIF provider for a repo from the + // GCP project (used during uninstall to clean up IAM resources). + DeleteWIFProvider(ctx context.Context, repo string) error } // ErrMintNotFound indicates the mint function does not exist. diff --git a/internal/repos/install_test.go b/internal/repos/install_test.go index 7a7b405680..9e3b0deeba 100644 --- a/internal/repos/install_test.go +++ b/internal/repos/install_test.go @@ -47,6 +47,10 @@ func (f *fakeWIFProvisioner) DeletePerRepoWIF(_ context.Context, _ string) error return nil } +func (f *fakeWIFProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} + // noopProgress is a no-op progress callback for tests. func noopProgress(_, _, _ string) {} diff --git a/internal/repos/manifest_edit.go b/internal/repos/manifest_edit.go new file mode 100644 index 0000000000..b28c9d2e53 --- /dev/null +++ b/internal/repos/manifest_edit.go @@ -0,0 +1,258 @@ +package repos + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +var repoNamePattern = regexp.MustCompile(`^[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+$`) + +// ManifestEditConfig holds inputs for manifest add/remove operations. +type ManifestEditConfig struct { + Manifest *Manifest + ManifestPath string + DryRun bool +} + +// ManifestAddResult holds the outcome of adding repos to a manifest. +type ManifestAddResult struct { + Added []string + Skipped []string +} + +// ManifestRemoveResult holds the outcome of removing repos from a manifest. +type ManifestRemoveResult struct { + Removed []string + Skipped []string +} + +// AddToManifest appends repo entries to the manifest, skipping duplicates. +// When client is non-nil, each non-glob repo is probed for existing +// installation state and per-repo overrides are populated where the +// discovered values differ from manifest defaults. +// Returns the result and the modified manifest. The manifest is written to +// disk only when ManifestPath is set and DryRun is false. +func AddToManifest(ctx context.Context, cfg ManifestEditConfig, entries []RepoEntry, client forge.Client, progress ProgressFunc) (*ManifestAddResult, *Manifest, error) { + if cfg.Manifest == nil { + return nil, nil, fmt.Errorf("manifest is required") + } + if len(entries) == 0 { + return nil, nil, fmt.Errorf("at least one repo is required") + } + if progress == nil { + progress = func(_, _, _ string) {} + } + + existing := make(map[string]bool, len(cfg.Manifest.Repos)) + for _, e := range cfg.Manifest.Repos { + existing[strings.ToLower(e.Repo)] = true + } + + for _, entry := range entries { + if !isGlob(entry.Repo) && !repoNamePattern.MatchString(entry.Repo) { + return nil, nil, fmt.Errorf("invalid repo name %q: expected owner/repo format", entry.Repo) + } + } + + if client != nil { + for i := range entries { + if isGlob(entries[i].Repo) || existing[strings.ToLower(entries[i].Repo)] { + continue + } + parts := strings.SplitN(entries[i].Repo, "/", 2) + if len(parts) != 2 { + continue + } + state, err := ProbeRepoState(ctx, client, parts[0], parts[1]) + if err != nil && !state.Installed { + progress(entries[i].Repo, "discover", fmt.Sprintf("probe failed: %v", err)) + continue + } + if !state.Installed { + continue + } + progress(entries[i].Repo, "discover", "existing installation detected") + if state.InferenceRegion != "" && state.InferenceRegion != cfg.Manifest.Defaults.InferenceRegion { + entries[i].InferenceRegion = NullableString{Set: true, Value: state.InferenceRegion} + } + if state.FullsendRef != "" && state.FullsendRef != cfg.Manifest.Defaults.FullsendRef { + entries[i].FullsendRef = NullableString{Set: true, Value: state.FullsendRef} + } + } + } + + result := &ManifestAddResult{} + var toAdd []RepoEntry + + for _, entry := range entries { + if existing[strings.ToLower(entry.Repo)] { + result.Skipped = append(result.Skipped, entry.Repo) + progress(entry.Repo, "manifest", "Already in manifest, skipping") + continue + } + result.Added = append(result.Added, entry.Repo) + toAdd = append(toAdd, entry) + existing[strings.ToLower(entry.Repo)] = true + } + + if len(toAdd) == 0 { + return result, cfg.Manifest, nil + } + + if cfg.DryRun { + for _, entry := range toAdd { + progress(entry.Repo, "dry-run", "Would add to manifest") + } + return result, cfg.Manifest, nil + } + + cfg.Manifest.Repos = append(cfg.Manifest.Repos, toAdd...) + + if cfg.ManifestPath != "" { + if err := writeManifest(cfg.ManifestPath, cfg.Manifest); err != nil { + return nil, nil, err + } + } + + for _, entry := range toAdd { + progress(entry.Repo, "manifest", "Added to manifest") + } + + return result, cfg.Manifest, nil +} + +// RemoveFromManifest removes matching repo entries from the manifest. Patterns +// containing glob characters (*, ?, [) are matched against manifest entries +// using filepath.Match. Returns the result and the modified manifest. +func RemoveFromManifest(cfg ManifestEditConfig, repos []string, progress ProgressFunc) (*ManifestRemoveResult, *Manifest, error) { + if cfg.Manifest == nil { + return nil, nil, fmt.Errorf("manifest is required") + } + if len(repos) == 0 { + return nil, nil, fmt.Errorf("at least one repo is required") + } + if progress == nil { + progress = func(_, _, _ string) {} + } + + toRemove, err := matchManifestEntries(cfg.Manifest.Repos, repos) + if err != nil { + return nil, nil, err + } + + result := &ManifestRemoveResult{} + kept := make([]RepoEntry, 0, len(cfg.Manifest.Repos)) + for _, entry := range cfg.Manifest.Repos { + if toRemove[entry.Repo] { + result.Removed = append(result.Removed, entry.Repo) + } else { + kept = append(kept, entry) + } + } + + for _, pattern := range repos { + matched := false + for _, r := range result.Removed { + if ok, _ := matchesPattern(pattern, r); ok { + matched = true + break + } + } + if !matched && !isGlob(pattern) { + result.Skipped = append(result.Skipped, pattern) + progress(pattern, "manifest", "Not found in manifest") + } + } + + if len(result.Removed) == 0 { + return result, cfg.Manifest, nil + } + + if cfg.DryRun { + for _, r := range result.Removed { + progress(r, "dry-run", "Would remove from manifest") + } + return result, cfg.Manifest, nil + } + + cfg.Manifest.Repos = kept + + if cfg.ManifestPath != "" { + if err := writeManifest(cfg.ManifestPath, cfg.Manifest); err != nil { + return nil, nil, err + } + } + + for _, r := range result.Removed { + progress(r, "manifest", "Removed from manifest") + } + + return result, cfg.Manifest, nil +} + +// MatchManifestRepos returns the list of repo names from the manifest that +// match any of the given patterns. Used by CLI commands to resolve positional +// args (which may contain globs) against manifest entries. +func MatchManifestRepos(manifest *Manifest, patterns []string) ([]string, error) { + matched, err := matchManifestEntries(manifest.Repos, patterns) + if err != nil { + return nil, err + } + result := make([]string, 0, len(matched)) + for _, entry := range manifest.Repos { + if matched[entry.Repo] { + result = append(result, entry.Repo) + } + } + return result, nil +} + +// matchManifestEntries builds a set of manifest repo names that match any of +// the given patterns (exact or glob). +func matchManifestEntries(entries []RepoEntry, patterns []string) (map[string]bool, error) { + matched := make(map[string]bool) + for _, entry := range entries { + for _, pattern := range patterns { + ok, err := matchesPattern(pattern, entry.Repo) + if err != nil { + return nil, fmt.Errorf("invalid glob pattern %q: %w", pattern, err) + } + if ok { + matched[entry.Repo] = true + break + } + } + } + return matched, nil +} + +func matchesPattern(pattern, name string) (bool, error) { + if strings.EqualFold(pattern, name) { + return true, nil + } + if !isGlob(pattern) { + return false, nil + } + return filepath.Match(strings.ToLower(pattern), strings.ToLower(name)) +} + +func isGlob(s string) bool { + return strings.ContainsAny(s, "*?[") +} + +func writeManifest(path string, m *Manifest) error { + data, err := MarshalWithHeader(m) + if err != nil { + return fmt.Errorf("marshalling manifest: %w", err) + } + if err := os.WriteFile(path, data, 0o644); err != nil { + return fmt.Errorf("writing manifest: %w", err) + } + return nil +} diff --git a/internal/repos/manifest_edit_test.go b/internal/repos/manifest_edit_test.go new file mode 100644 index 0000000000..cf8e8b8d1b --- /dev/null +++ b/internal/repos/manifest_edit_test.go @@ -0,0 +1,512 @@ +package repos + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +func TestAddToManifest_Basic(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "repos.yaml") + + manifest := testManifest("acme/existing") + data, err := MarshalWithHeader(manifest) + if err != nil { + t.Fatalf("MarshalWithHeader() error = %v", err) + } + if err := os.WriteFile(manifestPath, data, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + result, updated, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + ManifestPath: manifestPath, + }, []RepoEntry{{Repo: "acme/new-repo"}}, nil, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + if len(result.Added) != 1 || result.Added[0] != "acme/new-repo" { + t.Errorf("Added = %v, want [acme/new-repo]", result.Added) + } + if len(result.Skipped) != 0 { + t.Errorf("Skipped = %v, want []", result.Skipped) + } + if len(updated.Repos) != 2 { + t.Errorf("manifest has %d repos, want 2", len(updated.Repos)) + } + + // Verify file was written. + reloaded, err := LoadManifest(context.Background(), manifestPath) + if err != nil { + t.Fatalf("LoadManifest() error = %v", err) + } + if len(reloaded.Repos) != 2 { + t.Errorf("reloaded manifest has %d repos, want 2", len(reloaded.Repos)) + } +} + +func TestAddToManifest_Duplicate(t *testing.T) { + manifest := testManifest("acme/api") + + result, _, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{{Repo: "acme/api"}}, nil, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + if len(result.Skipped) != 1 || result.Skipped[0] != "acme/api" { + t.Errorf("Skipped = %v, want [acme/api]", result.Skipped) + } + if len(result.Added) != 0 { + t.Errorf("Added = %v, want []", result.Added) + } +} + +func TestAddToManifest_DryRun(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "repos.yaml") + + manifest := testManifest("acme/existing") + data, err := MarshalWithHeader(manifest) + if err != nil { + t.Fatalf("MarshalWithHeader() error = %v", err) + } + if err := os.WriteFile(manifestPath, data, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + var phases []string + progress := func(_, phase, _ string) { + phases = append(phases, phase) + } + + result, _, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + ManifestPath: manifestPath, + DryRun: true, + }, []RepoEntry{{Repo: "acme/new"}}, nil, progress) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + if len(result.Added) != 1 { + t.Errorf("Added = %v, want [acme/new]", result.Added) + } + + // File should be unchanged. + reloaded, err := LoadManifest(context.Background(), manifestPath) + if err != nil { + t.Fatalf("LoadManifest() error = %v", err) + } + if len(reloaded.Repos) != 1 { + t.Errorf("reloaded manifest has %d repos, want 1 (dry-run)", len(reloaded.Repos)) + } + + hasDryRun := false + for _, p := range phases { + if p == "dry-run" { + hasDryRun = true + } + } + if !hasDryRun { + t.Error("missing 'dry-run' phase callback") + } +} + +func TestAddToManifest_Multiple(t *testing.T) { + manifest := testManifest("acme/existing") + + result, updated, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{ + {Repo: "acme/new-a"}, + {Repo: "acme/existing"}, + {Repo: "acme/new-b"}, + }, nil, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + if len(result.Added) != 2 { + t.Errorf("Added = %v, want 2 entries", result.Added) + } + if len(result.Skipped) != 1 { + t.Errorf("Skipped = %v, want [acme/existing]", result.Skipped) + } + if len(updated.Repos) != 3 { + t.Errorf("manifest has %d repos, want 3", len(updated.Repos)) + } +} + +func TestAddToManifest_NoManifest(t *testing.T) { + _, _, err := AddToManifest(context.Background(), ManifestEditConfig{}, []RepoEntry{{Repo: "acme/api"}}, nil, nil) + if err == nil { + t.Fatal("AddToManifest() error = nil, want error for nil manifest") + } +} + +func TestAddToManifest_EmptyRepos(t *testing.T) { + _, _, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: testManifest(), + }, nil, nil, nil) + if err == nil { + t.Fatal("AddToManifest() error = nil, want error for empty repos") + } +} + +func TestAddToManifest_InvalidRepoName(t *testing.T) { + _, _, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: testManifest(), + }, []RepoEntry{{Repo: "invalid-no-slash"}}, nil, nil) + if err == nil { + t.Fatal("AddToManifest() error = nil, want error for invalid repo name") + } + if !strings.Contains(err.Error(), "invalid repo name") { + t.Errorf("error = %q, want to contain 'invalid repo name'", err.Error()) + } +} + +func TestAddToManifest_GlobRepoAllowed(t *testing.T) { + manifest := testManifest() + result, _, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{{Repo: "acme/*"}}, nil, nil) + if err != nil { + t.Fatalf("AddToManifest() should allow glob entries: %v", err) + } + if len(result.Added) != 1 { + t.Errorf("Added = %v, want [acme/*]", result.Added) + } +} + +func TestAddToManifest_DiscoverInstalled(t *testing.T) { + fc := forge.NewFakeClient() + fc.VariableValues["acme/api/FULLSEND_PER_REPO_INSTALL"] = "true" + fc.VariableValues["acme/api/FULLSEND_MINT_URL"] = "https://mint.example.com" + fc.VariableValues["acme/api/FULLSEND_GCP_REGION"] = "us-east1" + fc.FileContents["acme/api/.github/workflows/fullsend.yml"] = []byte( + `uses: fullsend-ai/fullsend/.github/workflows/reusable-dispatch.yml@v2.1.0`) + + manifest := testManifest() + manifest.Defaults = DefaultsConfig{ + InferenceRegion: "us-central1", + FullsendRef: "v2.3.0", + } + + result, updated, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{{Repo: "acme/api"}}, fc, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + if len(result.Added) != 1 { + t.Fatalf("Added = %v, want [acme/api]", result.Added) + } + entry := updated.Repos[len(updated.Repos)-1] + if !entry.InferenceRegion.Set || entry.InferenceRegion.Value != "us-east1" { + t.Errorf("InferenceRegion = %+v, want {Set:true Value:us-east1}", entry.InferenceRegion) + } + if !entry.FullsendRef.Set || entry.FullsendRef.Value != "v2.1.0" { + t.Errorf("FullsendRef = %+v, want {Set:true Value:v2.1.0}", entry.FullsendRef) + } +} + +func TestAddToManifest_DiscoverInstalledMatchesDefaults(t *testing.T) { + fc := forge.NewFakeClient() + fc.VariableValues["acme/api/FULLSEND_PER_REPO_INSTALL"] = "true" + fc.VariableValues["acme/api/FULLSEND_MINT_URL"] = "https://mint.example.com" + fc.VariableValues["acme/api/FULLSEND_GCP_REGION"] = "us-central1" + fc.FileContents["acme/api/.github/workflows/fullsend.yml"] = []byte( + `uses: fullsend-ai/fullsend/.github/workflows/reusable-dispatch.yml@v2.3.0`) + + manifest := testManifest() + manifest.Defaults = DefaultsConfig{ + InferenceRegion: "us-central1", + FullsendRef: "v2.3.0", + } + + _, updated, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{{Repo: "acme/api"}}, fc, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + entry := updated.Repos[len(updated.Repos)-1] + if entry.InferenceRegion.Set { + t.Error("InferenceRegion should not be set when matching defaults") + } + if entry.FullsendRef.Set { + t.Error("FullsendRef should not be set when matching defaults") + } +} + +func TestAddToManifest_DiscoverNotInstalled(t *testing.T) { + fc := forge.NewFakeClient() + + manifest := testManifest() + manifest.Defaults = DefaultsConfig{ + InferenceRegion: "us-central1", + FullsendRef: "v2.3.0", + } + + _, updated, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{{Repo: "acme/api"}}, fc, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + entry := updated.Repos[len(updated.Repos)-1] + if entry.InferenceRegion.Set { + t.Error("InferenceRegion should not be set for uninstalled repo") + } + if entry.FullsendRef.Set { + t.Error("FullsendRef should not be set for uninstalled repo") + } +} + +func TestAddToManifest_DiscoverGlobSkipped(t *testing.T) { + fc := forge.NewFakeClient() + + manifest := testManifest() + result, _, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{{Repo: "acme/*"}}, fc, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v", err) + } + if len(result.Added) != 1 || result.Added[0] != "acme/*" { + t.Errorf("Added = %v, want [acme/*]", result.Added) + } +} + +func TestAddToManifest_DiscoverProbeError(t *testing.T) { + fc := forge.NewFakeClient() + fc.Errors["ListRepoVariables"] = fmt.Errorf("api error") + + manifest := testManifest() + manifest.Defaults = DefaultsConfig{FullsendRef: "v2.3.0"} + + result, _, err := AddToManifest(context.Background(), ManifestEditConfig{ + Manifest: manifest, + }, []RepoEntry{{Repo: "acme/api"}}, fc, nil) + + if err != nil { + t.Fatalf("AddToManifest() error = %v, want graceful skip on probe error", err) + } + if len(result.Added) != 1 { + t.Errorf("Added = %v, want [acme/api] even on probe error", result.Added) + } +} + +func TestRemoveFromManifest_Basic(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "repos.yaml") + + manifest := testManifest("acme/api", "acme/web", "acme/docs") + data, err := MarshalWithHeader(manifest) + if err != nil { + t.Fatalf("MarshalWithHeader() error = %v", err) + } + if err := os.WriteFile(manifestPath, data, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + result, updated, err := RemoveFromManifest(ManifestEditConfig{ + Manifest: manifest, + ManifestPath: manifestPath, + }, []string{"acme/api", "acme/docs"}, nil) + + if err != nil { + t.Fatalf("RemoveFromManifest() error = %v", err) + } + if len(result.Removed) != 2 { + t.Errorf("Removed = %v, want 2 entries", result.Removed) + } + if len(result.Skipped) != 0 { + t.Errorf("Skipped = %v, want []", result.Skipped) + } + if len(updated.Repos) != 1 { + t.Errorf("manifest has %d repos, want 1", len(updated.Repos)) + } + if updated.Repos[0].Repo != "acme/web" { + t.Errorf("remaining repo = %q, want acme/web", updated.Repos[0].Repo) + } + + reloaded, err := LoadManifest(context.Background(), manifestPath) + if err != nil { + t.Fatalf("LoadManifest() error = %v", err) + } + if len(reloaded.Repos) != 1 { + t.Errorf("reloaded manifest has %d repos, want 1", len(reloaded.Repos)) + } +} + +func TestRemoveFromManifest_Glob(t *testing.T) { + manifest := testManifest("acme/api", "acme/web", "other/docs") + + result, updated, err := RemoveFromManifest(ManifestEditConfig{ + Manifest: manifest, + }, []string{"acme/*"}, nil) + + if err != nil { + t.Fatalf("RemoveFromManifest() error = %v", err) + } + if len(result.Removed) != 2 { + t.Errorf("Removed = %v, want [acme/api, acme/web]", result.Removed) + } + if len(updated.Repos) != 1 { + t.Errorf("manifest has %d repos, want 1", len(updated.Repos)) + } + if updated.Repos[0].Repo != "other/docs" { + t.Errorf("remaining repo = %q, want other/docs", updated.Repos[0].Repo) + } +} + +func TestRemoveFromManifest_NotFound(t *testing.T) { + manifest := testManifest("acme/web") + + var msgs []string + progress := func(_, _, msg string) { msgs = append(msgs, msg) } + + result, _, err := RemoveFromManifest(ManifestEditConfig{ + Manifest: manifest, + }, []string{"acme/missing"}, progress) + + if err != nil { + t.Fatalf("RemoveFromManifest() error = %v", err) + } + if len(result.Skipped) != 1 || result.Skipped[0] != "acme/missing" { + t.Errorf("Skipped = %v, want [acme/missing]", result.Skipped) + } + if len(result.Removed) != 0 { + t.Errorf("Removed = %v, want []", result.Removed) + } +} + +func TestRemoveFromManifest_DryRun(t *testing.T) { + dir := t.TempDir() + manifestPath := filepath.Join(dir, "repos.yaml") + + manifest := testManifest("acme/api", "acme/web") + data, err := MarshalWithHeader(manifest) + if err != nil { + t.Fatalf("MarshalWithHeader() error = %v", err) + } + if err := os.WriteFile(manifestPath, data, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + var phases []string + progress := func(_, phase, _ string) { phases = append(phases, phase) } + + result, _, err := RemoveFromManifest(ManifestEditConfig{ + Manifest: manifest, + ManifestPath: manifestPath, + DryRun: true, + }, []string{"acme/api"}, progress) + + if err != nil { + t.Fatalf("RemoveFromManifest() error = %v", err) + } + if len(result.Removed) != 1 { + t.Errorf("Removed = %v, want [acme/api]", result.Removed) + } + + reloaded, err := LoadManifest(context.Background(), manifestPath) + if err != nil { + t.Fatalf("LoadManifest() error = %v", err) + } + if len(reloaded.Repos) != 2 { + t.Errorf("reloaded manifest has %d repos, want 2 (dry-run)", len(reloaded.Repos)) + } + + hasDryRun := false + for _, p := range phases { + if p == "dry-run" { + hasDryRun = true + } + } + if !hasDryRun { + t.Error("missing 'dry-run' phase callback") + } +} + +func TestRemoveFromManifest_NoManifest(t *testing.T) { + _, _, err := RemoveFromManifest(ManifestEditConfig{}, []string{"acme/api"}, nil) + if err == nil { + t.Fatal("RemoveFromManifest() error = nil, want error for nil manifest") + } +} + +func TestRemoveFromManifest_EmptyRepos(t *testing.T) { + _, _, err := RemoveFromManifest(ManifestEditConfig{ + Manifest: testManifest(), + }, nil, nil) + if err == nil { + t.Fatal("RemoveFromManifest() error = nil, want error for empty repos") + } +} + +func TestMatchManifestRepos_Exact(t *testing.T) { + manifest := testManifest("acme/api", "acme/web", "other/docs") + matched, err := MatchManifestRepos(manifest, []string{"acme/api"}) + if err != nil { + t.Fatal(err) + } + if len(matched) != 1 || matched[0] != "acme/api" { + t.Errorf("MatchManifestRepos() = %v, want [acme/api]", matched) + } +} + +func TestMatchManifestRepos_Glob(t *testing.T) { + manifest := testManifest("acme/api", "acme/web", "other/docs") + matched, err := MatchManifestRepos(manifest, []string{"acme/*"}) + if err != nil { + t.Fatal(err) + } + if len(matched) != 2 { + t.Errorf("MatchManifestRepos() = %v, want [acme/api, acme/web]", matched) + } +} + +func TestMatchManifestRepos_CaseInsensitive(t *testing.T) { + manifest := testManifest("Acme/API") + matched, err := MatchManifestRepos(manifest, []string{"acme/api"}) + if err != nil { + t.Fatal(err) + } + if len(matched) != 1 { + t.Errorf("MatchManifestRepos() = %v, want [Acme/API]", matched) + } +} + +func TestMatchManifestRepos_NoMatch(t *testing.T) { + manifest := testManifest("acme/api") + matched, err := MatchManifestRepos(manifest, []string{"other/repo"}) + if err != nil { + t.Fatal(err) + } + if len(matched) != 0 { + t.Errorf("MatchManifestRepos() = %v, want []", matched) + } +} + +func TestMatchManifestRepos_BadPattern(t *testing.T) { + manifest := testManifest("acme/api") + _, err := MatchManifestRepos(manifest, []string{"acme/[invalid"}) + if err == nil { + t.Error("expected error for malformed glob pattern") + } +} diff --git a/internal/repos/status.go b/internal/repos/status.go index efdbb7484d..6cac095d3d 100644 --- a/internal/repos/status.go +++ b/internal/repos/status.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "regexp" - "strings" "sync" "github.com/fullsend-ai/fullsend/internal/forge" @@ -20,6 +19,42 @@ var workflowPaths = []string{ ".github/workflows/fullsend.yaml", } +// RepoState holds the installation state of a single repo as read +// from GitHub variables and workflow files. +type RepoState struct { + Installed bool + MintURL string + InferenceRegion string + FullsendRef string +} + +// ProbeRepoState reads a repo's current per-repo installation state +// from GitHub variables and workflow files. +func ProbeRepoState(ctx context.Context, client forge.Client, owner, repo string) (RepoState, error) { + vars, err := client.ListRepoVariables(ctx, owner, repo) + if err != nil { + return RepoState{}, fmt.Errorf("listing variables for %s/%s: %w", owner, repo, err) + } + + if vars[forge.PerRepoGuardVar] != "true" { + return RepoState{}, nil + } + + state := RepoState{ + Installed: true, + MintURL: vars["FULLSEND_MINT_URL"], + InferenceRegion: vars["FULLSEND_GCP_REGION"], + } + + ref, err := readWorkflowRef(ctx, client, owner, repo) + if err != nil { + return state, fmt.Errorf("reading workflow for %s/%s: %w", owner, repo, err) + } + state.FullsendRef = ref + + return state, nil +} + // Drift describes a single field that differs between the manifest's // desired state and the repo's actual state. type Drift struct { @@ -72,7 +107,11 @@ func Status(ctx context.Context, manifest *Manifest, client forge.Client, maxCon } if len(repoFilter) > 0 { - resolved = filterRepos(resolved, repoFilter) + var filterErr error + resolved, filterErr = filterRepos(resolved, repoFilter) + if filterErr != nil { + return nil, filterErr + } } if maxConcurrency < 1 { @@ -129,27 +168,22 @@ func checkRepoStatus(ctx context.Context, client forge.Client, owner, repo strin ExpectedRegion: cfg.InferenceRegion, } - vars, err := client.ListRepoVariables(ctx, owner, repo) + state, err := ProbeRepoState(ctx, client, owner, repo) if err != nil { - status.Error = fmt.Sprintf("listing variables: %v", err) - return status + status.Error = err.Error() } - guard := vars[forge.PerRepoGuardVar] - if guard != "true" { + if !state.Installed { return status } status.Installed = true + status.MintURL = state.MintURL + status.Region = state.InferenceRegion + status.CurrentRef = state.FullsendRef - status.MintURL = vars["FULLSEND_MINT_URL"] - status.Region = vars["FULLSEND_GCP_REGION"] - - ref, err := readWorkflowRef(ctx, client, owner, repo) if err != nil { - status.Error = fmt.Sprintf("reading workflow: %v", err) return status } - status.CurrentRef = ref if cfg.MintURL != "" && status.MintURL != cfg.MintURL { status.Drifts = append(status.Drifts, Drift{ @@ -201,17 +235,20 @@ func extractWorkflowRef(content []byte) string { return string(m[1]) } -func filterRepos(repos []ResolvedRepo, filter []string) []ResolvedRepo { - allowed := make(map[string]bool, len(filter)) - for _, f := range filter { - allowed[strings.ToLower(f)] = true - } +func filterRepos(repos []ResolvedRepo, filter []string) ([]ResolvedRepo, error) { var result []ResolvedRepo for _, rr := range repos { - fullName := strings.ToLower(rr.Owner + "/" + rr.Repo) - if allowed[fullName] { - result = append(result, rr) + fullName := rr.Owner + "/" + rr.Repo + for _, pattern := range filter { + ok, err := matchesPattern(pattern, fullName) + if err != nil { + return nil, fmt.Errorf("invalid glob pattern %q: %w", pattern, err) + } + if ok { + result = append(result, rr) + break + } } } - return result + return result, nil } diff --git a/internal/repos/status_test.go b/internal/repos/status_test.go index 0fe79e1101..14bf116c76 100644 --- a/internal/repos/status_test.go +++ b/internal/repos/status_test.go @@ -51,6 +51,58 @@ jobs: fc.FileContents[owner+"/"+repo+"/.github/workflows/fullsend.yml"] = []byte(workflow) } +func TestProbeRepoState_Installed(t *testing.T) { + fc := forge.NewFakeClient() + populateInstalledRepo(fc, "acme", "api", "v2.3.0", "https://mint.example.com", "us-east1") + + state, err := ProbeRepoState(context.Background(), fc, "acme", "api") + if err != nil { + t.Fatalf("ProbeRepoState() error = %v", err) + } + if !state.Installed { + t.Fatal("Installed = false, want true") + } + if state.MintURL != "https://mint.example.com" { + t.Errorf("MintURL = %q, want https://mint.example.com", state.MintURL) + } + if state.InferenceRegion != "us-east1" { + t.Errorf("InferenceRegion = %q, want us-east1", state.InferenceRegion) + } + if state.FullsendRef != "v2.3.0" { + t.Errorf("FullsendRef = %q, want v2.3.0", state.FullsendRef) + } +} + +func TestProbeRepoState_NotInstalled(t *testing.T) { + fc := forge.NewFakeClient() + + state, err := ProbeRepoState(context.Background(), fc, "acme", "api") + if err != nil { + t.Fatalf("ProbeRepoState() error = %v", err) + } + if state.Installed { + t.Fatal("Installed = true, want false") + } +} + +func TestProbeRepoState_WorkflowError(t *testing.T) { + fc := forge.NewFakeClient() + fc.VariableValues["acme/api/FULLSEND_PER_REPO_INSTALL"] = "true" + fc.VariableValues["acme/api/FULLSEND_GCP_REGION"] = "us-east1" + fc.Errors["GetFileContent"] = fmt.Errorf("server error") + + state, err := ProbeRepoState(context.Background(), fc, "acme", "api") + if err == nil { + t.Fatal("expected error for workflow read failure") + } + if !state.Installed { + t.Fatal("Installed = false, want true even on workflow error") + } + if state.InferenceRegion != "us-east1" { + t.Errorf("InferenceRegion = %q, want us-east1", state.InferenceRegion) + } +} + func TestStatus_AllInstalled_NoDrift(t *testing.T) { fc := forge.NewFakeClient() m := newTestManifest() @@ -630,7 +682,10 @@ func TestFilterRepos(t *testing.T) { } t.Run("single filter", func(t *testing.T) { - result := filterRepos(repos, []string{"acme-corp/api-server"}) + result, err := filterRepos(repos, []string{"acme-corp/api-server"}) + if err != nil { + t.Fatal(err) + } if len(result) != 1 { t.Fatalf("got %d results, want 1", len(result)) } @@ -640,25 +695,84 @@ func TestFilterRepos(t *testing.T) { }) t.Run("multiple filters", func(t *testing.T) { - result := filterRepos(repos, []string{"acme-corp/api-server", "other-org/tool"}) + result, err := filterRepos(repos, []string{"acme-corp/api-server", "other-org/tool"}) + if err != nil { + t.Fatal(err) + } if len(result) != 2 { t.Fatalf("got %d results, want 2", len(result)) } }) t.Run("no match", func(t *testing.T) { - result := filterRepos(repos, []string{"nonexistent/repo"}) + result, err := filterRepos(repos, []string{"nonexistent/repo"}) + if err != nil { + t.Fatal(err) + } if len(result) != 0 { t.Fatalf("got %d results, want 0", len(result)) } }) t.Run("case insensitive", func(t *testing.T) { - result := filterRepos(repos, []string{"ACME-CORP/API-SERVER"}) + result, err := filterRepos(repos, []string{"ACME-CORP/API-SERVER"}) + if err != nil { + t.Fatal(err) + } if len(result) != 1 { t.Fatalf("got %d results, want 1", len(result)) } }) + + t.Run("glob wildcard", func(t *testing.T) { + result, err := filterRepos(repos, []string{"acme-corp/*"}) + if err != nil { + t.Fatal(err) + } + if len(result) != 2 { + t.Fatalf("got %d results, want 2", len(result)) + } + }) + + t.Run("glob question mark", func(t *testing.T) { + result, err := filterRepos(repos, []string{"other-org/too?"}) + if err != nil { + t.Fatal(err) + } + if len(result) != 1 { + t.Fatalf("got %d results, want 1", len(result)) + } + if result[0].Repo != "tool" { + t.Errorf("repo = %q, want tool", result[0].Repo) + } + }) + + t.Run("glob no match", func(t *testing.T) { + result, err := filterRepos(repos, []string{"missing-org/*"}) + if err != nil { + t.Fatal(err) + } + if len(result) != 0 { + t.Fatalf("got %d results, want 0", len(result)) + } + }) + + t.Run("glob case insensitive", func(t *testing.T) { + result, err := filterRepos(repos, []string{"ACME-CORP/*"}) + if err != nil { + t.Fatal(err) + } + if len(result) != 2 { + t.Fatalf("got %d results, want 2", len(result)) + } + }) + + t.Run("bad pattern", func(t *testing.T) { + _, err := filterRepos(repos, []string{"acme-corp/[invalid"}) + if err == nil { + t.Error("expected error for malformed glob pattern") + } + }) } func TestStatus_GuardVarFalse(t *testing.T) { diff --git a/internal/repos/uninstall.go b/internal/repos/uninstall.go new file mode 100644 index 0000000000..222c5270fc --- /dev/null +++ b/internal/repos/uninstall.go @@ -0,0 +1,256 @@ +package repos + +import ( + "context" + "errors" + "fmt" + "strings" + "sync" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +var uninstallVariables = []string{ + forge.PerRepoGuardVar, + "FULLSEND_MINT_URL", + "FULLSEND_GCP_REGION", +} + +var uninstallSecrets = []string{ + "FULLSEND_GCP_PROJECT_ID", + "FULLSEND_GCP_WIF_PROVIDER", +} + +// UninstallConfig holds all inputs for a multi-repo uninstall operation. +type UninstallConfig struct { + Manifest *Manifest + Repos []string + DryRun bool + SkipWIFCleanup bool + MaxConcurrency int +} + +// UninstallResult holds the outcome of uninstalling fullsend from a single repo. +type UninstallResult struct { + Owner string + Repo string + Success bool + Error error + WorkflowDeleted bool + VarsDeleted int + SecretsDeleted int + WIFDeregistered bool +} + +// Uninstall tears down fullsend from the specified repos. +// +// It runs in two phases: +// 1. Parallel per-repo cleanup (bounded by MaxConcurrency): delete workflow +// file, then delete variables and secrets. If workflow deletion fails, +// variables and secrets are left intact. +// 2. Sequential WIF cleanup (only for Phase 1 successes): deregister from +// mint's PER_REPO_WIF_REPOS and delete WIF provider. Sequential because +// mint env var updates are read-modify-write operations. +// +// Does NOT modify repos.yaml — use RemoveFromManifest for that. +func Uninstall(ctx context.Context, cfg UninstallConfig, + client forge.Client, provisionerFactory ProvisionerFactory, + progress ProgressFunc) ([]UninstallResult, error) { + + if len(cfg.Repos) == 0 { + return nil, fmt.Errorf("at least one repo is required") + } + if cfg.MaxConcurrency <= 0 || cfg.MaxConcurrency > 32 { + return nil, fmt.Errorf("MaxConcurrency must be between 1 and 32, got %d", cfg.MaxConcurrency) + } + if progress == nil { + progress = func(_, _, _ string) {} + } + + parsed := make([]struct{ owner, repo string }, len(cfg.Repos)) + for i, r := range cfg.Repos { + owner, name, err := splitOwnerRepo(r) + if err != nil { + return nil, err + } + parsed[i].owner = owner + parsed[i].repo = name + } + + if cfg.DryRun { + results := make([]UninstallResult, len(parsed)) + for i, p := range parsed { + results[i] = UninstallResult{ + Owner: p.owner, + Repo: p.repo, + Success: true, + } + progress(p.owner+"/"+p.repo, "dry-run", "Would uninstall") + } + return results, nil + } + + // Phase 1: Parallel per-repo cleanup. + results := make([]UninstallResult, len(parsed)) + sem := make(chan struct{}, cfg.MaxConcurrency) + var wg sync.WaitGroup + + for i, p := range parsed { + wg.Add(1) + go func(idx int, owner, repo string) { + defer wg.Done() + select { + case sem <- struct{}{}: + case <-ctx.Done(): + results[idx] = UninstallResult{ + Owner: owner, + Repo: repo, + Error: ctx.Err(), + } + return + } + defer func() { <-sem }() + + results[idx] = uninstallRepoResources(ctx, owner, repo, client, progress) + }(i, p.owner, p.repo) + } + wg.Wait() + + // Phase 2: Sequential WIF cleanup (only for Phase 1 successes). + if !cfg.SkipWIFCleanup && provisionerFactory != nil && cfg.Manifest != nil { + for i := range results { + if results[i].Error != nil || !results[i].WorkflowDeleted { + continue + } + if ctx.Err() != nil { + for j := i; j < len(results); j++ { + if results[j].Error != nil || !results[j].WorkflowDeleted { + continue + } + if _, ok := resolveConfigWithGlobs(cfg.Manifest, results[j].Owner, results[j].Repo); ok { + results[j].Error = fmt.Errorf("WIF cleanup skipped: %w", ctx.Err()) + } + } + break + } + + fullName := results[i].Owner + "/" + results[i].Repo + resolved, ok := resolveConfigWithGlobs(cfg.Manifest, results[i].Owner, results[i].Repo) + if !ok { + progress(fullName, "wif", "Not in manifest, skipping WIF cleanup") + results[i].Success = true + continue + } + + prov := provisionerFactory(resolved) + progress(fullName, "wif", "Deregistering from mint and deleting WIF provider") + if err := prov.DeletePerRepoWIF(ctx, fullName); err != nil { + results[i].Error = fmt.Errorf("WIF cleanup: %w", err) + progress(fullName, "wif", fmt.Sprintf("WIF cleanup failed: %v", err)) + continue + } + results[i].WIFDeregistered = true + progress(fullName, "wif", "WIF cleanup complete") + } + } + + for i := range results { + if results[i].Error == nil { + results[i].Success = true + } + } + + return results, nil +} + +func uninstallRepoResources(ctx context.Context, owner, repo string, + client forge.Client, progress ProgressFunc) UninstallResult { + + fullName := owner + "/" + repo + result := UninstallResult{Owner: owner, Repo: repo} + + progress(fullName, "workflow", "Deleting workflow file") + _, err := client.DeleteFiles(ctx, owner, repo, + "chore: remove fullsend workflow", workflowPaths) + if err != nil { + result.Error = fmt.Errorf("deleting workflow: %w", err) + progress(fullName, "workflow", fmt.Sprintf("Failed: %v", err)) + return result + } + result.WorkflowDeleted = true + progress(fullName, "workflow", "Workflow deleted") + + var varsDeleted, secretsDeleted int + var varErr, secretErr error + var innerWg sync.WaitGroup + + innerWg.Add(2) + go func() { + defer innerWg.Done() + for _, name := range uninstallVariables { + if delErr := client.DeleteRepoVariable(ctx, owner, repo, name); delErr != nil { + varErr = fmt.Errorf("deleting variable %s: %w", name, delErr) + return + } + varsDeleted++ + } + }() + go func() { + defer innerWg.Done() + for _, name := range uninstallSecrets { + if delErr := client.DeleteRepoSecret(ctx, owner, repo, name); delErr != nil { + secretErr = fmt.Errorf("deleting secret %s: %w", name, delErr) + return + } + secretsDeleted++ + } + }() + innerWg.Wait() + + result.VarsDeleted = varsDeleted + result.SecretsDeleted = secretsDeleted + + if varErr != nil && secretErr != nil { + result.Error = errors.Join(varErr, secretErr) + progress(fullName, "cleanup", fmt.Sprintf("Failed: %v; %v", varErr, secretErr)) + return result + } + if varErr != nil { + result.Error = varErr + progress(fullName, "vars", fmt.Sprintf("Failed: %v", varErr)) + return result + } + if secretErr != nil { + result.Error = secretErr + progress(fullName, "secrets", fmt.Sprintf("Failed: %v", secretErr)) + return result + } + + progress(fullName, "done", fmt.Sprintf("Removed: %d vars, %d secrets", varsDeleted, secretsDeleted)) + return result +} + +// resolveConfigWithGlobs resolves config for a repo, falling back to +// glob-pattern matching when the exact entry lookup fails. +func resolveConfigWithGlobs(m *Manifest, owner, repo string) (ResolvedConfig, bool) { + if resolved, ok := m.ResolveConfig(owner, repo); ok { + return resolved, true + } + fullName := owner + "/" + repo + for _, e := range m.Repos { + if ok, _ := matchesPattern(e.Repo, fullName); ok { + return m.ResolveConfigForEntry(owner, repo, e), true + } + } + return ResolvedConfig{}, false +} + +// splitOwnerRepo splits "owner/repo" and rejects glob characters. Callers +// that accept glob patterns must filter them out before calling this. +func splitOwnerRepo(fullName string) (string, string, error) { + if !repoNamePattern.MatchString(fullName) { + return "", "", fmt.Errorf("invalid repo format %q: expected owner/repo with alphanumeric, dash, dot, or underscore characters", fullName) + } + parts := strings.SplitN(fullName, "/", 2) + return parts[0], parts[1], nil +} diff --git a/internal/repos/uninstall_test.go b/internal/repos/uninstall_test.go new file mode 100644 index 0000000000..fd2e5165e3 --- /dev/null +++ b/internal/repos/uninstall_test.go @@ -0,0 +1,820 @@ +package repos + +import ( + "context" + "fmt" + "sync" + "testing" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +type uninstallFakeProvisioner struct { + mu sync.Mutex + deleteCalls []string + deleteErr error + deleteErrs map[string]error +} + +func (f *uninstallFakeProvisioner) DiscoverMint(_ context.Context) (*MintDiscovery, error) { + return &MintDiscovery{URL: "https://mint.example.com"}, nil +} + +func (f *uninstallFakeProvisioner) ProvisionWIF(_ context.Context) (string, error) { + return fakeWIFProvider, nil +} + +func (f *uninstallFakeProvisioner) RegisterPerRepoWIF(_ context.Context, _ string) error { + return nil +} + +func (f *uninstallFakeProvisioner) EnsureOrgInMint(_ context.Context, _ string, _ string) error { + return nil +} + +func (f *uninstallFakeProvisioner) DeletePerRepoWIF(_ context.Context, repo string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.deleteCalls = append(f.deleteCalls, repo) + if err, ok := f.deleteErrs[repo]; ok { + return err + } + return f.deleteErr +} + +func (f *uninstallFakeProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} + +func newInstalledFakeClient(repos ...string) *forge.FakeClient { + client := forge.NewFakeClient() + for _, r := range repos { + client.VariableValues[r+"/"+forge.PerRepoGuardVar] = "true" + client.VariableValues[r+"/FULLSEND_MINT_URL"] = "https://mint.example.com" + client.VariableValues[r+"/FULLSEND_GCP_REGION"] = "us-central1" + client.VariablesExist[r+"/"+forge.PerRepoGuardVar] = true + client.VariablesExist[r+"/FULLSEND_MINT_URL"] = true + client.VariablesExist[r+"/FULLSEND_GCP_REGION"] = true + client.Secrets[r+"/FULLSEND_GCP_PROJECT_ID"] = true + client.Secrets[r+"/FULLSEND_GCP_WIF_PROVIDER"] = true + client.FileContents[r+"/.github/workflows/fullsend.yml"] = []byte("name: fullsend\n") + } + return client +} + +func testManifest(repos ...string) *Manifest { + m := &Manifest{ + Version: 1, + Mint: MintConfig{ + URL: "https://mint.example.com", + Project: "test-project", + Region: "us-central1", + }, + Defaults: DefaultsConfig{ + InferenceProject: "test-inference", + InferenceRegion: "us-central1", + FullsendRef: "v1.0.0", + }, + } + for _, r := range repos { + m.Repos = append(m.Repos, RepoEntry{Repo: r}) + } + return m +} + +func TestUninstall_InstalledRepo(t *testing.T) { + client := newInstalledFakeClient("acme/api") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + if len(results) != 1 { + t.Fatalf("got %d results, want 1", len(results)) + } + r := results[0] + if !r.Success { + t.Errorf("Success = false, want true; Error = %v", r.Error) + } + if !r.WorkflowDeleted { + t.Error("WorkflowDeleted = false, want true") + } + if r.VarsDeleted != 3 { + t.Errorf("VarsDeleted = %d, want 3", r.VarsDeleted) + } + if r.SecretsDeleted != 2 { + t.Errorf("SecretsDeleted = %d, want 2", r.SecretsDeleted) + } + if !r.WIFDeregistered { + t.Error("WIFDeregistered = false, want true") + } + + if len(client.DeletedFiles) == 0 { + t.Error("no files were deleted") + } + if len(client.DeletedVariables) != 3 { + t.Errorf("deleted %d variables, want 3", len(client.DeletedVariables)) + } + if len(client.DeletedSecrets) != 2 { + t.Errorf("deleted %d secrets, want 2", len(client.DeletedSecrets)) + } + if len(prov.deleteCalls) != 1 || prov.deleteCalls[0] != "acme/api" { + t.Errorf("DeletePerRepoWIF calls = %v, want [acme/api]", prov.deleteCalls) + } +} + +func TestUninstall_GlobManifestEntry_WIFCleanup(t *testing.T) { + client := newInstalledFakeClient("acme/api") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + manifest := testManifest() + manifest.Repos = []RepoEntry{{Repo: "acme/*"}} + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: manifest, + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + if len(results) != 1 || !results[0].Success { + t.Fatalf("expected 1 successful result, got %+v", results) + } + if !results[0].WIFDeregistered { + t.Error("WIFDeregistered = false, want true — glob entry should match for WIF cleanup") + } + if len(prov.deleteCalls) != 1 { + t.Errorf("DeletePerRepoWIF calls = %v, want [acme/api]", prov.deleteCalls) + } +} + +func TestResolveConfigWithGlobs_ExactMatch(t *testing.T) { + m := testManifest("acme/api") + resolved, ok := resolveConfigWithGlobs(m, "acme", "api") + if !ok { + t.Fatal("expected ok=true for exact match") + } + if resolved.Owner != "acme" || resolved.Repo != "api" { + t.Errorf("resolved = %s/%s, want acme/api", resolved.Owner, resolved.Repo) + } +} + +func TestResolveConfigWithGlobs_GlobMatch(t *testing.T) { + m := testManifest() + m.Repos = []RepoEntry{{Repo: "acme/*"}} + resolved, ok := resolveConfigWithGlobs(m, "acme", "api") + if !ok { + t.Fatal("expected ok=true for glob match") + } + if resolved.Owner != "acme" || resolved.Repo != "api" { + t.Errorf("resolved = %s/%s, want acme/api", resolved.Owner, resolved.Repo) + } +} + +func TestResolveConfigWithGlobs_NoMatch(t *testing.T) { + m := testManifest("other/repo") + _, ok := resolveConfigWithGlobs(m, "acme", "api") + if ok { + t.Error("expected ok=false for no match") + } +} + +func TestUninstall_NonInstalledRepo(t *testing.T) { + client := forge.NewFakeClient() + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if !r.Success { + t.Errorf("Success = false, want true; Error = %v", r.Error) + } + if !r.WorkflowDeleted { + t.Error("WorkflowDeleted = false, want true (file already absent)") + } +} + +func TestUninstall_YamlExtensionFallback(t *testing.T) { + client := forge.NewFakeClient() + client.FileContents["acme/api/.github/workflows/fullsend.yaml"] = []byte("name: fullsend\n") + + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if !r.Success { + t.Errorf("Success = false; Error = %v", r.Error) + } + if !r.WorkflowDeleted { + t.Error("WorkflowDeleted = false, want true") + } + found := false + for _, f := range client.DeletedFiles { + if f.Path == ".github/workflows/fullsend.yaml" { + found = true + } + } + if !found { + t.Error("fullsend.yaml was not deleted") + } +} + +func TestUninstall_SkipWIFCleanup(t *testing.T) { + client := newInstalledFakeClient("acme/api") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + SkipWIFCleanup: true, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if !r.Success { + t.Errorf("Success = false; Error = %v", r.Error) + } + if r.WIFDeregistered { + t.Error("WIFDeregistered = true, want false with --skip-wif-cleanup") + } + if len(prov.deleteCalls) != 0 { + t.Errorf("DeletePerRepoWIF calls = %v, want none", prov.deleteCalls) + } +} + +func TestUninstall_DryRun(t *testing.T) { + client := newInstalledFakeClient("acme/api") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + DryRun: true, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if !r.Success { + t.Errorf("Success = false; Error = %v", r.Error) + } + if len(client.DeletedFiles) != 0 { + t.Errorf("dry-run deleted %d files, want 0", len(client.DeletedFiles)) + } + if len(client.DeletedVariables) != 0 { + t.Errorf("dry-run deleted %d variables, want 0", len(client.DeletedVariables)) + } + if len(client.DeletedSecrets) != 0 { + t.Errorf("dry-run deleted %d secrets, want 0", len(client.DeletedSecrets)) + } + if len(prov.deleteCalls) != 0 { + t.Errorf("dry-run made %d provisioner calls, want 0", len(prov.deleteCalls)) + } +} + +func TestUninstall_MultipleRepos(t *testing.T) { + client := newInstalledFakeClient("acme/api", "acme/web", "acme/docs") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + manifest := testManifest("acme/api", "acme/web", "acme/docs") + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: manifest, + Repos: []string{"acme/api", "acme/web", "acme/docs"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + if len(results) != 3 { + t.Fatalf("got %d results, want 3", len(results)) + } + for _, r := range results { + if !r.Success { + t.Errorf("%s/%s: Success = false; Error = %v", r.Owner, r.Repo, r.Error) + } + if !r.WIFDeregistered { + t.Errorf("%s/%s: WIFDeregistered = false", r.Owner, r.Repo) + } + } + if len(prov.deleteCalls) != 3 { + t.Errorf("DeletePerRepoWIF calls = %d, want 3", len(prov.deleteCalls)) + } +} + +func TestUninstall_PartialFailure(t *testing.T) { + client := newInstalledFakeClient("acme/api", "acme/web") + client.Errors["DeleteFiles"] = fmt.Errorf("permission denied") + + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + manifest := testManifest("acme/api", "acme/web") + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: manifest, + Repos: []string{"acme/api", "acme/web"}, + MaxConcurrency: 1, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + for _, r := range results { + if r.Success { + t.Errorf("%s/%s: Success = true, want false (global DeleteFiles error)", r.Owner, r.Repo) + } + if r.WorkflowDeleted { + t.Errorf("%s/%s: WorkflowDeleted = true, want false", r.Owner, r.Repo) + } + } + if len(prov.deleteCalls) != 0 { + t.Errorf("DeletePerRepoWIF calls = %d, want 0 (Phase 1 failed)", len(prov.deleteCalls)) + } +} + +func TestUninstall_WorkflowFailure_SkipsVarsAndSecrets(t *testing.T) { + client := forge.NewFakeClient() + client.FileContents["acme/api/.github/workflows/fullsend.yml"] = []byte("name: fullsend\n") + client.VariableValues["acme/api/"+forge.PerRepoGuardVar] = "true" + client.VariablesExist["acme/api/"+forge.PerRepoGuardVar] = true + client.Errors["DeleteFiles"] = fmt.Errorf("branch protection") + + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if r.Success { + t.Error("Success = true, want false") + } + if r.WorkflowDeleted { + t.Error("WorkflowDeleted = true, want false") + } + if len(client.DeletedVariables) != 0 { + t.Errorf("deleted %d variables, want 0 (workflow deletion failed)", len(client.DeletedVariables)) + } + if len(client.DeletedSecrets) != 0 { + t.Errorf("deleted %d secrets, want 0 (workflow deletion failed)", len(client.DeletedSecrets)) + } +} + +type sequentialUninstallProvisioner struct { + mu *sync.Mutex + sequence *[]string +} + +func (p *sequentialUninstallProvisioner) DiscoverMint(_ context.Context) (*MintDiscovery, error) { + return &MintDiscovery{URL: "https://mint.example.com"}, nil +} +func (p *sequentialUninstallProvisioner) ProvisionWIF(_ context.Context) (string, error) { + return fakeWIFProvider, nil +} +func (p *sequentialUninstallProvisioner) RegisterPerRepoWIF(_ context.Context, _ string) error { + return nil +} +func (p *sequentialUninstallProvisioner) EnsureOrgInMint(_ context.Context, _ string, _ string) error { + return nil +} +func (p *sequentialUninstallProvisioner) DeletePerRepoWIF(_ context.Context, repo string) error { + p.mu.Lock() + defer p.mu.Unlock() + *p.sequence = append(*p.sequence, repo) + return nil +} + +func (p *sequentialUninstallProvisioner) DeleteWIFProvider(_ context.Context, _ string) error { + return nil +} + +func TestUninstall_WIFSequential(t *testing.T) { + repos := []string{"acme/a", "acme/b", "acme/c", "acme/d", "acme/e"} + client := newInstalledFakeClient(repos...) + + var mu sync.Mutex + var sequence []string + + sequentialProv := &sequentialUninstallProvisioner{ + mu: &mu, + sequence: &sequence, + } + + factory := func(_ ResolvedConfig) WIFProvisioner { return sequentialProv } + manifest := testManifest(repos...) + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: manifest, + Repos: repos, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + for _, r := range results { + if !r.Success { + t.Errorf("%s/%s: Success = false; Error = %v", r.Owner, r.Repo, r.Error) + } + } + + mu.Lock() + defer mu.Unlock() + if len(sequence) != len(repos) { + t.Errorf("WIF calls = %d, want %d", len(sequence), len(repos)) + } +} + +func TestUninstall_WIFFailure_DoesNotAffectOtherRepos(t *testing.T) { + client := newInstalledFakeClient("acme/api", "acme/web") + prov := &uninstallFakeProvisioner{ + deleteErrs: map[string]error{ + "acme/api": fmt.Errorf("mint deregistration failed"), + }, + } + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + manifest := testManifest("acme/api", "acme/web") + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: manifest, + Repos: []string{"acme/api", "acme/web"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + + var apiResult, webResult UninstallResult + for _, r := range results { + switch r.Owner + "/" + r.Repo { + case "acme/api": + apiResult = r + case "acme/web": + webResult = r + } + } + + if apiResult.Success { + t.Error("acme/api: Success = true, want false (WIF failed)") + } + if apiResult.WIFDeregistered { + t.Error("acme/api: WIFDeregistered = true, want false") + } + if !apiResult.WorkflowDeleted { + t.Error("acme/api: WorkflowDeleted = false, want true") + } + if !webResult.Success { + t.Errorf("acme/web: Success = false; Error = %v", webResult.Error) + } + if !webResult.WIFDeregistered { + t.Error("acme/web: WIFDeregistered = false, want true") + } +} + +func TestUninstall_EmptyRepos(t *testing.T) { + _, err := Uninstall(context.Background(), UninstallConfig{ + MaxConcurrency: 4, + }, forge.NewFakeClient(), nil, nil) + + if err == nil { + t.Fatal("Uninstall() error = nil, want error for empty repos") + } +} + +func TestUninstall_InvalidRepoFormat(t *testing.T) { + _, err := Uninstall(context.Background(), UninstallConfig{ + Repos: []string{"just-a-name"}, + MaxConcurrency: 4, + }, forge.NewFakeClient(), nil, nil) + + if err == nil { + t.Fatal("Uninstall() error = nil, want error for invalid repo format") + } +} + +func TestUninstall_InvalidConcurrency(t *testing.T) { + _, err := Uninstall(context.Background(), UninstallConfig{ + Repos: []string{"acme/api"}, + MaxConcurrency: 0, + }, forge.NewFakeClient(), nil, nil) + + if err == nil { + t.Fatal("Uninstall() error = nil, want error for invalid concurrency") + } +} + +func TestUninstall_NoManifest_SkipsWIF(t *testing.T) { + client := newInstalledFakeClient("acme/api") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if !r.Success { + t.Errorf("Success = false; Error = %v", r.Error) + } + if r.WIFDeregistered { + t.Error("WIFDeregistered = true, want false (no manifest)") + } + if len(prov.deleteCalls) != 0 { + t.Errorf("DeletePerRepoWIF calls = %d, want 0", len(prov.deleteCalls)) + } +} + +func TestUninstall_RepoNotInManifest_SkipsWIF(t *testing.T) { + client := newInstalledFakeClient("acme/api") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + manifest := testManifest("acme/other") + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: manifest, + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if !r.Success { + t.Errorf("Success = false; Error = %v", r.Error) + } + if r.WIFDeregistered { + t.Error("WIFDeregistered = true, want false (not in manifest)") + } +} + +func TestUninstall_VariableDeleteError(t *testing.T) { + client := newInstalledFakeClient("acme/api") + client.Errors["DeleteRepoVariable"] = fmt.Errorf("permission denied") + + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if r.Success { + t.Error("Success = true, want false (variable deletion failed)") + } + if !r.WorkflowDeleted { + t.Error("WorkflowDeleted = false, want true") + } + if len(prov.deleteCalls) != 0 { + t.Errorf("DeletePerRepoWIF calls = %d, want 0", len(prov.deleteCalls)) + } +} + +func TestUninstall_SecretDeleteError(t *testing.T) { + client := newInstalledFakeClient("acme/api") + client.Errors["DeleteRepoSecret"] = fmt.Errorf("permission denied") + + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + results, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + r := results[0] + if r.Success { + t.Error("Success = true, want false (secret deletion failed)") + } + if !r.WorkflowDeleted { + t.Error("WorkflowDeleted = false, want true") + } +} + +func TestUninstall_ProgressCallbacks(t *testing.T) { + client := newInstalledFakeClient("acme/api") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + + var mu sync.Mutex + var phases []string + progress := func(_, phase, _ string) { + mu.Lock() + defer mu.Unlock() + phases = append(phases, phase) + } + + _, err := Uninstall(context.Background(), UninstallConfig{ + Manifest: testManifest("acme/api"), + Repos: []string{"acme/api"}, + MaxConcurrency: 4, + }, client, factory, progress) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + + mu.Lock() + defer mu.Unlock() + if len(phases) == 0 { + t.Error("no progress callbacks received") + } + + hasWorkflow, hasDone, hasWIF := false, false, false + for _, p := range phases { + switch p { + case "workflow": + hasWorkflow = true + case "done": + hasDone = true + case "wif": + hasWIF = true + } + } + if !hasWorkflow { + t.Error("missing 'workflow' phase callback") + } + if !hasDone { + t.Error("missing 'done' phase callback") + } + if !hasWIF { + t.Error("missing 'wif' phase callback") + } +} + +func TestUninstall_ContextCancelled_SkipsWIF(t *testing.T) { + client := newInstalledFakeClient("acme/api", "acme/web") + prov := &uninstallFakeProvisioner{} + factory := func(_ ResolvedConfig) WIFProvisioner { return prov } + manifest := testManifest("acme/api", "acme/web") + + ctx, cancel := context.WithCancel(context.Background()) + + results, err := Uninstall(ctx, UninstallConfig{ + Manifest: manifest, + Repos: []string{"acme/api", "acme/web"}, + MaxConcurrency: 1, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + for _, r := range results { + if !r.WorkflowDeleted { + t.Errorf("%s/%s: WorkflowDeleted = false", r.Owner, r.Repo) + } + } + + cancel() + + client2 := newInstalledFakeClient("acme/api2") + prov2 := &uninstallFakeProvisioner{} + factory2 := func(_ ResolvedConfig) WIFProvisioner { return prov2 } + + results2, err := Uninstall(ctx, UninstallConfig{ + Manifest: testManifest("acme/api2"), + Repos: []string{"acme/api2"}, + MaxConcurrency: 4, + }, client2, factory2, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + _ = results2 + if len(prov2.deleteCalls) != 0 { + t.Errorf("WIF calls after cancellation = %d, want 0", len(prov2.deleteCalls)) + } +} + +func TestUninstall_ContextCancelledDuringPhase2_MarksRemaining(t *testing.T) { + client := newInstalledFakeClient("acme/api", "acme/web") + manifest := testManifest("acme/api", "acme/web") + + ctx, cancel := context.WithCancel(context.Background()) + prov := &uninstallFakeProvisioner{} + cancellingProv := &cancellingDeleteProvisioner{cancel: cancel, inner: prov} + factory := func(_ ResolvedConfig) WIFProvisioner { return cancellingProv } + + results, err := Uninstall(ctx, UninstallConfig{ + Manifest: manifest, + Repos: []string{"acme/api", "acme/web"}, + MaxConcurrency: 1, + }, client, factory, nil) + + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + if len(results) != 2 { + t.Fatalf("got %d results, want 2", len(results)) + } + + var successCount int + for _, r := range results { + if r.Success { + successCount++ + } + } + if successCount > 1 { + t.Errorf("at most 1 repo should succeed when context is cancelled during WIF cleanup, got %d", successCount) + } + + var errCount int + for _, r := range results { + if r.Error != nil { + errCount++ + } + } + if errCount == 0 { + t.Error("expected at least one repo to have an error from cancelled context") + } +} + +type cancellingDeleteProvisioner struct { + cancel context.CancelFunc + inner *uninstallFakeProvisioner + called bool +} + +func (p *cancellingDeleteProvisioner) DiscoverMint(ctx context.Context) (*MintDiscovery, error) { + return p.inner.DiscoverMint(ctx) +} + +func (p *cancellingDeleteProvisioner) ProvisionWIF(ctx context.Context) (string, error) { + return p.inner.ProvisionWIF(ctx) +} + +func (p *cancellingDeleteProvisioner) RegisterPerRepoWIF(ctx context.Context, repo string) error { + return p.inner.RegisterPerRepoWIF(ctx, repo) +} + +func (p *cancellingDeleteProvisioner) EnsureOrgInMint(ctx context.Context, owner, project string) error { + return p.inner.EnsureOrgInMint(ctx, owner, project) +} + +func (p *cancellingDeleteProvisioner) DeletePerRepoWIF(ctx context.Context, repo string) error { + if !p.called { + p.called = true + p.cancel() + } + return p.inner.DeletePerRepoWIF(ctx, repo) +} + +func (p *cancellingDeleteProvisioner) DeleteWIFProvider(ctx context.Context, repo string) error { + return p.inner.DeleteWIFProvider(ctx, repo) +}