First-pass review (Netero only). This is a mechanical first pass, so the full review panel has not yet looked at this PR. The panel will review once these findings are addressed. Posting these early so they don't cost parallel review time later.

The change is well-tested: 76% test density, and the SQL query has genuine coverage for period normalization, cross-org exclusion, and cross-user aggregation (NormalizesNonUTCPeriodStart and ExcludesSpendBeforePeriodStart would fail without the query's date handling). The shared AISpendPeriodWindow reuse across UserAISpendStatus and the new response type is clean, and the route/handler/SDK/generated code all line up.

Severity count: 1 P2, 1 Note.

The P2 is the one to weigh before the panel round. As Netero put it: "This is authorization logic, so the duplication is a drift hazard." The new row's RBACObject rebuilds the same ACL that Group.RBACObject already produces, so a future change to group read authorization would land on GET /organizations/{org}/groups but silently miss this endpoint. Sibling rows (GetGroupsRow) avoid this by delegating.

The Note is a design question your own inline comment already raises: the role-access test asserts every role (including a plain Member) sees the group, so it never exercises the filtered-out path. Worth deciding whether a role should be excluded and testing that.

🤖 This review was automatically generated with Coder Agents.

Panel round (first full-panel pass). Strong PR overall: the SQL aggregate is correct by construction (the LEFT JOIN on group_ai_budgets can't fan out the spend SUM because the budget PK is group_id), cross-org isolation is enforced at both the SQL WHERE clause and the dbauthz post-filter, and the raw-SQL test layer genuinely hunts edges (cross-org exclusion, id-not-in-list, period boundary, cross-user aggregation, non-UTC normalization). The AISpendPeriodWindow extraction is a clean, minimal refactor with no wire-format change, and CRF-1 was fixed correctly by delegating to Group.RBACObject.

But the CRF-2 fix regressed. The OtherOrgMember case added to revive the dead empty-response branch fails deterministically: I ran TestOrganizationGroupsAISpendRoleAccess on head 4b4aa4d and OtherOrgMember returns 404 (blocked at the org-membership middleware, before the handler), while the other five cases pass. So CI is red and the branch CRF-2 targeted is still dead. That is a merge blocker.

Two P2s follow. The license/experiment gate tests don't test their gates: the route nests under templateRBACEnabledMW, so both negative tests 403 at the parent FeatureTemplateRBAC check before reaching the FeatureAIBridge/experiment gates they name (Ryosuke proved it by deleting each gate, the tests stayed green). And your own open question about member visibility is now a concrete disclosure decision a human must make: on a default deployment any org member can read every group's aggregate AI spend.

Severity count: 1 P1, 2 P2, 5 P3, 1 Nit, 4 Note.

Hisoka, who found nothing to break, put the shape of it well: "I came looking for a fight. I fought the join, the clock, the routing, the embedding, and the authz filter. None of them broke." The engine is sound. The instrument panel (the two gate tests) reads green while measuring nothing, and one committed test is red.

enterprise/coderd/coderd.go:607

P3 [CRF-7] The sibling /groups/{group}/ai/budget route lacks the ExperimentAIGatewayCostControl gate that every other cost-control route carries (Razor P3, Meruem P3)

The new spend route and the user AI routes gate on both the experiment and FeatureAIBridge (with the TODO(AIGOV-443) kill-switch comment). The group budget CRUD routes at coderd.go:607-613 gate on FeatureAIBridge only. Consequence: on a licensed deployment with the experiment off, operators can still create/read/delete group budgets while reading spend returns 403, an incomplete kill-switch. This route is outside this PR's diff, but you're actively extending this surface, so you're the right person to reconcile it. The class fix is to hang the experiment gate on the cost-control boundary once rather than per leaf route. A human should decide whether the divergence is intended.

🤖

🤖 This review was automatically generated with Coder Agents.

Re-review paused (churn guard blocked). Solid progress on f402a88: of the 12 open findings, 9 have code fixes and 1 (CRF-5, member visibility) was accepted by @johnstcn as fine as-is. The panel hasn't re-verified those fixes yet, because two findings are still silent (no code change and no response), and the review process holds the panel until every finding gets at least a reasoned answer.

Two items need a word from you before the panel re-runs:

• CRF-7 (P3) - the sibling /groups/{group}/ai/budget route (coderd.go:607-613) gates on FeatureAIBridge only, without the ExperimentAIGatewayCostControl gate that this PR's spend route and the user AI routes carry. It's outside this PR's diff, so a fix here is optional, but it needs a disposition: fix it, file a follow-up ticket, or say why the divergence is intended.

• CRF-12 (Note) - the aicostcontrol.sql comment says spend is aggregated "until NOW" while the query has no upper bound. It's correct today and mirrors GetUserAISpendSince, so a won't-fix is entirely reasonable; it just needs to be stated (a one-line "acknowledged, matches sibling, leaving as-is" clears it).

Neither requires a code change. A short reply on each thread (or a ticket link for CRF-7) unblocks the next round, at which point the panel will verify the nine fixes and re-check CRF-3/CRF-4 against the code.

🤖 This review was automatically generated with Coder Agents.