Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions taosmd/migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ def _validate_registry(db: str, migs: Sequence[Migration]) -> None:
"claims": "claims.db",
"collections": "collections.db",
"knowledge_graph": "knowledge-graph.db",
"receipts": "receipts.db",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Receipts db not registered 🐞 Bug ⚙ Maintainability

migrations.DB_FILES now includes a receipts entry, but migrations.REGISTRY does not, so
status_all()/migrate_all() won’t include receipts and migrate(conn, 'receipts') would raise
“unknown database”. This leaves the new DB inconsistent with the migrations framework used by other
stores and blocks future schema evolution via migrations.
Agent Prompt
### Issue description
`DB_FILES` lists `receipts`, but there is no matching `REGISTRY` entry, and migration resolution rejects unknown DB names.

### Issue Context
Other persistent stores call `migrations.migrate(conn, '<db_name>')` and that requires the db to exist in `REGISTRY`.

### Fix Focus Areas
- taosmd/migrations.py[358-419]
- taosmd/migrations.py[395-404]

### Suggested fix approach
- Either:
  1) Add a baseline `receipts` migration tuple and include it in `REGISTRY` (and update the receipts store to call `migrations.migrate(conn, 'receipts')`), or
  2) Remove the `receipts` entry from `DB_FILES` until the DB is actually managed by migrations.
- Keep `DB_FILES` and `REGISTRY` aligned so DB-wide tooling behaves predictably.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

"session_catalog": "session-catalog.db",
"vector_memory": "vector-memory.db",
}
Expand Down
284 changes: 284 additions & 0 deletions taosmd/receipts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,284 @@
"""Receipt store module for A2A bus message delivery receipts.

Stores delivered and seen marks for A2A messages per (message_id, agent_id).
Key: (message_id, agent_id) -> delivered_at (nullable), seen_at (nullable)
"""

from __future__ import annotations

import sqlite3
import time
from pathlib import Path
from typing import Any, Dict, List, Optional

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: Unused imports Any and Dict

Any and Dict are imported but never referenced in this file.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

from dataclasses import dataclass


@dataclass
class Receipt:
"""A2A message receipt record."""
message_id: int
agent_id: str
delivered_at: Optional[float] = None
seen_at: Optional[float] = None

@classmethod
def from_db_row(cls, row: tuple) -> "Receipt":
"""Create a Receipt from a database row."""
return cls(
message_id=row[0],
agent_id=row[1],
delivered_at=row[2],
seen_at=row[3],
)

def to_db_values(self) -> tuple:
"""Convert to database values tuple."""
return (self.message_id, self.agent_id, self.delivered_at, self.seen_at)

def is_expired(self, now: Optional[float] = None) -> bool:
"""Check if this receipt has expired (TTL-based pruning)."""
if now is None:
now = time.time()
if self.delivered_at is None:
return False
# Default TTL of 30 days
ttl = 30 * 24 * 60 * 60

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: is_expired hardcodes a 30-day TTL

The TTL is hardcoded to 30 days here, but prune_expired accepts a configurable ttl_days parameter. If callers pass a non-default TTL to prune_expired, is_expired will still use 30 days, leading to inconsistent expiration behavior.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

return now - self.delivered_at > ttl

def has_been_delivered(self) -> bool:
"""Check if the message has been delivered to this agent."""
return self.delivered_at is not None

def has_been_seen(self) -> bool:
"""Check if the message has been seen by this agent."""
return self.seen_at is not None

def is_delivered_to_agent(self, agent_id: str) -> bool:
"""Check if the message has been delivered to the specified agent."""
return self.agent_id == agent_id and self.delivered_at is not None

def is_seen_by_agent(self, agent_id: str) -> bool:
"""Check if the message has been seen by the specified agent."""
return self.agent_id == agent_id and self.seen_at is not None


_RECEIPTS_SCHEMA = """
CREATE TABLE IF NOT EXISTS receipts (
message_id INTEGER NOT NULL,
agent_id TEXT NOT NULL,
delivered_at REAL,
seen_at REAL,
PRIMARY KEY (message_id, agent_id)
)
"""
Comment on lines +65 to +73

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Unused _RECEIPTS_SCHEMA constant duplicated inline in _init_schema.

The module-level _RECEIPTS_SCHEMA string is never referenced; _init_schema re-writes the identical CREATE TABLE SQL inline instead. Any future schema tweak risks updating only one copy.

♻️ Use the constant instead of duplicating the SQL
     def _init_schema(self) -> None:
         """Initialize the database schema."""
         conn = self._get_conn()
-        conn.execute(
-            """
-            CREATE TABLE IF NOT EXISTS receipts (
-                message_id INTEGER NOT NULL,
-                agent_id TEXT NOT NULL,
-                delivered_at REAL,
-                seen_at REAL,
-                PRIMARY KEY (message_id, agent_id)
-            )
-            """
-        )
+        conn.execute(_RECEIPTS_SCHEMA)

Also applies to: 96-109

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@taosmd/receipts.py` around lines 65 - 73, Update _init_schema to execute the
existing _RECEIPTS_SCHEMA constant instead of duplicating the CREATE TABLE SQL
inline, ensuring the single schema definition remains authoritative and removing
the redundant inline statement.



class ReceiptStore:
"""Store for A2A message delivery receipts."""

def __init__(
self,
data_dir: str | Path = "data",
):
self._data_dir = Path(data_dir)
self._conn: sqlite3.Connection | None = None
self._init_schema()

def _get_conn(self) -> sqlite3.Connection:
"""Get or create the database connection."""
if self._conn is None:
path = self._data_dir / "receipts.db"
path.parent.mkdir(parents=True, exist_ok=True)
self._conn = sqlite3.connect(str(path))
self._conn.row_factory = sqlite3.Row
Comment on lines +89 to +93

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Sqlite connect bypasses wal 🐞 Bug ☼ Reliability

ReceiptStore opens SQLite via sqlite3.connect directly, bypassing the project’s
taosmd._db.connect() helper that enables WAL and sets busy_timeout. Under concurrent access,
this increases the likelihood of database is locked errors and loses the repo-standard
observability warning when WAL can’t be enabled.
Agent Prompt
### Issue description
`ReceiptStore._get_conn()` uses `sqlite3.connect()` directly. The repo has a shared connection helper (`taosmd._db.connect`) that sets WAL mode and a busy timeout to reduce SQLITE_BUSY errors.

### Issue Context
Other SQLite-backed stores in this repo consistently use `_db.connect()` before running schema/migrations.

### Fix Focus Areas
- taosmd/receipts.py[87-94]

### Suggested fix approach
- Replace `sqlite3.connect(str(path))` with `taosmd._db.connect(str(path))` (or `_db.connect(path)`), then keep setting `row_factory` as you do now.
- Ensure imports align with other stores (e.g., `from taosmd import _db`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

return self._conn
Comment on lines +79 to +94

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: check how sibling stores (archive/knowledge_graph/etc.) default data_dir.
rg -n --type=py -C2 'data_dir\s*:\s*.*=\s*' taosmd/archive.py taosmd/knowledge_graph.py taosmd/session_catalog.py taosmd/vector_memory.py taosmd/collections.py taosmd/claims/store.py

Repository: jaylfc/taosmd

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Tracked receipt and DB files:"
git ls-files | rg '(^|/)receipts\.py$|(^|/)_db\.py$|(^|/)(archive|knowledge_graph|session_catalog|vector_memory|collections).*\.py$' || true

echo
echo "Receipts outline:"
ast-grep outline taosmd/receipts.py --view expanded || true

echo
echo "Relevant receipts lines:"
cat -n taosmd/receipts.py | sed -n '1,140p'

echo
echo "_db outline and connect helper content:"
cat -n taosmd/_db.py | sed -n '1,220p'

echo
echo "Search data_dir defaults across tracked py files:"
rg -n --type=py -C1 'data_dir\s*:\s*.*=\s*|Home|home\(\)|__init__' taosmd || true

echo
echo "Search sqlite3 direct connects and connect helper usages:"
rg -n --type=py -C1 'sqlite3\.connect|from taosmd\._db import|connect\(' taosmd || true

Repository: jaylfc/taosmd

Length of output: 25458


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Locate DEFAULT_DATA_DIR and taOSmd data dir resolver:"
rg -n --type=py -C2 'DEFAULT_DATA_DIR|TAOSMD_DATA_DIR|Path\.home\(\) \* "\.taosmd"|\.taosmd|data_dir\s*:\s*(Path|str|None)=\s*' taosmd || true

echo
echo "Auto setup data handling:"
cat -n taosmd/auto_setup.py | sed -n '1,120p'

echo
echo "Agent data_dir resolver and store construction:"
cat -n taosmd/agents.py | sed -n '140,210p'

echo
echo "Existing stores using shared _db.connect:"
for f in taosmd/archive.py taosmd/collections.py taosmd/taosmd_backend.py; do
  echo "--- $f"
  cat -n "$f" | sed -n '1,120p'
done

Repository: jaylfc/taosmd

Length of output: 40384


Align ReceiptStore with the canonical ~/.taosmd/ data path and reuse the shared SQLite helper.

data_dir: str | Path = "data" creates the receipt database under CWD’s data/, diverging from taosmd._resolve_data_dir() / ~/.taosmd/. Also, sqlite3.connect leaves this store out of WAL mode and busy-timeout protection that other DB-backed modules use for concurrent access.

🔧 Proposed fix
+from taosmd._db import connect as _db_connect
+
 class ReceiptStore:
     """Store for A2A message delivery receipts."""

     def __init__(
         self,
-        data_dir: str | Path = "data",
+        data_dir: str | Path = Path.home() / ".taosmd",
     ):
         self._data_dir = Path(data_dir)
         self._conn: sqlite3.Connection | None = None
         self._init_schema()

     def _get_conn(self) -> sqlite3.Connection:
         """Get or create the database connection."""
         if self._conn is None:
             path = self._data_dir / "receipts.db"
             path.parent.mkdir(parents=True, exist_ok=True)
-            self._conn = sqlite3.connect(str(path))
+            self._conn = _db_connect(path)
             self._conn.row_factory = sqlite3.Row
             self._conn.row_factory = sqlite3.Row
         return self._conn
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def __init__(
self,
data_dir: str | Path = "data",
):
self._data_dir = Path(data_dir)
self._conn: sqlite3.Connection | None = None
self._init_schema()
def _get_conn(self) -> sqlite3.Connection:
"""Get or create the database connection."""
if self._conn is None:
path = self._data_dir / "receipts.db"
path.parent.mkdir(parents=True, exist_ok=True)
self._conn = sqlite3.connect(str(path))
self._conn.row_factory = sqlite3.Row
return self._conn
from taosmd._db import connect as _db_connect
class ReceiptStore:
"""Store for A2A message delivery receipts."""
def __init__(
self,
data_dir: str | Path = Path.home() / ".taosmd",
):
self._data_dir = Path(data_dir)
self._conn: sqlite3.Connection | None = None
self._init_schema()
def _get_conn(self) -> sqlite3.Connection:
"""Get or create the database connection."""
if self._conn is None:
path = self._data_dir / "receipts.db"
path.parent.mkdir(parents=True, exist_ok=True)
self._conn = _db_connect(path)
self._conn.row_factory = sqlite3.Row
self._conn.row_factory = sqlite3.Row
return self._conn
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@taosmd/receipts.py` around lines 79 - 94, Update ReceiptStore.__init__ to
default data_dir through the canonical taosmd._resolve_data_dir() path,
preserving explicit overrides. Replace the direct sqlite3.connect call in
_get_conn with the shared SQLite connection helper used by other database-backed
modules so WAL mode and busy-timeout settings are applied.

Source: Coding guidelines


def _init_schema(self) -> None:
"""Initialize the database schema."""
conn = self._get_conn()
conn.execute(
"""
CREATE TABLE IF NOT EXISTS receipts (
message_id INTEGER NOT NULL,
agent_id TEXT NOT NULL,
delivered_at REAL,
seen_at REAL,
PRIMARY KEY (message_id, agent_id)
)
"""
)

def close(self) -> None:
"""Close the database connection."""
if self._conn is not None:
self._conn.close()
self._conn = None

def record_delivered(
self,
message_id: int,
agent_id: str,
delivered_at: Optional[float] = None,
upsert: bool = True,
) -> bool:
"""Record that a message was delivered to an agent.

Args:
message_id: The ID of the message.
agent_id: The ID of the agent that received the message.
delivered_at: When the message was delivered (defaults to now).
upsert: If True, update existing record. If False, skip if exists.

Returns:
True if a new row was inserted or updated, False if skip-if-exists.
"""
if delivered_at is None:
delivered_at = time.time()

conn = self._get_conn()
cursor = conn.execute(
"SELECT delivered_at FROM receipts WHERE message_id = ? AND agent_id = ?",
(message_id, agent_id),
)
exists = cursor.fetchone() is not None

if exists and not upsert:
return False

conn.execute(
"""
INSERT INTO receipts (message_id, agent_id, delivered_at, seen_at)
VALUES (?, ?, ?, NULL)
ON CONFLICT(message_id, agent_id) DO UPDATE SET
delivered_at = excluded.delivered_at,
seen_at = receipts.seen_at
""",
(message_id, agent_id, delivered_at),
)
conn.commit()
return True
Comment on lines +117 to +159

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

TOCTOU race between the existence check and the upsert in record_delivered.

When upsert=False, the method does a separate SELECT to decide whether to skip, then a separate INSERT ... ON CONFLICT DO UPDATE. If this store/DB file is accessed from more than one connection concurrently (multiple processes/threads), two callers can both observe "not exists" and both proceed, so the second write silently overwrites delivered_at even though upsert=False was requested. SQLite's ON CONFLICT ... DO NOTHING makes the skip-if-exists path atomic and removes the race entirely.

🔒 Atomic skip-if-exists via DO NOTHING
-        conn = self._get_conn()
-        cursor = conn.execute(
-            "SELECT delivered_at FROM receipts WHERE message_id = ? AND agent_id = ?",
-            (message_id, agent_id),
-        )
-        exists = cursor.fetchone() is not None
-
-        if exists and not upsert:
-            return False
-
-        conn.execute(
-            """
-            INSERT INTO receipts (message_id, agent_id, delivered_at, seen_at)
-            VALUES (?, ?, ?, NULL)
-            ON CONFLICT(message_id, agent_id) DO UPDATE SET
-                delivered_at = excluded.delivered_at,
-                seen_at = receipts.seen_at
-            """,
-            (message_id, agent_id, delivered_at),
-        )
-        conn.commit()
-        return True
+        conn = self._get_conn()
+        conflict_action = (
+            "DO UPDATE SET delivered_at = excluded.delivered_at, "
+            "seen_at = receipts.seen_at"
+            if upsert
+            else "DO NOTHING"
+        )
+        cursor = conn.execute(
+            f"""
+            INSERT INTO receipts (message_id, agent_id, delivered_at, seen_at)
+            VALUES (?, ?, ?, NULL)
+            ON CONFLICT(message_id, agent_id) {conflict_action}
+            """,
+            (message_id, agent_id, delivered_at),
+        )
+        conn.commit()
+        return upsert or cursor.rowcount > 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def record_delivered(
self,
message_id: int,
agent_id: str,
delivered_at: Optional[float] = None,
upsert: bool = True,
) -> bool:
"""Record that a message was delivered to an agent.
Args:
message_id: The ID of the message.
agent_id: The ID of the agent that received the message.
delivered_at: When the message was delivered (defaults to now).
upsert: If True, update existing record. If False, skip if exists.
Returns:
True if a new row was inserted or updated, False if skip-if-exists.
"""
if delivered_at is None:
delivered_at = time.time()
conn = self._get_conn()
cursor = conn.execute(
"SELECT delivered_at FROM receipts WHERE message_id = ? AND agent_id = ?",
(message_id, agent_id),
)
exists = cursor.fetchone() is not None
if exists and not upsert:
return False
conn.execute(
"""
INSERT INTO receipts (message_id, agent_id, delivered_at, seen_at)
VALUES (?, ?, ?, NULL)
ON CONFLICT(message_id, agent_id) DO UPDATE SET
delivered_at = excluded.delivered_at,
seen_at = receipts.seen_at
""",
(message_id, agent_id, delivered_at),
)
conn.commit()
return True
def record_delivered(
self,
message_id: int,
agent_id: str,
delivered_at: Optional[float] = None,
upsert: bool = True,
) -> bool:
"""Record that a message was delivered to an agent.
Args:
message_id: The ID of the message.
agent_id: The ID of the agent that received the message.
delivered_at: When the message was delivered (defaults to now).
upsert: If True, update existing record. If False, skip if exists.
Returns:
True if a new row was inserted or updated, False if skip-if-exists.
"""
if delivered_at is None:
delivered_at = time.time()
conn = self._get_conn()
conflict_action = (
"DO UPDATE SET delivered_at = excluded.delivered_at, "
"seen_at = receipts.seen_at"
if upsert
else "DO NOTHING"
)
cursor = conn.execute(
f"""
INSERT INTO receipts (message_id, agent_id, delivered_at, seen_at)
VALUES (?, ?, ?, NULL)
ON CONFLICT(message_id, agent_id) {conflict_action}
""",
(message_id, agent_id, delivered_at),
)
conn.commit()
return upsert or cursor.rowcount > 0
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@taosmd/receipts.py` around lines 117 - 159, Update record_delivered to handle
upsert=False with a single atomic INSERT ... ON CONFLICT DO NOTHING statement,
removing the separate existence SELECT for that path. Return True only when a
row is inserted and False when the conflict skips the write; preserve the
existing upsert=True update behavior and transaction commit handling.


def record_seen(
self,
message_id: int,
agent_id: str,
seen_at: Optional[float] = None,
) -> bool:
"""Record that an agent has seen a message.

Args:
message_id: The ID of the message.
agent_id: The ID of the agent that saw the message.
seen_at: When the message was seen (defaults to now).

Returns:
True if the seen_at was updated, False if the record doesn't exist.
"""
if seen_at is None:
seen_at = time.time()

conn = self._get_conn()
cursor = conn.execute(
"SELECT seen_at FROM receipts WHERE message_id = ? AND agent_id = ?",
(message_id, agent_id),
)
row = cursor.fetchone()
if row is None:
return False

existing_seen = row["seen_at"]
# Only update if moving from null to a value
if existing_seen is not None and existing_seen >= seen_at:
return False

conn.execute(
"UPDATE receipts SET seen_at = ? WHERE message_id = ? AND agent_id = ?",
(seen_at, message_id, agent_id),
)
conn.commit()
return True
Comment on lines +161 to +199

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

TOCTOU race in record_seen's monotonic check.

The read (SELECT seen_at) and the conditional UPDATE are two separate statements. Under concurrent writers, both can pass the monotonic check with a stale read and then apply their UPDATE in the wrong order, letting an older seen_at clobber a newer one. Fold the monotonic condition into the UPDATE's WHERE clause so the check-and-set is atomic.

🔒 Atomic monotonic update
-        conn = self._get_conn()
-        cursor = conn.execute(
-            "SELECT seen_at FROM receipts WHERE message_id = ? AND agent_id = ?",
-            (message_id, agent_id),
-        )
-        row = cursor.fetchone()
-        if row is None:
-            return False
-
-        existing_seen = row["seen_at"]
-        # Only update if moving from null to a value
-        if existing_seen is not None and existing_seen >= seen_at:
-            return False
-
-        conn.execute(
-            "UPDATE receipts SET seen_at = ? WHERE message_id = ? AND agent_id = ?",
-            (seen_at, message_id, agent_id),
-        )
-        conn.commit()
-        return True
+        conn = self._get_conn()
+        cursor = conn.execute(
+            """
+            UPDATE receipts SET seen_at = ?
+            WHERE message_id = ? AND agent_id = ?
+              AND (seen_at IS NULL OR seen_at < ?)
+            """,
+            (seen_at, message_id, agent_id, seen_at),
+        )
+        conn.commit()
+        return cursor.rowcount > 0

Note: this also changes the "record doesn't exist" and "already newer" cases to both return False uniformly, matching the original ambiguity but without the race.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def record_seen(
self,
message_id: int,
agent_id: str,
seen_at: Optional[float] = None,
) -> bool:
"""Record that an agent has seen a message.
Args:
message_id: The ID of the message.
agent_id: The ID of the agent that saw the message.
seen_at: When the message was seen (defaults to now).
Returns:
True if the seen_at was updated, False if the record doesn't exist.
"""
if seen_at is None:
seen_at = time.time()
conn = self._get_conn()
cursor = conn.execute(
"SELECT seen_at FROM receipts WHERE message_id = ? AND agent_id = ?",
(message_id, agent_id),
)
row = cursor.fetchone()
if row is None:
return False
existing_seen = row["seen_at"]
# Only update if moving from null to a value
if existing_seen is not None and existing_seen >= seen_at:
return False
conn.execute(
"UPDATE receipts SET seen_at = ? WHERE message_id = ? AND agent_id = ?",
(seen_at, message_id, agent_id),
)
conn.commit()
return True
def record_seen(
self,
message_id: int,
agent_id: str,
seen_at: Optional[float] = None,
) -> bool:
"""Record that an agent has seen a message.
Args:
message_id: The ID of the message.
agent_id: The ID of the agent that saw the message.
seen_at: When the message was seen (defaults to now).
Returns:
True if the seen_at was updated, False if the record doesn't exist.
"""
if seen_at is None:
seen_at = time.time()
conn = self._get_conn()
cursor = conn.execute(
"""
UPDATE receipts SET seen_at = ?
WHERE message_id = ? AND agent_id = ?
AND (seen_at IS NULL OR seen_at < ?)
""",
(seen_at, message_id, agent_id, seen_at),
)
conn.commit()
return cursor.rowcount > 0
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@taosmd/receipts.py` around lines 161 - 199, Update record_seen to perform the
monotonic check atomically by moving the seen_at comparison into the UPDATE
statement’s WHERE clause, allowing updates only when the stored value is null or
older than the new timestamp. Use the UPDATE row count to return True only when
a row was changed and False uniformly when the receipt is missing or already
newer; remove the separate SELECT/check flow while preserving timestamp
defaulting and commit behavior.


def get_receipt(
self,
message_id: int,
agent_id: str,
) -> Optional[Receipt]:
"""Get a receipt for a specific message and agent."""
conn = self._get_conn()
cursor = conn.execute(
"SELECT message_id, agent_id, delivered_at, seen_at "
"FROM receipts WHERE message_id = ? AND agent_id = ?",
(message_id, agent_id),
)
row = cursor.fetchone()
if row is None:
return None
return Receipt.from_db_row(row)

def get_delivered_by_message(self, message_id: int) -> List[Receipt]:
"""Get all receipts for a message, including delivered info."""
conn = self._get_conn()
cursor = conn.execute(
"SELECT message_id, agent_id, delivered_at, seen_at "
"FROM receipts WHERE message_id = ?",
(message_id,),
)
rows = cursor.fetchall()
return [Receipt.from_db_row(row) for row in rows]

def get_read_by_message(self, message_id: int) -> List[Receipt]:
"""Get all receipts where the message has been seen, including seen info."""
conn = self._get_conn()
cursor = conn.execute(
"SELECT message_id, agent_id, delivered_at, seen_at "
"FROM receipts WHERE message_id = ? AND seen_at IS NOT NULL",
(message_id,),
)
rows = cursor.fetchall()
return [Receipt.from_db_row(row) for row in rows]

def prune_expired(self, ttl_days: int = 30) -> int:
"""Remove expired receipts.

Args:
ttl_days: Time-to-live in days for receipts.

Returns:
Number of rows pruned.
"""
conn = self._get_conn()
cursor = conn.execute(
"SELECT message_id, agent_id, delivered_at FROM receipts WHERE delivered_at IS NOT NULL"
)
rows = cursor.fetchall()

now = time.time()
ttl_seconds = ttl_days * 24 * 60 * 60

expired_message_ids = []
for row in rows:
if now - row["delivered_at"] > ttl_seconds:
expired_message_ids.append(row["message_id"])

if not expired_message_ids:
return 0

placeholders = ",".join("?" for _ in expired_message_ids)
cursor = conn.execute(
f"DELETE FROM receipts WHERE message_id IN ({placeholders})",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: prune_expired over-deletes by collapsing to message_id

The method collects expired message_id values and deletes WHERE message_id IN (...). Because the primary key is (message_id, agent_id), this deletes ALL agent receipts for an expired message, not just the expired (message_id, agent_id) pairs. Non-expired agents' receipts for the same message will be incorrectly removed.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

expired_message_ids,
)
Comment on lines +258 to +270

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Overbroad receipt pruning 🐞 Bug ≡ Correctness

ReceiptStore.prune_expired() deletes rows by message_id only, so an expired receipt for one
(message_id, agent_id) can delete receipts for other agents that are not expired. This breaks the
stated per-(message_id, agent_id) semantics and can cause delivered/seen marks to disappear
incorrectly.
Agent Prompt
### Issue description
`prune_expired()` currently builds a list of expired `message_id`s and runs `DELETE FROM receipts WHERE message_id IN (...)`. Because the receipts table key is `(message_id, agent_id)`, this can delete *non-expired* receipts for other agents that share the same message_id.

### Issue Context
Receipts are stored per `(message_id, agent_id)` and should be pruned per-row (or at least per composite key), not per message.

### Fix Focus Areas
- taosmd/receipts.py[240-273]

### Suggested fix approach
- Compute expiry using a cutoff timestamp (`cutoff = now - ttl_seconds`) and delete with a single SQL statement that prunes per-row:
  - `DELETE FROM receipts WHERE delivered_at IS NOT NULL AND delivered_at < ?`
- If you need more complex logic later, track and delete by `(message_id, agent_id)` pairs (e.g., `WHERE (message_id, agent_id) IN ((?,?),...)`).
- Consider adding an index on `delivered_at` if prune is frequent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

deleted_count = cursor.rowcount
conn.commit()
return deleted_count

def get_all_by_agent(self, agent_id: str) -> List[Receipt]:
"""Get all receipts for a specific agent."""
conn = self._get_conn()
cursor = conn.execute(
"SELECT message_id, agent_id, delivered_at, seen_at "
"FROM receipts WHERE agent_id = ?",
(agent_id,),
)
rows = cursor.fetchall()
return [Receipt.from_db_row(row) for row in rows]
Loading