-
-
Notifications
You must be signed in to change notification settings - Fork 3
tsk-qh3bfs [OPEN] POST /a2a/import: admin batch import with historic #230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,8 @@ | |
| agent_name TEXT, | ||
| app_id TEXT, | ||
| project TEXT, | ||
| source TEXT, | ||
| source_id TEXT, | ||
| summary TEXT NOT NULL DEFAULT '', | ||
| file_path TEXT NOT NULL, | ||
| line_number INTEGER NOT NULL, | ||
|
|
@@ -67,6 +69,9 @@ | |
| CREATE INDEX IF NOT EXISTS idx_archive_type ON archive_index(event_type); | ||
| CREATE INDEX IF NOT EXISTS idx_archive_agent ON archive_index(agent_name); | ||
| CREATE INDEX IF NOT EXISTS idx_archive_app ON archive_index(app_id); | ||
| CREATE UNIQUE INDEX IF NOT EXISTS idx_archive_source_uid | ||
| ON archive_index (source, source_id) | ||
| WHERE source IS NOT NULL AND source_id IS NOT NULL; | ||
|
Comment on lines
+72
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4. Unique index can orphan jsonl ArchiveStore.record writes and flushes the JSONL event line before inserting into archive_index; with the new unique index on (source, source_id), an IntegrityError can occur after the file append, leaving an unindexed/orphaned archive line and breaking file_path/line_number provenance. This is most likely under concurrent imports or any caller that bypasses the importer’s pre-check. Agent Prompt
|
||
|
|
||
| CREATE TABLE IF NOT EXISTS archive_settings ( | ||
| key TEXT PRIMARY KEY, | ||
|
|
@@ -179,8 +184,18 @@ async def record( | |
| app_id: str | None = None, | ||
| summary: str = "", | ||
| project: str | None = None, | ||
| source: str | None = None, | ||
| source_id: str | None = None, | ||
| timestamp: float | None = None, | ||
| ) -> int: | ||
| """Record an event to the archive. Returns the index row ID.""" | ||
| """Record an event to the archive. Returns the index row ID. | ||
|
|
||
| ``timestamp`` overrides the wall-clock time when provided (used by the | ||
| A2A batch importer to preserve historical timestamps, #211). ``source`` | ||
| and ``source_id`` tag imported A2A messages for idempotent re-import; | ||
| when both are non-null the unique index ``idx_archive_source_uid`` | ||
| enforces (source, source_id) uniqueness. | ||
| """ | ||
| # Skip user activity events if tracking is disabled | ||
| if event_type in USER_ACTIVITY_EVENTS and not self._user_tracking_enabled: | ||
| return -1 | ||
|
|
@@ -192,13 +207,15 @@ async def record( | |
| if key in data and isinstance(data[key], str): | ||
| data[key], _ = redact_secrets(data[key]) | ||
|
|
||
| ts = time.time() | ||
| ts = timestamp if timestamp is not None else time.time() | ||
| event = { | ||
| "timestamp": ts, | ||
| "event_type": event_type, | ||
| "agent_name": agent_name, | ||
| "app_id": app_id, | ||
| "project": project, | ||
| "source": source, | ||
| "source_id": source_id, | ||
| "summary": summary, | ||
| "data": data, | ||
| } | ||
|
|
@@ -232,9 +249,11 @@ async def record( | |
| # Index for fast lookup | ||
| cursor = self._conn.execute( | ||
| """INSERT INTO archive_index | ||
| (timestamp, event_type, agent_name, app_id, project, summary, file_path, line_number, data_json) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)""", | ||
| (ts, event_type, agent_name, app_id, project, summary, file_path, line_count, json.dumps(data, default=str)), | ||
| (timestamp, event_type, agent_name, app_id, project, source, source_id, | ||
| summary, file_path, line_number, data_json) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", | ||
| (ts, event_type, agent_name, app_id, project, source, source_id, | ||
| summary, file_path, line_count, json.dumps(data, default=str)), | ||
| ) | ||
|
|
||
| # Index in FTS for full-text search | ||
|
|
@@ -342,6 +361,35 @@ async def get_event(self, event_id: int) -> dict | None: | |
| result["data"] = {} | ||
| return result | ||
|
|
||
| async def find_source_ids( | ||
| self, source: str, event_type: str = EVENT_A2A | ||
| ) -> dict[str, int]: | ||
| """Return ``{source_id: archive_id}`` for all events tagged with ``source``. | ||
|
|
||
| Backs idempotent A2A batch import (#211): lets the importer skip rows whose | ||
| ``(source, source_id)`` already exists and resolve ``reply_to_source_id`` | ||
| to the archive row id of the originally imported message. Only rows with a | ||
| non-null ``source_id`` are considered. | ||
| """ | ||
| rows = self._conn.execute( | ||
| "SELECT source_id, id FROM archive_index " | ||
| "WHERE source = ? AND source_id IS NOT NULL AND event_type = ?", | ||
| (source, event_type), | ||
| ).fetchall() | ||
| return {row["source_id"]: row["id"] for row in rows} | ||
|
|
||
| async def find_reply_target( | ||
| self, source: str, source_id: str, event_type: str = EVENT_A2A | ||
| ) -> int | None: | ||
| """Return the archive row id for a ``(source, source_id)`` pair, or None.""" | ||
| row = self._conn.execute( | ||
| "SELECT id FROM archive_index " | ||
| "WHERE source = ? AND source_id = ? AND event_type = ? " | ||
| "ORDER BY id ASC LIMIT 1", | ||
| (source, source_id, event_type), | ||
| ).fetchone() | ||
| return row["id"] if row else None | ||
|
|
||
| async def count( | ||
| self, | ||
| event_type: str | None = None, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -199,6 +199,24 @@ def _archive_index_project(conn: sqlite3.Connection) -> None: | |
| add_column(conn, "archive_index", "project", "TEXT") | ||
|
|
||
|
|
||
| def _archive_index_source_uid(conn: sqlite3.Connection) -> None: | ||
| """Add source/source_id columns for idempotent A2A batch import (#211). | ||
|
|
||
| ``source`` and ``source_id`` tag each imported A2A message with the | ||
| external origin and its stable per-source id, so a re-import can skip | ||
| rows already present without relying on the JSON payload alone. The | ||
| partial unique index enforces (source, source_id) uniqueness at the | ||
| database level as a safety net against duplicate writes. | ||
| """ | ||
| add_column(conn, "archive_index", "source", "TEXT") | ||
| add_column(conn, "archive_index", "source_id", "TEXT") | ||
| conn.execute( | ||
| "CREATE UNIQUE INDEX IF NOT EXISTS idx_archive_source_uid " | ||
| "ON archive_index (source, source_id) " | ||
| "WHERE source IS NOT NULL AND source_id IS NOT NULL" | ||
| ) | ||
|
|
||
|
|
||
| _ARCHIVE_INDEX: tuple[Migration, ...] = ( | ||
| Migration( | ||
| 1, "archive_index_baseline", _archive_index_baseline, | ||
|
|
@@ -208,6 +226,13 @@ def _archive_index_project(conn: sqlite3.Connection) -> None: | |
| 2, "archive_index_project", _archive_index_project, | ||
| lambda c: has_column(c, "archive_index", "project"), | ||
| ), | ||
| Migration( | ||
| 3, "archive_index_source_uid", _archive_index_source_uid, | ||
| lambda c: ( | ||
| has_column(c, "archive_index", "source") | ||
| and has_column(c, "archive_index", "source_id") | ||
| ), | ||
|
Comment on lines
+231
to
+234
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 5. Migration detect skips index The archive_index_source_uid migration’s detect function only checks for source/source_id columns; if those columns exist but idx_archive_source_uid is missing, the migration will be treated as applied and the uniqueness constraint will not be created. This can silently disable the idempotency safety net on some upgraded databases. Agent Prompt
|
||
| ), | ||
| ) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -228,6 +228,24 @@ async def a2a_send( | |
| payload["blocks"] = blocks | ||
| return await self._run("POST", "/a2a/send", payload) | ||
|
|
||
| async def a2a_import( | ||
| self, | ||
| source: str, | ||
| messages: list[dict], | ||
| *, | ||
| defer_index: bool = False, | ||
| **_opts, | ||
| ) -> dict: | ||
| """POST /a2a/import: admin batch-import historical messages onto the remote bus. | ||
|
|
||
| Returns ``{"imported", "skipped", "first_id", "last_id"}`` (taOSmd #211 Q3a). | ||
| Uses the client's bearer token, which must be an admin token. | ||
| """ | ||
| payload: dict = {"source": source, "messages": messages} | ||
| if defer_index: | ||
| payload["defer_index"] = True | ||
| return await self._run("POST", "/a2a/import", payload) | ||
|
|
||
|
Comment on lines
+231
to
+248
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find callers of a2a_import and compare the remote-forwarding pattern.
rg -nP --type=py '\ba2a_import\s*\(' -C4
rg -nP --type=py '_get_remote\s*\(' -C2 taosmd/service.pyRepository: jaylfc/taosmd Length of output: 151 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== locate relevant files =="
fd -a 'remote\.py|service\.py' . | sed -n '1,120p'
echo "== search a2a_import occurrences =="
rg -n 'a2a_import|\ba2a_send\b|_get_remote|RemoteClient|/a2a/import' . || true
echo "== list python files under repo =="
git ls-files '*.py' | sed -n '1,200p'Repository: jaylfc/taosmd Length of output: 29199 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== git status/stat =="
git status --short
git diff --stat || true
git log --oneline -5 || true
echo "== tracked python files containing import/export/API names =="
git ls-files '*.py' | xargs rg -n 'a2a_import|a2a_send|RemoteClient|_get_remote|/a2a/import|import .*a2a' -- || trueRepository: jaylfc/taosmd Length of output: 13587 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== service remote forwarding for a2a methods =="
sed -n '340,680p' taosmd/service.py | nl -ba -v340
echo "== http_server a2a_import handler =="
sed -n '1500,1540p' taosmd/http_server.py | nl -ba -v1500
echo "== cli around RemoteClient creation =="
sed -n '500,545p' taosmd/cli.py | nl -ba -v500
echo "== module exported/importable a2a_import references =="
python3 - <<'PY'
from pathlib import Path
import re
root = Path('.')
for p in root.glob('**/*.py'):
text = p.read_text(errors='ignore')
if r'\ba2a_import\b' in text:
# Print non-test, non-import-site occurrences plus import statements that mention it
for i, line in enumerate(text.splitlines(), 1):
if line.strip().startswith('from ') and '.service import' in line or re.search(r'\ba2a_import\b', line):
continue
matches = []
for p in sorted(root.glob('**/*.py')):
text = p.read_text(errors='ignore')
occurrences = [(i+1, line.strip()) for i,line in enumerate(text.splitlines(),1) if re.search(r'\ba2a_import\b', line)]
if not occurrences:
continue
print(f'FILE {p}')
for i,line in occurrences:
print(f'{i}: {line}')
PY
echo "== __all__ in service/remote =="
sed -n '1200,1220p' taosmd/service.py | nl -ba -v1200
sed -n '410,425p' taosmd/remote.py | nl -ba -v410
sed -n '1,120p' taosmd/__init__.py | nl -ba -v1Repository: jaylfc/taosmd Length of output: 239 🏁 Script executed: #!/bin/bash
set -euo pipefail
export LC_ALL=C
echo "== service remote forwarding for a2 a methods =="
sed -n '340,680p' taosmd/service.py
echo "== http_server a2a_import handler =="
sed -n '1500,1540p' taosmd/http_server.py
echo "== cli around RemoteClient creation =="
sed -n '500,545p' taosmd/cli.py
echo "== python call occurrences =="
python3 - <<'PY'
from pathlib import Path
import re
root = Path('.')
for p in sorted(root.glob('**/*.py')):
text = p.read_text(errors='ignore')
occurrences = [(i+1, line.strip()) for i,line in enumerate(text.splitlines(),1) if re.search(r'\ba2a_import\s*\(', line)]
if not occurrences:
continue
print(f'FILE {p}')
for i,line in occurrences:
print(f'{i}: {line}')
PY
echo "== __all__ in service/remote/api =="
sed -n '1200,1220p' taosmd/service.py
sed -n '410,425p' taosmd/remote.py
sed -n '1,140p' taosmd/__init__.py
sed -n '1,160p' taosmd/api.pyRepository: jaylfc/taosmd Length of output: 29868 Forward
🤖 Prompt for AI Agents |
||
| async def a2a_feed( | ||
| self, | ||
| *, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
CREATE UNIQUE INDEXon new columns runs before the migration that adds them.ArchiveStore.initexecutesINDEX_SCHEMA(Line 114) and only then callsmigrations.migrate(Line 118). On an existing database,CREATE TABLE IF NOT EXISTS archive_indexis a no-op, so the table still lackssourceandsource_idwhen this statement runs. SQLite then raisesOperationalError: no such column: sourceandinit()fails for every upgraded installation. Fresh databases are unaffected, which hides the fault in tests that start from an empty data dir.Move the index creation out of
INDEX_SCHEMAand rely on_archive_index_source_uidintaosmd/migrations.py, which already creates the same index after adding the columns.🐛 Proposed fix: create the index only in the migration
migrations._archive_index_source_uidcreatesidx_archive_source_uidfor both fresh and existing databases, because migration 3 runs whenever the columns are missing.Run the following script to confirm the ordering and that no other code path creates the index first:
🤖 Prompt for AI Agents