Skip to content

fix(identity): close the check-then-act race on max_agents - #942

Open
AmirF194 wants to merge 1 commit into
tokencanopy:mainfrom
AmirF194:fix/agent-cap-concurrency-race
Open

fix(identity): close the check-then-act race on max_agents#942
AmirF194 wants to merge 1 commit into
tokencanopy:mainfrom
AmirF194:fix/agent-cap-concurrency-race

Conversation

@AmirF194

Copy link
Copy Markdown
Contributor

Summary

EnforceAgentCreate (the max_agents pre-check) and CreateAgent (the insert) ran as two independent operations with nothing serializing them: concurrent POST /v1/agents requests could all read the same pre-insert count and all pass. Reported with a live reproduction against real Postgres: 8 concurrent requests against a cap of 1, all 8 succeeded.

CreateAgentWithLimit now takes a per-user advisory lock (keyspace 2, distinct from claimOrCreateDomain's keyspace 1) and re-checks the count inside the same transaction as the INSERT, mirroring #901's fix for max_domains. handleCreateAgent calls it directly instead of EnforceAgentCreate + CreateAgent. EnforceAgentCreate stays wired, called only on the limit-exceeded path, to attach the plan/upgrade-URL details a bare AgentLimitExceededError doesn't carry; if it's unset or doesn't corroborate, the endpoint still answers 402 from the bare error.

Operational risk

No schema change, no behavior change on the non-concurrent path or in the 402 response shape (still limit_exceeded / resource agents, same fields). A request that previously slipped past the cap under a race now gets 402 limit_exceeded, the same response the endpoint already returns for a non-racy over-cap request.

internal/agent/oauth_handlers.go's issueOAuthCodeWithNewAgent (the OAuth auto-provisioning path) also creates an agent, via CreateAgentTx, and does not check max_agents at all, before or after this change. Left alone here (different mechanism, and I don't have context on the intended cap behavior for auto-provisioning); flagged in the issue.

Client surface checklist

Not applicable: internal concurrency fix, the request/response shape of POST /v1/agents is unchanged.

Test plan

  • New e2e test (internal/e2e/agents_create_race_e2e_test.go, -tags integration): 8 concurrent POST /v1/agents against max_agents=1. Fails on main (8/8 succeed, cap blown 8x), passes on this branch (1 created, 7 402 limit_exceeded), both against a real Postgres 16 container.
  • go test ./internal/identity/... and ./internal/httpapi/... green, including two new unit tests for the GetLimits-failure and enforcer-unwired error-mapping paths.
  • go test ./... (the Go tests job's own command) green.
  • internal/httpapi coverage 87.0%, above the package's 73% floor.
  • Not verified: contention beyond 8 concurrent requests, or across more than one connection pool / process.

Fixes #940

EnforceAgentCreate counted existing agents and CreateAgent inserted the
new one as two independent operations with nothing serializing them, so
concurrent POST /v1/agents requests could all read the same pre-insert
count and all pass. Reproduced against real Postgres: 8 concurrent
requests against a cap of 1, all 8 succeeded.

CreateAgentWithLimit takes a per-user advisory lock (keyspace 2, distinct
from claimOrCreateDomain's keyspace 1) and re-checks the count inside the
same transaction as the INSERT, mirroring tokencanopy#901's fix for max_domains.
EnforceAgentCreate stays wired on the limit-exceeded path only, to attach
the plan/upgrade-URL details a bare AgentLimitExceededError doesn't carry.

Fixes tokencanopy#940
@AmirF194
AmirF194 force-pushed the fix/agent-cap-concurrency-race branch from 49f252c to 70e3b1b Compare August 28, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

limits: max_agents is not enforced under concurrency (check-then-act)

1 participant