forked from molecule-ai/molecule-core
a33c879017
Closes #3026. Final piece of RFC #2945. ## What's new New package internal/messagestore/ holds: - MessageStore interface — single read-side contract operators implement to plug in alternative chat-history backends. - ChatMessage / ChatAttachment / ListOptions types — canonical data shapes returned by any impl, mirrors canvas's TS ChatMessage. - PostgresMessageStore — platform-default impl wrapping the activity_logs query + A2A-envelope parser ported in PR-C. Behavior is byte-identical to the pre-PR-D handler. ## What moves The activity_logs query, the parser (activityRowToChatMessages, extractRequestText, extractChatResponseText, extractFilesFromTask, etc.), and the internal-self-message predicate all migrate from internal/handlers/chat_history.go into the new package. handlers/ chat_history.go becomes a thin HTTP-shape adapter: parse query params → store.List(ctx, workspaceID, opts) → emit JSON Compile-time interface assertion in postgres_store.go catches future drift if the interface evolves and the impl falls behind. ## Why this PR OSS operators wanting to: - Tier hot/warm/cold storage (recent in Postgres, archival in S3) - Use a vector store with hybrid search (Pinecone, Weaviate) - Run an in-memory store for ephemeral test environments - Federate history across regions …had no extension point — they'd have to fork the handler. This PR makes that a constructor swap at router.go. ## Tests Parser-level (22 tests, MOVED to internal/messagestore/postgres_ store_test.go): every TS test case in canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts has a Go counterpart. Timestamp preservation, user/agent extraction, internal-self filter, role decision (status=error vs agent-error prefix), v0/v1 file shapes, malformed JSON resilience. Handler-level (9 NEW tests in internal/handlers/chat_history_test.go): thin adapter coverage using a fake MessageStore. UUID validation, before_ts RFC3339 validation, default limit, max-limit clamp, invalid-limit fallback, before_ts passthrough, empty-array (not null) JSON shape, attachment shape preservation, store-error → 502 mapping. Compile-time interface conformance: PostgresMessageStore satisfies MessageStore, fakeStore (test fake) satisfies MessageStore. Mutation-tested. Removed UUID validation in the handler; confirmed TestChatHistoryHandler_RejectsNonUUIDWorkspaceID fires red (status 200 instead of 400, non-UUID reaches the store). Restored, all green. Full handlers + messagestore + router test runs green; full repo go test ./... green. ## SSOT decision ChatMessage / ChatAttachment / parser / DB query all live in internal/messagestore/ ONLY. handlers/chat_history.go imports the package and uses the types via messagestore.ChatMessage etc. — no re-declaration anywhere. ## Three weakest spots (hostile-reviewer self-pass) 1. The internal-self prefix list (Delegation results are ready...) is a package var in messagestore/postgres_store.go. A future impl that wants to override the predicate must reach into the package to use IsInternalSelfMessage or define its own. Acceptable: the predicate is part of the contract; if an impl wants different semantics it owns that decision explicitly. 2. ListOptions has Limit + BeforeTS + HasBefore; future paging needs (after_ts, peer_id filter, role filter) require additive struct field additions, which is a soft API break for any impl that handles ListOptions positionally. Mitigated by Go's struct-literal convention (named fields by default); also flagged in the interface comment for impl authors. 3. The handler does NOT log when a store returns an error — it just maps to 502. An impl that wants to surface its error class up the stack can't, today. If/when an impl needs that, the interface can add a typed-error contract in a follow-up. Today's coverage is sufficient: most ops issues land in the store impl's own logs. ## Security review - Untrusted input? Same as PR-C — agent-emitted JSON parsed defensively. New fakeStore in tests can't reach production. - Trust boundary? Same. Interface lives BEHIND wsAuth; impls only see workspace IDs already authenticated. - Auth/authz? Inherited from handler; the interface doesn't authenticate. - PII / secrets in logs? Documented in the interface contract: impls MUST NOT log full message bodies / attachment URIs. The Postgres impl logs nothing on the happy path. - Output sanitization? Same plain-text + opaque-URI surface as PR-C. Canvas validates attachment-URI schemes. No security-relevant changes beyond what /chat-history already exposes via PR-C. Considered, not skipped. ## Versioning / backwards compat - New internal package. Zero public API change. - Single caller site in router.go updated (one-line constructor change). NewChatHistoryHandler() → NewChatHistoryHandler(store). - No schema change, no migration. - Existing /chat-history endpoint unchanged on the wire — clients don't notice the refactor. ## Phasing This is the final RFC #2945 piece. Follow-ups parked: - PR-C-2 (canvas migration): swap canvas loadMessagesFromDB to call /chat-history instead of /activity. Independent of this PR; blocked only by canvas team's calendar. - Sample alternative impls (S3, in-memory) for OSS docs: separate PR when the first OSS consumer materializes; demonstration code untested against a real workload is anti-pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|---|---|---|
| .. | ||
| admin_delegations_route_test.go | ||
| admin_test_token_route_test.go | ||
| canvas_proxy_test.go | ||
| canvas_proxy.go | ||
| cp_proxy_test.go | ||
| cp_proxy.go | ||
| router.go | ||