Files
multica/server/internal/integrations/lark/outbound_test.go
Bohan Jiang ce28d0aa0e feat(integrations): add platform-agnostic channel foundation (MUL-3515) (#4412)
* feat(integrations): add platform-agnostic channel foundation

Introduce server/internal/integrations/channel — the contract every
inbound IM integration implements, so the core never learns a platform's
event JSON. Four pieces:

- Channel interface (Type/Connect/Disconnect/Send/Capabilities) + Factory
  + Config (channel_type + opaque JSON blob, maps to channel_installation).
- Normalized InboundMessage/OutboundMessage envelopes + Source/MediaRef/
  ReplyCtx/MsgType/ChatType. Envelope holds only cross-platform-true
  fields; platform specifics live in Raw, read only by the adapter.
- Capability bitmask: declaration only, no degrade logic in core.
- Registry: Type->Factory map, last-writer-wins, concurrency-safe.

Pure package (no DB/network/platform deps). Foundation for MUL-3515; the
lark cutover + lark_*->channel_* generalization land in follow-up PRs.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* feat(channel): generalize lark_* tables into channel_* (DB layer)

Migration 123 creates channel_installation / channel_user_binding /
channel_chat_session_binding / channel_inbound_message_dedup /
channel_inbound_audit / channel_outbound_card_message /
channel_binding_token. Each carries a channel_type discriminator and a
JSONB config for platform-specific identifiers/credentials; cross-platform
columns stay flat. Existing Feishu rows are backfilled (channel_type=
'feishu', app_secret_encrypted via base64). NO foreign keys / cascades
(MUL-3515 §4) — integrity moves to the app layer in the cutover.

queries/channel.sql ports the lark query surface to channel_*, JSONB-aware,
plus DeleteChannelUserBindingsByWorkspaceMember /
DeleteChannelChatSessionBindingBySession for the app-layer cleanup that
replaces the removed cascades.

lark_* tables/queries are left in place here and removed once the Go
cutover lands, so this commit ships green on its own.

Verified: sqlc generate, go build ./..., full migrate chain (1..123) on
Postgres 17, and a real-data backfill spot-check (base64 round-trip,
NULL-strip, functional unique index on (channel_type, app_id)).

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* fix(channel): name app_id query param + multi-IM install key + null-safe binding merge

Addresses review on MUL-3515 (PR #4412):

- GetChannelInstallationByAppID: explicitly name params and cast app_id to
  ::text so sqlc emits AppID string. A bare $2 next to `config ->> 'app_id'`
  was mis-attributed to the JSONB config column, generating Config []byte.

- channel_installation uniqueness -> (workspace_id, agent_id, channel_type),
  with the UpsertChannelInstallation conflict key matched. Lets one agent
  hold one installation per IM (feishu + slack + ...) instead of a later
  install clobbering an earlier one. Behaviorally identical in the current
  feishu-only world; "one agent, at most one IM overall" stays an app-layer
  rule per MUL-3515 §4, not a DB constraint.

- CreateChannelUserBinding merges jsonb_strip_nulls(EXCLUDED.config) so a
  re-bind carrying {"union_id": null} no longer erases an already-captured
  union_id, restoring the old COALESCE(EXCLUDED.union_id, ...) semantics.

Regenerated with sqlc v1.31.1. Verified on PG17: re-install replaces in
place, feishu+slack coexist, null re-bind keeps union_id, real union_id wins.

Co-authored-by: multica-agent <github@multica.ai>

* feat(lark): channel-backed Feishu store + fix base64 backfill wrapping

Cutover step 1 of switching the lark Go code from lark_* onto the channel_*
tables (MUL-3515). Introduces the JSONB config boundary the rest of the
cutover sits on, and fixes a latent backfill bug surfaced while building it.

- migration 123: strip newlines from the app_secret_encrypted base64 backfill.
  PostgreSQL encode(...,'base64') MIME-wraps at 76 chars, and a secretbox-
  sealed ~72-byte secret exceeds that. Go's encoding/json decodes a JSON
  string into []byte with base64.StdEncoding, which rejects embedded newlines,
  so without the strip every migrated installation would fail to decrypt its
  app secret once reads move to channel_installation.config.

- store.go: flat domain types (Installation / UserBinding / ChatSessionBinding)
  with field parity to the retired db.Lark* rows, plus the feishu config codec.
  Row->domain mappers decode the JSONB config; the secret decoder is
  whitespace-tolerant so legacy MIME-wrapped data still round-trips, while the
  encoder emits unwrapped base64. Binding config encodes an absent union_id as
  "{}" so the upsert's jsonb_strip_nulls merge never clobbers a stored union_id.

- store_test.go: 72-byte secret round-trip, MIME-wrapped tolerance, optional
  null-strip, and flat-column preservation. Verified on PG17.

Field parity keeps the upcoming ~190 db.LarkInstallation call sites a
mechanical rename. No call sites switched yet; behavior unchanged.

Co-authored-by: multica-agent <github@multica.ai>

* feat(lark): route inbound integration onto channel_* + explicit membership checks

Cutover step 2 (MUL-3515): switch the Feishu Go code from the lark_* queries to
channel_* via a ChannelStore adapter, and replace the removed member foreign key
with explicit application-layer membership checks. No user-visible behavior change.

- channel_store.go: ChannelStore embeds *db.Queries and SHADOWS the ~24 lark
  query methods with channel_*-backed equivalents, keeping the db.Lark*
  signatures so the dispatcher/hub/services and their ~20k lines of tests stay
  untouched; the feishu JSONB config is (de)coded by store.go. Adds
  IsWorkspaceMember and a tx-aware WithTx. Only production wiring swaps
  *db.Queries for *ChannelStore.

- Membership re-check (§4 removed the lark_user_binding -> member FK, so a
  binding row no longer proves current membership):
  * the dispatcher inbound identity step verifies membership after the binding
    lookup; a former member's stale binding is dropped as non_workspace_member
    + audited and never reaches chat_session (§4.3 safety property).
  * RedeemAndBind and BindInstallerTx replace the now-dead FK (23503) branch
    with an explicit IsWorkspaceMember gate, preserving the existing
    ErrBindingNotWorkspaceMember outcome without burning the token.

- router wires the ChannelStore into the patcher, typing indicator, dispatcher,
  hub, and the union_id/region backfills; constructor-based services wrap
  *db.Queries internally so their signatures and nil-check tests are unchanged.

Verified: go build ./... ; go vet ; gofmt ; go test -race ./internal/integrations/...
(full lark suite green unchanged + new membership drop/error tests). Adapter
field mappings (secret base64, union_id RMW, chat-id/open-id remaps, dedup,
token, card) checked end-to-end against a PG17 channel_* schema.

lark_* tables and queries remain (unused at runtime) until the S3 cleanup-hooks
and S4 drop-tables/rename commits.

Co-authored-by: multica-agent <github@multica.ai>

* fix(channel): renumber generalization migration 123 -> 124

main merged 123_issue_stage after this branch forked, so the branch's 123_channel_generalization now collides on the migration number. The runner keys schema_migrations by full version string and would still apply both, but a duplicate number is a merge hazard and convention violation, so move the channel migration to the next free slot (124).

issue_stage (ALTER issue ADD COLUMN stage) and the channel generalization touch disjoint tables; verified on PG17 that 123_issue_stage applies cleanly on a DB already carrying 124_channel_generalization, so the two are order-independent. sqlc regenerated (v1.31.1): only the migration-number comment changed.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* feat(channel): prune channel bindings on member removal + chat session delete

MUL-3515 §4 dropped every channel_* foreign key, so the old ON DELETE CASCADE that cleared a user's channel_user_binding when they left a workspace, and a chat's channel_chat_session_binding when its chat_session was deleted, no longer fires. Re-establish that integrity in the application layer, inside the existing transactions: revokeAndRemoveMember -> DeleteChannelUserBindingsByWorkspaceMember, DeleteChatSession -> DeleteChannelChatSessionBindingBySession.

Adds real-DB tests for both paths, including a scoping check that a remaining member's binding survives the prune. Verified on PG17: both new tests plus the existing revocation tests and the full handler package pass.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* fix(channel): scope Lark/Feishu store reads to channel_type='feishu'

The S2 cutover routed the Feishu integration onto channel_*, but the Lark-facing ChannelStore wrappers read installation / chat-session-binding / outbound-card rows across ALL channel_type values. Once a second IM exists, that would let the Lark hub supervise a non-Feishu installation, the Lark install list show it, /lark/installations/{id} revoke another channel's row, and the outbound patcher / typing indicator act on a non-Feishu chat binding or card.

Add a channel_type predicate to the six read/list channel queries and pass channelTypeFeishu from every wrapper: GetChannelInstallation, GetChannelInstallationInWorkspace, ListChannelInstallationsByWorkspace, ListActiveChannelInstallations, GetChannelChatSessionBindingBySession, GetChannelOutboundCardByTask.

The S3 cleanup deletes (DeleteChannelUserBindingsByWorkspaceMember / DeleteChannelChatSessionBindingBySession) stay all-channel on purpose: a member leaving or a chat_session being deleted should clear every IM's binding. Adds a real-DB test that seeds a Slack installation/binding/card next to the Feishu ones and asserts the Lark wrappers never return them.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* refactor(channel): replace db.Lark* translation layer with lark domain types

S2 introduced ChannelStore as a translation layer that read/wrote channel_* but kept the retired db.Lark* struct/param shapes so the dispatcher/hub/services and their ~20k lines of tests did not have to change. This collapses that layer: the store now takes and returns the package's flat domain types (Installation, UserBinding, ChatSessionBinding, InboundMessageDedup, BindingTokenRow, OutboundCardMessage) and the *Params types in params.go, with channel-neutral field names (ChannelUserID / ChannelChatID / ...). All call sites, fakes, and tests move to the domain types.

No behavior change: only channel_* is read/written (as before); db.Lark* is now unused, and the lark_* tables + queries/lark.sql are removed in the next commit. Verified on PG17: go build / vet / gofmt clean, go test -race ./internal/integrations/... green (the ~20k-line fake suite), and the lark + handler suites pass.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* refactor(channel): drop lark_* tables and queries (remove old path)

The Go cutover (previous commit) moved the lark package entirely onto channel_* and the domain types, leaving the lark_* tables, queries/lark.sql, and the generated db.Lark* models unused. Remove them per the design (§5: replace, do not keep both): migration 125 drops the seven lark_* tables (data already lives in channel_* since migration 124), and queries/lark.sql is deleted + sqlc regenerated, removing the db.Lark* models and lark query methods.

The 125 down recreates the authoritative pre-drop schema (bot_union_id, region, per-installation dedup PK, thread-reply columns). Verified on PG17: fresh migrate up ends with lark_* gone + channel_* present; isolated 125 down/up round-trips correctly; go build / vet / gofmt clean; go test -race ./internal/integrations/... and the handler suite pass.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* fix(migrations): remove trailing blank line at EOF of 125 down migration

git diff --check flagged a blank line at EOF of 125_drop_lark_tables.down.sql (a pg_dump-generation artifact). Whitespace only; the recreate SQL is unchanged.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* refactor(channel): defer lark_* table drop to a follow-up migration

Preflight deploy review: dropping lark_* in the same release that cuts over (old migration 125) is not rollback/rolling-safe — the v0.3.27 release still reads lark_*, so a rolling deploy or a post-deploy code rollback would hit "relation does not exist". Remove the drop and keep the old tables for one release (standard expand/contract): migration 124 already backfilled lark_* -> channel_*, the new code reads/writes only channel_*, and the physical drop moves to a separate cleanup migration once this ships and is observed.

The lark_* tables remain in the schema, so sqlc regenerates the (now unused) db.Lark* models; queries/lark.sql stays deleted (the new code uses channel_*). No code path reads lark_* — only the destructive drop is deferred, keeping the design's no-compat-layer / no-dual-write rule while being deploy-safe.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* fix(channel): skip orphaned installations in hub-boot active scan

Preflight deploy review: channel_installation dropped the workspace/agent FK (MUL-3515 §4), so unlike lark_installation it does not cascade away when its workspace is deleted or its agent is hard-deleted (e.g. runtime teardown). The hub-boot query then keeps opening a WebSocket for a bot whose owner is gone.

JOIN ListActiveChannelInstallations to live workspace + agent so an orphaned installation is never connected, uniformly for every deletion path. The JOIN matches the old ON DELETE CASCADE semantics (row existence, not agent archival), so an archived-but-present agent's installation is still listed; the orphaned row's encrypted secret is thereby never decrypted/used.

Tests: a real-DB handler test asserts a deleted-workspace/agent installation and a non-Feishu one are both excluded; the lark scope test's active-list assertion moved there since the JOIN now needs real workspace/agent fixtures. (Physically deleting dormant orphaned channel rows on workspace/agent deletion is a separate app-layer-cleanup follow-up.)

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* docs(channel): document non-rolling cutover constraint for the lark->channel migration

Elon deploy review: keeping the lark_* tables (deferred drop) stops old v0.3.27 code from crashing, but is not full expand/contract. Migration 124 is a one-time backfill; afterwards new code runs on channel_* (lease + dedup on channel_*) while pre-cutover code runs on lark_* (lease + dedup on lark_*). If both run concurrently during a rolling deploy, each side claims the same Feishu bot's WS lease on its own table and double-processes inbound events.

This release therefore requires a NON-ROLLING cutover (stop the old hub before applying migration 124 + starting new code; rollback is not lossless once new code writes channel_*). Documented where deployers/reviewers see it: migration 124 header gains a ROLLOUT note; the channel_store.go header is corrected (lark_* tables are retained one release for rollback safety, not "gone"; the store still never touches them). Comment-only — no schema/codegen/behavior change.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* feat(lark): add MULTICA_LARK_HUB_DISABLED switch for the channel cutover

The lark_*->channel_* cutover needs a way to make the Feishu bot briefly unavailable WITHOUT taking down the whole multica-api process — the Lark hub is a goroutine inside it, not a separate Deployment. MULTICA_LARK_HUB_DISABLED=true parks the hub at startup: the API serves HTTP normally but never claims a WS lease or opens a Feishu connection.

Rollout (see migration 124 ROLLOUT note): ship the new release with the flag SET so new pods run API-only while old pods (hub on lark_*) drain during the rolling deploy — the two hubs never overlap. After the old pods are gone and migration 124 has run, flip the flag off; the new hub comes up on channel_*. The old backend does NOT need this switch — its hub stops when k8s terminates the old pods, not via a flag. Nil-ing LarkHub reuses the existing not-configured path so both the startup start and the shutdown join skip it.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* docs(channel): point migration 124 ROLLOUT note at the hub-disable switch

Refine the rollout note to use MULTICA_LARK_HUB_DISABLED for a bot-only cutover (new pods serve API with the hub parked while old pods drain; flip the switch off after the migration), instead of the earlier whole-API recreate. Comment-only.

MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

* docs(channel): fix migration 124 rollout order and document self-host cutover

The previous ROLLOUT note shipped the new (channel_*) build before
running migration 124, so the channel_*-backed HTTP paths (installation
list/install/revoke, chat-session delete, member revoke) would 500 in
the window between new-pod boot and the deferred migration. Restate the
runbook around two explicit invariants — channel_* must exist before the
new build serves those paths, and the old/new hubs must never overlap —
and order the steps so channel_* is created first (park old hub -> snapshot
-> deploy parked new build -> unpark). Document that default self-host
(entrypoint migrate + single-replica Recreate) satisfies both invariants
automatically and needs no manual steps; only prd / multi-replica rolling
self-host needs the switch procedure. Clarify in main.go that the
hub-park switch is generation-agnostic (parks whichever hub the build
carries), which is what enables the preparatory release.

Refs MUL-3515

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-24 12:46:20 +08:00

634 lines
24 KiB
Go

package lark
import (
"context"
"encoding/json"
"errors"
"strings"
"sync"
"testing"
"time"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/multica-ai/multica/server/internal/events"
db "github.com/multica-ai/multica/server/pkg/db/generated"
"github.com/multica-ai/multica/server/pkg/protocol"
)
type fakePatcherQueries struct {
mu sync.Mutex
binding ChatSessionBinding
bindingErr error
installation Installation
installationErr error
agent db.Agent
agentErr error
card OutboundCardMessage
cardErr error
created []CreateOutboundCardMessageParams
createReturn OutboundCardMessage
statusUpdates []UpdateOutboundCardStatusParams
}
func (f *fakePatcherQueries) GetAgentTask(ctx context.Context, id pgtype.UUID) (db.AgentTaskQueue, error) {
return db.AgentTaskQueue{}, nil
}
func (f *fakePatcherQueries) GetChatSession(ctx context.Context, id pgtype.UUID) (db.ChatSession, error) {
return db.ChatSession{}, nil
}
func (f *fakePatcherQueries) GetAgent(ctx context.Context, id pgtype.UUID) (db.Agent, error) {
return f.agent, f.agentErr
}
func (f *fakePatcherQueries) GetLarkInstallation(ctx context.Context, id pgtype.UUID) (Installation, error) {
return f.installation, f.installationErr
}
func (f *fakePatcherQueries) GetLarkChatSessionBindingBySession(ctx context.Context, sessID pgtype.UUID) (ChatSessionBinding, error) {
return f.binding, f.bindingErr
}
func (f *fakePatcherQueries) GetLarkOutboundCardByTask(ctx context.Context, taskID pgtype.UUID) (OutboundCardMessage, error) {
return f.card, f.cardErr
}
func (f *fakePatcherQueries) CreateLarkOutboundCardMessage(ctx context.Context, arg CreateOutboundCardMessageParams) (OutboundCardMessage, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.created = append(f.created, arg)
return f.createReturn, nil
}
func (f *fakePatcherQueries) UpdateLarkOutboundCardStatus(ctx context.Context, arg UpdateOutboundCardStatusParams) error {
f.mu.Lock()
defer f.mu.Unlock()
f.statusUpdates = append(f.statusUpdates, arg)
return nil
}
type fakeCredentials struct{ secret string }
func (f fakeCredentials) DecryptAppSecret(inst Installation) (string, error) {
return f.secret, nil
}
type fakeAPIClient struct {
mu sync.Mutex
sent []SendCardParams
patched []PatchCardParams
textSent []SendTextParams
mdCardSent []SendMarkdownCardParams
sendReturn string
sendErr error
patchErr error
textSendErr error
textSendReturn string
mdCardErr error
mdCardReturn string
bindingSent []BindingPromptParams
// threadReplyErr, when non-nil, is returned by the three send
// methods whenever the call carries a thread ReplyTarget, while the
// attempt is still recorded. Tests inject either a classified
// *APIError (to exercise the chat-level fallback) or an ambiguous
// transport error (to assert no fallback happens).
threadReplyErr error
}
// errThreadReplyClassified is a Lark business error the fallback path
// recognizes (230071 = group does not support reply in thread), so a
// thread send that returns it triggers the chat-level retry.
var errThreadReplyClassified = &APIError{Op: "send text message", Code: 230071, Msg: "group does not support reply in thread"}
// errThreadReplyTransport is an ambiguous, non-classified failure: the
// fallback path must NOT retry it at chat level.
var errThreadReplyTransport = errors.New("fake: transport failure")
func (f *fakeAPIClient) IsConfigured() bool { return true }
func (f *fakeAPIClient) SendInteractiveCard(ctx context.Context, p SendCardParams) (string, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.sent = append(f.sent, p)
if f.threadReplyErr != nil && p.ReplyTarget.IsSet() {
return "", f.threadReplyErr
}
return f.sendReturn, f.sendErr
}
func (f *fakeAPIClient) PatchInteractiveCard(ctx context.Context, p PatchCardParams) error {
f.mu.Lock()
defer f.mu.Unlock()
f.patched = append(f.patched, p)
return f.patchErr
}
func (f *fakeAPIClient) SendTextMessage(ctx context.Context, p SendTextParams) (string, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.textSent = append(f.textSent, p)
if f.threadReplyErr != nil && p.ReplyTarget.IsSet() {
return "", f.threadReplyErr
}
return f.textSendReturn, f.textSendErr
}
func (f *fakeAPIClient) SendMarkdownCard(ctx context.Context, p SendMarkdownCardParams) (string, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.mdCardSent = append(f.mdCardSent, p)
if f.threadReplyErr != nil && p.ReplyTarget.IsSet() {
return "", f.threadReplyErr
}
return f.mdCardReturn, f.mdCardErr
}
func (f *fakeAPIClient) SendBindingPromptCard(ctx context.Context, p BindingPromptParams) error {
f.mu.Lock()
defer f.mu.Unlock()
f.bindingSent = append(f.bindingSent, p)
return nil
}
func (f *fakeAPIClient) GetBotInfo(ctx context.Context, creds InstallationCredentials) (BotInfo, error) {
return BotInfo{}, nil
}
func (f *fakeAPIClient) GetMessage(ctx context.Context, creds InstallationCredentials, messageID string) ([]LarkMessage, error) {
return nil, nil
}
func (f *fakeAPIClient) ListChatMessages(ctx context.Context, creds InstallationCredentials, p ListMessagesParams) ([]LarkMessage, error) {
return nil, nil
}
func (f *fakeAPIClient) BatchGetUsers(ctx context.Context, creds InstallationCredentials, openIDs []string) (map[string]string, error) {
return nil, nil
}
func (f *fakeAPIClient) AddMessageReaction(ctx context.Context, p AddReactionParams) (string, error) {
return "fake-reaction-id", nil
}
func (f *fakeAPIClient) DeleteMessageReaction(ctx context.Context, p DeleteReactionParams) error {
return nil
}
func newTestPatcher(t *testing.T) (*Patcher, *fakePatcherQueries, *fakeAPIClient) {
t.Helper()
q := &fakePatcherQueries{
binding: ChatSessionBinding{
ChatSessionID: uuidFromString(t, "cccccccc-cccc-cccc-cccc-cccccccccccc"),
InstallationID: uuidFromString(t, "1111aaaa-1111-1111-1111-111111111111"),
ChannelChatID: "oc_test_chat",
ChatType: "p2p",
},
installation: Installation{
ID: uuidFromString(t, "1111aaaa-1111-1111-1111-111111111111"),
AppID: "cli_test_app",
AppSecretEncrypted: []byte("ciphertext"),
Status: string(InstallationActive),
AgentID: uuidFromString(t, "aaaa1111-aaaa-aaaa-aaaa-aaaaaaaaaaaa"),
},
agent: db.Agent{Name: "TestAgent"},
cardErr: pgx.ErrNoRows,
}
api := &fakeAPIClient{sendReturn: "lark_card_msg_1", textSendReturn: "lark_text_msg_1"}
p := NewPatcher(q, fakeCredentials{secret: "shh"}, api, PatcherConfig{
Logger: newDiscardLogger(),
Now: time.Now,
})
return p, q, api
}
// TestPatcherSendsPlainTextOnChatDone pins the new behaviour Bohan asked
// for: when the agent finishes replying, the Patcher posts the reply as
// a plain Lark IM text message (msg_type=text), not nested inside an
// interactive card. This is the load-bearing UX call — the prior card
// chrome made every reply look like a system notification.
func TestPatcherSendsPlainTextOnChatDone(t *testing.T) {
p, q, api := newTestPatcher(t)
taskID := uuidFromString(t, "ee333333-ee33-ee33-ee33-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Content: "Hello! I'm cc, a coding agent…",
},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 1 {
t.Fatalf("expected one SendTextMessage call on ChatDone; got %d", len(api.textSent))
}
got := api.textSent[0]
if got.Text != "Hello! I'm cc, a coding agent…" {
t.Errorf("text mismatch: got %q", got.Text)
}
if got.ChatID != ChatID(q.binding.ChannelChatID) {
t.Errorf("chat_id mismatch: got %q want %q", got.ChatID, q.binding.ChannelChatID)
}
if got.InstallationID.AppID != "cli_test_app" {
t.Errorf("expected installation app_id propagated; got %q", got.InstallationID.AppID)
}
if len(api.sent) != 0 || len(api.patched) != 0 {
t.Errorf("ChatDone must NOT send / patch any card; got sent=%d patched=%d",
len(api.sent), len(api.patched))
}
}
// TestPatcherRoutesMarkdownReplyToCard pins the two-path chat reply:
// when the agent's body contains markdown syntax, the Patcher MUST
// route to SendMarkdownCard (schema-2.0 interactive card with a
// `tag: "markdown"` body element) so Lark renders the formatting
// instead of leaving raw `**bold**` / `# heading` characters in the
// transcript. Plain prose continues to go through SendTextMessage.
func TestPatcherRoutesMarkdownReplyToCard(t *testing.T) {
p, q, api := newTestPatcher(t)
taskID := uuidFromString(t, "ee444444-ee44-ee44-ee44-eeeeeeeeeeee")
body := "# Summary\n\n- bullet one\n- bullet two\n\n```go\nfunc f() {}\n```\n"
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Content: body,
},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.mdCardSent) != 1 {
t.Fatalf("expected one SendMarkdownCard call; got %d", len(api.mdCardSent))
}
got := api.mdCardSent[0]
if got.Markdown != body {
t.Errorf("markdown body must be forwarded verbatim; got %q", got.Markdown)
}
if got.ChatID != ChatID(q.binding.ChannelChatID) {
t.Errorf("chat_id mismatch: got %q want %q", got.ChatID, q.binding.ChannelChatID)
}
if len(api.textSent) != 0 {
t.Errorf("markdown body must NOT also fire SendTextMessage; got %d", len(api.textSent))
}
if len(api.sent) != 0 || len(api.patched) != 0 {
t.Errorf("ChatDone must NOT use legacy card paths; sent=%d patched=%d", len(api.sent), len(api.patched))
}
}
// TestPatcherRoutesPlainReplyToText is the inverse: a short prose
// reply without any markdown syntax should stay on the cheap
// msg_type=text path so the user sees a normal IM bubble.
func TestPatcherRoutesPlainReplyToText(t *testing.T) {
p, q, api := newTestPatcher(t)
taskID := uuidFromString(t, "ee555555-ee55-ee55-ee55-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Content: "Sure, on it.",
},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 1 {
t.Fatalf("plain prose must take the text path; got %d text sends", len(api.textSent))
}
if len(api.mdCardSent) != 0 {
t.Errorf("plain prose must NOT wrap in a markdown card; got %d card sends", len(api.mdCardSent))
}
}
// TestPatcherDropsEmptyChatReply guards the fallback we deliberately
// removed: the previous design rendered "Done." when content was
// empty. Now an empty Content is silently dropped (no text message
// sent at all). Showing nothing is better than showing the misleading
// "Done." fallback, which Bohan reported confused him in the live env.
func TestPatcherDropsEmptyChatReply(t *testing.T) {
p, q, api := newTestPatcher(t)
taskID := uuidFromString(t, "ee777777-ee77-ee77-ee77-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Content: "",
},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 0 {
t.Errorf("empty content must drop, not render the Done. fallback; got %d text sends", len(api.textSent))
}
}
func TestPatcherSkipsWhenNoChatSessionBinding(t *testing.T) {
p, q, api := newTestPatcher(t)
q.bindingErr = pgx.ErrNoRows
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(uuidFromString(t, "ee222222-ee22-ee22-ee22-eeeeeeeeeeee")),
ChatSessionID: uuidString(uuidFromString(t, "cc222222-cc22-cc22-cc22-cccccccccccc")),
Payload: protocol.ChatDonePayload{
Content: "irrelevant — no binding",
},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 0 || len(api.sent) != 0 {
t.Fatalf("web-only chat sessions must produce no outbound; got text=%d cards=%d",
len(api.textSent), len(api.sent))
}
}
// TestPatcherFailEventSendsErrorCard verifies the failure path still
// surfaces a card. The visual distinction between a successful reply
// (plain text bubble) and a failure (red header card) is genuinely
// useful — and failures are rare enough that the card chrome isn't
// noisy. One-shot send (no patching of any prior thinking card,
// because there isn't one anymore).
func TestPatcherFailEventSendsErrorCard(t *testing.T) {
p, q, api := newTestPatcher(t)
taskID := uuidFromString(t, "ee444444-ee44-ee44-ee44-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventTaskFailed,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: map[string]any{
"task_id": uuidString(taskID),
"chat_session_id": uuidString(q.binding.ChatSessionID),
"error": "boom",
},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.sent) != 1 {
t.Fatalf("fail event must send an error card; got %d card sends", len(api.sent))
}
if len(api.patched) != 0 {
t.Errorf("fail event must NOT patch any card (no prior card lifecycle); got %d patches", len(api.patched))
}
if !strings.Contains(api.sent[0].CardJSON, "boom") {
t.Errorf("error card body should embed the error message; got %s", api.sent[0].CardJSON)
}
}
func TestPatcherSwallowsInstallationLoadErrors(t *testing.T) {
p, q, api := newTestPatcher(t)
q.installationErr = errors.New("db down")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(uuidFromString(t, "ee555555-ee55-ee55-ee55-eeeeeeeeeeee")),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{
Content: "would-be reply",
},
})
// The patcher logs but never panics; no outbound.
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 0 || len(api.sent) != 0 {
t.Fatalf("DB failure must not produce outbound; got text=%d cards=%d",
len(api.textSent), len(api.sent))
}
}
// TestPatcherIgnoresEventTaskCompletedForChatTasks pins the no-extra-send
// invariant. TaskService publishes ChatDone (with content) immediately
// before TaskCompleted (without content) for every chat task. The
// Patcher must NOT react to TaskCompleted — doing so would either
// re-send the same text reply (duplicate bubble) or send the "Done."
// fallback (the original bug Bohan reported). The fix is to leave
// EventTaskCompleted unsubscribed; this test asserts exactly one
// outbound text message from the sequence.
func TestPatcherIgnoresEventTaskCompletedForChatTasks(t *testing.T) {
p, q, api := newTestPatcher(t)
taskID := uuidFromString(t, "ee666666-ee66-ee66-ee66-eeeeeeeeeeee")
// Step 1: ChatDone arrives with the real agent reply. Plain text
// is sent to Lark.
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Content: "Hello! I'm cc, a coding agent…",
},
})
// Step 2: TaskCompleted fires immediately after with no content.
// The Patcher MUST NOT send a second message — neither a
// duplicate of the reply nor the "Done." fallback.
p.handleEvent(events.Event{
Type: protocol.EventTaskCompleted,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: map[string]any{
"task_id": uuidString(taskID),
"chat_session_id": uuidString(q.binding.ChatSessionID),
"status": "completed",
},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 1 {
t.Fatalf("exactly one text send expected (ChatDone); EventTaskCompleted must be ignored. Got %d sends", len(api.textSent))
}
if api.textSent[0].Text != "Hello! I'm cc, a coding agent…" {
t.Errorf("text content mismatch; got %q", api.textSent[0].Text)
}
if len(api.sent) != 0 || len(api.patched) != 0 {
t.Errorf("no card outbound expected on the success path; got sent=%d patched=%d",
len(api.sent), len(api.patched))
}
}
// TestDefaultRendererConfigCarriesUpdateMulti pins the streaming-card
// contract: Lark refuses PatchInteractiveCard on a card whose config
// does not declare update_multi=true. Since the Patcher's whole
// raison d'être is to send a thinking card and then patch it forward
// to streaming/final/error, ANY kind missing update_multi would make
// the patch silently no-op against Lark while the local DB row still
// flips. Hence the assertion covers every kind, not just the final
// patched kinds.
func TestDefaultRendererConfigCarriesUpdateMulti(t *testing.T) {
r := NewDefaultRenderer()
for _, kind := range []CardKind{CardKindThinking, CardKindRunning, CardKindFinal, CardKindError} {
t.Run(string(kind), func(t *testing.T) {
out, err := r.Render(RenderInput{Kind: kind, Content: "x", ErrorMessage: "y"})
if err != nil {
t.Fatalf("render: %v", err)
}
var doc map[string]any
if err := json.Unmarshal([]byte(out.JSON), &doc); err != nil {
t.Fatalf("decode card json: %v", err)
}
cfg, ok := doc["config"].(map[string]any)
if !ok {
t.Fatalf("missing config block: %v", doc)
}
if v, _ := cfg["update_multi"].(bool); !v {
t.Errorf("config.update_multi must be true so subsequent patches apply; got %v", cfg)
}
if v, _ := cfg["wide_screen_mode"].(bool); !v {
t.Errorf("config.wide_screen_mode regression: %v", cfg)
}
})
}
}
// TestPatcherRepliesInThreadWhenTriggerWasInThread pins the core
// behavior of this feature: when the chat binding's most-recent trigger
// lived inside a Lark topic (last_lark_thread_id set), the agent reply
// is routed through the reply endpoint targeting that message with
// reply_in_thread=true, so it lands inside the 话题 instead of the group.
func TestPatcherRepliesInThreadWhenTriggerWasInThread(t *testing.T) {
p, q, api := newTestPatcher(t)
q.binding.LastMessageID = pgtype.Text{String: "om_trigger", Valid: true}
q.binding.LastThreadID = pgtype.Text{String: "omt_topic", Valid: true}
taskID := uuidFromString(t, "ee666666-ee66-ee66-ee66-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{Content: "in-thread reply"},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 1 {
t.Fatalf("expected one text send; got %d", len(api.textSent))
}
got := api.textSent[0].ReplyTarget
if got.MessageID != "om_trigger" || !got.InThread {
t.Errorf("expected thread reply target {om_trigger, InThread:true}; got %+v", got)
}
}
// TestPatcherSendsToChatWhenNoThread verifies that a non-thread trigger
// (no last_lark_thread_id on the binding) keeps the historical
// chat-level send: ReplyTarget stays empty so SendTextMessage targets
// the chat by chat_id. This is the no-behavior-change guarantee for
// normal group / p2p chats.
func TestPatcherSendsToChatWhenNoThread(t *testing.T) {
p, q, api := newTestPatcher(t)
// binding has a message id but NO thread id → must not thread.
q.binding.LastMessageID = pgtype.Text{String: "om_trigger", Valid: true}
taskID := uuidFromString(t, "ee777777-ee77-ee77-ee77-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{Content: "plain reply"},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 1 {
t.Fatalf("expected one text send; got %d", len(api.textSent))
}
if api.textSent[0].ReplyTarget.IsSet() {
t.Errorf("non-thread trigger must NOT route through the reply endpoint; got %+v",
api.textSent[0].ReplyTarget)
}
}
// TestPatcherThreadReplyMarkdownRoutesToThread verifies the markdown
// card path also threads when the trigger was in a topic.
func TestPatcherThreadReplyMarkdownRoutesToThread(t *testing.T) {
p, q, api := newTestPatcher(t)
q.binding.LastMessageID = pgtype.Text{String: "om_trigger", Valid: true}
q.binding.LastThreadID = pgtype.Text{String: "omt_topic", Valid: true}
taskID := uuidFromString(t, "ee888888-ee88-ee88-ee88-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{Content: "# heading\n- bullet"},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.mdCardSent) != 1 {
t.Fatalf("expected one markdown card send; got %d", len(api.mdCardSent))
}
got := api.mdCardSent[0].ReplyTarget
if got.MessageID != "om_trigger" || !got.InThread {
t.Errorf("expected markdown thread reply target {om_trigger, InThread:true}; got %+v", got)
}
}
// TestPatcherThreadReplyFallsBackToChatLevel verifies that when a
// threaded send fails with a classified "topic cannot receive this
// reply" Lark error (e.g. the trigger message was recalled or the topic
// was aggregated), the patcher retries once at the chat level so the
// agent's reply is never silently lost.
func TestPatcherThreadReplyFallsBackToChatLevel(t *testing.T) {
p, q, api := newTestPatcher(t)
api.threadReplyErr = errThreadReplyClassified
q.binding.LastMessageID = pgtype.Text{String: "om_trigger", Valid: true}
q.binding.LastThreadID = pgtype.Text{String: "omt_topic", Valid: true}
taskID := uuidFromString(t, "ee999999-ee99-ee99-ee99-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{Content: "reply that must survive"},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 2 {
t.Fatalf("expected two text sends (thread attempt + chat-level fallback); got %d", len(api.textSent))
}
if !api.textSent[0].ReplyTarget.IsSet() {
t.Errorf("first attempt should be the thread reply; got %+v", api.textSent[0].ReplyTarget)
}
if api.textSent[1].ReplyTarget.IsSet() {
t.Errorf("fallback attempt must be chat-level (empty ReplyTarget); got %+v", api.textSent[1].ReplyTarget)
}
}
// TestPatcherThreadReplyDoesNotFallBackOnAmbiguousError verifies that a
// non-classified failure (transport error, 5xx, timeout, rate limit)
// from the threaded send is NOT retried at chat level: a blind retry
// could duplicate the reply or leak a thread-only reply into the group.
func TestPatcherThreadReplyDoesNotFallBackOnAmbiguousError(t *testing.T) {
p, q, api := newTestPatcher(t)
api.threadReplyErr = errThreadReplyTransport
q.binding.LastMessageID = pgtype.Text{String: "om_trigger", Valid: true}
q.binding.LastThreadID = pgtype.Text{String: "omt_topic", Valid: true}
taskID := uuidFromString(t, "ee888888-ee88-ee88-ee88-eeeeeeeeeeee")
p.handleEvent(events.Event{
Type: protocol.EventChatDone,
TaskID: uuidString(taskID),
ChatSessionID: uuidString(q.binding.ChatSessionID),
Payload: protocol.ChatDonePayload{Content: "reply that must not duplicate"},
})
api.mu.Lock()
defer api.mu.Unlock()
if len(api.textSent) != 1 {
t.Fatalf("expected a single thread attempt with no chat-level fallback; got %d sends", len(api.textSent))
}
if !api.textSent[0].ReplyTarget.IsSet() {
t.Errorf("the single attempt should be the thread reply; got %+v", api.textSent[0].ReplyTarget)
}
}