fix-forward #4004 (tsk-pxwdmj): make the one-open-request-per-kind check atomic inside AuthBrokerStore.create

changelog.d/tsk-4bw2ew-signin-broker-cap.md

@@ -0,0 +1,2 @@
+### Fixed
+- Sign-in broker now allows only one open request per agent per kind. A second `POST /api/auth-broker/requests` for the same agent and kind while the first is pending returns `409` with detail "a sign-in request is already open for this agent" and raises no notification.

docs/agent-coordination.md

@@ -2355,14 +2355,16 @@ is not usable inside a container until that follow-up lands. Store:
    `{"kind": "claude-subscription", "url": "...", "prompt": "..."}`. The url
    must be `https` on an exact allowlisted host (`claude.ai`,
    `console.anthropic.com`; no subdomain, userinfo or non-443 port), else `422`:
-   a card the user taps must never open a site the agent chose. An agent may have
-   at most 3 open (non-completed, non-expired) requests at once; the 4th returns
-   `429` with detail ``too many open sign-in requests``. Returns
-   `{id, kind, status, host, expires_in_seconds}` and raises an `auth_broker`
-   notification to the agent's owner. An agent the registry cannot attribute (a
-   bound local-token agent with no registry row) gets its card sent to each
-   admin instead; the card is never a broadcast (`user_id` null). With no admin
-   to receive it, create is `409`.
+a card the user taps must never open a site the agent chose. An agent may have
+  at most one pending or code-ready (non-expired) request per kind at once; a
+  second request for the same kind returns `409` with detail
+  ``a sign-in request is already open for this agent``. Returns
+  `{id, kind, status, host, expires_in_seconds}` and raises an `auth_broker`
+  notification to the agent's owner. An agent the registry cannot attribute (a
+  bound local-token agent with no registry row) gets its card sent to each
+  admin instead; the card is never a broadcast (`user_id` null). With no admin
+to receive it, create is `409`. In addition, an agent may hold at most 3 pending or code-ready (non-expired) requests across all statuses including code_read;
+   the next one returns `429` with detail ``too many open sign-in requests``.
 2. The desktop (bell, toast, Notifications app) renders that notification as a
    sign-in card: the agent's name, the host, an **Open** button (system browser;
    Firefox on taOSmobile) and a code field. The card re-checks the same host

tests/test_auth_broker.py

@@ -71,10 +71,15 @@ async def test_create_refuses_url_off_allowlist(client, app, url):
 
 @pytest.mark.asyncio
 async def test_create_accepts_allowlisted_hosts(client, app):
-    for url in (SIGNIN_URL, "https://console.anthropic.com/oauth/authorize?x=1"):
-        resp = await _create(client, app, url=url)
-        assert resp.status_code == 200, resp.text
-        assert resp.json()["id"]
+    # First request succeeds
+    resp1 = await _create(client, app, url=SIGNIN_URL)
+    assert resp1.status_code == 200, resp1.text
+    assert resp1.json()["id"]
+
+    # Second request with different allowlisted host but same kind, different agent is accepted
+    resp2 = await _create(client, app, "other-agent", url="https://console.anthropic.com/oauth/authorize?x=1")
+    assert resp2.status_code == 200, resp2.text
+    assert resp2.json()["id"]
 
 
 @pytest.mark.asyncio
@@ -376,31 +381,33 @@ async def test_secret_grant_preserves_existing_agents(client, app, monkeypatch,
 
 @pytest.mark.asyncio
 async def test_create_request_cap_per_agent(client, app):
-    """An agent can have at most MAX_OPEN_PER_AGENT open requests."""
+    """An agent can have at most one open request per kind."""
     from tinyagentos.auth_broker import MAX_OPEN_PER_AGENT
 
     hdr = _bearer(app, "haiku-agent")
     other_hdr = _bearer(app, "other-agent")
 
-    # Create MAX_OPEN_PER_AGENT requests for haiku-agent
-    for _ in range(MAX_OPEN_PER_AGENT):
-        resp = await _create(client, app, "haiku-agent")
-        assert resp.status_code == 200, resp.text
+    # First request succeeds
+    resp = await _create(client, app, "haiku-agent")
+    assert resp.status_code == 200, resp.text
 
-    # The (MAX_OPEN_PER_AGENT + 1)th request should be rejected with 429
+    # Second request for same agent and kind is rejected with 409
     resp = await _create(client, app, "haiku-agent")
-    assert resp.status_code == 429, resp.text
-    assert "too many open sign-in requests" in resp.text
+    assert resp.status_code == 409, resp.text
+    assert "a sign-in request is already open for this agent" in resp.text
 
-    # No 4th notification was added for haiku-agent
+    # Only one notification for haiku-agent
     rows = await app.state.notifications.list(limit=50)
     haiku_notifs = [r for r in rows if r.get("source") == "auth_broker" and r["data"]["agent"] == "haiku-agent"]
-    assert len(haiku_notifs) == MAX_OPEN_PER_AGENT
+    assert len(haiku_notifs) == 1
 
     # A different agent can still create
     resp = await _create(client, app, "other-agent")
     assert resp.status_code == 200, resp.text
 
+    # The store-level MAX_OPEN_PER_AGENT cap still applies as a fallback
+    # (tested separately in test_store_create_enforces_cap_under_lock)
+
 
 @pytest.mark.asyncio
 async def test_expired_code_ready_cleared_on_create(client, app, monkeypatch):
@@ -436,6 +443,35 @@ async def test_expired_code_ready_cleared_on_create(client, app, monkeypatch):
     assert new_rec["status"] == "pending"
 
 
+@pytest.mark.asyncio
+async def test_store_create_refuses_second_open_same_kind(tmp_data_dir):
+    """AuthBrokerStore.create refuses a second open request for the same requester and kind.
+
+    This test demonstrates the race condition that was fixed by making the one-open-request-per-kind
+    check atomic inside AuthBrokerStore.create().
+    """
+    from tinyagentos.auth_broker import KIND_CLAUDE_SUBSCRIPTION, AuthBrokerStore, OpenRequestExists
+
+    store = AuthBrokerStore(tmp_data_dir / "auth_broker_test.db")
+    await store.init()
+    try:
+        url = "https://claude.ai/oauth/authorize?code=true&client_id=abc&state=xyz"
+
+        # First create succeeds
+        rec1 = await store.create(
+            requester="agent-y", kind=KIND_CLAUDE_SUBSCRIPTION, url=url
+        )
+        assert rec1["status"] == "pending"
+
+        # Second create for same requester and kind should raise OpenRequestExists
+        with pytest.raises(OpenRequestExists):
+            await store.create(
+                requester="agent-y", kind=KIND_CLAUDE_SUBSCRIPTION, url=url
+            )
+    finally:
+        await store.close()
+
+
 @pytest.mark.asyncio
 async def test_store_create_enforces_cap_under_lock(tmp_data_dir):
     """AuthBrokerStore.create enforces MAX_OPEN_PER_AGENT inside its lock.
@@ -452,11 +488,14 @@ async def test_store_create_enforces_cap_under_lock(tmp_data_dir):
         url = "https://claude.ai/oauth/authorize?code=true&client_id=abc&state=xyz"
 
         # Create MAX_OPEN_PER_AGENT requests for "agent-x"
-        for _ in range(MAX_OPEN_PER_AGENT):
+        for i in range(MAX_OPEN_PER_AGENT):
             rec = await store.create(
                 requester="agent-x", kind=KIND_CLAUDE_SUBSCRIPTION, url=url
             )
             assert rec["status"] == "pending"
+            # Move this request to code_read so the next create can succeed
+            await store.set_code(rec["id"], "c")
+            await store.take_code(rec["id"])
 
         # The (MAX_OPEN_PER_AGENT + 1)th create for "agent-x" must raise
         with pytest.raises(PendingCapExceeded) as exc_info:
@@ -490,6 +529,9 @@ async def test_cap_reject_leaves_no_open_transaction(app):
             requester="cap-test-agent", kind=KIND_CLAUDE_SUBSCRIPTION, url=url
         )
         assert rec["status"] == "pending"
+        # Move this request to code_read so the next create can succeed
+        await store.set_code(rec["id"], "c")
+        await store.take_code(rec["id"])
 
     # The next create must raise PendingCapExceeded
     with pytest.raises(PendingCapExceeded):
@@ -515,6 +557,9 @@ async def test_rollback_exception_does_not_mask_pending_capexceeded(app):
             requester="rollback-test-agent", kind=KIND_CLAUDE_SUBSCRIPTION, url=url
         )
         assert rec["status"] == "pending"
+        # Move this request to code_read so the next create can succeed
+        await store.set_code(rec["id"], "c")
+        await store.take_code(rec["id"])
 
     # Monkeypatch rollback to raise an exception
     async def failing_rollback():
@@ -530,3 +575,74 @@ async def failing_rollback():
     assert exc_info.value.key == "rollback-test-agent"
     assert exc_info.value.cap == MAX_OPEN_PER_AGENT
     assert exc_info.value.pending == MAX_OPEN_PER_AGENT
+
+
+
+@pytest.mark.asyncio
+async def test_create_second_request_same_agent_same_kind_returns_409(client, app):
+    """A second create by the same agent for the same kind while the first is pending
+    returns 409 and adds no second auth_broker notification."""
+    hdr = _bearer(app, "haiku-agent")
+
+    # First create succeeds
+    resp1 = await _create(client, app, "haiku-agent")
+    assert resp1.status_code == 200, resp1.text
+    rid1 = resp1.json()["id"]
+
+    # Second create for same agent and kind should be 409
+    resp2 = await _create(client, app, "haiku-agent")
+    assert resp2.status_code == 409, resp2.text
+    assert "a sign-in request is already open for this agent" in resp2.text
+
+    # Only one notification should exist
+    rows = await app.state.notifications.list(limit=50)
+    hits = [r for r in rows if r.get("source") == "auth_broker" and r["data"]["agent"] == "haiku-agent"]
+    assert len(hits) == 1, f"Expected 1 notification, got {len(hits)}"
+
+
+@pytest.mark.asyncio
+async def test_create_after_expiry_succeeds(client, app, monkeypatch):
+    """After the first request expires, a new create for the same agent and kind succeeds."""
+    import tinyagentos.auth_broker as broker
+
+    t0 = 1_800_000_000.0
+    monkeypatch.setattr(broker, "_now", lambda: t0)
+    hdr = _bearer(app, "haiku-agent")
+
+    # Create first request
+    resp1 = await _create(client, app, "haiku-agent")
+    assert resp1.status_code == 200, resp1.text
+    rid1 = resp1.json()["id"]
+
+    # Advance time past expiry
+    monkeypatch.setattr(broker, "_now", lambda: t0 + broker.EXPIRY_SECONDS + 1)
+
+    # New create should succeed
+    resp2 = await _create(client, app, "haiku-agent")
+    assert resp2.status_code == 200, resp2.text
+    rid2 = resp2.json()["id"]
+    assert rid2 != rid1
+
+    # Two notifications should exist (one for each request)
+    rows = await app.state.notifications.list(limit=50)
+    hits = [r for r in rows if r.get("source") == "auth_broker" and r["data"]["agent"] == "haiku-agent"]
+    assert len(hits) == 2, f"Expected 2 notifications, got {len(hits)}"
+
+
+@pytest.mark.asyncio
+async def test_different_agent_not_blocked(client, app):
+    """A different agent can create a request even when another agent has one open."""
+    # First agent creates a request
+    resp1 = await _create(client, app, "haiku-agent")
+    assert resp1.status_code == 200, resp1.text
+
+    # Different agent creates a request - should succeed
+    resp2 = await _create(client, app, "other-agent")
+    assert resp2.status_code == 200, resp2.text
+
+    # Both should have notifications
+    rows = await app.state.notifications.list(limit=50)
+    haiku_hits = [r for r in rows if r.get("source") == "auth_broker" and r["data"]["agent"] == "haiku-agent"]
+    other_hits = [r for r in rows if r.get("source") == "auth_broker" and r["data"]["agent"] == "other-agent"]
+    assert len(haiku_hits) == 1
+    assert len(other_hits) == 1

tinyagentos/auth_broker.py

@@ -25,6 +25,8 @@
 import aiosqlite
 
 from tinyagentos.base_store import BaseStore, PendingCapExceeded
+class OpenRequestExists(Exception):
+    """Raised when a create operation would exceed the one-open-request-per-kind limit."""
 
 logger = logging.getLogger(__name__)
 
@@ -126,6 +128,28 @@ async def count_open(self, requester: str) -> int:
                 count += 1
         return count
 
+    async def open_request(self, requester: str, kind: str) -> dict | None:
+        """Return the newest open request for *requester* and *kind*.
+
+        An open request has status 'pending' or 'code_ready' and is not expired.
+        Returns None if no such request exists.
+        """
+        self._require_db()
+        now = _now()
+        async with self._db.execute(
+            "SELECT * FROM auth_broker_requests"
+            " WHERE requester = ? AND kind = ? AND status IN ('pending', 'code_ready')"
+            " ORDER BY created_at DESC LIMIT 1",
+            (requester, kind),
+        ) as cur:
+            row = await cur.fetchone()
+        if row is None:
+            return None
+        rec = _row(row)
+        if self.expired(rec, now):
+            return None
+        return rec
+
     async def create(self, *, requester: str, kind: str, url: str, prompt: str = "") -> dict:
         self._require_db()
         if kind not in KINDS:
@@ -149,6 +173,11 @@ async def create(self, *, requester: str, kind: str, url: str, prompt: str = "")
                     " WHERE status = 'code_ready' AND created_at < ?",
                     (now - EXPIRY_SECONDS,),
                 )
+                # Check for an existing open request for this requester and kind
+                # This must be inside the lock to prevent the race condition where
+                # two concurrent creates both pass this check and both insert
+                if await self.open_request(requester, kind) is not None:
+                    raise OpenRequestExists(requester)
                 open_count = 0
                 async with self._db.execute(
                     "SELECT * FROM auth_broker_requests WHERE requester = ? AND status != 'completed'",

tinyagentos/routes/auth_broker.py

@@ -22,7 +22,9 @@
 from pydantic import BaseModel, Field
 
 from tinyagentos.agent_token_auth import check_agent_identity
-from tinyagentos.auth_broker import EXPIRY_SECONDS, AuthBrokerStore, signin_host
+from tinyagentos.auth_broker import (
+    EXPIRY_SECONDS, AuthBrokerStore, OpenRequestExists, signin_host
+)
 from tinyagentos.auth_context import (
     current_user,
     require_agent_owner_or_admin,
@@ -105,6 +107,8 @@ async def create_request(request: Request, body: CreateBody):
             status_code=422,
             detail="url must be https on an allowed sign-in host: claude.ai, console.anthropic.com",
         )
+    # Check for an existing open request for this agent and kind.
+    store = _store(request)
     # The card goes to the agent's owner. An agent the registry cannot attribute
     # (a bound local-token agent with no registry row) goes to the admins, who
     # are the only users allowed to answer it. Never user_id=None: that is a
@@ -117,7 +121,7 @@ async def create_request(request: Request, body: CreateBody):
         if not recipients:
             raise HTTPException(status_code=409, detail="no user to receive the sign-in card")
     try:
-        rec = await _store(request).create(
+        rec = await store.create(
             requester=requester, kind=body.kind, url=body.url, prompt=body.prompt.strip()
         )
     except ValueError as exc:
@@ -127,6 +131,11 @@ async def create_request(request: Request, body: CreateBody):
             status_code=429,
             detail="too many open sign-in requests",
         )
+    except OpenRequestExists:
+        raise HTTPException(
+            status_code=409,
+            detail="a sign-in request is already open for this agent",
+        )
 
     notifs = getattr(request.app.state, "notifications", None)
     if notifs is not None: