FIX-FORWARD PR #284: register the archive source_uid migration, add fresh-install + upgrade + import tests

[jaylfc]

CARD TITLE (intent, not commit subject): FIX-FORWARD PR #284: register the archive source_uid migration, add fresh-install + upgrade + import tests

Autonomous build of board card tsk-uyznqh.

Files:
taosmd/capabilities.py | 3 +-
taosmd/http_server.py | 34 +++-
taosmd/migrations.py | 25 +++
taosmd/remote.py | 18 ++
taosmd/service.py | 184 ++++++++++++++++++++-
tests/test_a2a.py | 31 ++++
tests/test_migrations.py | 93 ++++++++++-
10 files changed, 430 insertions(+), 13 deletions(-)

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Caution

Review failed

An error occurred during the review process. Please try again later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitar-bot]

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

_Gitar

[kilo-code-bot]

CRITICAL: Merge conflict markers left in module docstring

Lines 108 (<<<<<<< HEAD), 113 (=======), and 118 (>>>>>>>) are unresolved merge conflict markers inside the module docstring. The other branch removed GET /a2a/threads and GET /a2a/threads/{thread}/messages from the endpoint table, but those handlers still exist in http_server.py (lines 1726-1744). The resolved docstring should include the new POST /a2a/import alongside the existing A2A endpoints, and all conflict markers must be removed.

---

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

[kilo-code-bot]

WARNING: __all__ silently drops public API functions

fetch_by_ref (service.py:346), a2a_threads (service.py:689), and a2a_thread_messages (service.py:776) still exist in the module but were removed from __all__. Any code using from taosmd.service import * will no longer receive these names. Either restore them to __all__ or remove the functions if they are being deprecated.

---

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

[kilo-code-bot]

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

| Severity | Count |
|----------|
| CRITICAL | 1 |
| WARNING | 1 |

Issue Details (click to expand)

CRITICAL

File	Line	Issue
taosmd/http_server.py	108	Unresolved merge conflict markers (<<<<<<< HEAD, =======, >>>>>>>) left in module docstring; also removed GET /a2a/threads and GET /a2a/threads/{thread}/messages from endpoint table despite handlers still existing

WARNING

File	Line	Issue
taosmd/service.py	1446	fetch_by_ref, a2a_threads, and a2a_thread_messages removed from __all__ but functions still exist in module, breaking from taosmd.service import *

Files Reviewed (7 files)

• taosmd/http_server.py - 1 issue
• taosmd/service.py - 1 issue
• taosmd/archive.py
• taosmd/migrations.py
• taosmd/remote.py
• taosmd/capabilities.py
• tests/test_a2a.py
• tests/test_migrations.py

Fix these issues in Kilo Cloud

---

_{Reviewed by step-3.7-flash · Input: 164.9K · Output: 36.3K · Cached: 1.1M}

[jaylfc]

Review: BLOCKED

Two blockers, both from the same root cause: the carry forward exec/tsk-sgoilz merge (2a1af3d) was resolved mechanically and one side was taken wholesale. The migration half of this PR (the actual #284 fix-forward) is sound and I verified it separately; everything below is damage from the carry-forward.

All four checks are green, the full suite is 1400 passed / 12 skipped / 0 failed on the trial-merged tree, and neither blocker is visible to any of that. That is the interesting part, so I have written down why in each case.

---

B1 (CRITICAL): unresolved conflict markers are on the head commit

taosmd/http_server.py lines 108, 113, 118 on ed1d215:

108:<<<<<<< HEAD
113:=======
118:>>>>>>> 04e6b07af77bb30d797e0867a2647196c18b7c28

Same scan against origin/master returns nothing, so this is real and not a broken pattern.

Why every check stayed green: the markers sit inside the module docstring, so they are string content. ast.parse succeeds and the interpreter never sees them. No test can catch this, and none did. It would ship <<<<<<< HEAD into the published endpoint table.

The fix is a union, not a deletion. Please do not just remove the markers and keep one side, because both sides carry content the other drops:

• Keep the HEAD versions of the /a2a/messages and /a2a/stream rows. The other side restates both without the since is an epoch timestamp, values below 1e9 return 400 notes. That validation is still live: _SINCE_MIN_EPOCH = 1_000_000_000 at line 271. Taking the other side would make the docs understate real behaviour.
• Keep the HEAD /a2a/threads and /a2a/threads/{thread}/messages rows. Both handlers are still routed, at lines 1051-1054.
• Add only the POST /a2a/import block from the other side.

B2 (HIGH): __all__ drops three public names that still exist

taosmd/service.py. Measured with AST rather than grep (my first grep pattern returned 0 on master too, so it was not evidence):

master: 49 defs, __all__ = 39
head  : 50 defs, __all__ = 37

fetch_by_ref           master(def=True, all=True)  head(def=True, all=False)
a2a_threads            master(def=True, all=True)  head(def=True, all=False)
a2a_thread_messages    master(def=True, all=True)  head(def=True, all=False)

DROPPED: a2a_thread_messages, a2a_threads, fetch_by_ref
ADDED:   a2a_import

All three functions are still defined, so this is a pure export-surface regression: from taosmd.service import * silently stops providing them. Restore the three and keep a2a_import.

Worth flagging to whoever owns it: deleted-symbols-gate passed on this commit. The gate does not appear to cover __all__ membership when the def survives, which is exactly the shape a bad merge produces.

---

Non-blocking

3. find_reply_target (archive.py:372) is dead code. Zero callers anywhere in the tree. Control: find_source_ids has a real caller at service.py:938. a2a_import resolves replies through the in-memory id_map instead, so this is 18 lines of untested API surface. Drop it, or wire it in.

4. Formatting artifacts from the same merge. "last_id": last_id, sits at column 0 inside the return dict (service.py:1036), the __all__ continuation lines are at column 0, and there are zero blank lines between a2a_thread_messages and async def a2a_import (PEP8 wants two). Column-0 string-literal lines went from 2 on master to 4 on head. All valid Python, all signs the resolution was mechanical.

5. a2a_import is thinly tested for its size. 182 lines of service code plus the remote client and the HTTP handler, and exactly one test (test_a2a_import_preserves_historical_ts), which is happy path. The docstring documents idempotency on (source, source_id), reply_to_source_id resolution, forward-reference behaviour, defer_index, and whole-batch refusal. None of those have a test. Idempotency is the headline property and the one a re-run will exercise first in production. I would want at least: a double-import asserting skipped, a reply_to resolution assert, and the unresolvable-reply_to 400.

   For the record, the 400 path itself is correct by construction even though it is untested: the dispatch wrapper maps ValueError to 400 at http_server.py:1163, so the service-layer raises do surface as documented.

6. Docstring slightly overclaims on partial writes. "leaves the archive untouched (fail-loud, zero partial writes)" holds for validation failures, but a failure part way through the write loop leaves earlier rows committed. Idempotency makes that recoverable on re-run, so the design is fine; the sentence just wants narrowing to validation.

7. Question, not a finding: capabilities.py extends the existing a2a.v1 probe with a2a_import and "/a2a/import" rather than minting a new identifier. The module's own contract says additive backwards-compatible changes keep the same identifier, so this is compliant, and no build can have the new probe without the new symbol since they ship together. The only residual is that a consumer cannot tell an a2a.v1 that has import from one that does not. Fine to leave as is; answer if there was a reason to care.

---

What I verified and you do not need to redo

• Trial-merged into master clean; full suite 1400 passed, 12 skipped, 0 failed, working tree clean, with archive_index_source_uid present in the same command.
• Red witness on the migration is clean. Removing the Migration(3, ...) registration, which is the exact Revise PR #230: the new index breaks every existing install on upgrade #284 defect, turns 5 tests red and leaves 39 passing, including both new ones. My first attempt broke the syntax and produced an IndentationError, which is a red for the wrong reason; the second run did an ast.parse control first.
• The migration itself is sound. The unique index is partial (WHERE source IS NOT NULL AND source_id IS NOT NULL), so adding it to a populated table is safe: both columns are new, every existing row is NULL, none qualifies.
• Good call on writing your own positive control into the fresh-install test ("project is also present, so a failure reads as this migration did not run"). That was unprompted and it is the right instinct.

Fix B1 and B2 and I will re-review promptly. The migration work underneath this is good and I do not want it stuck behind merge damage.

[jaylfc]

Closing this in favour of the revision card tsk-xe3h5v, which carries the fix list from the review above.

To be clear about why: the migration work on this branch is correct and I am not asking for it back. The red witness is clean (removing Migration(3, ...) turns 5 tests red and leaves 39 passing), the partial unique index is the right call on a populated table, and the fresh-install test carries its own positive control. That half of the card is done.

What is wrong is purely the carry-forward squash: unresolved conflict markers committed into the http_server.py module docstring, and three public names dropped from service.__all__. Both slipped past four green checks and a 1400-pass suite because neither is reachable by a test.

I am closing rather than leaving this open because a blocked PR has no revision path in this fleet, and because closing it frees the throttle slot and un-blocks the one-PR-per-task guard so the follow-up can open cleanly. This is the same route #260 took to a merged #299.

tsk-xe3h5v instructs branching from exec/tsk-uyznqh rather than master, so none of the 182 lines of a2a_import work gets rebuilt. That branch is deliberately NOT deleted. Please leave it in place.