Copilot comments v1

application/api/user/agents/routes.py

@@ -1359,11 +1359,13 @@ class RegenerateAgentKey(Resource):
                         500,
                     )
 
-                # Re-point key-string-only history to the new key so it is not
-                # orphaned. conversations/token analytics also match on
-                # agent_id, but stack_logs (webhook + system-error logs) has no
-                # agent_id column, and the 24h rate-limit window is keyed by the
-                # api-key string only. Runs in the same transaction as the swap.
+                # Re-point key-string history to the new key. conversations,
+                # token_usage and stack_logs (since 0026) all carry agent_id, so
+                # per-agent analytics survive rotation via agent_id regardless.
+                # These rewrites still run to (a) preserve the 24h rate-limit
+                # window, which is keyed by api_key only (token_usage), and (b)
+                # keep the api_key column consistent / re-attach any legacy rows
+                # whose agent_id is NULL. Runs in the same transaction as the swap.
                 StackLogsRepository(conn).reassign_api_key(
                     old_key=old_key, new_key=new_key
                 )

application/logging.py

@@ -5,7 +5,7 @@ import time
 
 import logging
 import uuid
-from typing import Any, Callable, Dict, Generator, List
+from typing import Any, Callable, Dict, Generator, List, Optional
 
 from application.core import log_context
 from application.storage.db.repositories.stack_logs import StackLogsRepository
@@ -247,7 +247,7 @@ def _log_activity_to_db(
     query: str,
     stacks: List[Dict],
     level: str,
-    agent_id: str = None,
+    agent_id: Optional[str] = None,
 ) -> None:
     """Append a per-request activity log row to Postgres (``stack_logs``)."""
     try:

application/storage/db/repositories/stack_logs.py

@@ -10,6 +10,7 @@ Covers the single operation the legacy Mongo code performs:
 from __future__ import annotations
 
 import json
+import uuid
 from datetime import datetime
 from typing import Optional
 
@@ -57,14 +58,16 @@ class StackLogsRepository:
         stacks: Optional[list] = None,
         timestamp: Optional[datetime] = None,
     ) -> None:
-        # ``agent_id`` is a UUID column. Coerce anything not shaped like a UUID
-        # (36 chars with hyphens) to NULL so a stray/legacy id never breaks the
-        # activity-log write. Mirrors TokenUsageRepository.insert.
+        # ``agent_id`` is a UUID column. Parse it with ``uuid.UUID`` and coerce
+        # anything that isn't a valid UUID (e.g. a 24-hex legacy Mongo ObjectId)
+        # to NULL so a stray/legacy id never reaches ``CAST(... AS uuid)`` and
+        # breaks the activity-log write.
         agent_id_uuid: Optional[str] = None
         if agent_id:
-            s = str(agent_id)
-            if len(s) == 36 and "-" in s:
-                agent_id_uuid = s
+            try:
+                agent_id_uuid = str(uuid.UUID(str(agent_id)))
+            except (ValueError, AttributeError, TypeError):
+                agent_id_uuid = None
         self._conn.execute(
             text(
                 """
@@ -98,17 +101,18 @@ class StackLogsRepository:
     def reassign_api_key(self, *, old_key: str, new_key: str) -> int:
         """Re-point historical rows from ``old_key`` to ``new_key``.
 
-        ``stack_logs`` has no ``agent_id`` column, so webhook / system-error
-        logs are matched by the api-key string alone (see the analytics
-        ``webhook_where`` / ``system_where`` clauses). When an agent's key is
-        rotated this rewrite keeps that history attached to the agent instead
-        of orphaning it.
+        Since migration 0026 ``stack_logs`` has an ``agent_id`` column and the
+        analytics ``webhook_where`` / ``system_where`` clauses match
+        ``agent_id`` first with ``api_key`` as a fallback, so most rows already
+        survive a key rotation via ``agent_id``. This rewrite still runs to keep
+        the ``api_key`` column consistent and to re-attach any rows whose
+        ``agent_id`` is NULL (e.g. a log written without agent context).
 
         Matched by ``api_key`` only — deliberately NOT scoped by ``user_id``:
         ``agents.key`` is globally unique, so a key maps to exactly one agent,
         and rows are stamped with the *caller's* user_id (which is not the
         owner for webhook / external-api-key traffic). Scoping by owner would
-        skip precisely the rows this migration exists to preserve.
+        skip precisely the rows this rewrite exists to preserve.
         Returns the number of rows updated.
         """
         if not old_key or not new_key:

frontend/src/modals/AgentDetailsModal.tsx

@@ -82,15 +82,18 @@ export default function AgentDetailsModal({
 
   const handleRegenerateKey = async () => {
     setLoading('apiKey', true);
-    const response = await userService.regenerateAgentKey(agent.id ?? '', token);
-    if (!response.ok) {
+    try {
+      const response = await userService.regenerateAgentKey(
+        agent.id ?? '',
+        token,
+      );
+      if (!response.ok) return;
+      const data = await response.json();
+      setApiKey(data.key);
+      onKeyRegenerated?.(data.key);
+    } finally {
       setLoading('apiKey', false);
-      return;
     }
-    const data = await response.json();
-    setApiKey(data.key);
-    onKeyRegenerated?.(data.key);
-    setLoading('apiKey', false);
   };
 
   useEffect(() => {

tests/storage/db/repositories/test_stack_logs.py

@@ -199,11 +199,35 @@ class TestInsert:
         assert dict(row._mapping)["stacks"][0]["data"]["result"] == "ok"
 
 
+class TestAgentId:
+    def test_valid_agent_id_stored(self, pg_conn):
+        repo = _repo(pg_conn)
+        agent_id = "11111111-2222-3333-4444-555555555555"
+        repo.insert(activity_id="ai-valid", agent_id=agent_id)
+        row = pg_conn.execute(
+            text("SELECT agent_id FROM stack_logs WHERE activity_id = 'ai-valid'")
+        ).fetchone()
+        assert str(dict(row._mapping)["agent_id"]) == agent_id
+
+    def test_invalid_agent_id_coerced_to_null(self, pg_conn):
+        # A 24-hex legacy Mongo ObjectId is not a valid UUID; it must be
+        # coerced to NULL rather than reaching CAST(... AS uuid) and killing
+        # the whole activity-log write.
+        repo = _repo(pg_conn)
+        repo.insert(activity_id="ai-legacy", agent_id="507f1f77bcf86cd799439011")
+        row = pg_conn.execute(
+            text("SELECT agent_id FROM stack_logs WHERE activity_id = 'ai-legacy'")
+        ).fetchone()
+        assert dict(row._mapping)["agent_id"] is None
+
+
 class TestReassignApiKey:
     def test_rewrites_matching_rows_regardless_of_user(self, pg_conn):
-        # stack_logs has no agent_id; on key rotation, rows must follow the
-        # key. Rows are stamped with the caller's user_id (not the owner),
-        # so the migration must NOT be scoped by user_id.
+        # On key rotation the rewrite is intentionally NOT scoped by user_id:
+        # agents.key is unique so a key maps to one agent, and rows are stamped
+        # with the caller's user_id (not the owner), so scoping by owner would
+        # skip caller-logged rows. (Rows also carry agent_id since 0026, but
+        # this rewrite still covers api_key consistency and NULL-agent_id rows.)
         repo = _repo(pg_conn)
         repo.insert(activity_id="ra-1", api_key="old-k", user_id="owner")
         repo.insert(activity_id="ra-2", api_key="old-k", user_id="a-caller")