fix(#5896): set FULLSEND_WIF_PROVIDER and store bot token in Secret Manager by fullsend-ai-coder[bot] · Pull Request #5898 · fullsend-ai/fullsend

fullsend-ai-coder · 2026-08-04T12:30:36Z

Summary

Fixes three bugs in GitLab WIF mode that were discovered during E2E testing on an internal GitLab instance. Two are missing variables that the implementation plan specified but were omitted during PR #5874. The third is an undocumented WIF requirement for OIDC issuer reachability.

Changes

Bug 1 — FULLSEND_WIF_PROVIDER not set: Added FULLSEND_WIF_PROVIDER to installProtectedVarsForForge so it is written as a protected CI/CD variable when WIF mode is active. The scaffold templates reference this variable for the forge-token WIF credential exchange.
Bug 2 — FULLSEND_BOT_TOKEN_SECRET not set: Modified setupGitLabBotToken to support WIF-mode credential storage. When BotTokenWIFConfig is provided, the bot PAT is stored in GCP Secret Manager (create-or-update pattern), the WIF service account is granted secretAccessor on the secret, and FULLSEND_BOT_TOKEN_SECRET is set as a protected CI/CD variable pointing to the secret name. In variable mode (nil config), existing behavior is preserved.
Bug 3 — OIDC issuer reachability (docs): Added a note to ADR 0067's credential model section documenting that WIF mode requires the GitLab instance's OIDC discovery endpoints to be publicly reachable by GCP's STS service.

Testing

New unit tests for WIF-mode bot token storage (TestSetupGitLabBotToken_WIFMode)
New unit test for secret ID generation (TestBotTokenSecretID)
Updated existing tests to verify FULLSEND_WIF_PROVIDER in protected variables
All existing setupGitLabBotToken tests pass with the new signature (nil wifCfg)
go vet passes on changed packages
go build ./... succeeds

Closes #5896

Post-script verification

Branch is not main/master (agent/5896-gitlab-wif-bugs)
Secret scan passed (gitleaks — 75f34672a628827160af063870e14902573c9b0a..HEAD)
PR body secret scan passed (gitleaks — no-git)

fullsend-ai-review · 2026-08-04T12:31:42Z

🤖 Finished Review · ✅ Success · Started 12:31 PM UTC · Completed 12:46 PM UTC
Commit: 6c5969b · View workflow run →

github-actions · 2026-08-04T12:32:42Z

Site preview

Preview: https://4682c946-site.fullsend-ai.workers.dev

Commit: cf34b8312a8736fedfe3ee3ee7fae2242163cbab

codecov · 2026-08-04T12:36:53Z

Codecov Report

❌ Patch coverage is 82.53968% with 11 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/repos.go	0.00%	6 Missing ⚠️
internal/cli/repos_gitlab.go	89.13%	3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

fullsend-ai-review · 2026-08-04T12:46:09Z

Review

Findings

Low

[secret-lifecycle] internal/repos/uninstall.go:23 — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable and revokes the bot PAT at the GitLab level (via cleanupGitLabBotToken), but does not delete the underlying Secret Manager secret. The bot PAT remains stored in GCP Secret Manager with an active version and the mint service account retains its IAM binding to read it. The revoked PAT is unusable, bounding practical risk. Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[secret-id-collision] internal/cli/repos_gitlab.go:36 — The secretIDSanitizer maps dots to hyphens, so botTokenSecretID("my.group", "repo") and botTokenSecretID("my-group", "repo") produce the same secret ID. The slash-vs-dot collision is resolved (slashes now map to double underscores), but the dot-vs-hyphen collision remains. Practical risk is low since GitLab rarely has two groups differing only by dot/hyphen.
[partial-failure] internal/cli/repos_gitlab.go:123 — In the WIF path, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: storeSecretManagerToken detects existing secrets and disables old versions on re-entry.
[privilege-scope] internal/cli/repos_gitlab.go:131 — SetSecretIAMBinding grants roles/secretmanager.secretAccessor to the mint service account. This is an additive IAM binding — on re-install with a different service account, the old binding would not be removed. Minor concern since project ID changes are rare.

Previous run

Review

Findings

Low

[secret-lifecycle] internal/repos/uninstall.go:23 — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable and revokes the bot PAT at the GitLab level (via cleanupGitLabBotToken), but does not delete the underlying Secret Manager secret. Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[secret-id-collision] internal/cli/repos_gitlab.go:36 — The secretIDSanitizer maps dots to hyphens, so botTokenSecretID("my.group", "repo") and botTokenSecretID("my-group", "repo") produce the same secret ID. The slash-vs-dot collision is resolved (slashes now map to double underscores), but the dot-vs-hyphen collision remains. Practical risk is low since GitLab rarely has two groups differing only by dot/hyphen.
[partial-failure] internal/cli/repos_gitlab.go:123 — In the WIF path, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: storeSecretManagerToken detects existing secrets and disables old versions on re-entry.
[privilege-scope] internal/cli/repos_gitlab.go:131 — SetSecretIAMBinding grants roles/secretmanager.secretAccessor to the mint service account. This is an additive IAM binding — on re-install with a different service account, the old binding would not be removed. Minor concern since project ID changes are rare.
[stale-documentation] docs/plans/gitlab-cron-polling-implementation.md:1708 — Plan document uninstall pseudocode omits FULLSEND_WIF_PROVIDER and FULLSEND_BOT_TOKEN_SECRET. The document already carries a top-of-file staleness note.

Previous run (2)

Review

Findings

Low

[secret-lifecycle] internal/repos/uninstall.go:20 — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable and revokes the bot PAT at the GitLab level (via cleanupGitLabBotToken), but does not delete the Secret Manager secret itself. The revoked PAT persists in GCP Secret Manager. Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[secret-id-collision] internal/cli/repos_gitlab.go:32 — The secretIDSanitizer maps dots to hyphens, so botTokenSecretID("my.group", "repo") and botTokenSecretID("my-group", "repo") produce the same secret ID. The prior review's slash-vs-dot collision is resolved (slashes now map to double underscores), but the dot-vs-hyphen collision remains. Practical risk is low since GitLab rarely has two groups differing only by dot/hyphen.
[partial-failure] internal/cli/repos_gitlab.go:109 — In the WIF path, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: storeSecretManagerToken detects existing secrets and disables old versions on re-entry.
[stale-documentation] docs/plans/gitlab-cron-polling-implementation.md:1708 — Plan document uninstall pseudocode omits FULLSEND_WIF_PROVIDER and FULLSEND_BOT_TOKEN_SECRET. The document already carries a top-of-file staleness note.

Previous run (3)

Review

Findings

Low

[secret-lifecycle] internal/repos/uninstall.go:20 — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable and revokes the bot PAT at the GitLab level (via cleanupGitLabBotToken), but does not delete the Secret Manager secret itself. The revoked PAT persists in GCP Secret Manager. Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[secret-id-collision] internal/cli/repos_gitlab.go:32 — The secretIDSanitizer maps dots to hyphens, so botTokenSecretID("my.group", "repo") and botTokenSecretID("my-group", "repo") produce the same secret ID. The prior review's slash-vs-dot collision is resolved (slashes now map to double underscores), but the dot-vs-hyphen collision remains. Practical risk is low since GitLab rarely has two groups differing only by dot/hyphen.
[partial-failure] internal/cli/repos_gitlab.go:109 — In the WIF path, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: storeSecretManagerToken detects existing secrets and disables old versions on re-entry.
[coherence] internal/dispatch/gcf/provisioner.go — provisioner.go still constructs the SA email inline via fmt.Sprintf at three call sites (StoreAgentPEM, CreateFunction, UpdateFunction). Now that MintServiceAccountEmail is exported in the same file, these could be consolidated for consistency.
[stale-documentation] docs/plans/gitlab-cron-polling-implementation.md:1633 — Plan document references FULLSEND_WIF_PROVIDER and FULLSEND_BOT_TOKEN_SECRET as future implementation steps that are now implemented by this PR.

Previous run (4)

Review

Findings

High

[invalid-input] internal/cli/repos_gitlab.go:43 — botTokenSecretID maps slashes to dots (strings.ReplaceAll(owner, "/", ".")), but GCP Secret Manager secret IDs only allow [a-zA-Z0-9_-] — dots are not valid characters per the API documentation. Any GitLab subgroup path (e.g., owner="group/subgroup") will produce a secret ID containing dots (e.g., fullsend-bot-token-group.subgroup--repo) that GCP will reject at the API level. The secretIDSanitizer regex explicitly preserves dots, and the validation patterns in gcp.go were updated to allow dots — but client-side validation changes cannot override the server-side GCP API constraint.
Remediation: Replace dots with a different valid separator in botTokenSecretID (e.g., double-underscores: strings.ReplaceAll(owner, "/", "__")). Update secretIDSanitizer to [^a-zA-Z0-9_\-] (no dots). Revert the dot additions in secretResourcePattern and secretVersionPattern in gcp.go. The collision concern (group/sub vs group-sub) needs a separator that is itself valid in Secret Manager IDs.

Medium

[pattern-mismatch] internal/dispatch/gcf/gcp.go:27 — Both secretResourcePattern (line 27) and secretVersionPattern (line 31) were updated to allow dots in secret names, changing the character class from [a-zA-Z0-9_-] to [a-zA-Z0-9_.\-]. These validation patterns now accept secret names that GCP will reject at the API level. This is the validation-side manifestation of the same root cause as the [invalid-input] finding above.
Remediation: Revert both patterns to [a-zA-Z0-9_-]+ (without dots) once botTokenSecretID is fixed to not produce dots.

Low

[secret-id-collision] internal/cli/repos_gitlab.go:43 — botTokenSecretID maps slashes in the owner to dots, which means a GitLab subgroup path group/sub and a flat group named group.sub produce identical secret IDs. The practical risk is low because owning both paths on the same GitLab instance is an unusual configuration. Note: this collision vector will need to be re-evaluated once the dot separator is replaced per the [invalid-input] finding.
[partial-failure] internal/cli/repos_gitlab.go:116 — In the WIF path of setupGitLabBotToken, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: the function detects existing secrets and disables old versions, consistent with the StoreAgentPEM pattern.
[secret-lifecycle] internal/repos/uninstall.go:18 — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable but does not delete the Secret Manager secret itself. After uninstall, the bot PAT persists in GCP Secret Manager with its IAM binding. The PAT inside is dead (project access token is revoked by cleanupGitLabBotToken). Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[stale-documentation] docs/plans/gitlab-cron-polling-implementation.md — Plan document references FULLSEND_WIF_PROVIDER and FULLSEND_BOT_TOKEN_SECRET as future implementation steps that are now implemented by this PR.
[coherence] internal/dispatch/gcf/provisioner.go — provisioner.go still constructs the SA email inline via fmt.Sprintf at three call sites. Now that MintServiceAccountEmail is exported in the same file, these could be consolidated for consistency.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (5)

Review

Findings

Medium

[pattern-mismatch] internal/dispatch/gcf/gcp.go:31 — secretVersionPattern was not updated to allow dots in the secret name portion. The PR updated secretResourcePattern (line 27) to include dots for SetSecretIAMBinding, but secretVersionPattern still uses [a-zA-Z0-9_-]+ (no dots). When storeSecretManagerToken updates an existing secret whose ID contains a dot (produced by botTokenSecretID for GitLab groups/repos with dots in their names), DisableSecretVersion resolves the latest version name from the GCP API (e.g., projects/123/secrets/fullsend-bot-token-my.group--repo/versions/1), then validates it against secretVersionPattern, which rejects it. Because storeSecretManagerToken treats DisableSecretVersion failure as a non-fatal warning, old secret versions remain enabled rather than being disabled.
Remediation: Update secretVersionPattern to [a-zA-Z0-9_.\-]+ to match the expanded secretResourcePattern.

Low

[secret-id-collision] internal/cli/repos_gitlab.go:40 — botTokenSecretID maps slashes in the owner to dots (strings.ReplaceAll(owner, "/", ".")), which means a GitLab subgroup path group/sub and a flat group named group.sub produce identical secret IDs. The -- separator prevents owner/repo boundary collisions, but the slash-to-dot mapping creates intra-owner collisions. The practical risk is low because owning both group/sub and group.sub on the same GitLab instance is an unusual configuration.
[partial-failure] internal/cli/repos_gitlab.go:116 — In the WIF path of setupGitLabBotToken, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: the function detects existing secrets and disables old versions, consistent with the StoreAgentPEM pattern.
[secret-lifecycle] internal/repos/uninstall.go:18 — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable but does not delete the Secret Manager secret itself. After uninstall, the bot PAT persists in GCP Secret Manager with its IAM binding. The PAT inside is dead (GitLab project access token is revoked by cleanupGitLabBotToken). Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[coherence] internal/dispatch/gcf/provisioner.go:236 — provisioner.go still constructs the SA email inline via fmt.Sprintf at three call sites (lines 236, 892, 947). Now that MintServiceAccountEmail is exported in the same file, these could be consolidated for consistency.
[coherence] internal/cli/admin.go:838 — admin.go hardcodes the SA email pattern in a display path. With MintServiceAccountEmail now exported, this could be consolidated.
[stale-documentation] docs/plans/gitlab-cron-polling-implementation.md — Plan document references FULLSEND_WIF_PROVIDER and FULLSEND_BOT_TOKEN_SECRET as future implementation steps that are now implemented by this PR.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (6)

Review

Findings

Medium

[secret-id-collision] internal/cli/repos_gitlab.go:40 — botTokenSecretID sanitizes owner/repo by replacing characters outside [a-zA-Z0-9_.-] with hyphens. GitLab subgroup paths contain slashes (e.g., group/subgroup) which are replaced with hyphens, creating collisions: group/sub + repo produces the same secret ID as group-sub + repo (both yield fullsend-bot-token-group-sub--repo). The -- separator prevents owner/repo boundary collisions but not intra-owner collisions from slash-to-hyphen mapping. The second installation silently overwrites the first repo's bot PAT in Secret Manager.
Remediation: Use a separator that cannot appear in GitLab names, or include a short hash suffix to guarantee uniqueness.
[input-validation] internal/cli/repos_gitlab.go:31 — secretIDSanitizer preserves dots in secret IDs (regex [^a-zA-Z0-9_.\-]), but SetSecretIAMBinding validates the resource path against secretResourcePattern (gcp.go:27: [a-zA-Z0-9_-]+), which does not allow dots. For any GitLab group or repo with dots in the name (e.g., my.company/repo), CreateSecret and AddSecretVersion succeed, but SetSecretIAMBinding fails with "invalid secret resource path", leaving the secret stored but inaccessible to the mint service account. The install fails closed but with a confusing error.
Remediation: Either add dots to secretResourcePattern in gcp.go (dots are valid in Secret Manager IDs per the GCP API) or remove dots from secretIDSanitizer (replace with hyphens like other disallowed characters).

Low

[secrets-lifecycle] internal/cli/repos_gitlab.go:118 — When storeSecretManagerToken updates an existing secret, DisableSecretVersion failure is treated as a non-fatal warning. Old bot PAT values may remain as enabled secret versions. Functionally safe since the scaffold reads latest, and old PATs are revoked on re-install.
[partial-failure] internal/cli/repos_gitlab.go:116 — In the WIF path of setupGitLabBotToken, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: the function detects existing secrets and disables old versions, consistent with the StoreAgentPEM pattern.
[uninstall-cleanup-gap] internal/repos/uninstall.go — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable but does not delete the Secret Manager secret itself. After uninstall, the bot PAT persists in GCP Secret Manager with its IAM binding. The PAT inside is dead (GitLab project access token is revoked by cleanupGitLabBotToken). Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[stale-documentation] docs/plans/gitlab-cron-polling-implementation.md — Plan document references FULLSEND_WIF_PROVIDER and FULLSEND_BOT_TOKEN_SECRET as future implementation steps that are now implemented by this PR.
[coherence] internal/dispatch/gcf/provisioner.go:242 — provisioner.go still constructs the SA email inline via fmt.Sprintf at three call sites. Now that MintServiceAccountEmail is exported, these could be consolidated for consistency.
[coherence] internal/cli/admin.go:838 — admin.go hardcodes the SA email pattern in a display path. With MintServiceAccountEmail now exported, this could be consolidated.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (7)

Review

Findings

Medium

[secret-id-collision] internal/cli/repos_gitlab.go:40 — botTokenSecretID sanitizes owner/repo by replacing all characters outside [a-zA-Z0-9_-] with hyphens, creating collisions for repos whose group names differ only by dots vs hyphens. For example, my.company/repo and my-company/repo produce the same secret ID fullsend-bot-token-my-company--repo. The second installation silently overwrites the first repo's bot PAT in Secret Manager, causing authentication failures for the first repo.
Remediation: Preserve dots in the secret ID (dots are valid in Secret Manager IDs: [a-zA-Z0-9_.-]) or add a short hash suffix to guarantee uniqueness.

Low

[error-handling] internal/cli/repos_gitlab.go:166 — In storeSecretManagerToken, the DisableSecretVersion error is logged as a warning but execution continues. If disabling fails, old PAT versions remain enabled in Secret Manager. Functionally safe since the scaffold reads latest, but old PATs remain accessible by version number.
[partial-failure] internal/cli/repos_gitlab.go:116 — In the WIF path of setupGitLabBotToken, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. Retry-safe: the function detects existing secrets and disables old versions, consistent with the StoreAgentPEM pattern.
[uninstall-cleanup-gap] internal/repos/uninstall.go — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable but does not delete the Secret Manager secret itself. After uninstall, the bot PAT persists in GCP Secret Manager with its IAM binding. Consistent with existing behavior for agent PEM secrets (GCP cleanup deferred to inference deprovision).
[test-adequacy] internal/cli/repos_gitlab_test.go — TestBotTokenSecretID_Collision tests ("a","b-c") vs ("a-b","c") (differentiated by the -- separator) but does not test the real collision vector: inputs differing by characters the sanitizer maps to the same output, like ("a","b.c") vs ("a","b-c").
[coherence] internal/dispatch/gcf/provisioner.go:242 — provisioner.go still constructs the SA email inline via fmt.Sprintf at three call sites. Now that MintServiceAccountEmail is exported, these could be consolidated for consistency.
[coherence] internal/cli/admin.go:838 — admin.go hardcodes the SA email pattern in a display path. With MintServiceAccountEmail now exported, this could be consolidated.
[within-file-consistency] internal/cli/repos_gitlab_test.go — New WIF-mode test functions use map[string]any{} while existing tests in the same file use map[string]interface{}{}. Both are equivalent in Go 1.18+ but mixing in the same file is inconsistent.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (8)

Review

Findings

Medium

[missing-new-identifiers] docs/guides/getting-started/operations.md:39 — The GitLab CI/CD variables table lists variables set during install but is missing the two new protected CI/CD variables introduced by this PR: FULLSEND_WIF_PROVIDER and FULLSEND_BOT_TOKEN_SECRET.
Remediation: Add two rows to the GitLab CI/CD variables table for the new variables.

Low

[error-handling] internal/cli/repos_gitlab.go:168 — In storeSecretManagerToken, the error from DisableSecretVersion is silently discarded (_ = gcpClient.DisableSecretVersion(...)). If disabling fails, the old version remains enabled alongside the new version. The scaffold reads latest so this is functionally safe, but old PATs remain accessible.
Remediation: Log the error so operators can detect the failure.
[partial-failure] internal/cli/repos_gitlab.go:126 — In the WIF path of setupGitLabBotToken, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the already-created secret is not cleaned up. The function handles this gracefully on retry (detects existing secret, disables old version), consistent with the StoreAgentPEM pattern.
[secrets-handling] internal/repos/uninstall.go — The uninstall path removes the FULLSEND_BOT_TOKEN_SECRET CI/CD variable but does not delete the Secret Manager secret itself. After uninstall, the bot PAT remains in GCP Secret Manager. Consistent with existing behavior for agent PEM secrets.
[coherence] internal/dispatch/gcf/provisioner.go:236 — The new MintServiceAccountEmail helper resolves the prior review finding about SA email duplication. However, provisioner.go still constructs the SA email inline at three call sites (lines 236, 892, 947) using fmt.Sprintf. Consider adopting the helper internally for consistency.
[coherence] internal/cli/admin.go:838 — admin.go hardcodes the SA email pattern in a display path. With MintServiceAccountEmail now exported, this could be consolidated.
[ordering] internal/repos/uninstall.go:28 — The new entries FULLSEND_BOT_TOKEN_SECRET and FULLSEND_WIF_PROVIDER are inserted after FULLSEND_SA rather than in alphabetical order within the gitlabUninstallVars slice.

Labels: PR modifies dispatch provisioner code (internal/dispatch/gcf/provisioner.go), WIF credential management and Secret Manager token storage (component/mint), and user-facing docs (ADR, CLI docs, infrastructure reference).

Previous run (9)

Review

Findings

Medium

[stale-reference] internal/repos/uninstall.go:18 — The PR adds two new CI/CD variables via CreateProtectedCIVariable — FULLSEND_WIF_PROVIDER (in installProtectedVarsForForge) and FULLSEND_BOT_TOKEN_SECRET (in setupGitLabBotToken WIF path) — but neither is listed in gitlabUninstallVars. On uninstall, these variables will be orphaned on the GitLab project. FULLSEND_SA is already in the list and follows the same creation pattern, confirming these new variables should also be included.
Remediation: Add FULLSEND_BOT_TOKEN_SECRET and FULLSEND_WIF_PROVIDER to the gitlabUninstallVars slice in internal/repos/uninstall.go.

Low

[secret-id-length] internal/cli/repos_gitlab.go:35 — botTokenSecretID has no length validation. GCP Secret Manager IDs must be 1–255 characters. The prefix fullsend-bot-token- is 19 characters, leaving 236 for the sanitized owner-repo string. Additionally, the sanitizer could produce colliding secret IDs for repos like a/b-c and a-b/c since / is replaced with -.
Remediation: Add a length check after sanitization. Consider using a separator character (e.g., --) that distinguishes the owner/repo boundary.
[stale-secret-versions] internal/cli/repos_gitlab.go:86 — storeSecretManagerToken calls AddSecretVersion on every invocation (including re-installs) without disabling previous versions. Old PAT values accumulate as enabled secret versions in Secret Manager. The GCFClient interface already includes DisableSecretVersion.
Remediation: After adding a new version, disable prior versions.
[partial-failure] internal/cli/repos_gitlab.go:62 — In the WIF path, if storeSecretManagerToken succeeds but SetSecretIAMBinding or CreateProtectedCIVariable fails, the function returns an error without cleaning up the already-created secret. The re-install path handles idempotency (GetSecret checks existence), so this is an operational concern rather than a data-loss risk.
[coherence] internal/cli/repos_gitlab.go:67 — Service account email construction (fullsend-mint@ + projectID + .iam.gserviceaccount.com) is duplicated between setupGitLabBotToken and installProtectedVarsForForge in install.go:353.
Remediation: Extract SA email construction into a shared helper function.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (10)

Review

Findings

Medium

[logic-error] internal/repos/install.go:353 — installProtectedVarsForForge unconditionally includes FULLSEND_WIF_PROVIDER: cfg.WIFProvider in the returned map when cfg.InferenceProject != "". In the batch install ReuseSecrets path (batch_install.go line 399), WIFProvider is only computed when !d.secretsExist — when secrets already exist, it stays empty. This causes FULLSEND_WIF_PROVIDER to be written as an empty string via CreateProtectedCIVariable, overwriting any existing correct value and breaking WIF authentication in scaffold pipelines.
Remediation: Only include the FULLSEND_WIF_PROVIDER key when cfg.WIFProvider is non-empty:
```
vars := map[string]string{
    "FULLSEND_SA": "fullsend-mint@" + cfg.InferenceProject + ".iam.gserviceaccount.com",
}
if cfg.WIFProvider != "" {
    vars["FULLSEND_WIF_PROVIDER"] = cfg.WIFProvider
}
return vars
```
[stale-doc] docs/guides/infrastructure/infrastructure-reference.md:256 — The "Additional variables when --inference-project is provided (WIF mode)" section does not list FULLSEND_WIF_PROVIDER or FULLSEND_BOT_TOKEN_SECRET, which this PR adds as protected CI/CD variables.
Remediation: Add both variables to the WIF mode section.
[stale-doc] docs/guides/infrastructure/infrastructure-reference.md:248 — FULLSEND_FORGE_TOKEN is listed unconditionally under "Target repo CI/CD variables (protected)" for GitLab. After this PR, it is only written in variable mode — WIF mode stores in Secret Manager and sets FULLSEND_BOT_TOKEN_SECRET instead.
Remediation: Add a "(variable mode only)" qualifier.
[stale-doc] docs/cli/repos.md:144 — States the bot token is always "stored as the FULLSEND_FORGE_TOKEN CI/CD variable." After this PR, WIF mode stores in GCP Secret Manager instead.
Remediation: Update to note both storage paths.

Low

[naming-convention] internal/cli/repos_gitlab.go — BotTokenWIFConfig is exported but only used within the cli package. Other config structs in this file follow the unexported convention.
Remediation: Rename to botTokenWIFConfig.

Labels: PR modifies the install pipeline (internal/repos/install.go, internal/cli/repos_gitlab.go) and fixes bugs in GitLab WIF credential setup

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review · 2026-08-04T12:51:09Z

🤖 Finished Review · ✅ Success · Started 12:51 PM UTC · Completed 1:05 PM UTC
Commit: 632d9c2 · View workflow run →

fullsend-ai-review · 2026-08-04T13:16:39Z

🤖 Finished Review · ✅ Success · Started 1:16 PM UTC · Completed 1:35 PM UTC
Commit: fbcfab2 · View workflow run →

Superseded by updated review

fullsend-ai-review · 2026-08-04T13:40:35Z

🤖 Finished Review · ✅ Success · Started 1:40 PM UTC · Completed 2:00 PM UTC
Commit: d1c88d4 · View workflow run →

Superseded by updated review

…anager Signed-off-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

fullsend-ai-review · 2026-08-04T18:32:16Z

🤖 Finished Review · ✅ Success · Started 6:32 PM UTC · Completed 6:49 PM UTC
Commit: 39135d6 · View workflow run →

fullsend-ai-review · 2026-08-04T19:31:20Z

🤖 Finished Review · ✅ Success · Started 7:31 PM UTC · Completed 7:47 PM UTC
Commit: bd34606 · View workflow run →

fullsend-ai-review · 2026-08-04T20:03:16Z

🤖 Finished Review · ✅ Success · Started 8:03 PM UTC · Completed 8:18 PM UTC
Commit: cf34b83 · View workflow run →

fullsend-ai-retro · 2026-08-04T20:26:56Z

🤖 Finished Retro · ✅ Success · Started 8:26 PM UTC · Completed 8:41 PM UTC
Commit: cf34b83 · View workflow run →

fullsend-ai-retro · 2026-08-04T20:41:38Z

Retro: PR #5898 — fix(#5896): set FULLSEND_WIF_PROVIDER and store bot token in Secret Manager

Timeline

Time (UTC)	Event
11:55	Issue #5896 created by ggallen — 3 GitLab WIF bugs
11:59	Triage agent ran (high severity, cross-referenced #5877)
12:06	Code agent dispatched via `/fs-code`
12:30	PR #5898 opened (+582/−41, 12 files)
12:31	First review: found [medium] logic-error in WIF provider handling
12:46–15:13	6 fix agent runs dispatched — all failed at guard clause
12:50	ggallen confirmed logic-error fix manually
15:13–18:31	~3 hour gap — ggallen manually fixing review findings
18:31	Push with all fixes; review approved
19:30	ggallen approved
20:24	PR merged

Key findings

1. Bot-detection regex bug caused 6 wasted fix dispatches (evidence for #5536)

All 6 fix runs failed identically at the guard clause in reusable-fix.yml:306. The guard uses gh pr view --json author --jq '.author.login' which returns app/fullsend-ai-coder — this doesn't match the \[bot\]$ regex, so the workflow misclassifies the bot-authored PR as human-authored. The dispatch router (using github.event.pull_request.user.login = fullsend-ai-coder[bot]) correctly identified the PR and dispatched, but the fix guard re-checks via a different API that returns a different format. This forced ggallen to manually fix 3 real bugs over ~6 hours. This is the same root cause documented in #5536 and earlier #1569.

2. Silent guard-clause failures (evidence for #5863)

None of the 6 fix failures posted a PR comment. ggallen had no visible indicator that the fix agent was being blocked. The only signal was buried in workflow run logs.

3. Review agent quality was excellent

The review agent found 7 unique findings with 100% precision (0 false positives):

3 medium/high findings were genuine bugs fixed before merge: empty WIF provider in ReuseSecrets path, dots in GCP Secret Manager IDs (would fail at API level for all subgroup installations), overly permissive validation regex
4 low findings were real architectural debt, correctly scoped — ggallen tracked them in follow-up #5912
The [high] invalid-input finding (GCP secret IDs with dots) would have broken all GitLab subgroup installations in production

4. Review finding repetition across runs (evidence for #2959)

The same 4 low-severity findings were posted 5–6 times each across 11 review runs. The review agent correctly stopped flagging fixed issues but re-posted unchanged low-severity findings on every push.

Existing issue coverage

All improvement opportunities identified in this retro are covered by existing open issues:

Bot detection regex: #5536 (this PR provides strong new evidence — 6 failed fix runs, ~6 hours of manual human work)
Silent guard-clause feedback: #5863
Fix dispatch loop: #902, #5811
Review finding deduplication: #2959, #2816
Code agent plan adherence: #474, #423

fullsend-ai-coder Bot requested a review from a team as a code owner August 4, 2026 12:30

fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 4, 2026

fullsend-ai-coder Bot assigned ggallen Aug 4, 2026

github-actions Bot deployed to site-preview August 4, 2026 12:32 View deployment