From 6c4c74a5047dd4b75599617e666ec6370521237d Mon Sep 17 00:00:00 2001 From: Nan Gao Date: Sat, 10 Oct 2026 14:00:00 +0800 Subject: [PATCH] fix(channels): share chat-to-thread bindings through the database (#6558) * fix(channels): share chat-to-thread bindings through the database ChannelManager kept its IM chat-to-thread bindings (channel_name:chat_id[:topic_id] -> thread_id) in a per-process JSON file, channels/store.json, loaded once and rewritten whole on every change. With several Gateway processes a binding created on one replica was invisible to the others (the next message for the same chat opened a second thread) and concurrent writers clobbered each other's file. - New channel_thread_bindings table (migration 0036_channel_thread_bindings) and harness package deerflow.persistence.channel_thread_bindings: key PK (the legacy composite string), channel_name, chat_id, nullable topic_id, thread_id, user_id, epoch created_at/updated_at; index (channel_name, chat_id). Dialect upsert keeps created_at; prefix remove and batched ON CONFLICT DO NOTHING import. - app.channels.store.ChannelStore is an async protocol: SqlChannelStore for sqlite/postgres, JsonChannelStore (file I/O via asyncio.to_thread) for database.backend: memory. resolve_channel_store picks at ChannelService construction; ChannelService.start() imports an existing store.json once into an empty table and renames it store.json.migrated (a concurrently importing peer neither duplicates nor clobbers; a populated table leaves the file alone). - Callers updated: manager, connection_identity.lookup_thread_id, Feishu (send() awaits the mapping writer; the synchronous lark callback bridges its two store calls to the Gateway loop with a bounded wait), Discord (channels_dir). - Head pins in the 0027_0028/0029/0032/0034/0035 migration tests point at the new head; docs (channels and migrations AGENTS.md, CONFIGURATION.md, README) and CHANGELOG updated. * docs(changelog): link the channel thread bindings entry to #6558 * fix(channels): reject only the Gateway loop in the Feishu store bridge lark-oapi 1.5.5 invokes the message callback synchronously from its async frame handler, on the private event loop that FeishuChannel._run_ws starts for the SDK thread. The bridge's guard treated any running loop as unsafe, so every real inbound message skipped the stored-topic lookup and the pending-mapping restore: a reply whose parent card was mapped but whose root was not fell back to the unknown root, and could consume an unrelated pending clarification. Only a wait issued from the Gateway loop itself (the loop that executes the submitted coroutine) is refused now; the SDK's own loop may block for the bounded wait, as that thread blocked on the JSON rewrite before. Tests drive _on_message from a worker thread running its own loop, the production shape, for both the lookup and the write path, and pin the Gateway-loop refusal. * fix(channels): defer unresolved Feishu lookups and harden the store.json import Review follow-ups on the chat-to-thread binding store (#6558): - Import: a legacy entry whose key, channel_name, chat_id, topic_id, thread_id or user_id exceeds its column width is skipped and counted with the malformed ones instead of failing the whole batch on PostgreSQL (SQLite silently accepted the same row, so the backends diverged on one file). The corrupt-file warning now says what each caller does: the JSON store starts fresh, the import is skipped and the file kept. - Feishu bridge: store calls run on the Gateway loop under a 3s asyncio.wait_for (the SDK loop also services ws keepalives; the old 10s bound could stall it per message), and the lark-thread wait is that budget plus 1s. A deadline or store error is StoreLookupUnresolved, never a miss: _on_message no longer routes such a reply onto its unknown root (which created a second thread and later overwrote the parent alias). The message joins a per-chat FIFO drained on the Gateway loop with backoff for up to 30s, then is dropped with a warning. _on_message is split into parse / resolve / dispatch so the deferred path dispatches the same way; later messages for the chat queue behind the deferred one. * fix(tests): expect follow_redirects in the Jina client kwargs after #5808 #5808 made JinaClient pass follow_redirects=True to httpx.AsyncClient and updated test_jina_client.py, but test_jina_response_limit.py and test_jina_retry_after.py still pin the exact client kwargs to proxy and trust_env, so every backend-unit-tests shard that lands on them fails on main (the backend unit-test workflow only runs on pull requests, which is why the push to main did not surface it). Update the two expectations to include the new keyword. (cherry picked from commit d5aa23156ce19ce36d95df03953442f83ae6d050) * Revert "fix(tests): expect follow_redirects in the Jina client kwargs after #5808" This reverts commit 5bee00bfb09c65a30f77fb4df47a1f82c47ecef7. * fix(channels): bound deferred Feishu routing by the inbound queue capacity Review follow-up on #6558: a store outage let every subsequent message for a chat join the deferred queue with no capacity check (the bus reservation was only taken after routing succeeded), and each queued message started a fresh 30s retry budget when it reached the head, so a burst could be retained for N x 30s and grow without bound. - Intake is reserved BEFORE routing for every store-backed message (bus.reserve_inbound is thread-safe and already used from SDK threads), so deferred and live messages share the one configured bound, channels.inbound_queue_maxsize; a message that cannot be admitted is dropped at once with a warning, never queued or routed as a miss. - The reservation travels with the message into dispatch (committed by _prepare_inbound), is released on the batch path (the flush reserves at publish time), on drop after the budget, on a failed deferral, on dispatch failure and for every leftover when the drain task is cancelled by stop(). - The retry budget runs from the message's arrival: a queued message whose budget is spent when it reaches the head is dropped without a lookup. - The bounded resolver returns the unresolved outcome instead of raising it through the tracked task, which logged an ERROR for an expected, handled store timeout. - Upstream's new blocking-io test constructs the store by its old class name; it now uses JsonChannelStore. --------- Co-authored-by: Willem Jiang --- CHANGELOG.md | 24 + README.md | 2 +- backend/app/channels/AGENTS.md | 6 +- backend/app/channels/connection_identity.py | 9 +- backend/app/channels/discord.py | 10 +- backend/app/channels/feishu.py | 690 +++++++++++++----- backend/app/channels/manager.py | 4 +- backend/app/channels/runtime_config_store.py | 2 +- backend/app/channels/service.py | 26 +- backend/app/channels/store.py | 409 ++++++++--- backend/app/gateway/deps.py | 4 + backend/docs/CONFIGURATION.md | 1 + .../channel_thread_bindings/__init__.py | 20 + .../channel_thread_bindings/model.py | 116 +++ .../channel_thread_bindings/sql.py | 139 ++++ .../deerflow/persistence/migrations/AGENTS.md | 6 +- .../versions/0038_channel_thread_bindings.py | 74 ++ .../deerflow/persistence/models/__init__.py | 2 + .../test_channel_thread_binding_store.py | 103 +++ .../blocking_io/test_discord_channel_state.py | 6 +- .../test_model_status_agent_config_offloop.py | 6 +- backend/tests/test_buzz_channel.py | 10 +- .../tests/test_channel_intake_backpressure.py | 12 +- backend/tests/test_channel_stream_errors.py | 4 +- .../test_channel_thread_binding_store.py | 674 +++++++++++++++++ backend/tests/test_channels.py | 483 +++++------- backend/tests/test_dingtalk_channel.py | 2 +- backend/tests/test_feishu_parser.py | 511 +++++++++++-- backend/tests/test_github_dispatcher.py | 12 +- backend/tests/test_github_token_plumbing.py | 4 +- ...migration_0027_0028_notification_outbox.py | 2 +- ...st_migration_0029_scheduler_agent_tasks.py | 2 +- ...migration_0032_activity_and_task_events.py | 6 +- ..._migration_0034_run_event_seq_watermark.py | 6 +- .../test_migration_0035_login_throttle.py | 6 +- ...gration_0037_project_document_summaries.py | 4 +- ..._migration_0038_channel_thread_bindings.py | 187 +++++ .../tests/test_multi_pod_inbound_dedupe.py | 6 +- backend/tests/test_qq_binding.py | 4 +- backend/tests/test_qq_lifecycle.py | 7 +- backend/tests/test_run_origin.py | 4 +- backend/tests/test_trace_entry_points.py | 4 +- 42 files changed, 2915 insertions(+), 694 deletions(-) create mode 100644 backend/packages/harness/deerflow/persistence/channel_thread_bindings/__init__.py create mode 100644 backend/packages/harness/deerflow/persistence/channel_thread_bindings/model.py create mode 100644 backend/packages/harness/deerflow/persistence/channel_thread_bindings/sql.py create mode 100644 backend/packages/harness/deerflow/persistence/migrations/versions/0038_channel_thread_bindings.py create mode 100644 backend/tests/blocking_io/test_channel_thread_binding_store.py create mode 100644 backend/tests/test_channel_thread_binding_store.py create mode 100644 backend/tests/test_migration_0038_channel_thread_bindings.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ed540b4c74..d6b3579ef2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -833,6 +833,29 @@ This release closes that milestone with **439 merged pull requests**. ### Fixed +- **channels:** IM conversations now keep their DeerFlow thread across Gateway + replicas. The `ChannelManager` kept its chat-to-thread bindings + (`channel_name:chat_id[:topic_id]` → `thread_id`) in a per-process JSON file, + `channels/store.json`, loaded once and rewritten whole on every change, so + with several Gateway instances a binding created on one was invisible to the + others — the next message for the same chat landing elsewhere opened a second + thread — and concurrent writers clobbered each other's file. The bindings now + live in the shared `channel_thread_bindings` table (migration + `0038_channel_thread_bindings`) whenever `database.backend` is `sqlite` or + `postgres`; `memory` keeps the JSON file. On the first start after upgrading, + an existing `store.json` is imported once into an empty table (`INSERT … ON + CONFLICT DO NOTHING`, so two replicas importing at the same time cannot + duplicate a binding) and renamed `store.json.migrated`; a populated table + leaves the file untouched; entries whose key components exceed the table's + column widths are skipped and counted instead of failing the whole import. + The store API is async so the database never blocks the Gateway loop; + Feishu's synchronous lark callback bridges its lookups to that loop with a + short bounded wait, and a lookup the database does not answer in time is + retried on the Gateway loop (up to 30 s, then the message is dropped with a + warning) rather than routed as a missing mapping onto a new thread; such + deferred messages hold a slot of the bounded inbound queue + (`channels.inbound_queue_maxsize`), so a database outage cannot grow the + backlog, and one that cannot be admitted is dropped with a warning. ([#6558]) - **runtime:** The JSONL event store no longer loses events after a torn final line. A write interrupted mid-record left the file without a trailing newline, so the next append was glued onto the partial record and both became one @@ -9426,5 +9449,6 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6537]: https://github.com/bytedance/deer-flow/pull/6537 [#6543]: https://github.com/bytedance/deer-flow/pull/6543 [#6556]: https://github.com/bytedance/deer-flow/pull/6556 +[#6558]: https://github.com/bytedance/deer-flow/pull/6558 [#6580]: https://github.com/bytedance/deer-flow/pull/6580 [#6582]: https://github.com/bytedance/deer-flow/pull/6582 diff --git a/README.md b/README.md index 88dedaef90..08376e08f7 100644 --- a/README.md +++ b/README.md @@ -527,7 +527,7 @@ Browser login uses `HttpOnly` session cookies. The login page offers a "keep me DeerFlow still uses `Forwarded` / `X-Forwarded-*` headers to recover the browser-facing scheme and origin behind a proxy. The bundled nginx sets `X-Forwarded-Proto`, but preserves an upstream HTTPS value and does not overwrite every forwarded header. Configure the outer trusted proxy to replace or strip client-supplied forwarding headers before traffic reaches DeerFlow. > [!IMPORTANT] -> The Gateway still owns active run tasks in process, so production defaults to a single Gateway worker (`GATEWAY_WORKERS=1`). Multi-worker deployments require Postgres, the Redis stream bridge (`stream_bridge.type: redis`), `run_ownership.heartbeat_enabled: true`, and `run_events.backend: db`; process-local memory/JSONL event stores cannot enforce singleton delivery receipts across workers. Kubernetes replicas run one worker per Pod, which the worker count cannot see: declare them with `deployment.multi_instance: true` (or `DEER_FLOW_MULTI_INSTANCE=1`, which deploy tooling such as a Helm chart can set from its replica count) so the startup gate enforces the same prerequisites instead of staying inert. The bridge shares SSE delivery and bounded `Last-Event-ID` replay across workers. When a valid reconnect cursor has been trimmed, or a subscriber that already established an empty-stream wait falls behind before its first delivery, Memory and Redis emit a machine-readable SSE `gap` event instead of silently returning a partial replay; the Web UI reloads durable thread/event state and resumes from the retained tail. Lease reconciliation marks runs from dead workers as errors, persists their delivery receipts, publishes the terminal stream marker, schedules retained-stream cleanup, and updates the affected thread status. SSE, `/wait`, and internal stream consumers use `stream_bridge.heartbeat_interval_seconds` (default `15`) for idle liveness checks; changing it requires a Gateway restart. Malformed Redis reconnect IDs live-tail new events instead of replaying the retained buffer, and the rolling retained-buffer TTL (`stream_ttl_seconds`) remains a cleanup safety net rather than a run timeout. Failed-login counters and lockouts for `POST /api/v1/auth/login/local` (`auth.local.max_login_attempts` / `lockout_seconds`) are kept in the shared `login_throttle` table whenever the application database is SQLite or Postgres (`auth.local.throttle_storage: auto`, the default), so every replica enforces one lockout per client IP; `memory` keeps the historical per-process counter, which under N replicas hands an attacker N × `max_login_attempts` guesses and logs a startup warning. IM channel state and other process-local services still need their own multi-worker coordination. +> The Gateway still owns active run tasks in process, so production defaults to a single Gateway worker (`GATEWAY_WORKERS=1`). Multi-worker deployments require Postgres, the Redis stream bridge (`stream_bridge.type: redis`), `run_ownership.heartbeat_enabled: true`, and `run_events.backend: db`; process-local memory/JSONL event stores cannot enforce singleton delivery receipts across workers. Kubernetes replicas run one worker per Pod, which the worker count cannot see: declare them with `deployment.multi_instance: true` (or `DEER_FLOW_MULTI_INSTANCE=1`, which deploy tooling such as a Helm chart can set from its replica count) so the startup gate enforces the same prerequisites instead of staying inert. The bridge shares SSE delivery and bounded `Last-Event-ID` replay across workers. When a valid reconnect cursor has been trimmed, or a subscriber that already established an empty-stream wait falls behind before its first delivery, Memory and Redis emit a machine-readable SSE `gap` event instead of silently returning a partial replay; the Web UI reloads durable thread/event state and resumes from the retained tail. Lease reconciliation marks runs from dead workers as errors, persists their delivery receipts, publishes the terminal stream marker, schedules retained-stream cleanup, and updates the affected thread status. SSE, `/wait`, and internal stream consumers use `stream_bridge.heartbeat_interval_seconds` (default `15`) for idle liveness checks; changing it requires a Gateway restart. Malformed Redis reconnect IDs live-tail new events instead of replaying the retained buffer, and the rolling retained-buffer TTL (`stream_ttl_seconds`) remains a cleanup safety net rather than a run timeout. Failed-login counters and lockouts for `POST /api/v1/auth/login/local` (`auth.local.max_login_attempts` / `lockout_seconds`) are kept in the shared `login_throttle` table whenever the application database is SQLite or Postgres (`auth.local.throttle_storage: auto`, the default), so every replica enforces one lockout per client IP; `memory` keeps the historical per-process counter, which under N replicas hands an attacker N × `max_login_attempts` guesses and logs a startup warning. IM chat-to-thread bindings live in the shared `channel_thread_bindings` table whenever the application database is SQLite or Postgres (an existing `channels/store.json` is imported once at startup and renamed `store.json.migrated`); the remaining IM channel state (each instance's platform connections, follow-up buffers) still needs its own multi-worker coordination. > > In single-process JSONL deployments, cancelling an admitted event-store mutation waits for its background file I/O, rollback, and bookkeeping to settle before releasing the thread write lock. This prevents an older cancelled write from recreating deleted records or rolling back a later successful write. Cancellation can therefore wait on slow storage; it does not stop an in-flight filesystem operation. Callers still waiting to acquire the lock can cancel without starting a mutation. A batch spanning multiple threads drains its current thread group before propagating cancellation; subsequent thread groups do not start. > diff --git a/backend/app/channels/AGENTS.md b/backend/app/channels/AGENTS.md index 933662179d..06cd79233c 100644 --- a/backend/app/channels/AGENTS.md +++ b/backend/app/channels/AGENTS.md @@ -9,7 +9,7 @@ Cursor writes drain before cancellation; token expiry drains cursor/auth clearin **Components**: - `message_bus.py` - Inbound queue → dispatcher; outbound uses a dispatch-start listener snapshot. Unsubscribe neither cancels nor drains captured callbacks. -- `store.py` - JSON persistence for `channel_name:chat_id[:topic_id]` → `thread_id`. Guard every `_data` access with `_lock`; `list_entries()` snapshots under the lock and formats after release so concurrent writers cannot resize the dict during iteration. +- `store.py` - `ChannelStore` protocol (async `get_thread_id`/`set_thread_id`/`remove`/`list_entries`) for `channel_name:chat_id[:topic_id]` → `thread_id`. `SqlChannelStore` (`channel_thread_bindings`, harness `persistence/channel_thread_bindings/`; upsert keeps `created_at`, topic-less `remove` deletes by key prefix) for sqlite/postgres, so replicas share bindings; `JsonChannelStore` (`{base_dir}/channels/store.json`, loaded once per process, file I/O via `to_thread` under `_lock`) only for `database.backend: memory`. `resolve_channel_store` picks; `ChannelService.start()` runs `SqlChannelStore.import_legacy_json` once (file + empty table → `INSERT … ON CONFLICT DO NOTHING`, rename to `store.json.migrated`, a peer's rename tolerated; populated table → file kept). Feishu's sync lark callback (on the SDK's own loop) bridges `_resolve_routing_bounded` to the Gateway loop via `_run_store_call` (tracked future; refuses only a wait from the Gateway loop itself). Store calls run under `FEISHU_STORE_LOOKUP_TIMEOUT_SECONDS` (3s; the SDK loop also services ws keepalives); timeout or store error = *unresolved* (`StoreLookupUnresolved`), never a miss: the message joins a per-chat FIFO drained on the Gateway loop with backoff up to `FEISHU_STORE_RESOLVE_TOTAL_BUDGET_SECONDS` (30s), then is dropped with a warning, never routed to a new conversation; each deferred message holds a bus intake reservation (the `inbound_queue_maxsize` bound) and its 30s budget runs from arrival. `discord.py` reads `channels_dir`. Tests: `test_channel_thread_binding_store.py` (also under `blocking_io/`). - `manager.py` - Core dispatcher: creates threads via `client.threads.create()`, routes commands including `/goal` (setting a goal persists it through Gateway and then routes the objective as a chat turn) and `/agent` (`list` is owner-scoped; `use` creates a fresh thread and persists the Custom Agent selection under both the channel restart key and Web's canonical routing metadata so existing checkpoint lineage never changes agent across IM/Web), keeps Slack/Discord on `client.runs.wait()`, uses `client.runs.stream(["messages-tuple", "values"])` for Feishu/Telegram incremental outbound updates, serializes same-thread Feishu turns in-manager when the channel's `ChannelRunPolicy.serialize_thread_runs=True` so rapid follow-ups queue instead of tripping the runtime busy reply, and switches to `client.runs.create()` (fire-and-forget, returns once the run is `pending`) for channels whose `ChannelRunPolicy.fire_and_forget=True` so long autonomous runs do not hit the SDK default 300s `httpx.ReadTimeout` Streaming `error` frames use the same failure path as transport exceptions. Publish the final outbound before releasing the inbound dedupe key, so provider redelivery can retry without overtaking the terminal reply. Log the frame's error name and message at WARNING with the thread ID; keep the synthetic error generic so payload details do not change reply selection. **What may be published from the stream is an allowlist, not a denylist** (`_accumulate_stream_text`): only assistant message types — LangChain serializes `AIMessage.type` as `"ai"` and `AIMessageChunk.type` as `"AIMessageChunk"`, plus the OpenAI-style `"assistant"` spelling for foreign runtimes — become displayable text. The previous rule rejected only payloads whose `type` contained `"tool"` and therefore published everything else, which leaked DeerFlow's hidden model context to every streaming IM channel: `DynamicContextMiddleware` injects recalled memory as a hidden `HumanMessage` (`type == "human"`) and rewrites the user's own turn into a new `HumanMessage`, `DurableContextMiddleware` injects a hidden `` `HumanMessage`, and LangGraph fans those state writes out on the `messages-tuple` stream. Proved live on a Buzz relay, which published a `` fact block and, in another run, a verbatim echo of the user's own message as the assistant's reply. Matching is by prefix (`ai` / `assistant`), never substring, because ordinary words contain `"ai"` (`chain`, `domain`). The message type is resolved by `_stream_payload_type`, which handles both the `model_dump()` shape DeerFlow's own gateway emits and LangChain's `to_json()` constructor shape (whose top-level `type` is the literal `"constructor"`, with the class name at the tail of the `id` path). A bare `str` payload is no longer accepted at all: it carries no type information, so it cannot be attributed to the assistant, and nothing in DeerFlow produces one (`runtime/serialization.py::serialize_messages_tuple` always emits `[message_dict, metadata]`). @@ -17,7 +17,7 @@ Cursor writes drain before cancellation; token expiry drains cursor/auth clearin - `wecom.py` - WeCom WebSocket channel with proactive `send_notification`; `_ws_transport_up` maps disconnects to `ChannelUnavailable` (no retry used). `capabilities.py` - SDK-free `CHANNEL_CAPABILITIES`. - `service.py` - Drain manager work before transports; retain timed-out instances for retry. Live Feishu/DingTalk/Telegram joins raise `ChannelStopTimeout`: retain, defer replacement/restart, log WARNING without traceback; wedged instances report not running. Reload returns snapshots; restart owns cached fallback/publication under the channel lock. `_stopping` fences starts/publication (including after pre-start I/O) until shutdown succeeds. Lock manager startup through stale cleanup; capture the shutdown generation before locking to fence queued starts. `stop()` stays unlocked to fence in-flight startup. - `slack.py` / `feishu.py` / `telegram.py` / `discord.py` / `dingtalk.py` - Platform-specific implementations (`feishu.py` tracks the running card `message_id` in memory and patches the same card in place; `telegram.py` accepts inbound text/photos/documents, preserves media captions, hands token-free attachment bytes to the shared upload pipeline, edits the "Working on it..." stream target in place via `editMessageText`, and can optionally send final Markdown replies as Rich Messages through `channels.telegram.rich_messages` — the Rich path only fires when `rich_messages` is on **and** the text contains a rich construct (fenced code, emphasis, a table, a task list, `
`, block math, or a link), so structured command/error replies (none of these) stay plain text and their newlines and `` tokens survive; `discord.py` runs `_on_message` on discord.py's private loop, so connection-repository work (identity lookup with the reservation commit, `/connect` binding) goes to the Gateway loop through `_submit_threadsafe_coroutine` — the SQLAlchemy engine is bound there — and bind replies return through `_run_on_discord_loop`; typing registers before that hand-off settles, and a failed hand-off skips the ack and stops the indicator it started unless another message still relies on it; `discord.py` registers typing loops before inbound handling yields and `_start_typing()` refuses work once `_running` is false; each tick sends the indicator with discord.py 2.x's `await channel.typing()` (`trigger_typing()` was removed in 2.0, and each typing loop logs its first failed tick at WARNING, later ones at DEBUG); because `stop()` runs on the main loop while typing tasks belong to `_discord_loop`, normal cross-thread cancellation, awaiting, and map cleanup are scheduled there with a bounded wait, while `_run_client()` drains tasks in its `finally` block before an exception/disconnect can make that loop unusable; an already-stopped foreign loop must never have its tasks awaited from the main loop; to that end every outbound cross-loop call (`send` / `send_file` / `_get_channel_or_thread`) goes through `_run_on_discord_loop`, which bounds the await (`DISCORD_OUTBOUND_TIMEOUT_SECONDS` 30s; `DISCORD_UPLOAD_TIMEOUT_SECONDS` 120s for file uploads, whose unbounded-size payload needs room for a slow uplink plus 429 retry-after) and fails fast with a `RuntimeError` when the client loop is missing or not running — a dead client becomes a logged send failure instead of a permanently hung `ChannelManager` worker — and `is_running` reports client-thread aliveness (like `feishu.py`) so `ensure_channel_ready` can restart the channel after `_run_client()` exits on a fatal error; `dingtalk.py` optionally uses AI Card streaming for in-place updates when `card_template_id` is configured, and overrides `receive_file` to download inbound images (`picture`/`richText`) and documents (`file`) by `downloadCode` into the thread uploads bucket, mirroring `feishu.py`) -- `buzz.py` - Buzz (Nostr relay) implementation: one NIP-42-authenticated websocket, pubkey-allowlist + mention/DM/thread-follow gating (thread-follow reads via the manager's `connection_identity.lookup_thread_id`, so bound authors' repo-only threads count), streaming replies via in-place kind-40003 edits; requires the `buzz` dependency extra. Its durable seen-event replay guard uses async `aseen()` / `arecord()` / `aflush()` boundaries: initial JSON reads and coalesced atomic writes run off the Gateway event loop, and `stop()` awaits the final flush before returning. **Subscription model** (operator-facing version in [IM_CHANNEL_CONNECTIONS.md](../../docs/IM_CHANNEL_CONNECTIONS.md#buzz-subscription-model)): the relay fans kind-9 chat events out **only** to channel-scoped subscriptions, proved against a live relay — `REQ {"kinds":[9]}` is accepted and answered with `EOSE` but never receives an event (the connector authenticated and then sat silent forever), `REQ {"kinds":[9],"#h":[uuid]}` works, and a multi-value `#h` matches nothing, so it is strictly one REQ per channel (the same shape as Buzz's own `buzz-acp` harness). Every connection therefore rebuilds three kinds of subscription after NIP-42 auth: `buzz-discovery` (`{"kinds":[39000]}`, a historical query returning exactly the channels this identity belongs to, one stored event each then `EOSE` — adding `#p` returns zero, do not "narrow" it), `buzz-membership` (`{"kinds":[44100,44101],"#p":[us],"since":}`, the relay-signed member-added/member-removed notifications whose `p` tag names the affected member and `h` tag the channel), and one `buzz-chat-` per discovered channel. Chat subscriptions open as each kind-39000 arrives; the discovery `EOSE` is the completeness barrier that retries any that failed and warns when discovery found nothing. A kind-44100 for our pubkey subscribes to the new channel **live** (then re-issues discovery so its name/type reach the DM-detection cache, but only when that channel's metadata is actually missing — an unconditional refresh is how a burst of 44100s multiplied into one discovery pass each); a kind-44101 issues `buzz_nostr.close_frame` for exactly that channel's subscription and drops its metadata. **The membership subscription is scoped to LIVE events** and this is load-bearing: buzz-relay *stores* 44100/44101 and serves history newest-first (default limit 2000), so an unscoped filter replayed the whole membership history on every connect — every stored add read as live, re-running discovery once each (M+1 discovery passes × N stored kind-39000 events per connect, observed live as two `channel discovery complete` lines and one channel logged `` because the 44100 path subscribed before its metadata arrived), re-subscribing channels we have since been removed from, and letting a stored 44101 transiently unsubscribe a channel we are still in. `since` is anchored at the moment the socket opened (`_session_started_at`) minus `MEMBERSHIP_LOOKBACK_SECONDS` (60) of slack, which covers both relay clock skew and a membership change published *during* the connect/auth handshake; the slack can only cost an idempotent replay of the last minute. **A relay `CLOSED` frame is recovered, not merely forgotten** — every subscription on the socket fails silently when dropped, so `_handle_closed` re-issues it, bounded by `MAX_RESUBSCRIBE_ATTEMPTS` (3) per subscription id per connection *and per auth epoch* (`_resubscribe_attempts` is reset on session start, on re-auth, and by `stop()`, so pre-auth rejections — which the auth branch already recovers wholesale — never spend the authenticated session's budget). The first retry is immediate (the common case is a one-off hiccup); later ones back off 1s then 2s, awaited inline in the read loop rather than in a background task that could outlive its own socket. **`auth-required:` before this socket has completed its NIP-42 handshake is the expected bootstrap sequence, not a refusal**, and `_handle_closed` short-circuits it ahead of every recovery path: the connector opens its control REQs immediately in case the relay serves unauthenticated reads, a closed relay answers `auth-required:` plus an `AUTH` challenge, and the auth branch re-opens everything. That case is logged at DEBUG and consumes neither the permanent-refusal branch nor the retry budget (a chat subscription is still dropped from `_chat_subscriptions`, since it genuinely is not subscribed and discovery is what re-opens it). Treating it as permanent produced an operator-facing warning claiming discovery/membership tracking was DOWN in the same run where discovery then completed, every channel was subscribed, and a brand-new channel's kind-44100 was picked up one second later. The boundary is the per-socket `_auth_completed` flag, set once the signed AUTH event has been sent and cleared on session entry, session exit, and `stop()`; the same reason *after* that stays loud, and says the subscription is down until the relay's next AUTH challenge or the next reconnect rather than borrowing the non-auth wording. Then `_is_transient_close` decides whether to retry at all: NIP-01/NIP-42 `auth-required:`/`restricted:`/`blocked:`/`mute:`/`invalid:`/`pow:` prefixes and buzz-relay's own removal/revocation prose are permanent (do not fight the relay over a channel that is no longer ours; a post-auth `auth-required:` is recovered by the AUTH branch, not by re-issuing), `rate-limited:`/`error:`/no-reason-at-all and anything unrecognized are transient — the default resolves toward "keep listening" because going silently deaf is the failure this exists to remove, and the attempt budget bounds a wrong guess. A chat `CLOSED` is only ever recovered for a channel already in `_chat_subscriptions`: a `CLOSED` is relay-supplied, so acting on an unknown one would let a relay induce a subscription just by naming a channel. Every subscription that goes unlistened is logged at WARNING, never INFO. Subscription ids are deterministic per channel precisely so one can be replaced or closed without disturbing the others on the socket, and `_chat_subscriptions` is per-socket state cleared on session end, on re-auth (a pre-auth REQ may have been rejected), and by `stop()`. Three bounds on remote-fed state: `MAX_CACHED_CHANNELS` (512) caps the kind-39000 metadata cache, the watermark map, and the resubscribe-attempt map; `MAX_CHANNEL_SUBSCRIPTIONS` (256, well under buzz-relay's own 1024-per-connection ceiling) caps live chat subscriptions — at the cap new channels are refused and named in a warning rather than evicting a working subscription. **Known bound (documented, not fixed):** the relay caps historical delivery at 2000 events per subscription, newest-first, even with a `since`, so >2000 unread messages in a *single* channel across a disconnect loses the oldest — the relay never sends them and the watermark advances past them. That is the one remaining path that can skip; everything else fails toward replay. **Trust model** (operator-facing version in [IM_CHANNEL_CONNECTIONS.md](../../docs/IM_CHANNEL_CONNECTIONS.md#buzz-trust-model)): every inbound `EVENT` is authenticated at the single `handle_relay_frame` choke point — the NIP-01 id is recomputed from the delivered payload and the BIP-340 Schnorr signature verified against the claimed `pubkey` (`buzz_nostr.verify_event`, pure and total: malformed input returns `False`, never raises) — so `ev["pubkey"]`, the authorization principal for both the allowlist and the `/connect` bind, cannot be forged by a relay the DeerFlow operator does not run. What remains trusted is the *authorship* of kind-39000 channel metadata: any member can sign one, and because per-channel subscriptions are now driven by discovery, a forged kind-39000 has two effects rather than one — it can mark a channel `type: "dm"` (relaxing `require_mention` for that channel) **and** it can induce a chat subscription for a channel of the forger's choosing, since the channels we listen to are exactly the channels we hold metadata for. Neither makes anything be *acted on*: `allowed_users` and per-event signature verification are independent gates, so an induced subscription only means the relay reads its own traffic back to a subscriber that drops it, bounded by `MAX_CHANNEL_SUBSCRIPTIONS` (which refuses rather than evicts, so it cannot displace a real channel). Same for a forged kind-44100, except its `p` tag is re-checked locally so it must at least name us. Closing this needs a configured trusted relay pubkey, which `relay_url` is not. `allowed_users` is deny-by-default (empty = nobody, unlike siblings' empty = everyone), so `start()` logs a WARNING when it is empty and each drop logs at DEBUG. The resubscribe cursor (`since`) is **per channel**, advances only for events that were actually processed, and never past `now + MAX_FUTURE_SKEW_SECONDS`, because it is peer-supplied (`created_at`) and a single future-dated event otherwise made the connector permanently deaf. Per channel rather than global is the safety-critical half: subscriptions are per channel, so one shared cursor is the newest event seen in *any* channel and a busy channel would drag it past a quiet channel's unread messages, skipping them on the next reconnect — measured on a live relay, three channels of one identity sat ~28h apart. Per-channel cursors can only ever cost duplicate delivery (absorbed by the manager's `event_id` dedupe), and an evicted cursor degrades to "no `since`", i.e. the relay's default backlog — both fail toward replay, never toward a miss. Streaming tracks every oversize chunk index (`_stream_targets` for chunk 0, `_stream_tails` for the rest), since the manager republishes cumulative text and reposting `chunks[1:]` per update flooded the channel; all of it is per-connection state cleared by `stop()`, and the remote-fed kind-39000 cache is capped at `MAX_CACHED_CHANNELS`. **`send()` refuses outright to publish text carrying a hidden model-context wrapper** (``, ``, `` — `_HIDDEN_CONTEXT_MARKERS`), logging at ERROR and clearing the stream bookkeeping on a blocked `is_final`. This is defense in depth behind the manager's allowlist, and it lives here rather than in a sibling connector because on Buzz a leak is permanent: every streaming update is an immutable public Nostr event, so a corrective edit only changes what clients render while the original leaked event stays on the relay. Matching is on the literal opening tag, so a reply that merely talks about memory is still published +- `buzz.py` - Buzz (Nostr relay) implementation: one NIP-42-authenticated websocket, pubkey-allowlist + mention/DM/thread-follow gating (thread-follow reads via the manager's `connection_identity.lookup_thread_id`, so bound authors' repo-only threads count), streaming replies via in-place kind-40003 edits; requires the `buzz` dependency extra. Its durable seen-event replay guard uses async `aseen()` / `arecord()` / `aflush()` boundaries: initial JSON reads and coalesced atomic writes run off the Gateway event loop, and `stop()` awaits the final flush before returning. **Subscription model** (operator-facing version in [IM_CHANNEL_CONNECTIONS.md](../../docs/IM_CHANNEL_CONNECTIONS.md#buzz-subscription-model)): the relay fans kind-9 chat events out **only** to channel-scoped subscriptions, proved on a live relay: `REQ {"kinds":[9]}` is accepted and answered with `EOSE` but never receives an event, `REQ {"kinds":[9],"#h":[uuid]}` works, and a multi-value `#h` matches nothing, so it is strictly one REQ per channel. Every connection therefore rebuilds three kinds of subscription after NIP-42 auth: `buzz-discovery` (`{"kinds":[39000]}`, a historical query returning exactly the channels this identity belongs to, one stored event each then `EOSE` — adding `#p` returns zero, do not "narrow" it), `buzz-membership` (`{"kinds":[44100,44101],"#p":[us],"since":}`, the relay-signed member-added/member-removed notifications whose `p` tag names the affected member and `h` tag the channel), and one `buzz-chat-` per discovered channel. Chat subscriptions open as each kind-39000 arrives; the discovery `EOSE` is the completeness barrier that retries any that failed and warns when discovery found nothing. A kind-44100 for our pubkey subscribes to the new channel **live** (then re-issues discovery so its name/type reach the DM-detection cache, but only when that channel's metadata is actually missing — an unconditional refresh is how a burst of 44100s multiplied into one discovery pass each); a kind-44101 issues `buzz_nostr.close_frame` for exactly that channel's subscription and drops its metadata. **The membership subscription is scoped to LIVE events** and this is load-bearing: buzz-relay *stores* 44100/44101 and serves history newest-first (default limit 2000), so an unscoped filter replayed the whole membership history on every connect — every stored add read as live, re-running discovery once each (M+1 discovery passes × N stored kind-39000 events per connect, observed live as two `channel discovery complete` lines and one channel logged `` because the 44100 path subscribed before its metadata arrived), re-subscribing channels we have since been removed from, and letting a stored 44101 transiently unsubscribe a channel we are still in. `since` is anchored at the moment the socket opened (`_session_started_at`) minus `MEMBERSHIP_LOOKBACK_SECONDS` (60) of slack, which covers both relay clock skew and a membership change published *during* the connect/auth handshake; the slack can only cost an idempotent replay of the last minute. **A relay `CLOSED` frame is recovered** — every subscription on the socket fails silently when dropped, so `_handle_closed` re-issues it, bounded by `MAX_RESUBSCRIBE_ATTEMPTS` (3) per subscription id per connection *and per auth epoch* (`_resubscribe_attempts` is reset on session start, on re-auth, and by `stop()`, so pre-auth rejections — which the auth branch already recovers wholesale — never spend the authenticated session's budget). The first retry is immediate (a one-off hiccup is the common case); later ones back off 1s then 2s, awaited inline in the read loop, not in a background task that could outlive its socket. **`auth-required:` before this socket has completed its NIP-42 handshake is the expected bootstrap sequence, not a refusal**, and `_handle_closed` short-circuits it ahead of every recovery path: the connector opens its control REQs immediately in case the relay serves unauthenticated reads, a closed relay answers `auth-required:` plus an `AUTH` challenge, and the auth branch re-opens everything. That case is logged at DEBUG and consumes neither the permanent-refusal branch nor the retry budget (a chat subscription is still dropped from `_chat_subscriptions`, since it genuinely is not subscribed and discovery is what re-opens it). Treating it as permanent produced a false operator-facing warning that discovery/membership tracking was DOWN while discovery then completed normally. The boundary is the per-socket `_auth_completed` flag, set once the signed AUTH event has been sent and cleared on session entry, session exit, and `stop()`; the same reason *after* that stays loud, and says the subscription is down until the relay's next AUTH challenge or the next reconnect rather than borrowing the non-auth wording. Then `_is_transient_close` decides whether to retry at all: NIP-01/NIP-42 `auth-required:`/`restricted:`/`blocked:`/`mute:`/`invalid:`/`pow:` prefixes and buzz-relay's own removal/revocation prose are permanent (do not fight the relay over a channel that is no longer ours), `rate-limited:`/`error:`/no-reason-at-all and anything unrecognized are transient — the default resolves toward "keep listening" because going silently deaf is the failure this exists to remove, and the attempt budget bounds a wrong guess. A chat `CLOSED` is only ever recovered for a channel already in `_chat_subscriptions`: a `CLOSED` is relay-supplied, so acting on an unknown one would let a relay induce a subscription just by naming a channel. Every subscription that goes unlistened is logged at WARNING, never INFO. Subscription ids are deterministic per channel so one can be replaced or closed without disturbing the others, and `_chat_subscriptions` is per-socket state cleared on session end, on re-auth (a pre-auth REQ may have been rejected), and by `stop()`. Three bounds on remote-fed state: `MAX_CACHED_CHANNELS` (512) caps the kind-39000 metadata cache, the watermark map, and the resubscribe-attempt map; `MAX_CHANNEL_SUBSCRIPTIONS` (256, well under buzz-relay's own 1024-per-connection ceiling) caps live chat subscriptions — at the cap new channels are refused and named in a warning rather than evicting a working subscription. **Known bound:** the relay caps historical delivery at 2000 events per subscription, newest-first, even with a `since`, so >2000 unread messages in a *single* channel across a disconnect loses the oldest — the relay never sends them and the watermark advances past them. That is the one remaining path that can skip; everything else fails toward replay. **Trust model** (operator-facing version in [IM_CHANNEL_CONNECTIONS.md](../../docs/IM_CHANNEL_CONNECTIONS.md#buzz-trust-model)): every inbound `EVENT` is authenticated at the single `handle_relay_frame` choke point — the NIP-01 id is recomputed from the delivered payload and the BIP-340 Schnorr signature verified against the claimed `pubkey` (`buzz_nostr.verify_event`, pure and total: malformed input returns `False`, never raises) — so `ev["pubkey"]`, the authorization principal for both the allowlist and the `/connect` bind, cannot be forged by a relay the DeerFlow operator does not run. What remains trusted is the *authorship* of kind-39000 channel metadata: any member can sign one, and because per-channel subscriptions are now driven by discovery, a forged kind-39000 has two effects rather than one — it can mark a channel `type: "dm"` (relaxing `require_mention` for that channel) **and** it can induce a chat subscription for a channel of the forger's choosing, since the channels we listen to are exactly the channels we hold metadata for. Neither makes anything be *acted on*: `allowed_users` and per-event signature verification are independent gates, so an induced subscription only means the relay reads its own traffic back to a subscriber that drops it, bounded by `MAX_CHANNEL_SUBSCRIPTIONS` (which refuses rather than evicts, so it cannot displace a real channel). Same for a forged kind-44100, except its `p` tag is re-checked locally so it must at least name us. Closing this needs a configured trusted relay pubkey, which `relay_url` is not. `allowed_users` is deny-by-default (empty = nobody, unlike siblings' empty = everyone), so `start()` logs a WARNING when it is empty and each drop logs at DEBUG. The resubscribe cursor (`since`) is **per channel**, advances only for events that were actually processed, and never past `now + MAX_FUTURE_SKEW_SECONDS`, because it is peer-supplied (`created_at`) and a single future-dated event otherwise made the connector permanently deaf. Per channel rather than global is the safety-critical half: one shared cursor is the newest event seen in *any* channel, so a busy channel would drag it past a quiet channel's unread messages, skipping them on the next reconnect. Per-channel cursors can only ever cost duplicate delivery (absorbed by the manager's `event_id` dedupe), and an evicted cursor degrades to "no `since`", i.e. the relay's default backlog — both fail toward replay, never toward a miss. Streaming tracks every oversize chunk index (`_stream_targets` for chunk 0, `_stream_tails` for the rest), since the manager republishes cumulative text and reposting `chunks[1:]` per update flooded the channel; all of it is per-connection state cleared by `stop()`, and the remote-fed kind-39000 cache is capped at `MAX_CACHED_CHANNELS`. **`send()` refuses outright to publish text carrying a hidden model-context wrapper** (``, ``, `` — `_HIDDEN_CONTEXT_MARKERS`), logging at ERROR and clearing the stream bookkeeping on a blocked `is_final`. This is defense in depth behind the manager's allowlist, and it lives here rather than in a sibling connector because on Buzz a leak is permanent: every streaming update is an immutable public Nostr event, so a corrective edit only changes what clients render while the original leaked event stays on the relay. Matching is on the literal opening tag, so a reply that merely talks about memory is still published Buzz seen-event shutdown stays bounded and retryable: `aflush()` awaits any in-flight write, attempts at most one final snapshot, leaves a still-changing generation dirty for fail-open replay, and returns without a live persistence timer. A Gateway cancellation does not cancel the underlying worker-thread write; `BuzzChannel.stop()` tracks cleanup completion separately from transport admission so ChannelService can retry the retained channel without racing a second snapshot against that write. The store is quiesced before stop (including the already-stopped guard), so a timed-out relay task that records after stop only marks data dirty and cannot schedule detached file work; a repeated `stop()` still drains that dirty state, while `BuzzChannel.start()` explicitly resumes scheduling and flushes it automatically. - `github.py` - Webhook-driven GitHub channel. Inbound messages come from `POST /api/webhooks/github`; outbound is log-only because GitHub agents post explicitly with `gh` from their sandbox when they choose to comment or create a PR - `app/gateway/routers/channel_connections.py` - Browser-facing user connection and disconnect APIs @@ -81,7 +81,7 @@ The cached value is reused for both the blocking (`runs.wait`) and streaming (`_ - Custom agents declare a `github:` block in their `config.yaml` to bind to repos and event triggers; the webhook route is fail-closed by default (mounted only when `GITHUB_WEBHOOK_SECRET` is set) and exempt from auth/CSRF because authenticity is enforced by HMAC. - Registry caching is keyed by the configured agent store's opaque signature: file storage uses agent config mtimes, while database storage hashes the ordered owner/name/config/soul contents so same-timestamp writes still invalidate webhook routing. - Outbound is **log-only** by design: each agent posts its own reply mid-run via the `gh` CLI from its sandbox, so the manager uses `fire_and_forget=True` and `runs.create()` returns once pending. -- **Follow-up buffering while busy** (issue #4121): because outbound is log-only, the pre-existing `THREAD_BUSY_MESSAGE` reply on a `ConflictError` was invisible to the commenter — a comment posted while a run was already active looked like it had been silently ignored. When `ChannelRunPolicy.buffer_followups_on_busy=True` (GitHub's default), a `ConflictError` on `runs.create()` now also appends the triggering message to a per-thread, in-memory buffer (`ChannelManager._followup_buffers`) — deduped by GitHub delivery id, capped at `FOLLOWUP_BUFFER_MAX_PER_THREAD` (20, oldest dropped with a WARNING log on overflow). The first successful `runs.create()` on a thread now captures its `run_id` and spawns a background watcher that subscribes to that run's `StreamBridge` stream; once it observes `END_SENTINEL`, the watcher drains up to `FOLLOWUP_DRAIN_BATCH_SIZE` (10) buffered entries into one ``-wrapped input and fires a follow-up `runs.create()` — itself watched the same way, so a backlog deeper than one batch chains into further drain cycles instead of growing one unbounded input. If that follow-up `runs.create()` itself hits `ConflictError` (e.g. a manual Web UI turn or a scheduled run raced onto the same thread), the batch is requeued rather than lost, and is retried whenever this manager next successfully creates and watches a run on that thread. Reactions/acknowledgment (e.g. GitHub's `eyes`/`confused` reaction API) on buffered comments are intentionally **out of scope** for this mechanism and left to a follow-up — comments are coalesced silently. **Plumbing**: the watcher needs the Gateway's `StreamBridge` singleton, which `ChannelManager` did not previously have access to; it is threaded from `app.py`'s lifespan (where `app.state.stream_bridge` is already set by `langgraph_runtime`) through `start_channel_service(get_stream_bridge=...)` → `ChannelService.__init__` → `ChannelManager.__init__`, as a zero-arg closure mirroring the existing `launch_run=lambda **kwargs: launch_scheduled_thread_run(app=app, **kwargs)` pattern used for `ScheduledTaskService` in the same lifespan function. A `ChannelManager` constructed without it (e.g. directly in a test) still buffers safely — it just has no watcher to auto-drain. **Scope limitation**: the buffer and watcher state are per-process, in-memory. Under `GATEWAY_WORKERS>1` or multi-pod, a follow-up comment routed to a different worker process than the one running the busy thread's agent will not see that buffer. This is a known, deliberately deferred limitation with the same shape as the cross-pod gap described for issue #4120 (a shared buffer store or IM-leader election would be needed to close it) — single-process/single-pod deployments, the safe default, see no correctness issue from this, only the documented per-process scope. +- **Follow-up buffering while busy** (issue #4121): outbound is log-only, so the `THREAD_BUSY_MESSAGE` reply on a `ConflictError` was invisible to the commenter — a comment posted during an active run looked silently ignored. When `ChannelRunPolicy.buffer_followups_on_busy=True` (GitHub's default), a `ConflictError` on `runs.create()` also appends the triggering message to a per-thread in-memory buffer (`ChannelManager._followup_buffers`) — deduped by GitHub delivery id, capped at `FOLLOWUP_BUFFER_MAX_PER_THREAD` (20, oldest dropped with a WARNING). The first successful `runs.create()` on a thread captures its `run_id` and spawns a watcher on that run's `StreamBridge` stream; on `END_SENTINEL` it drains up to `FOLLOWUP_DRAIN_BATCH_SIZE` (10) entries into one ``-wrapped input and fires a follow-up `runs.create()` — itself watched, so a deeper backlog chains into further drain cycles instead of one unbounded input. If that follow-up hits `ConflictError` (a concurrent turn raced onto the thread), the batch is requeued and retried when this manager next creates and watches a run on that thread. Reactions/acknowledgment on buffered comments are intentionally **out of scope** — comments coalesce silently. **Plumbing**: the watcher needs the Gateway's `StreamBridge` singleton, threaded from `app.py`'s lifespan (`app.state.stream_bridge`, set by `langgraph_runtime`) through `start_channel_service(get_stream_bridge=...)` → `ChannelService.__init__` → `ChannelManager.__init__` as a zero-arg closure, mirroring `ScheduledTaskService`'s `launch_run` closure. A `ChannelManager` constructed without it (tests) still buffers safely, with no watcher to auto-drain. **Scope limitation**: buffer and watcher state are per-process; under `GATEWAY_WORKERS>1` or multi-pod a follow-up routed to another worker does not see the buffer — a known, deferred limitation (same shape as #4120; needs a shared buffer store or IM-leader election); single-process deployments are unaffected. - See [backend/docs/GITHUB_AGENTS.md](../../docs/GITHUB_AGENTS.md) for the architecture diagrams: webhook → fan-out → `InboundMessage` dispatch, `preferred_thread_id = UUID5(repo, number, agent_name)` thread determinism, mention-handle precedence chain, GH token lifecycle via `GH_TOKEN`/`GITHUB_TOKEN` per-call `extra_env`, and the narrow `ConflictError` (HTTP 409) thread-create race recovery. diff --git a/backend/app/channels/connection_identity.py b/backend/app/channels/connection_identity.py index 3acd291c72..5b0b8a018b 100644 --- a/backend/app/channels/connection_identity.py +++ b/backend/app/channels/connection_identity.py @@ -49,12 +49,13 @@ async def lookup_thread_id(msg: InboundMessage, *, repo: Any, store: Any) -> str A message carrying a resolved connection is mapped in the connection repository and nowhere else (``ChannelManager._store_thread_id`` writes only there); every - other message is mapped in the JSON ``ChannelStore``. Adapters that gate on "is - this thread already ours?" must use this rather than reading the store directly, - or bound users' threads look unengaged. + other message is mapped in the global ``ChannelStore`` (the shared + ``channel_thread_bindings`` table, or the legacy JSON file on a memory database). + Adapters that gate on "is this thread already ours?" must use this rather than + reading the store directly, or bound users' threads look unengaged. """ if msg.connection_id and repo is not None: return await repo.get_thread_id(msg.connection_id, msg.chat_id, msg.topic_id) if store is None: return None - return store.get_thread_id(msg.channel_name, msg.chat_id, topic_id=msg.topic_id) + return await store.get_thread_id(msg.channel_name, msg.chat_id, topic_id=msg.topic_id) diff --git a/backend/app/channels/discord.py b/backend/app/channels/discord.py index 4aeeffa602..5b800ef120 100644 --- a/backend/app/channels/discord.py +++ b/backend/app/channels/discord.py @@ -113,8 +113,8 @@ class DiscordChannel(Channel): self._allowed_channels: frozenset[str] = _parse_allowed_channels(config.get("allowed_channels")) # Session tracking: channel_id -> Discord thread_id (in-memory, persisted to JSON). - # Uses a dedicated JSON file separate from ChannelStore, which maps IM - # conversations to DeerFlow thread IDs — a different concern. + # Uses a dedicated JSON file next to the channel store's state, separate from + # the ChannelStore itself, which maps IM conversations to DeerFlow thread IDs. self._active_threads: dict[str, str] = {} # Reverse-lookup set for O(1) thread ID checks (avoids O(n) scan of _active_threads.values()). self._active_thread_ids: set[str] = set() @@ -127,9 +127,9 @@ class DiscordChannel(Channel): # error) — ChannelService then stops the instance, and an ungated flush # would overwrite the persisted mappings with an empty snapshot (#2897). self._thread_store_loaded = False - store = config.get("channel_store") - if store is not None: - self._thread_store_path = store._path.parent / "discord_threads.json" + channels_dir = getattr(config.get("channel_store"), "channels_dir", None) + if channels_dir is not None: + self._thread_store_path = Path(channels_dir) / "discord_threads.json" else: self._thread_store_path = Path.home() / ".deer-flow" / "channels" / "discord_threads.json" diff --git a/backend/app/channels/feishu.py b/backend/app/channels/feishu.py index ea6f155069..e677c46387 100644 --- a/backend/app/channels/feishu.py +++ b/backend/app/channels/feishu.py @@ -9,7 +9,12 @@ import re import stat import threading import time -from typing import Any, Literal +from collections import deque +from collections.abc import Callable, Coroutine +from concurrent.futures import CancelledError as FutureCancelledError +from concurrent.futures import Future +from dataclasses import dataclass, field +from typing import Any, Literal, TypeVar from app.channels.base import Channel, ChannelStopTimeout from app.channels.commands import is_known_channel_command, strip_leading_mentions @@ -37,6 +42,91 @@ from deerflow.uploads.manager import ( logger = logging.getLogger(__name__) PENDING_CLARIFICATION_TTL_SECONDS = 30 * 60 +# Channel-store deadlines. ``_on_message`` runs on lark-oapi's private event loop, +# which also carries the SDK's websocket reads, pongs and its 120s ping loop: +# every second the bridge blocks that loop is a second the connection cannot +# service keepalives, so the bound is a few seconds (one indexed SELECT per +# candidate), not the database's 30s command timeout. The deadline lives on the +# Gateway-loop side (``asyncio.wait_for`` around the store calls, which actually +# cancels them); the bridge wait only adds slack for a stalled Gateway loop. +FEISHU_STORE_LOOKUP_TIMEOUT_SECONDS = 3.0 +FEISHU_STORE_BRIDGE_TIMEOUT_SECONDS = FEISHU_STORE_LOOKUP_TIMEOUT_SECONDS + 1.0 +# A lookup that missed its deadline (or failed) is *unresolved*, never a miss: +# the message is deferred to the Gateway loop, which retries with backoff up to +# the database's own command budget and then drops it rather than opening a new +# conversation for a reply whose mapping the database could still have served. +# The budget runs from the message's arrival, not from when it reaches its +# chat's queue head, and every deferred message holds a bus intake reservation +# taken before routing, so the deferred backlog is bounded by +# ``inbound_queue_maxsize`` exactly like live messages (a message that cannot be +# admitted is dropped with a warning, never queued). +FEISHU_STORE_RESOLVE_TOTAL_BUDGET_SECONDS = 30.0 +FEISHU_STORE_RESOLVE_RETRY_BACKOFF_SECONDS = 1.0 +FEISHU_STORE_RESOLVE_RETRY_MAX_BACKOFF_SECONDS = 8.0 +_T = TypeVar("_T") + + +class StoreLookupUnresolved(Exception): + """A channel-store call produced no answer: deadline, store error, no Gateway loop, or channel stop. + + Distinct from a *miss* (the store answered "no mapping"): an unresolved lookup + must never route a message as if the mapping did not exist. + """ + + def __init__(self, reason: str, *, future: Future[Any] | None = None) -> None: + super().__init__(reason) + self.reason = reason + # The still-running bridge future when the lark-thread wait gave up before + # the coroutine's own deadline; the deferred path awaits its outcome first. + self.future = future + + +@dataclass(slots=True) +class _ParsedMessage: + """The content of one lark message event, independent of how it is routed.""" + + chat_id: str + msg_id: str + sender_id: str + root_id: str | None + parent_id: str | None + feishu_thread_id: str | None + chat_type: str | None + text: str + files: list[dict[str, Any]] + msg_type: InboundMessageType = InboundMessageType.CHAT + + +@dataclass(frozen=True, slots=True) +class _Routing: + """Where a parsed message goes: resolved (stored mapping), plain miss, or pending clarification.""" + + topic_id: str | None + from_stored_mapping: bool + from_pending: bool + + +@dataclass(slots=True) +class _RoutingAttempt: + """Retry state for one message: once a pending clarification is consumed the route is decided. + + ``reservation`` is the bus intake slot taken before routing; it travels with + the message into dispatch (committed there) or is released on every drop. + ``arrived_at`` anchors the per-message retry budget. + """ + + parsed: _ParsedMessage + reservation: InboundReservation | None = None + arrived_at: float = field(default_factory=time.monotonic) + pending: dict[str, Any] | None = None + attempted: bool = False + + def release_reservation(self) -> None: + if self.reservation is not None: + self.reservation.release() + self.reservation = None + + FEISHU_INBOUND_BATCH_WINDOW_SECONDS = 0.75 FEISHU_MAX_INBOUND_FILE_BYTES = 20_000_000 SOURCE_PREVIEW_METADATA_KEY = "feishu_source_preview" @@ -85,6 +175,10 @@ class FeishuChannel(Channel): self._CreateImageRequestBody = None self._GetMessageResourceRequest = None self._thread_lock = threading.Lock() + # Per-chat FIFO of messages whose routing is deferred to the Gateway loop + # (unresolved store lookups); guarded by ``_thread_lock``. A chat with a + # queue has one drain task owning that deque. + self._deferred_routing: dict[str, deque[_RoutingAttempt]] = {} @staticmethod def _non_empty_str(value: Any) -> str | None: @@ -712,10 +806,10 @@ class FeishuChannel(Channel): running_card_id, ) fallback_card_id = await self._reply_card(source_message_id, card_text) - self._remember_thread_mapping(msg, source_message_id, fallback_card_id) + await self._remember_thread_mapping(msg, source_message_id, fallback_card_id) self._remember_pending_clarification(msg, fallback_card_id) else: - self._remember_thread_mapping(msg, source_message_id, running_card_id) + await self._remember_thread_mapping(msg, source_message_id, running_card_id) self._remember_pending_clarification(msg, running_card_id) logger.info("[Feishu] running card updated: source=%s card=%s", source_message_id, running_card_id) elif msg.is_final: @@ -723,7 +817,7 @@ class FeishuChannel(Channel): source_message_id, self._compose_card_text(msg.text, msg.metadata), ) - self._remember_thread_mapping(msg, source_message_id, final_card_id) + await self._remember_thread_mapping(msg, source_message_id, final_card_id) self._remember_pending_clarification(msg, final_card_id) elif awaited_running_card_task: logger.warning( @@ -736,7 +830,7 @@ class FeishuChannel(Channel): msg.text, metadata=msg.metadata, ) - self._remember_thread_mapping(msg, source_message_id, created_card_id) + await self._remember_thread_mapping(msg, source_message_id, created_card_id) if msg.is_final: self._running_card_ids.pop(source_message_id, None) @@ -747,7 +841,7 @@ class FeishuChannel(Channel): # -- internal ---------------------------------------------------------- - def _remember_thread_mapping(self, msg: OutboundMessage, *topic_ids: str | None) -> None: + async def _remember_thread_mapping(self, msg: OutboundMessage, *topic_ids: str | None) -> None: store = self.config.get("channel_store") if store is None or not msg.thread_id: return @@ -771,7 +865,7 @@ class FeishuChannel(Channel): continue seen.add(topic_id) try: - store.set_thread_id( + await store.set_thread_id( self.name, msg.chat_id, msg.thread_id, @@ -781,6 +875,58 @@ class FeishuChannel(Channel): except Exception: logger.exception("[Feishu] failed to remember thread mapping for topic_id=%s", topic_id) + def _run_store_call(self, make_coroutine: Callable[[], Coroutine[Any, Any, _T]], *, name: str, msg_id: Any) -> _T: + """Run an async ``channel_store`` call from the synchronous lark callback. + + The store is async (it may be the shared database table). ``_on_message`` + runs on lark-oapi's thread, so the call is submitted to the Gateway loop + (``_main_loop``) through the tracked threadsafe-future helper (``stop()`` + drains or cancels it) and awaited here for at most + ``FEISHU_STORE_BRIDGE_TIMEOUT_SECONDS``. + + That callback does run inside an event loop: ``_run_ws`` gives the SDK + thread its own loop and lark-oapi 1.5.5 calls ``_on_message`` + synchronously from its async frame handler, so ``get_running_loop()`` + succeeds here on every real message. Blocking that loop briefly is fine — + it is the SDK's private loop in its own thread, the one that used to block + on the JSON file rewrite the store did inline — because the coroutine runs + on the Gateway loop, not on it. The only wait that must be refused is one + issued from the Gateway loop itself (``get_running_loop() is _main_loop``): + it would block the loop that has to execute the submitted coroutine. + + Every way of *not* getting an answer — no running Gateway loop, called on + the Gateway loop, channel stopping, bridge wait exhausted, cancelled by + stop, or the coroutine's own ``StoreLookupUnresolved`` — raises + ``StoreLookupUnresolved`` so the caller can defer the message; a timed-out + wait is never reported as "no mapping". Other exceptions the coroutine + raised propagate unchanged. + """ + loop = self._main_loop + if loop is None or not loop.is_running(): + logger.warning("[Feishu] main loop not running, cannot %s", name) + raise StoreLookupUnresolved("main loop not running") + try: + current = asyncio.get_running_loop() + except RuntimeError: + current = None + if current is loop: + logger.warning("[Feishu] %s requested on the Gateway loop itself; a blocking wait here would deadlock it, skipping the channel store call", name) + raise StoreLookupUnresolved("called on the Gateway loop") + future = self._submit_threadsafe_coroutine_future(make_coroutine(), loop, name=name, msg_id=msg_id) + if future is None: + logger.info("[Feishu] channel stopping, skipped %s", name) + raise StoreLookupUnresolved("channel stopping") + try: + return future.result(timeout=FEISHU_STORE_BRIDGE_TIMEOUT_SECONDS) + except TimeoutError: + # Not cancelled: the coroutine carries its own deadline and the + # deferred path awaits its outcome before retrying. + logger.warning("[Feishu] %s did not answer within %.0fs; deferring message %s", name, FEISHU_STORE_BRIDGE_TIMEOUT_SECONDS, msg_id) + raise StoreLookupUnresolved("bridge wait exhausted", future=future) from None + except FutureCancelledError: + logger.info("[Feishu] %s cancelled by channel stop", name) + raise StoreLookupUnresolved("cancelled by channel stop") from None + def _remember_pending_clarification(self, msg: OutboundMessage, card_message_id: str | None) -> None: if not msg.is_final or msg.metadata.get(PENDING_CLARIFICATION_METADATA_KEY) is not True: return @@ -834,41 +980,239 @@ class FeishuChannel(Channel): self._pending_clarifications.pop(key, None) return None - def _ensure_pending_thread_mapping(self, chat_id: str, user_id: str, pending: dict[str, Any]) -> None: + def _route_without_store(self, parsed: _ParsedMessage) -> _Routing: + """Routing for a channel constructed without a ``channel_store`` (no lookups possible).""" + topic_id: str | None = parsed.root_id or parsed.msg_id + if parsed.chat_type == "p2p": + topic_id = None + if parsed.msg_type != InboundMessageType.CHAT: + return _Routing(topic_id, False, False) + pending = self._consume_pending_clarification(parsed.chat_id, parsed.sender_id) + pending_topic_id = self._non_empty_str(pending.get("topic_id")) if pending else None + if not pending_topic_id: + return _Routing(topic_id, False, False) + return _Routing(pending_topic_id, False, True) + + async def _resolve_routing(self, attempt: _RoutingAttempt) -> _Routing: + """Decide a message's topic on the Gateway loop, consulting and updating the channel store. + + Stored mappings for the root / parent / thread ids win (``_first_stored_topic``); + otherwise the route is the root (or the message itself; ``None`` in P2P chats), + unless a pending clarification for the sender names a topic — that topic's + mapping is then persisted so the manager finds the clarified thread. The + pending clarification is consumed once: a retried attempt keeps the decision + and only redoes the write. + """ store = self.config.get("channel_store") - topic_id = self._non_empty_str(pending.get("topic_id")) - thread_id = self._non_empty_str(pending.get("thread_id")) - if store is None or not topic_id or not thread_id: - return + parsed = attempt.parsed + if attempt.pending is None: + candidates = [candidate for candidate in (self._non_empty_str(parsed.root_id), parsed.parent_id, parsed.feishu_thread_id) if candidate] + if candidates: + stored = await self._first_stored_topic(store, parsed.chat_id, candidates) + if stored: + return _Routing(stored, True, False) + topic_id: str | None = parsed.root_id or parsed.msg_id + if parsed.chat_type == "p2p": + topic_id = None + if parsed.msg_type != InboundMessageType.CHAT: + return _Routing(topic_id, False, False) + pending = self._consume_pending_clarification(parsed.chat_id, parsed.sender_id) + if not (pending and self._non_empty_str(pending.get("topic_id"))): + return _Routing(topic_id, False, False) + attempt.pending = pending + pending_topic_id = self._non_empty_str(attempt.pending.get("topic_id")) + thread_id = self._non_empty_str(attempt.pending.get("thread_id")) + if thread_id: + await store.set_thread_id(self.name, parsed.chat_id, thread_id, topic_id=pending_topic_id, user_id=parsed.sender_id) + return _Routing(pending_topic_id, False, True) + + async def _resolve_routing_bounded(self, attempt: _RoutingAttempt) -> _Routing | StoreLookupUnresolved: + """``_resolve_routing`` under the store deadline; no answer in time (or a store error) is *unresolved*. + + The unresolved outcome is *returned*, not raised: this coroutine runs as a + tracked submission whose failure the base class reports at ERROR, and an + unanswered lookup during a store outage is an expected, handled outcome. + """ + attempt.attempted = True try: - store.set_thread_id(self.name, chat_id, thread_id, topic_id=topic_id, user_id=user_id) - except Exception: - logger.exception("[Feishu] failed to restore pending clarification mapping for topic_id=%s", topic_id) + return await asyncio.wait_for(self._resolve_routing(attempt), FEISHU_STORE_LOOKUP_TIMEOUT_SECONDS) + except TimeoutError: + return StoreLookupUnresolved(f"channel store did not answer within {FEISHU_STORE_LOOKUP_TIMEOUT_SECONDS:g}s") + except Exception as exc: + # A failing store is not "no mapping": routing on it would open a new + # conversation for a reply whose thread the database still knows. + logger.warning("[Feishu] channel store call failed for message %s (%s); treating the lookup as unresolved", attempt.parsed.msg_id, exc.__class__.__name__, exc_info=True) + return StoreLookupUnresolved(f"channel store error: {exc.__class__.__name__}") - def _resolve_topic_id( - self, - chat_id: str, - msg_id: str, - *, - root_id: str | None, - parent_id: str | None, - thread_id: str | None, - ) -> tuple[str, bool]: - store = self.config.get("channel_store") - candidates = [root_id, parent_id, thread_id] + async def _first_stored_topic(self, store: Any, chat_id: str, candidates: list[str]) -> str | None: + """The first candidate topic id the store already maps to a thread, in priority order.""" + for candidate in candidates: + if await store.get_thread_id(self.name, chat_id, topic_id=candidate): + return candidate + return None - if store is not None: - for candidate in candidates: - candidate = self._non_empty_str(candidate) - if not candidate: - continue + def _defer_routing(self, attempt: _RoutingAttempt, unresolved: StoreLookupUnresolved) -> None: + """Hand an unresolved message to the Gateway loop (lark thread; never blocks). + + The chat gets a FIFO deque and one drain task; later messages for the same + chat queue behind it in ``_on_message`` so none overtakes a deferred + predecessor onto a different thread. Without a running Gateway loop the + message is dropped (logged) — nothing could dispatch it anyway. + """ + parsed = attempt.parsed + loop = self._main_loop + if loop is None or not loop.is_running(): + attempt.release_reservation() + logger.warning("[Feishu] dropping message %s for chat %s: %s and no Gateway loop to defer it to", parsed.msg_id, parsed.chat_id, unresolved.reason) + return + queue: deque[_RoutingAttempt] = deque([attempt]) + with self._thread_lock: + existing = self._deferred_routing.get(parsed.chat_id) + if existing is not None: + existing.append(attempt) + return + self._deferred_routing[parsed.chat_id] = queue + scheduled = self._submit_threadsafe_coroutine( + self._drain_deferred_routing(parsed.chat_id, queue, unresolved.future), + loop, + name="deferred_routing", + msg_id=parsed.msg_id, + ) + if not scheduled: + with self._thread_lock: + if self._deferred_routing.get(parsed.chat_id) is queue: + del self._deferred_routing[parsed.chat_id] + attempt.release_reservation() + logger.warning("[Feishu] dropping message %s for chat %s: %s and the channel is stopping", parsed.msg_id, parsed.chat_id, unresolved.reason) + return + logger.info("[Feishu] deferred message %s for chat %s: %s; retrying on the Gateway loop for up to %.0fs", parsed.msg_id, parsed.chat_id, unresolved.reason, FEISHU_STORE_RESOLVE_TOTAL_BUDGET_SECONDS) + + async def _drain_deferred_routing(self, chat_id: str, queue: deque[_RoutingAttempt], first_future: Future[Any] | None) -> None: + """Resolve and dispatch a chat's deferred messages in order, then retire the queue.""" + try: + while True: + with self._thread_lock: + if not queue: + if self._deferred_routing.get(chat_id) is queue: + del self._deferred_routing[chat_id] + return + attempt = queue[0] + routing: _Routing | None = None + if first_future is not None: + # The lark thread's own call may still finish (its deadline is + # the coroutine's); use that answer before spending retries. + pending_first, first_future = first_future, None + try: + outcome = await asyncio.shield(asyncio.wrap_future(pending_first)) + except StoreLookupUnresolved: + outcome = None + routing = outcome if isinstance(outcome, _Routing) else None + if routing is None: + routing = await self._retry_routing(attempt) try: - if store.get_thread_id(self.name, chat_id, topic_id=candidate): - return candidate, True - except Exception: - logger.exception("[Feishu] failed to resolve stored topic mapping for topic_id=%s", candidate) + if routing is None: + attempt.release_reservation() + logger.warning( + "[Feishu] dropping message %s for chat %s: the channel store did not answer within %gs of its arrival; not routing it as a new conversation", + attempt.parsed.msg_id, + chat_id, + FEISHU_STORE_RESOLVE_TOTAL_BUDGET_SECONDS, + ) + else: + self._dispatch_inbound(attempt.parsed, routing, reservation=attempt.reservation) + attempt.reservation = None + finally: + with self._thread_lock: + if queue and queue[0] is attempt: + queue.popleft() + finally: + with self._thread_lock: + leftovers = list(queue) + queue.clear() + if self._deferred_routing.get(chat_id) is queue: + del self._deferred_routing[chat_id] + for leftover in leftovers: + leftover.release_reservation() + if leftovers: + logger.warning("[Feishu] dropping %d deferred messages for chat %s: the channel is stopping", len(leftovers), chat_id) - return root_id or msg_id, False + async def _retry_routing(self, attempt: _RoutingAttempt) -> _Routing | None: + """Retry the bounded resolution with backoff until the message's own budget; ``None`` when it never answered. + + The budget is ``FEISHU_STORE_RESOLVE_TOTAL_BUDGET_SECONDS`` from the message's + arrival, so a message that waited behind others in its chat's queue does not + start a fresh budget at the head: one whose budget is already spent is dropped + without another lookup. + """ + deadline = attempt.arrived_at + FEISHU_STORE_RESOLVE_TOTAL_BUDGET_SECONDS + backoff = FEISHU_STORE_RESOLVE_RETRY_BACKOFF_SECONDS + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + return None + if attempt.attempted: + await asyncio.sleep(min(backoff, remaining)) + backoff = min(backoff * 2, FEISHU_STORE_RESOLVE_RETRY_MAX_BACKOFF_SECONDS) + if time.monotonic() >= deadline: + return None + outcome = await self._resolve_routing_bounded(attempt) + if isinstance(outcome, _Routing): + return outcome + logger.debug("[Feishu] retrying routing for message %s: %s", attempt.parsed.msg_id, outcome.reason) + + def _dispatch_inbound(self, parsed: _ParsedMessage, routing: _Routing, *, reservation: InboundReservation | None = None) -> None: + """Build the inbound for a routed message and hand it to batching / the Gateway loop. + + ``reservation`` is the intake slot taken before routing; it is handed to + ``_schedule_prepare_inbound`` (committed or released there) or released on the + batch path (the batch flush reserves when it publishes) and on failure. + """ + try: + source_preview = None + if self._should_include_source_preview(chat_type=parsed.chat_type, root_id=parsed.root_id, parent_id=parsed.parent_id, thread_id=parsed.feishu_thread_id): + source_preview = self._compact_source_preview(parsed.text) + + metadata = { + "message_id": parsed.msg_id, + "root_id": parsed.root_id, + "parent_id": parsed.parent_id, + "thread_id": parsed.feishu_thread_id, + "topic_id": routing.topic_id, + "user_id": parsed.sender_id, + RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY: routing.from_pending, + } + if source_preview: + metadata[SOURCE_PREVIEW_METADATA_KEY] = source_preview + + inbound = self._make_inbound( + chat_id=parsed.chat_id, + user_id=parsed.sender_id, + text=parsed.text, + msg_type=parsed.msg_type, + thread_ts=parsed.msg_id, + files=parsed.files, + metadata=metadata, + ) + inbound.topic_id = routing.topic_id + + if self._is_batchable_file_inbound( + msg_type=parsed.msg_type, + text=parsed.text, + files=parsed.files, + root_id=parsed.root_id, + parent_id=parsed.parent_id, + thread_id=parsed.feishu_thread_id, + ): + if reservation is not None: + reservation.release() + self._queue_file_inbound_batch(parsed.msg_id, inbound) + return + + self._schedule_prepare_inbound(parsed.msg_id, inbound, reservation=reservation) + except Exception: + if reservation is not None: + reservation.release() + logger.exception("[Feishu] error dispatching message %s", parsed.msg_id) @staticmethod def _is_batchable_file_inbound( @@ -888,11 +1232,13 @@ class FeishuChannel(Channel): inbound: InboundMessage, *, source_message_ids: list[str] | None = None, + reservation: InboundReservation | None = None, ) -> None: if self._main_loop and self._main_loop.is_running(): - reservation = self._reserve_inbound(inbound) if reservation is None: - return + reservation = self._reserve_inbound(inbound) + if reservation is None: + return logger.info("[Feishu] publishing inbound message to bus (type=%s, msg_id=%s)", inbound.msg_type.value, msg_id) scheduled = self._submit_threadsafe_coroutine( self._prepare_inbound( @@ -909,6 +1255,8 @@ class FeishuChannel(Channel): if not scheduled: logger.info("[Feishu] main loop stopped before reserved inbound could be scheduled") else: + if reservation is not None: + reservation.release() logger.warning("[Feishu] main loop not running, cannot publish inbound message") def _schedule_batch_flush(self, key: tuple[str, str], source_message_id: str) -> None: @@ -1074,93 +1422,110 @@ class FeishuChannel(Channel): await self._reply_card(message_id, "Feishu connected to DeerFlow.") return True - def _on_message(self, event) -> None: - """Called by lark-oapi when a message is received (runs in lark thread).""" - try: - logger.info("[Feishu] raw event received: type=%s", type(event).__name__) - message = event.event.message - chat_id = message.chat_id - msg_id = message.message_id - sender_id = event.event.sender.sender_id.open_id + def _parse_message_event(self, event) -> _ParsedMessage | None: + """Extract text, attachments and ids from a lark message event; ``None`` for an empty message.""" + message = event.event.message + chat_id = message.chat_id + msg_id = message.message_id + sender_id = event.event.sender.sender_id.open_id - root_id = getattr(message, "root_id", None) or None - chat_type = getattr(message, "chat_type", None) - parent_id = self._non_empty_str(getattr(message, "parent_id", None)) - feishu_thread_id = self._non_empty_str(getattr(message, "thread_id", None)) + root_id = getattr(message, "root_id", None) or None + chat_type = getattr(message, "chat_type", None) + parent_id = self._non_empty_str(getattr(message, "parent_id", None)) + feishu_thread_id = self._non_empty_str(getattr(message, "thread_id", None)) - # Parse message content - content = json.loads(message.content) + # Parse message content + content = json.loads(message.content) - # files_list store the any-file-key in feishu messages, which can be used to download the file content later - # In Feishu channel, image_keys are independent of file_keys. - # The file_key includes files, videos, and audio, but does not include stickers. - files_list = [] + # files_list store the any-file-key in feishu messages, which can be used to download the file content later + # In Feishu channel, image_keys are independent of file_keys. + # The file_key includes files, videos, and audio, but does not include stickers. + files_list = [] - if "text" in content: - # Handle plain text messages - text = content["text"] - elif "file_key" in content: - file_key = content.get("file_key") - if isinstance(file_key, str) and file_key: - files_list.append({"file_key": file_key}) - text = "[file]" - else: - text = "" - elif "image_key" in content: - image_key = content.get("image_key") - if isinstance(image_key, str) and image_key: - files_list.append({"image_key": image_key}) - text = "[image]" - else: - text = "" - elif "content" in content and isinstance(content["content"], list): - # Handle rich-text messages with a top-level "content" list (e.g., topic groups/posts) - text_paragraphs: list[str] = [] - for paragraph in content["content"]: - if isinstance(paragraph, list): - paragraph_text_parts: list[str] = [] - for element in paragraph: - if isinstance(element, dict): - # Include both normal text and @ mentions - if element.get("tag") in ("text", "at"): - text_value = element.get("text", "") - if text_value: - paragraph_text_parts.append(text_value) - elif element.get("tag") == "img": - image_key = element.get("image_key") - if isinstance(image_key, str) and image_key: - files_list.append({"image_key": image_key}) - paragraph_text_parts.append("[image]") - elif element.get("tag") in ("file", "media"): - file_key = element.get("file_key") - if isinstance(file_key, str) and file_key: - files_list.append({"file_key": file_key}) - paragraph_text_parts.append("[file]") - if paragraph_text_parts: - # Join text segments within a paragraph with spaces to avoid "helloworld" - text_paragraphs.append(" ".join(paragraph_text_parts)) - - # Join paragraphs with blank lines to preserve paragraph boundaries - text = "\n\n".join(text_paragraphs) + if "text" in content: + # Handle plain text messages + text = content["text"] + elif "file_key" in content: + file_key = content.get("file_key") + if isinstance(file_key, str) and file_key: + files_list.append({"file_key": file_key}) + text = "[file]" else: text = "" - text = text.strip() + elif "image_key" in content: + image_key = content.get("image_key") + if isinstance(image_key, str) and image_key: + files_list.append({"image_key": image_key}) + text = "[image]" + else: + text = "" + elif "content" in content and isinstance(content["content"], list): + # Handle rich-text messages with a top-level "content" list (e.g., topic groups/posts) + text_paragraphs: list[str] = [] + for paragraph in content["content"]: + if isinstance(paragraph, list): + paragraph_text_parts: list[str] = [] + for element in paragraph: + if isinstance(element, dict): + # Include both normal text and @ mentions + if element.get("tag") in ("text", "at"): + text_value = element.get("text", "") + if text_value: + paragraph_text_parts.append(text_value) + elif element.get("tag") == "img": + image_key = element.get("image_key") + if isinstance(image_key, str) and image_key: + files_list.append({"image_key": image_key}) + paragraph_text_parts.append("[image]") + elif element.get("tag") in ("file", "media"): + file_key = element.get("file_key") + if isinstance(file_key, str) and file_key: + files_list.append({"file_key": file_key}) + paragraph_text_parts.append("[file]") + if paragraph_text_parts: + # Join text segments within a paragraph with spaces to avoid "helloworld" + text_paragraphs.append(" ".join(paragraph_text_parts)) - logger.info( - "[Feishu] parsed message: chat_id=%s, msg_id=%s, root_id=%s, parent_id=%s, thread_id=%s, chat_type=%s, sender=%s, text_len=%d", - chat_id, - msg_id, - root_id, - parent_id, - feishu_thread_id, - chat_type, - sender_id, - len(text or ""), - ) + # Join paragraphs with blank lines to preserve paragraph boundaries + text = "\n\n".join(text_paragraphs) + else: + text = "" + text = text.strip() - if not (text or files_list): - logger.info("[Feishu] empty text, ignoring message") + logger.info( + "[Feishu] parsed message: chat_id=%s, msg_id=%s, root_id=%s, parent_id=%s, thread_id=%s, chat_type=%s, sender=%s, text_len=%d", + chat_id, + msg_id, + root_id, + parent_id, + feishu_thread_id, + chat_type, + sender_id, + len(text or ""), + ) + + if not (text or files_list): + logger.info("[Feishu] empty text, ignoring message") + return None + return _ParsedMessage( + chat_id=chat_id, + msg_id=msg_id, + sender_id=sender_id, + root_id=root_id, + parent_id=parent_id, + feishu_thread_id=feishu_thread_id, + chat_type=chat_type, + text=text, + files=files_list, + ) + + def _on_message(self, event) -> None: + """Called by lark-oapi when a message is received (runs synchronously on the SDK's loop thread).""" + try: + parsed = self._parse_message_event(event) + if parsed is None: return + msg_id, chat_id, sender_id, text = parsed.msg_id, parsed.chat_id, parsed.sender_id, parsed.text connect_code = self._pending_connect_code(text) if connect_code: @@ -1191,75 +1556,40 @@ class FeishuChannel(Channel): # inbound so ChannelManager._handle_command parses the bare command. command_text = strip_leading_mentions(text) if _is_feishu_command(command_text): - msg_type = InboundMessageType.COMMAND - text = command_text - else: - msg_type = InboundMessageType.CHAT + parsed.msg_type = InboundMessageType.COMMAND + parsed.text = command_text # topic_id determines which LangGraph thread the message maps to. # P2P chats: topic_id=None so all messages share one thread (like Telegram DMs). # But check stored mappings first for backward compatibility with pre-upgrade P2P threads. - topic_id, resolved_from_stored_mapping = self._resolve_topic_id( - chat_id, - msg_id, - root_id=root_id, - parent_id=parent_id, - thread_id=feishu_thread_id, - ) - if chat_type == "p2p" and not resolved_from_stored_mapping: - topic_id = None - resolved_from_pending = False - if msg_type == InboundMessageType.CHAT and not resolved_from_stored_mapping: - pending = self._consume_pending_clarification(chat_id, sender_id) - pending_topic_id = self._non_empty_str(pending.get("topic_id")) if pending else None - if pending_topic_id: - topic_id = pending_topic_id - self._ensure_pending_thread_mapping(chat_id, sender_id, pending) - resolved_from_pending = True - - source_preview = None - if self._should_include_source_preview( - chat_type=chat_type, - root_id=root_id, - parent_id=parent_id, - thread_id=feishu_thread_id, - ): - source_preview = self._compact_source_preview(text) - - metadata = { - "message_id": msg_id, - "root_id": root_id, - "parent_id": parent_id, - "thread_id": feishu_thread_id, - "topic_id": topic_id, - "user_id": sender_id, - RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY: resolved_from_pending, - } - if source_preview: - metadata[SOURCE_PREVIEW_METADATA_KEY] = source_preview - - inbound = self._make_inbound( - chat_id=chat_id, - user_id=sender_id, - text=text, - msg_type=msg_type, - thread_ts=msg_id, - files=files_list, - metadata=metadata, - ) - inbound.topic_id = topic_id - - if self._is_batchable_file_inbound( - msg_type=msg_type, - text=text, - files=files_list, - root_id=root_id, - parent_id=parent_id, - thread_id=feishu_thread_id, - ): - self._queue_file_inbound_batch(msg_id, inbound) + if self.config.get("channel_store") is None: + self._dispatch_inbound(parsed, self._route_without_store(parsed)) return - self._schedule_prepare_inbound(msg_id, inbound) + # Reserve bounded intake *before* routing: a message that may be deferred + # while the store is slow holds a real slot of the inbound queue, so the + # deferred backlog shares ``inbound_queue_maxsize`` with live messages. + # The placeholder only feeds the bus's capacity warning. + placeholder = self._make_inbound(chat_id=chat_id, user_id=sender_id, text=parsed.text, msg_type=parsed.msg_type, thread_ts=msg_id, files=parsed.files) + reservation = self._reserve_inbound(placeholder) + if reservation is None: + logger.warning("[Feishu] dropping message %s for chat %s: inbound intake capacity exhausted", msg_id, chat_id) + return + attempt = _RoutingAttempt(parsed, reservation=reservation) + with self._thread_lock: + queue = self._deferred_routing.get(chat_id) + if queue is not None: + # Per-chat FIFO: never overtake a deferred predecessor onto another thread. + queue.append(attempt) + logger.info("[Feishu] queued message %s behind a deferred message for chat %s", msg_id, chat_id) + return + try: + outcome = self._run_store_call(lambda: self._resolve_routing_bounded(attempt), name="resolve_topic_mapping", msg_id=msg_id) + except StoreLookupUnresolved as unresolved: + outcome = unresolved + if isinstance(outcome, StoreLookupUnresolved): + self._defer_routing(attempt, outcome) + return + self._dispatch_inbound(parsed, outcome, reservation=attempt.reservation) except Exception: logger.exception("[Feishu] error processing message") diff --git a/backend/app/channels/manager.py b/backend/app/channels/manager.py index 20b8c2b450..5a8cb39efd 100644 --- a/backend/app/channels/manager.py +++ b/backend/app/channels/manager.py @@ -147,7 +147,7 @@ BOUND_IDENTITY_UNAVAILABLE_MESSAGE = "Channel connection verification is tempora # GitHub). This is parity with every other IM channel's dedupe (same # mechanism, same TTL), not a channel-specific gap. True idempotency against # a late/manual redelivery would require persisting the dedupe key in -# ``ChannelStore`` instead, which is not implemented here. +# the database-backed ``ChannelStore`` instead, which is not implemented here. # Follow-up buffering for busy fire_and_forget threads (issue #4121 Slice 2). # A ConflictError on a channel opted into ChannelRunPolicy.buffer_followups_on_busy # buffers the triggering message per-thread instead of only logging it; a @@ -2452,7 +2452,7 @@ class ChannelManager: ) return - self.store.set_thread_id( + await self.store.set_thread_id( msg.channel_name, msg.chat_id, thread_id, diff --git a/backend/app/channels/runtime_config_store.py b/backend/app/channels/runtime_config_store.py index ec7b810bfb..175eefeb6e 100644 --- a/backend/app/channels/runtime_config_store.py +++ b/backend/app/channels/runtime_config_store.py @@ -17,7 +17,7 @@ RUNTIME_CHANNEL_DISABLED_FLAG = "_runtime_disabled" class ChannelRuntimeConfigStore: """JSON-backed store for channel credentials entered from the UI. - This intentionally mirrors ``ChannelStore``: local/private deployments get + This intentionally mirrors ``JsonChannelStore``: local/private deployments get durable runtime configuration without needing a public callback URL or a config.yaml edit. """ diff --git a/backend/app/channels/service.py b/backend/app/channels/service.py index 6166792a01..977d982277 100644 --- a/backend/app/channels/service.py +++ b/backend/app/channels/service.py @@ -14,7 +14,7 @@ from app.channels.base import Channel, ChannelStopTimeout from app.channels.manager import DEFAULT_CHANNEL_MAX_CONCURRENCY, DEFAULT_CHANNEL_SHUTDOWN_GRACE_PERIOD_SECONDS, DEFAULT_GATEWAY_URL, DEFAULT_LANGGRAPH_URL, ChannelManager from app.channels.message_bus import DEFAULT_INBOUND_QUEUE_MAXSIZE, MessageBus from app.channels.runtime_config_store import merge_runtime_channel_configs -from app.channels.store import ChannelStore +from app.channels.store import ChannelStore, SqlChannelStore, resolve_channel_store from deerflow.utils.file_io import await_drained logger = logging.getLogger(__name__) @@ -128,13 +128,18 @@ class ChannelService: require_bound_identity: bool = False, app_config: AppConfig | None = None, get_stream_bridge: Callable[[], StreamBridge | None] | None = None, + store: ChannelStore | None = None, ) -> None: config = dict(channels_config or {}) inbound_queue_maxsize = _resolve_positive_int(config, "inbound_queue_maxsize", DEFAULT_INBOUND_QUEUE_MAXSIZE) max_concurrency = _resolve_positive_int(config, "max_concurrency", DEFAULT_CHANNEL_MAX_CONCURRENCY) shutdown_grace_period_seconds = _resolve_non_negative_float(config, "shutdown_grace_period_seconds", DEFAULT_CHANNEL_SHUTDOWN_GRACE_PERIOD_SECONDS) self.bus = MessageBus(inbound_queue_maxsize=inbound_queue_maxsize) - self.store = ChannelStore() + # Chat-to-thread bindings: the shared ``channel_thread_bindings`` table + # for a sqlite/postgres database, the legacy JSON file otherwise. The + # JSON path resolves ``base_dir`` through realpath, so ``from_app_config`` + # runs this constructor off the loop. + self.store: ChannelStore = store if store is not None else resolve_channel_store(app_config) self._connection_repo = connection_repo self._get_stream_bridge = get_stream_bridge langgraph_url = _resolve_service_url(config, "langgraph_url", _CHANNELS_LANGGRAPH_URL_ENV, DEFAULT_LANGGRAPH_URL) @@ -212,6 +217,17 @@ class ChannelService: async with self._manager_start_lock: if generation != self._shutdown_generation or self._running: return + if isinstance(self.store, SqlChannelStore): + # One-time move of a pre-database ``channels/store.json`` into the + # shared table (idempotent; a peer replica importing concurrently is + # tolerated). It runs under the start lock, after the generation + # check and before the manager starts, so the bindings are in the + # table before the first message is routed (a known chat would + # otherwise get a second thread), and a stop() that lands during + # the import bumps the generation and invalidates this start. + await self.store.import_legacy_json() + if generation != self._shutdown_generation: + return await self.manager.start() if generation != self._shutdown_generation: # Keep newer starts waiting until this late start is drained. @@ -660,9 +676,9 @@ async def start_channel_service( global _channel_service if _channel_service is not None: return _channel_service - # from_app_config reads the JSON channel store and runtime config files; - # keep that disk IO off the event loop. asyncio.to_thread forwards both - # args and kwargs to the target callable. + # from_app_config resolves the channel store path (realpath) and reads the + # runtime config files; keep that disk IO off the event loop. + # asyncio.to_thread forwards both args and kwargs to the target callable. service = await asyncio.to_thread(ChannelService.from_app_config, app_config, get_stream_bridge=get_stream_bridge) _channel_service = service diff --git a/backend/app/channels/store.py b/backend/app/channels/store.py index 83e0d22ee2..ff408ee256 100644 --- a/backend/app/channels/store.py +++ b/backend/app/channels/store.py @@ -1,25 +1,95 @@ -"""ChannelStore — persists IM chat-to-DeerFlow thread mappings.""" +"""ChannelStore — persists IM chat-to-DeerFlow thread bindings. + +``ChannelManager`` maps every unbound IM conversation +(``channel_name:chat_id[:topic_id]``) to the DeerFlow thread it runs on. Two +implementations share the :class:`ChannelStore` protocol: + +- :class:`SqlChannelStore` keeps the bindings in the ``channel_thread_bindings`` + table of the application database (SQLite or PostgreSQL). Every Gateway + replica sharing that database sees the same bindings, so a conversation + created on one replica continues on the same thread when the next message + lands on another. This is the store for every non-``memory`` database. +- :class:`JsonChannelStore` is the historical single file at + ``{base_dir}/channels/store.json``, loaded once per process and rewritten + atomically on every change. It stays for ``database.backend: memory`` (no + database to share) and as the source of the one-time import below. With + several Gateway processes each loads its own copy: a binding created on one + is invisible to the others and concurrent writers clobber each other's file. + +The protocol is async because the SQL store runs on the async engine and must +never block the Gateway event loop; the JSON store offloads its file I/O through +``asyncio.to_thread`` for the same reason (anchored by +``tests/blocking_io/test_channel_thread_binding_store.py``). Callers in a +synchronous SDK callback (Feishu's lark thread) bridge to the loop through +``Channel._submit_threadsafe_coroutine_future``. + +Selection (:func:`resolve_channel_store`) follows ``database.backend``: +``memory`` -> JSON; ``sqlite`` / ``postgres`` -> SQL whenever the persistence +engine is initialised, else JSON with a warning (a bare ``ChannelService`` +without the Gateway lifespan). ``ChannelService.start()`` then runs +:meth:`SqlChannelStore.import_legacy_json` once: if ``store.json`` exists and the +table is empty every entry is inserted with ``ON CONFLICT DO NOTHING`` (two +replicas importing concurrently cannot duplicate or clobber a binding) and the +file is renamed to ``store.json.migrated``; a populated table leaves the file +alone, and a missing file is a no-op. +""" from __future__ import annotations +import asyncio import json import logging import tempfile import threading import time from pathlib import Path -from typing import Any +from typing import Any, Protocol + +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from deerflow.persistence.channel_thread_bindings import ChannelThreadBinding, SqlChannelThreadBindingRepository, binding_key, split_binding_key +from deerflow.persistence.channel_thread_bindings.model import BINDING_KEY_LENGTH, CHANNEL_NAME_LENGTH, CHAT_ID_LENGTH, THREAD_ID_LENGTH, TOPIC_ID_LENGTH, USER_ID_LENGTH logger = logging.getLogger(__name__) +#: Suffix appended to ``store.json`` once its entries live in the database. +MIGRATED_SUFFIX = ".migrated" -class ChannelStore: + +class ChannelStore(Protocol): + """Async mapping from an IM conversation/topic to its DeerFlow ``thread_id``.""" + + async def get_thread_id(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> str | None: + """Look up the DeerFlow thread_id for a given IM conversation/topic.""" + ... + + async def set_thread_id(self, channel_name: str, chat_id: str, thread_id: str, *, topic_id: str | None = None, user_id: str = "") -> None: + """Create or update the mapping for an IM conversation/topic (an update keeps ``created_at``).""" + ... + + async def remove(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> bool: + """Remove one topic mapping, or — without ``topic_id`` — the base mapping and every topic of that chat.""" + ... + + async def list_entries(self, channel_name: str | None = None) -> list[dict[str, Any]]: + """List all stored mappings, optionally filtered by channel.""" + ... + + +def default_store_path() -> Path: + """``{base_dir}/channels/store.json`` (resolves ``base_dir`` through realpath: call off the loop).""" + from deerflow.config.paths import get_paths + + return Path(get_paths().base_dir) / "channels" / "store.json" + + +class JsonChannelStore: """JSON-file-backed store that maps IM conversations to DeerFlow threads. Data layout (on disk):: { - ":": { + ":[:]": { "thread_id": "", "user_id": "", "created_at": 1700000000.0, @@ -28,38 +98,39 @@ class ChannelStore: ... } - The store is intentionally simple — a single JSON file that is atomically - rewritten on every mutation. For production workloads with high concurrency, - this can be swapped for a proper database backend. + The file is read once (lazily, on the first access) and atomically + rewritten on every mutation, with the lock held across the mutation and the + write so concurrent writers in this process serialize. Nothing re-reads the + file afterwards, so processes sharing one file do not see each other's + bindings — use :class:`SqlChannelStore` whenever an application database + exists. Construction is I/O-free; every operation runs its file work in a + worker thread. """ def __init__(self, path: str | Path | None = None) -> None: - if path is None: - from deerflow.config.paths import get_paths - - path = Path(get_paths().base_dir) / "channels" / "store.json" - self._path = Path(path) - self._path.parent.mkdir(parents=True, exist_ok=True) - self._data: dict[str, dict[str, Any]] = self._load() + self._path = Path(path) if path is not None else default_store_path() + self._data: dict[str, dict[str, Any]] | None = None self._lock = threading.Lock() - # -- persistence ------------------------------------------------------- + @property + def path(self) -> Path: + return self._path - def _load(self) -> dict[str, dict[str, Any]]: - if self._path.exists(): - try: - return json.loads(self._path.read_text(encoding="utf-8")) - except (json.JSONDecodeError, OSError): - logger.warning("Corrupt channel store at %s, starting fresh", self._path) - return {} + @property + def channels_dir(self) -> Path: + """The directory sibling channel state files (Discord thread map, Buzz seen events) live in.""" + return self._path.parent + + # -- persistence (worker thread, lock held by the caller) --------------- + + def _ensure_loaded(self) -> dict[str, dict[str, Any]]: + if self._data is None: + self._data = _read_legacy_file(self._path, log_missing=False, on_corrupt="Corrupt channel store at %s, starting fresh") or {} + return self._data def _save(self) -> None: - fd = tempfile.NamedTemporaryFile( - mode="w", - dir=self._path.parent, - suffix=".tmp", - delete=False, - ) + self._path.parent.mkdir(parents=True, exist_ok=True) + fd = tempfile.NamedTemporaryFile(mode="w", dir=self._path.parent, suffix=".tmp", delete=False) try: json.dump(self._data, fd, indent=2) fd.close() @@ -69,37 +140,19 @@ class ChannelStore: Path(fd.name).unlink(missing_ok=True) raise - # -- key helpers ------------------------------------------------------- + # -- sync bodies ------------------------------------------------------- - @staticmethod - def _key(channel_name: str, chat_id: str, topic_id: str | None = None) -> str: - if topic_id: - return f"{channel_name}:{chat_id}:{topic_id}" - return f"{channel_name}:{chat_id}" - - # -- public API -------------------------------------------------------- - - def get_thread_id(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> str | None: - """Look up the DeerFlow thread_id for a given IM conversation/topic.""" + def _get_sync(self, key: str) -> str | None: with self._lock: - entry = self._data.get(self._key(channel_name, chat_id, topic_id)) + entry = self._ensure_loaded().get(key) return entry["thread_id"] if entry else None - def set_thread_id( - self, - channel_name: str, - chat_id: str, - thread_id: str, - *, - topic_id: str | None = None, - user_id: str = "", - ) -> None: - """Create or update the mapping for an IM conversation/topic.""" + def _set_sync(self, key: str, thread_id: str, user_id: str) -> None: with self._lock: - key = self._key(channel_name, chat_id, topic_id) + data = self._ensure_loaded() now = time.time() - existing = self._data.get(key) - self._data[key] = { + existing = data.get(key) + data[key] = { "thread_id": thread_id, "user_id": user_id, "created_at": existing["created_at"] if existing else now, @@ -107,47 +160,33 @@ class ChannelStore: } self._save() - def remove(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> bool: - """Remove a mapping. - - If ``topic_id`` is provided, only that specific conversation/topic mapping is removed. - If ``topic_id`` is omitted, all mappings whose key starts with - ``":"`` (including topic-specific ones) are removed. - - Returns True if at least one mapping was removed. - """ + def _remove_sync(self, channel_name: str, chat_id: str, topic_id: str | None) -> bool: with self._lock: - # Remove a specific conversation/topic mapping. + data = self._ensure_loaded() if topic_id is not None: - key = self._key(channel_name, chat_id, topic_id) - if key in self._data: - del self._data[key] - self._save() - return True - return False - - # Remove all mappings for this channel/chat_id (base and any topic-specific keys). - prefix = self._key(channel_name, chat_id) - keys_to_delete = [k for k in self._data if k == prefix or k.startswith(prefix + ":")] + key = binding_key(channel_name, chat_id, topic_id) + if key not in data: + return False + del data[key] + self._save() + return True + prefix = binding_key(channel_name, chat_id) + keys_to_delete = [k for k in data if k == prefix or k.startswith(prefix + ":")] if not keys_to_delete: return False - for k in keys_to_delete: - del self._data[k] + del data[k] self._save() return True - def list_entries(self, channel_name: str | None = None) -> list[dict[str, Any]]: - """List all stored mappings, optionally filtered by channel.""" + def _list_sync(self, channel_name: str | None) -> list[dict[str, Any]]: + # Snapshot under the lock, format after release: a concurrent writer + # cannot resize the dict while this iterates. with self._lock: - entries = [(key, entry.copy()) for key, entry in self._data.items()] - + entries = [(key, entry.copy()) for key, entry in self._ensure_loaded().items()] results = [] for key, entry in entries: - parts = key.split(":", 2) - ch = parts[0] - chat = parts[1] if len(parts) > 1 else "" - topic = parts[2] if len(parts) > 2 else None + ch, chat, topic = split_binding_key(key) if channel_name and ch != channel_name: continue item: dict[str, Any] = {"channel_name": ch, "chat_id": chat, **entry} @@ -155,3 +194,207 @@ class ChannelStore: item["topic_id"] = topic results.append(item) return results + + # -- public API -------------------------------------------------------- + + async def get_thread_id(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> str | None: + return await asyncio.to_thread(self._get_sync, binding_key(channel_name, chat_id, topic_id)) + + async def set_thread_id(self, channel_name: str, chat_id: str, thread_id: str, *, topic_id: str | None = None, user_id: str = "") -> None: + await asyncio.to_thread(self._set_sync, binding_key(channel_name, chat_id, topic_id), thread_id, user_id) + + async def remove(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> bool: + return await asyncio.to_thread(self._remove_sync, channel_name, chat_id, topic_id) + + async def list_entries(self, channel_name: str | None = None) -> list[dict[str, Any]]: + return await asyncio.to_thread(self._list_sync, channel_name) + + +class SqlChannelStore: + """Database-backed store over ``channel_thread_bindings``, shared by every replica using the database.""" + + def __init__(self, session_factory: async_sessionmaker[AsyncSession], *, legacy_path: str | Path | None = None) -> None: + self._repo = SqlChannelThreadBindingRepository(session_factory) + self._legacy_path = Path(legacy_path) if legacy_path is not None else None + + @property + def legacy_path(self) -> Path | None: + """The ``store.json`` this store imports from (and whose directory sibling state files use).""" + return self._legacy_path + + @property + def channels_dir(self) -> Path | None: + return self._legacy_path.parent if self._legacy_path is not None else None + + async def get_thread_id(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> str | None: + return await self._repo.get_thread_id(binding_key(channel_name, chat_id, topic_id)) + + async def set_thread_id(self, channel_name: str, chat_id: str, thread_id: str, *, topic_id: str | None = None, user_id: str = "") -> None: + await self._repo.upsert(ChannelThreadBinding.build(channel_name, chat_id, thread_id, topic_id=topic_id, user_id=user_id, now=time.time())) + + async def remove(self, channel_name: str, chat_id: str, topic_id: str | None = None) -> bool: + if topic_id is not None: + return await self._repo.delete(binding_key(channel_name, chat_id, topic_id)) + return await self._repo.delete_prefix(binding_key(channel_name, chat_id)) > 0 + + async def list_entries(self, channel_name: str | None = None) -> list[dict[str, Any]]: + return [binding.to_entry() for binding in await self._repo.list(channel_name)] + + async def import_legacy_json(self) -> int: + """Move the entries of ``legacy_path`` into the table once; returns how many rows this call inserted. + + No file, no legacy path or a populated table is a no-op (the file is + left where it is in the last case: a populated table is the source of + truth and the operator may still want the JSON). A corrupt file is + kept and reported. Otherwise every well-formed entry is inserted with + ``ON CONFLICT DO NOTHING`` — so two replicas starting together both + import without duplicating or clobbering anything — and the file is + renamed to ``store.json.migrated``; a rename that fails because a peer + already renamed it is tolerated. + """ + path = self._legacy_path + if path is None: + return 0 + entries = await asyncio.to_thread(lambda: _read_legacy_file(path, log_missing=True, on_corrupt="Corrupt legacy channel store at %s; skipping the import (file kept)")) + if entries is None: + return 0 + if await self._repo.count() > 0: + logger.info("Channel thread bindings are already in the database; leaving legacy %s in place without importing it", path) + return 0 + now = time.time() + bindings: list[ChannelThreadBinding] = [] + skipped = 0 + for key, entry in entries.items(): + binding = _legacy_binding(key, entry, now) + if binding is None: + skipped += 1 + continue + bindings.append(binding) + if skipped: + logger.warning( + "Skipping %d malformed or oversized entries in legacy channel store %s (not an object, no thread_id, empty channel/chat, or a component longer than its column)", + skipped, + path, + ) + inserted = await self._repo.insert_missing(bindings) + renamed = await asyncio.to_thread(_rename_migrated, path) + logger.info( + "Imported %d of %d chat-to-thread bindings from %s into channel_thread_bindings%s", + inserted, + len(bindings), + path, + f"; renamed to {path.name}{MIGRATED_SUFFIX}" if renamed else " (file already moved by a peer)", + ) + return inserted + + +def _read_legacy_file(path: Path, *, log_missing: bool, on_corrupt: str) -> dict[str, Any] | None: + """Parse ``store.json``; ``None`` when missing or corrupt. + + ``on_corrupt`` is the caller's warning (``%s`` = path): the JSON store starts + fresh, the import skips and keeps the file — the two outcomes differ, so the + wording is the caller's. + """ + try: + raw = path.read_text(encoding="utf-8") + except FileNotFoundError: + if log_missing: + logger.debug("No legacy channel store at %s", path) + return None + except OSError: + logger.warning(on_corrupt, path, exc_info=True) + return None + try: + data = json.loads(raw) + except json.JSONDecodeError: + logger.warning(on_corrupt, path) + return None + if not isinstance(data, dict): + logger.warning(on_corrupt, path) + return None + return data + + +def _legacy_binding(key: str, entry: Any, now: float) -> ChannelThreadBinding | None: + """One legacy entry as a binding, or ``None`` when it is malformed or would not fit the table. + + The column bounds are checked here because only PostgreSQL enforces + ``String(n)``: an oversized component would fail the whole import batch there + (``value too long for type character varying``) and take every IM channel down + with ``ChannelService.start()``, while SQLite silently accepts the same row. + """ + if not isinstance(entry, dict) or not isinstance(entry.get("thread_id"), str) or not entry["thread_id"]: + return None + channel_name, chat_id, topic_id = split_binding_key(key) + if not channel_name or not chat_id: + return None + thread_id = entry["thread_id"] + user_id = entry.get("user_id") + user_id = user_id if isinstance(user_id, str) else "" + if ( + len(key) > BINDING_KEY_LENGTH + or len(channel_name) > CHANNEL_NAME_LENGTH + or len(chat_id) > CHAT_ID_LENGTH + or (topic_id is not None and len(topic_id) > TOPIC_ID_LENGTH) + or len(thread_id) > THREAD_ID_LENGTH + or len(user_id) > USER_ID_LENGTH + ): + return None + created_at = _stamp(entry.get("created_at"), now) + updated_at = _stamp(entry.get("updated_at"), created_at) + return ChannelThreadBinding(key, channel_name, chat_id, topic_id, thread_id, user_id, created_at, updated_at) + + +def _stamp(value: Any, fallback: float) -> float: + return float(value) if isinstance(value, (int, float)) and not isinstance(value, bool) else fallback + + +def _rename_migrated(path: Path) -> bool: + """Rename ``store.json`` to ``store.json.migrated``; ``False`` when a peer already did.""" + try: + path.replace(path.with_name(path.name + MIGRATED_SUFFIX)) + except FileNotFoundError: + return False + except OSError: + logger.warning("Imported legacy channel store %s but could not rename it; the next start finds a populated table and leaves it alone", path, exc_info=True) + return False + return True + + +_UNRESOLVED = object() + + +def resolve_channel_store(app_config: Any | None, *, session_factory: Any = _UNRESOLVED, path: str | Path | None = None) -> ChannelStore: + """Pick the store for ``database.backend``: JSON for ``memory`` (or no config), SQL for a database. + + ``session_factory`` defaults to the process engine's + (``deerflow.persistence.engine.get_session_factory``); passing ``None`` + models a configured database whose engine is not initialised, which falls + back to the JSON file with a warning so a bare ``ChannelService`` keeps + working. ``path`` is the JSON file (default ``{base_dir}/channels/store.json``), + also the SQL store's import source and sibling-state directory. + """ + backend = getattr(getattr(app_config, "database", None), "backend", None) + store_path = Path(path) if path is not None else default_store_path() + if backend in ("sqlite", "postgres"): + if session_factory is _UNRESOLVED: + from deerflow.persistence.engine import get_session_factory + + session_factory = get_session_factory() + if session_factory is not None: + logger.info("IM chat-to-thread bindings use the shared channel_thread_bindings table (database.backend=%s)", backend) + return SqlChannelStore(session_factory, legacy_path=store_path) + logger.warning("database.backend=%s but no persistence engine is initialised; IM chat-to-thread bindings fall back to %s for this process", backend, store_path) + return JsonChannelStore(store_path) + + +__all__ = [ + "MIGRATED_SUFFIX", + "ChannelStore", + "JsonChannelStore", + "SqlChannelStore", + "binding_key", + "default_store_path", + "resolve_channel_store", + "split_binding_key", +] diff --git a/backend/app/gateway/deps.py b/backend/app/gateway/deps.py index 2a07ae2831..7f77e4b946 100644 --- a/backend/app/gateway/deps.py +++ b/backend/app/gateway/deps.py @@ -604,6 +604,10 @@ async def langgraph_runtime(app: FastAPI, startup_config: AppConfig) -> AsyncGen _validate_agent_storage(startup_config) # Warn when login lockouts stay per-process under several Gateway processes. _validate_login_throttle_storage(startup_config) + # IM chat-to-thread bindings need no companion warning: ``resolve_channel_store`` + # keeps them in the shared ``channel_thread_bindings`` table for every + # sqlite/postgres database, and the gate above already refuses a memory + # database -- the only JSON-file case -- under any multi-process signal. # Warn when a declared multi-instance deployment shares DeerMem's SQLite # retrieval index across instances through the memory volume. _validate_memory_retrieval_index(startup_config) diff --git a/backend/docs/CONFIGURATION.md b/backend/docs/CONFIGURATION.md index e7971a3992..af6f8bc4ca 100644 --- a/backend/docs/CONFIGURATION.md +++ b/backend/docs/CONFIGURATION.md @@ -589,6 +589,7 @@ Notes: - `GET /health/ready` pings the Redis stream bridge on every probe and answers 503 (`stream_bridge: unreachable`) while Redis is down, so the orchestrator drains that instance instead of routing it runs it cannot publish or stream; the memory bridge reports `not_configured`. When `sandbox.provisioner_url` is set, the body also carries the provisioner's `/health` verdict (`provisioner: ok|unreachable`), which never changes the status code because every instance shares one provisioner. That probe follows the sandbox clients' proxy policy: loopback, private, link-local and cluster-local provisioner addresses bypass `HTTP_PROXY`, external hosts keep the environment's proxy settings. - The declaration also drives the `agent_storage.backend: file` divergence warning, the inbound webhook dedupe warning, the `auth.local.throttle_storage: memory` warning, and the WeChat QR-login guard, which otherwise only look at the worker count. - Login lockouts are shared through the database: `auth.local.throttle_storage` (startup-only) defaults to `auto`, which keeps the per-IP failed-login counters for `POST /api/v1/auth/login/local` in the `login_throttle` table whenever `database.backend` is `sqlite` or `postgres`, so every instance enforces one `max_login_attempts` limit per client IP and a lockout on one instance holds on all of them. `memory` keeps the historical per-process counter (N instances give an attacker N × `max_login_attempts` guesses; a declared multi-instance deployment logs a warning), and `db` forces the table (it falls back to memory with a warning when the database backend is `memory`, and refuses to start when the configured database's engine is unavailable; `auto` falls back with a warning in that case). `max_login_attempts` and `lockout_seconds` stay live-read. +- IM chat-to-thread bindings are shared through the database too: with `database.backend: sqlite` or `postgres` the `ChannelManager` keeps the `channel_name:chat_id[:topic_id]` → thread mapping of unbound IM conversations in the `channel_thread_bindings` table (migration `0038_channel_thread_bindings`), so a conversation created on one instance continues on the same thread when its next message lands on another. On the first start after upgrading, an existing `{base_dir}/channels/store.json` is imported once — only into an empty table, with `INSERT … ON CONFLICT DO NOTHING` so concurrently starting instances cannot duplicate a binding — and renamed `store.json.migrated`; a populated table leaves the file untouched. `memory` keeps the per-process JSON file. - The declaration also drives the `agent_storage.backend: file` divergence warning, the inbound webhook dedupe warning, and the WeChat QR-login guard, which otherwise only look at the worker count. - With the default DeerMem backend, the declaration also warns when the derived SQLite retrieval index sits inside the shared `storage_path` (the default `{storage_path}/.retrieval`). Set `memory.backend_config.retrieval_index_path` to an instance-local directory (relative values resolve against `storage_path`): SQLite WAL is unsupported on network filesystems, and the index is rebuilt from the Markdown facts at startup and re-synced per user scope after a peer writes. - Restart-required: the gate runs once at startup. Restart all Gateway instances together after changing it. diff --git a/backend/packages/harness/deerflow/persistence/channel_thread_bindings/__init__.py b/backend/packages/harness/deerflow/persistence/channel_thread_bindings/__init__.py new file mode 100644 index 0000000000..d25fb37272 --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/channel_thread_bindings/__init__.py @@ -0,0 +1,20 @@ +"""Shared IM chat -> DeerFlow thread bindings (the database successor of ``channels/store.json``).""" + +from deerflow.persistence.channel_thread_bindings.model import ( + BINDING_KEY_LENGTH, + ChannelThreadBinding, + ChannelThreadBindingRow, + binding_key, + split_binding_key, +) +from deerflow.persistence.channel_thread_bindings.sql import IMPORT_BATCH_SIZE, SqlChannelThreadBindingRepository + +__all__ = [ + "BINDING_KEY_LENGTH", + "IMPORT_BATCH_SIZE", + "ChannelThreadBinding", + "ChannelThreadBindingRow", + "SqlChannelThreadBindingRepository", + "binding_key", + "split_binding_key", +] diff --git a/backend/packages/harness/deerflow/persistence/channel_thread_bindings/model.py b/backend/packages/harness/deerflow/persistence/channel_thread_bindings/model.py new file mode 100644 index 0000000000..9f7d36d9e6 --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/channel_thread_bindings/model.py @@ -0,0 +1,116 @@ +"""ORM model for the shared IM chat -> DeerFlow thread binding table (``channel_thread_bindings``). + +One row per unbound IM conversation the ``ChannelManager`` has routed to a +thread, keyed by the legacy composite ``channel_name:chat_id[:topic_id]`` string +that ``channels/store.json`` used. Every Gateway replica sharing the application +database reads and writes the same rows, so a conversation created on one +replica continues on the same thread when the next message lands on another. + +The shape is deliberately 1:1 with the JSON entries so the one-time import is +lossless: ``created_at`` / ``updated_at`` stay epoch seconds (the JSON stored +``time.time()`` floats), ``user_id`` is the platform sender id the manager +recorded (``""`` when unknown, never NULL), and the component columns +(``channel_name``, ``chat_id``, ``topic_id``) are what the legacy +``list_entries()`` parsed out of the key. Lookups always go through ``key``. + +Connection-bound conversations (``channel_conversations``) are a different +table on purpose: those rows belong to a user-owned connection; these are the +global fallback mapping for messages without one. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any + +from sqlalchemy import Double, Index, PrimaryKeyConstraint, String +from sqlalchemy.orm import Mapped, mapped_column + +from deerflow.persistence.base import Base + +#: Longest component values. ``channel_conversations`` already caps external ids +#: at 128, so 255 leaves every current provider id well inside the limit. +CHANNEL_NAME_LENGTH = 64 +CHAT_ID_LENGTH = 255 +TOPIC_ID_LENGTH = 255 +THREAD_ID_LENGTH = 64 +USER_ID_LENGTH = 255 +#: ``channel_name:chat_id:topic_id`` at their maximum lengths plus separators. +BINDING_KEY_LENGTH = CHANNEL_NAME_LENGTH + 1 + CHAT_ID_LENGTH + 1 + TOPIC_ID_LENGTH + + +def binding_key(channel_name: str, chat_id: str, topic_id: str | None = None) -> str: + """The legacy composite key: an empty or missing topic means "no topic".""" + if topic_id: + return f"{channel_name}:{chat_id}:{topic_id}" + return f"{channel_name}:{chat_id}" + + +def split_binding_key(key: str) -> tuple[str, str, str | None]: + """Split a legacy key into ``(channel_name, chat_id, topic_id)``. + + Mirrors the parsing the JSON store's ``list_entries()`` did: the first two + colons separate the components, so a chat id that itself carries a colon + lands partly in ``topic_id``. Rows written through the store carry exact + components; only imported legacy keys inherit this ambiguity, and lookups + never depend on it because they match the full ``key``. + """ + parts = key.split(":", 2) + channel = parts[0] + chat = parts[1] if len(parts) > 1 else "" + topic = parts[2] if len(parts) > 2 else None + return channel, chat, topic + + +@dataclass(frozen=True, slots=True) +class ChannelThreadBinding: + """One binding as the store and the import exchange it with the repository.""" + + key: str + channel_name: str + chat_id: str + topic_id: str | None + thread_id: str + user_id: str + created_at: float + updated_at: float + + @classmethod + def build(cls, channel_name: str, chat_id: str, thread_id: str, *, topic_id: str | None = None, user_id: str = "", now: float) -> ChannelThreadBinding: + """A fresh binding stamped ``now`` for both timestamps (the upsert keeps an existing ``created_at``).""" + topic = topic_id or None + return cls(binding_key(channel_name, chat_id, topic), channel_name, chat_id, topic, thread_id, user_id, now, now) + + def to_entry(self) -> dict[str, Any]: + """The ``list_entries()`` item shape the JSON store returned (``topic_id`` only when present).""" + item: dict[str, Any] = { + "channel_name": self.channel_name, + "chat_id": self.chat_id, + "thread_id": self.thread_id, + "user_id": self.user_id, + "created_at": self.created_at, + "updated_at": self.updated_at, + } + if self.topic_id is not None: + item["topic_id"] = self.topic_id + return item + + +class ChannelThreadBindingRow(Base): + __tablename__ = "channel_thread_bindings" + + key: Mapped[str] = mapped_column(String(BINDING_KEY_LENGTH), nullable=False) + channel_name: Mapped[str] = mapped_column(String(CHANNEL_NAME_LENGTH), nullable=False) + chat_id: Mapped[str] = mapped_column(String(CHAT_ID_LENGTH), nullable=False) + topic_id: Mapped[str | None] = mapped_column(String(TOPIC_ID_LENGTH), nullable=True) + thread_id: Mapped[str] = mapped_column(String(THREAD_ID_LENGTH), nullable=False) + user_id: Mapped[str] = mapped_column(String(USER_ID_LENGTH), nullable=False, default="") + # Epoch seconds, exactly what the JSON store recorded with ``time.time()``. + created_at: Mapped[float] = mapped_column(Double, nullable=False) + updated_at: Mapped[float] = mapped_column(Double, nullable=False) + + __table_args__ = ( + PrimaryKeyConstraint("key", name="pk_channel_thread_bindings"), + # Serves ``list_entries(channel_name)``; the prefix ``remove`` walks the primary key. + Index("ix_channel_thread_bindings_channel_chat", "channel_name", "chat_id"), + ) diff --git a/backend/packages/harness/deerflow/persistence/channel_thread_bindings/sql.py b/backend/packages/harness/deerflow/persistence/channel_thread_bindings/sql.py new file mode 100644 index 0000000000..7b66df68ed --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/channel_thread_bindings/sql.py @@ -0,0 +1,139 @@ +"""SQL repository for ``channel_thread_bindings`` (SQLite or PostgreSQL). + +Concurrency contract, shared with ``app.channels.store.SqlChannelStore``: + +- ``upsert`` is one dialect-native ``INSERT ... ON CONFLICT (key) DO UPDATE`` + that replaces ``thread_id`` / ``user_id`` / ``updated_at`` and keeps the + existing ``created_at`` — the overwrite semantics the JSON store had — so two + replicas recording the same conversation never produce two rows and never + lose the first stamp. +- ``insert_missing`` is the one-time import primitive: ``INSERT ... ON CONFLICT + DO NOTHING`` in bounded batches, returning only the rows it actually inserted, + so two replicas importing the same ``store.json`` concurrently can neither + duplicate nor clobber a binding that already exists (including one a peer + wrote live between the emptiness check and the insert). +- ``delete_prefix`` removes ``key == prefix`` and every ``key`` starting with + ``prefix + ":"`` (``startswith(autoescape=True)``, so ``%`` / ``_`` in a chat + id match literally) — exact parity with the JSON ``remove()`` that cleared a + chat's base mapping and all its topics by key prefix, independent of how a + legacy key was split into components. + +Database errors propagate: the manager cannot route an IM message without the +database anyway, and failing loudly beats silently splitting a conversation +across two threads. +""" + +from __future__ import annotations + +from collections.abc import Iterable + +from sqlalchemy import delete, func, or_, select +from sqlalchemy.dialects.postgresql import insert as pg_insert +from sqlalchemy.dialects.sqlite import insert as sqlite_insert +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from deerflow.persistence.channel_thread_bindings.model import ChannelThreadBinding, ChannelThreadBindingRow + +#: Rows per ``INSERT`` statement during the import; keeps bind-parameter counts small on both dialects. +IMPORT_BATCH_SIZE = 500 + +_COLUMNS = ( + ChannelThreadBindingRow.key, + ChannelThreadBindingRow.channel_name, + ChannelThreadBindingRow.chat_id, + ChannelThreadBindingRow.topic_id, + ChannelThreadBindingRow.thread_id, + ChannelThreadBindingRow.user_id, + ChannelThreadBindingRow.created_at, + ChannelThreadBindingRow.updated_at, +) + + +def _insert_for(session: AsyncSession): + dialect = session.get_bind().dialect.name + if dialect == "postgresql": + return pg_insert + if dialect == "sqlite": + return sqlite_insert + raise ValueError(f"Unsupported channel thread binding database dialect: {dialect}") + + +def _values(binding: ChannelThreadBinding) -> dict[str, object]: + return { + "key": binding.key, + "channel_name": binding.channel_name, + "chat_id": binding.chat_id, + "topic_id": binding.topic_id, + "thread_id": binding.thread_id, + "user_id": binding.user_id, + "created_at": binding.created_at, + "updated_at": binding.updated_at, + } + + +def _binding(row) -> ChannelThreadBinding: + key, channel_name, chat_id, topic_id, thread_id, user_id, created_at, updated_at = row + return ChannelThreadBinding(key, channel_name, chat_id, topic_id, thread_id, user_id or "", float(created_at), float(updated_at)) + + +class SqlChannelThreadBindingRepository: + """Persistence facade for ``channel_thread_bindings``.""" + + def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: + self._sf = session_factory + + async def get_thread_id(self, key: str) -> str | None: + async with self._sf() as session: + return (await session.execute(select(ChannelThreadBindingRow.thread_id).where(ChannelThreadBindingRow.key == key))).scalar_one_or_none() + + async def upsert(self, binding: ChannelThreadBinding) -> None: + """Create or overwrite the binding; an existing row keeps its ``created_at``.""" + async with self._sf() as session: + stmt = _insert_for(session)(ChannelThreadBindingRow).values(**_values(binding)) + stmt = stmt.on_conflict_do_update( + index_elements=[ChannelThreadBindingRow.key], + set_={"thread_id": stmt.excluded.thread_id, "user_id": stmt.excluded.user_id, "updated_at": stmt.excluded.updated_at}, + ) + await session.execute(stmt) + await session.commit() + + async def delete(self, key: str) -> bool: + async with self._sf() as session: + result = await session.execute(delete(ChannelThreadBindingRow).where(ChannelThreadBindingRow.key == key)) + await session.commit() + return result.rowcount > 0 + + async def delete_prefix(self, prefix: str) -> int: + """Delete ``prefix`` itself and every ``prefix:*`` key; returns the number of rows removed.""" + async with self._sf() as session: + predicate = or_(ChannelThreadBindingRow.key == prefix, ChannelThreadBindingRow.key.startswith(prefix + ":", autoescape=True)) + result = await session.execute(delete(ChannelThreadBindingRow).where(predicate)) + await session.commit() + return int(result.rowcount or 0) + + async def list(self, channel_name: str | None = None) -> list[ChannelThreadBinding]: + stmt = select(*_COLUMNS).order_by(ChannelThreadBindingRow.created_at, ChannelThreadBindingRow.key) + if channel_name: + stmt = stmt.where(ChannelThreadBindingRow.channel_name == channel_name) + async with self._sf() as session: + rows = (await session.execute(stmt)).all() + return [_binding(row) for row in rows] + + async def count(self) -> int: + async with self._sf() as session: + return int((await session.execute(select(func.count()).select_from(ChannelThreadBindingRow))).scalar_one()) + + async def insert_missing(self, bindings: Iterable[ChannelThreadBinding]) -> int: + """``INSERT ... ON CONFLICT DO NOTHING`` in batches; returns how many rows this call inserted.""" + rows = [_values(binding) for binding in bindings] + if not rows: + return 0 + inserted = 0 + async with self._sf() as session: + insert = _insert_for(session) + for start in range(0, len(rows), IMPORT_BATCH_SIZE): + batch = rows[start : start + IMPORT_BATCH_SIZE] + result = await session.execute(insert(ChannelThreadBindingRow).values(batch).on_conflict_do_nothing(index_elements=[ChannelThreadBindingRow.key])) + inserted += int(result.rowcount or 0) + await session.commit() + return inserted diff --git a/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md b/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md index cedd93a297..f8c7327998 100644 --- a/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md +++ b/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md @@ -1,6 +1,6 @@ ### Schema Migrations (`packages/harness/deerflow/persistence/migrations/`) -DeerFlow's application tables (`runs`, `threads_meta`, `feedback`, `users`, `run_events`, plus the four `channel_*` tables) are owned by alembic via a **hybrid bootstrap** strategy. LangGraph's checkpointer tables (`checkpoints`, `checkpoint_blobs`, `checkpoint_writes`, `checkpoint_migrations`) live in the same database but are owned by LangGraph and excluded from alembic's view via `migrations/_env_filters.py::include_object`. +DeerFlow's application tables (`runs`, `threads_meta`, `feedback`, `users`, `run_events`, plus the `channel_*` tables) are owned by alembic via a **hybrid bootstrap** strategy. LangGraph's checkpointer tables (`checkpoints`, `checkpoint_blobs`, `checkpoint_writes`, `checkpoint_migrations`) live in the same database but are owned by LangGraph and excluded from alembic's view via `migrations/_env_filters.py::include_object`. **Convention**: every ORM model change (new column, new table, new index) MUST ship as an alembic revision under `migrations/versions/`. The Gateway runs `alembic upgrade head` automatically on startup; routine production upgrades do not require manual Alembic commands. The audited offline recovery below is an exception for the out-of-tree incarnation revision. A new revision must always chain onto the current head — never insert one ahead of an already-shipped revision (re-parenting it): alembic only walks forward from a database's stamped revision, so every database already stamped at or past the insertion point treats the inserted revision as an applied ancestor and never executes it. Inserting `0023_run_change_seq` ahead of the shipped `0023_user_preferences` stranded those databases without the run-change clock schema until `0025_repair_run_change_seq` re-applied it (#5516). @@ -29,7 +29,8 @@ The empty-DB path keeps using `create_all` because `Base.metadata` is the only a `0028_parked_attempts` → `0029_scheduler_agent_tasks` → `0030_notification_claim_tokens` → `0031_scheduled_streak_boundary` → `0032_activity_and_task_events` → `0033_batch_result_artifact` → `0034_run_event_seq_watermark` → -`0035_login_throttle` → `0036_run_idempotency_request` → `0037_project_document_summaries` (current head). The preference +`0035_login_throttle` → `0036_run_idempotency_request` → `0037_project_document_summaries` → +`0038_channel_thread_bindings` (current head). The preference revision adds a separate owner/key table with a cascading users foreign key and does not alter users; the project-documents revision adds a new owner-scoped shelf table, and the MCP lease-token revision adds two nullable token columns to @@ -181,6 +182,7 @@ on installs that never enabled it. The convention is: - `migrations/versions/0035_login_throttle.py` — creates `login_throttle` (`ip` PK, `fail_count`, nullable epoch `locked_at`, nullable `lock_duration_seconds`, `updated_at` + `ix_login_throttle_updated_at` for the sweep's stale-counter predicate), the shared failed-login counter behind `POST /api/v1/auth/login/local` selected by `auth.local.throttle_storage` (`persistence/login_throttle/`: `SqlLoginThrottleStore` increments and starts the lock in one dialect-native upsert, applies check decisions with compare-and-set predicates in a separate write transaction, and sweeps served locks plus 24h-idle counters in bounded batches — the sweep's `DELETE` repeats the expiry predicate on its target, not only `ip IN (SELECT … LIMIT n)`, because PostgreSQL READ COMMITTED re-evaluates only the statement's own WHERE on a row it waited for and would otherwise delete a lock a concurrent `check` just extended). Table create/drop is guarded with a fresh inspector in both directions. New table, so the bootstrap forward-compat floor is unchanged; chains after `0034_run_event_seq_watermark`. Test: `tests/test_migration_0035_login_throttle.py` - `migrations/versions/0036_run_idempotency_request.py` — adds nullable `runs.idempotency_request_json` for versioned keyed-resume identities. The field is private persistence state, never part of `kwargs_json` or a run response; this keeps new-writer/old-reader rolling upgrades from exposing resume digests or synthetic input. Legacy rows remain NULL; new readers accept only the exact old-writer identity shape from `kwargs`, while truly identity-less resume fails closed. The bootstrap forward-compat floor is unchanged. Chains after `0035_login_throttle`. Test: `tests/test_migration_0036_run_idempotency_request.py` - `migrations/versions/0037_project_document_summaries.py` — adds nullable `project_documents.summary` (best-effort LLM one-line description, off by default; no backfill, so legacy rows stay NULL and the bootstrap forward-compat floor is unchanged); chains after `0036_run_idempotency_request`. Test: `tests/test_migration_0037_project_document_summaries.py` +- `migrations/versions/0038_channel_thread_bindings.py` — creates `channel_thread_bindings` (`key` PK = the legacy `channel_name:chat_id[:topic_id]` string, parsed `channel_name`/`chat_id`/nullable `topic_id`, `thread_id`, `user_id` (`""` when unknown, never NULL), epoch `Double` `created_at`/`updated_at` + `ix_channel_thread_bindings_channel_chat`), the shared successor of the per-process `channels/store.json` that `ChannelManager` used for unbound IM conversations (`persistence/channel_thread_bindings/`: `SqlChannelThreadBindingRepository` upserts with a dialect-native `ON CONFLICT (key) DO UPDATE` that keeps `created_at`, deletes a chat's base + topic rows by key prefix with `startswith(autoescape=True)`, and imports with batched `ON CONFLICT DO NOTHING` so two replicas importing the same file concurrently neither duplicate nor clobber a row; `app/channels/store.py::SqlChannelStore.import_legacy_json` runs that once from `ChannelService.start()` and renames the file to `store.json.migrated`). The shape is 1:1 with the JSON entries so the import is lossless; `split_binding_key` parses legacy keys on the first two colons exactly as the old `list_entries()` did (lookups always match the full `key`). Table create/drop and the index are guarded with a fresh inspector in both directions. New table, so the bootstrap forward-compat floor is unchanged; chains after `0037_project_document_summaries`. Test: `tests/test_migration_0038_channel_thread_bindings.py` - `persistence/bootstrap.py` — `bootstrap_schema(engine, backend=...)`, the three-branch provisioning decision, locked revision validation, and the narrow 0019 forward-compatibility exception - `extensions/loader.py::load_extensions` — registers each spec's `table_prefix` with `register_extension_table_prefix()` - Tests: `tests/test_persistence_bootstrap.py` (branches), `tests/test_persistence_bootstrap_concurrency.py` (concurrency), `tests/test_persistence_bootstrap_regression.py` (issue #3682), `tests/test_persistence_migrations_env.py` (filter, including extension-owned tables), `tests/test_extension_loader.py::TestTablePrefixRegistration` (spec-to-filter wiring), `tests/blocking_io/test_persistence_bootstrap.py` (asyncio.to_thread anchor), `tests/test_migration_0004_run_ownership_dedupe.py` + `tests/test_migration_0007_scheduled_run_active_dedupe.py` (dedupe-before-unique-index pre-steps), `tests/test_migration_0025_repair_run_change_seq.py` (issue #5516 skipped-revision heal) diff --git a/backend/packages/harness/deerflow/persistence/migrations/versions/0038_channel_thread_bindings.py b/backend/packages/harness/deerflow/persistence/migrations/versions/0038_channel_thread_bindings.py new file mode 100644 index 0000000000..85bd868b9d --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/migrations/versions/0038_channel_thread_bindings.py @@ -0,0 +1,74 @@ +"""Shared IM chat -> DeerFlow thread bindings for multi-replica channel routing. + +Revision ID: 0038_channel_thread_bindings +Revises: 0037_project_document_summaries + +Creates ``channel_thread_bindings``: one row per unbound IM conversation the +``ChannelManager`` routed to a thread, keyed by the legacy composite +``channel_name:chat_id[:topic_id]`` string from ``channels/store.json``, with +the parsed components, the ``thread_id``, the platform ``user_id`` and the +epoch ``created_at`` / ``updated_at`` stamps the JSON entries carried. Every +Gateway replica sharing the application database reads and writes the same +rows, replacing the per-process JSON file that made a binding created on one +replica invisible to the others (each then created its own thread for the same +chat). ``ChannelService.start()`` imports an existing ``store.json`` once and +renames it to ``store.json.migrated``. + +``_helpers.py`` only guards columns, so the table and its index are guarded +with a fresh inspector in both directions: an upgrade that finds the table (an +interrupted earlier attempt, or an empty-database bootstrap that already ran +``create_all``) completes, and a downgrade after the table is gone still moves +the version row. +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "0038_channel_thread_bindings" +down_revision: str | Sequence[str] | None = "0037_project_document_summaries" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +_TABLE = "channel_thread_bindings" +_INDEX = "ix_channel_thread_bindings_channel_chat" + + +def _has_table() -> bool: + return sa.inspect(op.get_bind()).has_table(_TABLE) + + +def _has_index() -> bool: + if not _has_table(): + return False + return _INDEX in {index["name"] for index in sa.inspect(op.get_bind()).get_indexes(_TABLE)} + + +def upgrade() -> None: + if not _has_table(): + op.create_table( + _TABLE, + sa.Column("key", sa.String(length=576), nullable=False), + sa.Column("channel_name", sa.String(length=64), nullable=False), + sa.Column("chat_id", sa.String(length=255), nullable=False), + sa.Column("topic_id", sa.String(length=255), nullable=True), + sa.Column("thread_id", sa.String(length=64), nullable=False), + sa.Column("user_id", sa.String(length=255), nullable=False), + sa.Column("created_at", sa.Double(), nullable=False), + sa.Column("updated_at", sa.Double(), nullable=False), + sa.PrimaryKeyConstraint("key", name="pk_channel_thread_bindings"), + ) + # Serves ``list_entries(channel_name)``; guarded separately so a table + # left behind by an interrupted attempt still gets its index. + if not _has_index(): + op.create_index(_INDEX, _TABLE, ["channel_name", "chat_id"]) + + +def downgrade() -> None: + if _has_index(): + op.drop_index(_INDEX, table_name=_TABLE) + if _has_table(): + op.drop_table(_TABLE) diff --git a/backend/packages/harness/deerflow/persistence/models/__init__.py b/backend/packages/harness/deerflow/persistence/models/__init__.py index 28cd9236ca..74f03e81f8 100644 --- a/backend/packages/harness/deerflow/persistence/models/__init__.py +++ b/backend/packages/harness/deerflow/persistence/models/__init__.py @@ -21,6 +21,7 @@ from deerflow.persistence.channel_connections.model import ( ChannelCredentialRow, ChannelOAuthStateRow, ) +from deerflow.persistence.channel_thread_bindings.model import ChannelThreadBindingRow from deerflow.persistence.feedback.model import FeedbackRow from deerflow.persistence.login_throttle.model import LoginThrottleRow from deerflow.persistence.managed_subagents.model import ManagedSubagentRow @@ -45,6 +46,7 @@ __all__ = [ "ChannelConversationRow", "ChannelCredentialRow", "ChannelOAuthStateRow", + "ChannelThreadBindingRow", "FeedbackRow", "LoginThrottleRow", "McpTaskRow", diff --git a/backend/tests/blocking_io/test_channel_thread_binding_store.py b/backend/tests/blocking_io/test_channel_thread_binding_store.py new file mode 100644 index 0000000000..6833561a10 --- /dev/null +++ b/backend/tests/blocking_io/test_channel_thread_binding_store.py @@ -0,0 +1,103 @@ +"""Regression anchors: the chat-to-thread binding store must not block the event loop. + +``ChannelManager`` resolves and records a conversation's thread on the Gateway +event loop for every inbound IM message (``_lookup_thread_id`` / +``_store_thread_id``), and ``ChannelService.start()`` runs the one-time +``store.json`` import there too. The SQL store runs every statement through the +async session (aiosqlite / asyncpg); the JSON store keeps the whole file on +disk and offloads its reads and atomic rewrites through ``asyncio.to_thread``. +If either regresses onto the loop — a synchronous driver call, a file read or +rename, a contended ``threading.Lock`` acquired from the loop — the strict +Blockbuster gate raises ``BlockingError`` here. + +The engine and the legacy fixture file are built off the loop: that is test +fixture work, not the production path under test. +""" + +from __future__ import annotations + +import asyncio +import json +from collections.abc import AsyncIterator +from pathlib import Path + +import pytest +import pytest_asyncio +import sqlalchemy as sa +from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine + +# Pre-import so lazy imports inside the store are cached no-ops under the gate. +import deerflow.persistence.models # noqa: F401 +from app.channels.connection_identity import lookup_thread_id +from app.channels.manager import ChannelManager +from app.channels.message_bus import InboundMessage, MessageBus +from app.channels.store import ChannelStore, JsonChannelStore, SqlChannelStore +from deerflow.persistence.base import Base +from deerflow.persistence.channel_thread_bindings import ChannelThreadBindingRow + +pytestmark = pytest.mark.asyncio + + +def _build_sqlite_engine(path: Path) -> AsyncEngine: + """Create the schema with a sync engine and return the async one (runs in a worker thread).""" + sync_engine = sa.create_engine(f"sqlite:///{path.as_posix()}") + try: + Base.metadata.create_all(sync_engine, tables=[ChannelThreadBindingRow.__table__]) + finally: + sync_engine.dispose() + return create_async_engine(f"sqlite+aiosqlite:///{path.as_posix()}") + + +def _write_legacy(path: Path) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"slack:C-legacy": {"thread_id": "legacy-thread", "user_id": "U0", "created_at": 1.0, "updated_at": 1.0}}), encoding="utf-8") + + +@pytest_asyncio.fixture(params=["json", "sql"]) +async def store(request, tmp_path) -> AsyncIterator[ChannelStore]: + engine: AsyncEngine | None = None + if request.param == "json": + store: ChannelStore = JsonChannelStore(tmp_path / "channels" / "store.json") + else: + engine = await asyncio.to_thread(_build_sqlite_engine, tmp_path / "bindings.db") + store = SqlChannelStore(async_sessionmaker(engine, expire_on_commit=False), legacy_path=tmp_path / "channels" / "store.json") + try: + yield store + finally: + if engine is not None: + await engine.dispose() + + +async def test_store_round_trip_does_not_block_loop(store) -> None: + """get / set / overwrite / list / remove on the loop, including the JSON file's atomic rewrite.""" + assert await store.get_thread_id("slack", "C1") is None + await store.set_thread_id("slack", "C1", "thread-1", user_id="U1") + await store.set_thread_id("slack", "C1", "thread-1b", topic_id="171.1", user_id="U1") + await store.set_thread_id("slack", "C1", "thread-2", user_id="U2") # overwrite + assert await store.get_thread_id("slack", "C1") == "thread-2" + assert await store.get_thread_id("slack", "C1", topic_id="171.1") == "thread-1b" + assert {entry["thread_id"] for entry in await store.list_entries("slack")} == {"thread-2", "thread-1b"} + assert await store.remove("slack", "C1", topic_id="171.1") is True + assert await store.remove("slack", "C1") is True + assert await store.list_entries() == [] + + +async def test_manager_lookup_and_store_do_not_block_loop(store) -> None: + """The production callers: ``ChannelManager`` resolving and recording a chat's thread.""" + manager = ChannelManager(bus=MessageBus(), store=store) + msg = InboundMessage(channel_name="slack", chat_id="C1", user_id="U1", text="hi", topic_id="171.1") + assert await manager._lookup_thread_id(msg) is None + assert await lookup_thread_id(msg, repo=None, store=store) is None + await manager._store_thread_id(msg, "thread-1") + assert await manager._lookup_thread_id(msg) == "thread-1" + assert await lookup_thread_id(msg, repo=None, store=store) == "thread-1" + + +async def test_legacy_import_does_not_block_loop(store, tmp_path) -> None: + """``ChannelService.start()`` imports ``store.json`` on the loop: read, insert, rename all offloaded.""" + if not isinstance(store, SqlChannelStore): + pytest.skip("the import exists only for the SQL store") + await asyncio.to_thread(_write_legacy, tmp_path / "channels" / "store.json") + assert await store.import_legacy_json() == 1 + assert await store.get_thread_id("slack", "C-legacy") == "legacy-thread" + assert await store.import_legacy_json() == 0 # the file was renamed; the no-op path stays off the loop too diff --git a/backend/tests/blocking_io/test_discord_channel_state.py b/backend/tests/blocking_io/test_discord_channel_state.py index 8c2d7d59c6..4024073aee 100644 --- a/backend/tests/blocking_io/test_discord_channel_state.py +++ b/backend/tests/blocking_io/test_discord_channel_state.py @@ -41,17 +41,17 @@ pytestmark = pytest.mark.asyncio class _FakeStore: - """Stand-in for ChannelStore so the thread-mapping file lands under tmp_path.""" + """Stand-in for a ChannelStore so the thread-mapping file lands under tmp_path.""" def __init__(self, tmp_path: Path) -> None: - self._path = tmp_path / "channel_store.json" + self.channels_dir = tmp_path async def test_discord_constructor_is_io_free_on_async_path(tmp_path: Path) -> None: """``__init__`` must not touch the filesystem — ``_start_channel`` constructs inline.""" # Direct construction on the event loop, no asyncio.to_thread wrapper — # mirrors the production _start_channel path. __init__ only resolves paths - # (Path.home / store._path.parent); if it regresses to doing exists/read_text + # (Path.home / store.channels_dir); if it regresses to doing exists/read_text # the Blockbuster gate raises BlockingError here. channel = DiscordChannel(bus=MessageBus(), config={"bot_token": "t", "channel_store": _FakeStore(tmp_path)}) assert channel._bot_token == "t" diff --git a/backend/tests/blocking_io/test_model_status_agent_config_offloop.py b/backend/tests/blocking_io/test_model_status_agent_config_offloop.py index ac76ac98a1..45fdc81750 100644 --- a/backend/tests/blocking_io/test_model_status_agent_config_offloop.py +++ b/backend/tests/blocking_io/test_model_status_agent_config_offloop.py @@ -19,7 +19,7 @@ import pytest from app.channels import manager as manager_module from app.channels.manager import ChannelManager from app.channels.message_bus import InboundMessage, MessageBus -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore pytestmark = pytest.mark.asyncio @@ -39,9 +39,9 @@ async def test_resolve_configured_model_name_loads_agent_config_off_loop(monkeyp # would itself perform file reads under the Blockbuster gate. monkeypatch.setattr(manager_module, "load_agent_config", spy_load_agent_config) - # ChannelStore's constructor mkdirs: setup I/O stays off the loop, same + # The JSON store constructor only resolves paths; keep setup I/O off the loop, same # as the file writes above — only the manager call is the unit under test. - store = await asyncio.to_thread(ChannelStore, path=tmp_path / "store.json") + store = await asyncio.to_thread(JsonChannelStore, path=tmp_path / "store.json") manager = ChannelManager(bus=MessageBus(), store=store) thread_id = "thread-1" # Seeded cache: the assistant resolution needs no client round-trip. diff --git a/backend/tests/test_buzz_channel.py b/backend/tests/test_buzz_channel.py index dc554ad1f2..d0a69c5528 100644 --- a/backend/tests/test_buzz_channel.py +++ b/backend/tests/test_buzz_channel.py @@ -261,7 +261,7 @@ def test_mention_free_channel_and_thread_follow_pass_without_mention(): assert len(captured) == 1 class FakeStore: - def get_thread_id(self, channel_name, chat_id, topic_id=None): + async def get_thread_id(self, channel_name, chat_id, topic_id=None): return "thread-1" if topic_id == "aa" * 32 else None ch2, captured2 = _started() @@ -959,11 +959,11 @@ async def test_bound_pubkey_unmentioned_reply_follows_a_thread_the_manager_mappe from datetime import UTC, datetime, timedelta from app.channels.manager import ChannelManager - from app.channels.store import ChannelStore + from app.channels.store import JsonChannelStore repo = sql_connection_repo await repo.create_oauth_state(owner_user_id="owner-bound", provider="buzz", state="tok-bind", expires_at=datetime.now(UTC) + timedelta(minutes=10)) - store = ChannelStore(tmp_path / "store.json") + store = JsonChannelStore(tmp_path / "store.json") ch, captured = _started(connection_repo=repo, channel_store=store) ch._transport = FakeTransport() manager = ChannelManager(bus=ch.bus, store=store, connection_repo=repo) @@ -978,7 +978,7 @@ async def test_bound_pubkey_unmentioned_reply_follows_a_thread_the_manager_mappe connection_id = captured[0].connection_id assert connection_id await manager._store_thread_id(captured[0], "thread-1") - assert store.get_thread_id("buzz", CHANNEL, topic_id=root) is None # the manager wrote the repo only + assert await store.get_thread_id("buzz", CHANNEL, topic_id=root) is None # the manager wrote the repo only await dispatch(_event(sk=SK_OWNER, content="follow-up", mentions=(), reply_to=root, created_at=1700000300)) assert len(captured) == 2 @@ -995,7 +995,7 @@ def test_bound_pubkey_thread_follow_ignores_a_json_mapping_the_manager_would_not the bind state, so "the bot stopped following my replies" stays triageable.""" class JsonStoreWithMapping: - def get_thread_id(self, channel_name, chat_id, topic_id=None): + async def get_thread_id(self, channel_name, chat_id, topic_id=None): return "legacy-thread" repo = FakeConnectionRepo(states={"tok-bind": "owner-bound"}) diff --git a/backend/tests/test_channel_intake_backpressure.py b/backend/tests/test_channel_intake_backpressure.py index ec79323ffd..3164586d6a 100644 --- a/backend/tests/test_channel_intake_backpressure.py +++ b/backend/tests/test_channel_intake_backpressure.py @@ -21,7 +21,7 @@ from app.channels.message_bus import ( ) from app.channels.service import ChannelService from app.channels.slack import SlackChannel -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore def _message(index: int, *, with_dedupe_identity: bool = False) -> InboundMessage: @@ -119,7 +119,7 @@ async def test_fixed_worker_pool_bounds_handler_and_queue_tasks(tmp_path: Path) bus = MessageBus(inbound_queue_maxsize=3) manager = ChannelManager( bus=bus, - store=ChannelStore(path=tmp_path / "store.json"), + store=JsonChannelStore(path=tmp_path / "store.json"), max_concurrency=2, ) release_handlers = asyncio.Event() @@ -164,7 +164,7 @@ async def test_stop_gracefully_drains_every_accepted_message_before_cancelling_w bus = MessageBus(inbound_queue_maxsize=2) manager = ChannelManager( bus=bus, - store=ChannelStore(path=tmp_path / "store.json"), + store=JsonChannelStore(path=tmp_path / "store.json"), max_concurrency=1, shutdown_grace_period_seconds=0.5, ) @@ -210,7 +210,7 @@ async def test_stop_cancels_after_grace_drops_queue_and_releases_dedupe(tmp_path bus = MessageBus(inbound_queue_maxsize=2) manager = ChannelManager( bus=bus, - store=ChannelStore(path=tmp_path / "store.json"), + store=JsonChannelStore(path=tmp_path / "store.json"), max_concurrency=1, shutdown_grace_period_seconds=0, ) @@ -257,7 +257,7 @@ async def test_successful_stop_waits_for_cancel_resistant_workers_and_watchers(t bus = MessageBus(inbound_queue_maxsize=1) manager = ChannelManager( bus=bus, - store=ChannelStore(path=tmp_path / "store.json"), + store=JsonChannelStore(path=tmp_path / "store.json"), max_concurrency=1, shutdown_grace_period_seconds=0.01, ) @@ -317,7 +317,7 @@ async def test_outer_shutdown_cancellation_does_not_start_an_unbounded_second_jo bus = MessageBus(inbound_queue_maxsize=1) manager = ChannelManager( bus=bus, - store=ChannelStore(path=tmp_path / "store.json"), + store=JsonChannelStore(path=tmp_path / "store.json"), max_concurrency=1, shutdown_grace_period_seconds=60, ) diff --git a/backend/tests/test_channel_stream_errors.py b/backend/tests/test_channel_stream_errors.py index 92f63ecf3a..498cf037b2 100644 --- a/backend/tests/test_channel_stream_errors.py +++ b/backend/tests/test_channel_stream_errors.py @@ -11,7 +11,7 @@ from langgraph_sdk.client import LangGraphClient from app.channels.manager import ChannelManager from app.channels.message_bus import InboundMessage, MessageBus -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore from app.gateway.routers import thread_runs from deerflow.runtime import DisconnectMode, RunRecord, RunStatus from deerflow.runtime.stream_bridge.memory import MemoryStreamBridge @@ -58,7 +58,7 @@ async def test_channel_consumes_error_sse_and_releases_dedupe_after_final_publis app.state.stream_bridge = bridge app.state.run_manager = SimpleNamespace(get=AsyncMock(return_value=record)) bus = MessageBus() - manager = ChannelManager(bus=bus, store=ChannelStore(path=tmp_path / "channels.json")) + manager = ChannelManager(bus=bus, store=JsonChannelStore(path=tmp_path / "channels.json")) inbound = InboundMessage(channel_name="feishu", chat_id="chat-1", user_id="user-1", text="Hello", metadata={"message_id": "message-1"}) assert await manager._is_duplicate_inbound(inbound) is False outbound = [] diff --git a/backend/tests/test_channel_thread_binding_store.py b/backend/tests/test_channel_thread_binding_store.py new file mode 100644 index 0000000000..13bb736383 --- /dev/null +++ b/backend/tests/test_channel_thread_binding_store.py @@ -0,0 +1,674 @@ +"""Chat-to-thread binding store contract: the legacy JSON file and the shared SQL table. + +``ChannelManager`` maps every unbound IM conversation (``channel_name:chat_id[:topic_id]``) +to the DeerFlow thread it runs on. Both stores must implement the semantics the +manager relied on while the mapping was a JSON file: topic keys are independent of +the base chat key, an overwrite keeps ``created_at`` and bumps ``updated_at``, +``remove`` without a topic clears the base mapping and every topic of that chat, +and ``list_entries`` returns the channel/chat/topic components next to the entry. + +The SQL store additionally shares that state across every Gateway replica using one +database, which is the whole point of the change (and the red-on-main reproduction +here: two SQL stores over one SQLite file see each other's bindings, two JSON stores +over one file do not — each process loads the file once and clobbers the other's +writes). The one-time import of ``channels/store.json`` into the table is pinned +here too: it runs once, is idempotent, tolerates two replicas importing at the same +time, renames the file afterwards and leaves it alone when the table is already +populated. +""" + +from __future__ import annotations + +import asyncio +import json +import logging +import os +import threading +import time +import uuid +from collections.abc import AsyncIterator +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path + +import pytest +import pytest_asyncio +import sqlalchemy as sa +from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine +from support.postgres import asyncpg_test_url + +from app.channels.store import ChannelStore, JsonChannelStore, SqlChannelStore, resolve_channel_store +from deerflow.persistence.base import Base +from deerflow.persistence.channel_thread_bindings import ChannelThreadBinding, ChannelThreadBindingRow, SqlChannelThreadBindingRepository, binding_key, split_binding_key +from deerflow.persistence.channel_thread_bindings.model import CHANNEL_NAME_LENGTH, CHAT_ID_LENGTH, TOPIC_ID_LENGTH, USER_ID_LENGTH +from deerflow.persistence.postgres_schema import build_asyncpg_connect_args + +pytestmark = pytest.mark.asyncio + + +async def _sqlite_engine(path: Path) -> AsyncEngine: + engine = create_async_engine(f"sqlite+aiosqlite:///{path.as_posix()}") + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all, tables=[ChannelThreadBindingRow.__table__], checkfirst=True) + return engine + + +def _sql_store(engine: AsyncEngine, *, legacy_path: Path | None = None) -> SqlChannelStore: + return SqlChannelStore(async_sessionmaker(engine, expire_on_commit=False), legacy_path=legacy_path) + + +@pytest_asyncio.fixture(params=["json", "sql"]) +async def store(request, tmp_path) -> AsyncIterator[ChannelStore]: + if request.param == "json": + yield JsonChannelStore(tmp_path / "store.json") + return + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + yield _sql_store(engine) + finally: + await engine.dispose() + + +def _write_legacy(path: Path, entries: dict) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(entries, indent=2), encoding="utf-8") + + +def _legacy_entry(thread_id: str, *, user_id: str = "u", created_at: float = 1_700_000_000.0, updated_at: float | None = None) -> dict: + return {"thread_id": thread_id, "user_id": user_id, "created_at": created_at, "updated_at": created_at if updated_at is None else updated_at} + + +# ── key helpers ───────────────────────────────────────────────────────────── + + +async def test_binding_key_matches_the_legacy_json_key_shape(): + assert binding_key("slack", "C1") == "slack:C1" + assert binding_key("slack", "C1", "171.1") == "slack:C1:171.1" + assert binding_key("slack", "C1", "") == "slack:C1" # an empty topic is "no topic", as the JSON store treated it + assert binding_key("slack", "C1", None) == "slack:C1" + + +async def test_split_binding_key_mirrors_the_legacy_list_entries_parsing(): + assert split_binding_key("slack:C1") == ("slack", "C1", None) + assert split_binding_key("slack:C1:171.1") == ("slack", "C1", "171.1") + # The legacy format is ambiguous once a chat id carries a colon; the JSON + # ``list_entries`` split on the first two colons and so does the import. + assert split_binding_key("github:owner/repo:42:coder") == ("github", "owner/repo", "42:coder") + assert split_binding_key("broken") == ("broken", "", None) + + +# ── shared contract ───────────────────────────────────────────────────────── + + +async def test_set_and_get_thread_id(store): + await store.set_thread_id("slack", "ch1", "thread-abc", user_id="u1") + assert await store.get_thread_id("slack", "ch1") == "thread-abc" + assert await store.get_thread_id("slack", "nonexistent") is None + assert await store.get_thread_id("slack", "ch1", topic_id="t") is None # a topic key is not the base key + + +async def test_topic_mappings_are_independent_of_the_base_mapping(store): + await store.set_thread_id("test", "chat1", "base-thread") + await store.set_thread_id("test", "chat1", "topic-thread", topic_id="topic-1") + await store.set_thread_id("test", "chat1", "topic-thread-2", topic_id="topic-2") + assert await store.get_thread_id("test", "chat1") == "base-thread" + assert await store.get_thread_id("test", "chat1", topic_id="topic-1") == "topic-thread" + assert await store.get_thread_id("test", "chat1", topic_id="topic-2") == "topic-thread-2" + # An empty topic id means "no topic", exactly as the JSON key builder treated it. + assert await store.get_thread_id("test", "chat1", topic_id="") == "base-thread" + + +async def test_overwrite_replaces_thread_and_user_but_preserves_created_at(store): + await store.set_thread_id("slack", "ch1", "t1", user_id="u1") + (entry,) = await store.list_entries() + created_at = entry["created_at"] + assert entry["updated_at"] == created_at + await asyncio.sleep(0.002) + await store.set_thread_id("slack", "ch1", "t2", user_id="u2") + (entry,) = await store.list_entries() + assert entry["thread_id"] == "t2" + assert entry["user_id"] == "u2" + assert entry["created_at"] == created_at + assert entry["updated_at"] > created_at + + +async def test_remove_specific_topic_leaves_the_rest(store): + await store.set_thread_id("slack", "ch1", "base") + await store.set_thread_id("slack", "ch1", "topic-a", topic_id="a") + await store.set_thread_id("slack", "ch1", "topic-b", topic_id="b") + assert await store.remove("slack", "ch1", topic_id="a") is True + assert await store.get_thread_id("slack", "ch1", topic_id="a") is None + assert await store.get_thread_id("slack", "ch1", topic_id="b") == "topic-b" + assert await store.get_thread_id("slack", "ch1") == "base" + assert await store.remove("slack", "ch1", topic_id="a") is False + + +async def test_remove_without_topic_clears_the_base_and_every_topic_of_that_chat_only(store): + await store.set_thread_id("slack", "ch1", "base") + await store.set_thread_id("slack", "ch1", "topic-a", topic_id="a") + await store.set_thread_id("slack", "ch1", "topic-b", topic_id="b:c") + await store.set_thread_id("slack", "ch10", "other-chat") # shares the prefix text, not the chat + await store.set_thread_id("feishu", "ch1", "other-channel") + assert await store.remove("slack", "ch1") is True + assert await store.get_thread_id("slack", "ch1") is None + assert await store.get_thread_id("slack", "ch1", topic_id="a") is None + assert await store.get_thread_id("slack", "ch1", topic_id="b:c") is None + assert await store.get_thread_id("slack", "ch10") == "other-chat" + assert await store.get_thread_id("feishu", "ch1") == "other-channel" + assert await store.remove("slack", "ch1") is False + assert await store.remove("slack", "nope") is False + + +async def test_remove_prefix_treats_like_wildcards_literally(store): + await store.set_thread_id("slack", "c_1", "underscore") + await store.set_thread_id("slack", "c%1", "percent") + await store.set_thread_id("slack", "cx1", "other") + assert await store.remove("slack", "c_1") is True + assert await store.get_thread_id("slack", "c_1") is None + assert await store.get_thread_id("slack", "c%1") == "percent" + assert await store.get_thread_id("slack", "cx1") == "other" + + +async def test_list_entries_all_and_filtered_with_the_legacy_shape(store): + await store.set_thread_id("slack", "ch1", "t1", user_id="u1") + await store.set_thread_id("feishu", "ch2", "t2", topic_id="om_1", user_id="u2") + entries = await store.list_entries() + assert len(entries) == 2 + by_channel = {entry["channel_name"]: entry for entry in entries} + slack = by_channel["slack"] + assert set(slack) == {"channel_name", "chat_id", "thread_id", "user_id", "created_at", "updated_at"} + assert (slack["chat_id"], slack["thread_id"], slack["user_id"]) == ("ch1", "t1", "u1") + assert isinstance(slack["created_at"], float) and isinstance(slack["updated_at"], float) + feishu = by_channel["feishu"] + assert feishu["topic_id"] == "om_1" # present only for topic mappings + assert (feishu["chat_id"], feishu["thread_id"], feishu["user_id"]) == ("ch2", "t2", "u2") + + filtered = await store.list_entries(channel_name="slack") + assert [entry["channel_name"] for entry in filtered] == ["slack"] + assert await store.list_entries(channel_name="telegram") == [] + assert await store.list_entries() != [] + + +async def test_user_id_defaults_to_an_empty_string(store): + await store.set_thread_id("slack", "ch1", "t1") + (entry,) = await store.list_entries() + assert entry["user_id"] == "" + + +async def test_entries_survive_a_new_store_instance_over_the_same_storage(tmp_path): + path = tmp_path / "store.json" + first = JsonChannelStore(path) + await first.set_thread_id("slack", "ch1", "t1") + assert await JsonChannelStore(path).get_thread_id("slack", "ch1") == "t1" + + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + await _sql_store(engine).set_thread_id("slack", "ch1", "t1") + assert await _sql_store(engine).get_thread_id("slack", "ch1") == "t1" + finally: + await engine.dispose() + + +# ── JSON-specific behaviour kept from the original store ──────────────────── + + +async def test_json_store_corrupt_file_starts_fresh(tmp_path, caplog): + path = tmp_path / "store.json" + path.write_text("not json", encoding="utf-8") + store = JsonChannelStore(path) + with caplog.at_level(logging.WARNING, logger="app.channels.store"): + assert await store.get_thread_id("x", "y") is None + assert "Corrupt channel store" in caplog.text + await store.set_thread_id("x", "y", "t") + assert json.loads(path.read_text(encoding="utf-8")) == {"x:y": {"thread_id": "t", "user_id": "", "created_at": pytest.approx(time.time(), abs=60), "updated_at": pytest.approx(time.time(), abs=60)}} + + +async def test_json_store_construction_is_io_free_and_exposes_the_channels_dir(tmp_path): + path = tmp_path / "nested" / "channels" / "store.json" + store = JsonChannelStore(path) + assert store.channels_dir == path.parent + assert not path.parent.exists() # nothing is created or read until the first access + + +async def test_json_store_concurrent_list_and_mutation(tmp_path, monkeypatch): + """``list_entries`` snapshots under the lock, so a concurrent writer cannot resize the dict mid-iteration.""" + store = JsonChannelStore(tmp_path / "store.json") + iteration_started = threading.Event() + mutation_requested = threading.Event() + mutation_finished = threading.Event() + + class CoordinatedData(dict): + def items(self): + iterator = iter(super().items()) + first = next(iterator) + iteration_started.set() + if store._lock.locked(): + assert mutation_requested.wait(timeout=5), "mutation thread never requested the store lock" + else: + assert mutation_finished.wait(timeout=5), "mutation thread never changed the unlocked store" + yield first + yield from iterator + + store._data = CoordinatedData( + { + "slack:ch1": {"thread_id": "t1", "user_id": "u1", "created_at": 1.0, "updated_at": 1.0}, + "feishu:ch2": {"thread_id": "t2", "user_id": "u2", "created_at": 2.0, "updated_at": 2.0}, + } + ) + monkeypatch.setattr(store, "_save", lambda: None) + + def mutate(): + assert iteration_started.wait(timeout=5), "list_entries never started iterating" + mutation_requested.set() + asyncio.run(store.set_thread_id("test", "new", "t3")) + mutation_finished.set() + + with ThreadPoolExecutor(max_workers=2) as executor: + list_future = executor.submit(lambda: asyncio.run(store.list_entries())) + mutation_future = executor.submit(mutate) + mutation_future.result(timeout=5) + entries = list_future.result(timeout=5) + + assert {(entry["channel_name"], entry["chat_id"]) for entry in entries} == {("slack", "ch1"), ("feishu", "ch2")} + + +# ── the multi-replica reproduction ────────────────────────────────────────── + + +async def test_two_json_stores_over_one_file_do_not_see_each_other(tmp_path): + """Today's bug: each Gateway process loads ``store.json`` once and rewrites the + whole file on every change, so a binding created on replica A is invisible to + replica B (B creates a second thread for the same chat) and B's next write + clobbers A's binding on disk.""" + path = tmp_path / "store.json" + replica_a = JsonChannelStore(path) + replica_b = JsonChannelStore(path) + assert await replica_b.get_thread_id("slack", "C1") is None # B has loaded (an empty) file + + await replica_a.set_thread_id("slack", "C1", "thread-from-a") + assert await replica_b.get_thread_id("slack", "C1") is None # invisible to B + + await replica_b.set_thread_id("slack", "C2", "thread-from-b") + assert json.loads(path.read_text(encoding="utf-8")).keys() == {"slack:C2"} # A's binding clobbered on disk + assert await JsonChannelStore(path).get_thread_id("slack", "C1") is None + + +async def test_two_sql_stores_over_one_database_share_every_binding(tmp_path): + engine_a = await _sqlite_engine(tmp_path / "bindings.db") + engine_b = create_async_engine(f"sqlite+aiosqlite:///{(tmp_path / 'bindings.db').as_posix()}") + try: + replica_a, replica_b = _sql_store(engine_a), _sql_store(engine_b) + assert await replica_b.get_thread_id("slack", "C1") is None + + await replica_a.set_thread_id("slack", "C1", "thread-from-a", user_id="u1") + assert await replica_b.get_thread_id("slack", "C1") == "thread-from-a" + + await replica_b.set_thread_id("slack", "C2", "thread-from-b") + assert await replica_a.get_thread_id("slack", "C1") == "thread-from-a" + assert await replica_a.get_thread_id("slack", "C2") == "thread-from-b" + assert {entry["chat_id"] for entry in await replica_a.list_entries()} == {"C1", "C2"} + + assert await replica_b.remove("slack", "C1") is True + assert await replica_a.get_thread_id("slack", "C1") is None + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_concurrent_set_from_two_sql_stores_keeps_one_row_per_key(tmp_path): + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + stores = [_sql_store(engine) for _ in range(2)] + await asyncio.gather(*(store.set_thread_id("slack", "C1", f"thread-{i}", topic_id="t") for i, store in enumerate(stores))) + entries = await stores[0].list_entries() + assert len(entries) == 1 + assert entries[0]["thread_id"] in {"thread-0", "thread-1"} + finally: + await engine.dispose() + + +# ── one-time import of channels/store.json ────────────────────────────────── + + +async def test_import_moves_every_legacy_entry_into_the_table_and_renames_the_file(tmp_path, caplog): + path = tmp_path / "channels" / "store.json" + _write_legacy( + path, + { + "slack:C1": _legacy_entry("thread-1", user_id="U1", created_at=1_700_000_000.0, updated_at=1_700_000_100.5), + "feishu:oc_1:om_1": _legacy_entry("thread-2", user_id="ou_1"), + "github:owner/repo:42:coder": _legacy_entry("thread-3", user_id=""), + }, + ) + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + store = _sql_store(engine, legacy_path=path) + assert store.channels_dir == path.parent + with caplog.at_level(logging.INFO, logger="app.channels.store"): + assert await store.import_legacy_json() == 3 + assert "3" in caplog.text and "store.json" in caplog.text + + assert await store.get_thread_id("slack", "C1") == "thread-1" + assert await store.get_thread_id("feishu", "oc_1", topic_id="om_1") == "thread-2" + assert await store.get_thread_id("github", "owner/repo", topic_id="42:coder") == "thread-3" + entries = {binding_key(e["channel_name"], e["chat_id"], e.get("topic_id")): e for e in await store.list_entries()} + assert entries["slack:C1"]["user_id"] == "U1" + assert entries["slack:C1"]["created_at"] == 1_700_000_000.0 + assert entries["slack:C1"]["updated_at"] == 1_700_000_100.5 # lossless: the stamps are copied, not regenerated + assert entries["feishu:oc_1:om_1"]["topic_id"] == "om_1" + assert entries["github:owner/repo:42:coder"]["chat_id"] == "owner/repo" + + assert not path.exists() + migrated = path.with_name("store.json.migrated") + assert set(json.loads(migrated.read_text(encoding="utf-8"))) == {"slack:C1", "feishu:oc_1:om_1", "github:owner/repo:42:coder"} + + # A second start finds no file: nothing to do. + assert await store.import_legacy_json() == 0 + assert len(await store.list_entries()) == 3 + finally: + await engine.dispose() + + +async def test_import_leaves_the_file_alone_when_the_table_is_already_populated(tmp_path, caplog): + path = tmp_path / "channels" / "store.json" + _write_legacy(path, {"slack:C1": _legacy_entry("thread-from-file")}) + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + store = _sql_store(engine, legacy_path=path) + await store.set_thread_id("slack", "C9", "thread-from-db") + with caplog.at_level(logging.INFO, logger="app.channels.store"): + assert await store.import_legacy_json() == 0 + assert path.exists() # kept for the operator; a populated table is the source of truth + assert not path.with_name("store.json.migrated").exists() + assert await store.get_thread_id("slack", "C1") is None + assert "already" in caplog.text + finally: + await engine.dispose() + + +async def test_import_without_a_file_or_without_a_legacy_path_is_a_no_op(tmp_path): + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + assert await _sql_store(engine, legacy_path=tmp_path / "missing" / "store.json").import_legacy_json() == 0 + assert await _sql_store(engine).import_legacy_json() == 0 + assert await _sql_store(engine).list_entries() == [] + finally: + await engine.dispose() + + +async def test_import_keeps_a_corrupt_or_malformed_file_and_skips_bad_entries(tmp_path, caplog): + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + corrupt = tmp_path / "corrupt" / "store.json" + corrupt.parent.mkdir(parents=True) + corrupt.write_text("not json", encoding="utf-8") + with caplog.at_level(logging.WARNING, logger="app.channels.store"): + assert await _sql_store(engine, legacy_path=corrupt).import_legacy_json() == 0 + assert corrupt.exists() + # Import-path wording: nothing "starts fresh" here, the file is kept and the import skipped. + assert "Corrupt legacy channel store at" in caplog.text and "skipping the import (file kept)" in caplog.text + assert "starting fresh" not in caplog.text + + partial = tmp_path / "partial" / "store.json" + _write_legacy( + partial, + { + "slack:C1": _legacy_entry("good"), + "slack:C2": {"user_id": "no-thread"}, # no thread_id: skipped + "slack:C3": "not-a-dict", # skipped + "slack:C4": {"thread_id": "stampless"}, # missing stamps and user_id get defaults + }, + ) + store = _sql_store(engine, legacy_path=partial) + with caplog.at_level(logging.WARNING, logger="app.channels.store"): + assert await store.import_legacy_json() == 2 + assert "Skipping 2 malformed or oversized entries" in caplog.text + assert await store.get_thread_id("slack", "C1") == "good" + assert await store.get_thread_id("slack", "C2") is None + assert await store.get_thread_id("slack", "C3") is None + (c4,) = [e for e in await store.list_entries() if e["chat_id"] == "C4"] + assert c4["user_id"] == "" and c4["created_at"] == pytest.approx(time.time(), abs=60) and c4["updated_at"] == c4["created_at"] + assert not partial.exists() and partial.with_name("store.json.migrated").exists() + finally: + await engine.dispose() + + +async def test_import_skips_oversized_components_instead_of_failing_the_whole_import(tmp_path, caplog): + """A key component longer than its column would make PostgreSQL reject the batch + (``value too long for type character varying``) and take every IM channel down + with the failed ``ChannelService.start()``; SQLite would silently accept it, so + the two backends diverged on the same file. Oversized entries are skipped and + counted with the malformed ones; the rest import and the file is still renamed.""" + path = tmp_path / "channels" / "store.json" + _write_legacy( + path, + { + "slack:C1": _legacy_entry("thread-1"), + f"slack:{'c' * (CHAT_ID_LENGTH + 1)}": _legacy_entry("thread-long-chat"), + f"slack:C2:{'t' * (TOPIC_ID_LENGTH + 1)}": _legacy_entry("thread-long-topic"), + "slack:C3": _legacy_entry("thread-3"), + f"{'x' * (CHANNEL_NAME_LENGTH + 1)}:C4": _legacy_entry("thread-long-channel"), + "slack:C5": _legacy_entry("u" * (USER_ID_LENGTH + 1), user_id="u" * (USER_ID_LENGTH + 1)), + }, + ) + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + store = _sql_store(engine, legacy_path=path) + with caplog.at_level(logging.WARNING, logger="app.channels.store"): + assert await store.import_legacy_json() == 2 + assert "Skipping 4 malformed or oversized entries" in caplog.text + assert await store.get_thread_id("slack", "C1") == "thread-1" + assert await store.get_thread_id("slack", "C3") == "thread-3" + assert await store.get_thread_id("slack", "c" * (CHAT_ID_LENGTH + 1)) is None + assert await store.get_thread_id("slack", "C2", topic_id="t" * (TOPIC_ID_LENGTH + 1)) is None + assert len(await store.list_entries()) == 2 + assert not path.exists() and path.with_name("store.json.migrated").exists() + finally: + await engine.dispose() + + +async def test_sqlite_accepts_an_oversized_row_so_the_import_validator_is_the_only_guard(tmp_path): + """Why the validator matters: SQLite ignores ``String(n)``, so without it the same + file imports cleanly on SQLite and fails on PostgreSQL.""" + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + repo = SqlChannelThreadBindingRepository(async_sessionmaker(engine, expire_on_commit=False)) + oversized = ChannelThreadBinding.build("slack", "c" * (CHAT_ID_LENGTH + 1), "thread-x", now=1.0) + assert await repo.insert_missing([oversized]) == 1 + assert await repo.get_thread_id(oversized.key) == "thread-x" + finally: + await engine.dispose() + + +async def test_concurrent_import_from_two_replicas_yields_no_duplicates_and_tolerates_the_lost_rename(tmp_path, caplog): + path = tmp_path / "channels" / "store.json" + _write_legacy(path, {f"slack:C{i}": _legacy_entry(f"thread-{i}") for i in range(25)}) + engine_a = await _sqlite_engine(tmp_path / "bindings.db") + engine_b = create_async_engine(f"sqlite+aiosqlite:///{(tmp_path / 'bindings.db').as_posix()}") + try: + replica_a, replica_b = _sql_store(engine_a, legacy_path=path), _sql_store(engine_b, legacy_path=path) + with caplog.at_level(logging.INFO, logger="app.channels.store"): + counts = await asyncio.gather(replica_a.import_legacy_json(), replica_b.import_legacy_json()) + assert sum(counts) == 25 # every row inserted exactly once between the two + assert len(await replica_a.list_entries()) == 25 + assert not path.exists() + assert path.with_name("store.json.migrated").exists() + assert "ERROR" not in caplog.text + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_repository_insert_missing_never_duplicates_or_overwrites(tmp_path): + """The import primitive: ``INSERT ... ON CONFLICT DO NOTHING`` in batches, reporting only what it inserted.""" + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + repo = SqlChannelThreadBindingRepository(async_sessionmaker(engine, expire_on_commit=False)) + store = _sql_store(engine) + await store.set_thread_id("slack", "C1", "live-thread", user_id="live") + rows = [ChannelThreadBinding.build("slack", f"C{i}", f"thread-{i}", user_id="imported", now=1.0) for i in range(1, 1201)] + assert await repo.insert_missing(rows) == 1199 + assert await repo.insert_missing(rows) == 0 + assert await repo.count() == 1200 + assert await store.get_thread_id("slack", "C1") == "live-thread" # DO NOTHING: the live row wins + finally: + await engine.dispose() + + +# ── selection ─────────────────────────────────────────────────────────────── + + +class _Config: + def __init__(self, backend: str) -> None: + self.database = type("Database", (), {"backend": backend})() + + +async def test_resolve_channel_store_picks_json_for_memory_and_sql_for_a_database(tmp_path, caplog): + path = tmp_path / "store.json" + assert isinstance(resolve_channel_store(None, path=path), JsonChannelStore) + assert isinstance(resolve_channel_store(_Config("memory"), session_factory=object(), path=path), JsonChannelStore) + for backend in ("sqlite", "postgres"): + store = resolve_channel_store(_Config(backend), session_factory=object(), path=path) + assert isinstance(store, SqlChannelStore) + assert store.channels_dir == path.parent + with caplog.at_level(logging.WARNING, logger="app.channels.store"): + fallback = resolve_channel_store(_Config("sqlite"), session_factory=None, path=path) + assert isinstance(fallback, JsonChannelStore) + assert "no persistence engine" in caplog.text + + +async def test_resolve_channel_store_uses_the_process_engine_by_default(tmp_path, monkeypatch): + from deerflow.persistence import engine as engine_module + + path = tmp_path / "store.json" + monkeypatch.setattr(engine_module, "get_session_factory", lambda: object()) + assert isinstance(resolve_channel_store(_Config("postgres"), path=path), SqlChannelStore) + monkeypatch.setattr(engine_module, "get_session_factory", lambda: None) + assert isinstance(resolve_channel_store(_Config("postgres"), path=path), JsonChannelStore) + + +# ── ChannelService wiring ─────────────────────────────────────────────────── + + +async def test_channel_service_start_imports_the_legacy_file_once_for_the_sql_store(tmp_path): + from app.channels.service import ChannelService + + path = tmp_path / "channels" / "store.json" + _write_legacy(path, {"slack:C1": _legacy_entry("thread-1", user_id="U1")}) + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + service = ChannelService(channels_config={}, store=_sql_store(engine, legacy_path=path)) + assert service.manager.store is service.store + await service.start() + try: + assert await service.store.get_thread_id("slack", "C1") == "thread-1" + assert not path.exists() and path.with_name("store.json.migrated").exists() + finally: + await service.stop() + # A restart of the service (or a peer starting later) imports nothing more. + _write_legacy(path, {"slack:C2": _legacy_entry("thread-2")}) + service = ChannelService(channels_config={}, store=_sql_store(engine, legacy_path=path)) + await service.start() + try: + assert await service.store.get_thread_id("slack", "C2") is None + assert path.exists() + finally: + await service.stop() + finally: + await engine.dispose() + + +async def test_channel_service_stop_during_the_legacy_import_invalidates_that_start(tmp_path): + """The import runs under the start lock after the generation check: a ``stop()`` that lands + while it is in flight bumps the generation, so the start returns without starting the manager + (and a later ``start()`` imports nothing more because the table is populated).""" + from unittest.mock import AsyncMock + + from app.channels.service import ChannelService + + path = tmp_path / "channels" / "store.json" + _write_legacy(path, {"slack:C1": _legacy_entry("thread-1")}) + engine = await _sqlite_engine(tmp_path / "bindings.db") + try: + store = _sql_store(engine, legacy_path=path) + import_entered = asyncio.Event() + release_import = asyncio.Event() + real_import = store.import_legacy_json + + async def gated_import(): + import_entered.set() + await release_import.wait() + return await real_import() + + store.import_legacy_json = gated_import # type: ignore[method-assign] + service = ChannelService(channels_config={}, store=store) + manager_start = AsyncMock(wraps=service.manager.start) + service.manager.start = manager_start # type: ignore[method-assign] + + start_task = asyncio.create_task(service.start()) + await asyncio.wait_for(import_entered.wait(), timeout=1) + await service.stop() # lands during the import: bumps the shutdown generation + release_import.set() + await asyncio.wait_for(start_task, timeout=5) + + manager_start.assert_not_awaited() # the invalidated start never started the manager + assert service._running is False + assert await store.get_thread_id("slack", "C1") == "thread-1" # the import itself completed + assert not path.exists() + finally: + await engine.dispose() + + +async def test_channel_service_start_leaves_a_json_store_untouched(tmp_path): + from app.channels.service import ChannelService + + path = tmp_path / "channels" / "store.json" + _write_legacy(path, {"slack:C1": _legacy_entry("thread-1")}) + service = ChannelService(channels_config={}, store=JsonChannelStore(path)) + await service.start() + try: + assert await service.store.get_thread_id("slack", "C1") == "thread-1" + assert path.exists() + finally: + await service.stop() + + +async def test_channel_service_defaults_to_the_json_store_without_a_database(tmp_path, monkeypatch): + from app.channels import store as store_module + from app.channels.service import ChannelService + + monkeypatch.setattr(store_module, "default_store_path", lambda: tmp_path / "channels" / "store.json") + service = ChannelService(channels_config={}) + assert isinstance(service.store, JsonChannelStore) + assert service.store.path == tmp_path / "channels" / "store.json" + assert isinstance(ChannelService(channels_config={}, app_config=_Config("memory")).store, JsonChannelStore) + + +# ── PostgreSQL (opt-in) ───────────────────────────────────────────────────── + + +async def test_sql_store_round_trips_on_postgres(tmp_path): + uri = os.environ.get("TEST_POSTGRES_URI") + if not uri: + pytest.skip("requires TEST_POSTGRES_URI (real Postgres channel_thread_bindings)") + schema = f"ctb_{uuid.uuid4().hex}" + engine = create_async_engine(asyncpg_test_url(uri), connect_args=build_asyncpg_connect_args(schema)) + try: + async with engine.begin() as conn: + await conn.execute(sa.text(f'CREATE SCHEMA "{schema}"')) + await conn.run_sync(Base.metadata.create_all, tables=[ChannelThreadBindingRow.__table__]) + path = tmp_path / "store.json" + _write_legacy(path, {"slack:C0": _legacy_entry("imported")}) + store = _sql_store(engine, legacy_path=path) + assert await store.import_legacy_json() == 1 + await store.set_thread_id("slack", "C1", "t1", user_id="u1") + await store.set_thread_id("slack", "C1", "t2", user_id="u2") + await store.set_thread_id("slack", "C1", "topic", topic_id="x") + assert await store.get_thread_id("slack", "C0") == "imported" + assert await store.get_thread_id("slack", "C1") == "t2" + assert len(await store.list_entries(channel_name="slack")) == 3 + assert await store.remove("slack", "C1") is True + assert await store.list_entries() == [{"channel_name": "slack", "chat_id": "C0", "thread_id": "imported", "user_id": "u", "created_at": 1_700_000_000.0, "updated_at": 1_700_000_000.0}] + finally: + async with engine.begin() as conn: + await conn.execute(sa.text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + await engine.dispose() diff --git a/backend/tests/test_channels.py b/backend/tests/test_channels.py index 84381adb44..8690c24f3c 100644 --- a/backend/tests/test_channels.py +++ b/backend/tests/test_channels.py @@ -1,4 +1,4 @@ -"""Tests for the IM channel system (MessageBus, ChannelStore, ChannelManager).""" +"""Tests for the IM channel system (MessageBus, ChannelManager, channel adapters).""" from __future__ import annotations @@ -7,7 +7,7 @@ import json import logging import tempfile import threading -from concurrent.futures import Future, ThreadPoolExecutor +from concurrent.futures import Future from pathlib import Path from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch @@ -23,7 +23,7 @@ from app.channels.message_bus import ( OutboundMessage, ResolvedAttachment, ) -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore from deerflow.skills.types import Skill, SkillCategory @@ -76,7 +76,7 @@ def test_channel_policy_explicit_interaction_mode_overrides_legacy_flag(tmp_path previous = CHANNEL_RUN_POLICY.get(channel_name) CHANNEL_RUN_POLICY[channel_name] = ChannelRunPolicy(is_interactive=False, interaction_mode=mode) try: - manager = ChannelManager(bus=MessageBus(), store=ChannelStore(path=tmp_path / "store.json")) + manager = ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=tmp_path / "store.json")) msg = InboundMessage(channel_name=channel_name, chat_id="chat", user_id="user", text="hello") context: dict[str, object] = {} asyncio.run(manager._apply_channel_policy(msg, context)) @@ -106,7 +106,7 @@ def test_channel_policy_legacy_noninteractive_remains_supported(tmp_path): previous = CHANNEL_RUN_POLICY.get(channel_name) CHANNEL_RUN_POLICY[channel_name] = ChannelRunPolicy(is_interactive=False) try: - manager = ChannelManager(bus=MessageBus(), store=ChannelStore(path=tmp_path / "store.json")) + manager = ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=tmp_path / "store.json")) msg = InboundMessage(channel_name=channel_name, chat_id="chat", user_id="user", text="hello") context: dict[str, object] = {} asyncio.run(manager._apply_channel_policy(msg, context)) @@ -289,111 +289,6 @@ class TestMessageBus: assert msg.metadata == {} -# --------------------------------------------------------------------------- -# ChannelStore tests -# --------------------------------------------------------------------------- - - -class TestChannelStore: - @pytest.fixture - def store(self, tmp_path): - return ChannelStore(path=tmp_path / "store.json") - - def test_set_and_get_thread_id(self, store): - store.set_thread_id("slack", "ch1", "thread-abc", user_id="u1") - assert store.get_thread_id("slack", "ch1") == "thread-abc" - - def test_get_nonexistent_returns_none(self, store): - assert store.get_thread_id("slack", "nonexistent") is None - - def test_remove(self, store): - store.set_thread_id("slack", "ch1", "t1") - assert store.remove("slack", "ch1") is True - assert store.get_thread_id("slack", "ch1") is None - - def test_remove_nonexistent_returns_false(self, store): - assert store.remove("slack", "nope") is False - - def test_list_entries_all(self, store): - store.set_thread_id("slack", "ch1", "t1") - store.set_thread_id("feishu", "ch2", "t2") - entries = store.list_entries() - assert len(entries) == 2 - - def test_list_entries_filtered(self, store): - store.set_thread_id("slack", "ch1", "t1") - store.set_thread_id("feishu", "ch2", "t2") - entries = store.list_entries(channel_name="slack") - assert len(entries) == 1 - assert entries[0]["channel_name"] == "slack" - - def test_channel_store_concurrent_list_and_mutation(self, store, monkeypatch): - iteration_started = threading.Event() - mutation_requested = threading.Event() - mutation_finished = threading.Event() - - class CoordinatedData(dict): - def items(self): - iterator = iter(super().items()) - first = next(iterator) - iteration_started.set() - - if store._lock.locked(): - assert mutation_requested.wait(timeout=5), "mutation thread never requested the store lock" - else: - assert mutation_finished.wait(timeout=5), "mutation thread never changed the unlocked store" - - yield first - yield from iterator - - store._data = CoordinatedData( - { - "slack:ch1": {"thread_id": "t1", "user_id": "u1", "created_at": 1.0, "updated_at": 1.0}, - "feishu:ch2": {"thread_id": "t2", "user_id": "u2", "created_at": 2.0, "updated_at": 2.0}, - } - ) - monkeypatch.setattr(store, "_save", lambda: None) - - def mutate(): - assert iteration_started.wait(timeout=5), "list_entries never started iterating" - mutation_requested.set() - store.set_thread_id("test", "new", "t3") - mutation_finished.set() - - with ThreadPoolExecutor(max_workers=2) as executor: - list_future = executor.submit(store.list_entries) - mutation_future = executor.submit(mutate) - mutation_future.result(timeout=5) - entries = list_future.result(timeout=5) - - assert {(entry["channel_name"], entry["chat_id"]) for entry in entries} == {("slack", "ch1"), ("feishu", "ch2")} - - def test_persistence(self, tmp_path): - path = tmp_path / "store.json" - store1 = ChannelStore(path=path) - store1.set_thread_id("slack", "ch1", "t1") - - store2 = ChannelStore(path=path) - assert store2.get_thread_id("slack", "ch1") == "t1" - - def test_update_preserves_created_at(self, store): - store.set_thread_id("slack", "ch1", "t1") - entries = store.list_entries() - created_at = entries[0]["created_at"] - - store.set_thread_id("slack", "ch1", "t2") - entries = store.list_entries() - assert entries[0]["created_at"] == created_at - assert entries[0]["thread_id"] == "t2" - assert entries[0]["updated_at"] >= created_at - - def test_corrupt_file_handled(self, tmp_path): - path = tmp_path / "store.json" - path.write_text("not json", encoding="utf-8") - store = ChannelStore(path=path) - assert store.get_thread_id("x", "y") is None - - # --------------------------------------------------------------------------- # Channel base class tests # --------------------------------------------------------------------------- @@ -804,7 +699,7 @@ class TestChannelManager: from app.channels.manager import ChannelManager bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, langgraph_url="http://localhost:8001") with patch("langgraph_sdk.get_client") as get_client: @@ -832,7 +727,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) created_ids: list[str] = [] @@ -868,7 +763,7 @@ class TestChannelManager: assert tid1 == tid2 == "thread-1" assert created1 is True assert created2 is False - assert store.get_thread_id("slack", "C1") == "thread-1" + assert await store.get_thread_id("slack", "C1") == "thread-1" _run(go()) @@ -885,7 +780,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) first_create_started = asyncio.Event() release_first_with_error = asyncio.Event() @@ -1000,7 +895,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, gateway_url="http://gateway:8001") reply = await manager._fetch_gateway("/api/memory", "memory") @@ -1042,7 +937,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, gateway_url="http://gateway:8001") msg = InboundMessage( channel_name="slack", @@ -1066,7 +961,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -1190,7 +1085,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -1232,7 +1127,7 @@ class TestChannelManager: } # Thread ID should be stored - thread_id = store.get_thread_id("test", "chat1", topic_id="topic1") + thread_id = await store.get_thread_id("test", "chat1", topic_id="topic1") assert thread_id == "test-thread-123" # runs.wait should be called with the thread_id @@ -1261,7 +1156,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -1307,7 +1202,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") + store = JsonChannelStore(path=tmp_path / "store.json") manager = ChannelManager(bus=bus, store=store) manager._client = _make_mock_langgraph_client() outbound_received: list[OutboundMessage] = [] @@ -1500,7 +1395,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") + store = JsonChannelStore(path=tmp_path / "store.json") manager = ChannelManager(bus=bus, store=store) manager._client = _make_mock_langgraph_client() manager._client.runs.stream = MagicMock(side_effect=lambda *a, **kw: _make_async_iterator(_ok_stream_events())) @@ -1555,7 +1450,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") + store = JsonChannelStore(path=tmp_path / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] key_present_during_final_publish: list[bool] = [] @@ -1619,7 +1514,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") + store = JsonChannelStore(path=tmp_path / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -1679,7 +1574,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") + store = JsonChannelStore(path=tmp_path / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -1745,7 +1640,7 @@ class TestChannelManager: """ from app.channels.manager import ChannelManager - manager = ChannelManager(bus=MessageBus(), store=ChannelStore(path=tmp_path / "store.json")) + manager = ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=tmp_path / "store.json")) def _gh(delivery: str, agent: str = "reviewer", owner_user_id: str = "alice") -> InboundMessage: # Shaped exactly as app.gateway.github.dispatcher.fanout_event @@ -1787,7 +1682,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") + store = JsonChannelStore(path=tmp_path / "store.json") manager = ChannelManager(bus=bus, store=store) client = _make_mock_langgraph_client() attempts = {"n": 0} @@ -1837,7 +1732,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -1875,7 +1770,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -1917,7 +1812,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -1944,7 +1839,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -1987,7 +1882,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager( bus=bus, store=store, @@ -2040,7 +1935,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -2160,7 +2055,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager( bus=bus, store=store, @@ -2220,7 +2115,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager( bus=bus, store=store, @@ -2260,7 +2155,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -2331,7 +2226,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -2398,7 +2293,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -2460,7 +2355,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -2514,7 +2409,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -2565,7 +2460,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -2659,7 +2554,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) msg = InboundMessage( @@ -2713,7 +2608,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) first_started = asyncio.Event() @@ -2791,7 +2686,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -2824,7 +2719,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) mock_client = _make_mock_langgraph_client() @@ -2859,7 +2754,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) mock_client = _make_mock_langgraph_client() @@ -2894,7 +2789,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) mock_client = _make_mock_langgraph_client(thread_id="new-thread-456") @@ -2920,7 +2815,7 @@ class TestChannelManager: await manager.stop() mock_client.threads.create.assert_not_called() - assert store.get_thread_id("test", "chat1") is None + assert await store.get_thread_id("test", "chat1") is None assert outbound_received[0].text.startswith("Unknown command: /new.") _run(go()) @@ -2930,10 +2825,10 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) - store.set_thread_id("test", "chat1", "base-thread") - store.set_thread_id("test", "chat1", "topic-thread", topic_id="topic-1") + await store.set_thread_id("test", "chat1", "base-thread") + await store.set_thread_id("test", "chat1", "topic-thread", topic_id="topic-1") outbound_received = [] @@ -2965,7 +2860,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) manager._skill_storage = _make_channel_skill_storage([_make_channel_skill(tmp_path, "data-analysis")]) @@ -3016,7 +2911,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) manager._skill_storage = _make_channel_skill_storage([_make_channel_skill(tmp_path, "data-analysis")]) @@ -3073,7 +2968,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) manager._skill_storage = _make_channel_skill_storage([_make_channel_skill(tmp_path, "data-analysis")]) @@ -3125,7 +3020,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) manager._skill_storage = _make_channel_skill_storage([_make_channel_skill(tmp_path, "data-analysis")]) @@ -3163,7 +3058,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager( bus=bus, store=store, @@ -3219,7 +3114,7 @@ class TestChannelManager: monkeypatch.setattr("app.channels.manager.load_agent_config", spy_load_agent_config) bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, default_session={"assistant_id": "analyst-agent"}) # A bound connection: the owner resolves to a real, non-default bucket. @@ -3262,8 +3157,8 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") - store.set_thread_id("test", "chat1", "legacy-thread", user_id="platform-user") + store = JsonChannelStore(path=tmp_path / "store.json") + await store.set_thread_id("test", "chat1", "legacy-thread", user_id="platform-user") manager = ChannelManager(bus=bus, store=store, connection_repo=EmptyConnectionRepo()) manager._remember_thread_agent("legacy-thread", "frontend-only") manager._skill_storage = _make_channel_skill_storage([_make_channel_skill(tmp_path, "data-analysis")]) @@ -3304,7 +3199,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) manager._skill_storage = _make_channel_skill_storage([_make_channel_skill(tmp_path, "data-analysis", enabled=False)]) @@ -3340,7 +3235,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) manager._skill_storage = _make_channel_skill_storage([_make_channel_skill(tmp_path, "frontend-design")]) @@ -3381,10 +3276,10 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) - store.set_thread_id("test", "chat1", "base-thread") - store.set_thread_id("test", "chat1", "topic-thread", topic_id="topic-1") + await store.set_thread_id("test", "chat1", "base-thread") + await store.set_thread_id("test", "chat1", "topic-thread", topic_id="topic-1") mock_client = _make_mock_langgraph_client() manager._client = mock_client @@ -3420,10 +3315,10 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) - store.set_thread_id("test", "chat1", "old-thread") + await store.set_thread_id("test", "chat1", "old-thread") mock_client = _make_mock_langgraph_client(thread_id="new-thread-456") manager._client = mock_client @@ -3447,7 +3342,7 @@ class TestChannelManager: await _wait_for(lambda: len(outbound_received) >= 1) await manager.stop() - new_thread = store.get_thread_id("test", "chat1") + new_thread = await store.get_thread_id("test", "chat1") assert new_thread == "new-thread-456" assert new_thread != "old-thread" assert "New conversation started" in outbound_received[0].text @@ -3473,7 +3368,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -3511,8 +3406,8 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "old-thread") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "old-thread") manager = ChannelManager(bus=bus, store=store) mock_client = _make_mock_langgraph_client(thread_id="research-thread") manager._client = mock_client @@ -3528,7 +3423,7 @@ class TestChannelManager: reply = await manager._handle_agent_command(msg, "use Researcher") assert loaded == [("researcher", "deerflow-user-1")] - assert store.get_thread_id("test", "chat1") == "research-thread" + assert await store.get_thread_id("test", "chat1") == "research-thread" create_kwargs = mock_client.threads.create.call_args.kwargs assert create_kwargs["metadata"]["channel_agent_name"] == "researcher" assert create_kwargs["metadata"]["agent_name"] == "researcher" @@ -3559,7 +3454,7 @@ class TestChannelManager: async def go(): manager = ChannelManager( bus=MessageBus(), - store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), + store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), channel_sessions={ "test": { "config": {config_carrier: {"agent_name": "configured-writer"}}, @@ -3599,7 +3494,7 @@ class TestChannelManager: bus = MessageBus() manager = ChannelManager( bus=bus, - store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), + store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), channel_sessions={ "test": { "assistant_id": "configured-writer", @@ -3633,7 +3528,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager( bus=bus, store=store, @@ -3669,7 +3564,7 @@ class TestChannelManager: async def go(): manager = ChannelManager( bus=MessageBus(), - store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), + store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), channel_sessions={ "test": { "config": {config_carrier: {"agent_name": "configured-writer"}}, @@ -3708,8 +3603,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a", "model-b"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") manager._client = mock_client @@ -3749,8 +3644,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a", "model-b"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -3787,8 +3682,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") manager._client = mock_client @@ -3813,8 +3708,8 @@ class TestChannelManager: from app.channels.manager import CHANNEL_MODEL_METADATA_KEY, ChannelManager async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) manager._thread_model_names["thread-1"] = "model-b" mock_client = _make_mock_langgraph_client(thread_id="thread-1") @@ -3843,7 +3738,7 @@ class TestChannelManager: async def go(): manager = ChannelManager( bus=MessageBus(), - store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), + store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), ) mock_client = _make_mock_langgraph_client() manager._client = mock_client @@ -3867,8 +3762,8 @@ class TestChannelManager: from app.channels.manager import ChannelManager async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") mock_client.threads.get = AsyncMock(return_value={"thread_id": "thread-1", "metadata": {"channel_model_name": "model-b"}}) @@ -3896,8 +3791,8 @@ class TestChannelManager: from app.channels.manager import ChannelManager async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -3925,7 +3820,7 @@ class TestChannelManager: # No thread, no session config -> server default. manager2 = ChannelManager( bus=MessageBus(), - store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), + store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json"), ) reply = await manager2._handle_model_command(msg, "") assert reply == "Current model: server default. Use /models to list available models." @@ -3938,8 +3833,8 @@ class TestChannelManager: from app.channels.manager import ChannelManager async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -3966,8 +3861,8 @@ class TestChannelManager: from app.channels.manager import ChannelManager async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -4012,8 +3907,8 @@ class TestChannelManager: monkeypatch.setattr("app.channels.manager.IN_PROCESS_CACHE_MAX_ENTRIES", 1) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") manager._client = mock_client @@ -4067,8 +3962,8 @@ class TestChannelManager: monkeypatch.setattr("app.channels.manager.load_agent_config", fake_load_agent_config) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -4102,8 +3997,8 @@ class TestChannelManager: monkeypatch.setattr("app.channels.manager.load_agent_config", fake_load_agent_config) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -4137,8 +4032,8 @@ class TestChannelManager: monkeypatch.setattr("app.channels.manager.load_agent_config", fake_load_agent_config) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -4166,8 +4061,8 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "test-thread-123", topic_id="topic1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "test-thread-123", topic_id="topic1") manager = ChannelManager(bus=bus, store=store) manager._client = _make_mock_langgraph_client() @@ -4207,8 +4102,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a", "model-b"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") @@ -4282,8 +4177,8 @@ class TestChannelManager: from deerflow.agents.lead_agent.agent import _get_runtime_config async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -4318,8 +4213,8 @@ class TestChannelManager: from deerflow.agents.lead_agent.agent import _get_runtime_config async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager( bus=MessageBus(), store=store, @@ -4368,8 +4263,8 @@ class TestChannelManager: ) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store, channel_sessions={"test": {}}) mock_client = _make_mock_langgraph_client(thread_id="thread-1") mock_client.threads.get = AsyncMock(return_value={"thread_id": "thread-1", "metadata": {"channel_agent_name": "coder"}}) @@ -4407,8 +4302,8 @@ class TestChannelManager: ) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") mock_client.threads.get = AsyncMock(return_value={"thread_id": "thread-1", "metadata": {"channel_agent_name": "coder"}}) @@ -4444,8 +4339,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["new-model"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) get_started = asyncio.Event() release_get = asyncio.Event() @@ -4493,8 +4388,8 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=bus, store=store, gateway_url="http://gateway:8001") manager._thread_model_names["thread-1"] = "model-b" manager._client = _make_mock_langgraph_client(thread_id="thread-1") @@ -4524,8 +4419,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) manager._remember_thread_model("thread-1", "gone-model") mock_client = _make_mock_langgraph_client(thread_id="thread-1") @@ -4558,8 +4453,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) # The replacement selection is current; the failed run used gone-model. manager._remember_thread_model("thread-1", "model-b") @@ -4597,8 +4492,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) manager._remember_thread_model("thread-1", "gone-model") mock_client = _make_mock_langgraph_client(thread_id="thread-1") @@ -4641,8 +4536,8 @@ class TestChannelManager: _mock_gateway_models(monkeypatch, ["model-a"]) async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) manager._client = _make_mock_langgraph_client(thread_id="thread-1") msg = InboundMessage( @@ -4679,8 +4574,8 @@ class TestChannelManager: bus = MessageBus() outbounds: list[OutboundMessage] = [] bus.subscribe_outbound(outbounds.append) - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=bus, store=store) manager._remember_thread_model("thread-1", "gone-model") mock_client = _make_mock_langgraph_client(thread_id="thread-1") @@ -4729,8 +4624,8 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=bus, store=store, gateway_url="http://gateway:8001") manager._thread_model_names["thread-1"] = "model-b" manager._client = _make_mock_langgraph_client(thread_id="thread-1") @@ -4768,8 +4663,8 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=bus, store=store, gateway_url="http://gateway:8001") manager._client = _make_mock_langgraph_client(thread_id="thread-1") outbound_received = [] @@ -4798,8 +4693,8 @@ class TestChannelManager: from app.channels.manager import ChannelManager async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") manager._client = mock_client @@ -4824,8 +4719,8 @@ class TestChannelManager: from app.channels.manager import ChannelManager async def go(): - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("test", "chat1", "thread-1") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("test", "chat1", "thread-1") manager = ChannelManager(bus=MessageBus(), store=store) mock_client = _make_mock_langgraph_client(thread_id="thread-1") mock_client.threads.get = AsyncMock(side_effect=RuntimeError("gateway down")) @@ -4851,7 +4746,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) # Return a different thread_id for each create call @@ -4905,7 +4800,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) mock_client = _make_mock_langgraph_client(thread_id="topic-thread-1") @@ -4960,7 +4855,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) mock_client = _make_mock_langgraph_client(thread_id="private-thread-1") @@ -5004,7 +4899,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) thread_ids = iter(["thread-A", "thread-B"]) @@ -5048,7 +4943,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -5098,7 +4993,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -5141,7 +5036,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -5204,7 +5099,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -5232,7 +5127,7 @@ class TestChannelManager: # A thread should be created mock_client.threads.create.assert_called_once() - assert store.get_thread_id("test", "chat1") == "bootstrap-thread" + assert await store.get_thread_id("test", "chat1") == "bootstrap-thread" _run(go()) @@ -5242,7 +5137,7 @@ class TestChannelManager: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received = [] @@ -5278,7 +5173,7 @@ class TestResolveRunParamsUserId: from app.channels.manager import ChannelManager bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") return ChannelManager(bus=bus, store=store) def test_safe_user_id_is_passed_through(self, monkeypatch): @@ -5617,7 +5512,7 @@ class TestGithubFireAndForget: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") # GitHub deliveries skip the bound-identity gate (authenticity is # enforced at the webhook route by HMAC), but constructing the # manager with the default require_bound_identity=False keeps the @@ -5668,7 +5563,7 @@ class TestGithubFireAndForget: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -5723,7 +5618,7 @@ class TestGithubFireAndForget: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -5778,7 +5673,7 @@ class TestGithubFireAndForget: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) mock_client = _make_mock_langgraph_client(thread_id="slack-thread-1") @@ -5801,7 +5696,7 @@ class TestGithubFireAndForget: _run(go()) -def _stale_pin_drain_setup(thread_id, *, model, pin=None, session_model=None): +async def _stale_pin_drain_setup(thread_id, *, model, pin=None, session_model=None): """Assemble the shared drain-recovery fixture: a manager with one buffered follow-up whose ``runs.create`` rejects *model* with an allowlist 400. @@ -5811,8 +5706,8 @@ def _stale_pin_drain_setup(thread_id, *, model, pin=None, session_model=None): from app.channels.manager import ChannelManager bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("github", "zhfeng/llm-gateway", thread_id) + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("github", "zhfeng/llm-gateway", thread_id) manager = ChannelManager(bus=bus, store=store) if pin is not None: manager._remember_thread_model(thread_id, pin) @@ -5898,7 +5793,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -5964,7 +5859,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] @@ -6010,7 +5905,7 @@ class TestGithubFollowupBuffer: not be buffered twice.""" from app.channels.manager import ChannelManager - manager = ChannelManager(bus=MessageBus(), store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")) + manager = ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")) thread_id = "gh-thread-dedup" msg = InboundMessage( channel_name="github", @@ -6032,7 +5927,7 @@ class TestGithubFollowupBuffer: deep enough in the backlog to hit the cap.""" from app.channels.manager import FOLLOWUP_BUFFER_MAX_PER_THREAD, ChannelManager - manager = ChannelManager(bus=MessageBus(), store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")) + manager = ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")) thread_id = "gh-thread-overflow" with caplog.at_level(logging.WARNING): @@ -6063,7 +5958,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) carrier_msg = InboundMessage( @@ -6117,7 +6012,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) carrier_msg = InboundMessage( @@ -6155,7 +6050,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) carrier_msg = InboundMessage( @@ -6187,7 +6082,7 @@ class TestGithubFollowupBuffer: async def go(): thread_id = "gh-thread-stale-pin" - manager, carrier_msg, mock_client, bus = _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") + manager, carrier_msg, mock_client, bus = await _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") mock_client.threads.update = AsyncMock(return_value={"thread_id": thread_id}) outbounds = [] @@ -6226,7 +6121,7 @@ class TestGithubFollowupBuffer: async def go(): thread_id = "gh-thread-replaced" - manager, carrier_msg, mock_client, _bus = _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") + manager, carrier_msg, mock_client, _bus = await _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") mock_client.threads.update = AsyncMock() async def reject_after_replacement(*args, **kwargs): @@ -6262,7 +6157,7 @@ class TestGithubFollowupBuffer: async def go(): thread_id = "gh-thread-lookup-boom" - manager, carrier_msg, mock_client, _bus = _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") + manager, carrier_msg, mock_client, _bus = await _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") mock_client.threads.update = AsyncMock(return_value={"thread_id": thread_id}) async def _boom(self, msg): @@ -6288,7 +6183,7 @@ class TestGithubFollowupBuffer: async def go(): thread_id = "gh-thread-persist-fails" - manager, carrier_msg, mock_client, _bus = _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") + manager, carrier_msg, mock_client, _bus = await _stale_pin_drain_setup(thread_id, model="gone-model", pin="gone-model") mock_client.threads.update = AsyncMock(side_effect=RuntimeError("store down")) with caplog.at_level(logging.INFO): @@ -6310,7 +6205,7 @@ class TestGithubFollowupBuffer: async def go(): thread_id = "gh-thread-session-cfg" - manager, carrier_msg, mock_client, _bus = _stale_pin_drain_setup(thread_id, model="cfg-model", session_model="cfg-model") + manager, carrier_msg, mock_client, _bus = await _stale_pin_drain_setup(thread_id, model="cfg-model", session_model="cfg-model") mock_client.threads.get = AsyncMock(return_value={"thread_id": thread_id, "metadata": {}}) mock_client.threads.update = AsyncMock() @@ -6334,7 +6229,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) carrier_msg = InboundMessage( @@ -6374,7 +6269,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) carrier_msg = InboundMessage( @@ -6418,7 +6313,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") bridge = MemoryStreamBridge() manager = ChannelManager(bus=bus, store=store, get_stream_bridge=lambda: bridge) @@ -6492,7 +6387,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") bridge = MemoryStreamBridge() manager = ChannelManager(bus=bus, store=store, get_stream_bridge=lambda: bridge) @@ -6558,7 +6453,7 @@ class TestGithubFollowupBuffer: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) carrier_msg = InboundMessage( @@ -6688,7 +6583,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, require_bound_identity=True) mock_client = _make_mock_langgraph_client() manager._client = mock_client @@ -6725,7 +6620,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, require_bound_identity=True) mock_client = _make_mock_langgraph_client() manager._client = mock_client @@ -6763,7 +6658,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, require_bound_identity=True) outbound_received = [] @@ -6801,7 +6696,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") repo = _BoundIdentityRepo( [ { @@ -6844,7 +6739,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") repo = _BoundIdentityRepo( [ { @@ -6894,7 +6789,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") repo = _BoundIdentityRepo( [ { @@ -6943,7 +6838,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, require_bound_identity=True) mock_client = _make_mock_langgraph_client(thread_id="thread-local") manager._client = mock_client @@ -6972,7 +6867,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, require_bound_identity=False) mock_client = _make_mock_langgraph_client(thread_id="thread-legacy") manager._client = mock_client @@ -7001,7 +6896,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, require_bound_identity=True) mock_client = _make_mock_langgraph_client() manager._client = mock_client @@ -7038,7 +6933,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") repo = _BoundIdentityRepo( [ { @@ -7096,7 +6991,7 @@ class TestChannelManagerBoundIdentityPolicy: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, require_bound_identity=True) mock_client = _make_mock_langgraph_client(thread_id="thread-webhook") manager._client = mock_client @@ -7151,7 +7046,7 @@ class TestChannelManagerConnectionRouting: ) bus = MessageBus() - store = ChannelStore(path=tmp_path / "legacy-store.json") + store = JsonChannelStore(path=tmp_path / "legacy-store.json") manager = ChannelManager(bus=bus, store=store, connection_repo=repo) mock_client = _make_mock_langgraph_client() mock_client.threads.create = AsyncMock( @@ -7189,7 +7084,7 @@ class TestChannelManagerConnectionRouting: assert await repo.get_thread_id(alice["id"], "C-shared", "1710000000.000100") == "thread-alice" assert await repo.get_thread_id(bob["id"], "C-shared", "1710000000.000100") == "thread-bob" - assert store.list_entries() == [] + assert await store.list_entries() == [] first_context = mock_client.runs.wait.call_args_list[0].kwargs["context"] second_context = mock_client.runs.wait.call_args_list[1].kwargs["context"] @@ -7359,7 +7254,7 @@ class TestHandleChatWithArtifacts: async def go(): bus = MessageBus() - store = ChannelStore(path=tmp_path / "store.json") + store = JsonChannelStore(path=tmp_path / "store.json") manager = ChannelManager(bus=bus, store=store) run_result = { @@ -7406,7 +7301,7 @@ class TestHandleChatWithArtifacts: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) run_result = { @@ -7453,7 +7348,7 @@ class TestHandleChatWithArtifacts: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) run_result = { @@ -7501,7 +7396,7 @@ class TestHandleChatWithArtifacts: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) run_result = { @@ -7545,7 +7440,7 @@ class TestHandleChatWithArtifacts: async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) # Turn 1: produces report.md @@ -7950,7 +7845,7 @@ class TestFeishuChannel: async def go(): bus = MessageBus() bus.publish_inbound = AsyncMock() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") channel = FeishuChannel(bus, config={"channel_store": store}) channel._api_client = MagicMock() @@ -8006,9 +7901,9 @@ class TestFeishuChannel: assert channel._reply_card.await_count == 1 channel._update_card.assert_awaited_once_with("om-running-card", "Hello") assert "om-source-msg" not in channel._running_card_tasks - assert store.get_thread_id("feishu", "chat-1", topic_id="om-source-msg") == "thread-1" - assert store.get_thread_id("feishu", "chat-1", topic_id="om-running-card") == "thread-1" - assert store.get_thread_id("feishu", "chat-1", topic_id="om-root-msg") == "thread-1" + assert await store.get_thread_id("feishu", "chat-1", topic_id="om-source-msg") == "thread-1" + assert await store.get_thread_id("feishu", "chat-1", topic_id="om-running-card") == "thread-1" + assert await store.get_thread_id("feishu", "chat-1", topic_id="om-root-msg") == "thread-1" _run(go()) @@ -13012,7 +12907,7 @@ class TestHandleGoalCommand: from app.channels.manager import ChannelManager bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store, gateway_url="http://gateway:8001") async def _lookup(msg): @@ -13478,7 +13373,7 @@ def test_streaming_chat_never_publishes_hidden_memory_context(monkeypatch): async def go(): bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") manager = ChannelManager(bus=bus, store=store) outbound_received: list[OutboundMessage] = [] diff --git a/backend/tests/test_dingtalk_channel.py b/backend/tests/test_dingtalk_channel.py index 427b3cf742..7c64bf30ff 100644 --- a/backend/tests/test_dingtalk_channel.py +++ b/backend/tests/test_dingtalk_channel.py @@ -408,7 +408,7 @@ class TestOnChatbotMessage: def test_p2p_message_without_sender_is_dropped(self, sender_staff_id): """A P2P chat_id *is* the sender, so an empty one keys every user to one thread. - ``ChannelStore._key`` builds ``f"{channel}:{chat_id}"`` for a topic-less conversation, + ``binding_key`` builds ``f"{channel}:{chat_id}"`` for a topic-less conversation, so publishing this would put every senderless P2P message under the literal ``"dingtalk:"`` — one shared thread, one shared history, across users. """ diff --git a/backend/tests/test_feishu_parser.py b/backend/tests/test_feishu_parser.py index 68f3d8fd00..888237eaad 100644 --- a/backend/tests/test_feishu_parser.py +++ b/backend/tests/test_feishu_parser.py @@ -2,6 +2,8 @@ import asyncio import json import tempfile import threading +import time +from collections import deque from io import BytesIO from pathlib import Path from unittest.mock import AsyncMock, MagicMock @@ -19,7 +21,7 @@ from app.channels.message_bus import ( MessageBus, OutboundMessage, ) -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore from deerflow.uploads.manager import PathTraversalError @@ -574,38 +576,410 @@ def test_feishu_on_message_extracts_image_and_file_keys(): assert channel._pending_inbound_batches == {} -def test_feishu_on_message_reuses_stored_parent_topic_for_card_replies(): - bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id( - "feishu", - "chat_1", - "deer-thread-1", - topic_id="om_clarification_card", - user_id="user_1", - ) - channel = FeishuChannel( - bus, - {"app_id": "test", "app_secret": "test", "channel_store": store}, - ) +async def _on_message_from_lark_thread(channel: FeishuChannel, event) -> None: + """Drive ``_on_message`` from a plain worker thread (no loop of its own) with the Gateway loop running. - event = MagicMock() - event.event.message.chat_id = "chat_1" - event.event.message.message_id = "msg_reply" - event.event.message.root_id = "om_unknown_root" - event.event.message.parent_id = "om_clarification_card" - event.event.message.thread_id = None - event.event.sender.sender_id.open_id = "user_1" - event.event.message.content = json.dumps({"text": "prod"}) + The channel store is async, so ``_on_message`` bridges its lookups to + ``channel._main_loop``; running the callback on the Gateway loop's own + thread would be the deadlock the bridge refuses. + """ + channel._main_loop = asyncio.get_running_loop() + await asyncio.to_thread(channel._on_message, event) + + +async def _on_message_from_sdk_loop(channel: FeishuChannel, event) -> float: + """Drive ``_on_message`` exactly as lark-oapi 1.5.5 does in production. + + ``FeishuChannel._run_ws`` gives the SDK thread its own event loop and + ``ws.Client.start()`` runs it; the SDK's async frame handler then calls + ``_on_message`` synchronously from inside that *running* loop, while the + Gateway loop (``channel._main_loop``) runs in another thread. So + ``asyncio.get_running_loop()`` succeeds inside the callback, but the loop it + returns is not the one the bridge submits to — blocking it briefly is fine + (it is the thread that used to block on the JSON rewrite); only a wait on + the Gateway loop itself would deadlock. + """ + channel._main_loop = asyncio.get_running_loop() + + def sdk_thread() -> float: + sdk_loop = asyncio.new_event_loop() + try: + + async def frame_handler() -> float: + assert asyncio.get_running_loop() is sdk_loop + started = time.monotonic() + channel._on_message(event) # synchronous call from inside the SDK's running loop + return time.monotonic() - started # how long the SDK's loop was blocked + + return sdk_loop.run_until_complete(frame_handler()) + finally: + sdk_loop.close() + + return await asyncio.to_thread(sdk_thread) + + +class _GatedStore: + """A channel store whose database only answers once ``gate`` is set (slow or hung DB).""" + + def __init__(self, inner: JsonChannelStore, gate: asyncio.Event) -> None: + self.inner = inner + self.gate = gate + self.lookups = 0 + self.channels_dir = inner.channels_dir + + async def get_thread_id(self, channel_name, chat_id, topic_id=None): + self.lookups += 1 + await self.gate.wait() + return await self.inner.get_thread_id(channel_name, chat_id, topic_id=topic_id) + + async def set_thread_id(self, channel_name, chat_id, thread_id, *, topic_id=None, user_id=""): + await self.gate.wait() + await self.inner.set_thread_id(channel_name, chat_id, thread_id, topic_id=topic_id, user_id=user_id) + + +async def _eventually(predicate, *, timeout: float = 5.0) -> None: + deadline = time.monotonic() + timeout + while not predicate(): + assert time.monotonic() < deadline, "condition not met in time" + await asyncio.sleep(0.01) + + +def _fast_store_budgets(monkeypatch, *, total: float = 5.0) -> None: + monkeypatch.setattr(feishu_module, "FEISHU_STORE_LOOKUP_TIMEOUT_SECONDS", 0.2) + monkeypatch.setattr(feishu_module, "FEISHU_STORE_BRIDGE_TIMEOUT_SECONDS", 0.3) + monkeypatch.setattr(feishu_module, "FEISHU_STORE_RESOLVE_RETRY_BACKOFF_SECONDS", 0.05) + monkeypatch.setattr(feishu_module, "FEISHU_STORE_RESOLVE_TOTAL_BUDGET_SECONDS", total) + + +def test_feishu_on_message_reuses_stored_parent_topic_for_card_replies(): + async def go(): + bus = MessageBus() + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id( + "feishu", + "chat_1", + "deer-thread-1", + topic_id="om_clarification_card", + user_id="user_1", + ) + channel = FeishuChannel( + bus, + {"app_id": "test", "app_secret": "test", "channel_store": store}, + ) + + event = MagicMock() + event.event.message.chat_id = "chat_1" + event.event.message.message_id = "msg_reply" + event.event.message.root_id = "om_unknown_root" + event.event.message.parent_id = "om_clarification_card" + event.event.message.thread_id = None + event.event.sender.sender_id.open_id = "user_1" + event.event.message.content = json.dumps({"text": "prod"}) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + await _on_message_from_lark_thread(channel, event) + + inbound = mock_make_inbound.return_value + assert inbound.topic_id == "om_clarification_card" + assert mock_make_inbound.call_args.kwargs["metadata"]["topic_id"] == "om_clarification_card" + + _run(go()) + + +def test_feishu_on_message_drops_a_store_backed_message_without_a_running_gateway_loop(caplog): + """No loop to bridge to (direct construction, no ``start()``): the mapping cannot be + consulted, so the message is dropped with a warning — an unresolved lookup is never + routed as a miss onto the unknown root (nothing could dispatch it without the loop anyway).""" + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + bus = MessageBus() + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + event = _make_text_event("prod", message_id="msg_reply", root_id="om_root", parent_id="om_card") with pytest.MonkeyPatch.context() as m: - mock_make_inbound = MagicMock() - m.setattr(channel, "_make_inbound", mock_make_inbound) - channel._on_message(event) + schedule = MagicMock() + batch = MagicMock() + m.setattr(channel, "_schedule_prepare_inbound", schedule) + m.setattr(channel, "_queue_file_inbound_batch", batch) + with caplog.at_level("WARNING", logger="app.channels.feishu"): + channel._on_message(event) - inbound = mock_make_inbound.return_value - assert inbound.topic_id == "om_clarification_card" - assert mock_make_inbound.call_args.kwargs["metadata"]["topic_id"] == "om_clarification_card" + schedule.assert_not_called() + batch.assert_not_called() + assert "main loop not running, cannot resolve_topic_mapping" in caplog.text + assert "dropping message msg_reply" in caplog.text + assert len(bus._inbound_reservations) == 0 # the intake slot taken before routing came back + + +def test_feishu_slow_store_defers_the_reply_until_it_resolves_to_the_mapped_parent_thread(monkeypatch, caplog): + """Thread 4 of the #6558 review: a lookup the database answers late is *unresolved*, not a + miss. The reply (parent card mapped to thread A, root unmapped) is not dispatched until + the store answers, then routes to A; no new conversation, no alias overwrite, and the + SDK's loop is never blocked past the short bridge bound.""" + + async def go(): + _fast_store_budgets(monkeypatch) + inner = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await inner.set_thread_id("feishu", "chat_1", "thread-a", topic_id="om_card", user_id="user_1") + gate = asyncio.Event() + store = _GatedStore(inner, gate) + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test", "channel_store": store}) + dispatched: list = [] + monkeypatch.setattr(channel, "_schedule_prepare_inbound", lambda msg_id, inbound, **kwargs: dispatched.append(inbound)) + event = _make_text_event("answer", message_id="msg_reply", root_id="om_unknown_root", parent_id="om_card") + + with caplog.at_level("WARNING", logger="app.channels.feishu"): + blocked_for = await _on_message_from_sdk_loop(channel, event) + assert blocked_for < feishu_module.FEISHU_STORE_BRIDGE_TIMEOUT_SECONDS + 0.5 # the lark loop is released at the short bound + assert dispatched == [] # unresolved: not routed as a miss + await asyncio.sleep(0.3) + assert dispatched == [] # still waiting for the database, retrying on the Gateway loop + + gate.set() + await _eventually(lambda: len(dispatched) == 1) + + inbound = dispatched[0] + assert inbound.topic_id == "om_card" and inbound.metadata["topic_id"] == "om_card" # thread A, not the unknown root + assert inbound.thread_ts == "msg_reply" + assert await inner.get_thread_id("feishu", "chat_1", topic_id="om_card") == "thread-a" # no alias overwrite + assert "dropping message" not in caplog.text + assert "Gateway loop itself" not in caplog.text + + _run(go()) + + +def test_feishu_store_that_never_answers_drops_the_message_instead_of_opening_a_new_conversation(monkeypatch, caplog): + async def go(): + _fast_store_budgets(monkeypatch, total=0.6) + inner = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await inner.set_thread_id("feishu", "chat_1", "thread-a", topic_id="om_card", user_id="user_1") + store = _GatedStore(inner, asyncio.Event()) # never set + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test", "channel_store": store}) + dispatched: list = [] + monkeypatch.setattr(channel, "_schedule_prepare_inbound", lambda msg_id, inbound, **kwargs: dispatched.append(inbound)) + event = _make_text_event("answer", message_id="msg_reply", root_id="om_unknown_root", parent_id="om_card") + + with caplog.at_level("WARNING", logger="app.channels.feishu"): + await _on_message_from_sdk_loop(channel, event) + await _eventually(lambda: "dropping message msg_reply" in caplog.text) + await asyncio.sleep(0.2) + + assert dispatched == [] # never routed as a miss + assert await inner.get_thread_id("feishu", "chat_1", topic_id="om_card") == "thread-a" + assert store.lookups >= 2 # it kept retrying up to the total budget before giving up + + _run(go()) + + +def _reply_event(msg_id: str): + return _make_text_event("answer", message_id=msg_id, root_id="om_unknown_root", parent_id="om_card") + + +def test_feishu_deferred_burst_is_bounded_by_inbound_queue_maxsize(monkeypatch, caplog): + """#6558 review: deferred messages must hold a bus intake slot. A burst during a store + outage retains exactly ``inbound_queue_maxsize`` messages, drops the rest immediately + with a warning, and returns the capacity once the store answers and the messages are + consumed from the bus.""" + + async def go(): + _fast_store_budgets(monkeypatch, total=30.0) + inner = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await inner.set_thread_id("feishu", "chat_1", "thread-a", topic_id="om_card", user_id="user_1") + gate = asyncio.Event() + store = _GatedStore(inner, gate) + bus = MessageBus(inbound_queue_maxsize=3) + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + channel._add_reaction = AsyncMock() + channel._ensure_running_card_started = MagicMock() + + with caplog.at_level("WARNING", logger="app.channels.feishu"): + for i in range(6): + await _on_message_from_sdk_loop(channel, _reply_event(f"msg_{i}")) + await asyncio.sleep(0.3) + assert bus.inbound_queue.qsize() == 0 # nothing routed as a miss + retained = len(bus._inbound_reservations) + assert retained == 3 # exactly the configured bound is retained (one slot per deferred message) + dropped = [line for line in caplog.text.splitlines() if "dropping message" in line] + assert len(dropped) == 3 and all("intake capacity" in line for line in dropped) + + gate.set() + delivered = [await asyncio.wait_for(bus.get_inbound(), timeout=5) for _ in range(3)] + assert [m.thread_ts for m in delivered] == ["msg_0", "msg_1", "msg_2"] # FIFO, the oldest survivors + assert all(m.topic_id == "om_card" for m in delivered) + await _eventually(lambda: len(bus._inbound_reservations) == 0) + assert bus.inbound_queue.qsize() == 0 + # Capacity is back: the bound can be reserved again in full. + probes = [bus.reserve_inbound(delivered[0]) for _ in range(3)] + for probe in probes: + probe.release() + assert await inner.get_thread_id("feishu", "chat_1", topic_id="om_card") == "thread-a" + + _run(go()) + + +def test_feishu_deferred_message_budget_runs_from_arrival_and_releases_capacity_on_drop(monkeypatch, caplog): + """The retry budget is per message from its arrival, not from when it reaches the queue + head: an expired message is dropped without another lookup and its intake slot is freed.""" + + async def go(): + _fast_store_budgets(monkeypatch, total=0.5) + inner = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = _GatedStore(inner, asyncio.Event()) # never answers + bus = MessageBus(inbound_queue_maxsize=4) + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + channel._main_loop = asyncio.get_running_loop() + parsed = channel._parse_message_event(_reply_event("msg_expired")) + reservation = bus.reserve_inbound(channel._make_inbound(chat_id="chat_1", user_id="user_1", text="x", msg_type=feishu_module.InboundMessageType.CHAT, thread_ts="msg_expired")) + attempt = feishu_module._RoutingAttempt(parsed, reservation=reservation, arrived_at=time.monotonic() - 100.0) + queue = deque([attempt]) + channel._deferred_routing["chat_1"] = queue + lookups_before = store.lookups + + with caplog.at_level("WARNING", logger="app.channels.feishu"): + await channel._drain_deferred_routing("chat_1", queue, None) + + assert store.lookups == lookups_before # expired at the head: no retry spent on it + assert "dropping message msg_expired" in caplog.text + assert len(bus._inbound_reservations) == 0 # the slot came back + assert "chat_1" not in channel._deferred_routing + + _run(go()) + + +def test_feishu_stop_releases_deferred_intake_reservations(monkeypatch, caplog): + async def go(): + _fast_store_budgets(monkeypatch, total=30.0) + inner = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + store = _GatedStore(inner, asyncio.Event()) # never answers + bus = MessageBus(inbound_queue_maxsize=4) + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + await _on_message_from_sdk_loop(channel, _reply_event("msg_0")) + await _on_message_from_sdk_loop(channel, _reply_event("msg_1")) + assert len(bus._inbound_reservations) == 2 + + with caplog.at_level("WARNING", logger="app.channels.feishu"): + await channel._close_and_drain_threadsafe_futures() # what stop() does to in-flight work + assert len(bus._inbound_reservations) == 0 + assert channel._deferred_routing == {} + assert "dropping 2 deferred messages for chat chat_1" in caplog.text or "dropping 1 deferred messages for chat chat_1" in caplog.text + + _run(go()) + + +def test_feishu_deferred_messages_keep_per_chat_fifo_order(monkeypatch): + """A later message for the same chat must not overtake a deferred predecessor.""" + + async def go(): + _fast_store_budgets(monkeypatch) + inner = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await inner.set_thread_id("feishu", "chat_1", "thread-a", topic_id="om_card", user_id="user_1") + gate = asyncio.Event() + store = _GatedStore(inner, gate) + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test", "channel_store": store}) + dispatched: list = [] + monkeypatch.setattr(channel, "_schedule_prepare_inbound", lambda msg_id, inbound, **kwargs: dispatched.append(inbound)) + + await _on_message_from_sdk_loop(channel, _make_text_event("first", message_id="msg_first", root_id="om_unknown_root", parent_id="om_card")) + # Needs no store answer itself (no root/parent/thread), but queues behind the deferred predecessor. + blocked_for = await _on_message_from_sdk_loop(channel, _make_text_event("second", message_id="msg_second")) + assert blocked_for < 0.2 + await asyncio.sleep(0.3) + assert dispatched == [] + + gate.set() + await _eventually(lambda: len(dispatched) == 2) + assert [inbound.thread_ts for inbound in dispatched] == ["msg_first", "msg_second"] + assert dispatched[0].topic_id == "om_card" + assert dispatched[1].topic_id == "msg_second" + + _run(go()) + + +def test_feishu_sdk_loop_callback_still_consults_the_stored_parent_mapping(caplog): + """The production shape (#6558 review): the SDK calls ``_on_message`` from inside its own + running loop. The bridge must still look the parent card up on the Gateway loop — otherwise + a reply whose parent is mapped but whose root is not falls back to the unknown root (a + different DeerFlow conversation) and may consume an unrelated pending clarification.""" + + async def go(): + bus = MessageBus() + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("feishu", "chat_1", "deer-thread-card", topic_id="om_card", user_id="user_1") + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + key = channel._pending_key("chat_1", "user_1") + channel._pending_clarifications[key] = [_pending("om_pending", thread_id="deer-thread-pending")] + event = _make_text_event("answer", message_id="msg_reply", root_id="om_unknown_root", parent_id="om_card") + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + with caplog.at_level("WARNING", logger="app.channels.feishu"): + await _on_message_from_sdk_loop(channel, event) + + metadata = mock_make_inbound.call_args.kwargs["metadata"] + assert metadata["topic_id"] == "om_card" # the persisted parent mapping won, via the bridge + assert metadata[RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY] is False + assert key in channel._pending_clarifications # the unrelated clarification was not consumed + assert "event-loop thread" not in caplog.text + assert "Gateway loop" not in caplog.text + assert "main loop not running" not in caplog.text + + _run(go()) + + +def test_feishu_sdk_loop_callback_restores_the_pending_mapping_through_the_bridge(caplog): + """Same production shape for the bridge's write path: restoring a pending clarification's + topic mapping goes through the Gateway loop from inside the SDK's running loop.""" + + async def go(): + bus = MessageBus() + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + key = channel._pending_key("chat_1", "user_1") + channel._pending_clarifications[key] = [_pending("om_original", thread_id="deer-thread-1", card_message_id="om_card")] + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + with caplog.at_level("WARNING", logger="app.channels.feishu"): + await _on_message_from_sdk_loop(channel, _make_text_event("2", message_id="msg_plain_2")) + + metadata = mock_make_inbound.call_args.kwargs["metadata"] + assert metadata["topic_id"] == "om_original" + assert metadata[RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY] is True + assert key not in channel._pending_clarifications + assert await store.get_thread_id("feishu", "chat_1", topic_id="om_original") == "deer-thread-1" + assert "event-loop thread" not in caplog.text + assert "Gateway loop" not in caplog.text + + _run(go()) + + +def test_feishu_store_bridge_refuses_to_block_the_gateway_loop_itself(caplog): + """Only a wait issued from the Gateway loop itself is refused: it would block the loop + that has to run the submitted coroutine. The refusal is reported as *unresolved* (never + a miss) and the coroutine factory is never invoked.""" + + async def go(): + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test", "channel_store": store}) + channel._main_loop = asyncio.get_running_loop() + factory_calls = 0 + + def make_coroutine(): + nonlocal factory_calls + factory_calls += 1 + return store.get_thread_id("feishu", "chat_1", topic_id="om_card") + + with caplog.at_level("WARNING", logger="app.channels.feishu"): + with pytest.raises(feishu_module.StoreLookupUnresolved): + channel._run_store_call(make_coroutine, name="resolve_topic_mapping", msg_id="msg_1") + + assert factory_calls == 0 + assert "resolve_topic_mapping requested on the Gateway loop itself" in caplog.text + + _run(go()) def _make_text_event( @@ -810,23 +1184,28 @@ def test_feishu_expired_file_batch_does_not_get_overwritten(monkeypatch): def test_feishu_plain_reply_consumes_pending_clarification_topic(): - bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("feishu", "chat_1", "deer-thread-1", topic_id="om_original", user_id="user_1") - channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) - channel._pending_clarifications[channel._pending_key("chat_1", "user_1")] = [_pending("om_original", thread_id="deer-thread-1", card_message_id="om_card")] + async def go(): + bus = MessageBus() + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("feishu", "chat_1", "deer-thread-1", topic_id="om_original", user_id="user_1") + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + channel._pending_clarifications[channel._pending_key("chat_1", "user_1")] = [_pending("om_original", thread_id="deer-thread-1", card_message_id="om_card")] - with pytest.MonkeyPatch.context() as m: - mock_make_inbound = MagicMock() - m.setattr(channel, "_make_inbound", mock_make_inbound) - channel._on_message(_make_text_event("2", message_id="msg_plain_2")) + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + await _on_message_from_lark_thread(channel, _make_text_event("2", message_id="msg_plain_2")) - inbound = mock_make_inbound.return_value - metadata = mock_make_inbound.call_args.kwargs["metadata"] - assert inbound.topic_id == "om_original" - assert metadata["topic_id"] == "om_original" - assert metadata[RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY] is True - assert channel._pending_key("chat_1", "user_1") not in channel._pending_clarifications + inbound = mock_make_inbound.return_value + metadata = mock_make_inbound.call_args.kwargs["metadata"] + assert inbound.topic_id == "om_original" + assert metadata["topic_id"] == "om_original" + assert metadata[RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY] is True + assert channel._pending_key("chat_1", "user_1") not in channel._pending_clarifications + # The pending clarification's mapping was restored through the store on the Gateway loop. + assert await store.get_thread_id("feishu", "chat_1", topic_id="om_original") == "deer-thread-1" + + _run(go()) def test_feishu_pending_clarification_is_consumed_once(): @@ -947,29 +1326,33 @@ def test_feishu_multiple_pending_clarifications_are_consumed_in_order(): def test_feishu_explicit_reply_prefers_stored_mapping_over_pending(): - bus = MessageBus() - store = ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") - store.set_thread_id("feishu", "chat_1", "deer-thread-card", topic_id="om_card", user_id="user_1") - channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) - key = channel._pending_key("chat_1", "user_1") - channel._pending_clarifications[key] = [_pending("om_pending", thread_id="deer-thread-pending")] + async def go(): + bus = MessageBus() + store = JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json") + await store.set_thread_id("feishu", "chat_1", "deer-thread-card", topic_id="om_card", user_id="user_1") + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test", "channel_store": store}) + key = channel._pending_key("chat_1", "user_1") + channel._pending_clarifications[key] = [_pending("om_pending", thread_id="deer-thread-pending")] - with pytest.MonkeyPatch.context() as m: - mock_make_inbound = MagicMock() - m.setattr(channel, "_make_inbound", mock_make_inbound) - channel._on_message( - _make_text_event( - "answer", - message_id="msg_reply", - root_id="om_unknown", - parent_id="om_card", + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + await _on_message_from_lark_thread( + channel, + _make_text_event( + "answer", + message_id="msg_reply", + root_id="om_unknown", + parent_id="om_card", + ), ) - ) - metadata = mock_make_inbound.call_args.kwargs["metadata"] - assert metadata["topic_id"] == "om_card" - assert metadata[RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY] is False - assert key in channel._pending_clarifications + metadata = mock_make_inbound.call_args.kwargs["metadata"] + assert metadata["topic_id"] == "om_card" + assert metadata[RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY] is False + assert key in channel._pending_clarifications + + _run(go()) @pytest.mark.parametrize("command", sorted(KNOWN_CHANNEL_COMMANDS)) diff --git a/backend/tests/test_github_dispatcher.py b/backend/tests/test_github_dispatcher.py index 9a361114c9..ab69c40613 100644 --- a/backend/tests/test_github_dispatcher.py +++ b/backend/tests/test_github_dispatcher.py @@ -51,7 +51,7 @@ async def _drain(bus: MessageBus) -> list[InboundMessage]: async def test_fanout_redelivery_progresses_beyond_queue_sized_prefix(base_dir: Path) -> None: """A redelivery must not keep failing on the same queue-sized prefix.""" from app.channels.manager import ChannelManager - from app.channels.store import ChannelStore + from app.channels.store import JsonChannelStore agent_names = {"alpha", "bravo", "charlie"} for name in agent_names: @@ -75,7 +75,7 @@ async def test_fanout_redelivery_progresses_beyond_queue_sized_prefix(base_dir: bus = MessageBus(inbound_queue_maxsize=1) manager = ChannelManager( bus=bus, - store=ChannelStore(path=base_dir / "fanout-store.json"), + store=JsonChannelStore(path=base_dir / "fanout-store.json"), max_concurrency=1, ) handled: list[str] = [] @@ -1240,9 +1240,9 @@ async def test_dedupe_identity_distinguishes_same_agent_name_across_users(base_d # differ: neither user's message is treated as a duplicate of the # other inside the same ChannelManager dedupe window. from app.channels.manager import ChannelManager - from app.channels.store import ChannelStore + from app.channels.store import JsonChannelStore - manager = ChannelManager(bus=MessageBus(), store=ChannelStore(path=base_dir / "dedupe-store.json")) + manager = ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=base_dir / "dedupe-store.json")) assert await manager._is_duplicate_inbound(by_owner["alice"]) is False assert await manager._is_duplicate_inbound(by_owner["bob"]) is False @@ -1265,7 +1265,7 @@ async def test_missing_delivery_header_leaves_dedupe_open(base_dir: Path) -> Non (willem-bd, PR #4104 review). """ from app.channels.manager import ChannelManager - from app.channels.store import ChannelStore + from app.channels.store import JsonChannelStore bus = MessageBus() _write_agent(base_dir, "default", "reviewer", {"name": "reviewer", "github": {"bindings": [{"repo": "a/b", "triggers": {"pull_request": {"actions": ["opened"]}}}]}}) @@ -1275,7 +1275,7 @@ async def test_missing_delivery_header_leaves_dedupe_open(base_dir: Path) -> Non "repository": {"full_name": "a/b"}, "sender": {"login": "u"}, } - manager = ChannelManager(bus=MessageBus(), store=ChannelStore(path=base_dir / "dedupe-store.json")) + manager = ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=base_dir / "dedupe-store.json")) await fanout_event(bus, "pull_request", "", payload) (first,) = await _drain(bus) diff --git a/backend/tests/test_github_token_plumbing.py b/backend/tests/test_github_token_plumbing.py index e10fbe6bc8..a9e0b8898a 100644 --- a/backend/tests/test_github_token_plumbing.py +++ b/backend/tests/test_github_token_plumbing.py @@ -29,7 +29,7 @@ from langgraph_sdk.errors import ConflictError from app.channels.manager import ChannelManager from app.channels.message_bus import InboundMessage, InboundMessageType, MessageBus -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore from deerflow.sandbox.local.local_sandbox import LocalSandbox from deerflow.sandbox.tools import _github_env_from_runtime, bash_tool @@ -504,7 +504,7 @@ def _github_msg(installation_id: int | None = 140594274) -> InboundMessage: def _new_manager() -> ChannelManager: bus = MessageBus() - store = ChannelStore(path=Path("/tmp/nonexistent-store-test.json")) + store = JsonChannelStore(path=Path("/tmp/nonexistent-store-test.json")) return ChannelManager(bus=bus, store=store) diff --git a/backend/tests/test_migration_0027_0028_notification_outbox.py b/backend/tests/test_migration_0027_0028_notification_outbox.py index a7adca5ef7..33b4a38678 100644 --- a/backend/tests/test_migration_0027_0028_notification_outbox.py +++ b/backend/tests/test_migration_0027_0028_notification_outbox.py @@ -23,7 +23,7 @@ OUTBOX = "0027_notification_deliveries" PARKED = "0028_parked_attempts" SCHEDULER = "0029_scheduler_agent_tasks" CLAIM_TOKENS = "0030_notification_claim_tokens" -HEAD = "0037_project_document_summaries" +HEAD = "0038_channel_thread_bindings" PREVIOUS = "0026_mcp_task_lease_tokens" TABLE = "notification_deliveries" diff --git a/backend/tests/test_migration_0029_scheduler_agent_tasks.py b/backend/tests/test_migration_0029_scheduler_agent_tasks.py index 9c1906d342..342d99f4ff 100644 --- a/backend/tests/test_migration_0029_scheduler_agent_tasks.py +++ b/backend/tests/test_migration_0029_scheduler_agent_tasks.py @@ -23,7 +23,7 @@ from deerflow.persistence.postgres_schema import build_asyncpg_connect_args from deerflow.persistence.scheduled_tasks import ScheduledTaskRepository REVISION = "0029_scheduler_agent_tasks" -CURRENT_HEAD = "0037_project_document_summaries" +CURRENT_HEAD = "0038_channel_thread_bindings" NEXT = "0030_notification_claim_tokens" PREVIOUS = "0028_parked_attempts" TASK_FIELDS = {"origin_thread_id", "goal_objective", "max_runs", "end_at", "standing_notes"} diff --git a/backend/tests/test_migration_0032_activity_and_task_events.py b/backend/tests/test_migration_0032_activity_and_task_events.py index 4f3d877c78..4ff73981eb 100644 --- a/backend/tests/test_migration_0032_activity_and_task_events.py +++ b/backend/tests/test_migration_0032_activity_and_task_events.py @@ -21,7 +21,8 @@ NEXT = "0033_batch_result_artifact" AFTER_NEXT = "0034_run_event_seq_watermark" LOGIN_THROTTLE = "0035_login_throttle" IDEMPOTENCY = "0036_run_idempotency_request" -CURRENT_HEAD = "0037_project_document_summaries" +DOCUMENT_SUMMARIES = "0037_project_document_summaries" +CURRENT_HEAD = "0038_channel_thread_bindings" PREVIOUS = "0031_scheduled_streak_boundary" TABLES = {"thread_read_markers", "thread_read_versions", "scheduled_task_events"} INDEXES = { @@ -40,7 +41,8 @@ async def test_0032_remains_in_the_single_migration_chain(): assert script.get_revision(AFTER_NEXT).down_revision == NEXT assert script.get_revision(LOGIN_THROTTLE).down_revision == AFTER_NEXT assert script.get_revision(IDEMPOTENCY).down_revision == LOGIN_THROTTLE - assert script.get_revision(CURRENT_HEAD).down_revision == IDEMPOTENCY + assert script.get_revision(DOCUMENT_SUMMARIES).down_revision == IDEMPOTENCY + assert script.get_revision(CURRENT_HEAD).down_revision == DOCUMENT_SUMMARIES # alembic_version.version_num is VARCHAR(32). assert len(REVISION) <= 32 diff --git a/backend/tests/test_migration_0034_run_event_seq_watermark.py b/backend/tests/test_migration_0034_run_event_seq_watermark.py index b6c11a056b..b3d8469883 100644 --- a/backend/tests/test_migration_0034_run_event_seq_watermark.py +++ b/backend/tests/test_migration_0034_run_event_seq_watermark.py @@ -21,7 +21,8 @@ REVISION = "0034_run_event_seq_watermark" PREVIOUS = "0033_batch_result_artifact" LOGIN_THROTTLE = "0035_login_throttle" IDEMPOTENCY = "0036_run_idempotency_request" -CURRENT_HEAD = "0037_project_document_summaries" +DOCUMENT_SUMMARIES = "0037_project_document_summaries" +CURRENT_HEAD = "0038_channel_thread_bindings" TABLE = "run_event_thread_seq" pytestmark = pytest.mark.asyncio @@ -32,7 +33,8 @@ async def test_0034_remains_in_the_single_migration_chain(): assert script.get_revision(REVISION).down_revision == PREVIOUS assert script.get_revision(LOGIN_THROTTLE).down_revision == REVISION assert script.get_revision(IDEMPOTENCY).down_revision == LOGIN_THROTTLE - assert script.get_revision(CURRENT_HEAD).down_revision == IDEMPOTENCY + assert script.get_revision(DOCUMENT_SUMMARIES).down_revision == IDEMPOTENCY + assert script.get_revision(CURRENT_HEAD).down_revision == DOCUMENT_SUMMARIES assert len(REVISION) <= 32 diff --git a/backend/tests/test_migration_0035_login_throttle.py b/backend/tests/test_migration_0035_login_throttle.py index f7c6f1fb64..46b69bdabc 100644 --- a/backend/tests/test_migration_0035_login_throttle.py +++ b/backend/tests/test_migration_0035_login_throttle.py @@ -22,7 +22,8 @@ from deerflow.persistence.postgres_schema import build_asyncpg_connect_args REVISION = "0035_login_throttle" IDEMPOTENCY = "0036_run_idempotency_request" -CURRENT_HEAD = "0037_project_document_summaries" +DOCUMENT_SUMMARIES = "0037_project_document_summaries" +CURRENT_HEAD = "0038_channel_thread_bindings" PREVIOUS = "0034_run_event_seq_watermark" TABLE = "login_throttle" COLUMNS = {"ip", "fail_count", "locked_at", "lock_duration_seconds", "updated_at"} @@ -35,7 +36,8 @@ async def test_0035_remains_in_the_single_head_chain_after_0034(): assert script.get_heads() == [CURRENT_HEAD] assert script.get_revision(REVISION).down_revision == PREVIOUS assert script.get_revision(IDEMPOTENCY).down_revision == REVISION - assert script.get_revision(CURRENT_HEAD).down_revision == IDEMPOTENCY + assert script.get_revision(DOCUMENT_SUMMARIES).down_revision == IDEMPOTENCY + assert script.get_revision(CURRENT_HEAD).down_revision == DOCUMENT_SUMMARIES # alembic_version.version_num is VARCHAR(32). assert len(REVISION) <= 32 diff --git a/backend/tests/test_migration_0037_project_document_summaries.py b/backend/tests/test_migration_0037_project_document_summaries.py index c227ee9015..68bf35f8c0 100644 --- a/backend/tests/test_migration_0037_project_document_summaries.py +++ b/backend/tests/test_migration_0037_project_document_summaries.py @@ -18,12 +18,14 @@ from deerflow.persistence.postgres_schema import build_asyncpg_connect_args REVISION = "0037_project_document_summaries" PREVIOUS = "0036_run_idempotency_request" +CURRENT_HEAD = "0038_channel_thread_bindings" pytestmark = pytest.mark.asyncio async def test_0037_chains_after_0036_and_is_single_head(): script = ScriptDirectory(str(bootstrap._MIGRATIONS_DIR)) - assert script.get_heads() == [REVISION] + assert script.get_heads() == [CURRENT_HEAD] + assert script.get_revision(CURRENT_HEAD).down_revision == REVISION assert script.get_revision(REVISION).down_revision == PREVIOUS # alembic_version.version_num is VARCHAR(32). assert len(REVISION) <= 32 diff --git a/backend/tests/test_migration_0038_channel_thread_bindings.py b/backend/tests/test_migration_0038_channel_thread_bindings.py new file mode 100644 index 0000000000..94225af37e --- /dev/null +++ b/backend/tests/test_migration_0038_channel_thread_bindings.py @@ -0,0 +1,187 @@ +"""Migration 0038: the shared ``channel_thread_bindings`` table (IM chat -> DeerFlow thread).""" + +from __future__ import annotations + +import asyncio +import os +import uuid + +import pytest +import sqlalchemy as sa +from alembic import command +from alembic.autogenerate import compare_metadata +from alembic.migration import MigrationContext +from alembic.script import ScriptDirectory +from sqlalchemy.ext.asyncio import create_async_engine +from support.postgres import asyncpg_test_url + +from deerflow.persistence import bootstrap +from deerflow.persistence.base import Base +from deerflow.persistence.channel_thread_bindings import ChannelThreadBindingRow +from deerflow.persistence.postgres_schema import build_asyncpg_connect_args + +REVISION = "0038_channel_thread_bindings" +PREVIOUS = "0037_project_document_summaries" +TABLE = "channel_thread_bindings" +COLUMNS = {"key", "channel_name", "chat_id", "topic_id", "thread_id", "user_id", "created_at", "updated_at"} +INDEX = "ix_channel_thread_bindings_channel_chat" +pytestmark = pytest.mark.asyncio + + +async def test_0038_is_the_single_head_and_chains_after_0037(): + script = ScriptDirectory(str(bootstrap._MIGRATIONS_DIR)) + assert script.get_heads() == [REVISION] + assert script.get_revision(REVISION).down_revision == PREVIOUS + # alembic_version.version_num is VARCHAR(32). + assert len(REVISION) <= 32 + + +def _engine(tmp_path, backend): + if backend == "sqlite": + return create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'migration.db'}"), None + uri = os.environ.get("TEST_POSTGRES_URI") + if not uri: + pytest.skip("requires TEST_POSTGRES_URI (real Postgres migration 0038)") + schema = f"channel_bindings_{uuid.uuid4().hex}" + return create_async_engine(asyncpg_test_url(uri), connect_args=build_asyncpg_connect_args(schema)), schema + + +async def _shape(engine) -> dict: + async with engine.connect() as conn: + + def read(sync): + inspector = sa.inspect(sync) + tables = set(inspector.get_table_names()) + if TABLE not in tables: + return {"tables": tables} + return { + "tables": tables, + "columns": {col["name"]: col for col in inspector.get_columns(TABLE)}, + "pk": inspector.get_pk_constraint(TABLE)["constrained_columns"], + "indexes": {index["name"]: index["column_names"] for index in inspector.get_indexes(TABLE)}, + } + + return await conn.run_sync(read) + + +async def _orm_diff(engine) -> list: + """Alembic's own drift check, restricted to the table this revision owns.""" + + def only_bindings(obj, name, type_, reflected, compare_to): + if type_ == "table": + return name == TABLE + return getattr(getattr(obj, "table", None), "name", TABLE) == TABLE + + async with engine.connect() as conn: + + def diff(sync): + context = MigrationContext.configure(sync, opts={"include_object": only_bindings, "compare_type": True, "compare_server_default": True}) + return compare_metadata(context, Base.metadata) + + return await conn.run_sync(diff) + + +@pytest.mark.parametrize("backend", ["sqlite", "postgres"]) +async def test_0038_creates_the_table_matching_the_orm_and_downgrades_cleanly(tmp_path, backend): + engine, schema = _engine(tmp_path, backend) + cfg = bootstrap._get_alembic_config(engine, postgres_schema=schema or "") + try: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'CREATE SCHEMA "{schema}"')) + await asyncio.to_thread(command.upgrade, cfg, PREVIOUS) + assert TABLE not in (await _shape(engine))["tables"] + + await asyncio.to_thread(command.upgrade, cfg, REVISION) + shape = await _shape(engine) + assert set(shape["columns"]) == COLUMNS + assert shape["pk"] == ["key"] + assert shape["columns"]["channel_name"]["nullable"] is False + assert shape["columns"]["chat_id"]["nullable"] is False + assert shape["columns"]["topic_id"]["nullable"] is True + assert shape["columns"]["thread_id"]["nullable"] is False + assert shape["columns"]["user_id"]["nullable"] is False + assert shape["columns"]["created_at"]["nullable"] is False + assert shape["columns"]["updated_at"]["nullable"] is False + assert shape["indexes"][INDEX] == ["channel_name", "chat_id"] # serves list_entries(channel) and the prefix remove + # ``make migrate-rev`` would propose nothing: the revision matches the ORM model. + assert await _orm_diff(engine) == [] + + async with engine.begin() as conn: + await conn.execute( + sa.insert(ChannelThreadBindingRow).values(key="slack:C1:171.1", channel_name="slack", chat_id="C1", topic_id="171.1", thread_id="thread-1", user_id="U1", created_at=1_700_000_000.0, updated_at=1_700_000_000.5) + ) + with pytest.raises(sa.exc.IntegrityError): + async with engine.begin() as conn: + await conn.execute(sa.insert(ChannelThreadBindingRow).values(key="slack:C1:171.1", channel_name="slack", chat_id="C1", thread_id="thread-2", user_id="", created_at=1.0, updated_at=1.0)) + async with engine.connect() as conn: + row = (await conn.execute(sa.text(f"SELECT channel_name, chat_id, topic_id, thread_id, user_id, created_at, updated_at FROM {TABLE}"))).one() + assert tuple(row) == ("slack", "C1", "171.1", "thread-1", "U1", 1_700_000_000.0, 1_700_000_000.5) + + await asyncio.to_thread(command.downgrade, cfg, PREVIOUS) + shape = await _shape(engine) + assert TABLE not in shape["tables"] + async with engine.connect() as conn: + # The index goes with the table on downgrade (named, so a leftover would be visible). + leftover = await conn.run_sync(lambda sync: [name for table in sa.inspect(sync).get_table_names() for name in {index["name"] for index in sa.inspect(sync).get_indexes(table)} if name == INDEX]) + assert leftover == [] + + # Downgrade then upgrade again is clean. + await asyncio.to_thread(command.upgrade, cfg, REVISION) + shape = await _shape(engine) + assert TABLE in shape["tables"] + assert INDEX in shape["indexes"] + finally: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + await engine.dispose() + + +@pytest.mark.parametrize("backend", ["sqlite", "postgres"]) +async def test_0038_completes_a_partially_applied_upgrade(tmp_path, backend): + """The table already exists (an interrupted earlier attempt): upgrade still lands.""" + engine, schema = _engine(tmp_path, backend) + cfg = bootstrap._get_alembic_config(engine, postgres_schema=schema or "") + try: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'CREATE SCHEMA "{schema}"')) + await asyncio.to_thread(command.upgrade, cfg, PREVIOUS) + async with engine.begin() as conn: + # The table exists but its index was never built (an interrupted earlier attempt). + await conn.run_sync(Base.metadata.create_all, tables=[ChannelThreadBindingRow.__table__]) + await conn.execute(sa.text(f"DROP INDEX {INDEX}")) + await conn.execute(sa.insert(ChannelThreadBindingRow).values(key="slack:C1", channel_name="slack", chat_id="C1", thread_id="thread-1", user_id="", created_at=1.0, updated_at=1.0)) + + await asyncio.to_thread(command.upgrade, cfg, REVISION) + assert INDEX in (await _shape(engine))["indexes"] # the partial upgrade is completed + async with engine.connect() as conn: + assert (await conn.execute(sa.text(f"SELECT thread_id FROM {TABLE} WHERE key='slack:C1'"))).scalar_one() == "thread-1" + assert (await conn.execute(sa.text("SELECT version_num FROM alembic_version"))).scalar_one() == REVISION + + # A downgrade after someone already dropped the table still completes. + async with engine.begin() as conn: + await conn.execute(sa.text(f"DROP TABLE {TABLE}")) + await asyncio.to_thread(command.downgrade, cfg, PREVIOUS) + async with engine.connect() as conn: + assert (await conn.execute(sa.text("SELECT version_num FROM alembic_version"))).scalar_one() == PREVIOUS + finally: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + await engine.dispose() + + +async def test_bootstrap_provisions_the_table_on_an_empty_database_and_is_idempotent(tmp_path): + engine = create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'bootstrap.db'}") + try: + await bootstrap.bootstrap_schema(engine, backend="sqlite") + await bootstrap.bootstrap_schema(engine, backend="sqlite") + shape = await _shape(engine) + assert set(shape["columns"]) == COLUMNS + assert INDEX in shape["indexes"] + async with engine.connect() as conn: + assert (await conn.execute(sa.text("SELECT version_num FROM alembic_version"))).scalar_one() == REVISION + finally: + await engine.dispose() diff --git a/backend/tests/test_multi_pod_inbound_dedupe.py b/backend/tests/test_multi_pod_inbound_dedupe.py index dcd3d7e086..5dc08aea1e 100644 --- a/backend/tests/test_multi_pod_inbound_dedupe.py +++ b/backend/tests/test_multi_pod_inbound_dedupe.py @@ -102,13 +102,13 @@ async def test_manager_injects_shared_store_and_dedupes_cross_pod(): loop would (``if await self._is_duplicate_inbound(msg): continue``).""" from app.channels.manager import ChannelManager from app.channels.message_bus import InboundMessage, MessageBus - from app.channels.store import ChannelStore + from app.channels.store import JsonChannelStore from deerflow.persistence.engine import get_session_factory sf = get_session_factory() shared_store = PostgresInboundDedupeStore(session_factory=sf) - manager_a = ChannelManager(bus=MessageBus(), store=ChannelStore(), inbound_dedupe_store=shared_store) - manager_b = ChannelManager(bus=MessageBus(), store=ChannelStore(), inbound_dedupe_store=shared_store) + manager_a = ChannelManager(bus=MessageBus(), store=JsonChannelStore(), inbound_dedupe_store=shared_store) + manager_b = ChannelManager(bus=MessageBus(), store=JsonChannelStore(), inbound_dedupe_store=shared_store) msg = InboundMessage( channel_name="github", diff --git a/backend/tests/test_qq_binding.py b/backend/tests/test_qq_binding.py index 828f6c9a6e..f5c987f195 100644 --- a/backend/tests/test_qq_binding.py +++ b/backend/tests/test_qq_binding.py @@ -9,7 +9,7 @@ import pytest_asyncio from app.channels.manager import ChannelManager from app.channels.message_bus import MessageBus from app.channels.qq import QQChannel -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore from deerflow.persistence.channel_connections import ChannelConnectionRepository from deerflow.persistence.engine import close_engine, get_session_factory, init_engine @@ -59,7 +59,7 @@ async def test_binding_transfer_rejects_queued_old_owner(repo, tmp_path, monkeyp channel.send = AsyncMock() manager = ChannelManager( bus, - ChannelStore(tmp_path / "threads.json"), + JsonChannelStore(tmp_path / "threads.json"), connection_repo=repo, require_bound_identity=True, ) diff --git a/backend/tests/test_qq_lifecycle.py b/backend/tests/test_qq_lifecycle.py index 21edbae804..b62b0c6535 100644 --- a/backend/tests/test_qq_lifecycle.py +++ b/backend/tests/test_qq_lifecycle.py @@ -317,11 +317,10 @@ async def test_cancelled_stop_drains_http_client_cleanup(): @pytest.mark.asyncio async def test_service_registers_starts_and_disposes_qq(tmp_path, monkeypatch): from app.channels import service as service_module - from app.channels.store import ChannelStore + from app.channels.store import JsonChannelStore from deerflow.config.app_config import AppConfig - store = ChannelStore(tmp_path / "channels.json") - monkeypatch.setattr(service_module, "ChannelStore", lambda: store) + store = JsonChannelStore(tmp_path / "channels.json") config = { "app_id": "fixture-app", "client_secret": "fixture-secret", @@ -329,7 +328,7 @@ async def test_service_registers_starts_and_disposes_qq(tmp_path, monkeypatch): } app_config = AppConfig.model_validate({"sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"}}) repository = object() - service = service_module.ChannelService({"qq": config}, connection_repo=repository, app_config=app_config) + service = service_module.ChannelService({"qq": config}, connection_repo=repository, app_config=app_config, store=store) async def connected(self, url): self._ready.set() diff --git a/backend/tests/test_run_origin.py b/backend/tests/test_run_origin.py index 9d00e42ce1..62963aa559 100644 --- a/backend/tests/test_run_origin.py +++ b/backend/tests/test_run_origin.py @@ -357,9 +357,9 @@ async def test_extension_handle_marks_its_threads_and_runs(gateway, monkeypatch, def _manager(): from app.channels.manager import ChannelManager from app.channels.message_bus import MessageBus - from app.channels.store import ChannelStore + from app.channels.store import JsonChannelStore - return ChannelManager(bus=MessageBus(), store=ChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")) + return ChannelManager(bus=MessageBus(), store=JsonChannelStore(path=Path(tempfile.mkdtemp()) / "store.json")) def _client(thread_id="im-thread"): diff --git a/backend/tests/test_trace_entry_points.py b/backend/tests/test_trace_entry_points.py index 3cc567cd13..9ba048eebf 100644 --- a/backend/tests/test_trace_entry_points.py +++ b/backend/tests/test_trace_entry_points.py @@ -20,7 +20,7 @@ import pytest from app.channels.manager import ChannelManager from app.channels.message_bus import InboundMessage, MessageBus -from app.channels.store import ChannelStore +from app.channels.store import JsonChannelStore from app.scheduler.service import ScheduledTaskService from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config from deerflow.trace_context import get_current_trace_id, request_trace_context @@ -199,7 +199,7 @@ async def test_inbound_messages_are_handled_under_distinct_trace_scopes(tmp_path bus = MessageBus(inbound_queue_maxsize=4) manager = ChannelManager( bus=bus, - store=ChannelStore(path=tmp_path / "store.json"), + store=JsonChannelStore(path=tmp_path / "store.json"), max_concurrency=1, ) seen: list[str | None] = []