feat(repos): add repos install command for bulk per-repo installation

[ggallen]

Summary

• Implements PR 5 from the repos management plan (ADR 0057)
• Adds fullsend repos install CLI command for bulk per-repo installation from a repos.yaml manifest
• Three-phase execution: parallel discovery → sequential WIF provisioning → parallel scaffold/config writes

Changes

• internal/repos/batch_install.go — BatchInstall() function with three-phase execution, ProvisionerFactory type, filterRepos() helper
• internal/repos/batch_install_test.go — 16 test cases covering: fresh install, partial install, repo filtering, dry-run, WIF serialization verification, org mint failure, WIF provisioning failure, per-repo registration failure, empty manifest, invalid manifest, multi-org, TOCTOU re-check, scaffold failure, nil progress, default concurrency, case-insensitive filter
• internal/cli/repos.go — repos subcommand group with repos install subcommand, flags: --manifest, --dry-run, --repo, --concurrency, --direct, --roles, etc.
• internal/cli/root.go — Wire newReposCmd() into root

Testing

• go test ./internal/repos/ — all tests pass
• Coverage: 92.1% overall, 94.7% on batch_install.go
• go vet clean
• make lint passes

Related Issue

ADR 0057 — repos management for per-repo installations

Checklist

• Follows conventional commits
• Tests added with >85% coverage
• Lint and vet pass
• No secrets or sensitive data

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 1:01 AM UTC · Ended 1:17 AM UTC
Commit: 0a95cac · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Add repos install command for bulk per-repo installations

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Add fullsend repos install to bulk-install per-repo scaffolds from repos.yaml.
• Implement a three-phase batch installer with filtering, concurrency, and dry-run support.
• Add unit tests covering partial installs, WIF failures, and manifest edge cases.

Diagram

graph TD
  cli["CLI: repos install"] -->|"run"| batch["repos.BatchInstall()"] -->|"phase1 discover"| gh{{"GitHub API"}}
  batch -->|"phase2 WIF"| wif["WIF provisioner"] -->|"org + IAM"| mint{{"Mint/GCP"}}
  batch -->|"phase3"| install["Install() per repo"] -->|"scaffold/vars"| repo["Repo changes"]
  wif -->|"provider"| install
  repo -->|"via"| gh

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Fully sequential per-repo install loop

• ➕ Simplest implementation and easiest to reason about
• ➕ Avoids any concurrency-related rate limiting or ordering issues
• ➖ Significantly slower for large manifests
• ➖ Does not leverage safe parallelism in discovery/scaffold phases

2. Parallel WIF with per-org/project locks

• ➕ Potentially faster than globally-serialized WIF provisioning
• ➕ Still prevents concurrent mutation of shared state per lock scope
• ➖ Higher complexity: requires correct lock granularity and deadlock avoidance
• ➖ Harder to test and validate against real GCP/Mint behavior

3. Single worker-pool pipeline with explicit stage barriers

• ➕ More uniform orchestration model (stages + barrier sync)
• ➕ Could centralize error handling/aggregation
• ➖ More abstraction than needed at current scope
• ➖ Harder for operators to correlate progress to phases

Recommendation: Keep the PR’s three-phase approach. It captures the key safety constraint (WIF provisioning is not concurrency-safe) while still parallelizing the phases that are safe and dominant in wall-clock time (discovery and scaffold/vars). The main alternative worth revisiting later is per-org/project locking for WIF if provisioning becomes a bottleneck, but it increases complexity and risk.

Files changed (4) +1358 / -0

Enhancement (3) +546 / -0

repos.goAdd 'repos install' Cobra command and options/flag plumbing +191/-0

Add 'repos install' Cobra command and options/flag plumbing

• Introduces the 'repos' command group with an 'install' subcommand. Loads a manifest, constructs a GitHub forge client and a per-repo WIF provisioner factory, and calls 'repos.BatchInstall()' with progress reporting and a scaffold commit function.

internal/cli/repos.go

root.goWire 'repos' command group into the root CLI +1/-0

Wire 'repos' command group into the root CLI

• Registers 'newReposCmd()' so the new 'fullsend repos ...' commands are available from the root command.

internal/cli/root.go

batch_install.goImplement three-phase BatchInstall orchestrator for multi-repo installs +354/-0

Implement three-phase BatchInstall orchestrator for multi-repo installs

• Adds 'BatchInstall()' with (1) parallel discovery using guard variables, (2) sequential org+repo WIF provisioning with TOCTOU re-checks, and (3) parallel per-repo 'Install()' to write scaffold/vars/secrets. Includes configurable concurrency, repo filtering, dry-run behavior, and a provisioner factory abstraction.

internal/repos/batch_install.go

Tests (1) +812 / -0

batch_install_test.goAdd comprehensive BatchInstall test coverage for success and failure modes +812/-0

Add comprehensive BatchInstall test coverage for success and failure modes

• Adds a large test suite with fake clients/provisioners validating concurrency behavior, WIF serialization, error partitioning, dry-run non-writing guarantees, filtering, multi-org handling, and edge cases like invalid/empty manifests.

internal/repos/batch_install_test.go

[github-actions]

Site preview

Preview: https://a001ebcb-site.fullsend-ai.workers.dev

Commit: 635722266a89f6168864caaea9361447ae93f8de

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

1. Vendored install breaks workflows ✓ Resolved 🐞 Bug ≡ Correctness

Description

When VendorBinary is enabled, the per-repo shim workflow is rendered to reference local reusable
workflows (e.g. ./.github/workflows/reusable-dispatch.yml), but the batch install path only
commits the shim + config and never writes those local reusable workflow files, causing installed
repos’ Actions runs to fail at runtime.

Code

internal/repos/batch_install.go[R293-308]

+			installCfg := InstallConfig{
+				Owner:            dr.repo.Owner,
+				Repo:             dr.repo.Repo,
+				Roles:            roles,
+				MintURL:          dr.resolved.MintURL,
+				InferenceProject: dr.resolved.InferenceProject,
+				InferenceRegion:  dr.resolved.InferenceRegion,
+				UpstreamRef:      ref,
+				UpstreamTag:      tag,
+				SkipGuardCheck:   true,
+				SkipMintCheck:    true,
+				SkipWIF:          true,
+				WIFProvider:      providerName,
+				VendorBinary:     cfg.VendorBinary,
+				Direct:           cfg.Direct,
+			}

Relevance

⭐⭐⭐ High

Team prioritizes vendored install correctness; reusable workflow vendoring path fixes accepted (PR
#2545, #1954).

PR-#2545
PR-#1954

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

VendorBinary triggers vendored template rendering that points at local reusable workflows, but the
per-repo scaffold collector used by Install() only returns the shim workflow (not the reusable
workflows). The existing per-repo setup path demonstrates vendored installs require appending
vendored assets separately, which the batch path does not do.

internal/repos/batch_install.go[268-310]
internal/scaffold/render.go[102-124]
internal/scaffold/installfiles.go[60-88]
internal/repos/install.go[289-322]
internal/cli/github.go[225-306]
PR-#1011

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`BatchInstall` passes `cfg.VendorBinary` into `InstallConfig.VendorBinary`, which causes scaffold templates to switch to *local* reusable workflow references. However, the install flow only commits the shim workflow + `.fullsend/config.yaml` and does not include vendored reusable workflow files, so workflows will reference files that do not exist.

## Issue Context
The existing per-repo setup flow vendors assets by explicitly appending vendored files (binary + reusable workflows + other assets). The new bulk install command must do the same (or remove/disable the flag).

## Fix Focus Areas
- internal/repos/batch_install.go[268-333]
- internal/repos/install.go[289-322]
- internal/scaffold/render.go[102-124]
- internal/scaffold/installfiles.go[60-88]
- internal/cli/repos.go[64-142]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Parallel stdin prompts hang ✓ Resolved 🐞 Bug ☼ Reliability

Description

The batch command passes os.Stdin into layers.CommitScaffoldFiles, but Phase 3 runs scaffold
commits in parallel; when fork selection prompts occur, multiple goroutines can read from stdin
concurrently and block or interleave input, hanging the bulk install.

Code

internal/cli/repos.go[R116-127]

+	scaffoldCommitFn := func(ctx context.Context, owner, repo string, files []forge.TreeFile, direct bool) error {
+		targetRepo, repoErr := client.GetRepo(ctx, owner, repo)
+		if repoErr != nil {
+			return fmt.Errorf("getting repo info: %w", repoErr)
+		}
+		commitMsg := "chore: initialize fullsend per-repo installation"
+		prTitle := "chore: initialize fullsend per-repo installation"
+		prBody := "This PR adds the fullsend scaffold files for per-repo installation.\n\n" +
+			"Merge this PR to activate fullsend workflows."
+		_, commitErr := layers.CommitScaffoldFiles(ctx, client, printer, owner, repo,
+			targetRepo.DefaultBranch, commitMsg, prTitle, prBody, files, direct, os.Stdin)
+		return commitErr

Relevance

⭐⭐ Medium

Mixed evidence: team added non-interactive flags but rejected stdin hang/terminal-guard suggestion
in PR #698.

PR-#698
PR-#290

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new command always supplies os.Stdin to scaffold commit logic, while BatchInstall runs Phase
3 installs concurrently. CommitScaffoldFiles prompts when in != nil, so multiple goroutines can
concurrently prompt/read from the same stdin stream.

internal/cli/repos.go[116-127]
internal/repos/batch_install.go[268-310]
internal/layers/commit.go[82-92]
internal/layers/commit.go[286-315]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`repos install` runs scaffold commits in parallel but passes `os.Stdin` into `layers.CommitScaffoldFiles`, which can prompt for fork-vs-upstream choice when the authenticated user is not the repo owner. Concurrent prompts/readers on stdin can hang or behave nondeterministically.

## Issue Context
`layers.CommitScaffoldFiles` is explicitly interactive when `in != nil`. Bulk operations should be non-interactive (pass `nil`) or serialize scaffold commits when prompting is possible.

## Fix Focus Areas
- internal/cli/repos.go[116-127]
- internal/repos/batch_install.go[268-331]
- internal/layers/commit.go[82-92]
- internal/layers/commit.go[286-315]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Skip mint flag ignored ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

--skip-mint-check is parsed into BatchInstallConfig.SkipMintCheck but BatchInstall never
consults it and still performs org mint registration/WIF steps, while also forcing
InstallConfig.SkipMintCheck=true, making the flag ineffective/misleading.

Code

internal/repos/batch_install.go[R300-305]

+				UpstreamRef:      ref,
+				UpstreamTag:      tag,
+				SkipGuardCheck:   true,
+				SkipMintCheck:    true,
+				SkipWIF:          true,
+				WIFProvider:      providerName,

Relevance

⭐⭐⭐ High

--skip-mint-check is an established, accepted behavior (PR #991); ignored flag would likely be
fixed.

PR-#991

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The command defines and forwards SkipMintCheck, but the batch installer still runs
EnsureOrgInMint/WIF provisioning and always sets SkipMintCheck: true in the per-repo
InstallConfig, so the flag cannot affect the execution path.

internal/cli/repos.go[64-72]
internal/cli/repos.go[130-142]
internal/repos/batch_install.go[168-189]
internal/repos/batch_install.go[293-305]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CLI exposes `--skip-mint-check`, but the batch install implementation doesn't use it to change behavior and unconditionally performs mint/WIF operations; additionally it always forces `InstallConfig.SkipMintCheck=true`.

## Issue Context
A user-facing flag that does nothing is confusing and can lead to unexpected failures (e.g., mint unreachable still fails despite `--skip-mint-check`). Either implement meaningful semantics for the flag in `BatchInstall` (and document what is skipped), or remove the flag from `repos install`.

## Fix Focus Areas
- internal/cli/repos.go[64-72]
- internal/cli/repos.go[130-142]
- internal/repos/batch_install.go[168-189]
- internal/repos/batch_install.go[293-305]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 1:17 AM UTC · Ended 1:21 AM UTC
Commit: 0a95cac · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure · Started 1:22 AM UTC · Completed 1:35 AM UTC
Commit: a8f31d5 · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 84.67742% with 57 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/repos.go	73.55%	27 Missing and 5 partials ⚠️
internal/repos/batch_install.go	90.03%	20 Missing and 5 partials ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

The rebase from c0d8e98 onto updated main (incorporating PR #3032's repos init merge) is clean. The code is unchanged from the prior approved review — the only delta is the updated base commit.

The root.go change correctly removes the duplicate newReposCmd() registration that existed on main at lines 51 and 55 (introduced when PR #3032 merged while this PR was in flight). After this PR, a single registration remains at line 51.

All prior-review resolutions remain intact:

• splitProjectAdapter correctly splits WIF provisioning (inference project) from mint operations (mint project)
• --roles flag defaults to config.PerRepoDefaultRoles() (6-role canonical list)
• Phase 3 fallback uses config.PerRepoDefaultRoles() instead of hardcoded slice
• Inference project/region validation in Phase 2 fails individual repos without aborting the batch
• checkPerRepoScopes is called before batch install operations
• Phase 2 sequential WIF loop checks ctx.Err() at the top of each iteration
• Test suite is comprehensive (25+ test cases, 92%+ coverage) with thread-safe test doubles
• Documentation covers CLI reference, CLI tree, operations table, and ADR 0057 implementation status

Findings

No findings.

Previous run

Review

The sole remaining finding from the prior review at SHA c90f40e has been addressed:

• [test-adequacy] RESOLVED: TestSplitProjectAdapter_MethodRouting in internal/cli/repos_test.go now verifies that each splitProjectAdapter method routes to the correct underlying provisioner — DiscoverMint, RegisterPerRepoWIF, EnsureOrgInMint, and DeletePerRepoWIF route to the mint provisioner, while ProvisionWIF routes to the inference provisioner. The test uses a trackingProvisioner that records calls and returns a distinguishing label, providing clear verification of correct routing.

The rebase onto the updated main branch (incorporating PR #3032's repos init merge) is clean. The root.go change correctly removes the duplicate newReposCmd() registration (lines 51 and 55 on main both called newReposCmd()) — leaving a single registration at line 51.

All prior-review resolutions remain intact:

• splitProjectAdapter correctly splits WIF provisioning (inference project) from mint operations (mint project)
• --roles flag defaults to config.PerRepoDefaultRoles() (6-role canonical list)
• Phase 3 fallback uses config.PerRepoDefaultRoles() instead of hardcoded slice
• Inference project/region validation in Phase 2 fails individual repos without aborting the batch
• checkPerRepoScopes is called before batch install operations
• Documentation is comprehensive across CLI reference, CLI tree, operations table, and ADR 0057 implementation status annotation

Findings

No findings.

Previous run

Review

The sole remaining finding from the prior review at SHA c90f40e has been addressed:

• [test-adequacy] RESOLVED: TestSplitProjectAdapter_MethodRouting in internal/cli/repos_test.go now verifies that each splitProjectAdapter method routes to the correct underlying provisioner — DiscoverMint, RegisterPerRepoWIF, EnsureOrgInMint, and DeletePerRepoWIF route to the mint provisioner, while ProvisionWIF routes to the inference provisioner. The test uses a trackingProvisioner that records calls and returns a distinguishing label, providing clear verification of correct routing.

The rebase onto the updated main branch (incorporating PR #3032's repos init merge) is clean. The root.go change correctly removes the duplicate newReposCmd() registration (lines 51 and 55 on main both called newReposCmd()) — leaving a single registration at line 51.

All prior-review resolutions remain intact:

• splitProjectAdapter correctly splits WIF provisioning (inference project) from mint operations (mint project)
• --roles flag defaults to config.PerRepoDefaultRoles() (6-role canonical list)
• Phase 3 fallback uses config.PerRepoDefaultRoles() instead of hardcoded slice
• Inference project/region validation in Phase 2 fails individual repos without aborting the batch
• checkPerRepoScopes is called before batch install operations
• Documentation is comprehensive across CLI reference, CLI tree, operations table, and ADR 0057 implementation status annotation

Findings

No findings.

Previous run (2)

Review

All three prior findings from the review at SHA 1a7a60a have been addressed:

• [logic-error] RESOLVED: The provisioner factory now creates a splitProjectAdapter with two underlying provisioners — mintProv (using resolved.MintProject) for DiscoverMint/EnsureOrgInMint/RegisterPerRepoWIF/DeletePerRepoWIF, and wifProv (using resolved.InferenceProject) for ProvisionWIF. This matches the admin.go pattern where WIF provisioning uses inferenceProject (line 998) while mint operations use the mint project.
• [API-contract-divergence] RESOLVED: The --roles flag default now uses config.PerRepoDefaultRoles() (line 231), producing the full 6-role list (triage,coder,review,fix,retro,prioritize) consistent with admin.go and github.go.
• [hardcoded-fallback] RESOLVED: The Phase 3 fallback in batch_install.go now uses config.PerRepoDefaultRoles() instead of a hardcoded 3-element slice.

The new inference project/region validation in Phase 2 (lines ~225–250 of batch_install.go) correctly operates per-repo — repos with missing InferenceProject or InferenceRegion are individually moved to the Failed list without aborting the batch for other repos. The TestBatchInstall_MissingInferenceProject and TestBatchInstall_MissingInferenceRegion tests verify this behavior.

The root.go change correctly removes the duplicate newReposCmd() registration that resulted from the repos init PR (#3032) merge, leaving a single registration at the correct position.

Findings

Low

• [test-adequacy] internal/cli/repos.go — The splitProjectAdapter is the critical fix for the prior high-severity finding (correct project routing for WIF vs mint operations), but it has no unit test verifying that each method routes to the correct underlying provisioner. The routing logic is trivially correct by inspection (each method is a one-line delegation), but a table-driven test constructing a splitProjectAdapter with two distinct mock provisioners and asserting each method dispatches to the expected one would guard against accidental field swaps in future refactors.

Previous run (3)

Review

All three prior findings from the review at SHA 1a7a60a have been addressed:

• [logic-error] RESOLVED: The provisioner factory now creates a splitProjectAdapter with two underlying provisioners — mintProv (using resolved.MintProject) for DiscoverMint/EnsureOrgInMint/RegisterPerRepoWIF/DeletePerRepoWIF, and wifProv (using resolved.InferenceProject) for ProvisionWIF. This matches the admin.go pattern where WIF provisioning uses inferenceProject (line 998) while mint operations use the mint project.
• [API-contract-divergence] RESOLVED: The --roles flag default now uses config.PerRepoDefaultRoles() (line 231), producing the full 6-role list (triage,coder,review,fix,retro,prioritize) consistent with admin.go and github.go.
• [hardcoded-fallback] RESOLVED: The Phase 3 fallback in batch_install.go now uses config.PerRepoDefaultRoles() instead of a hardcoded 3-element slice.

The new inference project/region validation in Phase 2 (lines ~225–250 of batch_install.go) correctly operates per-repo — repos with missing InferenceProject or InferenceRegion are individually moved to the Failed list without aborting the batch for other repos. The TestBatchInstall_MissingInferenceProject and TestBatchInstall_MissingInferenceRegion tests verify this behavior.

The root.go change correctly removes the duplicate newReposCmd() registration that resulted from the repos init PR (#3032) merge, leaving a single registration at the correct position.

Findings

Low

• [test-adequacy] internal/cli/repos.go — The splitProjectAdapter is the critical fix for the prior high-severity finding (correct project routing for WIF vs mint operations), but it has no unit test verifying that each method routes to the correct underlying provisioner. The routing logic is trivially correct by inspection (each method is a one-line delegation), but a table-driven test constructing a splitProjectAdapter with two distinct mock provisioners and asserting each method dispatches to the expected one would guard against accidental field swaps in future refactors.

Previous run (4)

Review

All four prior findings from the review at SHA 7b31ab8 have been addressed: the --roles default in cli-internals.md now correctly reads triage,coder,review, the BatchInstallConfig.Roles doc comment uses "coder", the implementation plan was updated to match the actual struct, and flag descriptions use consistent lowercase.

However, the new commit introduces a high-severity issue: the provisioner factory in repos.go creates WIF provisioners using resolved.MintProject as the GCP ProjectID, but ProvisionWIF creates IAM resources (service account, WIF pool, WIF provider, principal binding) that must reside in the inference project. The existing admin.go per-repo install path correctly uses inferenceProject for this operation. When MintProject ≠ InferenceProject (a supported configuration — the manifest has separate fields for each), WIF infrastructure is created in the wrong GCP project, causing GitHub Actions OIDC auth to obtain credentials scoped to the mint project instead of the inference project.

Additionally, the --roles flag default diverges from the canonical config.PerRepoDefaultRoles(), which could cause configuration drift between repos installed via repos install vs admin install.

Findings

High

• [logic-error] internal/cli/repos.go — The provisioner factory creates gcf.NewProvisioner with ProjectID: resolved.MintProject, but ProvisionWIF creates IAM infrastructure (service account, WIF pool/provider, principal binding) that must live in the inference project. In admin.go (line 998), the analogous WIF provisioner correctly uses inferenceProject. The RegisterPerRepoWIF and EnsureOrgInMint operations DO need the mint project (they modify the mint Cloud Function's env vars), but ProvisionWIF does not. When MintProject ≠ InferenceProject, WIF infrastructure is created in the wrong GCP project, and the resulting WIF provider path (projects/{mint-project-number}/...) stored in FULLSEND_GCP_WIF_PROVIDER will give GitHub Actions credentials for the mint project, not the inference project — causing inference calls to fail with GCP permission errors.
  Remediation: Split the provisioner into two: (1) a mint-project provisioner for EnsureOrgInMint and RegisterPerRepoWIF, and (2) an inference-project provisioner for ProvisionWIF, matching the admin.go pattern. In the CLI layer, create the WIF provisioner with resolved.InferenceProject as ProjectID and gcf.NewLiveGCFClient(resolved.InferenceProject).

Medium

• [API-contract-divergence] internal/cli/repos.go — The --roles flag default is ["triage", "coder", "review"] (3 roles), but config.PerRepoDefaultRoles() returns ["triage", "coder", "review", "fix", "retro", "prioritize"] (6 roles). Both admin.go (line 298) and github.go (line 115) use config.PerRepoDefaultRoles() for per-repo installs. A repo installed via fullsend repos install without explicit --roles will be missing the fix, retro, and prioritize agent workflows, creating silent configuration drift between installation methods.
  Remediation: Change the default to config.PerRepoDefaultRoles() instead of a hardcoded 3-element slice, or document the intentional difference and reasoning.

Low

• [hardcoded-fallback] internal/repos/batch_install.go — The Phase 3 fallback roles = []string{"triage", "coder", "review"} duplicates the CLI default without referencing config.PerRepoDefaultRoles(). Currently dead code (CLI always provides Roles via flag default), but perpetuates the 3-role divergence if BatchInstall is called programmatically with an empty Roles slice.

Previous run (5)

Review

All four prior findings from the review at SHA 7b31ab8 have been addressed: the --roles default in cli-internals.md now correctly reads triage,coder,review, the BatchInstallConfig.Roles doc comment uses "coder", the implementation plan was updated to match the actual struct, and flag descriptions use consistent lowercase.

However, the new commit introduces a high-severity issue: the provisioner factory in repos.go creates WIF provisioners using resolved.MintProject as the GCP ProjectID, but ProvisionWIF creates IAM resources (service account, WIF pool, WIF provider, principal binding) that must reside in the inference project. The existing admin.go per-repo install path correctly uses inferenceProject for this operation. When MintProject ≠ InferenceProject (a supported configuration — the manifest has separate fields for each), WIF infrastructure is created in the wrong GCP project, causing GitHub Actions OIDC auth to obtain credentials scoped to the mint project instead of the inference project.

Additionally, the --roles flag default diverges from the canonical config.PerRepoDefaultRoles(), which could cause configuration drift between repos installed via repos install vs admin install.

Findings

High

• [logic-error] internal/cli/repos.go — The provisioner factory creates gcf.NewProvisioner with ProjectID: resolved.MintProject, but ProvisionWIF creates IAM infrastructure (service account, WIF pool/provider, principal binding) that must live in the inference project. In admin.go (line 998), the analogous WIF provisioner correctly uses inferenceProject. The RegisterPerRepoWIF and EnsureOrgInMint operations DO need the mint project (they modify the mint Cloud Function's env vars), but ProvisionWIF does not. When MintProject ≠ InferenceProject, WIF infrastructure is created in the wrong GCP project, and the resulting WIF provider path (projects/{mint-project-number}/...) stored in FULLSEND_GCP_WIF_PROVIDER will give GitHub Actions credentials for the mint project, not the inference project — causing inference calls to fail with GCP permission errors.
  Remediation: Split the provisioner into two: (1) a mint-project provisioner for EnsureOrgInMint and RegisterPerRepoWIF, and (2) an inference-project provisioner for ProvisionWIF, matching the admin.go pattern. In the CLI layer, create the WIF provisioner with resolved.InferenceProject as ProjectID and gcf.NewLiveGCFClient(resolved.InferenceProject).

Medium

• [API-contract-divergence] internal/cli/repos.go — The --roles flag default is ["triage", "coder", "review"] (3 roles), but config.PerRepoDefaultRoles() returns ["triage", "coder", "review", "fix", "retro", "prioritize"] (6 roles). Both admin.go (line 298) and github.go (line 115) use config.PerRepoDefaultRoles() for per-repo installs. A repo installed via fullsend repos install without explicit --roles will be missing the fix, retro, and prioritize agent workflows, creating silent configuration drift between installation methods.
  Remediation: Change the default to config.PerRepoDefaultRoles() instead of a hardcoded 3-element slice, or document the intentional difference and reasoning.

Low

• [hardcoded-fallback] internal/repos/batch_install.go — The Phase 3 fallback roles = []string{"triage", "coder", "review"} duplicates the CLI default without referencing config.PerRepoDefaultRoles(). Currently dead code (CLI always provides Roles via flag default), but perpetuates the 3-role divergence if BatchInstall is called programmatically with an empty Roles slice.

Previous run (6)

Review

Findings

Medium

• [logic-error] docs/guides/dev/cli-internals.md — The --roles flag default in the CLI tree diagram is shown as triage,code,review but the Go code in repos.go correctly uses []string{"triage", "coder", "review"} and docs/cli/repos.md also correctly shows triage,coder,review. The valid role name is coder, not code (config.ValidRoles() does not include code). A user reading this doc and passing --roles triage,code,review explicitly would get a validation error from PerRepoConfig.Validate().
  Remediation: Change triage,code,review to triage,coder,review in the --roles line of the CLI tree diagram.

Low

• [logic-error] internal/repos/batch_install.go:19 — The doc comment on BatchInstallConfig.Roles says (e.g., "triage", "code") but code is not a valid role — the correct name is coder. This is a comment-only issue copied from the pre-existing InstallConfig.Roles comment in install.go, but it perpetuates the code vs coder confusion that caused the prior review's critical finding.

• [scope-deviation] internal/repos/batch_install.go — BatchInstallConfig is missing the SkipAppSetup field specified in the implementation plan (docs/plans/repos-management.md). The omission appears intentional since the batch install delegates to Install() with separate skip flags, but the plan should be updated to reflect the actual implementation.

• [flag-description-capitalization] internal/cli/repos.go — Six flag descriptions in newReposInstallCmd() start with capital letters (e.g., "Path or URL to repos.yaml manifest"), while the existing repos status command in the same file uses lowercase (e.g., "path or HTTPS URL to manifest file"). Inconsistent within the same file.

Previous run (7)

Review

Findings

Medium

• [logic-error] docs/guides/dev/cli-internals.md — The --roles flag default in the CLI tree diagram is shown as triage,code,review but the Go code in repos.go correctly uses []string{"triage", "coder", "review"} and docs/cli/repos.md also correctly shows triage,coder,review. The valid role name is coder, not code (config.ValidRoles() does not include code). A user reading this doc and passing --roles triage,code,review explicitly would get a validation error from PerRepoConfig.Validate().
  Remediation: Change triage,code,review to triage,coder,review in the --roles line of the CLI tree diagram.

Low

• [logic-error] internal/repos/batch_install.go:19 — The doc comment on BatchInstallConfig.Roles says (e.g., "triage", "code") but code is not a valid role — the correct name is coder. This is a comment-only issue copied from the pre-existing InstallConfig.Roles comment in install.go, but it perpetuates the code vs coder confusion that caused the prior review's critical finding.

• [scope-deviation] internal/repos/batch_install.go — BatchInstallConfig is missing the SkipAppSetup field specified in the implementation plan (docs/plans/repos-management.md). The omission appears intentional since the batch install delegates to Install() with separate skip flags, but the plan should be updated to reflect the actual implementation.

• [flag-description-capitalization] internal/cli/repos.go — Six flag descriptions in newReposInstallCmd() start with capital letters (e.g., "Path or URL to repos.yaml manifest"), while the existing repos status command in the same file uses lowercase (e.g., "path or HTTPS URL to manifest file"). Inconsistent within the same file.

Previous run (8)

Review

Findings

Critical

• [logic-error] internal/cli/repos.go:201 — The default value for the --roles flag is ["triage", "code", "review"], but "code" is not a valid role. config.ValidRoles() returns ["fullsend", "triage", "coder", "review", "fix", "retro", "prioritize", "e2e"] — the correct role name is "coder", not "code". This causes PerRepoConfig.Validate() to reject the role during BuildScaffoldFiles() → Install(), making repos install fail by default unless the user explicitly overrides --roles. The existing tests all pass Roles: []string{"triage"} explicitly, masking this bug.
  Remediation: Change the default to []string{"triage", "coder", "review"}, or use config.PerRepoDefaultRoles() to stay in sync with the canonical role list (as the existing per-repo install in admin.go and github.go does).

Low

• [logic-error] internal/repos/batch_install.go:281 — The hardcoded fallback roles = []string{"triage", "code", "review"} uses the same invalid role name "code". This path is only reached when cfg.Roles is empty (unreachable through the CLI since the flag always provides a default), but would fail if BatchInstall were called programmatically with an empty Roles slice.

• [documentation-completeness] docs/cli/repos.md:54 — The --roles flag documentation shows the default value triage,code,review which contains the invalid role name. Beyond the typo, there is no explanation of what roles are or cross-reference to role documentation for users unfamiliar with fullsend's role model.

Previous run (9)

Review

Well-executed implementation of repos install (PR 5 from the repos management plan, ADR 0057). The three-phase architecture — parallel discovery → sequential WIF → parallel scaffold — correctly respects GCP read-modify-write constraints while maximizing throughput. All prior findings from the review at SHA 33f0801 remain addressed:

• [logic-error] RESOLVED: Dry-run runs Phase 1 (guard variable discovery) before the early-return path, correctly reporting which repos are already installed vs. which would be newly installed.
• [stale-doc] RESOLVED: docs/guides/dev/cli-internals.md includes the repos subcommand tree with all flags.
• [test-adequacy] RESOLVED: Phase 2's sequential WIF loop checks ctx.Err() at the top of each iteration, and TestBatchInstall_ContextCancellation_Phase2 validates cancellation propagation via a cancellingProvisioner.
• [dead-code] RESOLVED: The guardCheckCount variable has been removed.
• [race-condition] RESOLVED: fakeWIFProvisioner and fakeScaffoldCommit in install_test.go use sync.Mutex for thread-safe call recording.

The test suite is comprehensive (25+ test cases, reported 92%+ coverage). TestBatchInstall_ConcurrencyCap properly validates the semaphore bound with 8 repos at MaxConcurrency 2, tracking peak active goroutines via atomic counters. The orgRepresentative map correctly routes each org to its own resolved config for EnsureOrgInMint. CLI-level validation rejects --concurrency values outside 1-32, and BatchInstall returns an error for invalid values rather than silently clamping.

Documentation is thorough: docs/cli/repos.md covers flags, common workflows, and the --skip-mint-check behavior note. The docs/cli/README.md and website/.vitepress/config.ts sidebar already reference the repos command group (merged via prior PRs). The ADR 0057 modification (adding an "Implementation status" section) is a minor cross-reference annotation, consistent with the repo's ADR immutability policy.

The website/.vitepress/config.ts reformatting that was previously bundled with this PR has been removed, addressing the prior review's scope-creep feedback.

Findings

Low

• [scope-creep] internal/repos/batch_install.go:194 — Phase 2 EnsureOrgInMint calls auto-register orgs in shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and the behavior is documented in docs/cli/repos.md with a Note callout. This is intentional and authorized by ADR 0057; consider whether an opt-in model (--register-orgs) would be preferable in a follow-up.

Previous run (10)

Review

Well-executed implementation of repos install (PR 5 from the repos management plan, ADR 0057). The three-phase architecture — parallel discovery → sequential WIF → parallel scaffold — correctly respects GCP read-modify-write constraints while maximizing throughput. All prior findings from the review at SHA 0f73306 have been addressed:

• [logic-error] RESOLVED: Dry-run now runs Phase 1 (guard variable discovery) before the early-return path, so --dry-run correctly reports which repos are already installed vs. which would be newly installed.
• [stale-doc] RESOLVED: docs/guides/dev/cli-internals.md now includes the repos subcommand tree with all flags.
• [test-adequacy] RESOLVED: Phase 2's sequential WIF loop now checks ctx.Err() at the top of each iteration (line 567), and TestBatchInstall_ContextCancellation_Phase2 validates cancellation propagation via a cancellingProvisioner that cancels the context after the first ProvisionWIF call.
• [dead-code] RESOLVED: The guardCheckCount variable has been removed.
• [race-condition] RESOLVED: fakeWIFProvisioner and fakeScaffoldCommit in install_test.go now use sync.Mutex for thread-safe call recording.

The test suite is comprehensive (25+ test cases, reported 92%+ coverage). TestBatchInstall_ConcurrencyCap properly validates the semaphore bound with 8 repos at MaxConcurrency 2, tracking peak active goroutines via atomic counters. The orgRepresentative map correctly routes each org to its own resolved config for EnsureOrgInMint. CLI-level validation rejects --concurrency values outside 1-32, and BatchInstall returns an error for invalid values rather than silently clamping.

Documentation is thorough: docs/cli/repos.md covers flags, common workflows, and the --skip-mint-check behavior note. The ADR 0057 modification (adding an "Implementation status" section) is a minor cross-reference annotation, consistent with the repo's ADR immutability policy. The config.ts reformatting (single→double quotes) aligns with .prettierrc (singleQuote: false), correcting prior format drift.

Findings

Low

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 EnsureOrgInMint calls auto-register orgs in shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and the behavior is documented in docs/cli/repos.md with a Note callout. This is intentional and authorized by ADR 0057; consider whether an opt-in model (--register-orgs) would be preferable in a follow-up.

• [scope-creep] website/.vitepress/config.ts — The functional change (adding the fullsend repos sidebar entry) is bundled with a whole-file reformatting (single→double quotes, ~450 lines). The reformatting aligns with the project's .prettierrc (singleQuote: false), correcting prior format drift rather than introducing arbitrary churn. However, it inflates the diff and creates merge conflict risk for concurrent PRs.

---

Labels: PR adds a new CLI repos install command with Go code in internal/cli/ and internal/repos/, implementing a feature from ADR 0057.

Previous run (11)

Review

Findings

Medium

• [logic-error] internal/repos/batch_install.go:86 — The dry-run path skips Phase 1 (guard variable discovery), so it reports all manifest repos as "Would install" — including repos that are already installed. A non-dry-run invocation would skip those repos. The --dry-run flag is documented as "Preview what would be installed without making changes," but the preview does not accurately reflect the skip/install partition. Run Phase 1 discovery even in dry-run mode so the output matches what a real run would do.

• [stale-doc] docs/guides/dev/cli-internals.md — The CLI command tree reference does not include the newly added fullsend repos subcommand group. This developer reference document lists all CLI commands; the missing entry makes the reference incomplete for developers navigating the CLI structure.

Low

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ContextCancellation_Phase1 validates context cancellation in Phase 1 using a contextAwareClient, addressing the prior review finding. However, Phase 2's sequential WIF loop does not check ctx.Err() between iterations, so a cancelled context won't short-circuit the loop — it will attempt each remaining repo, only to fail at the first context-dependent call. Consider adding a ctx.Err() check at the top of the WIF loop and a corresponding test.

• [dead-code] internal/repos/batch_install_test.go:615 — Variable guardCheckCount is declared and immediately suppressed with _ = guardCheckCount. This appears to be leftover from an earlier version of the TOCTOU re-check test.

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 EnsureOrgInMint calls auto-register orgs in shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and the behavior is documented in docs/cli/repos.md with a Note callout. This is intentional and authorized by the implementation plan; consider whether an opt-in model (--register-orgs) would be preferable in a follow-up.

• [scope-creep] website/.vitepress/config.ts — The functional change (adding the fullsend repos sidebar entry) is bundled with a whole-file reformatting (single→double quotes, ~800 lines). The reformatting aligns with the project's .prettierrc (singleQuote: false), correcting prior format drift rather than introducing arbitrary churn. However, it inflates the diff and creates merge conflict risk for concurrent PRs.

Previous run (12)

Review

Findings

Medium

• [logic-error] internal/repos/batch_install.go:86 — The dry-run path skips Phase 1 (guard variable discovery), so it reports all manifest repos as "Would install" — including repos that are already installed. A non-dry-run invocation would skip those repos. The --dry-run flag is documented as "Preview what would be installed without making changes," but the preview does not accurately reflect the skip/install partition. Run Phase 1 discovery even in dry-run mode so the output matches what a real run would do.

• [stale-doc] docs/guides/dev/cli-internals.md — The CLI command tree reference does not include the newly added fullsend repos subcommand group. This developer reference document lists all CLI commands; the missing entry makes the reference incomplete for developers navigating the CLI structure.

Low

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ContextCancellation_Phase1 validates context cancellation in Phase 1 using a contextAwareClient, addressing the prior review finding. However, Phase 2's sequential WIF loop does not check ctx.Err() between iterations, so a cancelled context won't short-circuit the loop — it will attempt each remaining repo, only to fail at the first context-dependent call. Consider adding a ctx.Err() check at the top of the WIF loop and a corresponding test.

• [dead-code] internal/repos/batch_install_test.go:615 — Variable guardCheckCount is declared and immediately suppressed with _ = guardCheckCount. This appears to be leftover from an earlier version of the TOCTOU re-check test.

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 EnsureOrgInMint calls auto-register orgs in shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and the behavior is documented in docs/cli/repos.md with a Note callout. This is intentional and authorized by the implementation plan; consider whether an opt-in model (--register-orgs) would be preferable in a follow-up.

• [scope-creep] website/.vitepress/config.ts — The functional change (adding the fullsend repos sidebar entry) is bundled with a whole-file reformatting (single→double quotes, ~800 lines). The reformatting aligns with the project's .prettierrc (singleQuote: false), correcting prior format drift rather than introducing arbitrary churn. However, it inflates the diff and creates merge conflict risk for concurrent PRs.

Previous run (13)

Review

Well-structured implementation of repos install (PR 5 from the repos management plan, ADR 0057). The three-phase architecture — parallel discovery → sequential WIF → parallel scaffold — correctly respects GCP read-modify-write constraints while maximizing throughput. All prior findings from earlier revisions have been addressed: the orgRepresentative map resolves the multi-org provisioner bug, checkPerRepoScopes is called before BatchInstall, comprehensive documentation is included, and the stale sidebar entry has been added to config.ts.

The test suite is thorough (20+ test cases, 92%+ coverage). TestBatchInstall_ConcurrencyCap properly validates the semaphore bound with 8 repos at MaxConcurrency 2. The mutex additions to fakeWIFProvisioner and fakeScaffoldCommit correctly harden test doubles for concurrent use — this is test strengthening, not weakening. BatchInstall now returns an error for invalid MaxConcurrency values instead of silently clamping.

Findings

Low

• [test-adequacy] internal/repos/batch_install_test.go — No test covers context cancellation propagation through Phase 1 (parallel discovery) or Phase 2 (sequential WIF). The FakeClient ignores context, so cancellation cannot be observed in those phases. Consider adding a test with a context-aware fake or renaming existing error-collection tests to avoid implying cancellation coverage.

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 EnsureOrgInMint calls auto-register orgs in shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and the behavior is documented in docs/cli/repos.md with a Note callout. Consider requiring an explicit --register-orgs opt-in flag in a follow-up.

• [scope-creep] website/.vitepress/config.ts — The functional change (adding the fullsend repos sidebar entry) is bundled with a whole-file reformatting (single→double quotes, ~800 lines). This inflates the PR diff and makes review harder. Style-only changes should be in a separate commit or PR.

Previous run (14)

Review

Findings

Low

• [stale-doc] website/.vitepress/config.ts:160 — The /cli/ sidebar uses manual entries (not getMarkdownFiles()). The new docs/cli/repos.md page will not appear in the website navigation without adding { text: 'fullsend repos', link: '/cli/repos' } to the sidebar items array. The page is still reachable via the link in docs/cli/README.md, but sidebar discovery is broken.

• [test-adequacy] internal/repos/batch_install_test.go — No test covers context cancellation propagation through Phase 1 (parallel discovery) or Phase 2 (sequential WIF). The FakeClient ignores context, so cancellation cannot be observed in those phases. Consider adding a test with a context-aware fake or renaming existing error-collection tests to avoid implying cancellation coverage.

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 EnsureOrgInMint calls auto-register orgs in shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and the behavior is now documented in docs/cli/repos.md with a Note callout. Consider requiring an explicit --register-orgs opt-in flag in a follow-up.

Previous run (15)

Review

Well-structured implementation of repos install (PR 5 from the repos management plan, ADR 0057). The three-phase architecture — parallel discovery → sequential WIF → parallel scaffold — correctly respects GCP read-modify-write constraints while maximizing throughput. Prior high and medium findings from earlier revisions (Phase 2 org-config mismatch, missing scope preflight, missing CLI docs) have all been addressed: orgRepresentative map resolves the multi-org provisioner bug, checkPerRepoScopes is called before BatchInstall, and comprehensive documentation has been added.

The test suite is thorough (20+ test cases, 92%+ coverage) and the TestBatchInstall_ConcurrencyCap test now properly validates the semaphore bound with 8 repos at MaxConcurrency 2. The mutex additions to fakeWIFProvisioner and fakeScaffoldCommit in install_test.go correctly harden test doubles for concurrent use without weakening assertions.

Findings

Low

• [stale-doc] website/.vitepress/config.ts:160 — The /cli/ sidebar uses manual entries (not getMarkdownFiles()). The new docs/cli/repos.md page will not appear in the website navigation without adding { text: 'fullsend repos', link: '/cli/repos' } to the sidebar items array. The page is still reachable via the link in docs/cli/README.md, but sidebar discovery is broken.

• [test-adequacy] internal/repos/batch_install_test.go — No test covers context cancellation propagation through Phase 1 (parallel discovery) or Phase 2 (sequential WIF). The FakeClient ignores context, so cancellation cannot be observed in those phases. Consider adding a test with a context-aware fake or renaming existing error-collection tests to avoid implying cancellation coverage.

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 EnsureOrgInMint calls auto-register orgs in shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and the behavior is now documented in docs/cli/repos.md with a Note callout. Consider requiring an explicit --register-orgs opt-in flag in a follow-up.

Previous run (16)

Review

Findings

Low

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 includes EnsureOrgInMint calls that auto-register orgs in shared mint infrastructure. Running repos install with a manifest containing a new org silently modifies shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and this behavior is now documented in docs/cli/repos.md with a Note callout. Consider requiring an explicit --register-orgs opt-in flag in a follow-up.

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ConcurrencyCap sets MaxConcurrency to 32 with a single repo entry, declares a maxSeen variable incremented in the progress callback, but only asserts maxSeen > 0. The test name implies it validates the concurrency cap but never asserts that peak concurrent goroutines stayed within the limit. Add multiple repos and track actual peak concurrency.

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ContextCancellation cancels the context before calling BatchInstall, then injects ctx.Err() via fakeScaffoldCommit. Since FakeClient ignores context, Phases 1 and 2 proceed normally — only Phase 3 fails. The test validates error collection but not actual cancellation propagation. Consider renaming the test or enhancing FakeClient to respect context.

Previous run (17)

Review

Findings

Low

• [scope-creep] internal/repos/batch_install.go:153 — Phase 2 includes EnsureOrgInMint calls that auto-register orgs in shared mint infrastructure. Running repos install with a manifest containing a new org silently modifies shared mint infrastructure. The --skip-mint-check flag provides an opt-out, and this behavior is now documented in docs/cli/repos.md with a Note callout. Consider requiring an explicit --register-orgs opt-in flag in a follow-up.

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ConcurrencyCap sets MaxConcurrency to 32 with a single repo entry, declares a maxSeen variable incremented in the progress callback, but only asserts maxSeen > 0. The test name implies it validates the concurrency cap but never asserts that peak concurrent goroutines stayed within the limit. Add multiple repos and track actual peak concurrency.

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ContextCancellation cancels the context before calling BatchInstall, then injects ctx.Err() via fakeScaffoldCommit. Since FakeClient ignores context, Phases 1 and 2 proceed normally — only Phase 3 fails. The test validates error collection but not actual cancellation propagation. Consider renaming the test or enhancing FakeClient to respect context.

---

Labels: PR adds a new CLI repos install command with Go code in internal/cli/ and internal/repos/, implementing a feature from ADR 0057.

Previous run (18)

Review

Findings

Medium

• [scope-creep] internal/repos/batch_install.go:456 — Phase 2 includes EnsureOrgInMint calls that auto-register orgs in shared mint infrastructure. While ADR 0057 documents this as part of the design, the behavior means repos install modifies mint configuration that serves all repos using that mint — not just the repos being installed. The --skip-mint-check flag (default false) provides an opt-out. Operators should be aware that running repos install with a new org in the manifest will register that org in the mint's ALLOWED_ORGS.

• [design-trajectory-deviation] internal/cli/repos.go:186 — The CLI layer instantiates gcfProvisionerAdapter directly, importing internal/dispatch/gcf. The repos package correctly uses the WIFProvisioner interface to decouple from the concrete GCF provisioner, but the CLI inlines GCF-specific construction rather than reusing a shared helper from admin.go. Consider extracting provisioner factory construction into a shared helper to keep GCF-specific logic in one place.

• [stale-doc] docs/ADRs/0057-repos-management.md:73 — ADR 0057 lists repos status, repos sync, repos diff, repos upgrade, repos upgrade-mint, and repos remove commands as decided. This PR implements only repos install. While ADRs are forward-looking design documents, adding an implementation status note would reduce confusion for readers who expect all listed commands to be available.

Low

• [concurrency-bound] internal/repos/batch_install.go:393 — BatchInstall() silently clamps MaxConcurrency values ≤ 0 to 4 and > 32 to 32. The CLI layer now validates and rejects values outside 1-32, but BatchInstall is a public function that other callers can invoke directly with out-of-range values.

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ContextCancellation cancels the context before calling BatchInstall, then injects a static error via fakeScaffoldCommit. This only tests Phase 3 failure handling, not cancellation propagation through Phase 1 or Phase 2 (since FakeClient ignores context).

• [test-adequacy] internal/repos/batch_install_test.go:1385 — TestBatchInstall_ConcurrencyCap sets MaxConcurrency to 100 and declares a maxSeen variable but never asserts on it. The test name implies it validates the concurrency cap, but the assertion is missing.

• [auth-bypass] internal/repos/batch_install.go:458 — The --skip-mint-check flag bypasses EnsureOrgInMint org registration validation in Phase 2. This is an intentional operator escape hatch (opt-in, default false), but weakens defense-in-depth by removing a server-side authorization check.

• [naming-alignment] internal/cli/repos.go:106 — reposInstallConfig mixes production config (flags) with test injection fields (testClient, testProvisioner). This is the established CLI pattern (admin.go uses the same approach), but the repos package demonstrates the cleaner alternative with interface parameters.

• [documentation-coherence] docs/cli/repos.md:31 — Flag descriptions are terse and don't explain flag interactions or when to use --skip-mint-check. Adding a brief "Common workflows" section would help operators.

• [stale-doc] docs/guides/getting-started/operations.md:29 — Operations guide doesn't mention repos install as an alternative workflow for manifest-managed installations.

• [missing-doc] docs/guides/getting-started/operations.md:52 — Standalone commands table doesn't include fullsend repos install.

• [incomplete-doc] docs/plans/repos-management.md:145 — The plan documents the full command set but only repos install is implemented. The PR correctly marks PR 5 as "In Review".

Previous run (19)

Review

Findings

Medium

• [scope-creep] internal/repos/batch_install.go:456 — Phase 2 includes EnsureOrgInMint calls that auto-register orgs in shared mint infrastructure. While ADR 0057 documents this as part of the design, the behavior means repos install modifies mint configuration that serves all repos using that mint — not just the repos being installed. The --skip-mint-check flag (default false) provides an opt-out. Operators should be aware that running repos install with a new org in the manifest will register that org in the mint's ALLOWED_ORGS.

• [design-trajectory-deviation] internal/cli/repos.go:186 — The CLI layer instantiates gcfProvisionerAdapter directly, importing internal/dispatch/gcf. The repos package correctly uses the WIFProvisioner interface to decouple from the concrete GCF provisioner, but the CLI inlines GCF-specific construction rather than reusing a shared helper from admin.go. Consider extracting provisioner factory construction into a shared helper to keep GCF-specific logic in one place.

• [stale-doc] docs/ADRs/0057-repos-management.md:73 — ADR 0057 lists repos status, repos sync, repos diff, repos upgrade, repos upgrade-mint, and repos remove commands as decided. This PR implements only repos install. While ADRs are forward-looking design documents, adding an implementation status note would reduce confusion for readers who expect all listed commands to be available.

Low

• [concurrency-bound] internal/repos/batch_install.go:393 — BatchInstall() silently clamps MaxConcurrency values ≤ 0 to 4 and > 32 to 32. The CLI layer now validates and rejects values outside 1-32, but BatchInstall is a public function that other callers can invoke directly with out-of-range values.

• [test-adequacy] internal/repos/batch_install_test.go — TestBatchInstall_ContextCancellation cancels the context before calling BatchInstall, then injects a static error via fakeScaffoldCommit. This only tests Phase 3 failure handling, not cancellation propagation through Phase 1 or Phase 2 (since FakeClient ignores context).

• [test-adequacy] internal/repos/batch_install_test.go:1385 — TestBatchInstall_ConcurrencyCap sets MaxConcurrency to 100 and declares a maxSeen variable but never asserts on it. The test name implies it validates the concurrency cap, but the assertion is missing.

• [auth-bypass] internal/repos/batch_install.go:458 — The --skip-mint-check flag bypasses EnsureOrgInMint org registration validation in Phase 2. This is an intentional operator escape hatch (opt-in, default false), but weakens defense-in-depth by removing a server-side authorization check.

• [naming-alignment] internal/cli/repos.go:106 — reposInstallConfig mixes production config (flags) with test injection fields (testClient, testProvisioner). This is the established CLI pattern (admin.go uses the same approach), but the repos package demonstrates the cleaner alternative with interface parameters.

• [documentation-coherence] docs/cli/repos.md:31 — Flag descriptions are terse and don't explain flag interactions or when to use --skip-mint-check. Adding a brief "Common workflows" section would help operators.

• [stale-doc] docs/guides/getting-started/operations.md:29 — Operations guide doesn't mention repos install as an alternative workflow for manifest-managed installations.

• [missing-doc] docs/guides/getting-started/operations.md:52 — Standalone commands table doesn't include fullsend repos install.

• [incomplete-doc] docs/plans/repos-management.md:145 — The plan documents the full command set but only repos install is implemented. The PR correctly marks PR 5 as "In Review".

Previous run (20)

Review

Findings

Low

• [documentation-format] internal/cli/repos.go:106 — Type reposInstallConfig lacks a documentation comment. Some config structs in the CLI layer (e.g., githubSetupConfig) have doc comments in the // {typeName} holds {description} format, though the pattern is not universal (e.g., perRepoInstallConfig also lacks one).

• [code-organization] internal/cli/repos.go:115 — Testing hooks comment (// Testing hooks.) is terse compared to the more descriptive comment used in admin.go (// Testing overrides -- when non-nil, used instead of resolving from the environment. Not set by CLI flag parsing.).

• [concurrency-bound] internal/cli/repos.go:141 — The --concurrency flag accepts any integer. BatchInstall() silently clamps values ≤ 0 to 4 and > 32 to 32, but docs/cli/repos.md advertises the range as 1-32. A user passing --concurrency 0 likely intends sequential execution (or expects an error), not concurrency 4.

• [test-adequacy] internal/repos/batch_install_test.go — No test covers context cancellation during any of the three phases. The existing 16 tests comprehensively cover success paths, failure modes, filtering, and serialization, but cancellation propagation through the semaphore-gated goroutines is untested.

Previous run

Review

Findings

High

• [logic-error] internal/repos/batch_install.go:159 — Phase 2 org-mint loop always uses toInstall[0].resolved to create the provisioner and extract mintURL, regardless of which org is being iterated. When repos span multiple orgs, every org except the one owning toInstall[0] gets a provisioner constructed with the wrong owner's config. The provisioner factory builds a gcf.Config with GitHubOrgs: []string{resolved.Owner} and Repo: resolved.Owner + "/" + resolved.Repo, so a provisioner created from org A's resolved config will pass org A's identity when calling EnsureOrgInMint for org B.
  Remediation: Find a representative discoveryResult for the current org and use its resolved config. Iterate toInstall to find the first entry where d.resolved.Owner == org and use that entry's resolved config.

Medium

• [auth-bypass] internal/cli/repos.go — The --skip-mint-check flag bypasses both DiscoverMint (URL discovery) and EnsureOrgInMint (org registration validation). When SkipMintCheck is true, the entire Phase 2 org-registration block is skipped, allowing repos to be installed into orgs not registered in the mint.
  Remediation: Consider whether --skip-mint-check should only skip DiscoverMint and not EnsureOrgInMint. If both should be skipped, document this clearly in the flag help text.

• [permission-expansion] internal/cli/repos.go — The existing admin.go install flow calls checkPerRepoScopes() and checkInstallScopes() to verify the GitHub token has required OAuth scopes before performing operations. The new repos install command calls resolveToken() but does not perform scope preflight checks.
  Remediation: Add a scope preflight check (checkPerRepoScopes at minimum) in the repos install command before invoking BatchInstall.

• [missing-doc] docs/cli/README.md:13 — The CLI overview lists command groups (github, inference, mint) but does not include the new fullsend repos command group.
  Remediation: Add fullsend repos to the command groups table in docs/cli/README.md and create a docs/cli/repos.md reference page documenting the repos install command and its flags.

Low

• [race-condition] internal/repos/install_test.go:13 — The fakeWIFProvisioner test double records calls via bare bool fields without synchronization, inconsistent with the mutex added to fakeScaffoldCommit in this PR. Currently no actual race since fakeWIFProvisioner is only used in single-threaded Install() tests.

• [edge-case] internal/repos/batch_install.go:161 — The if len(toInstall) == 0 { break } guard inside the org-mint loop is unreachable dead code.

• [concurrency-bound] internal/cli/repos.go — The --concurrency flag has no upper bound validation. An operator could set very high values causing rate limiting or resource exhaustion.

• [naming-convention] internal/cli/repos.go:37 — reposInstallOptions uses an Options suffix but established CLI patterns use Config suffix (e.g., githubSetupConfig, perRepoInstallConfig).

• [stale-doc] docs/plans/repos-management.md — PR 5 section in the implementation plan does not have a status marker. After merge, update to reflect implementation status.

---

Labels: PR adds a new CLI install command with Go code in internal/cli/ and internal/repos/.

Previous run (21)

Review

Findings

Low

• [documentation-format] internal/cli/repos.go:106 — Type reposInstallConfig lacks a documentation comment. Some config structs in the CLI layer (e.g., githubSetupConfig) have doc comments in the // {typeName} holds {description} format, though the pattern is not universal (e.g., perRepoInstallConfig also lacks one).

• [code-organization] internal/cli/repos.go:115 — Testing hooks comment (// Testing hooks.) is terse compared to the more descriptive comment used in admin.go (// Testing overrides -- when non-nil, used instead of resolving from the environment. Not set by CLI flag parsing.).

• [concurrency-bound] internal/cli/repos.go:141 — The --concurrency flag accepts any integer. BatchInstall() silently clamps values ≤ 0 to 4 and > 32 to 32, but docs/cli/repos.md advertises the range as 1-32. A user passing --concurrency 0 likely intends sequential execution (or expects an error), not concurrency 4.

• [test-adequacy] internal/repos/batch_install_test.go — No test covers context cancellation during any of the three phases. The existing 16 tests comprehensively cover success paths, failure modes, filtering, and serialization, but cancellation propagation through the semaphore-gated goroutines is untested.

Previous run (22)

Review

Findings

High

• [logic-error] internal/repos/batch_install.go:159 — Phase 2 org-mint loop always uses toInstall[0].resolved to create the provisioner and extract mintURL, regardless of which org is being iterated. When repos span multiple orgs, every org except the one owning toInstall[0] gets a provisioner constructed with the wrong owner's config. The provisioner factory builds a gcf.Config with GitHubOrgs: []string{resolved.Owner} and Repo: resolved.Owner + "/" + resolved.Repo, so a provisioner created from org A's resolved config will pass org A's identity when calling EnsureOrgInMint for org B.
  Remediation: Find a representative discoveryResult for the current org and use its resolved config. Iterate toInstall to find the first entry where d.resolved.Owner == org and use that entry's resolved config.

Medium

• [auth-bypass] internal/cli/repos.go — The --skip-mint-check flag bypasses both DiscoverMint (URL discovery) and EnsureOrgInMint (org registration validation). When SkipMintCheck is true, the entire Phase 2 org-registration block is skipped, allowing repos to be installed into orgs not registered in the mint.
  Remediation: Consider whether --skip-mint-check should only skip DiscoverMint and not EnsureOrgInMint. If both should be skipped, document this clearly in the flag help text.

• [permission-expansion] internal/cli/repos.go — The existing admin.go install flow calls checkPerRepoScopes() and checkInstallScopes() to verify the GitHub token has required OAuth scopes before performing operations. The new repos install command calls resolveToken() but does not perform scope preflight checks.
  Remediation: Add a scope preflight check (checkPerRepoScopes at minimum) in the repos install command before invoking BatchInstall.

• [missing-doc] docs/cli/README.md:13 — The CLI overview lists command groups (github, inference, mint) but does not include the new fullsend repos command group.
  Remediation: Add fullsend repos to the command groups table in docs/cli/README.md and create a docs/cli/repos.md reference page documenting the repos install command and its flags.

Low

• [race-condition] internal/repos/install_test.go:13 — The fakeWIFProvisioner test double records calls via bare bool fields without synchronization, inconsistent with the mutex added to fakeScaffoldCommit in this PR. Currently no actual race since fakeWIFProvisioner is only used in single-threaded Install() tests.

• [edge-case] internal/repos/batch_install.go:161 — The if len(toInstall) == 0 { break } guard inside the org-mint loop is unreachable dead code.

• [concurrency-bound] internal/cli/repos.go — The --concurrency flag has no upper bound validation. An operator could set very high values causing rate limiting or resource exhaustion.

• [naming-convention] internal/cli/repos.go:37 — reposInstallOptions uses an Options suffix but established CLI patterns use Config suffix (e.g., githubSetupConfig, perRepoInstallConfig).

• [stale-doc] docs/plans/repos-management.md — PR 5 section in the implementation plan does not have a status marker. After merge, update to reflect implementation status.

---

Labels: PR adds a new CLI install command with Go code in internal/cli/ and internal/repos/.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:44 AM UTC · Completed 1:56 AM UTC
Commit: e72df13 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure · Started 2:02 AM UTC · Completed 2:17 AM UTC
Commit: 10fdc0c · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:23 AM UTC · Completed 2:35 AM UTC
Commit: edb7e60 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 12:40 AM UTC · Ended 12:43 AM UTC
Commit: e8381e3 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:44 AM UTC · Completed 12:55 AM UTC
Commit: c90f40e · View workflow run →

Superseded by updated review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:05 AM UTC · Completed 1:13 AM UTC
Commit: c0d8e98 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:27 AM UTC · Completed 1:37 AM UTC
Commit: 6357222 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 2:13 AM UTC · Completed 2:21 AM UTC
Commit: 6357222 · View workflow run →

[fullsend-ai-retro]

PR #3033 added a repos install CLI command (2197 additions, 11 files) and went through 20 review bot runs over 4 days before merging. The fullsend-ai-review bot found some valuable issues (a critical 'code' vs 'coder' role typo, a dry-run logic error, dead code) but missed 12 significant findings that were caught by a human-orchestrated multi-model Review Squad (waynesun09): duplicate command registration (CRITICAL), gh.New ignoring GITHUB_API_URL (HIGH), WIF resource cleanup gaps (HIGH+MEDIUM), non-deterministic map iteration, context cancellation gaps, and flag type inconsistencies. The review bot also posted the identical scope-creep comment 6 times across review cycles despite repeated author acknowledgment. Three proposals target: (1) evidence for the known duplicate-finding dedup issue #1013 and scope-creep issue #3864, (2) teaching the review agent to cross-reference sibling code for codebase consistency, and (3) teaching the review agent to audit resource cleanup across multi-phase operations.

Proposals filed

• Evidence for #1013 and #3864: scope-creep finding repeated 6 times across 10+ review iterations on PR #3033 (in fullsend-ai/fullsend)
• Review agent should cross-reference sibling commands for codebase consistency when reviewing new CLI subcommands (in fullsend-ai/fullsend)
• Review agent should audit resource cleanup symmetry across multi-phase operations (in fullsend-ai/fullsend)