feat(mint): add add-role and remove-role CLI commands

[ifireball]

Summary

• Add fullsend mint add-role with three input modes: --slug + --pem, --slug + --use-existing-pem-secret, or --org (browser manifest flow)
• Add fullsend mint remove-role to prune ROLE_APP_IDS / ALLOWED_ROLES and delete PEM secrets by default (--keep-pem to retain)
• Extend the GCP provisioner with AddRoleToMint, RemoveRoleFromMint, and DeleteAgentPEM
• Document commands and IAM requirements in docs/guides/infrastructure/mint-administration.md

Test plan

• go test ./internal/dispatch/gcf/... ./internal/cli/...
• fullsend mint add-role coder --project=... --slug=... --pem=... --dry-run
• fullsend mint remove-role <role> --project=... --dry-run

Made with Cursor

[github-actions]

Site preview

Preview: https://27c88a24-site.fullsend-ai.workers.dev

Commit: 98069730ea8dfc727c231bcd368e5215dcb0f710

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:45 AM UTC · Completed 7:00 AM UTC
Commit: b405b36 · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 86.81055% with 55 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/mint_setup.go	86.25%	28 Missing and 19 partials ⚠️
internal/dispatch/gcf/provisioner.go	87.69%	4 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Low

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. Matches existing provisioner pattern (EnsureOrgInMint has explicit WARNING). Not a regression.

• [fail-open] internal/cli/mint_setup.go — mintTrafficRoleAppIDs falls back to discovery.RoleAppIDs (template env vars) when GetServiceTrafficEnvVars returns an error. The code now logs a warning via printer.StepWarn, which is an improvement over the prior review. The fallback is pragmatic — template and traffic env vars typically match, and downstream impact is limited.

• [permission-scope] internal/dispatch/gcf/provisioner.go — DeleteAgentPEM permanently deletes a Secret Manager secret. First code path performing irreversible secret deletion. The CLI gates this behind interactive confirmation unless --yolo is set, and the error message includes a gcloud secrets delete remediation command.

• [naming-convention] internal/cli/mint_setup.go — Functions use MintSetup infix (validateMintSetupRole, runMintSetupAddRole, runMintSetupRemoveRole, mintSetupAddRoleConfig) instead of the established runMint{Verb} pattern (e.g., runMintEnrollOrg, runMintStatus). Expected: runMintAddRole, runMintRemoveRole, mintAddRoleConfig.

• [code-organization] internal/cli/mint_setup.go — New file separates add-role/remove-role from mint.go. While minttoken.go provides precedent for splitting, the naming is inconsistent — minttoken.go splits a fundamentally different concern (runtime token minting) while add-role/remove-role are infrastructure lifecycle operations like deploy, enroll, and unenroll which remain in mint.go. Consider mint_role.go if splitting.

• [incomplete-doc] skills/mint-enroll/SKILL.md:85 — States PEMs and app IDs must exist from mint deploy --pem-dir or fullsend admin install but does not mention mint add-role as an alternative method for creating role PEMs and app IDs.

• [scope-alignment] internal/cli/mint_setup.go — The browser-based GitHub App creation flow (--org mode) delegates to the existing runAppSetup infrastructure. The three input modes are mutually exclusive, well-documented in mint-administration.md, and covered by tests. Architectural observation — not a defect.

Info

• [missing-authorization] — No linked issue for this non-trivial feature PR. The change is architecturally coherent (aligns with ADR-0029 and existing mint patterns), and the PR carries a ready-for-merge label.

• [error-handling] internal/cli/mint_setup.go — Prior finding resolved: runMintSetupRemoveRole now calls RemoveRoleFromMint before DeleteAgentPEM, which is the correct order. If PEM deletion fails, the role is already removed from the mint so no requests reach the missing PEM. The error message includes a gcloud secrets delete remediation.

• [injection-fix] internal/cli/mint.go:112 — The slug parameter passed to lookupAppID is now properly URL-path-escaped via url.PathEscape. Positive security fix.

• [permission-scope] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint expand the write surface for Cloud Run env vars. Both validate role name and project ID. Consistent with existing codebase patterns.

Previous run

Review

Findings

Low

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. Matches existing provisioner pattern (EnsureOrgInMint has explicit WARNING). Not a regression.

• [fail-open] internal/cli/mint_setup.go:493 — mintTrafficRoleAppIDs falls back to discovery.RoleAppIDs (template env vars) when GetServiceTrafficEnvVars returns an error. The code now logs a warning via printer.StepWarn, which is an improvement over the prior review. The fallback is pragmatic — template and traffic env vars typically match, and downstream impact is limited.

• [permission-scope] internal/dispatch/gcf/provisioner.go — DeleteAgentPEM permanently deletes a Secret Manager secret. First code path performing irreversible secret deletion. The CLI gates this behind interactive confirmation unless --yolo is set, and the error message includes a gcloud secrets delete remediation command.

• [incomplete-doc] skills/mint-enroll/SKILL.md:85 — States PEMs and app IDs must exist from mint deploy --pem-dir or fullsend admin install but does not mention mint add-role as an alternative method for creating role PEMs and app IDs.

• [naming-convention] internal/cli/mint_setup.go — Functions use MintSetup infix (validateMintSetupRole, runMintSetupAddRole, runMintSetupRemoveRole, mintSetupAddRoleConfig) instead of the established runMint{Verb} pattern (e.g., runMintEnrollOrg, runMintStatus). Expected: runMintAddRole, runMintRemoveRole, mintAddRoleConfig.

• [code-organization] internal/cli/mint_setup.go — New file separates add-role/remove-role from mint.go. While minttoken.go provides precedent for splitting, the naming is inconsistent — minttoken.go splits a fundamentally different concern (runtime token minting) while add-role/remove-role are infrastructure lifecycle operations like deploy, enroll, and unenroll which remain in mint.go. Consider mint_role.go if splitting.

Info

• [error-handling] internal/cli/mint_setup.go — Prior finding resolved: runMintSetupRemoveRole now calls RemoveRoleFromMint before DeleteAgentPEM, which is the correct order. If PEM deletion fails, the role is already removed from the mint so no requests reach the missing PEM. The error message includes a gcloud secrets delete remediation.

• [injection-fix] internal/cli/mint.go:112 — The slug parameter passed to lookupAppID is now properly URL-path-escaped via url.PathEscape. Positive security fix.

• [permission-scope] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint expand the write surface for Cloud Run env vars. Both validate role name and project ID. Consistent with existing codebase patterns.

Previous run (2)

Review

Findings

Low

• [fail-open] internal/cli/mint_setup.go:822 — mintTrafficRoleAppIDs falls back to discovery.RoleAppIDs (template env vars) when GetServiceTrafficEnvVars returns an error. The code now logs a warning via printer.StepWarn, which is an improvement over the prior review. The fallback is pragmatic — template and traffic env vars typically match, and downstream impact is limited (add-role with stale data is recoverable via --force; remove-role would fail safely with "not registered").

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. Matches existing provisioner pattern (EnsureOrgInMint has explicit WARNING). Not a regression.

• [permission-scope] internal/dispatch/gcf/provisioner.go — DeleteAgentPEM permanently deletes a Secret Manager secret. First code path performing irreversible secret deletion. The CLI gates this behind interactive confirmation unless --yolo is set, and the error message includes a gcloud secrets delete remediation command.

• [naming-convention] internal/cli/mint_setup.go — Functions use MintSetup infix (validateMintSetupRole, runMintSetupAddRole, runMintSetupRemoveRole, mintSetupAddRoleConfig) instead of the established runMint{Verb} pattern (e.g., runMintEnrollOrg, runMintStatus). Expected: runMintAddRole, runMintRemoveRole, mintAddRoleConfig.

• [code-organization] internal/cli/mint_setup.go — New file separates add-role/remove-role from mint.go. While minttoken.go provides precedent for splitting, the naming is inconsistent — minttoken.go splits a fundamentally different concern (runtime token minting) while add-role/remove-role are infrastructure lifecycle operations like deploy, enroll, and unenroll which remain in mint.go. Consider mint_role.go if splitting.

• [incomplete-doc] skills/mint-enroll/SKILL.md:84 — States PEMs and app IDs must exist from mint deploy --pem-dir or fullsend admin install but does not mention mint add-role as an alternative method for creating role PEMs and app IDs.

Info

• [error-handling] internal/cli/mint_setup.go:787 — Prior finding resolved: runMintSetupRemoveRole now calls RemoveRoleFromMint before DeleteAgentPEM, which is the correct order. If PEM deletion fails, the role is already removed from the mint so no requests reach the missing PEM. The error message includes a gcloud secrets delete remediation.

• [injection-fix] internal/cli/mint.go:112 — The slug parameter passed to lookupAppID is now properly URL-path-escaped via url.PathEscape. Positive security fix.

• [permission-scope] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint expand the write surface for Cloud Run env vars. Both validate role name and project ID. Consistent with existing codebase patterns.

Previous run (3)

Review

Findings

Medium

• [error-handling] internal/cli/mint_setup.go:787 — In runMintSetupRemoveRole, DeleteAgentPEM runs before RemoveRoleFromMint. If DeleteAgentPEM succeeds but RemoveRoleFromMint fails, the role remains in ROLE_APP_IDS/ALLOWED_ROLES but the PEM secret is gone. The mint will accept token requests for that role but fail to sign them at runtime. Re-running remove-role recovers (DeleteSecret is idempotent for 404), but there is a window of runtime failures between the partial failure and re-run.
  Remediation: Reverse the operation order — call RemoveRoleFromMint first (removing the role from env vars stops the mint from serving it), then call DeleteAgentPEM. An orphaned PEM secret is benign (just storage), while a missing PEM with an active role causes runtime failures.

• [fail-open] internal/cli/mint_setup.go:822 — mintTrafficRoleAppIDs silently falls back to discovery.RoleAppIDs when GetServiceTrafficEnvVars returns an error. If the traffic-serving revision has different roles than the function template, the guard checks may return a stale answer, potentially allowing force-less overwrites of a live role or blocking removal of a role that appears absent but is actually live. The error is suppressed without any log or user-visible warning.
  Remediation: At minimum, log the GetServiceTrafficEnvVars error so operators see it. Preferably, return the error rather than silently falling back.

Low

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. Matches existing provisioner pattern (EnsureOrgInMint has explicit WARNING). Not a regression.

• [permission-scope] internal/dispatch/gcf/provisioner.go — New method DeleteAgentPEM permanently deletes a Secret Manager secret. This is the first code path that performs irreversible secret deletion. The CLI now requires roles/secretmanager.admin for the remove-role path.

• [permission-scope] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint expand the write surface for Cloud Run env vars. The existing WARNING about concurrent writes applies equally here.

Info

• [injection-fix] internal/cli/mint.go:112 — The slug parameter passed to lookupAppID is now properly URL-path-escaped via url.PathEscape. Positive security fix.

Previous run (4)

Review

Findings

High

• [protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The PR has no linked issue justifying changes to governance/infrastructure files. Human approval is always required for protected-path changes; without a linked issue the justification is insufficient.

Medium

• [error-handling] internal/cli/mint_setup.go — In runMintSetupRemoveRole, RemoveRoleFromMint succeeds before DeleteAgentPEM is attempted. If DeleteAgentPEM fails, the role is already removed from ROLE_APP_IDS/ALLOWED_ROLES but the PEM secret is orphaned. Re-running remove-role will fail with "role is not registered on the mint" because the guard check passes first, leaving no CLI path to clean up the orphaned secret.
  Remediation: Either (1) reverse the operation order (delete PEM first, then remove role), or (2) catch the "not registered" error on re-run and still attempt PEM deletion, or (3) emit a clear remediation message telling the user to manually delete the secret via gcloud secrets delete.

• [missing-documentation] docs/guides/infrastructure/infrastructure-reference.md:7 — The infrastructure reference document lists mint management commands but does not include the new mint add-role and mint remove-role commands. The line reads "Managed by: fullsend mint deploy, fullsend mint enroll, ..." but omits the two new role management commands.
  Remediation: Update line 7 to include fullsend mint add-role and fullsend mint remove-role in the command list.

Low

• [error-handling] internal/cli/mint_setup.go — mintTrafficRoleAppIDs silently falls back to discovery.RoleAppIDs (template env vars) when GetServiceTrafficEnvVars fails. The "already registered" / "not registered" guard could use stale data during transient Cloud Run API failures. The fallback swallows the error without logging.

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. Matches existing provisioner pattern (EnsureOrgInMint has explicit WARNING). Not a regression.

• [code-organization] internal/cli/mint_setup.go — New file separates add-role/remove-role from mint.go. While minttoken.go provides precedent for splitting, the naming (mint_setup.go for role lifecycle operations) is inconsistent — deploy (also a setup operation) remains in mint.go.

• [naming-convention] internal/cli/mint_setup.go — Multiple functions use MintSetup infix (validateMintSetupRole, runMintSetupAddRole, runMintSetupRemoveRole, mintSetupAddRoleConfig), deviating from the established runMint{Verb} pattern (e.g., runMintEnrollOrg, runMintStatus).

• [stale-doc] docs/architecture.md:128 — The architecture document describes the per-role GitHub Apps model and PEM secrets but does not reference the new mint add-role/remove-role commands for managing role credentials after initial setup.

Info

• [missing-authorization] No linked issue for this non-trivial feature PR. The change is architecturally coherent (aligns with ADR-0029 and existing mint patterns), but non-trivial changes benefit from explicit authorization via a linked issue.

Previous run (5)

Review

Findings

Medium

• [error-handling] internal/cli/mint_setup.go:408 — In runMintSetupRemoveRole, RemoveRoleFromMint succeeds before DeleteAgentPEM is attempted. If DeleteAgentPEM fails, the role is already removed from ROLE_APP_IDS/ALLOWED_ROLES but the PEM secret is orphaned. Re-running remove-role will fail with "role is not registered on the mint" because the guard check passes first, leaving no CLI path to clean up the orphaned secret.
  Remediation: Either (1) attempt PEM deletion first (safe — the mint only reads PEMs at token-minting time, not at startup), or (2) catch the "not registered" error on re-run and still attempt PEM deletion, or (3) warn the user with a specific message about manual cleanup when PEM deletion fails.

Low

• [error-handling] internal/cli/mint_setup.go:459 — mintTrafficRoleAppIDs silently falls back to discovery.RoleAppIDs (template env vars) when GetServiceTrafficEnvVars fails. The "already registered" / "not registered" guard could use stale data during transient Cloud Run API failures. The fallback swallows the error without logging.

• [stale-doc] skills/mint-enroll/SKILL.md:85 — States PEMs and app IDs must exist from "mint deploy --pem-dir or fullsend admin install" but does not mention mint add-role as an alternative method for creating role PEMs and app IDs.

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. Matches existing provisioner pattern (EnsureOrgInMint has explicit WARNING). Not a regression.

Previous run (6)

Review

Findings

Medium

• [error-handling] internal/cli/mint_setup.go:408 — In runMintSetupRemoveRole, RemoveRoleFromMint succeeds before DeleteAgentPEM is attempted. If DeleteAgentPEM fails, the role is already removed from ROLE_APP_IDS/ALLOWED_ROLES but the PEM secret is orphaned. Re-running remove-role will fail with "role is not registered on the mint" because the guard check passes first, leaving no CLI path to clean up the orphaned secret.
  Remediation: Either (1) attempt PEM deletion first (safe — the mint only reads PEMs at token-minting time, not at startup), or (2) catch the "not registered" error on re-run and still attempt PEM deletion, or (3) warn the user with a specific message about manual cleanup when PEM deletion fails.

Low

• [error-handling] internal/cli/mint_setup.go:459 — mintTrafficRoleAppIDs silently falls back to discovery.RoleAppIDs (template env vars) when GetServiceTrafficEnvVars fails. The "already registered" / "not registered" guard could use stale data during transient Cloud Run API failures. The fallback swallows the error without logging.

• [stale-doc] skills/mint-enroll/SKILL.md:85 — States PEMs and app IDs must exist from "mint deploy --pem-dir or fullsend admin install" but does not mention mint add-role as an alternative method for creating role PEMs and app IDs.

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. Matches existing provisioner pattern (EnsureOrgInMint has explicit WARNING). Not a regression.

Previous run (7)

Review

Findings

High

• [logic-error] internal/cli/mint_setup.go:458 — resolveAddRoleFromBrowser has no dry-run guard. With --dry-run --org, the browser flow runs unconditionally — creating a real GitHub App and storing its PEM in Secret Manager via runAppSetup — but only the final AddRoleToMint call is skipped. This leaves the system in an inconsistent state (PEM created, ROLE_APP_IDS not updated). The --slug --pem mode correctly handles dry-run by returning early before StoreAgentPEM (line 522).
  Remediation: Add a dry-run guard at the top of resolveAddRoleFromBrowser (or before the switch on mode in runMintSetupAddRole) that skips the entire browser flow and returns a placeholder/zero app ID.

Medium

• [data-consistency] internal/cli/mint_setup.go:444 — Both add-role and remove-role check "already registered" / "not registered" using DiscoverMint (template env vars from GetFunction), but AddRoleToMint/RemoveRoleFromMint operate on GetServiceTrafficEnvVars (traffic revision). When the template and traffic revision diverge, the guard and the operation use different data sources, leading to confusing behavior (e.g., "already registered" error when the live mint is not serving that role).
  Remediation: Read ROLE_APP_IDS from the traffic-serving revision for the guard check, or add traffic ROLE_APP_IDS to the MintDiscovery struct.

• [error-handling] internal/cli/mint_setup.go:631 — remove-role reuses confirmUnenroll for its interactive confirmation, resulting in the error message "aborting unenroll" instead of "aborting remove-role" when the user types the wrong value.
  Remediation: Parameterize confirmUnenroll to accept a custom operation name, or create a separate confirmation function.

• [incomplete-doc] docs/reference/installation.md:613 — The standalone commands table and the per-command IAM role breakdown table both list mint commands but are missing the new mint add-role and mint remove-role commands.
  Remediation: Add rows for both commands to the commands table, and add columns with their IAM requirements to the IAM breakdown table.

• [incomplete-doc] docs/guides/dev/cli-internals.md:17 — The CLI command tree lists mint subcommands but is missing add-role and remove-role.
  Remediation: Add entries under the mint command section.

Low

• [code-organization] internal/cli/mint_setup.go — New file separates add-role/remove-role from mint.go without rationale. All existing mint commands live in mint.go. The name mint_setup.go is also inconsistent — deploy (also a setup operation) remains in mint.go.

• [stale-doc] skills/mint-enroll/SKILL.md:84 — States PEMs and app IDs must exist from "mint deploy --pem-dir or fullsend admin install" but does not mention mint add-role as an alternative.

• [naming-convention] internal/cli/mint_setup.go — Multiple functions use MintSetup infix (validateMintSetupRole, runMintSetupAddRole, runMintSetupRemoveRole, mintSetupAddRoleConfig), deviating from the established runMint{Verb} pattern (e.g., runMintEnrollOrg, runMintStatus).

• [input-validation] internal/cli/mint_setup.go — The --slug flag value is concatenated into a URL path in lookupAppID without URL encoding. Impact is limited (local CLI, unauthenticated GET), but a trivial fix.

• [test-inadequate] internal/dispatch/gcf/provisioner_test.go:3133 — TestDeleteAgentPEM_FixRoleUsesCoderSecret claims to verify the fix→coder secret mapping but only asserts DeleteSecret was called, not that the correct secret ID (fullsend-coder-app-pem) was passed.

• [code-organization] internal/dispatch/gcf/provisioner.go:854 — New functions inserted between StoreAgentPEM and MintDiscovery rather than grouped with other mint management operations.

Info

• [missing-authorization] No linked issue for this non-trivial feature PR. The change is architecturally coherent (aligns with ADR-0029 and existing mint patterns), but non-trivial changes benefit from explicit authorization via a linked issue.

• [race-condition] internal/dispatch/gcf/provisioner.go — AddRoleToMint and RemoveRoleFromMint use read-modify-write on Cloud Run env vars without locking. This matches the existing provisioner pattern (EnsureOrgInMint has an explicit WARNING). Not a new regression.

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 8:24 AM UTC · Ended 8:36 AM UTC
Commit: 4e21a60 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 8:40 AM UTC · Ended 8:49 AM UTC
Commit: 4e21a60 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:53 AM UTC · Completed 9:08 AM UTC
Commit: d8c20b3 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:12 AM UTC · Completed 9:24 AM UTC
Commit: 543d3ce · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:38 AM UTC · Completed 10:51 AM UTC
Commit: a4d5818 · View workflow run →

[waynesun09]

Review Squad — 9 agents (3x Claude, 2x Gemini, 2x Codex, 2x Claude-researcher)

4 new inline findings (all MEDIUM) after deduplication against existing review comments. The 5 findings already posted by fullsend-ai-review[bot] and ralphbean are not duplicated here.

Existing comments already cover: TOCTOU/data-consistency (line 472), non-atomic remove-role (line 418), race condition (provisioner.go:271), mintTrafficRoleAppIDs fallback (lines 471, 493), browser dry-run (line 486), slug validation (line 327).

Overall: well-structured feature with thorough tests (~780 new lines) and documentation. No blocking correctness or security issues on the happy path.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:53 PM UTC · Completed 4:07 PM UTC
Commit: 9806973 · View workflow run →

[waynesun09]

All review findings resolved — dry-run guards, traffic env var consistency, slug validation, PEM verification warnings, IAM docs, and failure remediation messages all addressed. CI green, tests comprehensive. LGTM.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 12:06 AM UTC · Completed 12:15 AM UTC
Commit: 9806973 · View workflow run →

[fullsend-ai-retro]

Retro: PR #2370 — feat(mint): add add-role and remove-role CLI commands

This was a well-executed human-authored PR (Cursor-assisted, 8 commits, 1,869 additions across 10 files). The review bot performed well on review quality — it caught the highest-severity issue (missing dry-run guard in browser flow) on the first review pass, and the author addressed all bot findings across subsequent commits.

What went well

• Review quality: The bot's first review found a real high-severity logic error (dry-run guard missing) and a medium data-consistency issue (template vs traffic env var divergence). Both were fixed promptly.
• Concurrency groups worked: Two review runs were correctly cancelled when new commits arrived before they completed, preventing full token waste.
• Human + bot collaboration: Human reviewer (ralphbean) caught a slug validation gap the bot missed. The Review Squad (9 agents via waynesun09) found 4 additional medium findings (PEM verification, misleading error, IAM docs gap, orphaned PEM). The author addressed all findings in commit 9806973.

Token waste observed

• 2 cancelled review runs consumed partial tokens (~12-16 min each) before cancellation. Already tracked by #1014 (debounce) and #1422 (dedup on rapid pushes).
• 1 duplicate review on commit a4d5818 — triggered by ready_for_review event after the same SHA was already bot-approved. Already tracked by #963 (skip dispatch when SHA already approved).
• Total waste: ~2-2.5x a full review's token cost out of 9-10 total runs.

Review quality gaps (already tracked)

• Bot missed slug input validation — covered by #2102.
• Review Squad found cross-file findings the single bot missed — covered by #1525.
• Bot missed mode-specific behavioral implications (misleading shared error message) — covered by #1556.

Proposals

No new proposals — all identified improvement areas are covered by existing open issues. The most impactful existing issues for this workflow's pain points are #963 (SHA-level dedup, would have saved 1 full review run) and #1014 (debounce, would have reduced cancelled-run waste).