fix(hub): GCP SA verification can silently fail to persist, and start/restart never checked Verified

[ptone]

Behaviour change for existing agents

This changes behaviour on live systems. Any already-created agent whose assigned GCP service account has Verified=false in the database will now receive a 400 validation error on start or restart, where previously it would start and fail silently with exit code 1 within seconds. Stop and suspend are not affected.

Remedy: re-run verification on the service account (POST .../gcp-service-accounts/<id>/verify). The 400 error message names the SA email and states it is unverified, so the remedy is discoverable from the error alone.

This is the right trade — a loud 400 with an actionable message beats a silent exit 1, which is the recurring theme on this tier — but it must not arrive as a surprise on an instance the operator is actively testing on.

What changed

Defect 1 — Verification persist failure (both directions)

handlers_gcp_identity.go, runGCPServiceAccountVerification:

When VerifyImpersonation returns a result (pass or fail) and the subsequent store.UpdateGCPServiceAccount also fails, the database state does not match reality. Both directions now return HTTP 500 with a shared error code gcp_verification_persist_failed and an actionable message telling the operator to retry verification. The log lines distinguish the two directions ("verification failed AND the failure could not be persisted" vs. "verification succeeded but the success could not be persisted") so ops can tell which case occurred.

One error code is correct because the remedy is identical in both directions: retry verification. The reviewer independently agreed with this. The distinction (which direction failed) is in the log line and the response message, not the error code, because the caller's action is the same either way.

Failure-path persist failure: The _ = s.store.UpdateGCPServiceAccount(...) silently discarded the error. The SA remained Verified=true in the database while the API returned 502 "verification failed" — keeping the agent-creation gate open for an SA that could not mint tokens.

Success-path persist failure: The handler called writeErrorFromErr which returned a generic 500 "Internal server error". The operator had no idea the probe succeeded, that the SA was still unverified in the DB, or that retrying would fix it. With the lifecycle check (Defect 2) now in place, this would surface later as an opaque "not verified" error at start time with nothing connecting it to the earlier 500.

Defect 2 — Lifecycle handler missing Verified check

handlers_agent_lifecycle.go, handleAgentLifecycle:

createAgentInProject (:770) and updateAgent (:2245) both guard against unverified SAs, but handleAgentLifecycle (start/restart) did not. An SA marked unverified after agent creation could still be started. Now start and restart look up the assigned SA (when MetadataMode=assign and ServiceAccountID is set) and reject the request with a 400 validation_error if the SA is not verified. Stop and suspend are exempt — you must always be able to shut down an agent. The error message includes the SA email so the operator knows which SA to re-verify.

No evidence of deliberate omission was found. Searched: git log --all --oneline --grep='Verified' -- pkg/hub/handlers_agent_lifecycle.go (zero results), grep -n -i 'verified\|intentional\|deliberat\|skip.*check' in the handler (zero results near the lifecycle dispatch).

Why the _ = at lines 361/373 is left unchanged

The _ = sites at handlers_gcp_identity.go:361 and :373 (auto-verify during SA registration) are not changed by this PR. Those sites run during SA registration when the SA defaults to Verified=false. If auto-verify succeeds but the store update fails, the SA stays unverified — the operator must explicitly verify it, which is the normal flow. If auto-verify fails and the store update also fails, the SA stays unverified — same outcome. Both directions fail closed because a new SA starts unverified.

This is the distinction from the explicit re-verification path (line 607), where a previously-verified SA could remain marked as verified after a failed re-verification. At registration, there is no prior verified state to corrupt. The next reader should not have to re-derive this.

Full package suite result

go vet ./pkg/hub/ — clean (exit 0).

go test ./pkg/hub/ — 2 pre-existing failures, 0 failures introduced by this PR:

Failing test	Cause	Introduced by this PR?
TestTemplateResource_UATConfinement	authz_agent_baseline_test.go:568 — expects nil Decision for global template confinement, gets Allowed:false	No. Fails identically on main with no changes applied. Authz confinement logic, unrelated to GCP SA verification.
TestScopedAdmin_ProjectAdminDeniedUnboundProject	scoped_admin_test.go:387 — expects 403, gets 405	No. Fails identically on main with no changes applied. Scoped admin routing, unrelated to GCP SA verification.

Both failures reproduced on a clean checkout of main with git stash && go test -tags sqlite -run '...'. Neither test involves GCP service accounts, verification, or agent lifecycle. No existing test broke as a result of the new lifecycle guard — no fixture that previously scaffolded an agent with MetadataMode=assign and an unverified SA was affected.

Test results and mutation analysis

All five new tests pass. Mutation results:

Test	What it proves	Mutation (revert the fix)
TestVerification_PersistFailure_DoesNotReportCleanFailure	Failure-path: returns 500 gcp_verification_persist_failed, not 502 gcp_verification_failed	Reverts to _ =: handler falls through to 502 path → test fails (502 ≠ 500, wrong code)
TestVerification_SuccessPersistFailure_DoesNotReportCleanSuccess	Success-path: returns 500 gcp_verification_persist_failed, not 200	Reverts to writeErrorFromErr: status stays 500 but code becomes internal_error and message becomes "Internal server error" → test fails (wrong code and message)
TestAgentLifecycle_Start_UnverifiedSA	Start/restart rejected with 400 when SA is unverified; stop still succeeds	Removing Verified check: start returns 200 → test fails (200 ≠ 400)
TestAgentLifecycle_Start_VerifiedSA	Verified SA proceeds past the check (no false positive)	Removing check does not change outcome — regression guard against over-restriction
TestAgentLifecycle_Start_NoGCPIdentity	Agent without GCP identity doesn't panic on nil guard chain	Removing check does not change outcome — nil-safety regression guard