fix(cli): warn that a GCP key may exist when CreateKey fails ambiguously

[cristim]

Closes #2117

This is item 4, the last one. Items 1 and 2 merged in #2233 and item 3 merged in #2240.

Summary

An interrupt or an ambiguous failure during the GCP CreateKey RPC can still leave a key minted in GCP: the server may have committed it before the client gave up. The CLI used to report a plain failure, which implied no key exists.

There is now a new createKeyError:

• Only a *googleapi.Error with a 4xx code, matched through errors.As, counts as a definite failure. That error is returned as before.

• Everything else is treated as ambiguous: a canceled context, an exceeded deadline, a 5xx, or any non-googleapi error.

• An ambiguous failure keeps the original error (wrapped with %w) and adds this text:

  the key may have been created before the request ended: list keys with gcloud iam service-accounts keys list --iam-account=<sa> --managed-by=user and delete any key created just now with gcloud iam service-accounts keys delete <KEY_ID> --iam-account=<sa>

This only reports the possible orphan; the CLI doesn't find or delete it. Auto-reconciling would need a new ListKeys permission, and it could delete a key that another operator created at the same time.

CHANGELOG.md gets one (#2117) entry covering all three PRs:

• the minted key is deleted when the upload panics;
• a second Ctrl-C aborts cleanup, and the local key file is removed first;
• a possibly orphaned key is reported.

Evidence (local, macOS, GOTOOLCHAIN=go1.26.9, using fakes and the signal-test harness, no GCP calls)

• Fail before / pass after: with configure_gcp.go reverted to origin/main, TestWriteServiceAccountKey_CreateFailureAmbiguous (500, context.Canceled and network-error cases) and TestRunConfigureGCP_InterruptDuringMint fail. Both pass with the change. The child process checks the "may have been created" text after its Canceled guard.
• Definite path: CreateFailureNoOrphan now uses googleapi.Error{Code: 403} and asserts the warning text is absent.
• Mutation probes: treating 5xx as definite fails the 500 case, and dropping --managed-by=user fails the command assertion.
• Checks:
  • make build, go vet ./... and go test -race ./cmd/... pass.
  • The GCP and interrupt tests pass with -count=5: 440 passed.
  • golangci-lint v2.10.1 reports 0 issues.
  • scripts/test-git-secrets-allowlist.sh passes 44/44.
• Review: the plan got two adversarial passes, which tightened the "definite" rule from any googleapi error to 4xx only, because a 5xx can follow a committed mint. An independent non-author reviewer (sonnet, high) then reviewed this diff in two passes, checked the CHANGELOG against merged fix(cli): delete the minted GCP key when the credentials upload panics #2233 and fix(cli): let a second Ctrl-C abort the GCP key cleanup #2240, and found no actionable findings at 9ab43925.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 31 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 65 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: be3580a1-50c2-4375-9a8e-3058490d8235

📥 Commits

Reviewing files that changed from the base of the PR and between 96ecab1 and 961d56f.

📒 Files selected for processing: 4

• CHANGELOG.md
• cmd/configure_gcp.go
• cmd/configure_gcp_signal_test.go
• cmd/configure_gcp_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review verdict: CLEAN at a4e5d41

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

How CreateKey errors are classified (cmd/configure_gcp.go:568-579, createKeyError):

Error	Class
*googleapi.Error 400-499 via errors.As (403, 400, 429)	Definite, no warning
googleapi 5xx	Ambiguous, warns
context.Canceled / DeadlineExceeded (incl. gcpSDKCallTimeout), url/transport errors	Ambiguous, warns

Verified:

• One call path, right account: the only path is :955 -> :611 -> :649. The SA email in the warning is the same variable that is passed to CreateKey.
• The error reaches the user: it is joined with the removeMintedGCPKey result and returned from RunE through cmd/main.go:89.
• Interrupts: a first Ctrl-C gives context.Canceled, so the warning prints. TestRunConfigureGCP_InterruptDuringMint asserts "may have been created" in the child process. A second Ctrl-C kills the process by signal (fix(cli): let a second Ctrl-C abort the GCP key cleanup #2240), so the warning may not print, which is acceptable for a forced abort.

Non-blocking note: 408 is 4xx, so it is treated as definite. It is a client-side timeout that could in theory follow a successful mint, but that is unlikely.

What was attacked and how (no GCP calls):

• Focused GCP and ServiceAccountKey tests pass with -race, including the 4 new subtests and InterruptDuringMint.
• The full ./cmd run has one failure, TestCSVRealPurchase_FailedPurchaseCarriesError. That was the red-main failure fixed by fix(cmd): repair red main after #2238 and drop the RI hint on SP-only runs #2243 (96ecab1), so this branch is being updated onto main.
• Fail-before: with configure_gcp.go reverted to main, InterruptDuringMint and the 3 ambiguous subtests fail.
• Mutation probes, both killed:
  • Treating 4xx as ambiguous fails NoOrphan and the 403 subtest.
  • Treating everything as definite fails InterruptDuringMint and the 500, canceled and non-googleapi subtests.

Gate: labels are mirrored from #2117. Merge after an update onto main and green CI.

[cristim]

Rebased onto main (includes #2243's red-main fix). git range-diff of a4e5d41 against new head 2153b28 shows the commit as identical (=), so the CLEAN verdict carries to 2153b28. (claude-cc-reviewer)

[cristim]

Rebase note [claude-cli-impl]: the first CI run on this PR failed in TestCSVRealPurchase_FailedPurchaseCarriesError. That failure came from main: #2238 turned main red and #2243 repaired it. I rebased onto the repaired main (96ecab1) with no content change. The PR head is now 961d56f, the targeted tests pass locally, and CI is re-running.

[cristim]

Head moved to 961d56f (claude-cli-impl rebased onto 96ecab1, same base as my rebase). git range-diff against the reviewed a4e5d41 shows the commit as identical (=), so the CLEAN verdict carries to 961d56f. (claude-cc-reviewer)

[cristim]

CR waived: quota, adversarial review + local verification + green CI (per CLAUDE.md:172-173). CodeRabbit posted 'Review limit reached'. Gate at 961d56f: CLEAN verdict (carried by range-diff =), CI green, mergeability CLEAN. (claude-cc-reviewer)