diff --git a/server/cmd/server/main.go b/server/cmd/server/main.go index 2ca5e4cb906..58566a35487 100644 --- a/server/cmd/server/main.go +++ b/server/cmd/server/main.go @@ -538,7 +538,11 @@ func main() { ) } if h.ChannelRouter != nil { - h.ChannelRouter.Drain() + drainCtx, drainCancel := context.WithTimeout(context.Background(), 10*time.Second) + if !h.ChannelRouter.Drain(drainCtx) { + slog.Warn("channel router: drain deadline reached; deferred media fallback remains durable") + } + drainCancel() } } diff --git a/server/cmd/server/router.go b/server/cmd/server/router.go index 843641c56b3..3177ffae10e 100644 --- a/server/cmd/server/router.go +++ b/server/cmd/server/router.go @@ -391,8 +391,9 @@ func NewRouterWithOptions(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus Credentials: installSvc, Logger: slog.Default(), }) + mediaResolver := lark.NewFeishuMediaResolver(larkClient, installSvc, store, slog.Default()) channelRouter.Register(channel.TypeFeishu, lark.NewFeishuResolverSet( - cs, feishuSession, auditLogger, resolverReplier, typingIndicator, + cs, feishuSession, auditLogger, resolverReplier, typingIndicator, mediaResolver, )) slog.Info("lark inbound pipeline wired", "connector", connectorLabel) diff --git a/server/internal/handler/chat_input_ownership_test.go b/server/internal/handler/chat_input_ownership_test.go index 93cfe4a3442..14db8220a89 100644 --- a/server/internal/handler/chat_input_ownership_test.go +++ b/server/internal/handler/chat_input_ownership_test.go @@ -349,12 +349,40 @@ func insertChannelChatTask(t *testing.T, ctx context.Context, agentID, runtimeID return taskID } -// TestCompleteTask_ChannelEmptyOutputWritesNoRow pins the MUL-4351 review fix: -// a legacy/channel task (chat_input_task_id NULL) that completes with empty -// output must NOT write an assistant row — so chat:done carries empty content -// and the Slack/Lark outbound keeps silently dropping it. The no_response -// fallback body must never reach an external channel. A non-empty channel -// completion still writes an ordinary message. +// insertSealedChannelChatTask creates a running channel-shaped chat task the +// way EnqueueChatTask now does: the task owns its input batch +// (chat_input_task_id = id) and the sealed user message carries the immutable +// channel_ingested stamp. +func insertSealedChannelChatTask(t *testing.T, ctx context.Context, agentID, runtimeID, sessionID, content string) string { + t.Helper() + var taskID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO agent_task_queue (agent_id, runtime_id, chat_session_id, status, priority, started_at, dispatched_at) + VALUES ($1, $2, $3, 'running', 2, now(), now()) + RETURNING id + `, agentID, runtimeID, sessionID).Scan(&taskID); err != nil { + t.Fatalf("setup: create sealed channel chat task: %v", err) + } + if _, err := testPool.Exec(ctx, ` + UPDATE agent_task_queue SET chat_input_task_id = id WHERE id = $1 + `, taskID); err != nil { + t.Fatalf("setup: set input owner: %v", err) + } + if _, err := testPool.Exec(ctx, ` + INSERT INTO chat_message (chat_session_id, role, content, task_id, channel_ingested) + VALUES ($1, 'user', $2, $3, TRUE) + `, sessionID, content, taskID); err != nil { + t.Fatalf("setup: seal channel user message: %v", err) + } + return taskID +} + +// TestCompleteTask_ChannelEmptyOutputWritesNoRow pins the MUL-4351 review fix +// for the LEGACY channel shape (chat_input_task_id NULL): an empty completion +// must NOT write an assistant row — so chat:done carries empty content and the +// Slack/Lark outbound keeps silently dropping it. The no_response fallback +// body must never reach an external channel. A non-empty channel completion +// still writes an ordinary message. func TestCompleteTask_ChannelEmptyOutputWritesNoRow(t *testing.T) { if testHandler == nil { t.Skip("database not available") @@ -384,3 +412,65 @@ func TestCompleteTask_ChannelEmptyOutputWritesNoRow(t *testing.T) { t.Fatalf("channel message = kind %q content %q, want message/'channel reply'", rows[0].MessageKind, rows[0].Content) } } + +// TestCompleteTask_SealedChannelEmptyOutputWritesNoRow pins the silent-drop +// contract for the NEW channel shape: sealed channel tasks own their input +// batch (chat_input_task_id = id), so the discriminator is the immutable +// channel_ingested stamp, not a NULL owner. An empty completion must write no +// assistant row — the outbound patcher forwards any non-empty content +// verbatim, so a no_response fallback row would be pushed to Feishu/Slack. +func TestCompleteTask_SealedChannelEmptyOutputWritesNoRow(t *testing.T) { + if testHandler == nil { + t.Skip("database not available") + } + ctx := context.Background() + agentID, sessionID, runtimeID, _ := setupDirectChatSession(t, ctx, "sealed channel chat") + + emptyTask := insertSealedChannelChatTask(t, ctx, agentID, runtimeID, sessionID, "[Image]") + if _, err := testHandler.TaskService.CompleteTask(ctx, parseUUID(emptyTask), completeResult(t, " "), "", ""); err != nil { + t.Fatalf("complete sealed channel task (empty): %v", err) + } + if rows := assistantRows(t, ctx, sessionID); len(rows) != 0 { + t.Fatalf("sealed channel empty completion must write NO assistant row, got %d", len(rows)) + } + + // Non-empty output still writes one ordinary message. + textTask := insertSealedChannelChatTask(t, ctx, agentID, runtimeID, sessionID, "hello") + if _, err := testHandler.TaskService.CompleteTask(ctx, parseUUID(textTask), completeResult(t, "sealed channel reply"), "", ""); err != nil { + t.Fatalf("complete sealed channel task (text): %v", err) + } + rows := assistantRows(t, ctx, sessionID) + if len(rows) != 1 || rows[0].MessageKind != protocol.ChatMessageKindMessage || rows[0].Content != "sealed channel reply" { + t.Fatalf("sealed channel non-empty completion = %+v, want one ordinary message", rows) + } +} + +// TestCompleteTask_SealedChannelRetryEmptyOutputWritesNoRow: an auto-retry +// clone inherits its parent's chat_input_task_id while the sealed messages +// stay tagged with the parent's id. The provenance check must follow the +// inherited owner — keying it off the retry's own id would misread the task +// as direct and push the no_response fallback to the external channel. +func TestCompleteTask_SealedChannelRetryEmptyOutputWritesNoRow(t *testing.T) { + if testHandler == nil { + t.Skip("database not available") + } + ctx := context.Background() + agentID, sessionID, runtimeID, _ := setupDirectChatSession(t, ctx, "sealed channel retry chat") + + parentTask := insertSealedChannelChatTask(t, ctx, agentID, runtimeID, sessionID, "[Video]") + var retryTask string + if err := testPool.QueryRow(ctx, ` + INSERT INTO agent_task_queue (agent_id, runtime_id, chat_session_id, status, priority, started_at, dispatched_at, parent_task_id, retry_of_task_id, chat_input_task_id) + VALUES ($1, $2, $3, 'running', 2, now(), now(), $4, $4, $4) + RETURNING id + `, agentID, runtimeID, sessionID, parentTask).Scan(&retryTask); err != nil { + t.Fatalf("setup: create retry clone: %v", err) + } + + if _, err := testHandler.TaskService.CompleteTask(ctx, parseUUID(retryTask), completeResult(t, ""), "", ""); err != nil { + t.Fatalf("complete sealed channel retry (empty): %v", err) + } + if rows := assistantRows(t, ctx, sessionID); len(rows) != 0 { + t.Fatalf("sealed channel retry empty completion must write NO assistant row, got %d", len(rows)) + } +} diff --git a/server/internal/handler/daemon.go b/server/internal/handler/daemon.go index 6b1b6985a9e..eba361492e5 100644 --- a/server/internal/handler/daemon.go +++ b/server/internal/handler/daemon.go @@ -2107,16 +2107,19 @@ func (h *Handler) buildClaimedTaskResponse(r *http.Request, task *db.AgentTaskQu } } // Resolve the user-message input batch for this run. A task-owned - // direct-chat task (chat_input_task_id set, MUL-4351) reads exactly - // the user messages tagged with its own input owner, so a message - // that arrived after this turn was sealed can never be absorbed here. - // Legacy and channel (Slack/Lark) tasks carry a NULL owner and keep - // the trailing-message selector — the run of user messages after the - // last assistant row, which also covers a debounced burst (MUL-2968: - // "看上海天气" then "还有青岛" must both be delivered) — so a rolling - // deploy never replays their history. Attachments are collected per - // included message so the agent can `multica attachment download ` - // (the inline markdown URL is signed + 30-min expiring on the CDN). + // task (chat_input_task_id set) reads exactly the user messages + // tagged with its own input owner, so a message that arrived after + // this turn was sealed can never be absorbed here. Direct-chat + // tasks have owned their single message since MUL-4351; channel + // (Slack/Lark) tasks now seal their trailing batch at enqueue too. + // Only legacy tasks created before that deploy carry a NULL owner + // and keep the trailing-message selector — the run of user messages + // after the last assistant row, which also covers a debounced burst + // (MUL-2968: "看上海天气" then "还有青岛" must both be delivered) — + // so a rolling deploy never replays their history. Attachments are + // collected per included message so the agent can + // `multica attachment download ` (the inline markdown URL is + // signed + 30-min expiring on the CDN). var unanswered []db.ChatMessage var inputLoadErr error if task.ChatInputTaskID.Valid { @@ -2645,6 +2648,9 @@ func (h *Handler) ResolveTaskSkillBundles(w http.ResponseWriter, r *http.Request // these, not just the latest. Every completed or failed run writes an // assistant row, so the anchor advances one turn at a time; the result is the // whole slice on the first turn and exactly the new message(s) thereafter. +// Legacy channel tasks also stop at the first unexpired media marker so an old +// run cannot consume a placeholder that has not been bound yet. New channel +// tasks use task-owned input batches and do not depend on this fallback. func trailingUserMessages(msgs []db.ChatMessage) []db.ChatMessage { start := 0 for i := len(msgs) - 1; i >= 0; i-- { @@ -2653,7 +2659,15 @@ func trailingUserMessages(msgs []db.ChatMessage) []db.ChatMessage { break } } - return msgs[start:] + msgs = msgs[start:] + now := time.Now() + for i := range msgs { + pending := msgs[i].ChannelMediaPendingUntil + if pending.Valid && pending.Time.After(now) { + return msgs[:i] + } + } + return msgs } // ListPendingTasksByRuntime returns queued/dispatched tasks for a runtime. diff --git a/server/internal/handler/daemon_chat_prompt_test.go b/server/internal/handler/daemon_chat_prompt_test.go index 2f4280f05b8..80682feb25b 100644 --- a/server/internal/handler/daemon_chat_prompt_test.go +++ b/server/internal/handler/daemon_chat_prompt_test.go @@ -2,7 +2,9 @@ package handler import ( "testing" + "time" + "github.com/jackc/pgx/v5/pgtype" db "github.com/multica-ai/multica/server/pkg/db/generated" ) @@ -10,6 +12,14 @@ func msg(role, content string) db.ChatMessage { return db.ChatMessage{Role: role, Content: content} } +func mediaPendingMsg(content string, until time.Time) db.ChatMessage { + return db.ChatMessage{ + Role: "user", + Content: content, + ChannelMediaPendingUntil: pgtype.Timestamptz{Time: until, Valid: true}, + } +} + func contents(msgs []db.ChatMessage) []string { out := make([]string, len(msgs)) for i, m := range msgs { @@ -76,6 +86,23 @@ func TestTrailingUserMessages(t *testing.T) { in: []db.ChatMessage{msg("user", "hi")}, want: []string{"hi"}, }, + { + name: "stops before media that is still pending", + in: []db.ChatMessage{ + msg("user", "ready"), + mediaPendingMsg("[Image]", time.Now().Add(time.Minute)), + msg("user", "after image"), + }, + want: []string{"ready"}, + }, + { + name: "expired media placeholder is ready", + in: []db.ChatMessage{ + mediaPendingMsg("[Image]", time.Now().Add(-time.Minute)), + msg("user", "after image"), + }, + want: []string{"[Image]", "after image"}, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/server/internal/integrations/channel/engine/provenance.go b/server/internal/integrations/channel/engine/provenance.go new file mode 100644 index 00000000000..be663c00689 --- /dev/null +++ b/server/internal/integrations/channel/engine/provenance.go @@ -0,0 +1,34 @@ +package engine + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" + + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +// ChannelProvenanceQueries is the single query the reply-origin check needs. +// *db.Queries satisfies it. +type ChannelProvenanceQueries interface { + TaskHasChannelIngestedMessages(ctx context.Context, taskID pgtype.UUID) (bool, error) +} + +// TaskInputIsChannelIngested reports whether a completed chat task took its +// input from the channel, so its reply (or failure notice) belongs on the +// external platform. Direct (web/mobile) tasks can reuse a channel-bound +// session, but their replies stay in Multica (MUL-4988). +// +// chat_input_task_id alone cannot discriminate: sealed channel tasks own an +// input batch exactly like direct tasks do. The verdict is the immutable +// channel_ingested stamp on the owned batch, keyed by the batch OWNER id so an +// auto-retry clone (which inherits chat_input_task_id while its messages stay +// tagged with the parent) reaches the same verdict as its parent. A NULL owner +// is a pre-sealing channel task — direct tasks have owned their batch since +// MUL-4351 — so it keeps the deliver-by-default behavior #5645 shipped with. +func TaskInputIsChannelIngested(ctx context.Context, q ChannelProvenanceQueries, task db.AgentTaskQueue) (bool, error) { + if !task.ChatInputTaskID.Valid { + return true, nil + } + return q.TaskHasChannelIngestedMessages(ctx, task.ChatInputTaskID) +} diff --git a/server/internal/integrations/channel/engine/resolvers.go b/server/internal/integrations/channel/engine/resolvers.go index 80b1d76a6cc..a4eab790699 100644 --- a/server/internal/integrations/channel/engine/resolvers.go +++ b/server/internal/integrations/channel/engine/resolvers.go @@ -89,16 +89,22 @@ type EnsureSessionParams struct { // AppendParams carries the inputs for SessionBinder.AppendMessage. ClaimToken // is the dedup owner-fence token; the binder runs the dedup Mark INSIDE its // chat_message+session tx so the durable write and the Mark commit atomically. +// MediaPendingUntil persists the placeholder fallback deadline. type AppendParams struct { - SessionID pgtype.UUID - Sender pgtype.UUID - InstallationID pgtype.UUID - Message channel.InboundMessage - ClaimToken pgtype.UUID + SessionID pgtype.UUID + Sender pgtype.UUID + InstallationID pgtype.UUID + Message channel.InboundMessage + ClaimToken pgtype.UUID + MediaPendingUntil pgtype.Timestamptz } // AppendResult reports what AppendMessage decided. type AppendResult struct { + // MessageID is the durable chat_message row created by AppendMessage. + // Detached media processing uses it to link attachments after the + // connector ACK path has completed. + MessageID pgtype.UUID // IssueCommand is non-nil when the message was an /issue command. IssueCommand *IssueCommand // DedupMarked is true when AppendMessage finalized the dedup claim in its @@ -106,6 +112,17 @@ type AppendResult struct { DedupMarked bool } +// BindMediaParams carries stored media references to the post-append +// attachment transaction. MessageID is the durable chat_message created by +// AppendMessage; media downloads must never run inside this transaction. +type BindMediaParams struct { + MessageID pgtype.UUID + SessionID pgtype.UUID + WorkspaceID pgtype.UUID + Sender pgtype.UUID + MediaRefs []channel.MediaRef +} + // IssueCommand is the parsed /issue command. type IssueCommand struct { Title string @@ -161,6 +178,35 @@ type Deduper interface { type SessionBinder interface { EnsureSession(ctx context.Context, p EnsureSessionParams) (pgtype.UUID, error) AppendMessage(ctx context.Context, p AppendParams) (AppendResult, error) + BindMedia(ctx context.Context, p BindMediaParams) error +} + +// MediaResolver resolves platform media after the user message and dedup mark +// are durable. The Router runs it off the connector ACK path and binds any +// returned MediaRefs; the independently scheduled task remains deferred until +// binding finishes or the persisted deadline expires. Implementations are +// best-effort: failures leave the stored placeholder text intact. +type MediaResolver interface { + // HasMedia reports whether msg references platform media that + // ResolveMedia would fetch. The Router calls it synchronously on the + // connector ACK path to decide whether to persist a media deadline and + // queue a resolution job at all, so implementations must be pure + // in-memory checks (no I/O). A false result keeps the message on the + // plain ingest path: no marker, no deferred run, no semaphore slot. + HasMedia(msg channel.InboundMessage) bool + ResolveMedia(ctx context.Context, inst ResolvedInstallation, sender ResolvedIdentity, sessionID pgtype.UUID, msg channel.InboundMessage) channel.InboundMessage + // DiscardMedia best-effort deletes objects that ResolveMedia uploaded but + // that will never gain an attachment row (deadline expiry, known-failed + // BindMedia). Without it those uploads are unreachable orphans: the dedup + // mark commits with the message before media runs, so a redelivered event + // is dropped as a duplicate and never re-resolves (and thus never + // overwrites) these keys, and workspace/session deletion only enumerates + // the attachment table. Implementations also self-invoke it for a + // result-uncertain upload (client error after a possible server-side + // write), whose key never reaches the Router. The Router skips it on + // ErrMediaBindResultUnknown — an unverifiable commit may have persisted + // rows that reference the objects. + DiscardMedia(ctx context.Context, refs []channel.MediaRef) } // Auditor records a dropped inbound event (no message body — drop-audit @@ -200,6 +246,7 @@ type ResolverSet struct { Identity IdentityResolver Dedup Deduper Session SessionBinder + Media MediaResolver Audit Auditor Replier OutboundReplier Typing TypingNotifier @@ -216,6 +263,7 @@ type IssueCreator interface { // trigger a chat run. Shared across platforms. type TaskEnqueuer interface { EnqueueChatTask(ctx context.Context, session db.ChatSession, initiatorUserID pgtype.UUID, forceFreshSession bool) (db.AgentTaskQueue, error) + PromoteChannelChatTasksIfMediaReady(ctx context.Context, sessionID pgtype.UUID) error } // SessionReader reads the rows the debounced flush + /issue identifier need. diff --git a/server/internal/integrations/channel/engine/router.go b/server/internal/integrations/channel/engine/router.go index 1335ea54875..ea1109cf645 100644 --- a/server/internal/integrations/channel/engine/router.go +++ b/server/internal/integrations/channel/engine/router.go @@ -21,8 +21,9 @@ import ( // channel.InboundMessage and calls Handle, which routes by ChannelType to that // platform's registered resolver set and runs the same ordered pipeline for // every platform — installation route → two-phase dedup → group @bot filter → -// identity + membership → ensure session → append+mark → /issue → debounced -// run trigger — then drives the detached outbound replier + typing indicator. +// identity + membership → ensure session → append+mark → /issue → durable +// debounced run trigger + detached media binding — then drives the detached +// outbound replier + typing indicator. // // The core contains no platform specifics: everything platform-shaped lives // behind the resolver interfaces (a feishu ResolverSet is the first @@ -39,7 +40,16 @@ type Router struct { batcher *pendingBatcher replyTimeout time.Duration + mediaTimeout time.Duration + mediaCtx context.Context + mediaCancel context.CancelFunc + mediaSem chan struct{} replyWg sync.WaitGroup + mediaWg sync.WaitGroup + + mediaQueueMu sync.Mutex + mediaQueues map[string]*mediaQueueEntry + stopping bool logger *slog.Logger @@ -53,7 +63,18 @@ type RouterConfig struct { // call. It runs off the connector ACK path, so it must stay strictly // under the platform ACK deadline (Lark: 3s). Defaults to 2.5s. ReplyTimeout time.Duration - Logger *slog.Logger + // MediaTimeout caps detached best-effort media download, upload, and + // attachment binding for one message. The budget starts at append time + // (it must match the persisted fire_at fallback), so it also spans any + // wait behind earlier media in the same session and for a global + // concurrency slot. Defaults to 45s. + MediaTimeout time.Duration + // MediaConcurrency caps concurrent media resolutions across all + // sessions, bounding burst memory (unknown-length uploads buffer up to + // the 100 MiB resource cap each) and platform download pressure. + // Per-session ordering is unaffected. Defaults to 8. + MediaConcurrency int + Logger *slog.Logger } // NewRouter builds a Router around the shared (platform-agnostic) services: @@ -64,20 +85,36 @@ func NewRouter(issues IssueCreator, tasks TaskEnqueuer, reader SessionReader, cf if cfg.ReplyTimeout == 0 { cfg.ReplyTimeout = 2500 * time.Millisecond } + if cfg.MediaTimeout == 0 { + cfg.MediaTimeout = 45 * time.Second + } + if cfg.MediaConcurrency == 0 { + cfg.MediaConcurrency = 8 + } if cfg.Logger == nil { cfg.Logger = slog.Default() } + mediaCtx, mediaCancel := context.WithCancel(context.Background()) return &Router{ sets: make(map[channel.Type]ResolverSet), issues: issues, tasks: tasks, reader: reader, replyTimeout: cfg.ReplyTimeout, + mediaTimeout: cfg.MediaTimeout, + mediaCtx: mediaCtx, + mediaCancel: mediaCancel, + mediaSem: make(chan struct{}, cfg.MediaConcurrency), logger: cfg.Logger, pendingFresh: make(map[string]bool), + mediaQueues: make(map[string]*mediaQueueEntry), } } +type mediaQueueEntry struct { + tail chan struct{} +} + // Register binds a platform's ResolverSet under t. Call at boot, before Run. // Registering an empty Type or a set missing a required resolver is ignored. func (r *Router) Register(t channel.Type, set ResolverSet) { @@ -97,13 +134,31 @@ func (r *Router) EnableRunBatching(window time.Duration) { r.batcher = newPendingBatcher(window) } -// Drain flushes debounced run triggers and joins in-flight reply goroutines. -// Call on shutdown AFTER the Supervisor has stopped delivering events. -func (r *Router) Drain() { - if r.batcher != nil { - r.batcher.FlushAll() +// Drain cancels detached media processing, flushes debounced run triggers, and +// joins media/reply goroutines until ctx ends. It returns whether everything +// completed. Call on shutdown AFTER the Supervisor has stopped delivering +// events; timed-out media retains its durable placeholder fallback. +func (r *Router) Drain(ctx context.Context) bool { + r.mediaQueueMu.Lock() + r.stopping = true + r.mediaCancel() + r.mediaQueueMu.Unlock() + + done := make(chan struct{}) + go func() { + if r.batcher != nil { + r.batcher.FlushAll() + } + r.mediaWg.Wait() + r.replyWg.Wait() + close(done) + }() + select { + case <-done: + return true + case <-ctx.Done(): + return false } - r.replyWg.Wait() } // ErrNoResolverSet is returned by Handle when a message arrives for a channel @@ -258,14 +313,22 @@ func (r *Router) processClaimed(ctx context.Context, set ResolverSet, msg channe // Single tx; an error rolled it back, nothing landed. Release. return Result{}, finalizeRelease, fmt.Errorf("ensure chat session: %w", err) } - // 6. Append message + in-tx dedup Mark — the durable transition point. + // The media deadline is persisted only when the message actually carries + // media: a plain text message must never wait behind the media semaphore + // or fall back to the 45s deadline after a crash. + mediaPendingUntil := pgtype.Timestamptz{} + resolveMedia := set.Media != nil && set.Media.HasMedia(msg) + if resolveMedia { + mediaPendingUntil = pgtype.Timestamptz{Time: time.Now().Add(r.mediaTimeout), Valid: true} + } appendRes, err := set.Session.AppendMessage(ctx, AppendParams{ - SessionID: sessionID, - Sender: identity.UserID, - InstallationID: inst.ID, - Message: msg, - ClaimToken: claimToken, + SessionID: sessionID, + Sender: identity.UserID, + InstallationID: inst.ID, + Message: msg, + ClaimToken: claimToken, + MediaPendingUntil: mediaPendingUntil, }) if err != nil { if errors.Is(err, ErrClaimLost) { @@ -312,9 +375,139 @@ func (r *Router) processClaimed(ctx context.Context, set ResolverSet, msg channe // session creator (group sessions are creator=installer). Latest sender // in a window wins (MUL-2645). r.scheduleRun(set, inst, msg, sessionID, identity.UserID) + if resolveMedia { + r.enqueueMedia(set, inst, identity, appendRes.MessageID, msg, sessionID, mediaPendingUntil.Time) + } return res, postAppendFinalize, nil } +// enqueueMedia detaches remote media I/O from Handle while preserving message +// order within a chat session. Run scheduling is independent and durable: the +// task service defers a task to the persisted media deadline, then media +// completion promotes it early. +func (r *Router) enqueueMedia(set ResolverSet, inst ResolvedInstallation, identity ResolvedIdentity, chatMessageID pgtype.UUID, msg channel.InboundMessage, sessionID pgtype.UUID, deadline time.Time) { + key := keyForSession(sessionID) + done := make(chan struct{}) + + r.mediaQueueMu.Lock() + if r.stopping { + r.mediaQueueMu.Unlock() + return + } + entry, ok := r.mediaQueues[key] + var previous <-chan struct{} + if !ok { + entry = &mediaQueueEntry{} + r.mediaQueues[key] = entry + } else { + previous = entry.tail + } + entry.tail = done + r.mediaWg.Add(1) + r.mediaQueueMu.Unlock() + + go func() { + defer r.mediaWg.Done() + defer close(done) + defer r.finishMediaQueue(key, done) + if previous != nil { + select { + case <-previous: + case <-r.mediaCtx.Done(): + } + } + select { + case r.mediaSem <- struct{}{}: + defer func() { <-r.mediaSem }() + case <-r.mediaCtx.Done(): + // Cancelled while queued for a slot: proceed without one. + // ResolveMedia returns immediately on the dead context and only + // the bounded DB finalize runs, preserving prompt marker + // clearing on shutdown. + } + r.resolveAndBindMedia(set, inst, identity, chatMessageID, msg, sessionID, deadline) + }() +} + +const mediaFinalizeTimeout = 5 * time.Second + +func (r *Router) resolveAndBindMedia(set ResolverSet, inst ResolvedInstallation, identity ResolvedIdentity, chatMessageID pgtype.UUID, msg channel.InboundMessage, sessionID pgtype.UUID, deadline time.Time) { + ctx, cancel := context.WithDeadline(r.mediaCtx, deadline) + defer cancel() + + resolved := set.Media.ResolveMedia(ctx, inst, identity, sessionID, msg) + finalizeCtx, finalizeCancel := context.WithTimeout(context.Background(), mediaFinalizeTimeout) + defer finalizeCancel() + var discardRefs []channel.MediaRef + if err := ctx.Err(); err != nil { + // Refs resolved before the deadline already sit in object storage but + // will never gain an attachment row — and the dedup mark committed + // with the message, so a redelivery is dropped and never re-resolves + // these keys. Collected for deletion below, after the user-facing + // marker clear and promotion. + discardRefs = resolved.MediaRefs + resolved.MediaRefs = nil + r.logger.Warn("channel router: media resolution incomplete; using placeholder", + "channel_type", string(msg.Source.ChannelType), + "event_id", msg.EventID, + "message_id", msg.MessageID, + "error", err) + } + if err := set.Session.BindMedia(finalizeCtx, BindMediaParams{ + MessageID: chatMessageID, + SessionID: sessionID, + WorkspaceID: inst.WorkspaceID, + Sender: identity.UserID, + MediaRefs: resolved.MediaRefs, + }); err != nil { + if errors.Is(err, ErrMediaBindResultUnknown) { + // The commit outcome could not be verified; deleting now could + // destroy objects that durably-committed attachment rows + // reference. Keep them — a rare orphan beats a broken attachment. + r.logger.Warn("channel router: media bind outcome unknown; keeping uploads", + "channel_type", string(msg.Source.ChannelType), + "event_id", msg.EventID, + "message_id", msg.MessageID, + "err", err) + } else { + // Known-failed bind: the uploads are unreachable through the + // attachment table — reclaim them. + discardRefs = append(discardRefs, resolved.MediaRefs...) + r.logger.Warn("channel router: media attachment binding failed", + "channel_type", string(msg.Source.ChannelType), + "event_id", msg.EventID, + "message_id", msg.MessageID, + "err", err) + } + } + if err := r.tasks.PromoteChannelChatTasksIfMediaReady(finalizeCtx, sessionID); err != nil { + r.logger.Warn("channel router: media-ready task promotion failed", + "channel_type", string(msg.Source.ChannelType), + "event_id", msg.EventID, + "message_id", msg.MessageID, + "err", err) + } + if len(discardRefs) > 0 { + // A fresh budget, not finalizeCtx: a bind that failed BECAUSE the + // finalize deadline expired must not hand the storage deletes an + // already-dead context, and running last keeps S3 round-trips from + // starving the marker clear / promotion above. + discardCtx, discardCancel := context.WithTimeout(context.Background(), mediaFinalizeTimeout) + defer discardCancel() + set.Media.DiscardMedia(discardCtx, discardRefs) + } +} + +func (r *Router) finishMediaQueue(key string, done chan struct{}) { + r.mediaQueueMu.Lock() + defer r.mediaQueueMu.Unlock() + entry, ok := r.mediaQueues[key] + if !ok || entry.tail != done { + return + } + delete(r.mediaQueues, key) +} + // scheduleRun hands the per-session run trigger to the debouncer (or fires it // inline when batching is disabled). func (r *Router) scheduleRun(set ResolverSet, inst ResolvedInstallation, msg channel.InboundMessage, sessionID, initiatorUserID pgtype.UUID) { diff --git a/server/internal/integrations/channel/engine/router_test.go b/server/internal/integrations/channel/engine/router_test.go index 5afe8b8a7d5..31b8c69661d 100644 --- a/server/internal/integrations/channel/engine/router_test.go +++ b/server/internal/integrations/channel/engine/router_test.go @@ -3,6 +3,7 @@ package engine import ( "context" "errors" + "fmt" "sync" "testing" "time" @@ -68,12 +69,15 @@ func (f *fakeDedup) marks() int { f.mu.Lock(); defer f.mu.Unlock(); return f. func (f *fakeDedup) releases() int { f.mu.Lock(); defer f.mu.Unlock(); return f.relCalls } type fakeBinder struct { + mu sync.Mutex ensureID pgtype.UUID ensureErr error appendResult AppendResult appendErr error + bindErr error lastEnsure EnsureSessionParams lastAppend AppendParams + lastBind BindMediaParams } func (f *fakeBinder) EnsureSession(_ context.Context, p EnsureSessionParams) (pgtype.UUID, error) { @@ -81,9 +85,27 @@ func (f *fakeBinder) EnsureSession(_ context.Context, p EnsureSessionParams) (pg return f.ensureID, f.ensureErr } func (f *fakeBinder) AppendMessage(_ context.Context, p AppendParams) (AppendResult, error) { + f.mu.Lock() + defer f.mu.Unlock() f.lastAppend = p return f.appendResult, f.appendErr } +func (f *fakeBinder) BindMedia(_ context.Context, p BindMediaParams) error { + f.mu.Lock() + defer f.mu.Unlock() + f.lastBind = p + return f.bindErr +} +func (f *fakeBinder) boundMedia() BindMediaParams { + f.mu.Lock() + defer f.mu.Unlock() + return f.lastBind +} +func (f *fakeBinder) appendedParams() AppendParams { + f.mu.Lock() + defer f.mu.Unlock() + return f.lastAppend +} type fakeAuditor struct { mu sync.Mutex @@ -140,6 +162,87 @@ func (f *fakeTyping) OnSettled(_ context.Context, _ pgtype.UUID) { func (f *fakeTyping) calls() int { f.mu.Lock(); defer f.mu.Unlock(); return f.count } func (f *fakeTyping) settledCalls() int { f.mu.Lock(); defer f.mu.Unlock(); return f.settled } +type fakeMedia struct { + mu sync.Mutex + count int + noMedia bool + waitForCancel bool + started chan struct{} + release <-chan struct{} + resolve func(context.Context, channel.InboundMessage) channel.InboundMessage + discardedRefs []channel.MediaRef + discardCalls int + discardCtxErr error +} + +func (f *fakeMedia) lastDiscardCtxErr() error { + f.mu.Lock() + defer f.mu.Unlock() + return f.discardCtxErr +} + +func (f *fakeMedia) HasMedia(_ channel.InboundMessage) bool { + f.mu.Lock() + defer f.mu.Unlock() + return !f.noMedia +} + +func (f *fakeMedia) DiscardMedia(ctx context.Context, refs []channel.MediaRef) { + f.mu.Lock() + defer f.mu.Unlock() + f.discardCalls++ + f.discardCtxErr = ctx.Err() + f.discardedRefs = append(f.discardedRefs, refs...) +} + +func (f *fakeMedia) discarded() []channel.MediaRef { + f.mu.Lock() + defer f.mu.Unlock() + return append([]channel.MediaRef(nil), f.discardedRefs...) +} + +func (f *fakeMedia) ResolveMedia(ctx context.Context, _ ResolvedInstallation, _ ResolvedIdentity, _ pgtype.UUID, msg channel.InboundMessage) channel.InboundMessage { + f.mu.Lock() + f.count++ + waitForCancel := f.waitForCancel + started := f.started + release := f.release + resolve := f.resolve + f.mu.Unlock() + if resolve != nil { + return resolve(ctx, msg) + } + if started != nil { + close(started) + } + if release != nil { + select { + case <-release: + case <-ctx.Done(): + return msg + } + } + if waitForCancel { + <-ctx.Done() + return msg + } + msg.MediaRefs = append(msg.MediaRefs, channel.MediaRef{ + Type: channel.MsgTypeImage, + StorageKey: "workspaces/ws/lark/image", + StorageURL: "https://cdn.example.test/image", + Filename: "image.png", + MimeType: "image/png", + SizeBytes: 3, + }) + return msg +} + +func (f *fakeMedia) calls() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.count +} + type fakeIssues struct { called bool params service.IssueCreateParams @@ -156,21 +259,42 @@ func (f *fakeIssues) Create(_ context.Context, p service.IssueCreateParams, _ se type fakeTasks struct { mu sync.Mutex called bool + callCount int + promotions int forceFresh bool initiator pgtype.UUID err error } +func (f *fakeTasks) PromoteChannelChatTasksIfMediaReady(_ context.Context, _ pgtype.UUID) error { + f.mu.Lock() + defer f.mu.Unlock() + f.promotions++ + return nil +} + func (f *fakeTasks) EnqueueChatTask(_ context.Context, _ db.ChatSession, initiator pgtype.UUID, forceFresh bool) (db.AgentTaskQueue, error) { f.mu.Lock() defer f.mu.Unlock() f.called = true + f.callCount++ f.forceFresh = forceFresh f.initiator = initiator return db.AgentTaskQueue{}, f.err } func (f *fakeTasks) wasCalled() bool { f.mu.Lock(); defer f.mu.Unlock(); return f.called } func (f *fakeTasks) freshArg() bool { f.mu.Lock(); defer f.mu.Unlock(); return f.forceFresh } +func (f *fakeTasks) calls() int { f.mu.Lock(); defer f.mu.Unlock(); return f.callCount } +func (f *fakeTasks) promotionCalls() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.promotions +} +func (f *fakeTasks) initiatorArg() pgtype.UUID { + f.mu.Lock() + defer f.mu.Unlock() + return f.initiator +} type fakeReader struct { session db.ChatSession @@ -221,6 +345,7 @@ type harness struct { audit *fakeAuditor replier *fakeReplier typing *fakeTyping + media *fakeMedia issues *fakeIssues tasks *fakeTasks reader *fakeReader @@ -229,13 +354,20 @@ type harness struct { func newHarness(t *testing.T) *harness { t.Helper() h := &harness{ - inst: &fakeInstaller{inst: activeResolved(t)}, - ident: &fakeIdentity{id: ResolvedIdentity{UserID: uuidFromString(t, "44444444-4444-4444-4444-444444444444")}}, - dedup: &fakeDedup{token: uuidFromString(t, "55555555-5555-5555-5555-555555555555")}, - binder: &fakeBinder{ensureID: uuidFromString(t, "66666666-6666-6666-6666-666666666666"), appendResult: AppendResult{DedupMarked: true}}, + inst: &fakeInstaller{inst: activeResolved(t)}, + ident: &fakeIdentity{id: ResolvedIdentity{UserID: uuidFromString(t, "44444444-4444-4444-4444-444444444444")}}, + dedup: &fakeDedup{token: uuidFromString(t, "55555555-5555-5555-5555-555555555555")}, + binder: &fakeBinder{ + ensureID: uuidFromString(t, "66666666-6666-6666-6666-666666666666"), + appendResult: AppendResult{ + MessageID: uuidFromString(t, "99999999-9999-4999-8999-999999999999"), + DedupMarked: true, + }, + }, audit: &fakeAuditor{}, replier: &fakeReplier{}, typing: &fakeTyping{}, + media: &fakeMedia{}, issues: &fakeIssues{}, tasks: &fakeTasks{}, reader: &fakeReader{ws: db.Workspace{IssuePrefix: "MUL"}}, @@ -249,6 +381,7 @@ func newHarness(t *testing.T) *harness { Audit: h.audit, Replier: h.replier, Typing: h.typing, + Media: h.media, OriginType: "lark_chat", }) return h @@ -286,6 +419,9 @@ func TestRouter_RevokedInstallation_Drops(t *testing.T) { if r, _ := h.audit.last(); r != DropReasonRevokedInstallation { t.Fatalf("expected revoked_installation, got %q", r) } + if h.media.calls() != 0 { + t.Fatal("revoked installation must not resolve media") + } } func TestRouter_Duplicate_Drops(t *testing.T) { @@ -297,6 +433,9 @@ func TestRouter_Duplicate_Drops(t *testing.T) { if r, _ := h.audit.last(); r != DropReasonDuplicate { t.Fatalf("expected duplicate, got %q", r) } + if h.media.calls() != 0 { + t.Fatal("duplicate message must not resolve media") + } } func TestRouter_GroupNotAddressed_Drops(t *testing.T) { @@ -313,6 +452,9 @@ func TestRouter_GroupNotAddressed_Drops(t *testing.T) { if h.dedup.marks() != 1 { t.Fatalf("group-filter drop must finalize Mark (1), got %d", h.dedup.marks()) } + if h.media.calls() != 0 { + t.Fatal("unaddressed group message must not resolve media") + } } func TestRouter_UnboundSender_NeedsBinding(t *testing.T) { @@ -327,6 +469,9 @@ func TestRouter_UnboundSender_NeedsBinding(t *testing.T) { if h.dedup.marks() != 1 { t.Fatalf("unbound drop must finalize Mark, got %d", h.dedup.marks()) } + if h.media.calls() != 0 { + t.Fatal("unbound sender must not resolve media") + } if !waitFor(time.Second, func() bool { for _, r := range h.replier.calls() { if r.Outcome == OutcomeNeedsBinding && r.Sender == "ou_user_a" { @@ -348,6 +493,9 @@ func TestRouter_NonMember_Drops(t *testing.T) { if r, _ := h.audit.last(); r != DropReasonNonWorkspaceMember { t.Fatalf("expected non_workspace_member, got %q", r) } + if h.media.calls() != 0 { + t.Fatal("non-member sender must not resolve media") + } } func TestRouter_EnsureSessionError_Releases(t *testing.T) { @@ -362,6 +510,25 @@ func TestRouter_EnsureSessionError_Releases(t *testing.T) { } } +func TestRouter_AppendErrorReleasesClaimAndAllowsMediaRetry(t *testing.T) { + h := newHarness(t) + h.binder.appendErr = errors.New("db down") + if err := h.router.Handle(context.Background(), p2pMessage(t)); err == nil { + t.Fatal("append error must surface to the caller") + } + if h.dedup.releases() != 1 || h.media.calls() != 0 { + t.Fatalf("failed attempt: releases=%d media_calls=%d, want release=1 media_calls=0", h.dedup.releases(), h.media.calls()) + } + + h.binder.appendErr = nil + if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { + t.Fatalf("retry: %v", err) + } + if !waitFor(time.Second, func() bool { return h.media.calls() == 1 }) { + t.Fatalf("retry media calls = %d, want 1 total", h.media.calls()) + } +} + func TestRouter_Ingested_InTxMark_FinalizeNone(t *testing.T) { h := newHarness(t) h.reader.session = db.ChatSession{} @@ -375,12 +542,240 @@ func TestRouter_Ingested_InTxMark_FinalizeNone(t *testing.T) { if h.dedup.releases() != 0 { t.Fatalf("a durable ingest must not Release, got %d", h.dedup.releases()) } - if !h.tasks.wasCalled() { + if !waitFor(time.Second, h.tasks.wasCalled) { t.Fatalf("ingest must trigger a chat run (inline, no batcher)") } if !waitFor(time.Second, func() bool { return h.typing.calls() == 1 }) { t.Fatalf("ingest must show the typing indicator") } + if h.media.calls() != 1 { + t.Fatalf("ingested message resolved media %d times, want 1", h.media.calls()) + } + if refs := h.binder.boundMedia().MediaRefs; len(refs) != 1 { + t.Fatalf("resolved media not bound after append: %+v", refs) + } +} + +func TestRouter_NoMediaMessageSkipsMediaPipeline(t *testing.T) { + h := newHarness(t) + h.media.noMedia = true + + if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { + t.Fatalf("Handle: %v", err) + } + if !waitFor(time.Second, h.tasks.wasCalled) { + t.Fatal("no-media message must still trigger a chat run") + } + if got := h.binder.appendedParams().MediaPendingUntil; got.Valid { + t.Fatalf("no-media message persisted a media deadline: %v", got.Time) + } + if h.media.calls() != 0 { + t.Fatalf("no-media message ran ResolveMedia %d times, want 0", h.media.calls()) + } + if h.tasks.promotionCalls() != 0 { + t.Fatalf("no-media message triggered %d promotions, want 0", h.tasks.promotionCalls()) + } +} + +func TestRouter_MediaResolverTimeoutAppendsOriginalMessage(t *testing.T) { + h := newHarness(t) + h.router = NewRouter(h.issues, h.tasks, h.reader, RouterConfig{MediaTimeout: 10 * time.Millisecond, Logger: discardLogger()}) + h.media.waitForCancel = true + h.router.Register(channel.TypeFeishu, ResolverSet{ + Installation: h.inst, + Identity: h.ident, + Dedup: h.dedup, + Session: h.binder, + Audit: h.audit, + Replier: h.replier, + Typing: h.typing, + Media: h.media, + OriginType: "lark_chat", + }) + + if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !waitFor(time.Second, func() bool { return h.media.calls() == 1 }) { + t.Fatalf("media resolver calls = %d, want 1", h.media.calls()) + } + if refs := h.binder.boundMedia().MediaRefs; len(refs) != 0 { + t.Fatalf("timed-out media refs must not attach: %+v", refs) + } + if !waitFor(time.Second, h.tasks.wasCalled) { + t.Fatalf("message should still be ingested and trigger a chat run") + } + if h.dedup.releases() != 0 { + t.Fatalf("media timeout must not release a durably appended message, got %d", h.dedup.releases()) + } +} + +func TestRouter_MediaBindFailureStillChecksPlaceholderPromotion(t *testing.T) { + h := newHarness(t) + h.binder.bindErr = errors.New("attachment write failed") + + if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { + t.Fatalf("Handle: %v", err) + } + if !waitFor(time.Second, func() bool { return h.tasks.promotionCalls() == 1 }) { + t.Fatal("binding failure did not check whether the cleared placeholder task could be promoted") + } + // Bind failure means the uploaded objects have no attachment row and no + // other reclaim path — they must be discarded. + if !waitFor(time.Second, func() bool { return len(h.media.discarded()) == 1 }) { + t.Fatalf("bind failure discarded refs = %+v, want the one uploaded ref", h.media.discarded()) + } + if refs := h.media.discarded(); refs[0].StorageKey != "workspaces/ws/lark/image" { + t.Fatalf("bind failure discarded refs = %+v, want the one uploaded ref", refs) + } + // A bind that failed because the finalize budget expired must not hand + // the storage deletes that same dead context. + if err := h.media.lastDiscardCtxErr(); err != nil { + t.Fatalf("discard ran on a dead context: %v", err) + } +} + +func TestRouter_MediaBindUnknownOutcomeKeepsUploads(t *testing.T) { + h := newHarness(t) + h.binder.bindErr = fmt.Errorf("commit media: %w", ErrMediaBindResultUnknown) + + if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { + t.Fatalf("Handle: %v", err) + } + if !waitFor(time.Second, func() bool { return h.tasks.promotionCalls() == 1 }) { + t.Fatal("unknown bind outcome must still check placeholder promotion") + } + // The commit may have durably landed despite the error report; deleting + // could destroy objects that persisted attachment rows reference. + if refs := h.media.discarded(); len(refs) != 0 { + t.Fatalf("unknown bind outcome must keep the uploads, discarded %+v", refs) + } +} + +func TestRouter_MediaDeadlineDiscardsUploadedRefs(t *testing.T) { + h := newHarness(t) + h.router = NewRouter(h.issues, h.tasks, h.reader, RouterConfig{MediaTimeout: 10 * time.Millisecond, Logger: discardLogger()}) + // A rich post where an early upload succeeded before the deadline killed + // the rest of the resolution: the resolver returns the partial refs. + h.media.resolve = func(ctx context.Context, msg channel.InboundMessage) channel.InboundMessage { + msg.MediaRefs = append(msg.MediaRefs, channel.MediaRef{ + Type: channel.MsgTypeImage, + StorageKey: "workspaces/ws/lark/uploaded-before-deadline", + }) + <-ctx.Done() + return msg + } + h.router.Register(channel.TypeFeishu, ResolverSet{ + Installation: h.inst, + Identity: h.ident, + Dedup: h.dedup, + Session: h.binder, + Audit: h.audit, + Replier: h.replier, + Typing: h.typing, + Media: h.media, + OriginType: "lark_chat", + }) + + if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { + t.Fatalf("Handle: %v", err) + } + if !waitFor(time.Second, func() bool { return len(h.media.discarded()) == 1 }) { + t.Fatalf("deadline expiry did not discard the uploaded ref: %+v", h.media.discarded()) + } + if got := h.media.discarded()[0].StorageKey; got != "workspaces/ws/lark/uploaded-before-deadline" { + t.Fatalf("discarded key = %q", got) + } + if refs := h.binder.boundMedia().MediaRefs; len(refs) != 0 { + t.Fatalf("timed-out refs must not bind: %+v", refs) + } +} + +func TestRouter_MediaResolutionDoesNotBlockInboundHandle(t *testing.T) { + h := newHarness(t) + h.reader.session = db.ChatSession{} + started := make(chan struct{}) + release := make(chan struct{}) + h.media.started = started + h.media.release = release + msg := p2pMessage(t) + + handled := make(chan error, 1) + go func() { + handled <- h.router.Handle(context.Background(), msg) + }() + + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("media resolver did not start") + } + select { + case err := <-handled: + if err != nil { + t.Fatalf("Handle: %v", err) + } + case <-time.After(100 * time.Millisecond): + t.Fatal("Handle waited for media resolution on the connector ACK path") + } + if !h.tasks.wasCalled() { + t.Fatal("durable run trigger was not scheduled while media resolution was pending") + } + + close(release) + if !waitFor(time.Second, func() bool { return h.tasks.promotionCalls() == 1 }) { + t.Fatal("media completion did not promote the durable deferred run") + } +} + +func TestRouter_MediaQueuePreservesSessionOrderWithoutCancellingRunBoundary(t *testing.T) { + h := newHarness(t) + timers := &fakeTimerFactory{} + h.router.batcher = newTestBatcher(timers) + secondStarted := make(chan struct{}) + releaseSecond := make(chan struct{}) + h.media.resolve = func(ctx context.Context, msg channel.InboundMessage) channel.InboundMessage { + if msg.MessageID == "m2" { + close(secondStarted) + select { + case <-releaseSecond: + case <-ctx.Done(): + } + } + return msg + } + + first := p2pMessage(t) + first.MessageID = "m1" + if err := h.router.Handle(context.Background(), first); err != nil { + t.Fatalf("first Handle: %v", err) + } + if got := h.router.batcher.pendingCount(); got != 1 { + t.Fatalf("first message did not arm a run, pending=%d", got) + } + + second := p2pMessage(t) + second.MessageID = "m2" + if err := h.router.Handle(context.Background(), second); err != nil { + t.Fatalf("second Handle: %v", err) + } + select { + case <-secondStarted: + case <-time.After(time.Second): + t.Fatal("second media job did not start") + } + if got := h.router.batcher.pendingCount(); got != 1 { + t.Fatalf("new media must keep one fenced run boundary, pending=%d", got) + } + timers.fireArmed() + if !waitFor(time.Second, func() bool { return h.tasks.calls() == 1 }) { + t.Fatal("durable run trigger did not flush while media was pending") + } + + close(releaseSecond) + if !waitFor(time.Second, func() bool { return h.tasks.promotionCalls() == 2 }) { + t.Fatalf("media completion promotions = %d, want 2", h.tasks.promotionCalls()) + } } func TestRouter_ClaimLost_Drops(t *testing.T) { @@ -434,7 +829,7 @@ func TestRouter_GroupSessionCreatorIsInstaller(t *testing.T) { t.Fatalf("group session creator must be the installer") } // And the run initiator is the sender, not the installer. - if h.tasks.initiator != h.ident.id.UserID { + if !waitFor(time.Second, h.tasks.wasCalled) || h.tasks.initiatorArg() != h.ident.id.UserID { t.Fatalf("run initiator must be the message sender") } } @@ -455,19 +850,19 @@ func TestRouter_FlushOffline_RepliesAgentOffline(t *testing.T) { if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { t.Fatalf("unexpected error: %v", err) } - // Inline flush (no batcher) emits the offline notice synchronously via replier. - found := false - for _, r := range h.replier.calls() { - if r.Outcome == OutcomeAgentOffline { - found = true + if !waitFor(time.Second, func() bool { + for _, r := range h.replier.calls() { + if r.Outcome == OutcomeAgentOffline { + return true + } } - } - if !found { + return false + }) { t.Fatalf("agent-no-runtime must emit an AgentOffline reply") } // The reaction was added on ingest but no task will run, so the bus-driven // clear never fires — the flush must clear the typing indicator itself. - if h.typing.settledCalls() != 1 { + if !waitFor(time.Second, func() bool { return h.typing.settledCalls() == 1 }) { t.Fatalf("offline flush must clear the typing indicator, got %d OnSettled calls", h.typing.settledCalls()) } } @@ -478,7 +873,7 @@ func TestRouter_FlushArchived_ClearsTyping(t *testing.T) { if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { t.Fatalf("unexpected error: %v", err) } - if h.typing.settledCalls() != 1 { + if !waitFor(time.Second, func() bool { return h.typing.settledCalls() == 1 }) { t.Fatalf("archived flush must clear the typing indicator, got %d OnSettled calls", h.typing.settledCalls()) } } @@ -488,7 +883,7 @@ func TestRouter_FlushSuccess_DoesNotClearTyping(t *testing.T) { if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { t.Fatalf("unexpected error: %v", err) } - if !h.tasks.wasCalled() { + if !waitFor(time.Second, h.tasks.wasCalled) { t.Fatalf("a healthy session must enqueue a task") } // A successfully enqueued task is cleared by the platform's bus-driven @@ -505,7 +900,7 @@ func TestRouter_ForceFresh_Propagates(t *testing.T) { if err := h.router.Handle(context.Background(), msg); err != nil { t.Fatalf("unexpected error: %v", err) } - if !h.tasks.freshArg() { + if !waitFor(time.Second, h.tasks.wasCalled) || !h.tasks.freshArg() { t.Fatalf("ForceFresh must propagate to EnqueueChatTask") } } @@ -517,7 +912,7 @@ func TestRouter_DrainJoinsReplies(t *testing.T) { t.Fatalf("unexpected error: %v", err) } done := make(chan struct{}) - go func() { h.router.Drain(); close(done) }() + go func() { h.router.Drain(context.Background()); close(done) }() select { case <-done: case <-time.After(2 * time.Second): @@ -528,6 +923,83 @@ func TestRouter_DrainJoinsReplies(t *testing.T) { } } +func TestRouter_MediaConcurrencyCapAppliesAcrossSessions(t *testing.T) { + h := newHarness(t) + h.router = NewRouter(h.issues, h.tasks, h.reader, RouterConfig{MediaConcurrency: 1, Logger: discardLogger()}) + release := make(chan struct{}) + h.media.resolve = func(ctx context.Context, msg channel.InboundMessage) channel.InboundMessage { + select { + case <-release: + case <-ctx.Done(): + } + return msg + } + h.router.Register(channel.TypeFeishu, ResolverSet{ + Installation: h.inst, + Identity: h.ident, + Dedup: h.dedup, + Session: h.binder, + Audit: h.audit, + Replier: h.replier, + Typing: h.typing, + Media: h.media, + OriginType: "lark_chat", + }) + + first := p2pMessage(t) + first.MessageID = "m1" + if err := h.router.Handle(context.Background(), first); err != nil { + t.Fatalf("first Handle: %v", err) + } + if !waitFor(time.Second, func() bool { return h.media.calls() == 1 }) { + t.Fatalf("first media job did not start, calls=%d", h.media.calls()) + } + + // A different session gets its own queue; only the global cap gates it. + h.binder.ensureID = uuidFromString(t, "77777777-7777-4777-8777-777777777777") + second := p2pMessage(t) + second.MessageID = "m2" + second.Source.ChatID = "oc_chat_b" + if err := h.router.Handle(context.Background(), second); err != nil { + t.Fatalf("second Handle: %v", err) + } + if waitFor(150*time.Millisecond, func() bool { return h.media.calls() == 2 }) { + t.Fatal("second media job ran while the only concurrency slot was held") + } + + close(release) + if !waitFor(time.Second, func() bool { return h.media.calls() == 2 }) { + t.Fatalf("second media job never ran after the slot freed, calls=%d", h.media.calls()) + } +} + +func TestRouter_DrainHonorsDeadlineWhenMediaResolverIgnoresCancellation(t *testing.T) { + h := newHarness(t) + release := make(chan struct{}) + started := make(chan struct{}) + h.media.resolve = func(_ context.Context, msg channel.InboundMessage) channel.InboundMessage { + close(started) + <-release + return msg + } + + if err := h.router.Handle(context.Background(), p2pMessage(t)); err != nil { + t.Fatalf("Handle: %v", err) + } + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("media resolver did not start") + } + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + if h.router.Drain(ctx) { + t.Fatal("Drain reported completion for a wedged media resolver") + } + close(release) +} + func TestRouter_EmptyMessageID_SkipsDedup(t *testing.T) { h := newHarness(t) msg := p2pMessage(t) @@ -538,7 +1010,7 @@ func TestRouter_EmptyMessageID_SkipsDedup(t *testing.T) { if h.dedup.claimCalls != 0 { t.Fatalf("empty message id must skip the dedup claim, got %d", h.dedup.claimCalls) } - if !h.tasks.wasCalled() { + if !waitFor(time.Second, h.tasks.wasCalled) { t.Fatalf("message must still ingest without a dedup key") } } diff --git a/server/internal/integrations/channel/engine/session.go b/server/internal/integrations/channel/engine/session.go index cba3e0a835b..668b92fe4c7 100644 --- a/server/internal/integrations/channel/engine/session.go +++ b/server/internal/integrations/channel/engine/session.go @@ -4,7 +4,9 @@ import ( "context" "errors" "fmt" + "time" + "github.com/google/uuid" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgtype" @@ -42,6 +44,10 @@ type SessionQueries interface { CreateChatSession(ctx context.Context, arg db.CreateChatSessionParams) (db.ChatSession, error) CreateChannelChatSessionBinding(ctx context.Context, arg db.CreateChannelChatSessionBindingParams) (db.ChannelChatSessionBinding, error) CreateChatMessage(ctx context.Context, arg db.CreateChatMessageParams) (db.ChatMessage, error) + ClearChatMessageChannelMediaPending(ctx context.Context, arg db.ClearChatMessageChannelMediaPendingParams) error + CreateAttachment(ctx context.Context, arg db.CreateAttachmentParams) (db.Attachment, error) + LinkAttachmentsToChatMessage(ctx context.Context, arg db.LinkAttachmentsToChatMessageParams) ([]pgtype.UUID, error) + ListAttachmentsByChatMessage(ctx context.Context, arg db.ListAttachmentsByChatMessageParams) ([]db.Attachment, error) TouchChatSession(ctx context.Context, id pgtype.UUID) error GetMostRecentUserChatMessage(ctx context.Context, chatSessionID pgtype.UUID) (db.ChatMessage, error) UpdateChannelChatSessionBindingReplyTarget(ctx context.Context, arg db.UpdateChannelChatSessionBindingReplyTargetParams) error @@ -71,6 +77,18 @@ func (a dbSessionQueries) CreateChannelChatSessionBinding(ctx context.Context, a func (a dbSessionQueries) CreateChatMessage(ctx context.Context, arg db.CreateChatMessageParams) (db.ChatMessage, error) { return a.q.CreateChatMessage(ctx, arg) } +func (a dbSessionQueries) ClearChatMessageChannelMediaPending(ctx context.Context, arg db.ClearChatMessageChannelMediaPendingParams) error { + return a.q.ClearChatMessageChannelMediaPending(ctx, arg) +} +func (a dbSessionQueries) CreateAttachment(ctx context.Context, arg db.CreateAttachmentParams) (db.Attachment, error) { + return a.q.CreateAttachment(ctx, arg) +} +func (a dbSessionQueries) LinkAttachmentsToChatMessage(ctx context.Context, arg db.LinkAttachmentsToChatMessageParams) ([]pgtype.UUID, error) { + return a.q.LinkAttachmentsToChatMessage(ctx, arg) +} +func (a dbSessionQueries) ListAttachmentsByChatMessage(ctx context.Context, arg db.ListAttachmentsByChatMessageParams) ([]db.Attachment, error) { + return a.q.ListAttachmentsByChatMessage(ctx, arg) +} func (a dbSessionQueries) TouchChatSession(ctx context.Context, id pgtype.UUID) error { return a.q.TouchChatSession(ctx, id) } @@ -241,21 +259,34 @@ func (s *ChatSession) createSessionAndBinding(ctx context.Context, in EnsureSess // its own binding row, recording the real thread here per session does not clash // across sibling threads. type AppendInput struct { - SessionID pgtype.UUID - Sender pgtype.UUID - InstallationID pgtype.UUID - Body string - CommandText string - MessageID string - ThreadID string - ClaimToken pgtype.UUID + SessionID pgtype.UUID + Sender pgtype.UUID + InstallationID pgtype.UUID + Body string + CommandText string + MessageID string + ThreadID string + ClaimToken pgtype.UUID + MediaPendingUntil pgtype.Timestamptz +} + +// BindMediaInput links already-uploaded media to a durable chat message in a +// short database-only transaction. Remote downloads/uploads happen before +// this call and outside the connector ACK path. +type BindMediaInput struct { + MessageID pgtype.UUID + SessionID pgtype.UUID + WorkspaceID pgtype.UUID + Sender pgtype.UUID + MediaRefs []channel.MediaRef } // AppendUserMessage writes the user message into the chat_session (touching it // and recording the reply target), runs the in-tx dedup Mark when a claim token -// is supplied, and returns the parsed `/issue` command when present. Returns -// ErrClaimLost when a concurrent reclaim rotated the dedup token mid-flight, in -// which case the whole transaction rolls back (no chat_message lands). +// is supplied, and returns the durable message id plus the parsed `/issue` +// command when present. Returns ErrClaimLost when a concurrent reclaim rotated +// the dedup token mid-flight, in which case the whole transaction rolls back +// (no chat_message lands). func (s *ChatSession) AppendUserMessage(ctx context.Context, in AppendInput) (AppendResult, error) { tx, err := s.tx.Begin(ctx) if err != nil { @@ -280,11 +311,17 @@ func (s *ChatSession) AppendUserMessage(ctx context.Context, in AppendInput) (Ap } } - if _, err := qtx.CreateChatMessage(ctx, db.CreateChatMessageParams{ - ChatSessionID: in.SessionID, - Role: "user", - Content: in.Body, - }); err != nil { + // channel_ingested is the immutable provenance the cancel path gates on: + // it must be stamped in the same transaction as the message so no later + // binding deletion (archive, installation rebind) can strip it. + msg, err := qtx.CreateChatMessage(ctx, db.CreateChatMessageParams{ + ChatSessionID: in.SessionID, + Role: "user", + Content: in.Body, + ChannelMediaPendingUntil: in.MediaPendingUntil, + ChannelIngested: pgtype.Bool{Bool: true, Valid: true}, + }) + if err != nil { return AppendResult{}, fmt.Errorf("create chat message: %w", err) } if err := qtx.TouchChatSession(ctx, in.SessionID); err != nil { @@ -324,7 +361,175 @@ func (s *ChatSession) AppendUserMessage(ctx context.Context, in AppendInput) (Ap if err := tx.Commit(ctx); err != nil { return AppendResult{}, fmt.Errorf("commit: %w", err) } - return AppendResult{IssueCommand: cmd, DedupMarked: markedInTx}, nil + return AppendResult{MessageID: msg.ID, IssueCommand: cmd, DedupMarked: markedInTx}, nil +} + +// BindMediaRefs creates attachment rows, links them to an existing durable chat +// message, and clears its media-pending marker. A link failure rolls back the +// attachment rows, then clears the marker separately so the placeholder can be +// promoted immediately for graceful degradation. +func (s *ChatSession) BindMediaRefs(ctx context.Context, in BindMediaInput) error { + tx, err := s.tx.Begin(ctx) + if err != nil { + return fmt.Errorf("begin media tx: %w", err) + } + defer tx.Rollback(ctx) + qtx := s.q.WithTx(tx) + if len(in.MediaRefs) > 0 { + if err := s.bindMediaRefs(ctx, qtx, in); err != nil { + _ = tx.Rollback(ctx) + if clearErr := s.clearMediaPending(ctx, s.q, in); clearErr != nil { + return errors.Join(err, clearErr) + } + return err + } + } + if err := s.clearMediaPending(ctx, qtx, in); err != nil { + return err + } + if err := tx.Commit(ctx); err != nil { + return s.resolveMediaCommitOutcome(in, err) + } + return nil +} + +// ErrMediaBindResultUnknown marks a media bind whose durable outcome could not +// be established: the commit reported an error AND the follow-up verification +// failed. The router must NOT discard the uploaded objects on this error — a +// lost commit ack may have durably persisted attachment rows that reference +// them, and a rare orphan beats a broken attachment. +var ErrMediaBindResultUnknown = errors.New("media bind result unknown") + +const mediaBindVerifyTimeout = 5 * time.Second + +// resolveMediaCommitOutcome converges an ambiguous media-bind commit into a +// definite verdict. A Commit error is not a rollback guarantee — a lost ack or +// a context expiring mid-commit can report failure after Postgres durably +// committed the attachment + link rows, and discarding the objects on that +// report would break the persisted attachments. The transaction is atomic, so +// any of this batch's URLs being present proves the whole bind (marker clear +// included) landed; none present proves the rollback. The check runs on a +// fresh budget because the ambiguous case IS the caller's context expiring. +func (s *ChatSession) resolveMediaCommitOutcome(in BindMediaInput, commitErr error) error { + if len(in.MediaRefs) == 0 { + // Only the marker clear was in flight; there is nothing a discard + // could delete, so the plain error is safe in both outcomes. + return fmt.Errorf("commit media: %w", commitErr) + } + verifyCtx, cancel := context.WithTimeout(context.Background(), mediaBindVerifyTimeout) + defer cancel() + atts, err := s.q.ListAttachmentsByChatMessage(verifyCtx, db.ListAttachmentsByChatMessageParams{ + ChatMessageID: in.MessageID, + WorkspaceID: in.WorkspaceID, + }) + if err != nil { + return fmt.Errorf("commit media: %w: %w", ErrMediaBindResultUnknown, errors.Join(commitErr, err)) + } + bound := make(map[string]bool, len(atts)) + for _, a := range atts { + bound[a.Url] = true + } + for _, ref := range in.MediaRefs { + if bound[ref.StorageURL] { + // The commit landed despite the error report. + return nil + } + } + return fmt.Errorf("commit media: %w", commitErr) +} + +func (s *ChatSession) clearMediaPending(ctx context.Context, q SessionQueries, in BindMediaInput) error { + if err := q.ClearChatMessageChannelMediaPending(ctx, db.ClearChatMessageChannelMediaPendingParams{ + ID: in.MessageID, + ChatSessionID: in.SessionID, + }); err != nil { + return fmt.Errorf("clear chat message media pending: %w", err) + } + return nil +} + +func (s *ChatSession) bindMediaRefs(ctx context.Context, qtx SessionQueries, in BindMediaInput) error { + if !in.WorkspaceID.Valid { + return errors.New("bind media refs: workspace_id is required") + } + if !in.MessageID.Valid { + return errors.New("bind media refs: message_id is required") + } + ids := make([]pgtype.UUID, 0, len(in.MediaRefs)) + for _, ref := range in.MediaRefs { + if ref.StorageURL == "" { + return errors.New("bind media refs: storage_url is required") + } + id, err := uuid.NewV7() + if err != nil { + return fmt.Errorf("create attachment id: %w", err) + } + contentType := ref.MimeType + if contentType == "" { + contentType = "application/octet-stream" + } + filename := ref.Filename + if filename == "" { + filename = defaultMediaFilename(ref.Type, id.String(), contentType) + } + att, err := qtx.CreateAttachment(ctx, db.CreateAttachmentParams{ + ID: pgtype.UUID{Bytes: id, Valid: true}, + WorkspaceID: in.WorkspaceID, + ChatSessionID: in.SessionID, + UploaderType: "member", + UploaderID: in.Sender, + Filename: filename, + Url: ref.StorageURL, + ContentType: contentType, + SizeBytes: ref.SizeBytes, + }) + if err != nil { + return fmt.Errorf("create chat attachment: %w", err) + } + ids = append(ids, att.ID) + } + if len(ids) == 0 { + return nil + } + if _, err := qtx.LinkAttachmentsToChatMessage(ctx, db.LinkAttachmentsToChatMessageParams{ + ChatMessageID: in.MessageID, + ChatSessionID: in.SessionID, + WorkspaceID: in.WorkspaceID, + UploaderType: "member", + UploaderID: in.Sender, + AttachmentIds: ids, + }); err != nil { + return fmt.Errorf("link chat attachments: %w", err) + } + return nil +} + +func defaultMediaFilename(kind channel.MsgType, id, contentType string) string { + prefix := "attachment" + switch kind { + case channel.MsgTypeImage: + prefix = "image" + case channel.MsgTypeVideo: + prefix = "video" + case channel.MsgTypeAudio: + prefix = "audio" + case channel.MsgTypeFile: + prefix = "file" + } + ext := "" + switch contentType { + case "image/jpeg": + ext = ".jpg" + case "image/png": + ext = ".png" + case "image/gif": + ext = ".gif" + case "image/webp": + ext = ".webp" + case "video/mp4": + ext = ".mp4" + } + return prefix + "-" + id + ext } func isUniqueViolation(err error) bool { diff --git a/server/internal/integrations/channel/engine/session_db_test.go b/server/internal/integrations/channel/engine/session_db_test.go new file mode 100644 index 00000000000..60a50125586 --- /dev/null +++ b/server/internal/integrations/channel/engine/session_db_test.go @@ -0,0 +1,336 @@ +package engine + +import ( + "context" + "errors" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/multica-ai/multica/server/internal/integrations/channel" + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +func sessionPersistenceTestDB(t *testing.T) *pgxpool.Pool { + t.Helper() + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + dsn = "postgres://multica:multica@localhost:5432/multica?sslmode=disable" + } + ctx := context.Background() + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Skipf("no database: %v", err) + } + if err := pool.Ping(ctx); err != nil { + pool.Close() + t.Skipf("database not reachable: %v", err) + } + var migrated bool + if err := pool.QueryRow(ctx, `SELECT to_regclass('public.attachment') IS NOT NULL`).Scan(&migrated); err != nil || !migrated { + pool.Close() + t.Skip("attachment table not present (database not migrated)") + } + t.Cleanup(pool.Close) + return pool +} + +type sessionPersistenceFixture struct { + workspaceID pgtype.UUID + userID pgtype.UUID + sessionID pgtype.UUID +} + +func seedSessionPersistenceFixture(t *testing.T, pool *pgxpool.Pool) sessionPersistenceFixture { + t.Helper() + ctx := context.Background() + suffix := time.Now().UnixNano() + var f sessionPersistenceFixture + var runtimeID, agentID pgtype.UUID + + if err := pool.QueryRow(ctx, `INSERT INTO "user" (name, email) VALUES ($1, $2) RETURNING id`, + "Channel media test", fmt.Sprintf("channel-media-%d@multica.test", suffix)).Scan(&f.userID); err != nil { + t.Fatalf("create user: %v", err) + } + t.Cleanup(func() { + cleanupCtx := context.Background() + if f.workspaceID.Valid { + _, _ = pool.Exec(cleanupCtx, `DELETE FROM workspace WHERE id = $1`, f.workspaceID) + } + if f.userID.Valid { + _, _ = pool.Exec(cleanupCtx, `DELETE FROM "user" WHERE id = $1`, f.userID) + } + }) + if err := pool.QueryRow(ctx, `INSERT INTO workspace (name, slug, description) VALUES ($1, $2, '') RETURNING id`, + "Channel media test", fmt.Sprintf("channel-media-%d", suffix)).Scan(&f.workspaceID); err != nil { + t.Fatalf("create workspace: %v", err) + } + if _, err := pool.Exec(ctx, `INSERT INTO member (workspace_id, user_id, role) VALUES ($1, $2, 'owner')`, f.workspaceID, f.userID); err != nil { + t.Fatalf("create member: %v", err) + } + if err := pool.QueryRow(ctx, ` + INSERT INTO agent_runtime (workspace_id, name, runtime_mode, provider, owner_id) + VALUES ($1, $2, 'local', 'multica_daemon', $3) + RETURNING id`, f.workspaceID, fmt.Sprintf("channel-media-runtime-%d", suffix), f.userID).Scan(&runtimeID); err != nil { + t.Fatalf("create runtime: %v", err) + } + if err := pool.QueryRow(ctx, ` + INSERT INTO agent (workspace_id, name, runtime_mode, runtime_id, owner_id) + VALUES ($1, $2, 'local', $3, $4) + RETURNING id`, f.workspaceID, fmt.Sprintf("channel-media-agent-%d", suffix), runtimeID, f.userID).Scan(&agentID); err != nil { + t.Fatalf("create agent: %v", err) + } + if err := pool.QueryRow(ctx, ` + INSERT INTO chat_session (workspace_id, agent_id, creator_id, title) + VALUES ($1, $2, $3, 'Channel media test') + RETURNING id`, f.workspaceID, agentID, f.userID).Scan(&f.sessionID); err != nil { + t.Fatalf("create chat session: %v", err) + } + + return f +} + +func TestBindMediaRefs_PersistsAndLinksAttachmentToDurableMessage(t *testing.T) { + pool := sessionPersistenceTestDB(t) + fixture := seedSessionPersistenceFixture(t, pool) + session := NewChatSession(db.New(pool), pool, channel.TypeFeishu, SessionTitles{}) + + appendRes, err := session.AppendUserMessage(context.Background(), AppendInput{ + SessionID: fixture.sessionID, + Sender: fixture.userID, + Body: "[Image]", + MediaPendingUntil: pgtype.Timestamptz{Time: time.Now().Add(time.Minute), Valid: true}, + }) + if err != nil { + t.Fatalf("AppendUserMessage: %v", err) + } + err = session.BindMediaRefs(context.Background(), BindMediaInput{ + MessageID: appendRes.MessageID, + SessionID: fixture.sessionID, + WorkspaceID: fixture.workspaceID, + Sender: fixture.userID, + MediaRefs: []channel.MediaRef{{ + Type: channel.MsgTypeImage, + StorageKey: "workspaces/ws/lark/image", + StorageURL: "https://cdn.example.test/image", + Filename: "image.png", + MimeType: "image/png", + SizeBytes: 3, + }}, + }) + if err != nil { + t.Fatalf("BindMediaRefs: %v", err) + } + + var content, filename, url, contentType string + var mediaPendingUntil pgtype.Timestamptz + var sizeBytes int64 + var channelIngested bool + if err := pool.QueryRow(context.Background(), ` + SELECT m.content, a.filename, a.url, a.content_type, a.size_bytes, m.channel_media_pending_until, m.channel_ingested + FROM chat_message m + JOIN attachment a ON a.chat_message_id = m.id + WHERE m.chat_session_id = $1 AND a.chat_session_id = $1`, fixture.sessionID). + Scan(&content, &filename, &url, &contentType, &sizeBytes, &mediaPendingUntil, &channelIngested); err != nil { + t.Fatalf("load linked attachment: %v", err) + } + if content != "[Image]" || filename != "image.png" || url != "https://cdn.example.test/image" || contentType != "image/png" || sizeBytes != 3 { + t.Fatalf("persisted media mismatch: content=%q filename=%q url=%q content_type=%q size=%d", content, filename, url, contentType, sizeBytes) + } + if mediaPendingUntil.Valid { + t.Fatalf("media pending deadline was not cleared: %v", mediaPendingUntil.Time) + } + if !channelIngested { + t.Fatal("channel append must stamp channel_ingested for the cancel-path provenance gate") + } +} + +type failingLinkSessionQueries struct { + SessionQueries +} + +func (q failingLinkSessionQueries) WithTx(tx pgx.Tx) SessionQueries { + return failingLinkSessionQueries{SessionQueries: q.SessionQueries.WithTx(tx)} +} + +func (failingLinkSessionQueries) LinkAttachmentsToChatMessage(context.Context, db.LinkAttachmentsToChatMessageParams) ([]pgtype.UUID, error) { + return nil, errors.New("injected attachment link failure") +} + +func TestBindMediaRefs_LinkFailureKeepsMessageAndRollsBackAttachment(t *testing.T) { + pool := sessionPersistenceTestDB(t) + fixture := seedSessionPersistenceFixture(t, pool) + queries := failingLinkSessionQueries{SessionQueries: dbSessionQueries{q: db.New(pool)}} + session := newChatSessionWith(queries, pool, channel.TypeFeishu, SessionTitles{}) + + appendRes, err := session.AppendUserMessage(context.Background(), AppendInput{ + SessionID: fixture.sessionID, + Sender: fixture.userID, + Body: "rollback-media", + MediaPendingUntil: pgtype.Timestamptz{Time: time.Now().Add(time.Minute), Valid: true}, + }) + if err != nil { + t.Fatalf("AppendUserMessage: %v", err) + } + err = session.BindMediaRefs(context.Background(), BindMediaInput{ + MessageID: appendRes.MessageID, + SessionID: fixture.sessionID, + WorkspaceID: fixture.workspaceID, + Sender: fixture.userID, + MediaRefs: []channel.MediaRef{{ + Type: channel.MsgTypeImage, + StorageURL: "https://cdn.example.test/rollback.png", + Filename: "rollback.png", + MimeType: "image/png", + SizeBytes: 4, + }}, + }) + if err == nil || !strings.Contains(err.Error(), "injected attachment link failure") { + t.Fatalf("BindMediaRefs error = %v", err) + } + + var messageCount, attachmentCount int + var mediaPendingUntil pgtype.Timestamptz + ctx := context.Background() + if err := pool.QueryRow(ctx, `SELECT count(*) FROM chat_message WHERE chat_session_id = $1 AND content = 'rollback-media'`, fixture.sessionID).Scan(&messageCount); err != nil { + t.Fatalf("count messages: %v", err) + } + if err := pool.QueryRow(ctx, `SELECT count(*) FROM attachment WHERE chat_session_id = $1 AND filename = 'rollback.png'`, fixture.sessionID).Scan(&attachmentCount); err != nil { + t.Fatalf("count attachments: %v", err) + } + if err := pool.QueryRow(ctx, `SELECT channel_media_pending_until FROM chat_message WHERE id = $1`, appendRes.MessageID).Scan(&mediaPendingUntil); err != nil { + t.Fatalf("load fallback marker: %v", err) + } + if messageCount != 1 || attachmentCount != 0 { + t.Fatalf("fallback persistence mismatch: messages=%d attachments=%d", messageCount, attachmentCount) + } + if mediaPendingUntil.Valid { + t.Fatalf("failed attachment kept media pending until %v", mediaPendingUntil.Time) + } +} + +// lostAckTxStarter simulates a lost commit ack: the transaction durably +// commits, but the client is handed an error — the result-uncertain window +// the compensation protocol must not treat as "nothing landed". +type lostAckTxStarter struct{ pool *pgxpool.Pool } + +func (s *lostAckTxStarter) Begin(ctx context.Context) (pgx.Tx, error) { + tx, err := s.pool.Begin(ctx) + if err != nil { + return nil, err + } + return &lostAckTx{Tx: tx}, nil +} + +type lostAckTx struct{ pgx.Tx } + +func (t *lostAckTx) Commit(ctx context.Context) error { + if err := t.Tx.Commit(ctx); err != nil { + return err + } + return errors.New("injected lost commit ack") +} + +// rolledBackCommitTxStarter simulates a commit failure whose rollback is +// definite: nothing landed, so the caller may safely reclaim the uploads. +type rolledBackCommitTxStarter struct{ pool *pgxpool.Pool } + +func (s *rolledBackCommitTxStarter) Begin(ctx context.Context) (pgx.Tx, error) { + tx, err := s.pool.Begin(ctx) + if err != nil { + return nil, err + } + return &rolledBackCommitTx{Tx: tx}, nil +} + +type rolledBackCommitTx struct{ pgx.Tx } + +func (t *rolledBackCommitTx) Commit(ctx context.Context) error { + _ = t.Tx.Rollback(ctx) + return errors.New("injected commit failure") +} + +// bindOneMediaRef appends the user message through a healthy session, then +// binds one ref through bindSession — the seam tests inject commit faults into. +func bindOneMediaRef(t *testing.T, pool *pgxpool.Pool, bindSession *ChatSession, fixture sessionPersistenceFixture, url string) error { + t.Helper() + appendSession := NewChatSession(db.New(pool), pool, channel.TypeFeishu, SessionTitles{}) + appendRes, err := appendSession.AppendUserMessage(context.Background(), AppendInput{ + SessionID: fixture.sessionID, + Sender: fixture.userID, + Body: "[Image]", + MediaPendingUntil: pgtype.Timestamptz{Time: time.Now().Add(time.Minute), Valid: true}, + }) + if err != nil { + t.Fatalf("AppendUserMessage: %v", err) + } + return bindSession.BindMediaRefs(context.Background(), BindMediaInput{ + MessageID: appendRes.MessageID, + SessionID: fixture.sessionID, + WorkspaceID: fixture.workspaceID, + Sender: fixture.userID, + MediaRefs: []channel.MediaRef{{ + Type: channel.MsgTypeImage, + StorageKey: "workspaces/ws/lark/ack", + StorageURL: url, + Filename: "ack.png", + MimeType: "image/png", + SizeBytes: 1, + }}, + }) +} + +// A Commit error is not a rollback guarantee. When the rows durably landed +// despite the error report, BindMediaRefs must verify and report SUCCESS — +// returning an error would make the router delete objects that persisted +// attachment rows reference. +func TestBindMediaRefs_LostCommitAckVerifiesAndKeepsBoundAttachment(t *testing.T) { + pool := sessionPersistenceTestDB(t) + fixture := seedSessionPersistenceFixture(t, pool) + session := newChatSessionWith(dbSessionQueries{q: db.New(pool)}, &lostAckTxStarter{pool: pool}, channel.TypeFeishu, SessionTitles{}) + + if err := bindOneMediaRef(t, pool, session, fixture, "https://cdn.example.test/lost-ack.png"); err != nil { + t.Fatalf("lost-ack bind must verify the durable commit and succeed, got %v", err) + } + var attachments int + if err := pool.QueryRow(context.Background(), ` + SELECT count(*) FROM attachment WHERE chat_session_id = $1 AND url = 'https://cdn.example.test/lost-ack.png' + `, fixture.sessionID).Scan(&attachments); err != nil { + t.Fatalf("count attachments: %v", err) + } + if attachments != 1 { + t.Fatalf("attachments = %d, want the durably committed row", attachments) + } +} + +// The mirror case: a commit failure whose transaction definitely rolled back +// must stay an error — and NOT the result-unknown sentinel — so the router +// reclaims the uploads. +func TestBindMediaRefs_RolledBackCommitStaysDiscardableError(t *testing.T) { + pool := sessionPersistenceTestDB(t) + fixture := seedSessionPersistenceFixture(t, pool) + session := newChatSessionWith(dbSessionQueries{q: db.New(pool)}, &rolledBackCommitTxStarter{pool: pool}, channel.TypeFeishu, SessionTitles{}) + + err := bindOneMediaRef(t, pool, session, fixture, "https://cdn.example.test/rolled-back.png") + if err == nil || !strings.Contains(err.Error(), "injected commit failure") { + t.Fatalf("rolled-back commit error = %v", err) + } + if errors.Is(err, ErrMediaBindResultUnknown) { + t.Fatalf("verified rollback must not be reported as result-unknown: %v", err) + } + var attachments int + if err := pool.QueryRow(context.Background(), ` + SELECT count(*) FROM attachment WHERE chat_session_id = $1 AND url = 'https://cdn.example.test/rolled-back.png' + `, fixture.sessionID).Scan(&attachments); err != nil { + t.Fatalf("count attachments: %v", err) + } + if attachments != 0 { + t.Fatalf("attachments = %d, want none after rollback", attachments) + } +} diff --git a/server/internal/integrations/channel/engine/session_test.go b/server/internal/integrations/channel/engine/session_test.go index 3b9dd8e5658..b7c74d1ea15 100644 --- a/server/internal/integrations/channel/engine/session_test.go +++ b/server/internal/integrations/channel/engine/session_test.go @@ -35,14 +35,20 @@ func (fakeTxStarter) Begin(context.Context) (pgx.Tx, error) { return fakeTx{}, n // fakeSessionQueries is an in-memory SessionQueries for unit tests. type fakeSessionQueries struct { - bindings map[string]pgtype.UUID - nextSession byte - createdSessions int - messages []string - touched int - replyTargets int - lockedWorkspace int // count of LockWorkspaceForChatSessionCreate calls - lastConfig []byte // config of the most recent CreateChannelChatSessionBinding + bindings map[string]pgtype.UUID + nextSession byte + createdSessions int + messages []string + messageID pgtype.UUID + lastCreate db.CreateChatMessageParams + touched int + replyTargets int + lockedWorkspace int // count of LockWorkspaceForChatSessionCreate calls + lastConfig []byte // config of the most recent CreateChannelChatSessionBinding + attachments []db.CreateAttachmentParams + linked db.LinkAttachmentsToChatMessageParams + mediaCleared int + boundAttachments []db.Attachment prevMessage *string // GetMostRecentUserChatMessage result; nil → ErrNoRows markRows int64 // MarkChannelInboundDedupProcessed result @@ -51,7 +57,7 @@ type fakeSessionQueries struct { } func newFake() *fakeSessionQueries { - return &fakeSessionQueries{bindings: map[string]pgtype.UUID{}, markRows: 1} + return &fakeSessionQueries{bindings: map[string]pgtype.UUID{}, markRows: 1, messageID: uid(42)} } func bindKey(inst pgtype.UUID, chat string) string { return fmt.Sprintf("%x|%s", inst.Bytes, chat) } @@ -89,7 +95,27 @@ func (f *fakeSessionQueries) CreateChannelChatSessionBinding(_ context.Context, func (f *fakeSessionQueries) CreateChatMessage(_ context.Context, arg db.CreateChatMessageParams) (db.ChatMessage, error) { f.messages = append(f.messages, arg.Content) - return db.ChatMessage{}, nil + f.lastCreate = arg + return db.ChatMessage{ID: f.messageID}, nil +} + +func (f *fakeSessionQueries) ClearChatMessageChannelMediaPending(context.Context, db.ClearChatMessageChannelMediaPendingParams) error { + f.mediaCleared++ + return nil +} + +func (f *fakeSessionQueries) CreateAttachment(_ context.Context, arg db.CreateAttachmentParams) (db.Attachment, error) { + f.attachments = append(f.attachments, arg) + return db.Attachment{ID: arg.ID}, nil +} + +func (f *fakeSessionQueries) LinkAttachmentsToChatMessage(_ context.Context, arg db.LinkAttachmentsToChatMessageParams) ([]pgtype.UUID, error) { + f.linked = arg + return append([]pgtype.UUID(nil), arg.AttachmentIds...), nil +} + +func (f *fakeSessionQueries) ListAttachmentsByChatMessage(context.Context, db.ListAttachmentsByChatMessageParams) ([]db.Attachment, error) { + return f.boundAttachments, nil } func (f *fakeSessionQueries) TouchChatSession(context.Context, pgtype.UUID) error { @@ -286,6 +312,65 @@ func TestAppendUserMessage_CommandTextOverridesEnrichedBody(t *testing.T) { } } +func TestBindMediaRefs_CreatesAndLinksChatAttachments(t *testing.T) { + f := newFake() + s := newTestSession(f) + res, err := s.AppendUserMessage(context.Background(), AppendInput{ + SessionID: uid(1), + Sender: uid(7), + Body: "[Image]", + MessageID: "om_image", + }) + if err != nil { + t.Fatalf("AppendUserMessage: %v", err) + } + if res.IssueCommand != nil { + t.Fatalf("media placeholder must not parse as /issue: %+v", res.IssueCommand) + } + if res.MessageID != f.messageID { + t.Fatalf("message id = %v, want %v", res.MessageID, f.messageID) + } + if !f.lastCreate.ChannelIngested.Valid || !f.lastCreate.ChannelIngested.Bool { + t.Fatalf("channel append must stamp channel_ingested, got %+v", f.lastCreate.ChannelIngested) + } + err = s.BindMediaRefs(context.Background(), BindMediaInput{ + MessageID: res.MessageID, + SessionID: uid(1), + WorkspaceID: uid(9), + Sender: uid(7), + MediaRefs: []channel.MediaRef{ + { + Type: channel.MsgTypeImage, + StorageKey: "lark/cli/img.png", + StorageURL: "https://cdn.example.test/lark/cli/img.png", + Filename: "screenshot.png", + MimeType: "image/png", + SizeBytes: 3, + }, + }, + }) + if err != nil { + t.Fatalf("BindMediaRefs: %v", err) + } + if len(f.attachments) != 1 { + t.Fatalf("attachments created = %d, want 1", len(f.attachments)) + } + att := f.attachments[0] + if att.WorkspaceID != uid(9) || att.ChatSessionID != uid(1) || att.UploaderType != "member" || att.UploaderID != uid(7) { + t.Fatalf("attachment ownership/session wrong: %+v", att) + } + if att.Filename != "screenshot.png" || att.Url != "https://cdn.example.test/lark/cli/img.png" || + att.ContentType != "image/png" || att.SizeBytes != 3 { + t.Fatalf("attachment metadata wrong: %+v", att) + } + if f.linked.ChatMessageID != res.MessageID || f.linked.ChatSessionID != uid(1) || f.linked.WorkspaceID != uid(9) { + t.Fatalf("link params wrong: %+v", f.linked) + } + if len(f.linked.AttachmentIds) != 1 || f.linked.AttachmentIds[0] != att.ID { + t.Fatalf("linked ids = %+v, want attachment id %v", f.linked.AttachmentIds, att.ID) + } +} + func TestAppendUserMessage_BareIssueUsesPreviousMessage(t *testing.T) { f := newFake() prev := "Make the export button work" diff --git a/server/internal/integrations/channel/message.go b/server/internal/integrations/channel/message.go index 6b688a67872..5c87626695f 100644 --- a/server/internal/integrations/channel/message.go +++ b/server/internal/integrations/channel/message.go @@ -86,6 +86,10 @@ type MediaRef struct { Type MsgType // StorageKey locates the persisted object in Multica object storage. StorageKey string + // StorageURL is the object URL returned by the storage backend and + // persisted on the attachment row so the existing attachment download + // endpoints can re-open it later. + StorageURL string // Filename is the original display name, when the platform supplies // one. Filename string diff --git a/server/internal/integrations/lark/client.go b/server/internal/integrations/lark/client.go index a51ca6c75f7..2a9b8b58fd6 100644 --- a/server/internal/integrations/lark/client.go +++ b/server/internal/integrations/lark/client.go @@ -3,6 +3,7 @@ package lark import ( "context" "errors" + "io" "log/slog" ) @@ -96,6 +97,12 @@ type APIClient interface { // adapter: flattening and block assembly are the enricher's job. ListChatMessages(ctx context.Context, creds InstallationCredentials, p ListMessagesParams) ([]LarkMessage, error) + // DownloadMessageResource downloads one binary resource attached to a + // message via GET /open-apis/im/v1/messages/{message_id}/resources/{file_key}. + // Type is the Open Platform resource class ("image" for image_key, + // "file" for file_key-backed video/file/audio). + DownloadMessageResource(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResource, error) + // BatchGetUsers resolves a set of user open_ids to their display names // via GET /open-apis/contact/v3/users/batch. The enricher uses it to // label recent-context / quoted / forwarded speakers (and the sender @@ -136,6 +143,26 @@ type ListMessagesParams struct { EndTime int64 } +type DownloadResourceParams struct { + MessageID string + FileKey string + Type string +} + +type DownloadedResource struct { + Data []byte + ContentType string + Filename string + SizeBytes int64 +} + +type DownloadedResourceStream struct { + Body io.ReadCloser + ContentType string + Filename string + SizeBytes int64 +} + // LarkMessage is the normalized slice of an IM v1 message item the // enricher needs. Body.content is passed through raw (still the // JSON-encoded, msg_type-specific string Lark double-encodes) so the @@ -383,6 +410,11 @@ func (s *stubAPIClient) ListChatMessages(ctx context.Context, creds Installation return nil, ErrAPIClientNotConfigured } +func (s *stubAPIClient) DownloadMessageResource(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResource, error) { + s.log.Warn("lark stub client: DownloadMessageResource called", "message_id", p.MessageID) + return DownloadedResource{}, ErrAPIClientNotConfigured +} + func (s *stubAPIClient) BatchGetUsers(ctx context.Context, creds InstallationCredentials, openIDs []string) (map[string]string, error) { s.log.Warn("lark stub client: BatchGetUsers called", "count", len(openIDs)) return nil, ErrAPIClientNotConfigured diff --git a/server/internal/integrations/lark/content_flatten.go b/server/internal/integrations/lark/content_flatten.go index 85e1c6dab4d..30ceb14ac01 100644 --- a/server/internal/integrations/lark/content_flatten.go +++ b/server/internal/integrations/lark/content_flatten.go @@ -21,12 +21,13 @@ import ( // knows which one applies — so flattening stays mention-agnostic. // // Non-text media types render as a stable bracketed placeholder so the -// agent sees that *something* was attached without us downloading the -// binary. Attachment ingestion is explicitly out of scope (tracked as a -// separate attachment-pipeline issue), and merge_forward is intercepted -// by the enricher before it reaches here (expanding it needs an HTTP -// round-trip); the inline placeholder is only a fallback for a forward -// nested inside another forward. +// agent sees that *something* was attached without this fast path +// downloading the binary; the detached media resolver separately fetches +// the resource and binds it as a chat attachment, with the placeholder +// as the durable fallback. merge_forward is intercepted by the enricher +// before it reaches here (expanding it needs an HTTP round-trip); the +// inline placeholder is only a fallback for a forward nested inside +// another forward. func flattenContent(msgType, rawContent string) string { switch msgType { case "text": @@ -39,7 +40,7 @@ func flattenContent(msgType, rawContent string) string { return "[File]" case "audio": return "[Audio]" - case "media": + case "media", "video": return "[Video]" case "sticker": return "[Sticker]" @@ -80,6 +81,11 @@ type larkPostSpan struct { Href string `json:"href"` UserID string `json:"user_id"` UserName string `json:"user_name"` + ImageKey string `json:"image_key"` + FileKey string `json:"file_key"` + FileName string `json:"file_name"` + Name string `json:"name"` + MimeType string `json:"mime_type"` } // flattenPostContent flattens a received `post` body.content into plain diff --git a/server/internal/integrations/lark/content_flatten_test.go b/server/internal/integrations/lark/content_flatten_test.go index f20775fdab0..0d37d91dc39 100644 --- a/server/internal/integrations/lark/content_flatten_test.go +++ b/server/internal/integrations/lark/content_flatten_test.go @@ -83,6 +83,7 @@ func TestFlattenContent_DispatchByType(t *testing.T) { {"file", "file", `{"file_key":"f"}`, "[File]"}, {"audio", "audio", `{"file_key":"f"}`, "[Audio]"}, {"media", "media", `{"file_key":"f"}`, "[Video]"}, + {"video", "video", `{"file_key":"f"}`, "[Video]"}, {"sticker", "sticker", `{"file_key":"f"}`, "[Sticker]"}, {"interactive", "interactive", `{"title":"t"}`, "[interactive card]"}, {"share_chat", "share_chat", `{"chat_id":"oc"}`, "[Shared Chat]"}, diff --git a/server/internal/integrations/lark/feishu_channel.go b/server/internal/integrations/lark/feishu_channel.go index abfb781bede..de778bb2037 100644 --- a/server/internal/integrations/lark/feishu_channel.go +++ b/server/internal/integrations/lark/feishu_channel.go @@ -98,20 +98,24 @@ func (c *feishuChannel) Capabilities() channel.Capability { } func (c *feishuChannel) installationCredentials() (InstallationCredentials, error) { - if c.creds == nil { + return installationCredentialsFor(c.inst, c.creds) +} + +func installationCredentialsFor(inst Installation, resolver CredentialsResolver) (InstallationCredentials, error) { + if resolver == nil { return InstallationCredentials{}, errors.New("lark: credentials resolver missing") } - secret, err := c.creds.DecryptAppSecret(c.inst) + secret, err := resolver.DecryptAppSecret(inst) if err != nil { return InstallationCredentials{}, fmt.Errorf("decrypt app_secret: %w", err) } creds := InstallationCredentials{ - AppID: c.inst.AppID, + AppID: inst.AppID, AppSecret: secret, - Region: RegionOrDefault(c.inst.Region), + Region: RegionOrDefault(inst.Region), } - if c.inst.TenantKey.Valid { - creds.TenantKey = c.inst.TenantKey.String + if inst.TenantKey.Valid { + creds.TenantKey = inst.TenantKey.String } return creds, nil } @@ -131,6 +135,7 @@ func channelMessageFromLark(lm InboundMessage) channel.InboundMessage { MessageID: lm.MessageID, Type: channelMsgType(lm.MessageType), Text: lm.Body, + MediaRefs: lm.MediaRefs, ReplyTo: reply, AddressedToBot: lm.AddressedToBot, ForceFresh: lm.ForceFreshSession, diff --git a/server/internal/integrations/lark/feishu_channel_test.go b/server/internal/integrations/lark/feishu_channel_test.go index 6bb1317ad56..72500f01193 100644 --- a/server/internal/integrations/lark/feishu_channel_test.go +++ b/server/internal/integrations/lark/feishu_channel_test.go @@ -1,9 +1,13 @@ package lark import ( + "bytes" "context" "encoding/base64" "encoding/json" + "errors" + "io" + "strings" "testing" "github.com/multica-ai/multica/server/internal/integrations/channel" @@ -14,8 +18,13 @@ import ( // SendTextMessage — the single method feishuChannel.Send calls. type fakeSender struct { APIClient - last SendTextParams - msgID string + last SendTextParams + msgID string + downloadCalls []DownloadResourceParams + downloaded DownloadedResource + downloadErr error + downloadedByKey map[string]DownloadedResource + downloadErrByKey map[string]error } func (f *fakeSender) SendTextMessage(_ context.Context, p SendTextParams) (string, error) { @@ -23,10 +32,86 @@ func (f *fakeSender) SendTextMessage(_ context.Context, p SendTextParams) (strin return f.msgID, nil } +func (f *fakeSender) DownloadMessageResource(_ context.Context, _ InstallationCredentials, p DownloadResourceParams) (DownloadedResource, error) { + return f.download(p) +} + +func (f *fakeSender) DownloadMessageResourceStream(_ context.Context, _ InstallationCredentials, p DownloadResourceParams) (DownloadedResourceStream, error) { + got, err := f.download(p) + if err != nil { + return DownloadedResourceStream{}, err + } + return DownloadedResourceStream{ + Body: io.NopCloser(bytes.NewReader(got.Data)), + ContentType: got.ContentType, + Filename: got.Filename, + SizeBytes: got.SizeBytes, + }, nil +} + +func (f *fakeSender) download(p DownloadResourceParams) (DownloadedResource, error) { + f.downloadCalls = append(f.downloadCalls, p) + if err := f.downloadErrByKey[p.FileKey]; err != nil { + return DownloadedResource{}, err + } + if got, ok := f.downloadedByKey[p.FileKey]; ok { + return got, nil + } + return f.downloaded, f.downloadErr +} + +type fakeMediaStorage struct { + uploads []fakeMediaUpload + deleted []string + err error +} + +func (s *fakeMediaStorage) Delete(_ context.Context, key string) { + s.deleted = append(s.deleted, key) +} + +type fakeMediaUpload struct { + key string + data []byte + sizeBytes int64 + streamed bool + contentType string + filename string +} + +func (s *fakeMediaStorage) Upload(_ context.Context, key string, data []byte, contentType string, filename string) (string, error) { + if s.err != nil { + return "", s.err + } + s.uploads = append(s.uploads, fakeMediaUpload{key: key, data: append([]byte(nil), data...), contentType: contentType, filename: filename}) + return "https://cdn.example.test/" + key, nil +} + +func (s *fakeMediaStorage) UploadStream(_ context.Context, key string, data io.Reader, sizeBytes int64, contentType string, filename string) (string, error) { + if s.err != nil { + return "", s.err + } + body, err := io.ReadAll(data) + if err != nil { + return "", err + } + s.uploads = append(s.uploads, fakeMediaUpload{key: key, data: body, sizeBytes: sizeBytes, streamed: true, contentType: contentType, filename: filename}) + return "https://cdn.example.test/" + key, nil +} + type fakeCreds struct{ secret string } func (f fakeCreds) DecryptAppSecret(_ Installation) (string, error) { return f.secret, nil } +func testMediaInstallation(t *testing.T) engine.ResolvedInstallation { + t.Helper() + return engine.ResolvedInstallation{ + ID: uuidFromString(t, "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"), + WorkspaceID: uuidFromString(t, "11111111-1111-1111-1111-111111111111"), + Platform: Installation{AppID: "cli_app", Region: "feishu"}, + } +} + // feishuConfigJSON builds a channel_installation.config blob like migration 124 // backfills — the shape the Feishu factory decodes. func feishuConfigJSON(t *testing.T, appID, region string) []byte { @@ -133,6 +218,346 @@ func TestFeishuChannel_SendMapsTextAndReplyTarget(t *testing.T) { } } +func TestFeishuMediaResolver_HasMedia(t *testing.T) { + resolver := NewFeishuMediaResolver(&fakeSender{}, fakeCreds{secret: "plain"}, &fakeMediaStorage{}, newDiscardLogger()) + cases := []struct { + name string + lm InboundMessage + want bool + }{ + {"text", InboundMessage{MessageID: "om_t", MessageType: "text", Body: "hello", Content: `{"text":"hello"}`}, false}, + {"image", InboundMessage{MessageID: "om_i", MessageType: "image", Body: "[Image]", Content: `{"image_key":"img_k"}`}, true}, + {"video", InboundMessage{MessageID: "om_v", MessageType: "media", Body: "[Video]", Content: `{"file_key":"file_k"}`}, true}, + {"post with image", InboundMessage{MessageID: "om_p", MessageType: "post", + Content: `{"content":[[{"tag":"img","image_key":"img_post"}]]}`}, true}, + {"post text only", InboundMessage{MessageID: "om_pt", MessageType: "post", + Content: `{"content":[[{"tag":"text","text":"plain"}]]}`}, false}, + {"image missing key", InboundMessage{MessageID: "om_bad", MessageType: "image", Content: `{}`}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := resolver.HasMedia(channelMessageFromLark(tc.lm)); got != tc.want { + t.Fatalf("HasMedia = %v, want %v", got, tc.want) + } + }) + } + if !resolver.HasMedia(channel.InboundMessage{MediaRefs: []channel.MediaRef{{Type: channel.MsgTypeImage}}}) { + t.Fatal("pre-resolved MediaRefs must report media") + } +} + +func TestFeishuMediaResolver_DiscardMediaDeletesUploadedObjects(t *testing.T) { + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(&fakeSender{}, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + resolver.DiscardMedia(context.Background(), []channel.MediaRef{ + {Type: channel.MsgTypeImage, StorageKey: "workspaces/ws/lark/inst/aaa"}, + {Type: channel.MsgTypeVideo, StorageKey: ""}, + {Type: channel.MsgTypeVideo, StorageKey: "workspaces/ws/lark/inst/bbb"}, + }) + want := []string{"workspaces/ws/lark/inst/aaa", "workspaces/ws/lark/inst/bbb"} + if len(storage.deleted) != 2 || storage.deleted[0] != want[0] || storage.deleted[1] != want[1] { + t.Fatalf("deleted keys = %v, want %v", storage.deleted, want) + } +} + +func TestFeishuMediaResolver_AttachesImageMediaRef(t *testing.T) { + sender := &fakeSender{downloaded: DownloadedResource{ + Data: []byte{1, 2, 3}, + ContentType: "image/png", + Filename: "from-header.png", + SizeBytes: 3, + }} + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + lm := InboundMessage{ + EventID: "evt-image", + AppID: "cli_app", + ChatID: "oc_dm", + ChatType: ChatTypeP2P, + MessageID: "om_image", + SenderOpenID: "ou_user", + MessageType: "image", + Body: "[Image]", + Content: `{"image_key":"img_v3_key"}`, + } + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), channelMessageFromLark(lm)) + if len(sender.downloadCalls) != 1 { + t.Fatalf("download calls = %d, want 1", len(sender.downloadCalls)) + } + call := sender.downloadCalls[0] + if call.MessageID != "om_image" || call.FileKey != "img_v3_key" || call.Type != "image" { + t.Fatalf("download params wrong: %+v", call) + } + if len(storage.uploads) != 1 { + t.Fatalf("uploads = %d, want 1", len(storage.uploads)) + } + up := storage.uploads[0] + if up.contentType != "image/png" || up.filename != "from-header.png" || string(up.data) != string([]byte{1, 2, 3}) { + t.Fatalf("upload metadata wrong: %+v", up) + } + if !up.streamed || up.sizeBytes != 3 { + t.Fatalf("known-length resource did not stream with its declared size: %+v", up) + } + if !strings.Contains(up.key, "workspaces/11111111-1111-1111-1111-111111111111/lark/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/") { + t.Fatalf("upload key should be workspace-scoped, got %q", up.key) + } + if len(got.MediaRefs) != 1 { + t.Fatalf("media refs = %+v, want 1", got.MediaRefs) + } + ref := got.MediaRefs[0] + if ref.Type != channel.MsgTypeImage || ref.Filename != "from-header.png" || ref.MimeType != "image/png" || + ref.SizeBytes != 3 || ref.StorageURL == "" || ref.StorageKey == "" { + t.Fatalf("media ref wrong: %+v", ref) + } +} + +func TestFeishuMediaResolver_UnknownLengthUsesBufferedUpload(t *testing.T) { + sender := &fakeSender{downloaded: DownloadedResource{ + Data: []byte{1, 2, 3}, + ContentType: "image/png", + SizeBytes: 0, + }} + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + lm := InboundMessage{ + MessageID: "om_unknown_length", + MessageType: "image", + Body: "[Image]", + Content: `{"image_key":"img_unknown_length"}`, + } + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), channelMessageFromLark(lm)) + if len(storage.uploads) != 1 || storage.uploads[0].streamed { + t.Fatalf("unknown-length resource must use buffered Upload: %+v", storage.uploads) + } + if len(got.MediaRefs) != 1 || got.MediaRefs[0].SizeBytes != 3 { + t.Fatalf("buffered upload size not recorded: %+v", got.MediaRefs) + } +} + +func TestFeishuMediaResolver_AttachesPostEmbeddedImageMediaRef(t *testing.T) { + sender := &fakeSender{downloaded: DownloadedResource{ + Data: []byte{4, 5, 6}, + ContentType: "image/png", + Filename: "post-image.png", + SizeBytes: 3, + }} + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + rawPost := `{"content":[[{"tag":"img","image_key":"img_post_key"}],[{"tag":"text","text":"识别一下图片"}]]}` + lm := InboundMessage{ + EventID: "evt-post-image", + AppID: "cli_app", + ChatID: "oc_dm", + ChatType: ChatTypeP2P, + MessageID: "om_post_image", + SenderOpenID: "ou_user", + MessageType: "post", + Body: flattenPostContent(rawPost), + Content: rawPost, + } + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), channelMessageFromLark(lm)) + if got.Text != "[Image]\n识别一下图片" { + t.Fatalf("message text = %q, want post placeholder plus text", got.Text) + } + if len(sender.downloadCalls) != 1 { + t.Fatalf("download calls = %d, want 1", len(sender.downloadCalls)) + } + call := sender.downloadCalls[0] + if call.MessageID != "om_post_image" || call.FileKey != "img_post_key" || call.Type != "image" { + t.Fatalf("download params wrong: %+v", call) + } + if len(storage.uploads) != 1 { + t.Fatalf("uploads = %d, want 1", len(storage.uploads)) + } + if len(got.MediaRefs) != 1 { + t.Fatalf("media refs = %+v, want 1", got.MediaRefs) + } + ref := got.MediaRefs[0] + if ref.Type != channel.MsgTypeImage || ref.Filename != "post-image.png" || ref.MimeType != "image/png" || + ref.SizeBytes != 3 || ref.StorageURL == "" || ref.StorageKey == "" { + t.Fatalf("post image ref wrong: %+v", ref) + } +} + +func TestFeishuMediaResolver_AttachesPostEmbeddedVideoMediaRef(t *testing.T) { + sender := &fakeSender{downloaded: DownloadedResource{ + Data: []byte("mp4"), + ContentType: "video/mp4", + Filename: "demo.mp4", + SizeBytes: 3, + }} + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + rawPost := `{"content":[[{"tag":"text","text":"看一下视频"},{"tag":"media","file_key":"file_post_key","file_name":"demo.mp4"}]]}` + lm := InboundMessage{ + EventID: "evt-post-video", + AppID: "cli_app", + ChatID: "oc_dm", + ChatType: ChatTypeP2P, + MessageID: "om_post_video", + SenderOpenID: "ou_user", + MessageType: "post", + Body: flattenPostContent(rawPost), + Content: rawPost, + } + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), channelMessageFromLark(lm)) + if len(sender.downloadCalls) != 1 { + t.Fatalf("download calls = %d, want 1", len(sender.downloadCalls)) + } + call := sender.downloadCalls[0] + if call.MessageID != "om_post_video" || call.FileKey != "file_post_key" || call.Type != "file" { + t.Fatalf("download params wrong: %+v", call) + } + if len(got.MediaRefs) != 1 || got.MediaRefs[0].Type != channel.MsgTypeVideo || got.MediaRefs[0].Filename != "demo.mp4" { + t.Fatalf("post video ref wrong: %+v", got.MediaRefs) + } +} + +func TestFeishuMediaResolver_AttachesVideoMediaRef(t *testing.T) { + sender := &fakeSender{downloaded: DownloadedResource{ + Data: []byte("mp4"), + ContentType: "video/mp4", + SizeBytes: 3, + }} + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + lm := InboundMessage{ + EventID: "evt-video", + AppID: "cli_app", + ChatID: "oc_dm", + ChatType: ChatTypeP2P, + MessageID: "om_video", + SenderOpenID: "ou_user", + MessageType: "media", + Body: "[Video]", + Content: `{"file_key":"file_v3_key","file_name":"clip.mp4"}`, + } + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), channelMessageFromLark(lm)) + if len(sender.downloadCalls) != 1 { + t.Fatalf("download calls = %d, want 1", len(sender.downloadCalls)) + } + call := sender.downloadCalls[0] + if call.MessageID != "om_video" || call.FileKey != "file_v3_key" || call.Type != "file" { + t.Fatalf("download params wrong: %+v", call) + } + if len(got.MediaRefs) != 1 || got.MediaRefs[0].Type != channel.MsgTypeVideo || got.MediaRefs[0].Filename != "clip.mp4" { + t.Fatalf("video ref wrong: %+v", got.MediaRefs) + } +} + +func TestFeishuMediaResolver_RetryReusesObjectKey(t *testing.T) { + sender := &fakeSender{downloaded: DownloadedResource{ + Data: []byte{1, 2, 3}, + ContentType: "image/png", + Filename: "shot.png", + }} + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + lm := InboundMessage{ + MessageID: "om_retry", + MessageType: "image", + Body: "[Image]", + Content: `{"image_key":"img_retry"}`, + } + + for range 2 { + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), channelMessageFromLark(lm)) + if len(got.MediaRefs) != 1 { + t.Fatalf("media refs = %+v, want 1", got.MediaRefs) + } + } + if len(storage.uploads) != 2 { + t.Fatalf("uploads = %d, want 2 retry attempts", len(storage.uploads)) + } + if storage.uploads[0].key != storage.uploads[1].key { + t.Fatalf("retry object keys differ: %q vs %q", storage.uploads[0].key, storage.uploads[1].key) + } +} + +func TestFeishuMediaResolver_DownloadFailurePreservesMessage(t *testing.T) { + sender := &fakeSender{downloadErr: errors.New("download unavailable")} + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + lm := InboundMessage{ + MessageID: "om_download_failure", + MessageType: "image", + Body: "[Image]", + Content: `{"image_key":"img_failure"}`, + } + before := channelMessageFromLark(lm) + + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), before) + if got.Text != before.Text || len(got.MediaRefs) != 0 { + t.Fatalf("download failure changed message: before=%+v after=%+v", before, got) + } + if len(storage.uploads) != 0 { + t.Fatalf("download failure uploaded %d objects", len(storage.uploads)) + } +} + +func TestFeishuMediaResolver_UploadFailurePreservesMessage(t *testing.T) { + sender := &fakeSender{downloaded: DownloadedResource{Data: []byte{1}, ContentType: "image/png"}} + storage := &fakeMediaStorage{err: errors.New("storage unavailable")} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + lm := InboundMessage{ + MessageID: "om_upload_failure", + MessageType: "image", + Body: "[Image]", + Content: `{"image_key":"img_failure"}`, + } + before := channelMessageFromLark(lm) + + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), before) + if got.Text != before.Text || len(got.MediaRefs) != 0 { + t.Fatalf("upload failure changed message: before=%+v after=%+v", before, got) + } + // A result-uncertain upload (the client saw an error, but the object may + // have landed server-side) must be idempotently deleted right away — the + // key never reaches the router, so nothing else can reclaim it. + if len(storage.deleted) != 1 || !strings.Contains(storage.deleted[0], "workspaces/11111111-1111-1111-1111-111111111111/lark/") { + t.Fatalf("attempted upload key must be deleted, got %v", storage.deleted) + } +} + +func TestFeishuMediaResolver_PostPartialFailureKeepsTextAndSuccessfulMedia(t *testing.T) { + sender := &fakeSender{ + downloadedByKey: map[string]DownloadedResource{ + "img_ok": {Data: []byte{1}, ContentType: "image/png", Filename: "ok.png"}, + }, + downloadErrByKey: map[string]error{"video_failed": errors.New("video unavailable")}, + } + storage := &fakeMediaStorage{} + resolver := NewFeishuMediaResolver(sender, fakeCreds{secret: "plain"}, storage, newDiscardLogger()) + rawPost := `{"content":[[{"tag":"text","text":"inspect"},{"tag":"img","image_key":"img_ok"},{"tag":"media","file_key":"video_failed","file_name":"failed.mp4"}]]}` + lm := InboundMessage{ + MessageID: "om_partial", + MessageType: "post", + Body: flattenPostContent(rawPost), + Content: rawPost, + } + before := channelMessageFromLark(lm) + + got := resolver.ResolveMedia(context.Background(), testMediaInstallation(t), engine.ResolvedIdentity{}, + uuidFromString(t, "22222222-2222-2222-2222-222222222222"), before) + if got.Text != before.Text { + t.Fatalf("partial failure changed text: got %q want %q", got.Text, before.Text) + } + if len(sender.downloadCalls) != 2 || len(storage.uploads) != 1 || len(got.MediaRefs) != 1 { + t.Fatalf("partial failure result: downloads=%d uploads=%d refs=%+v", len(sender.downloadCalls), len(storage.uploads), got.MediaRefs) + } + if got.MediaRefs[0].Type != channel.MsgTypeImage || got.MediaRefs[0].Filename != "ok.png" { + t.Fatalf("successful media ref lost: %+v", got.MediaRefs[0]) + } +} + func TestOutboundReplyTarget(t *testing.T) { if got := outboundReplyTarget(channel.OutboundMessage{}); got.IsSet() { t.Fatalf("no ReplyTo must yield an unset target, got %+v", got) diff --git a/server/internal/integrations/lark/feishu_resolvers.go b/server/internal/integrations/lark/feishu_resolvers.go index 297fe26425c..69133905f1a 100644 --- a/server/internal/integrations/lark/feishu_resolvers.go +++ b/server/internal/integrations/lark/feishu_resolvers.go @@ -44,7 +44,7 @@ func larkMsgFromRaw(msg channel.InboundMessage) (InboundMessage, error) { // shared session service, audit logger, and (optional) outbound replier + // typing indicator. Feishu is just another consumer of the channel-agnostic // engine.ChatSession — there is no Feishu-specific session implementation. -func NewFeishuResolverSet(store *ChannelStore, session *engine.ChatSession, audit AuditLogger, replier OutcomeReplier, typing *TypingIndicatorManager) engine.ResolverSet { +func NewFeishuResolverSet(store *ChannelStore, session *engine.ChatSession, audit AuditLogger, replier OutcomeReplier, typing *TypingIndicatorManager, media engine.MediaResolver) engine.ResolverSet { set := engine.ResolverSet{ Installation: &feishuInstallationResolver{store: store}, Identity: &feishuIdentityResolver{store: store}, @@ -59,6 +59,9 @@ func NewFeishuResolverSet(store *ChannelStore, session *engine.ChatSession, audi if typing != nil { set.Typing = &feishuTypingNotifier{mgr: typing} } + if media != nil { + set.Media = media + } return set } @@ -157,6 +160,7 @@ func (r *feishuDeduper) Release(ctx context.Context, installationID pgtype.UUID, type chatSession interface { EnsureSession(ctx context.Context, in engine.EnsureSessionInput) (pgtype.UUID, error) AppendUserMessage(ctx context.Context, in engine.AppendInput) (engine.AppendResult, error) + BindMediaRefs(ctx context.Context, in engine.BindMediaInput) error } type feishuSessionBinder struct{ session chatSession } @@ -207,14 +211,25 @@ func (r *feishuSessionBinder) AppendMessage(ctx context.Context, p engine.Append return engine.AppendResult{}, err } return r.session.AppendUserMessage(ctx, engine.AppendInput{ - SessionID: p.SessionID, - Sender: p.Sender, - InstallationID: p.InstallationID, - Body: p.Message.Text, - CommandText: lm.CommandBody, - MessageID: p.Message.MessageID, - ThreadID: p.Message.Source.ThreadID, - ClaimToken: p.ClaimToken, + SessionID: p.SessionID, + Sender: p.Sender, + InstallationID: p.InstallationID, + Body: p.Message.Text, + CommandText: lm.CommandBody, + MessageID: p.Message.MessageID, + ThreadID: p.Message.Source.ThreadID, + ClaimToken: p.ClaimToken, + MediaPendingUntil: p.MediaPendingUntil, + }) +} + +func (r *feishuSessionBinder) BindMedia(ctx context.Context, p engine.BindMediaParams) error { + return r.session.BindMediaRefs(ctx, engine.BindMediaInput{ + MessageID: p.MessageID, + SessionID: p.SessionID, + WorkspaceID: p.WorkspaceID, + Sender: p.Sender, + MediaRefs: p.MediaRefs, }) } diff --git a/server/internal/integrations/lark/feishu_resolvers_test.go b/server/internal/integrations/lark/feishu_resolvers_test.go index 9b376a9cc0b..26d2be798fb 100644 --- a/server/internal/integrations/lark/feishu_resolvers_test.go +++ b/server/internal/integrations/lark/feishu_resolvers_test.go @@ -23,6 +23,7 @@ func binderUUID(b byte) pgtype.UUID { type fakeChatSession struct { ensureIn engine.EnsureSessionInput appendIn engine.AppendInput + mediaIn engine.BindMediaInput } func (f *fakeChatSession) EnsureSession(_ context.Context, in engine.EnsureSessionInput) (pgtype.UUID, error) { @@ -35,6 +36,11 @@ func (f *fakeChatSession) AppendUserMessage(_ context.Context, in engine.AppendI return engine.AppendResult{}, nil } +func (f *fakeChatSession) BindMediaRefs(_ context.Context, in engine.BindMediaInput) error { + f.mediaIn = in + return nil +} + func TestFeishuSessionBinder_EnsureSessionMapping(t *testing.T) { f := &fakeChatSession{} b := &feishuSessionBinder{session: f} @@ -150,3 +156,22 @@ func TestFeishuSessionBinder_AppendUsesUnenrichedCommandBody(t *testing.T) { t.Errorf("append mapping wrong: %+v", got) } } + +func TestFeishuSessionBinder_BindMediaMapping(t *testing.T) { + f := &fakeChatSession{} + b := &feishuSessionBinder{session: f} + ref := channel.MediaRef{Type: channel.MsgTypeImage, StorageURL: "https://cdn.example.test/image.png"} + if err := b.BindMedia(context.Background(), engine.BindMediaParams{ + MessageID: binderUUID(4), + SessionID: binderUUID(1), + WorkspaceID: binderUUID(2), + Sender: binderUUID(7), + MediaRefs: []channel.MediaRef{ref}, + }); err != nil { + t.Fatalf("BindMedia: %v", err) + } + got := f.mediaIn + if got.MessageID != binderUUID(4) || got.SessionID != binderUUID(1) || got.WorkspaceID != binderUUID(2) || got.Sender != binderUUID(7) || len(got.MediaRefs) != 1 || got.MediaRefs[0] != ref { + t.Fatalf("media mapping wrong: %+v", got) + } +} diff --git a/server/internal/integrations/lark/feishu_types.go b/server/internal/integrations/lark/feishu_types.go index 179733420d2..e96d329db88 100644 --- a/server/internal/integrations/lark/feishu_types.go +++ b/server/internal/integrations/lark/feishu_types.go @@ -1,6 +1,10 @@ package lark -import "github.com/jackc/pgx/v5/pgtype" +import ( + "github.com/jackc/pgx/v5/pgtype" + + "github.com/multica-ai/multica/server/internal/integrations/channel" +) // This file holds the Feishu adapter's native-ish inbound/outbound value // types. The WS connector decodes a raw Lark event into an InboundMessage; @@ -22,6 +26,11 @@ type InboundMessage struct { MessageID string SenderOpenID OpenID Body string + // Content is the raw msg_type-specific JSON string Lark sends in + // event.message.content. Text/post decoding consumes it immediately; media + // ingestion keeps it so the adapter can extract image_key/file_key before + // translating to channel.InboundMessage. + Content string // ForceFreshSession marks this dispatch as a one-off fresh start: the // daemon should skip prior session resume when it claims the resulting // chat task. @@ -54,6 +63,12 @@ type InboundMessage struct { // enricher prepends quoted/forwarded context). `/issue` is parsed from // THIS, not the enriched Body. CommandBody string + + // MediaRefs are downloaded Feishu resources already persisted into Multica + // object storage. The detached media resolver fills these after the user + // message is appended — off the connector ACK path — and the channel + // engine then binds them to that message as chat attachments. + MediaRefs []channel.MediaRef } // Outcome categorizes what the inbound pipeline decided. The OutcomeReplier diff --git a/server/internal/integrations/lark/http_client.go b/server/internal/integrations/lark/http_client.go index fc1fefc7946..0fadb8513b1 100644 --- a/server/internal/integrations/lark/http_client.go +++ b/server/internal/integrations/lark/http_client.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "log/slog" + "mime" "net/http" "net/url" "strconv" @@ -53,6 +54,18 @@ const ( // latency from a self-hosted Multica deployment to feishu.cn. defaultRequestTimeout = 10 * time.Second + // defaultResourceDownloadTimeout is intentionally longer than normal + // OpenAPI calls because message videos are binary transfers, not JSON + // RPCs. Keep it below the inbound dedup stale-claim window (60s), so a + // slow download does not invite a second replica to reclaim the same + // message before this one can append and mark it processed. + defaultResourceDownloadTimeout = 45 * time.Second + + // Feishu caps message resources at 100 MiB. Keep the local transport guard + // aligned with that contract; detached media processing keeps large + // transfers off the connector ACK path. + maxMessageResourceBytes = 100 << 20 + // Lark's "invalid tenant_access_token" / "tenant_access_token // expired" error codes. When we see either, drop the cached token // so the next call refreshes from /tenant_access_token/internal. @@ -80,6 +93,15 @@ type HTTPClientConfig struct { // defaultRequestTimeout. HTTPClient *http.Client + // ResourceHTTPClient is used only for message resource downloads. It + // deliberately does not share HTTPClient's shorter timeout: image/video + // resource transfers are bounded by ResourceDownloadTimeout instead. + ResourceHTTPClient *http.Client + + // ResourceDownloadTimeout caps a single message resource download. Zero + // defaults to defaultResourceDownloadTimeout. + ResourceDownloadTimeout time.Duration + // Now is overridable for deterministic token-expiry tests. Now func() time.Time @@ -99,6 +121,12 @@ func (c HTTPClientConfig) withDefaults() HTTPClientConfig { if c.HTTPClient == nil { c.HTTPClient = &http.Client{Timeout: defaultRequestTimeout} } + if c.ResourceDownloadTimeout == 0 { + c.ResourceDownloadTimeout = defaultResourceDownloadTimeout + } + if c.ResourceHTTPClient == nil { + c.ResourceHTTPClient = &http.Client{Timeout: c.ResourceDownloadTimeout} + } if c.Now == nil { c.Now = time.Now } @@ -654,6 +682,195 @@ func (c *httpAPIClient) ListChatMessages(ctx context.Context, creds Installation return out, nil } +// DownloadMessageResource obtains a binary message resource (image, video, +// file, audio) from Lark/Feishu. Business errors are still represented as +// JSON with a code/msg body on some failures, so JSON-looking responses are +// checked before being treated as resource bytes. +func (c *httpAPIClient) DownloadMessageResource(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResource, error) { + stream, err := c.DownloadMessageResourceStream(ctx, creds, p) + if err != nil { + return DownloadedResource{}, err + } + defer stream.Body.Close() + rawBody, err := io.ReadAll(stream.Body) + if err != nil { + return DownloadedResource{}, fmt.Errorf("lark http client: download resource: read body: %w", err) + } + sizeBytes := stream.SizeBytes + if sizeBytes == 0 { + sizeBytes = int64(len(rawBody)) + } + return DownloadedResource{ + Data: rawBody, + ContentType: stream.ContentType, + Filename: stream.Filename, + SizeBytes: sizeBytes, + }, nil +} + +func (c *httpAPIClient) DownloadMessageResourceStream(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResourceStream, error) { + if p.MessageID == "" { + return DownloadedResourceStream{}, errors.New("lark http client: missing message_id") + } + if p.FileKey == "" { + return DownloadedResourceStream{}, errors.New("lark http client: missing file_key") + } + token, err := c.tenantAccessToken(ctx, creds) + if err != nil { + return DownloadedResourceStream{}, err + } + q := url.Values{} + if p.Type != "" { + q.Set("type", p.Type) + } + path := "/open-apis/im/v1/messages/" + url.PathEscape(p.MessageID) + "/resources/" + url.PathEscape(p.FileKey) + if encoded := q.Encode(); encoded != "" { + path += "?" + encoded + } + + downloadCtx := ctx + var cancel context.CancelFunc + if c.cfg.ResourceDownloadTimeout > 0 { + downloadCtx, cancel = context.WithTimeout(ctx, c.cfg.ResourceDownloadTimeout) + } + req, err := http.NewRequestWithContext(downloadCtx, http.MethodGet, c.resolveBaseURL(creds)+path, nil) + if err != nil { + if cancel != nil { + cancel() + } + return DownloadedResourceStream{}, fmt.Errorf("lark http client: download resource: new request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+token) + resp, err := c.cfg.ResourceHTTPClient.Do(req) + if err != nil { + if cancel != nil { + cancel() + } + return DownloadedResourceStream{}, fmt.Errorf("lark http client: download resource: http do: %w", err) + } + closeWithCancel := func() { + resp.Body.Close() + if cancel != nil { + cancel() + } + } + if resp.ContentLength > maxMessageResourceBytes { + closeWithCancel() + return DownloadedResourceStream{}, fmt.Errorf("lark http client: download resource: resource exceeds %d bytes", maxMessageResourceBytes) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + rawBody, readErr := readMessageResourceErrorBody(resp.Body) + closeWithCancel() + if readErr != nil { + return DownloadedResourceStream{}, readErr + } + return DownloadedResourceStream{}, fmt.Errorf("lark http client: download resource: http %d: %s", resp.StatusCode, truncate(string(rawBody), 512)) + } + contentType := resp.Header.Get("Content-Type") + if strings.Contains(strings.ToLower(contentType), "json") { + rawBody, readErr := readMessageResourceErrorBody(resp.Body) + closeWithCancel() + if readErr != nil { + return DownloadedResourceStream{}, readErr + } + var apiResp struct { + Code int `json:"code"` + Msg string `json:"msg"` + } + if err := json.Unmarshal(rawBody, &apiResp); err == nil && apiResp.Code != 0 { + if isTokenError(apiResp.Code) { + c.invalidateToken(creds.AppID) + } + return DownloadedResourceStream{}, &APIError{Op: "download resource", Code: apiResp.Code, Msg: apiResp.Msg} + } + return DownloadedResourceStream{ + Body: cancelOnClose(io.NopCloser(bytes.NewReader(rawBody)), cancel), + ContentType: contentType, + Filename: filenameFromContentDisposition(resp.Header.Get("Content-Disposition")), + SizeBytes: int64(len(rawBody)), + }, nil + } + if contentType == "" { + contentType = "application/octet-stream" + } + sizeBytes := resp.ContentLength + if sizeBytes < 0 { + sizeBytes = 0 + } + return DownloadedResourceStream{ + Body: cancelOnClose(&maxBytesReadCloser{r: resp.Body, remaining: maxMessageResourceBytes}, cancel), + ContentType: contentType, + Filename: filenameFromContentDisposition(resp.Header.Get("Content-Disposition")), + SizeBytes: sizeBytes, + }, nil +} + +func readMessageResourceErrorBody(body io.Reader) ([]byte, error) { + rawBody, err := io.ReadAll(io.LimitReader(body, maxMessageResourceBytes+1)) + if err != nil { + return nil, fmt.Errorf("lark http client: download resource: read body: %w", err) + } + if len(rawBody) > maxMessageResourceBytes { + return nil, fmt.Errorf("lark http client: download resource: resource exceeds %d bytes", maxMessageResourceBytes) + } + return rawBody, nil +} + +type maxBytesReadCloser struct { + r io.ReadCloser + remaining int64 +} + +func (r *maxBytesReadCloser) Read(p []byte) (int, error) { + if r.remaining > 0 { + if int64(len(p)) > r.remaining { + p = p[:r.remaining] + } + n, err := r.r.Read(p) + r.remaining -= int64(n) + return n, err + } + var one [1]byte + n, err := r.r.Read(one[:]) + if n > 0 { + return 0, fmt.Errorf("lark http client: download resource: resource exceeds %d bytes", maxMessageResourceBytes) + } + return 0, err +} + +func (r *maxBytesReadCloser) Close() error { + return r.r.Close() +} + +type cancelReadCloser struct { + io.ReadCloser + cancel context.CancelFunc +} + +func cancelOnClose(body io.ReadCloser, cancel context.CancelFunc) io.ReadCloser { + if cancel == nil { + return body + } + return &cancelReadCloser{ReadCloser: body, cancel: cancel} +} + +func (r *cancelReadCloser) Close() error { + err := r.ReadCloser.Close() + r.cancel() + return err +} + +func filenameFromContentDisposition(raw string) string { + if raw == "" { + return "" + } + _, params, err := mime.ParseMediaType(raw) + if err != nil { + return "" + } + return params["filename"] +} + // larkBatchGetUsersMaxIDs is Lark's hard cap on user_ids per // contact/v3/users/batch call. We drop the overflow rather than error so // a caller asking for more still gets the first 50 resolved. diff --git a/server/internal/integrations/lark/http_client_test.go b/server/internal/integrations/lark/http_client_test.go index db2fad94f90..109d9838d39 100644 --- a/server/internal/integrations/lark/http_client_test.go +++ b/server/internal/integrations/lark/http_client_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "strconv" "strings" "sync/atomic" "testing" @@ -240,6 +241,201 @@ func TestHTTPClient_IsConfigured(t *testing.T) { } } +func TestHTTPClient_DownloadMessageResource(t *testing.T) { + fake := newLarkFake(t) + fake.stubToken("tok_resource", 7200) + fake.mux.HandleFunc("/open-apis/im/v1/messages/om_1/resources/img_1", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("resource: method = %s, want GET", r.Method) + } + if got := r.URL.Query().Get("type"); got != "image" { + t.Errorf("resource type = %q, want image", got) + } + if got := r.Header.Get("Authorization"); got != "Bearer tok_resource" { + t.Errorf("auth = %q", got) + } + w.Header().Set("Content-Type", "image/png") + w.Header().Set("Content-Disposition", `attachment; filename="shot.png"`) + _, _ = w.Write([]byte{1, 2, 3}) + }) + c := newTestClient(fake, time.Now) + got, err := c.DownloadMessageResource(context.Background(), testCreds(), DownloadResourceParams{ + MessageID: "om_1", + FileKey: "img_1", + Type: "image", + }) + if err != nil { + t.Fatalf("DownloadMessageResource: %v", err) + } + if string(got.Data) != string([]byte{1, 2, 3}) || got.ContentType != "image/png" || + got.Filename != "shot.png" || got.SizeBytes != 3 { + t.Fatalf("downloaded resource wrong: %+v", got) + } +} + +func TestHTTPClient_DownloadMessageResourceUsesResourceTimeout(t *testing.T) { + fake := newLarkFake(t) + fake.stubToken("tok_resource", 7200) + fake.mux.HandleFunc("/open-apis/im/v1/messages/om_video/resources/file_video", func(w http.ResponseWriter, r *http.Request) { + if got := r.URL.Query().Get("type"); got != "file" { + t.Errorf("resource type = %q, want file", got) + } + time.Sleep(80 * time.Millisecond) + w.Header().Set("Content-Type", "video/mp4") + w.Header().Set("Content-Disposition", `attachment; filename="clip.mp4"`) + _, _ = w.Write([]byte("slow-video")) + }) + c := NewHTTPAPIClient(HTTPClientConfig{ + BaseURL: fake.URL(), + HTTPClient: &http.Client{Timeout: 20 * time.Millisecond}, + ResourceDownloadTimeout: 500 * time.Millisecond, + Now: time.Now, + }).(*httpAPIClient) + got, err := c.DownloadMessageResource(context.Background(), testCreds(), DownloadResourceParams{ + MessageID: "om_video", + FileKey: "file_video", + Type: "file", + }) + if err != nil { + t.Fatalf("DownloadMessageResource slow video: %v", err) + } + if string(got.Data) != "slow-video" || got.ContentType != "video/mp4" || got.Filename != "clip.mp4" { + t.Fatalf("downloaded slow video wrong: %+v", got) + } +} + +func TestHTTPClient_DownloadMessageResourceExceedingTimeoutIsCancelled(t *testing.T) { + fake := newLarkFake(t) + fake.stubToken("tok_resource", 7200) + fake.mux.HandleFunc("/open-apis/im/v1/messages/om_timeout/resources/file_timeout", func(w http.ResponseWriter, r *http.Request) { + select { + case <-r.Context().Done(): + return + case <-time.After(500 * time.Millisecond): + _, _ = w.Write([]byte("too late")) + } + }) + c := NewHTTPAPIClient(HTTPClientConfig{ + BaseURL: fake.URL(), + ResourceDownloadTimeout: 20 * time.Millisecond, + Now: time.Now, + }).(*httpAPIClient) + + _, err := c.DownloadMessageResource(context.Background(), testCreds(), DownloadResourceParams{ + MessageID: "om_timeout", + FileKey: "file_timeout", + Type: "file", + }) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("timeout error = %v, want context deadline exceeded", err) + } +} + +func TestHTTPClient_DownloadMessageResourceRejectsDeclaredOversize(t *testing.T) { + fake := newLarkFake(t) + fake.stubToken("tok_resource", 7200) + fake.mux.HandleFunc("/open-apis/im/v1/messages/om_large/resources/file_large", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "video/mp4") + w.Header().Set("Content-Length", strconv.FormatInt(maxMessageResourceBytes+1, 10)) + w.WriteHeader(http.StatusOK) + }) + c := newTestClient(fake, time.Now) + + _, err := c.DownloadMessageResourceStream(context.Background(), testCreds(), DownloadResourceParams{ + MessageID: "om_large", + FileKey: "file_large", + Type: "file", + }) + if err == nil || !strings.Contains(err.Error(), "resource exceeds") { + t.Fatalf("declared oversize error = %v", err) + } +} + +func TestHTTPClient_DownloadMessageResourceAllowsDeclaredFeishuLimit(t *testing.T) { + fake := newLarkFake(t) + fake.stubToken("tok_resource", 7200) + fake.mux.HandleFunc("/open-apis/im/v1/messages/om_limit/resources/file_limit", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "video/mp4") + w.Header().Set("Content-Length", strconv.FormatInt(maxMessageResourceBytes, 10)) + w.WriteHeader(http.StatusOK) + }) + c := newTestClient(fake, time.Now) + + got, err := c.DownloadMessageResourceStream(context.Background(), testCreds(), DownloadResourceParams{ + MessageID: "om_limit", + FileKey: "file_limit", + Type: "file", + }) + if err != nil { + t.Fatalf("declared Feishu-limit resource should be accepted: %v", err) + } + got.Body.Close() + if got.SizeBytes != maxMessageResourceBytes { + t.Fatalf("SizeBytes = %d, want %d", got.SizeBytes, maxMessageResourceBytes) + } +} + +func TestHTTPClient_DownloadMessageResourceAllowsAbovePreviousLocalLimit(t *testing.T) { + const previousLocalLimit = 20 << 20 + fake := newLarkFake(t) + fake.stubToken("tok_resource", 7200) + fake.mux.HandleFunc("/open-apis/im/v1/messages/om_above_previous/resources/file_above_previous", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "video/mp4") + w.Header().Set("Content-Length", strconv.FormatInt(previousLocalLimit+1, 10)) + w.WriteHeader(http.StatusOK) + }) + c := newTestClient(fake, time.Now) + + got, err := c.DownloadMessageResourceStream(context.Background(), testCreds(), DownloadResourceParams{ + MessageID: "om_above_previous", + FileKey: "file_above_previous", + Type: "file", + }) + if err != nil { + t.Fatalf("resource above previous 20MiB local limit should be accepted: %v", err) + } + got.Body.Close() + if got.SizeBytes != previousLocalLimit+1 { + t.Fatalf("SizeBytes = %d, want %d", got.SizeBytes, previousLocalLimit+1) + } +} + +func TestMaxBytesReadCloserEnforcesUnknownLengthBoundary(t *testing.T) { + t.Run("exact limit", func(t *testing.T) { + body := &maxBytesReadCloser{r: io.NopCloser(strings.NewReader("abc")), remaining: 3} + got, err := io.ReadAll(body) + if err != nil || string(got) != "abc" { + t.Fatalf("exact limit: body=%q err=%v", got, err) + } + }) + + t.Run("one byte over", func(t *testing.T) { + body := &maxBytesReadCloser{r: io.NopCloser(strings.NewReader("abcd")), remaining: 3} + got, err := io.ReadAll(body) + if err == nil || !strings.Contains(err.Error(), "resource exceeds") || string(got) != "abc" { + t.Fatalf("overflow: body=%q err=%v", got, err) + } + }) +} + +func TestHTTPClient_DownloadMessageResourceBusinessError(t *testing.T) { + fake := newLarkFake(t) + fake.stubToken("tok_resource", 7200) + fake.mux.HandleFunc("/open-apis/im/v1/messages/om_1/resources/img_1", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + writeJSON(w, map[string]any{"code": 234003, "msg": "File not in msg"}) + }) + c := newTestClient(fake, time.Now) + _, err := c.DownloadMessageResource(context.Background(), testCreds(), DownloadResourceParams{ + MessageID: "om_1", + FileKey: "img_1", + Type: "image", + }) + if err == nil || !strings.Contains(err.Error(), "234003") { + t.Fatalf("expected APIError with code, got %v", err) + } +} + // TestHTTPClient_StubReportsNotConfigured pins that the stub never // claims wired outbound — handlers gate install / management UI on // this signal. @@ -1109,8 +1305,8 @@ func TestHTTPClient_GetBotInfo_HappyPath(t *testing.T) { "code": 0, "msg": "ok", "bot": map[string]any{ - "open_id": "ou_bot_42", - "app_name": "PersonalAgent", + "open_id": "ou_bot_42", + "app_name": "PersonalAgent", "avatar_url": "https://example/avatar.png", }, }) diff --git a/server/internal/integrations/lark/inbound_enricher_test.go b/server/internal/integrations/lark/inbound_enricher_test.go index 6031ba18c78..158b31e4d52 100644 --- a/server/internal/integrations/lark/inbound_enricher_test.go +++ b/server/internal/integrations/lark/inbound_enricher_test.go @@ -80,6 +80,9 @@ func (f *enricherFakeClient) BatchGetUsers(ctx context.Context, creds Installati } return out, nil } +func (f *enricherFakeClient) DownloadMessageResource(context.Context, InstallationCredentials, DownloadResourceParams) (DownloadedResource, error) { + return DownloadedResource{}, nil +} // Unused-by-enricher methods — present only to satisfy APIClient. func (f *enricherFakeClient) SendInteractiveCard(context.Context, SendCardParams) (string, error) { diff --git a/server/internal/integrations/lark/media_ingest.go b/server/internal/integrations/lark/media_ingest.go new file mode 100644 index 00000000000..66f3a2c309f --- /dev/null +++ b/server/internal/integrations/lark/media_ingest.go @@ -0,0 +1,418 @@ +package lark + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "io" + "log/slog" + "mime" + "path" + "strings" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "github.com/multica-ai/multica/server/internal/integrations/channel" + "github.com/multica-ai/multica/server/internal/integrations/channel/engine" +) + +type mediaStorage interface { + Upload(ctx context.Context, key string, data []byte, contentType string, filename string) (string, error) +} + +type mediaStreamStorage interface { + UploadStream(ctx context.Context, key string, data io.Reader, sizeBytes int64, contentType string, filename string) (string, error) +} + +type mediaDeleteStorage interface { + Delete(ctx context.Context, key string) +} + +// mediaUploadDiscardTimeout bounds the immediate idempotent delete after a +// result-uncertain upload error. Fresh budget: the failing case includes the +// resolve context itself having expired mid-write. +const mediaUploadDiscardTimeout = 5 * time.Second + +type messageResourceStreamer interface { + DownloadMessageResourceStream(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResourceStream, error) +} + +type feishuMediaResolver struct { + api APIClient + creds CredentialsResolver + storage mediaStorage + logger *slog.Logger +} + +func NewFeishuMediaResolver(api APIClient, creds CredentialsResolver, storage mediaStorage, logger *slog.Logger) engine.MediaResolver { + if logger == nil { + logger = slog.Default() + } + return &feishuMediaResolver{api: api, creds: creds, storage: storage, logger: logger} +} + +// DiscardMedia deletes uploaded objects that will never gain an attachment +// row (resolution deadline expired, attachment binding failed). Deletion is +// keyed by StorageKey and best-effort: the storage backends log their own +// failures, and a leaked object here has no other reclaim path. +func (r *feishuMediaResolver) DiscardMedia(ctx context.Context, refs []channel.MediaRef) { + if len(refs) == 0 { + return + } + deleter, ok := r.storage.(mediaDeleteStorage) + if !ok { + r.logMediaWarn("lark media discard skipped: storage cannot delete", InboundMessage{}, nil) + return + } + for _, ref := range refs { + if ref.StorageKey == "" { + continue + } + deleter.Delete(ctx, ref.StorageKey) + } +} + +// HasMedia reports whether the message carries downloadable Feishu resources +// (standalone image/video or post-embedded img/media spans). Pure in-memory +// decode of the already-received payload — it runs on the connector ACK path. +func (r *feishuMediaResolver) HasMedia(msg channel.InboundMessage) bool { + if len(msg.MediaRefs) > 0 { + return true + } + lm, err := larkMsgFromRaw(msg) + if err != nil { + return false + } + return len(mediaResourcesFromMessage(lm)) > 0 +} + +func (r *feishuMediaResolver) ResolveMedia(ctx context.Context, inst engine.ResolvedInstallation, _ engine.ResolvedIdentity, _ pgtype.UUID, msg channel.InboundMessage) channel.InboundMessage { + if len(msg.MediaRefs) > 0 { + return msg + } + lm, err := larkMsgFromRaw(msg) + if err != nil { + r.logMediaWarn("lark media ingest skipped: raw decode failed", InboundMessage{MessageID: msg.MessageID}, err) + return msg + } + resources := mediaResourcesFromMessage(lm) + if len(resources) == 0 { + return msg + } + if r.api == nil || r.creds == nil || r.storage == nil { + r.logMediaWarn("lark media ingest skipped: missing dependency", lm, nil) + return msg + } + larkInst, ok := inst.Platform.(Installation) + if !ok { + r.logMediaWarn("lark media ingest skipped: installation payload unavailable", lm, nil) + return msg + } + creds, err := installationCredentialsFor(larkInst, r.creds) + if err != nil { + r.logMediaWarn("lark media ingest skipped: credentials unavailable", lm, err) + return msg + } + for _, res := range resources { + got, err := r.downloadResource(ctx, creds, DownloadResourceParams{ + MessageID: res.messageID, + FileKey: res.key, + Type: res.fetchType, + }) + if err != nil { + r.logMediaWarn("lark media download failed", lm, err) + continue + } + contentType := got.ContentType + if contentType == "" { + contentType = res.mimeType + } + if contentType == "" { + contentType = "application/octet-stream" + } + filename := mediaFilename(lm, res, got, contentType) + key := mediaObjectKey(inst, res) + link, uploadedBytes, err := r.uploadResource(ctx, key, got.Body, got.SizeBytes, contentType, filename) + if err != nil { + r.logMediaWarn("lark media upload failed", lm, err) + // The object may have landed server-side even though the client + // saw an error (lost response, deadline firing mid-write) — and + // with the key never reaching the router, DiscardMedia would + // never learn about it. The key is deterministic and already + // computed, so delete it idempotently on a fresh budget. + discardCtx, discardCancel := context.WithTimeout(context.Background(), mediaUploadDiscardTimeout) + r.DiscardMedia(discardCtx, []channel.MediaRef{{StorageKey: key}}) + discardCancel() + continue + } + sizeBytes := got.SizeBytes + if sizeBytes == 0 { + sizeBytes = uploadedBytes + } + msg.MediaRefs = append(msg.MediaRefs, channel.MediaRef{ + Type: res.kind, + StorageKey: key, + StorageURL: link, + Filename: filename, + MimeType: contentType, + SizeBytes: sizeBytes, + }) + } + return msg +} + +func mediaObjectKey(inst engine.ResolvedInstallation, res larkMediaResource) string { + sum := sha256.Sum256([]byte(res.messageID + "\x00" + res.fetchType + "\x00" + res.key)) + return path.Join( + "workspaces", + uuidString(inst.WorkspaceID), + "lark", + uuidString(inst.ID), + hex.EncodeToString(sum[:]), + ) +} + +func (r *feishuMediaResolver) downloadResource(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResourceStream, error) { + if streamer, ok := r.api.(messageResourceStreamer); ok { + return streamer.DownloadMessageResourceStream(ctx, creds, p) + } + got, err := r.api.DownloadMessageResource(ctx, creds, p) + if err != nil { + return DownloadedResourceStream{}, err + } + return DownloadedResourceStream{ + Body: io.NopCloser(bytes.NewReader(got.Data)), + ContentType: got.ContentType, + Filename: got.Filename, + SizeBytes: got.SizeBytes, + }, nil +} + +func (r *feishuMediaResolver) uploadResource(ctx context.Context, key string, body io.ReadCloser, sizeBytes int64, contentType string, filename string) (string, int64, error) { + defer body.Close() + counter := &countingReader{r: body} + if streamStorage, ok := r.storage.(mediaStreamStorage); ok && sizeBytes > 0 { + link, err := streamStorage.UploadStream(ctx, key, counter, sizeBytes, contentType, filename) + return link, counter.n, err + } + // Unknown-length HTTP bodies cannot be sent through S3 PutObject as a + // non-seekable stream. The transport already enforces the 100 MiB resource + // cap, so buffer this uncommon fallback and use the seekable Upload path. + data, err := io.ReadAll(counter) + if err != nil { + return "", counter.n, err + } + link, err := r.storage.Upload(ctx, key, data, contentType, filename) + return link, int64(len(data)), err +} + +type countingReader struct { + r io.Reader + n int64 +} + +func (r *countingReader) Read(p []byte) (int, error) { + n, err := r.r.Read(p) + r.n += int64(n) + return n, err +} + +func (r *feishuMediaResolver) logMediaWarn(msg string, lm InboundMessage, err error) { + if r.logger == nil { + return + } + args := []any{"message_id", lm.MessageID, "message_type", lm.MessageType} + if err != nil { + args = append(args, "err", err) + } + r.logger.Warn(msg, args...) +} + +type larkMediaResource struct { + key string + kind channel.MsgType + fetchType string + filename string + mimeType string + sizeBytes int64 + messageID string +} + +func mediaResourcesFromMessage(lm InboundMessage) []larkMediaResource { + var payload struct { + ImageKey string `json:"image_key"` + FileKey string `json:"file_key"` + FileName string `json:"file_name"` + Name string `json:"name"` + MimeType string `json:"mime_type"` + ContentType string `json:"content_type"` + Size int64 `json:"size"` + SizeBytes int64 `json:"size_bytes"` + } + if lm.Content == "" || json.Unmarshal([]byte(lm.Content), &payload) != nil { + return nil + } + filename := firstNonEmpty(payload.FileName, payload.Name) + mimeType := firstNonEmpty(payload.MimeType, payload.ContentType) + sizeBytes := payload.SizeBytes + if sizeBytes == 0 { + sizeBytes = payload.Size + } + switch lm.MessageType { + case "image": + if payload.ImageKey == "" { + return nil + } + return []larkMediaResource{{ + key: payload.ImageKey, + kind: channel.MsgTypeImage, + fetchType: "image", + filename: filename, + mimeType: mimeType, + sizeBytes: sizeBytes, + messageID: lm.MessageID, + }} + case "post": + return mediaResourcesFromPost(lm) + case "media", "video": + if payload.FileKey == "" { + return nil + } + return []larkMediaResource{{ + key: payload.FileKey, + kind: channel.MsgTypeVideo, + fetchType: "file", + filename: filename, + mimeType: mimeType, + sizeBytes: sizeBytes, + messageID: lm.MessageID, + }} + default: + return nil + } +} + +func mediaResourcesFromPost(lm InboundMessage) []larkMediaResource { + if lm.Content == "" { + return nil + } + var doc larkPostContent + if err := json.Unmarshal([]byte(lm.Content), &doc); err != nil { + return nil + } + var out []larkMediaResource + for _, para := range doc.Content { + for _, span := range para { + switch span.Tag { + case "img": + if span.ImageKey == "" { + continue + } + out = append(out, larkMediaResource{ + key: span.ImageKey, + kind: channel.MsgTypeImage, + fetchType: "image", + filename: firstNonEmpty(span.FileName, span.Name), + mimeType: span.MimeType, + messageID: lm.MessageID, + }) + case "media": + if span.FileKey == "" { + continue + } + out = append(out, larkMediaResource{ + key: span.FileKey, + kind: channel.MsgTypeVideo, + fetchType: "file", + filename: firstNonEmpty(span.FileName, span.Name), + mimeType: span.MimeType, + messageID: lm.MessageID, + }) + } + } + } + return out +} + +func mediaFilename(lm InboundMessage, res larkMediaResource, got DownloadedResourceStream, contentType string) string { + for _, candidate := range []string{got.Filename, res.filename} { + if name := cleanFilename(candidate); name != "" { + return name + } + } + prefix := "feishu-file" + switch res.kind { + case channel.MsgTypeImage: + prefix = "feishu-image" + case channel.MsgTypeVideo: + prefix = "feishu-video" + } + return prefix + "-" + safePathSegment(lm.MessageID) + mediaExtension(contentType) +} + +func cleanFilename(name string) string { + name = strings.TrimSpace(name) + if name == "" { + return "" + } + name = path.Base(strings.ReplaceAll(name, "\\", "/")) + if name == "." || name == "/" { + return "" + } + return name +} + +func mediaExtension(contentType string) string { + if semi := strings.IndexByte(contentType, ';'); semi >= 0 { + contentType = strings.TrimSpace(contentType[:semi]) + } + switch contentType { + case "image/jpeg": + return ".jpg" + case "image/png": + return ".png" + case "image/gif": + return ".gif" + case "image/webp": + return ".webp" + case "video/mp4": + return ".mp4" + } + if exts, err := mime.ExtensionsByType(contentType); err == nil && len(exts) > 0 { + return exts[0] + } + return "" +} + +func safePathSegment(s string) string { + s = strings.TrimSpace(s) + if s == "" { + return "unknown" + } + var b strings.Builder + for _, r := range s { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '-', r == '_': + b.WriteRune(r) + default: + b.WriteByte('_') + } + } + out := strings.Trim(b.String(), "_") + if out == "" { + return "unknown" + } + return out +} + +func firstNonEmpty(values ...string) string { + for _, v := range values { + if strings.TrimSpace(v) != "" { + return strings.TrimSpace(v) + } + } + return "" +} diff --git a/server/internal/integrations/lark/outbound.go b/server/internal/integrations/lark/outbound.go index 5eae713462b..e9893c18c44 100644 --- a/server/internal/integrations/lark/outbound.go +++ b/server/internal/integrations/lark/outbound.go @@ -11,6 +11,7 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" "github.com/multica-ai/multica/server/internal/events" + "github.com/multica-ai/multica/server/internal/integrations/channel/engine" db "github.com/multica-ai/multica/server/pkg/db/generated" "github.com/multica-ai/multica/server/pkg/protocol" ) @@ -142,6 +143,7 @@ func (defaultRenderer) Render(in RenderInput) (CardRender, error) { // without a real Postgres connection. type PatcherQueries interface { GetAgentTask(ctx context.Context, id pgtype.UUID) (db.AgentTaskQueue, error) + TaskHasChannelIngestedMessages(ctx context.Context, taskID pgtype.UUID) (bool, error) GetChatSession(ctx context.Context, id pgtype.UUID) (db.ChatSession, error) GetAgent(ctx context.Context, id pgtype.UUID) (db.Agent, error) GetLarkInstallation(ctx context.Context, id pgtype.UUID) (Installation, error) @@ -299,12 +301,18 @@ func (p *Patcher) processEvent(ctx context.Context, e events.Event) error { // Only bound sessions reach here, so classify the task origin before // spending any send work. Web/mobile direct-chat tasks can reuse a session // that originated in Lark, but their replies belong only in Multica. - // Channel-created tasks leave chat_input_task_id NULL and continue below. + // Sealed channel tasks own an input batch just like direct tasks, so the + // discriminator is the immutable channel_ingested provenance of that + // batch, not chat_input_task_id presence (which #5645 originally used). task, err := p.queries.GetAgentTask(ctx, taskID) if err != nil { return fmt.Errorf("load agent task: %w", err) } - if task.ChatInputTaskID.Valid { + deliver, err := engine.TaskInputIsChannelIngested(ctx, p.queries, task) + if err != nil { + return fmt.Errorf("classify task input origin: %w", err) + } + if !deliver { return nil } diff --git a/server/internal/integrations/lark/outbound_test.go b/server/internal/integrations/lark/outbound_test.go index 8fd178e30eb..b8964710d65 100644 --- a/server/internal/integrations/lark/outbound_test.go +++ b/server/internal/integrations/lark/outbound_test.go @@ -17,25 +17,29 @@ import ( ) type fakePatcherQueries struct { - mu sync.Mutex - task db.AgentTaskQueue - taskErr error - binding ChatSessionBinding - bindingErr error - installation Installation - installationErr error - agent db.Agent - agentErr error - card OutboundCardMessage - cardErr error - created []CreateOutboundCardMessageParams - createReturn OutboundCardMessage - statusUpdates []UpdateOutboundCardStatusParams + mu sync.Mutex + task db.AgentTaskQueue + taskErr error + taskChannelIngested bool + 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 f.task, f.taskErr } +func (f *fakePatcherQueries) TaskHasChannelIngestedMessages(ctx context.Context, taskID pgtype.UUID) (bool, error) { + return f.taskChannelIngested, nil +} func (f *fakePatcherQueries) GetChatSession(ctx context.Context, id pgtype.UUID) (db.ChatSession, error) { return db.ChatSession{}, nil } @@ -151,6 +155,9 @@ func (f *fakeAPIClient) GetMessage(ctx context.Context, creds InstallationCreden func (f *fakeAPIClient) ListChatMessages(ctx context.Context, creds InstallationCredentials, p ListMessagesParams) ([]LarkMessage, error) { return nil, nil } +func (f *fakeAPIClient) DownloadMessageResource(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResource, error) { + return DownloadedResource{}, nil +} func (f *fakeAPIClient) BatchGetUsers(ctx context.Context, creds InstallationCredentials, openIDs []string) (map[string]string, error) { return nil, nil } @@ -193,6 +200,34 @@ func newTestPatcher(t *testing.T) (*Patcher, *fakePatcherQueries, *fakeAPIClient // 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. +// TestPatcherSendsSealedChannelTaskReply is the other half of the boundary: +// a sealed channel task owns an input batch exactly like a direct task, so +// gating outbound on owner presence alone would silently drop every channel +// reply. Channel provenance must let the reply through. +func TestPatcherSendsSealedChannelTaskReply(t *testing.T) { + p, q, api := newTestPatcher(t) + taskID := uuidFromString(t, "ee555555-ee55-ee55-ee55-eeeeeeeeeeee") + q.task = db.AgentTaskQueue{ChatInputTaskID: taskID} + q.taskChannelIngested = true + + 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: "channel answer", + }, + }) + + api.mu.Lock() + defer api.mu.Unlock() + if len(api.textSent) != 1 || api.textSent[0].Text != "channel answer" { + t.Fatalf("sealed channel reply must reach Lark; textSent=%+v", api.textSent) + } +} + func TestPatcherSendsPlainTextOnChatDone(t *testing.T) { p, q, api := newTestPatcher(t) taskID := uuidFromString(t, "ee333333-ee33-ee33-ee33-eeeeeeeeeeee") @@ -349,9 +384,9 @@ func TestPatcherSkipsWhenNoChatSessionBinding(t *testing.T) { // TestPatcherSkipsDirectChatTaskOnBoundSession guards the channel boundary: // opening a Lark-bound session in the web/mobile UI must not make that direct -// conversation's reply or failure leak back into the external chat. Direct -// tasks own an input batch through chat_input_task_id; channel tasks leave it -// NULL and continue through the existing outbound paths. +// conversation's reply or failure leak back into the external chat. Sealed +// channel tasks own an input batch too, so the discriminator is the +// channel_ingested provenance of the owned batch, not owner presence. func TestPatcherSkipsDirectChatTaskOnBoundSession(t *testing.T) { tests := []struct { name string diff --git a/server/internal/integrations/lark/outcome_replier_test.go b/server/internal/integrations/lark/outcome_replier_test.go index 89bb04600a2..50ddc8f9405 100644 --- a/server/internal/integrations/lark/outcome_replier_test.go +++ b/server/internal/integrations/lark/outcome_replier_test.go @@ -90,6 +90,9 @@ func (s *stubAPIClientWithRecorder) GetMessage(ctx context.Context, creds Instal func (s *stubAPIClientWithRecorder) ListChatMessages(ctx context.Context, creds InstallationCredentials, p ListMessagesParams) ([]LarkMessage, error) { return nil, nil } +func (s *stubAPIClientWithRecorder) DownloadMessageResource(ctx context.Context, creds InstallationCredentials, p DownloadResourceParams) (DownloadedResource, error) { + return DownloadedResource{}, nil +} func (s *stubAPIClientWithRecorder) BatchGetUsers(ctx context.Context, creds InstallationCredentials, openIDs []string) (map[string]string, error) { return nil, nil } diff --git a/server/internal/integrations/lark/typing_indicator_test.go b/server/internal/integrations/lark/typing_indicator_test.go index c9a10db3e28..8779fec0fdf 100644 --- a/server/internal/integrations/lark/typing_indicator_test.go +++ b/server/internal/integrations/lark/typing_indicator_test.go @@ -56,6 +56,9 @@ func (f *fakeTypingAPIClient) GetMessage(context.Context, InstallationCredential func (f *fakeTypingAPIClient) ListChatMessages(context.Context, InstallationCredentials, ListMessagesParams) ([]LarkMessage, error) { return nil, nil } +func (f *fakeTypingAPIClient) DownloadMessageResource(context.Context, InstallationCredentials, DownloadResourceParams) (DownloadedResource, error) { + return DownloadedResource{}, nil +} func (f *fakeTypingAPIClient) BatchGetUsers(context.Context, InstallationCredentials, []string) (map[string]string, error) { return nil, nil } diff --git a/server/internal/integrations/lark/ws_frame_decoder.go b/server/internal/integrations/lark/ws_frame_decoder.go index f09dc2b3417..4c4578b55bd 100644 --- a/server/internal/integrations/lark/ws_frame_decoder.go +++ b/server/internal/integrations/lark/ws_frame_decoder.go @@ -73,6 +73,7 @@ func (d *LarkJSONFrameDecoder) Decode(payload []byte, inst Installation) (Inboun MessageID: evt.Message.MessageID, SenderOpenID: OpenID(evt.Sender.SenderID.OpenID), MessageType: evt.Message.MessageType, + Content: evt.Message.Content, CreateTime: evt.Message.CreateTime, // parent_id / root_id are populated by Lark only in reply // scenarios. The enricher keys quoted-reply expansion off @@ -91,16 +92,18 @@ func (d *LarkJSONFrameDecoder) Decode(payload []byte, inst Installation) (Inboun botUnionID = inst.BotUnionID.String } - // text + post are flattened synchronously here (no external calls — - // the decoder must stay fast and dependency-free). merge_forward - // leaves Body empty: it needs an HTTP round-trip to expand and is - // handled downstream by the enricher, which keys off MessageType. - // Other types (image, file, …) also leave Body empty in this MVP; - // attachment ingestion is a separate issue. + // text + post are flattened synchronously here (no external calls — the + // decoder must stay fast and dependency-free). merge_forward leaves Body + // empty: it needs an HTTP round-trip to expand and is handled downstream by + // the enricher, which keys off MessageType. Standalone media gets a short + // visible marker while the channel adapter separately downloads and binds + // the binary as a Multica attachment. switch evt.Message.MessageType { case "text", "post": msg.Body = resolveMentions(flattenContent(evt.Message.MessageType, evt.Message.Content), evt.Message.Mentions, inst.BotOpenID, botUnionID) + case "image", "file", "audio", "media", "video": + msg.Body = flattenContent(evt.Message.MessageType, evt.Message.Content) } // Snapshot the user's own text as the command source BEFORE any diff --git a/server/internal/integrations/lark/ws_frame_decoder_test.go b/server/internal/integrations/lark/ws_frame_decoder_test.go index dcc51e54aae..da02c250c3a 100644 --- a/server/internal/integrations/lark/ws_frame_decoder_test.go +++ b/server/internal/integrations/lark/ws_frame_decoder_test.go @@ -429,7 +429,7 @@ func TestLarkJSONFrameDecoderMessageContentEmptyOnInvalidContentJSON(t *testing. } } -func TestLarkJSONFrameDecoderNonTextMessageHasEmptyBody(t *testing.T) { +func TestLarkJSONFrameDecoderMediaMessageKeepsPlaceholderAndContent(t *testing.T) { t.Parallel() raw := []byte(`{ "type":"event_callback", @@ -443,8 +443,11 @@ func TestLarkJSONFrameDecoderNonTextMessageHasEmptyBody(t *testing.T) { if err != nil || !ok { t.Fatalf("ok=%v err=%v", ok, err) } - if msg.Body != "" { - t.Errorf("Body = %q; non-text messages should have empty body in MVP", msg.Body) + if msg.Body != "[Image]" { + t.Errorf("Body = %q; want media placeholder", msg.Body) + } + if msg.Content != `{"image_key":"img1"}` { + t.Errorf("Content = %q; want raw media content", msg.Content) } if msg.MessageID == "" { t.Error("MessageID should still be populated for non-text events") diff --git a/server/internal/integrations/slack/outbound.go b/server/internal/integrations/slack/outbound.go index 434b256a843..e68cb690407 100644 --- a/server/internal/integrations/slack/outbound.go +++ b/server/internal/integrations/slack/outbound.go @@ -14,6 +14,7 @@ import ( "github.com/multica-ai/multica/server/internal/events" "github.com/multica-ai/multica/server/internal/integrations/channel" + "github.com/multica-ai/multica/server/internal/integrations/channel/engine" "github.com/multica-ai/multica/server/internal/util" db "github.com/multica-ai/multica/server/pkg/db/generated" "github.com/multica-ai/multica/server/pkg/protocol" @@ -23,6 +24,7 @@ import ( // subscriber needs. *db.Queries satisfies it. type outboundQueries interface { GetAgentTask(ctx context.Context, id pgtype.UUID) (db.AgentTaskQueue, error) + TaskHasChannelIngestedMessages(ctx context.Context, taskID pgtype.UUID) (bool, error) GetChannelChatSessionBindingBySession(ctx context.Context, arg db.GetChannelChatSessionBindingBySessionParams) (db.ChannelChatSessionBinding, error) GetChannelInstallation(ctx context.Context, arg db.GetChannelInstallationParams) (db.ChannelInstallation, error) } @@ -101,7 +103,10 @@ func (o *Outbound) processEvent(ctx context.Context, e events.Event) error { // before loading credentials or sending. Web/mobile direct-chat tasks can // reuse a session that originated in Slack, but their replies belong only in // Multica. Outbound delivery fails closed when the origin cannot be - // established; channel-created tasks leave chat_input_task_id NULL and send. + // established. Sealed channel tasks own an input batch just like direct + // tasks, so the discriminator is the immutable channel_ingested provenance + // of that batch, not chat_input_task_id presence (which #5645 originally + // used). taskID, ok := chatDoneTaskID(e) if !ok { return nil @@ -110,7 +115,11 @@ func (o *Outbound) processEvent(ctx context.Context, e events.Event) error { if err != nil { return fmt.Errorf("load agent task: %w", err) } - if task.ChatInputTaskID.Valid { + deliver, err := engine.TaskInputIsChannelIngested(ctx, o.q, task) + if err != nil { + return fmt.Errorf("classify task input origin: %w", err) + } + if !deliver { return nil } inst, err := o.q.GetChannelInstallation(ctx, db.GetChannelInstallationParams{ diff --git a/server/internal/integrations/slack/outbound_test.go b/server/internal/integrations/slack/outbound_test.go index 724e7e98c9a..5faa7f6f111 100644 --- a/server/internal/integrations/slack/outbound_test.go +++ b/server/internal/integrations/slack/outbound_test.go @@ -23,18 +23,23 @@ func uid(b byte) pgtype.UUID { } type fakeOutboundQueries struct { - binding db.ChannelChatSessionBinding - bindingErr error - inst db.ChannelInstallation - instErr error - task db.AgentTaskQueue - taskErr error + binding db.ChannelChatSessionBinding + bindingErr error + inst db.ChannelInstallation + instErr error + task db.AgentTaskQueue + taskErr error + taskChannelIngested bool } func (f *fakeOutboundQueries) GetAgentTask(context.Context, pgtype.UUID) (db.AgentTaskQueue, error) { return f.task, f.taskErr } +func (f *fakeOutboundQueries) TaskHasChannelIngestedMessages(context.Context, pgtype.UUID) (bool, error) { + return f.taskChannelIngested, nil +} + func (f *fakeOutboundQueries) GetChannelChatSessionBindingBySession(context.Context, db.GetChannelChatSessionBindingBySessionParams) (db.ChannelChatSessionBinding, error) { return f.binding, f.bindingErr } @@ -102,6 +107,29 @@ func TestOutbound_SkipsDirectChatTaskOnBoundSlackSession(t *testing.T) { } } +// A sealed channel task owns an input batch exactly like a direct task; the +// outbound gate must key on channel provenance, not owner presence, or every +// Slack reply is silently dropped. +func TestOutbound_PostsSealedChannelTaskReply(t *testing.T) { + q := &fakeOutboundQueries{ + task: db.AgentTaskQueue{ChatInputTaskID: uid(2)}, + taskChannelIngested: true, + binding: db.ChannelChatSessionBinding{ + InstallationID: uid(1), + ChannelChatID: "C123", + Config: []byte(`{"channel_id":"C123"}`), + }, + inst: db.ChannelInstallation{ID: uid(1), Status: "active", Config: slackInstallConfigJSON()}, + } + fs := &fakeSender{} + + newTestOutbound(q, fs).handleEvent(chatDoneEvent("00000000-0000-0000-0000-000000000001", "channel answer")) + + if fs.called != 1 { + t.Fatalf("sender called %d times, want 1 for a sealed channel task", fs.called) + } +} + func TestOutbound_PostsReplyToBoundSlackChannel(t *testing.T) { q := &fakeOutboundQueries{ // Composite isolation key; real channel + reply thread come from config / diff --git a/server/internal/integrations/slack/resolvers.go b/server/internal/integrations/slack/resolvers.go index fe3f95ec04a..b07a3e4084d 100644 --- a/server/internal/integrations/slack/resolvers.go +++ b/server/internal/integrations/slack/resolvers.go @@ -340,10 +340,21 @@ func (r *sessionBinder) AppendMessage(ctx context.Context, p engine.AppendParams InstallationID: p.InstallationID, Body: p.Message.Text, // Slack text is not enriched, so the command source is the body itself. - CommandText: p.Message.Text, - MessageID: p.Message.MessageID, - ThreadID: replyThread, - ClaimToken: p.ClaimToken, + CommandText: p.Message.Text, + MessageID: p.Message.MessageID, + ThreadID: replyThread, + ClaimToken: p.ClaimToken, + MediaPendingUntil: p.MediaPendingUntil, + }) +} + +func (r *sessionBinder) BindMedia(ctx context.Context, p engine.BindMediaParams) error { + return r.session.BindMediaRefs(ctx, engine.BindMediaInput{ + MessageID: p.MessageID, + SessionID: p.SessionID, + WorkspaceID: p.WorkspaceID, + Sender: p.Sender, + MediaRefs: p.MediaRefs, }) } diff --git a/server/internal/service/attribution_stamp_test.go b/server/internal/service/attribution_stamp_test.go index 3b152289640..5549c7d5271 100644 --- a/server/internal/service/attribution_stamp_test.go +++ b/server/internal/service/attribution_stamp_test.go @@ -1266,3 +1266,88 @@ func TestEnqueueChatTaskStampsChatEvidence(t *testing.T) { t.Errorf("trigger_evidence_ref_id = %s, want chat session %s", util.UUIDToString(evidenceRef), chatSessionID) } } + +func TestEnqueueChatTaskDefersForChannelMediaAndPromotesWhenReady(t *testing.T) { + pool := newResolveOriginatorPool(t) + ctx := context.Background() + q := db.New(pool) + workspaceID, userID, agentID, _ := seedAttributionFixture(t, pool) + + var chatSessionID, priorMessageID, messageID string + if err := pool.QueryRow(ctx, ` + INSERT INTO chat_session (workspace_id, agent_id, creator_id) + VALUES ($1, $2, $3) RETURNING id`, workspaceID, agentID, userID).Scan(&chatSessionID); err != nil { + t.Fatalf("seed chat session: %v", err) + } + t.Cleanup(func() { + _, _ = pool.Exec(context.Background(), `DELETE FROM chat_session WHERE id = $1`, chatSessionID) + }) + if err := pool.QueryRow(ctx, ` + INSERT INTO chat_message (chat_session_id, role, content) + VALUES ($1, 'user', 'first') RETURNING id`, chatSessionID).Scan(&priorMessageID); err != nil { + t.Fatalf("seed prior message: %v", err) + } + + svc := &TaskService{Queries: q, TxStarter: pool, Bus: events.New()} + priorTask, err := svc.EnqueueChatTask(ctx, db.ChatSession{ + ID: util.MustParseUUID(chatSessionID), + AgentID: util.MustParseUUID(agentID), + }, util.MustParseUUID(userID), false) + if err != nil { + t.Fatalf("enqueue prior chat task: %v", err) + } + deadline := time.Now().Add(time.Minute) + if err := pool.QueryRow(ctx, ` + INSERT INTO chat_message (chat_session_id, role, content, channel_media_pending_until) + VALUES ($1, 'user', '[Image]', $2) RETURNING id`, chatSessionID, deadline).Scan(&messageID); err != nil { + t.Fatalf("seed pending media message: %v", err) + } + + task, err := svc.EnqueueChatTask(ctx, db.ChatSession{ + ID: util.MustParseUUID(chatSessionID), + AgentID: util.MustParseUUID(agentID), + }, util.MustParseUUID(userID), false) + if err != nil { + t.Fatalf("EnqueueChatTask: %v", err) + } + if task.Status != "deferred" || !task.FireAt.Valid { + t.Fatalf("task = status %q fire_at %v, want durable deferred task", task.Status, task.FireAt) + } + if !task.ChatInputTaskID.Valid || task.ChatInputTaskID.Bytes != task.ID.Bytes { + t.Fatalf("task input owner = %s, want self %s", util.UUIDToString(task.ChatInputTaskID), util.UUIDToString(task.ID)) + } + if task.FireAt.Time.Before(deadline.Add(-time.Second)) || task.FireAt.Time.After(deadline.Add(time.Second)) { + t.Fatalf("task fire_at = %v, want media deadline %v", task.FireAt.Time, deadline) + } + var linkedTaskID pgtype.UUID + if err := pool.QueryRow(ctx, `SELECT task_id FROM chat_message WHERE id = $1`, messageID).Scan(&linkedTaskID); err != nil { + t.Fatalf("load sealed media message: %v", err) + } + if !linkedTaskID.Valid || linkedTaskID.Bytes != task.ID.Bytes { + t.Fatalf("message task_id = %s, want %s", util.UUIDToString(linkedTaskID), util.UUIDToString(task.ID)) + } + if err := pool.QueryRow(ctx, `SELECT task_id FROM chat_message WHERE id = $1`, priorMessageID).Scan(&linkedTaskID); err != nil { + t.Fatalf("load prior sealed message: %v", err) + } + if !linkedTaskID.Valid || linkedTaskID.Bytes != priorTask.ID.Bytes { + t.Fatalf("prior message task_id = %s, want unchanged owner %s", util.UUIDToString(linkedTaskID), util.UUIDToString(priorTask.ID)) + } + + if err := q.ClearChatMessageChannelMediaPending(ctx, db.ClearChatMessageChannelMediaPendingParams{ + ID: util.MustParseUUID(messageID), + ChatSessionID: util.MustParseUUID(chatSessionID), + }); err != nil { + t.Fatalf("clear pending media: %v", err) + } + if err := svc.PromoteChannelChatTasksIfMediaReady(ctx, util.MustParseUUID(chatSessionID)); err != nil { + t.Fatalf("PromoteChannelChatTasksIfMediaReady: %v", err) + } + var status string + var fireAt pgtype.Timestamptz + if err := pool.QueryRow(ctx, `SELECT status, fire_at FROM agent_task_queue WHERE id = $1`, task.ID).Scan(&status, &fireAt); err != nil { + t.Fatalf("load promoted task: %v", err) + } + if status != "queued" || fireAt.Valid { + t.Fatalf("promoted task = status %q fire_at %v, want queued with no deadline", status, fireAt) + } +} diff --git a/server/internal/service/task.go b/server/internal/service/task.go index 2900d42085c..385059b097c 100644 --- a/server/internal/service/task.go +++ b/server/internal/service/task.go @@ -1397,8 +1397,10 @@ var ErrChatTaskAgentArchived = errors.New("chat task: agent archived") // path returns a task row, not this error. var ErrChatTaskAgentNoRuntime = errors.New("chat task: agent has no runtime") -// EnqueueChatTask creates a queued task for a chat session. -// Unlike issue tasks, chat tasks have no issue_id. +// EnqueueChatTask creates a task-owned input batch for a chat session. Channel +// media makes the task deferred until binding completes or its durable fallback +// deadline expires; other chat tasks are queued immediately. Unlike issue +// tasks, chat tasks have no issue_id. // // Errors split into two layers: // @@ -1450,12 +1452,28 @@ func (s *TaskService) EnqueueChatTask(ctx context.Context, chatSession db.ChatSe } attrSource, _, attrEvidenceKind, attrEvidenceRef := attributionCreateParams(attr) runtimeMCPOverlay := s.buildRuntimeMCPOverlay(ctx, initiatorUserID, agent) - task, err := s.Queries.CreateChatTask(ctx, db.CreateChatTaskParams{ + + tx, err := s.TxStarter.Begin(ctx) + if err != nil { + return db.AgentTaskQueue{}, fmt.Errorf("begin chat task enqueue: %w", err) + } + defer tx.Rollback(ctx) + qtx := s.Queries.WithTx(tx) + mediaPendingUntil, err := qtx.GetChannelMediaPendingUntil(ctx, chatSession.ID) + if err != nil && !errors.Is(err, pgx.ErrNoRows) { + slog.Error("chat task enqueue failed", "chat_session_id", util.UUIDToString(chatSession.ID), "error", err) + return db.AgentTaskQueue{}, fmt.Errorf("load channel media pending deadline: %w", err) + } + if errors.Is(err, pgx.ErrNoRows) { + mediaPendingUntil = pgtype.Timestamptz{} + } + task, err := qtx.CreateChatTask(ctx, db.CreateChatTaskParams{ AgentID: chatSession.AgentID, RuntimeID: agent.RuntimeID, Priority: 2, // medium priority for chat ChatSessionID: chatSession.ID, InitiatorUserID: initiatorUserID, + FireAt: mediaPendingUntil, OriginatorUserID: initiatorUserID, AccountableUserID: attr.AccountableUserID, ForceFreshSession: pgtype.Bool{ @@ -1472,6 +1490,52 @@ func (s *TaskService) EnqueueChatTask(ctx context.Context, chatSession db.ChatSe slog.Error("chat task enqueue failed", "chat_session_id", util.UUIDToString(chatSession.ID), "error", err) return db.AgentTaskQueue{}, fmt.Errorf("create chat task: %w", err) } + task, err = qtx.SetChatTaskInputOwnerSelf(ctx, task.ID) + if err != nil { + return db.AgentTaskQueue{}, fmt.Errorf("set channel chat task input owner: %w", err) + } + if err := qtx.LinkUnownedChannelChatMessagesToTask(ctx, db.LinkUnownedChannelChatMessagesToTaskParams{ + TaskID: task.ID, + ChatSessionID: chatSession.ID, + }); err != nil { + return db.AgentTaskQueue{}, fmt.Errorf("seal channel chat task input: %w", err) + } + // The deadline read above ran before the seal; under READ COMMITTED a media + // message committed between the two statements is sealed into this task + // without deferring it, and the daemon could claim it before its media + // binds. Re-derive the deferral from the sealed batch itself. + switch corrected, err := qtx.DeferChatTaskForSealedPendingMedia(ctx, task.ID); { + case err == nil: + task = corrected + case !errors.Is(err, pgx.ErrNoRows): + return db.AgentTaskQueue{}, fmt.Errorf("defer chat task for sealed pending media: %w", err) + } + if err := tx.Commit(ctx); err != nil { + return db.AgentTaskQueue{}, fmt.Errorf("commit chat task enqueue: %w", err) + } + + if task.Status == "deferred" { + slog.Info("chat task deferred for channel media", + "task_id", util.UUIDToString(task.ID), + "chat_session_id", util.UUIDToString(chatSession.ID), + "agent_id", util.UUIDToString(chatSession.AgentID), + "fire_at", task.FireAt.Time, + ) + // Fence the clear-vs-create race: media completion may have found no + // committed task after our deadline read. Re-check after commit so the + // task is promoted immediately when the marker has already cleared. + // The task is already durably committed, so a fence failure must not + // surface as an enqueue failure — the router would treat it as "no + // task" and clear the typing indicator while the run still happens. + // The claim-path deferred promoter re-queues it at fire_at regardless. + if err := s.PromoteChannelChatTasksIfMediaReady(ctx, chatSession.ID); err != nil { + slog.Warn("chat task media-ready fence failed; deferred task falls back to its deadline", + "task_id", util.UUIDToString(task.ID), + "chat_session_id", util.UUIDToString(chatSession.ID), + "error", err) + } + return task, nil + } slog.Info("chat task enqueued", "task_id", util.UUIDToString(task.ID), "chat_session_id", util.UUIDToString(chatSession.ID), "agent_id", util.UUIDToString(chatSession.AgentID)) // See EnqueueTaskForIssue for ordering rationale. @@ -1480,6 +1544,27 @@ func (s *TaskService) EnqueueChatTask(ctx context.Context, chatSession db.ChatSe return task, nil } +// PromoteChannelChatTasksIfMediaReady queues channel tasks as soon as every +// unexpired media marker in the session has been cleared. If the process dies +// first, the normal deferred-task promoter queues them at their persisted +// fire_at deadline, preserving the placeholder fallback across restarts. +func (s *TaskService) PromoteChannelChatTasksIfMediaReady(ctx context.Context, sessionID pgtype.UUID) error { + tasks, err := s.Queries.PromoteChannelChatTasksIfMediaReady(ctx, sessionID) + if err != nil { + return fmt.Errorf("promote channel chat tasks after media: %w", err) + } + for _, task := range tasks { + slog.Info("channel media-ready chat task promoted", + "task_id", util.UUIDToString(task.ID), + "chat_session_id", util.UUIDToString(sessionID), + "agent_id", util.UUIDToString(task.AgentID), + ) + s.broadcastTaskEvent(ctx, protocol.EventTaskQueued, task) + s.NotifyTaskEnqueued(ctx, task) + } + return nil +} + // DirectChatSendResult carries the rows a transactional direct-chat send // persisted, so the handler can broadcast the user message and shape its // response without re-reading them. @@ -1758,6 +1843,17 @@ func (s *TaskService) CancelTaskWithResult(ctx context.Context, taskID pgtype.UU }, nil } +// chatInputOwnerID resolves the id the task's user-message input batch is +// keyed on: chat_input_task_id when set (auto-retry clones inherit their +// parent's, so provenance checks reach the parent's sealed messages), falling +// back to the task's own id for legacy rows. +func chatInputOwnerID(task db.AgentTaskQueue) pgtype.UUID { + if task.ChatInputTaskID.Valid { + return task.ChatInputTaskID + } + return task.ID +} + func (s *TaskService) finalizeCancelledChatMessage(ctx context.Context, task db.AgentTaskQueue, opts CancelTaskOptions) *CancelledChatMessageResult { if !task.ChatSessionID.Valid { return nil @@ -1768,7 +1864,25 @@ func (s *TaskService) finalizeCancelledChatMessage(ctx context.Context, task db. if err != nil { return fmt.Errorf("list cancelled chat task messages: %w", err) } - if len(messages) == 0 && task.StartedAt.Valid && opts.ClientSupportsDraftRestore { + restorable := len(messages) == 0 + if restorable { + // Channel-ingested user messages are the durable record of what + // the platform sender wrote — the sender has no Multica composer + // to restore a draft into. The gate is the immutable per-message + // channel_ingested stamp, NOT the channel_chat_session_binding + // row: archiving a session or rebinding an installation deletes + // the binding while the messages (and a still-cancellable task) + // remain. Keyed by the input-batch owner id so an auto-retry + // clone (which inherits chat_input_task_id) reaches the same + // verdict as its parent. A channel task settles as "Stopped." + // below instead of deleting its sealed input batch. + channelIngested, err := qtx.TaskHasChannelIngestedMessages(ctx, chatInputOwnerID(task)) + if err != nil { + return fmt.Errorf("check cancelled chat channel provenance: %w", err) + } + restorable = !channelIngested + } + if restorable && task.StartedAt.Valid && opts.ClientSupportsDraftRestore { // A started task's daemon learns of the cancellation by polling // and may still be flushing its transcript tail, so "empty" is // not trustworthy yet. Defer the judgment until the daemon acks @@ -1788,7 +1902,7 @@ func (s *TaskService) finalizeCancelledChatMessage(ctx context.Context, task db. } return nil } - if len(messages) == 0 { + if restorable { // Detach attachments BEFORE deleting the user message — the // attachment FK is ON DELETE CASCADE, so deleting first would // destroy rows the restored draft needs to re-bind. @@ -1895,7 +2009,19 @@ func (s *TaskService) FinalizeDeferredCancelledChat(ctx context.Context, taskID if err != nil { return fmt.Errorf("list cancelled chat task messages: %w", err) } - if len(messages) == 0 { + restorable := len(messages) == 0 + if restorable { + // Same immutable-provenance guard as finalizeCancelledChatMessage: + // channel tasks never restore-delete their sealed input. The sync + // path no longer defers such tasks; this covers markers created by + // an older replica during a rolling deploy. + channelIngested, err := qtx.TaskHasChannelIngestedMessages(ctx, chatInputOwnerID(claimed)) + if err != nil { + return fmt.Errorf("check cancelled chat channel provenance: %w", err) + } + restorable = !channelIngested + } + if restorable { // The transcript stayed empty through the daemon flush: same // outcome as the synchronous empty branch, but the cancel HTTP // response is long gone and the broadcast is best-effort. The @@ -2755,19 +2881,22 @@ const chatNoResponseFallback = "The agent finished this turn without a text repl // completed chat task inside the caller's completion transaction, returning the // row (nil when none is written). // -// Task-owned direct (web/mobile) tasks — chat_input_task_id set — get the -// explicit single-outcome contract: a non-empty final output becomes an ordinary -// assistant message, and an empty/whitespace output becomes a visible -// no_response outcome carrying a non-empty English fallback body. It never -// auto-retries: an empty output is a legitimate terminal result (a tool-only -// turn) and re-running it would repeat side effects already performed. +// Direct (web/mobile) tasks get the explicit single-outcome contract: a +// non-empty final output becomes an ordinary assistant message, and an +// empty/whitespace output becomes a visible no_response outcome carrying a +// non-empty English fallback body. It never auto-retries: an empty output is a +// legitimate terminal result (a tool-only turn) and re-running it would repeat +// side effects already performed. // -// Legacy and channel (Slack/Lark) tasks — chat_input_task_id NULL — keep the -// prior behavior: a non-empty output writes an ordinary assistant message, but -// an EMPTY output writes NO row, so chat:done carries empty content and the -// channel outbound silently drops it. This preserves Slack/Lark empty-completion -// semantics unchanged (MUL-4351 review): the no_response fallback body must never -// be pushed to an external channel. +// Channel (Slack/Lark) and legacy tasks keep the prior behavior: a non-empty +// output writes an ordinary assistant message, but an EMPTY output writes NO +// row, so chat:done carries empty content and the channel outbound silently +// drops it (MUL-4351 review): the no_response fallback body must never be +// pushed to an external channel. Channel tasks now own a sealed input batch +// (chat_input_task_id set) just like direct tasks, so the discriminator is the +// immutable channel_ingested stamp on the owned batch — keyed by the batch +// owner id so an auto-retry clone (which inherits chat_input_task_id) reaches +// the same verdict as its parent — while a NULL owner marks a legacy task. func (s *TaskService) writeChatCompletionOutcome(ctx context.Context, qtx *db.Queries, task db.AgentTaskQueue, result []byte) (*db.ChatMessage, error) { // result is the daemon request re-marshalled by the handler, so it is always // valid JSON; an empty Output is the only case this branch cares about. @@ -2803,10 +2932,21 @@ func (s *TaskService) writeChatCompletionOutcome(ctx context.Context, qtx *db.Qu // Channel/legacy empty completion with nothing to show: emit no assistant // row, only an empty chat:done for typing/lifecycle. Keeps the Slack/Lark - // silent-drop path. Attachments still force a row — the agent produced a - // deliverable the user must see. - if isEmpty && pendingAttachments == 0 && !task.ChatInputTaskID.Valid { - return nil, nil + // silent-drop path — the outbound patcher forwards any non-empty content + // verbatim, so the fallback body must never be written for a channel task. + // Attachments still force a row — the agent produced a deliverable the + // user must see. + if isEmpty && pendingAttachments == 0 { + if !task.ChatInputTaskID.Valid { + return nil, nil // legacy task + } + channelIngested, err := qtx.TaskHasChannelIngestedMessages(ctx, task.ChatInputTaskID) + if err != nil { + return nil, fmt.Errorf("check chat completion channel provenance: %w", err) + } + if channelIngested { + return nil, nil // channel task + } } params := db.CreateChatMessageParams{ diff --git a/server/internal/service/task_cancel_finalize_test.go b/server/internal/service/task_cancel_finalize_test.go index 1560a5088e2..5c0e10e42b5 100644 --- a/server/internal/service/task_cancel_finalize_test.go +++ b/server/internal/service/task_cancel_finalize_test.go @@ -581,3 +581,104 @@ func TestListChatFinalizeDeferredExpired_HonorsGrace(t *testing.T) { t.Error("backdated marker should be returned once past the grace period") } } + +// seedChannelIngest mirrors the channel append path: the user message carries +// the immutable channel_ingested stamp and the session has a routing binding. +func (f cancelFinalizeFixture) seedChannelIngest(t *testing.T, ctx context.Context) { + t.Helper() + if _, err := f.pool.Exec(ctx, ` + UPDATE chat_message SET channel_ingested = TRUE WHERE id = $1 + `, f.userMessageID); err != nil { + t.Fatalf("stamp channel_ingested: %v", err) + } + if _, err := f.pool.Exec(ctx, ` + INSERT INTO channel_chat_session_binding ( + chat_session_id, installation_id, channel_type, channel_chat_id, chat_type, config + ) + VALUES ($1, gen_random_uuid(), 'feishu', $2, 'p2p', '{}'::jsonb) + `, f.chatSessionID, "oc_cancel_"+f.chatSessionID); err != nil { + t.Fatalf("create channel binding: %v", err) + } + t.Cleanup(func() { + _, _ = f.pool.Exec(context.Background(), `DELETE FROM channel_chat_session_binding WHERE chat_session_id = $1`, f.chatSessionID) + }) +} + +// unbindChannelSession deletes the routing binding the way archiving a session +// or rebinding an installation does — the provenance gate must survive it. +func (f cancelFinalizeFixture) unbindChannelSession(t *testing.T, ctx context.Context) { + t.Helper() + if _, err := f.pool.Exec(ctx, ` + DELETE FROM channel_chat_session_binding WHERE chat_session_id = $1 + `, f.chatSessionID); err != nil { + t.Fatalf("delete channel binding: %v", err) + } +} + +// Channel-ingested user messages are the durable record of what the platform +// sender wrote — the sender has no Multica composer to restore a draft into. +// The gate is the immutable per-message channel_ingested stamp, so it must +// hold even after archiving/rebinding deleted the session binding: cancelling +// the sealed queued task keeps the messages and settles as "Stopped.". +func TestCancelTask_ChannelIngested_ArchivedSessionKeepsSealedMessages(t *testing.T) { + ctx := context.Background() + pool := newCancelFinalizePool(t) + f := createCancelFinalizeFixture(t, ctx, pool, "queued", false) + f.seedChannelIngest(t, ctx) + f.unbindChannelSession(t, ctx) + svc := NewTaskService(db.New(pool), pool, nil, events.New()) + + result, err := svc.CancelTaskWithResult(ctx, util.MustParseUUID(f.taskID), CancelTaskOptions{ClientSupportsDraftRestore: true}) + if err != nil { + t.Fatalf("cancel task: %v", err) + } + if result.CancelledChatMessage != nil { + t.Fatalf("channel task must not produce a restore result, got %+v", result.CancelledChatMessage) + } + if !f.userMessageExists(t, ctx) { + t.Error("channel user chat message must be preserved") + } + if got := f.assistantMessages(t, ctx); len(got) != 1 || got[0] != "Stopped." { + t.Errorf("assistant messages = %v, want [Stopped.]", got) + } + if restores := f.draftRestores(t, ctx); len(restores) != 0 { + t.Errorf("channel task must not create draft restores, got %d", len(restores)) + } + if got := f.chatFinalizeDeferredAt(t, ctx); got != nil { + t.Errorf("channel task must not defer finalize, got %v", got) + } +} + +// Started-empty channel task after archive/unbind: the sync path settles it as +// "Stopped." without marking it deferred, and a marker left behind by an older +// replica during a rolling deploy must still not delete the sealed input when +// the deferred path claims it. +func TestFinalizeDeferredCancelledChat_ChannelIngested_ArchivedSessionKeepsSealedMessages(t *testing.T) { + ctx := context.Background() + pool := newCancelFinalizePool(t) + f := createCancelFinalizeFixture(t, ctx, pool, "running", true) + f.seedChannelIngest(t, ctx) + f.unbindChannelSession(t, ctx) + svc := NewTaskService(db.New(pool), pool, nil, events.New()) + + if _, err := svc.CancelTaskWithResult(ctx, util.MustParseUUID(f.taskID), CancelTaskOptions{ClientSupportsDraftRestore: true}); err != nil { + t.Fatalf("cancel task: %v", err) + } + if got := f.chatFinalizeDeferredAt(t, ctx); got != nil { + t.Errorf("channel task must settle synchronously, deferred at %v", got) + } + + if _, err := pool.Exec(ctx, ` + UPDATE agent_task_queue SET chat_finalize_deferred_at = now() WHERE id = $1 + `, f.taskID); err != nil { + t.Fatalf("mark deferred: %v", err) + } + svc.FinalizeDeferredCancelledChat(ctx, util.MustParseUUID(f.taskID)) + + if !f.userMessageExists(t, ctx) { + t.Error("channel user chat message must be preserved by the deferred path") + } + if restores := f.draftRestores(t, ctx); len(restores) != 0 { + t.Errorf("channel task must not create draft restores, got %d", len(restores)) + } +} diff --git a/server/internal/service/task_channel_media_race_test.go b/server/internal/service/task_channel_media_race_test.go new file mode 100644 index 00000000000..99df3192fcf --- /dev/null +++ b/server/internal/service/task_channel_media_race_test.go @@ -0,0 +1,137 @@ +package service + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/multica-ai/multica/server/internal/events" + "github.com/multica-ai/multica/server/internal/util" + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +// raceInjectTxStarter wraps the pool so the transaction handed to +// EnqueueChatTask runs inject() immediately before the input-batch seal +// statement executes. That reproduces, deterministically, a channel media +// message whose append commits between the session-wide deadline read and +// LinkUnownedChannelChatMessagesToTask — the READ COMMITTED window where the +// seal sees a message the deadline read did not. +type raceInjectTxStarter struct { + pool *pgxpool.Pool + inject func() +} + +func (s *raceInjectTxStarter) Begin(ctx context.Context) (pgx.Tx, error) { + tx, err := s.pool.Begin(ctx) + if err != nil { + return nil, err + } + return &raceInjectTx{Tx: tx, inject: s.inject}, nil +} + +type raceInjectTx struct { + pgx.Tx + inject func() +} + +func (t *raceInjectTx) Exec(ctx context.Context, sql string, args ...any) (pgconn.CommandTag, error) { + if strings.Contains(sql, "LinkUnownedChannelChatMessagesToTask") && t.inject != nil { + t.inject() + t.inject = nil + } + return t.Tx.Exec(ctx, sql, args...) +} + +// TestEnqueueChatTaskDefersWhenMediaMessageCommitsDuringEnqueue pins the fix +// for the enqueue-vs-append race: a media-pending message sealed into the task +// after the deadline read must still leave the task deferred (not claimable) +// until its media binds or the persisted deadline expires. +func TestEnqueueChatTaskDefersWhenMediaMessageCommitsDuringEnqueue(t *testing.T) { + pool := newResolveOriginatorPool(t) + ctx := context.Background() + q := db.New(pool) + workspaceID, userID, agentID, _ := seedAttributionFixture(t, pool) + + var chatSessionID string + if err := pool.QueryRow(ctx, ` + INSERT INTO chat_session (workspace_id, agent_id, creator_id) + VALUES ($1, $2, $3) RETURNING id`, workspaceID, agentID, userID).Scan(&chatSessionID); err != nil { + t.Fatalf("seed chat session: %v", err) + } + t.Cleanup(func() { + _, _ = pool.Exec(context.Background(), `DELETE FROM chat_session WHERE id = $1`, chatSessionID) + }) + // The message that armed this flush: plain text, no media marker. + if _, err := pool.Exec(ctx, ` + INSERT INTO chat_message (chat_session_id, role, content) + VALUES ($1, 'user', 'look at this')`, chatSessionID); err != nil { + t.Fatalf("seed text message: %v", err) + } + + deadline := time.Now().Add(time.Minute) + var mediaMessageID string + svc := &TaskService{ + Queries: q, + TxStarter: &raceInjectTxStarter{pool: pool, inject: func() { + // A concurrent Handle appends an image message (with its media + // marker) and commits — after the deadline read, before the seal. + if err := pool.QueryRow(ctx, ` + INSERT INTO chat_message (chat_session_id, role, content, channel_media_pending_until) + VALUES ($1, 'user', '[Image]', $2) RETURNING id`, chatSessionID, deadline).Scan(&mediaMessageID); err != nil { + t.Errorf("inject media message: %v", err) + } + }}, + Bus: events.New(), + } + + task, err := svc.EnqueueChatTask(ctx, db.ChatSession{ + ID: util.MustParseUUID(chatSessionID), + AgentID: util.MustParseUUID(agentID), + }, util.MustParseUUID(userID), false) + if err != nil { + t.Fatalf("EnqueueChatTask: %v", err) + } + if mediaMessageID == "" { + t.Fatal("race injection did not run") + } + + // The injected message must be sealed into this task's input batch... + var linkedTaskID pgtype.UUID + if err := pool.QueryRow(ctx, `SELECT task_id FROM chat_message WHERE id = $1`, mediaMessageID).Scan(&linkedTaskID); err != nil { + t.Fatalf("load sealed media message: %v", err) + } + if !linkedTaskID.Valid || linkedTaskID.Bytes != task.ID.Bytes { + t.Fatalf("media message task_id = %s, want %s", util.UUIDToString(linkedTaskID), util.UUIDToString(task.ID)) + } + // ...and therefore the task must not be claimable before the media binds. + if task.Status != "deferred" || !task.FireAt.Valid { + t.Fatalf("task = status %q fire_at %v, want deferred until the sealed media deadline", task.Status, task.FireAt) + } + if task.FireAt.Time.Before(deadline.Add(-time.Second)) || task.FireAt.Time.After(deadline.Add(time.Second)) { + t.Fatalf("task fire_at = %v, want sealed media deadline %v", task.FireAt.Time, deadline) + } + + // Media completion still promotes it through the normal path. + if err := q.ClearChatMessageChannelMediaPending(ctx, db.ClearChatMessageChannelMediaPendingParams{ + ID: util.MustParseUUID(mediaMessageID), + ChatSessionID: util.MustParseUUID(chatSessionID), + }); err != nil { + t.Fatalf("clear pending media: %v", err) + } + if err := svc.PromoteChannelChatTasksIfMediaReady(ctx, util.MustParseUUID(chatSessionID)); err != nil { + t.Fatalf("PromoteChannelChatTasksIfMediaReady: %v", err) + } + var status string + if err := pool.QueryRow(ctx, `SELECT status FROM agent_task_queue WHERE id = $1`, task.ID).Scan(&status); err != nil { + t.Fatalf("load promoted task: %v", err) + } + if status != "queued" { + t.Fatalf("promoted task status = %q, want queued", status) + } +} diff --git a/server/internal/storage/local.go b/server/internal/storage/local.go index 0c8bec88744..aae7134f2c2 100644 --- a/server/internal/storage/local.go +++ b/server/internal/storage/local.go @@ -152,6 +152,36 @@ func (s *LocalStorage) Upload(ctx context.Context, key string, data []byte, cont return fmt.Sprintf("/uploads/%s", key), nil } +func (s *LocalStorage) UploadStream(ctx context.Context, key string, data io.Reader, _ int64, contentType string, filename string) (string, error) { + dest := filepath.Join(s.uploadDir, key) + if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil { + return "", fmt.Errorf("local storage MkdirAll: %w", err) + } + f, err := os.Create(dest) + if err != nil { + return "", fmt.Errorf("local storage Create: %w", err) + } + if _, err := io.Copy(f, data); err != nil { + _ = f.Close() + _ = os.Remove(dest) + return "", fmt.Errorf("local storage stream copy: %w", err) + } + if err := f.Close(); err != nil { + _ = os.Remove(dest) + return "", fmt.Errorf("local storage Close: %w", err) + } + if filename != "" { + body, _ := json.Marshal(localMeta{Filename: filename, ContentType: contentType}) + if err := os.WriteFile(dest+metaSuffix, body, 0644); err != nil { + slog.Error("local storage meta write failed", "key", key, "error", err) + } + } + + if s.baseURL != "" { + return fmt.Sprintf("%s/uploads/%s", s.baseURL, key), nil + } + return fmt.Sprintf("/uploads/%s", key), nil +} func (s *LocalStorage) GetFilePath(key string) string { return filepath.Join(s.uploadDir, key) } diff --git a/server/internal/storage/s3.go b/server/internal/storage/s3.go index 0a481c1da0d..2d173dbeaca 100644 --- a/server/internal/storage/s3.go +++ b/server/internal/storage/s3.go @@ -12,6 +12,7 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -274,6 +275,34 @@ func (s *S3Storage) Upload(ctx context.Context, key string, data []byte, content return s.uploadedURL(key), nil } +func (s *S3Storage) UploadStream(ctx context.Context, key string, data io.Reader, sizeBytes int64, contentType string, filename string) (string, error) { + if sizeBytes <= 0 { + return "", fmt.Errorf("s3 PutObject: content length is required for streaming upload") + } + input := &s3.PutObjectInput{ + Bucket: aws.String(s.bucket), + Key: aws.String(key), + Body: data, + ContentLength: aws.Int64(sizeBytes), + ContentType: aws.String(contentType), + ContentDisposition: aws.String(ContentDisposition(contentType, filename)), + CacheControl: aws.String("max-age=432000,public"), + StorageClass: s.storageClass(), + } + _, err := s.client.PutObject(ctx, input, func(opts *s3.Options) { + // A non-seekable stream cannot be rewound for SigV4 payload hashing. + // S3 supports UNSIGNED-PAYLOAD for authenticated requests. Avoid the + // optional trailing-checksum path as well: it requires another rewind + // or aws-chunked framing that S3-compatible backends handle unevenly. + opts.APIOptions = append(opts.APIOptions, v4.SwapComputePayloadSHA256ForUnsignedPayloadMiddleware) + opts.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired + }) + if err != nil { + return "", fmt.Errorf("s3 PutObject: %w", err) + } + return s.uploadedURL(key), nil +} + // uploadedURL returns the URL stored for client consumption after an upload. // Priority: CDN domain > custom endpoint > AWS S3 region-qualified host. The CDN // domain wins even when a custom endpoint is set so S3-compatible backends diff --git a/server/internal/storage/s3_test.go b/server/internal/storage/s3_test.go index aee5a212452..58ecc029ade 100644 --- a/server/internal/storage/s3_test.go +++ b/server/internal/storage/s3_test.go @@ -2,6 +2,9 @@ package storage import ( "context" + "io" + "net/http" + "net/http/httptest" "net/url" "strings" "testing" @@ -12,6 +15,67 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3" ) +func TestS3StorageUploadStreamUsesFixedLengthRequest(t *testing.T) { + type observedRequest struct { + contentLength int64 + decodedLength string + encoding string + transfer []string + body string + } + observed := make(chan observedRequest, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read request body: %v", err) + } + observed <- observedRequest{ + contentLength: r.ContentLength, + decodedLength: r.Header.Get("X-Amz-Decoded-Content-Length"), + encoding: r.Header.Get("Content-Encoding"), + transfer: append([]string(nil), r.TransferEncoding...), + body: string(body), + } + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + client := s3.New(s3.Options{ + Region: "us-east-1", + Credentials: aws.NewCredentialsCache(credentials.NewStaticCredentialsProvider("AKID", "SECRET", "")), + BaseEndpoint: aws.String(server.URL), + UsePathStyle: true, + }) + store := &S3Storage{ + client: client, + bucket: "test-bucket", + region: "us-east-1", + endpointURL: server.URL, + usePathStyle: true, + } + payload := "streamed payload" + nonSeekable := io.LimitReader(strings.NewReader(payload), int64(len(payload))) + if _, err := store.UploadStream(context.Background(), "uploads/media.bin", nonSeekable, int64(len(payload)), "application/octet-stream", "media.bin"); err != nil { + t.Fatalf("UploadStream: %v", err) + } + + got := <-observed + if got.contentLength != int64(len(payload)) || len(got.transfer) != 0 { + t.Fatalf("request is not a fixed-length upload: content_length=%d decoded_length=%q encoding=%q transfer=%v", + got.contentLength, got.decodedLength, got.encoding, got.transfer) + } + if got.body != payload { + t.Fatalf("request body = %q, want %q", got.body, payload) + } +} + +func TestS3StorageUploadStreamRejectsUnknownLength(t *testing.T) { + store := &S3Storage{} + if _, err := store.UploadStream(context.Background(), "uploads/media.bin", strings.NewReader("payload"), 0, "application/octet-stream", "media.bin"); err == nil { + t.Fatal("UploadStream accepted an unknown content length") + } +} + func TestS3StorageKeyFromURL_CustomEndpointPreservesNestedKey(t *testing.T) { s := &S3Storage{ bucket: "test-bucket", diff --git a/server/migrations/207_chat_message_channel_media_pending.down.sql b/server/migrations/207_chat_message_channel_media_pending.down.sql new file mode 100644 index 00000000000..944c9780c72 --- /dev/null +++ b/server/migrations/207_chat_message_channel_media_pending.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE chat_message + DROP COLUMN channel_media_pending_until; diff --git a/server/migrations/207_chat_message_channel_media_pending.up.sql b/server/migrations/207_chat_message_channel_media_pending.up.sql new file mode 100644 index 00000000000..2eb2aef8007 --- /dev/null +++ b/server/migrations/207_chat_message_channel_media_pending.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE chat_message + ADD COLUMN channel_media_pending_until TIMESTAMPTZ; diff --git a/server/migrations/208_chat_message_channel_ingested.down.sql b/server/migrations/208_chat_message_channel_ingested.down.sql new file mode 100644 index 00000000000..be0368b78dd --- /dev/null +++ b/server/migrations/208_chat_message_channel_ingested.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE chat_message + DROP COLUMN channel_ingested; diff --git a/server/migrations/208_chat_message_channel_ingested.up.sql b/server/migrations/208_chat_message_channel_ingested.up.sql new file mode 100644 index 00000000000..e2eff7d2c89 --- /dev/null +++ b/server/migrations/208_chat_message_channel_ingested.up.sql @@ -0,0 +1,7 @@ +-- Immutable provenance for channel-ingested (Feishu/Slack) user messages. +-- The cancel draft-restore path gates on this instead of the deletable +-- channel_chat_session_binding row: archiving a session or rebinding an +-- installation deletes the binding but must never expose the original +-- inbound messages to restore-deletion. +ALTER TABLE chat_message + ADD COLUMN channel_ingested BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/server/pkg/db/generated/chat.sql.go b/server/pkg/db/generated/chat.sql.go index 4457f05e0d5..6a77ab38948 100644 --- a/server/pkg/db/generated/chat.sql.go +++ b/server/pkg/db/generated/chat.sql.go @@ -31,6 +31,22 @@ func (q *Queries) ChatSessionHasUserMessage(ctx context.Context, chatSessionID p return has_user_message, err } +const clearChatMessageChannelMediaPending = `-- name: ClearChatMessageChannelMediaPending :exec +UPDATE chat_message +SET channel_media_pending_until = NULL +WHERE id = $1 AND chat_session_id = $2 +` + +type ClearChatMessageChannelMediaPendingParams struct { + ID pgtype.UUID `json:"id"` + ChatSessionID pgtype.UUID `json:"chat_session_id"` +} + +func (q *Queries) ClearChatMessageChannelMediaPending(ctx context.Context, arg ClearChatMessageChannelMediaPendingParams) error { + _, err := q.db.Exec(ctx, clearChatMessageChannelMediaPending, arg.ID, arg.ChatSessionID) + return err +} + const createChatDraftRestore = `-- name: CreateChatDraftRestore :one INSERT INTO chat_draft_restore (id, chat_session_id, task_id, content, attachment_ids) VALUES ($1, $2, $3, $4, $5) @@ -70,19 +86,29 @@ func (q *Queries) CreateChatDraftRestore(ctx context.Context, arg CreateChatDraf } const createChatMessage = `-- name: CreateChatMessage :one -INSERT INTO chat_message (chat_session_id, role, content, task_id, failure_reason, elapsed_ms, message_kind) -VALUES ($1, $2, $3, $4, $5, $6, COALESCE($7::text, 'message')) -RETURNING id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind +INSERT INTO chat_message ( + chat_session_id, role, content, task_id, failure_reason, elapsed_ms, + message_kind, channel_media_pending_until, channel_ingested +) +VALUES ( + $1, $2, $3, $4, $5, $6, + COALESCE($7::text, 'message'), + $8, + COALESCE($9::boolean, FALSE) +) +RETURNING id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind, channel_media_pending_until, channel_ingested ` type CreateChatMessageParams struct { - ChatSessionID pgtype.UUID `json:"chat_session_id"` - Role string `json:"role"` - Content string `json:"content"` - TaskID pgtype.UUID `json:"task_id"` - FailureReason pgtype.Text `json:"failure_reason"` - ElapsedMs pgtype.Int8 `json:"elapsed_ms"` - MessageKind pgtype.Text `json:"message_kind"` + ChatSessionID pgtype.UUID `json:"chat_session_id"` + Role string `json:"role"` + Content string `json:"content"` + TaskID pgtype.UUID `json:"task_id"` + FailureReason pgtype.Text `json:"failure_reason"` + ElapsedMs pgtype.Int8 `json:"elapsed_ms"` + MessageKind pgtype.Text `json:"message_kind"` + ChannelMediaPendingUntil pgtype.Timestamptz `json:"channel_media_pending_until"` + ChannelIngested pgtype.Bool `json:"channel_ingested"` } // message_kind defaults to 'message' via COALESCE so every existing caller @@ -97,6 +123,8 @@ func (q *Queries) CreateChatMessage(ctx context.Context, arg CreateChatMessagePa arg.FailureReason, arg.ElapsedMs, arg.MessageKind, + arg.ChannelMediaPendingUntil, + arg.ChannelIngested, ) var i ChatMessage err := row.Scan( @@ -109,6 +137,8 @@ func (q *Queries) CreateChatMessage(ctx context.Context, arg CreateChatMessagePa &i.FailureReason, &i.ElapsedMs, &i.MessageKind, + &i.ChannelMediaPendingUntil, + &i.ChannelIngested, ) return i, err } @@ -160,36 +190,41 @@ const createChatTask = `-- name: CreateChatTask :one INSERT INTO agent_task_queue ( agent_id, runtime_id, issue_id, status, priority, chat_session_id, initiator_user_id, originator_user_id, accountable_user_id, force_fresh_session, runtime_mcp_overlay, - runtime_connected_apps, originator_source, trigger_evidence_kind, trigger_evidence_ref_id + runtime_connected_apps, originator_source, trigger_evidence_kind, trigger_evidence_ref_id, + fire_at ) VALUES ( - $1, $2, NULL, 'queued', $3, $4, $5, - $6, + $1, $2, NULL, + CASE WHEN $6::timestamptz IS NULL THEN 'queued' ELSE 'deferred' END, + $3, $4, $5, $7, - COALESCE($8::boolean, FALSE), - $9, + $8, + COALESCE($9::boolean, FALSE), $10, $11, $12, - $13 + $13, + $14, + $6::timestamptz ) RETURNING id, agent_id, issue_id, status, priority, dispatched_at, started_at, completed_at, result, error, created_at, context, runtime_id, session_id, work_dir, trigger_comment_id, chat_session_id, autopilot_run_id, attempt, max_attempts, parent_task_id, failure_reason, trigger_summary, force_fresh_session, is_leader_task, wait_reason, initiator_user_id, handoff_note, prepare_lease_expires_at, squad_id, runtime_mcp_overlay, escalation_for_task_id, fire_at, originator_user_id, runtime_connected_apps, coalesced_comment_ids, delivered_comment_ids, chat_input_task_id, chat_finalize_deferred_at, originator_source, delegated_from_task_id, retry_of_task_id, rerun_of_task_id, rule_version_id, trigger_evidence_kind, trigger_evidence_ref_id, accountable_user_id ` type CreateChatTaskParams struct { - AgentID pgtype.UUID `json:"agent_id"` - RuntimeID pgtype.UUID `json:"runtime_id"` - Priority int32 `json:"priority"` - ChatSessionID pgtype.UUID `json:"chat_session_id"` - InitiatorUserID pgtype.UUID `json:"initiator_user_id"` - OriginatorUserID pgtype.UUID `json:"originator_user_id"` - AccountableUserID pgtype.UUID `json:"accountable_user_id"` - ForceFreshSession pgtype.Bool `json:"force_fresh_session"` - RuntimeMcpOverlay []byte `json:"runtime_mcp_overlay"` - RuntimeConnectedApps []byte `json:"runtime_connected_apps"` - OriginatorSource pgtype.Text `json:"originator_source"` - TriggerEvidenceKind pgtype.Text `json:"trigger_evidence_kind"` - TriggerEvidenceRefID pgtype.UUID `json:"trigger_evidence_ref_id"` + AgentID pgtype.UUID `json:"agent_id"` + RuntimeID pgtype.UUID `json:"runtime_id"` + Priority int32 `json:"priority"` + ChatSessionID pgtype.UUID `json:"chat_session_id"` + InitiatorUserID pgtype.UUID `json:"initiator_user_id"` + FireAt pgtype.Timestamptz `json:"fire_at"` + OriginatorUserID pgtype.UUID `json:"originator_user_id"` + AccountableUserID pgtype.UUID `json:"accountable_user_id"` + ForceFreshSession pgtype.Bool `json:"force_fresh_session"` + RuntimeMcpOverlay []byte `json:"runtime_mcp_overlay"` + RuntimeConnectedApps []byte `json:"runtime_connected_apps"` + OriginatorSource pgtype.Text `json:"originator_source"` + TriggerEvidenceKind pgtype.Text `json:"trigger_evidence_kind"` + TriggerEvidenceRefID pgtype.UUID `json:"trigger_evidence_ref_id"` } // The chat sender (initiator) is a direct_human originator and accountable; @@ -202,6 +237,7 @@ func (q *Queries) CreateChatTask(ctx context.Context, arg CreateChatTaskParams) arg.Priority, arg.ChatSessionID, arg.InitiatorUserID, + arg.FireAt, arg.OriginatorUserID, arg.AccountableUserID, arg.ForceFreshSession, @@ -264,6 +300,83 @@ func (q *Queries) CreateChatTask(ctx context.Context, arg CreateChatTaskParams) return i, err } +const deferChatTaskForSealedPendingMedia = `-- name: DeferChatTaskForSealedPendingMedia :one +UPDATE agent_task_queue AS task +SET status = 'deferred', fire_at = pending.max_until +FROM ( + SELECT max(message.channel_media_pending_until) AS max_until + FROM chat_message AS message + WHERE message.task_id = $1 + AND message.role = 'user' + AND message.channel_media_pending_until > now() +) AS pending +WHERE task.id = $1 + AND pending.max_until IS NOT NULL + AND (task.fire_at IS NULL OR task.fire_at < pending.max_until) +RETURNING task.id, task.agent_id, task.issue_id, task.status, task.priority, task.dispatched_at, task.started_at, task.completed_at, task.result, task.error, task.created_at, task.context, task.runtime_id, task.session_id, task.work_dir, task.trigger_comment_id, task.chat_session_id, task.autopilot_run_id, task.attempt, task.max_attempts, task.parent_task_id, task.failure_reason, task.trigger_summary, task.force_fresh_session, task.is_leader_task, task.wait_reason, task.initiator_user_id, task.handoff_note, task.prepare_lease_expires_at, task.squad_id, task.runtime_mcp_overlay, task.escalation_for_task_id, task.fire_at, task.originator_user_id, task.runtime_connected_apps, task.coalesced_comment_ids, task.delivered_comment_ids, task.chat_input_task_id, task.chat_finalize_deferred_at, task.originator_source, task.delegated_from_task_id, task.retry_of_task_id, task.rerun_of_task_id, task.rule_version_id, task.trigger_evidence_kind, task.trigger_evidence_ref_id, task.accountable_user_id +` + +// Closes the enqueue-vs-append race: under READ COMMITTED a media message can +// commit between GetChannelMediaPendingUntil and the batch seal above, landing +// an unexpired media marker inside a task the deadline read decided was +// 'queued'. Re-derive the deferral from the sealed batch itself, in the same +// transaction, so a task is never claimable while its own input still has an +// unexpired marker. No row (ErrNoRows) means no correction was needed. +func (q *Queries) DeferChatTaskForSealedPendingMedia(ctx context.Context, taskID pgtype.UUID) (AgentTaskQueue, error) { + row := q.db.QueryRow(ctx, deferChatTaskForSealedPendingMedia, taskID) + var i AgentTaskQueue + err := row.Scan( + &i.ID, + &i.AgentID, + &i.IssueID, + &i.Status, + &i.Priority, + &i.DispatchedAt, + &i.StartedAt, + &i.CompletedAt, + &i.Result, + &i.Error, + &i.CreatedAt, + &i.Context, + &i.RuntimeID, + &i.SessionID, + &i.WorkDir, + &i.TriggerCommentID, + &i.ChatSessionID, + &i.AutopilotRunID, + &i.Attempt, + &i.MaxAttempts, + &i.ParentTaskID, + &i.FailureReason, + &i.TriggerSummary, + &i.ForceFreshSession, + &i.IsLeaderTask, + &i.WaitReason, + &i.InitiatorUserID, + &i.HandoffNote, + &i.PrepareLeaseExpiresAt, + &i.SquadID, + &i.RuntimeMcpOverlay, + &i.EscalationForTaskID, + &i.FireAt, + &i.OriginatorUserID, + &i.RuntimeConnectedApps, + &i.CoalescedCommentIds, + &i.DeliveredCommentIds, + &i.ChatInputTaskID, + &i.ChatFinalizeDeferredAt, + &i.OriginatorSource, + &i.DelegatedFromTaskID, + &i.RetryOfTaskID, + &i.RerunOfTaskID, + &i.RuleVersionID, + &i.TriggerEvidenceKind, + &i.TriggerEvidenceRefID, + &i.AccountableUserID, + ) + return i, err +} + const deleteChatDraftRestore = `-- name: DeleteChatDraftRestore :execrows DELETE FROM chat_draft_restore WHERE id = $1 AND chat_session_id = $2 @@ -357,7 +470,7 @@ func (q *Queries) DeleteChatSession(ctx context.Context, arg DeleteChatSessionPa const deleteUserChatMessageByTask = `-- name: DeleteUserChatMessageByTask :one DELETE FROM chat_message WHERE task_id = $1 AND role = 'user' -RETURNING id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind +RETURNING id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind, channel_media_pending_until, channel_ingested ` func (q *Queries) DeleteUserChatMessageByTask(ctx context.Context, taskID pgtype.UUID) (ChatMessage, error) { @@ -373,12 +486,33 @@ func (q *Queries) DeleteUserChatMessageByTask(ctx context.Context, taskID pgtype &i.FailureReason, &i.ElapsedMs, &i.MessageKind, + &i.ChannelMediaPendingUntil, + &i.ChannelIngested, ) return i, err } +const getChannelMediaPendingUntil = `-- name: GetChannelMediaPendingUntil :one +SELECT channel_media_pending_until +FROM chat_message +WHERE chat_session_id = $1 + AND role = 'user' + AND channel_media_pending_until > now() +ORDER BY channel_media_pending_until DESC +LIMIT 1 +` + +// The latest unexpired media deadline gates a channel task. Using a durable +// task fire_at means a process restart still produces the placeholder fallback. +func (q *Queries) GetChannelMediaPendingUntil(ctx context.Context, chatSessionID pgtype.UUID) (pgtype.Timestamptz, error) { + row := q.db.QueryRow(ctx, getChannelMediaPendingUntil, chatSessionID) + var channel_media_pending_until pgtype.Timestamptz + err := row.Scan(&channel_media_pending_until) + return channel_media_pending_until, err +} + const getChatMessage = `-- name: GetChatMessage :one -SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind FROM chat_message +SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind, channel_media_pending_until, channel_ingested FROM chat_message WHERE id = $1 ` @@ -395,6 +529,8 @@ func (q *Queries) GetChatMessage(ctx context.Context, id pgtype.UUID) (ChatMessa &i.FailureReason, &i.ElapsedMs, &i.MessageKind, + &i.ChannelMediaPendingUntil, + &i.ChannelIngested, ) return i, err } @@ -498,7 +634,7 @@ func (q *Queries) GetLastChatTaskSession(ctx context.Context, chatSessionID pgty } const getMostRecentUserChatMessage = `-- name: GetMostRecentUserChatMessage :one -SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind FROM chat_message +SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind, channel_media_pending_until, channel_ingested FROM chat_message WHERE chat_session_id = $1 AND role = 'user' ORDER BY created_at DESC LIMIT 1 @@ -522,6 +658,8 @@ func (q *Queries) GetMostRecentUserChatMessage(ctx context.Context, chatSessionI &i.FailureReason, &i.ElapsedMs, &i.MessageKind, + &i.ChannelMediaPendingUntil, + &i.ChannelIngested, ) return i, err } @@ -602,6 +740,34 @@ func (q *Queries) LinkChatMessageToTask(ctx context.Context, arg LinkChatMessage return err } +const linkUnownedChannelChatMessagesToTask = `-- name: LinkUnownedChannelChatMessagesToTask :exec +UPDATE chat_message AS message +SET task_id = $1 +WHERE message.chat_session_id = $2 + AND message.role = 'user' + AND message.task_id IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM chat_message AS prior + WHERE prior.chat_session_id = $2 + AND prior.role != 'user' + AND (prior.created_at, prior.id) > (message.created_at, message.id) + ) +` + +type LinkUnownedChannelChatMessagesToTaskParams struct { + TaskID pgtype.UUID `json:"task_id"` + ChatSessionID pgtype.UUID `json:"chat_session_id"` +} + +// Seals the trailing channel-message batch to its task. The task row and these +// links are committed together, so an older in-flight task cannot absorb a +// newer media message and a later assistant row cannot hide that message. +func (q *Queries) LinkUnownedChannelChatMessagesToTask(ctx context.Context, arg LinkUnownedChannelChatMessagesToTaskParams) error { + _, err := q.db.Exec(ctx, linkUnownedChannelChatMessagesToTask, arg.TaskID, arg.ChatSessionID) + return err +} + const listAllChatSessionsByCreator = `-- name: ListAllChatSessionsByCreator :many SELECT cs.id, cs.workspace_id, cs.agent_id, cs.creator_id, cs.title, cs.session_id, cs.work_dir, cs.status, cs.created_at, cs.updated_at, cs.unread_since, cs.runtime_id, cs.last_read_at, cs.is_agent_intro, cs.pinned_at, CASE WHEN cs.status = 'archived' THEN 0 @@ -739,7 +905,7 @@ func (q *Queries) ListChatDraftRestoresBySession(ctx context.Context, chatSessio } const listChatInputMessages = `-- name: ListChatInputMessages :many -SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind FROM chat_message +SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind, channel_media_pending_until, channel_ingested FROM chat_message WHERE task_id = $1 AND role = 'user' ORDER BY created_at ASC, id ASC ` @@ -770,6 +936,8 @@ func (q *Queries) ListChatInputMessages(ctx context.Context, taskID pgtype.UUID) &i.FailureReason, &i.ElapsedMs, &i.MessageKind, + &i.ChannelMediaPendingUntil, + &i.ChannelIngested, ); err != nil { return nil, err } @@ -782,9 +950,9 @@ func (q *Queries) ListChatInputMessages(ctx context.Context, taskID pgtype.UUID) } const listChatMessages = `-- name: ListChatMessages :many -SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind FROM chat_message +SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind, channel_media_pending_until, channel_ingested FROM chat_message WHERE chat_session_id = $1 -ORDER BY created_at ASC +ORDER BY created_at ASC, id ASC ` func (q *Queries) ListChatMessages(ctx context.Context, chatSessionID pgtype.UUID) ([]ChatMessage, error) { @@ -806,6 +974,8 @@ func (q *Queries) ListChatMessages(ctx context.Context, chatSessionID pgtype.UUI &i.FailureReason, &i.ElapsedMs, &i.MessageKind, + &i.ChannelMediaPendingUntil, + &i.ChannelIngested, ); err != nil { return nil, err } @@ -818,7 +988,7 @@ func (q *Queries) ListChatMessages(ctx context.Context, chatSessionID pgtype.UUI } const listChatMessagesPage = `-- name: ListChatMessagesPage :many -SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind FROM chat_message +SELECT id, chat_session_id, role, content, task_id, created_at, failure_reason, elapsed_ms, message_kind, channel_media_pending_until, channel_ingested FROM chat_message WHERE chat_session_id = $1 AND ( $3::timestamptz IS NULL @@ -859,6 +1029,8 @@ func (q *Queries) ListChatMessagesPage(ctx context.Context, arg ListChatMessages &i.FailureReason, &i.ElapsedMs, &i.MessageKind, + &i.ChannelMediaPendingUntil, + &i.ChannelIngested, ); err != nil { return nil, err } @@ -1184,6 +1356,95 @@ func (q *Queries) MarkChatSessionRead(ctx context.Context, id pgtype.UUID) error return err } +const promoteChannelChatTasksIfMediaReady = `-- name: PromoteChannelChatTasksIfMediaReady :many +UPDATE agent_task_queue AS task +SET status = 'queued', fire_at = NULL +WHERE task.chat_session_id = $1 + AND task.status = 'deferred' + AND task.issue_id IS NULL + AND task.parent_task_id IS NULL + AND task.escalation_for_task_id IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM chat_message AS message + WHERE message.chat_session_id = $1 + AND message.role = 'user' + AND message.channel_media_pending_until > now() + ) +RETURNING task.id, task.agent_id, task.issue_id, task.status, task.priority, task.dispatched_at, task.started_at, task.completed_at, task.result, task.error, task.created_at, task.context, task.runtime_id, task.session_id, task.work_dir, task.trigger_comment_id, task.chat_session_id, task.autopilot_run_id, task.attempt, task.max_attempts, task.parent_task_id, task.failure_reason, task.trigger_summary, task.force_fresh_session, task.is_leader_task, task.wait_reason, task.initiator_user_id, task.handoff_note, task.prepare_lease_expires_at, task.squad_id, task.runtime_mcp_overlay, task.escalation_for_task_id, task.fire_at, task.originator_user_id, task.runtime_connected_apps, task.coalesced_comment_ids, task.delivered_comment_ids, task.chat_input_task_id, task.chat_finalize_deferred_at, task.originator_source, task.delegated_from_task_id, task.retry_of_task_id, task.rerun_of_task_id, task.rule_version_id, task.trigger_evidence_kind, task.trigger_evidence_ref_id, task.accountable_user_id +` + +// Media completion may race with the 3s run batcher. Promote every original +// channel task waiting for this session only after all unexpired media markers +// are gone; retry/escalation/direct-chat deferred tasks are excluded. +func (q *Queries) PromoteChannelChatTasksIfMediaReady(ctx context.Context, chatSessionID pgtype.UUID) ([]AgentTaskQueue, error) { + rows, err := q.db.Query(ctx, promoteChannelChatTasksIfMediaReady, chatSessionID) + if err != nil { + return nil, err + } + defer rows.Close() + items := []AgentTaskQueue{} + for rows.Next() { + var i AgentTaskQueue + if err := rows.Scan( + &i.ID, + &i.AgentID, + &i.IssueID, + &i.Status, + &i.Priority, + &i.DispatchedAt, + &i.StartedAt, + &i.CompletedAt, + &i.Result, + &i.Error, + &i.CreatedAt, + &i.Context, + &i.RuntimeID, + &i.SessionID, + &i.WorkDir, + &i.TriggerCommentID, + &i.ChatSessionID, + &i.AutopilotRunID, + &i.Attempt, + &i.MaxAttempts, + &i.ParentTaskID, + &i.FailureReason, + &i.TriggerSummary, + &i.ForceFreshSession, + &i.IsLeaderTask, + &i.WaitReason, + &i.InitiatorUserID, + &i.HandoffNote, + &i.PrepareLeaseExpiresAt, + &i.SquadID, + &i.RuntimeMcpOverlay, + &i.EscalationForTaskID, + &i.FireAt, + &i.OriginatorUserID, + &i.RuntimeConnectedApps, + &i.CoalescedCommentIds, + &i.DeliveredCommentIds, + &i.ChatInputTaskID, + &i.ChatFinalizeDeferredAt, + &i.OriginatorSource, + &i.DelegatedFromTaskID, + &i.RetryOfTaskID, + &i.RerunOfTaskID, + &i.RuleVersionID, + &i.TriggerEvidenceKind, + &i.TriggerEvidenceRefID, + &i.AccountableUserID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const setChatSessionArchived = `-- name: SetChatSessionArchived :one UPDATE chat_session SET status = CASE WHEN $2::bool THEN 'archived' ELSE 'active' END, @@ -1274,9 +1535,10 @@ RETURNING id, agent_id, issue_id, status, priority, dispatched_at, started_at, c // Stamps a freshly-created direct-chat task as the owner of its own input batch // (chat_input_task_id = id), so a later claim loads exactly the user messages // tagged with this task id (ListChatInputMessages) rather than scanning trailing -// history. Runs in the same transaction as CreateChatTask + the user message -// insert on the direct-send path. Channel and legacy tasks skip this call and -// keep chat_input_task_id NULL, so a rolling deploy never replays their history. +// history. Runs in the same transaction as CreateChatTask + message ownership: +// direct-send inserts one owned message, while channel enqueue seals its +// trailing unowned batch. Legacy tasks keep chat_input_task_id NULL and retain +// the trailing-history fallback during rolling deploys. func (q *Queries) SetChatTaskInputOwnerSelf(ctx context.Context, id pgtype.UUID) (AgentTaskQueue, error) { row := q.db.QueryRow(ctx, setChatTaskInputOwnerSelf, id) var i AgentTaskQueue @@ -1332,6 +1594,31 @@ func (q *Queries) SetChatTaskInputOwnerSelf(ctx context.Context, id pgtype.UUID) return i, err } +const taskHasChannelIngestedMessages = `-- name: TaskHasChannelIngestedMessages :one +SELECT EXISTS ( + SELECT 1 FROM chat_message + WHERE task_id = $1 + AND role = 'user' + AND channel_ingested +) AS channel_ingested +` + +// Immutable channel provenance for a task's user-message input batch: +// channel_ingested is stamped inside the channel append transaction and never +// mutated afterwards, so it survives session archiving and installation +// rebinds that delete the channel_chat_session_binding row. Callers pass the +// batch OWNER id (chat_input_task_id, which auto-retry clones inherit), not +// necessarily the task's own id. The cancel restore-delete and the +// empty-completion silent-drop both gate on this — a channel sender has no +// Multica composer for a restored draft, and the no_response fallback body +// must never be pushed to an external channel. +func (q *Queries) TaskHasChannelIngestedMessages(ctx context.Context, taskID pgtype.UUID) (bool, error) { + row := q.db.QueryRow(ctx, taskHasChannelIngestedMessages, taskID) + var channel_ingested bool + err := row.Scan(&channel_ingested) + return channel_ingested, err +} + const touchChatSession = `-- name: TouchChatSession :exec UPDATE chat_session SET updated_at = now() WHERE id = $1 diff --git a/server/pkg/db/generated/models.go b/server/pkg/db/generated/models.go index d2d393c0b81..ec92bec4f19 100644 --- a/server/pkg/db/generated/models.go +++ b/server/pkg/db/generated/models.go @@ -351,15 +351,17 @@ type ChatDraftRestore struct { } type ChatMessage struct { - ID pgtype.UUID `json:"id"` - ChatSessionID pgtype.UUID `json:"chat_session_id"` - Role string `json:"role"` - Content string `json:"content"` - TaskID pgtype.UUID `json:"task_id"` - CreatedAt pgtype.Timestamptz `json:"created_at"` - FailureReason pgtype.Text `json:"failure_reason"` - ElapsedMs pgtype.Int8 `json:"elapsed_ms"` - MessageKind string `json:"message_kind"` + ID pgtype.UUID `json:"id"` + ChatSessionID pgtype.UUID `json:"chat_session_id"` + Role string `json:"role"` + Content string `json:"content"` + TaskID pgtype.UUID `json:"task_id"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + FailureReason pgtype.Text `json:"failure_reason"` + ElapsedMs pgtype.Int8 `json:"elapsed_ms"` + MessageKind string `json:"message_kind"` + ChannelMediaPendingUntil pgtype.Timestamptz `json:"channel_media_pending_until"` + ChannelIngested bool `json:"channel_ingested"` } type ChatPinnedAgent struct { diff --git a/server/pkg/db/queries/chat.sql b/server/pkg/db/queries/chat.sql index bb1a7d87be3..8e3adc1c23f 100644 --- a/server/pkg/db/queries/chat.sql +++ b/server/pkg/db/queries/chat.sql @@ -151,15 +151,94 @@ WHERE id = $1; -- message_kind defaults to 'message' via COALESCE so every existing caller -- (which omits it) keeps writing ordinary messages; the empty-reply path passes -- 'no_response' to mark a visible turn with no text output (MUL-4351). -INSERT INTO chat_message (chat_session_id, role, content, task_id, failure_reason, elapsed_ms, message_kind) -VALUES ($1, $2, $3, sqlc.narg(task_id), sqlc.narg(failure_reason), sqlc.narg(elapsed_ms), COALESCE(sqlc.narg(message_kind)::text, 'message')) +INSERT INTO chat_message ( + chat_session_id, role, content, task_id, failure_reason, elapsed_ms, + message_kind, channel_media_pending_until, channel_ingested +) +VALUES ( + $1, $2, $3, sqlc.narg(task_id), sqlc.narg(failure_reason), sqlc.narg(elapsed_ms), + COALESCE(sqlc.narg(message_kind)::text, 'message'), + sqlc.narg(channel_media_pending_until), + COALESCE(sqlc.narg(channel_ingested)::boolean, FALSE) +) RETURNING *; +-- name: TaskHasChannelIngestedMessages :one +-- Immutable channel provenance for a task's user-message input batch: +-- channel_ingested is stamped inside the channel append transaction and never +-- mutated afterwards, so it survives session archiving and installation +-- rebinds that delete the channel_chat_session_binding row. Callers pass the +-- batch OWNER id (chat_input_task_id, which auto-retry clones inherit), not +-- necessarily the task's own id. The cancel restore-delete and the +-- empty-completion silent-drop both gate on this — a channel sender has no +-- Multica composer for a restored draft, and the no_response fallback body +-- must never be pushed to an external channel. +SELECT EXISTS ( + SELECT 1 FROM chat_message + WHERE task_id = $1 + AND role = 'user' + AND channel_ingested +) AS channel_ingested; + +-- name: GetChannelMediaPendingUntil :one +-- The latest unexpired media deadline gates a channel task. Using a durable +-- task fire_at means a process restart still produces the placeholder fallback. +SELECT channel_media_pending_until +FROM chat_message +WHERE chat_session_id = $1 + AND role = 'user' + AND channel_media_pending_until > now() +ORDER BY channel_media_pending_until DESC +LIMIT 1; + +-- name: ClearChatMessageChannelMediaPending :exec +UPDATE chat_message +SET channel_media_pending_until = NULL +WHERE id = $1 AND chat_session_id = $2; + -- name: LinkChatMessageToTask :exec UPDATE chat_message SET task_id = $2 WHERE id = $1 AND role = 'user'; +-- name: LinkUnownedChannelChatMessagesToTask :exec +-- Seals the trailing channel-message batch to its task. The task row and these +-- links are committed together, so an older in-flight task cannot absorb a +-- newer media message and a later assistant row cannot hide that message. +UPDATE chat_message AS message +SET task_id = @task_id +WHERE message.chat_session_id = @chat_session_id + AND message.role = 'user' + AND message.task_id IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM chat_message AS prior + WHERE prior.chat_session_id = @chat_session_id + AND prior.role != 'user' + AND (prior.created_at, prior.id) > (message.created_at, message.id) + ); + +-- name: DeferChatTaskForSealedPendingMedia :one +-- Closes the enqueue-vs-append race: under READ COMMITTED a media message can +-- commit between GetChannelMediaPendingUntil and the batch seal above, landing +-- an unexpired media marker inside a task the deadline read decided was +-- 'queued'. Re-derive the deferral from the sealed batch itself, in the same +-- transaction, so a task is never claimable while its own input still has an +-- unexpired marker. No row (ErrNoRows) means no correction was needed. +UPDATE agent_task_queue AS task +SET status = 'deferred', fire_at = pending.max_until +FROM ( + SELECT max(message.channel_media_pending_until) AS max_until + FROM chat_message AS message + WHERE message.task_id = @task_id + AND message.role = 'user' + AND message.channel_media_pending_until > now() +) AS pending +WHERE task.id = @task_id + AND pending.max_until IS NOT NULL + AND (task.fire_at IS NULL OR task.fire_at < pending.max_until) +RETURNING task.*; + -- name: DeleteUserChatMessageByTask :one DELETE FROM chat_message WHERE task_id = $1 AND role = 'user' @@ -168,7 +247,7 @@ RETURNING *; -- name: ListChatMessages :many SELECT * FROM chat_message WHERE chat_session_id = $1 -ORDER BY created_at ASC; +ORDER BY created_at ASC, id ASC; -- name: ListChatInputMessages :many -- Loads the immutable user-message input batch owned by a direct-chat task. @@ -203,10 +282,13 @@ WHERE id = $1; INSERT INTO agent_task_queue ( agent_id, runtime_id, issue_id, status, priority, chat_session_id, initiator_user_id, originator_user_id, accountable_user_id, force_fresh_session, runtime_mcp_overlay, - runtime_connected_apps, originator_source, trigger_evidence_kind, trigger_evidence_ref_id + runtime_connected_apps, originator_source, trigger_evidence_kind, trigger_evidence_ref_id, + fire_at ) VALUES ( - $1, $2, NULL, 'queued', $3, $4, $5, + $1, $2, NULL, + CASE WHEN sqlc.narg('fire_at')::timestamptz IS NULL THEN 'queued' ELSE 'deferred' END, + $3, $4, $5, sqlc.narg(originator_user_id), sqlc.narg(accountable_user_id), COALESCE(sqlc.narg('force_fresh_session')::boolean, FALSE), @@ -214,17 +296,39 @@ VALUES ( sqlc.narg(runtime_connected_apps), sqlc.narg(originator_source), sqlc.narg(trigger_evidence_kind), - sqlc.narg(trigger_evidence_ref_id) + sqlc.narg(trigger_evidence_ref_id), + sqlc.narg('fire_at')::timestamptz ) RETURNING *; +-- name: PromoteChannelChatTasksIfMediaReady :many +-- Media completion may race with the 3s run batcher. Promote every original +-- channel task waiting for this session only after all unexpired media markers +-- are gone; retry/escalation/direct-chat deferred tasks are excluded. +UPDATE agent_task_queue AS task +SET status = 'queued', fire_at = NULL +WHERE task.chat_session_id = @chat_session_id + AND task.status = 'deferred' + AND task.issue_id IS NULL + AND task.parent_task_id IS NULL + AND task.escalation_for_task_id IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM chat_message AS message + WHERE message.chat_session_id = @chat_session_id + AND message.role = 'user' + AND message.channel_media_pending_until > now() + ) +RETURNING task.*; + -- name: SetChatTaskInputOwnerSelf :one -- Stamps a freshly-created direct-chat task as the owner of its own input batch -- (chat_input_task_id = id), so a later claim loads exactly the user messages -- tagged with this task id (ListChatInputMessages) rather than scanning trailing --- history. Runs in the same transaction as CreateChatTask + the user message --- insert on the direct-send path. Channel and legacy tasks skip this call and --- keep chat_input_task_id NULL, so a rolling deploy never replays their history. +-- history. Runs in the same transaction as CreateChatTask + message ownership: +-- direct-send inserts one owned message, while channel enqueue seals its +-- trailing unowned batch. Legacy tasks keep chat_input_task_id NULL and retain +-- the trailing-history fallback during rolling deploys. UPDATE agent_task_queue SET chat_input_task_id = id WHERE id = $1