11 Commits

Author	SHA1	Message	Date
beast	077ac8acc9	refactor(channel): claim media ledger rows one at a time (MUL-5367) (#5993) ... The reconciler claimed a batch of ledger rows under one lease and settled them serially, so a tail row could expire and be reclaimed before its own DELETE was ever tried — inflating attempt/backoff for work that never happened, and delaying the row's first real attempt. Rows are now claimed one at a time, immediately before the work each claim authorizes, so attempt counts attempts. The per-row lease heartbeat is gone: the lease only has to cover one row's settle (30s delete timeout << 2m lease) and every settle write is already lease-token guarded. Migration 232 adds an index on next_attempt_at: migration 230's index leads with state and cannot serve the claim's cross-state ordering, so under backlog every claim in a sweep paid a full seq scan plus an external merge sort. Shutdown that lands mid-settle now stays quiet — the row keeps its lease and is reclaimed after expiry, like any interrupted worker.	2026-07-28 13:10:34 +08:00
beast	60048172a7	fix(lark): ingest inbound images and videos as chat attachments (MUL-4934) (#5580) ... * fix: ingest feishu media as chat attachments * fix: ingest feishu post embedded media * fix(lark): make inbound media retries safe * fix lark media resource limit * fix(lark): move inbound media off ack path * fix(channel): make inbound media runs durable * fix(channel): close enqueue-vs-append race on media deferral EnqueueChatTask read the session-wide media deadline in one statement and sealed the input batch in a later one. Under READ COMMITTED a media message committing between the two got sealed into a task the deadline read had already decided was 'queued', so the daemon could claim it before its attachment bound — the agent received the bare placeholder, and the later media-ready promotion was a no-op against a non-deferred task. After the seal, re-derive the deferral from the sealed batch itself in the same transaction (DeferChatTaskForSealedPendingMedia): if any sealed message still carries an unexpired media marker, flip the task to deferred with fire_at aligned to the latest marker. The existing post-commit promote fence already covers the opposite direction (marker cleared mid-transaction). Adds a deterministic regression test that injects the media append between the deadline read and the seal via a wrapped pgx.Tx. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): keep committed chat task out of enqueue error path The post-commit media-ready fence returned its error from EnqueueChatTask even though the deferred task was already durably committed. The router flush treats any enqueue error as "no task exists": it clears the typing indicator and logs an enqueue failure while the run still happens at its fire_at deadline. Log the fence failure instead — the claim-path deferred promoter re-queues the task regardless. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): cap global media resolution concurrency Media jobs were serialized per session but unbounded across sessions: a burst could open arbitrarily many concurrent 45s Lark downloads, and each unknown-length upload may buffer up to the 100 MiB resource cap in memory. Gate resolveAndBindMedia behind a global slot semaphore (default 8, RouterConfig.MediaConcurrency). Per-session ordering is unchanged; on shutdown a job cancelled while waiting for a slot proceeds straight to the bounded DB finalize so marker clearing stays prompt. Also document that the per-message media budget spans queue/slot waits (it must match the persisted fire_at) and why timed-out uploads cannot leak unbounded orphans. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(chat): keep channel-sealed user messages on task cancel Sealing the channel input batch stamps task_id onto channel user messages, which exposed them to the cancel draft-restore path: an empty-transcript cancel would DeleteUserChatMessageByTask the sealed Feishu/Slack messages and detach their attachments. Those messages are the durable record of what the platform sender wrote — the sender has no Multica composer to restore a draft into. Gate the restore-delete on ChatSessionHasChannelBinding in both the synchronous finalize and the deferred finalize (the latter covers markers left by an older replica during a rolling deploy); a bound session now settles as "Stopped." instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): skip the media pipeline for messages without media Every inbound message on a Media-enabled platform persisted a 45s media deadline and queued a resolution job, so a plain text message could wait behind the global media semaphore (its task deferred while other sessions download 100 MiB videos) and a crash between append and clear delayed a pure-text run to the full 45s fallback. Add MediaResolver.HasMedia — a pure in-memory probe the Router calls on the ACK path — and only persist the deadline / enqueue the job when the message actually references platform media. The Feishu resolver decodes the already-received payload and reports standalone image or video keys and post-embedded img/media spans. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(chat): gate cancel restore on immutable channel provenance The previous guard keyed the cancel restore-delete off ChatSessionHasChannelBinding, but a binding only proves routing exists right now: archiving a session and rebinding an installation both delete the binding while preserving chat history, so a still-cancellable sealed task could again restore-delete the original inbound messages. Persist provenance on the message instead: migration 203 adds chat_message.channel_ingested, stamped inside the channel append transaction and never mutated, and both cancel finalize paths now gate on TaskHasChannelIngestedMessages over the task's sealed batch. The binding-existence query is removed. Regression tests cover ingest -> archive/unbind -> cancel for a queued and a started task. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): reclaim media uploads that never gain an attachment row Deadline expiry dropped already-resolved refs and a BindMedia failure was log-only, leaving uploaded objects with no attachment row and no reclaim path — the dedup mark commits with the message before media runs, so a redelivery is dropped as a duplicate and never re-resolves (and thus never overwrites) those keys, and workspace/session deletion only enumerates the attachment table. Add MediaResolver.DiscardMedia — a best-effort delete by StorageKey — and call it from both failure paths in resolveAndBindMedia. The Feishu resolver forwards to the storage backend's Delete. Tests cover a partial upload discarded at the deadline, discard on bind failure, and key-level deletion in the resolver. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(server): refresh comments stale after detached media ingestion Channel tasks now seal a self-owned input batch, media ingestion is no longer out of scope for the flattener, and MediaRefs are filled by the detached resolver after append rather than by feishuChannel pre-engine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(chat): stop keying channel empty-completion silence off chat_input_task_id Sealing gave channel tasks a self input-owner, which broke writeChatCompletionOutcome's discriminator: it treated any owned task as direct, so an empty channel completion wrote the no_response fallback row and the outbound patcher — which forwards any non-empty chat:done content verbatim — pushed the English fallback body to Feishu/Slack, violating the MUL-4351 contract. Silence is now decided by the immutable channel_ingested provenance of the task's input batch, looked up by the batch OWNER id (chat_input_task_id): auto-retry clones inherit the owner while their sealed messages stay tagged with the parent's id, so keying off the task's own id would misread a channel retry as direct. The cancel-path provenance gates switch to the same owner key via chatInputOwnerID. chat_input_task_id is back to meaning only "input batch owner". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(migrations): renumber to 203/204 after upstream took 202 Upstream main merged 202_runtime_profile_add_qwen while this branch held 202/203, tripping TestMigrationNumericPrefixesStayUniqueAfterLegacySet on the CI merge tree. channel_media_pending becomes 203 and channel_ingested becomes 204; no content changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channels): gate outbound delivery on channel provenance, not owner Merging main brought #5645 (keep direct chat replies in Multica), whose outbound gate assumed channel tasks leave chat_input_task_id NULL. Sealed channel tasks own an input batch too, so on the merge tree every channel reply and failure notice was classified as direct and silently dropped — agents stopped replying in Feishu/Slack. Both outbound gates now call engine.TaskInputIsChannelIngested: a NULL owner keeps #5645's deliver-by-default for pre-sealing tasks, an owned batch delivers only when it carries the immutable channel_ingested stamp (keyed by the owner id, so auto-retry clones inherit the verdict). Direct replies stay in Multica; sealed channel replies reach the platform. Tests cover both directions on both platforms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): discard media orphans on a fresh context, after finalize DiscardMedia shared finalizeCtx with BindMedia, so a bind that failed because the finalize deadline expired handed the storage deletes an already-dead context — the compensation silently no-opped and the orphans leaked anyway. The deadline path also ran S3 deletes before the marker clear, eating the same 5s budget the user-facing bind/promotion needed. Collect the refs from both failure paths, run bind + promotion on the finalize budget first, then delete on a fresh discard context. The bind-failure test now pins that discard receives a live context. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): compensate result-uncertain media uploads and commits The compensation protocol treated "the call returned an error" as "the side effect did not happen", which is wrong in both directions across the result-uncertain windows: - An upload error can follow a server-side write (lost response, deadline mid-write). The attempted key never reached the router, so nothing could reclaim it and dedup guarantees no re-resolve. The resolver now idempotently deletes the deterministic key on a fresh budget right at the failure site. - A commit error is not a rollback guarantee: a lost ack can report failure after Postgres durably committed the attachment rows, and the router's discard would then delete objects those rows reference. BindMediaRefs now converges the ambiguity on a fresh budget — any of the batch's URLs present proves the atomic commit landed (bind reports success); none proves the rollback (discard stays safe); a failed verification returns ErrMediaBindResultUnknown and the router keeps the uploads, preferring a rare orphan over a broken attachment. Fault-injection coverage: an upload error deletes the attempted key; a lost-ack commit keeps the bound attachment and reports success; a verified rollback stays a discardable error; the router keeps uploads on the unknown-outcome sentinel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(migrations): renumber to 207/208 after upstream took 203-206 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(channel): note DiscardMedia self-invocation and the unknown-outcome skip Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(migrations): renumber to 212/213 after upstream took 207-211 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(channel): replace inline media compensation with an intent ledger and reconciler Inline best-effort compensation cannot answer "did my side effect happen?" at the moment it needs the answer — the DELETE/PUT reordering and the empty-read-vs-in-flight-COMMIT gaps were both instances of the same two-system atomicity problem. Persist the intent instead and let an asynchronous reconciler settle it: - channel_media_pending_object (migration 214; claim index 215 as its own single-statement CONCURRENTLY migration): a state machine row ('pending' -> 'deleting') with lease, attempt, and backoff columns. - The resolver upserts the row BEFORE each PUT, state-guarded so a key the reconciler owns is never resurrected (the resource is skipped). ObjectURL is a pure function of configuration, so the row carries the attachment URL pre-upload. - BindMediaRefs deletes the batch's rows INSIDE the attachment-insert transaction: commit landed <=> intents gone, atomically, so an ambiguous COMMIT never needs adjudication. A key already claimed to 'deleting' is skipped (placeholder stays). - Nothing is ever deleted inline. The reconciler — an independent worker so storage latency cannot starve other sweepers — claims due rows ('pending' past the settle delay, or expired leases) under a fresh lease, checks for a durable attachment reference only AFTER the claim (race-free: bind can no longer succeed on the key), deletes unreferenced objects outside any transaction, and backs off failed deletes with attempt-based retry. Crash windows converge for free. - The settle delay is a fixed constant carrying NO correctness weight; invariant tests pin it at >=10x every pipeline budget. Metrics cover deletes, referenced clears, delete failures, and ledger backlog. Removed: MediaResolver.DiscardMedia, ErrMediaBindResultUnknown, the post-commit verification, and both router discard branches. Tests: intent-before-upload ordering; upload error leaves the row and deletes nothing; bind-wins vs reconciler-wins on the same key; lost-ack and rolled-back commit injections (intent cleared iff the attachment landed); reconciler three-state settle; expired-lease reclaim; delete failure backoff and retry; settle invariants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): never build or sweep the media reconciler without storage store is nil when S3 is unconfigured AND the local upload dir fails to initialize, but the reconciler was constructed unconditionally and main only gates the goroutine on the reconciler pointer — the first unreferenced ledger row (rows can pre-exist from a boot where storage worked) would nil-pointer panic a bare goroutine and take down the process. Construct the reconciler only when a storage backend exists, and guard RunOnce defensively: with no deleter it skips the sweep without claiming, so rows are not stranded in 'deleting' until lease expiry. Test covers the pre-existing-row + missing-storage boot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(migrations): renumber to 213-216 after upstream took 212 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(channel): remove the dead pre-resolved MediaRefs ingress path lark.InboundMessage.MediaRefs and the resolver's early-returns for pre-populated refs were vestiges of the pre-detached synchronous design — no producer fills them before the router anymore. Worse, the intent ledger made the path actively misleading: refs arriving without ledger rows would be silently skipped at bind (with a log blaming the reconciler), contradicting the field's "already persisted" contract. Delete the field, its channelMessageFromLark mapping, and both early-returns; channel.InboundMessage.MediaRefs is now documented as what it actually is — ResolveMedia's output channel, always empty on ingress, attachable only through a claimed ledger intent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): enforce workspace tenancy on every ledger query The intent-ledger upsert's conflict branch guarded only on state, so a cross-workspace storage_key collision could rewrite the row's workspace/message/url ownership; release and delete keyed on (storage_key, lease_token) alone. The derived key embeds the workspace UUID so none of this is reachable today — but tenancy must be enforced by the workspace column in every query, never derived from the key string (MUL-3515 rule, restated in this PR's review). The upsert now updates only within the same workspace (a cross-tenant conflict updates nothing, returns no row, and the resolver skips the upload — the fail-safe direction), and release/delete take (workspace_id, storage_key, lease_token). Tests pin that a foreign workspace can neither steal, release, nor delete a row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(migrations): build the ledger primary key via a concurrent index storage_key TEXT PRIMARY KEY created its unique index implicitly at CREATE TABLE, against the repo convention that every migration index — including a new table's unique index — is built CONCURRENTLY in its own single-statement migration (the exact three-step pattern client_usage_daily shipped in 207-209). The table now declares storage_key NOT NULL, 216 builds the unique index concurrently, and 217 attaches the primary key USING INDEX; the claim index moves to 218. ON CONFLICT (storage_key) still resolves against the constraint, and the full down/up round-trip is verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): bound each reconciler object delete with its own timeout DeleteObject ran on the worker-lifetime context and the SDK's default HTTP client has no overall request timeout, so one black-holed connection would wedge the sequential sweep loop — and with it every later batch and the backlog gauge — forever; a single-replica deployment has no other worker to reclaim the lease. Each delete now gets a 30s timeout (well under the 2min lease), and a timed-out delete takes the existing release/backoff path. Covered by a blocking-deleter test with an injectable timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): anchor the media deadline to the DB clock and bound queue waits by it Two deadline gaps from review: - The persisted marker was an application-clock timestamp compared against SQL now() everywhere it is read, so a skewed app node could shrink the fallback window and hand the agent a placeholder before the resolver's local budget ended. The append transaction now anchors a relative budget (MediaPendingSeconds) with now() + make_interval, writer and readers sharing one clock; the local resolve budget stays monotonic app-side. A DB test pins that the remaining budget measured by the DB clock equals the requested one. - enqueueMedia's waits (per-session order, global slot) only watched shutdown, so in a burst an already-expired job kept its goroutine and payload until it reached the front. Both waits now also watch the message's deadline; on expiry the job skips the resolver entirely and runs only the empty finalize (marker clear + promotion), which also unblocks the session's later messages. Covered by a queued-expiry test that finalizes while the only slot is deterministically held. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(migrations): renumber to 216-221 after upstream took 213-215 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): start the local media budget before the append transaction The DB anchors the durable fallback at insert-time now(), but the local monotonic budget started only after AppendMessage returned — so the resolver outlived the fallback by the append/commit latency, a window where the deferred task is already claimable while the resolver still runs and the agent reads a placeholder that binds moments later. Capture the local deadline before calling AppendMessage, restoring the ordering local-gives-up <= durable-fallback-fires. A slow-append test pins that the resolver's context deadline is measured from the pre-append instant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(migrations): renumber to 224-229 after upstream took 216-223 Verified against the merged tree: the numeric-prefix uniqueness test passes and the full migration set applies cleanly from scratch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): heartbeat the reconciler lease per row One claim covers up to 50 rows under a single 2-minute lease, but the batch is processed sequentially and each delete may run its full 30s timeout — a few stalled deletes could outlive the lease mid-batch, letting another replica reclaim the tail: duplicate concurrent deletes, inflated attempt/backoff on rows whose owner was alive, and skewed metrics. The lease is now renewed before EACH row's settle work, so it only ever needs to cover one row's worst case (invariant-tested: lease >= 2x the per-delete timeout). A renewal that matches no row means another worker reclaimed it after a genuine expiry — the row is skipped, leaving the new owner's state untouched. Test simulates a mid-batch reclaim and pins the skip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): dedup post media resources and make local writes atomic A rich post may reference the same image_key/file_key in several spans. The object key derives from (message, type, key), so duplicates uploaded to the SAME key twice: LocalStorage.UploadStream truncated the destination up front and removed it outright on a copy error, so a second failing attempt destroyed the object the first success had produced — leaving an attachment row pointing at nothing. A second succeeding attempt instead produced two attachment rows for one object. Collapse duplicate spans by (fetch type, platform key) before the upload loop, and write local uploads through a temp file renamed into place so a failed write can only discard its own temp file. Tests cover a duplicated span uploading once and a failed re-upload leaving the previous object intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): fence late-materializing PUTs with a tombstone schedule A DELETE cannot be ordered against a PUT the client already abandoned: the store may materialize the object after the delete completes. The reconciler cleared the ledger row right after deleting, so such an object had no row and nothing to reclaim it — which made the settle delay the de-facto correctness barrier for the PUT/DELETE race, exactly what the design says it must not be. The row is now kept as a tombstone ('tombstoned' state, migration 226's CHECK) and re-deleted on a widening schedule (15m, 1h, 6h, 24h, the pass index carried in last_error), so a late materialization is reclaimed by a later pass; only after the schedule is exhausted is the row dropped. Claim, heartbeat, lease, and tenancy predicates are unchanged — a tombstone is claimed exactly like any other due row. A separate gauge reports tombstones so they cannot be mistaken for a backlog of objects awaiting reclaim, and the header comment now states precisely what state fences (bind/commit) versus what the schedule fences (late PUTs). Tests: the reviewer's interleaving — DELETE completes, the abandoned PUT materializes right after, and the object is gone by the end of the schedule — plus a full schedule walk asserting the object is counted once and the row clears at the end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): tombstones must re-delete, not re-ask the reference question A tombstone revisit ran the same reference check as a first settle, so an attachment carrying the same URL — a re-ingested copy of the object — sent the row down the "referenced, keep it" branch: the object was kept and the row cleared, abandoning the re-delete schedule that fences the ORIGINAL object against an abandoned PUT. A tombstone has already been judged unreferenced and deleted; it exists only to re-delete whatever materializes later, so it now goes straight to the delete + schedule tail (extracted as settleDeletedObject, shared with the first settle). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel): keep the tombstone schedule position in its own column The re-delete pass index was encoded into last_error, which the failure path also writes: one failed re-delete erased the position and restarted the walk. A store failing intermittently could therefore keep a tombstone alive indefinitely — every recovery would resume at pass 1 and the row would never reach the end of the schedule to be dropped. tombstone_pass is now its own column (the table is introduced in this PR, so migration 226 carries it), advanced only by a successful delete, and the tombstone write clears the now-stale last_error. Test walks the schedule across a failed re-delete and asserts it resumes rather than restarts, and that the row still terminates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(lark): derive media object keys per chat message The object key was derived from the platform message alone, so a second ingest of the same Feishu message reused the first ingest's ledger row. That row can be a tombstone (up to ~31h while the re-delete schedule runs), and the intent upsert refuses anything that has left 'pending', so the second ingest skipped the upload and silently produced a placeholder with no attachment. A re-ingest is reachable: the inbound dedup claim is reclaimable once 60s stale and the dedup row is only vacuumed after 24h. Keying on the chat message the object will attach to keeps the two ingests independent, and nothing leaks: each one's objects are covered by its own ledger row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(storage): route both local upload paths through one atomic write UploadStream wrote through a temp file and renamed into place, but the buffered Upload path still truncated the destination up front — the destructive shape the stream path exists to avoid, one caller away from coming back. Both now share writeAtomic, which also restores the 0644 the direct write used (CreateTemp makes files 0600). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(channel): gofmt the media-pending append fields Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(storage): keep the local upload chmod best-effort The rename-into-place rewrite made a failed chmod fail the whole upload. CreateTemp's 0600 has to be widened to the 0644 the direct write used, but an upload dir on a mount that ignores chmod (SMB/NFS/FUSE) accepted the old direct write fine — turning those deployments' uploads into hard errors would be a regression for a cosmetic property. Log and continue. Tests pin 0644 on both upload paths, and that a failed buffered upload leaves no temp litter and no damage to a previous object. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(channel): never re-delete an object an attachment references The tombstone pass skipped the reference check and deleted unconditionally, so a durable attachment carrying that URL lost the only object it can read — the dangling attachment the intent ledger exists to prevent, and the opposite of the posture every other path here takes ("a reclaimable orphan beats a broken attachment"). The check now runs on every pass. A positive result on a tombstone is unreachable by design — keys are per (chat message, resource) and a bind cannot attach a key that has left 'pending' — so reaching it means an invariant broke: keep the object, clear the row, log it, and count it on a dedicated reconciler_tombstone_referenced_total counter. The test's contract is flipped to assert the referenced object survives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(storage): make the local staging file reclaimable after a crash os.CreateTemp's random suffix meant a crash between the staging write and the rename left a file nothing could name: the ledger records only the final storage key, and DeleteObject removed only the object and its sidecar. Each leftover can approach the 100 MiB resource cap and they accumulate without bound. The staging path is now derived from the object key, so DeleteObject removes it alongside the object — which makes the media reconciler reclaim it too, since the intent row is written before the upload. Opening it 0644 directly also drops the chmod the previous commit had to make best-effort. Both read paths refuse the staging name (keys come from the request URL, and a half-written body should not be readable); a user-supplied ".tmp" extension is unaffected, since object keys are generated and never dot-prefixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(channel): renumber the media migrations after merging main main took 224 (agent_task_session_rollout_missing), so the ledger group moves to 225-230 and the cross-references inside the table migration follow. main's CompleteTask also grew a sessionRolloutMissing parameter; the three call sites this PR added to chat_input_ownership_test.go pass false. Verified the way the numbering is meant to be verified: full migration set applied from scratch on the merged tree, and the whole server suite run against that database. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(channel): let Postgres compute every reconciler deadline The reconciler built settle cutoffs, lease expiry, backoff and re-delete times from the process clock and compared them against the database's now(). A replica whose clock had drifted would therefore settle rows whose upload was still in flight (the object is deleted and the bind then refuses to attach — media silently lost), hand out leases that are born expired (rows churn between workers, attempt/backoff inflate), or compress the tombstone schedule that fences a late-materializing PUT. The four settle queries now take durations and derive their timestamps from now(), so every replica reads one clock. The parameter types are the guard: an app-side timestamp can no longer be passed. Test asserts the persisted lease, backoff and re-delete deadlines all track the database's now(). The generated code also picks up main's new agent_task_queue column in the two RETURNING task.* queries this PR adds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(lark): drop unrelated gofmt-only churn from this PR Six files carried whitespace/comment-reformatting with no functional change, unrelated to the inbound media pipeline. Reverting them to the base revision keeps the diff focused on the feature (75 -> 69 files): server/internal/service/empty_claim_cache.go server/internal/integrations/lark/markdown_detect.go server/internal/integrations/lark/ws_chunk_assembler.go server/internal/integrations/lark/ws_chunk_assembler_test.go server/internal/integrations/lark/ws_frame_test.go server/internal/integrations/lark/registration_test.go Verified: `git diff -w` against these files was already empty, so no behavior is affected. go vet clean; tests covering these files pass. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-07-27 15:44:39 +08:00
beast	4217de4389	fix(lark): tolerate binding token clock skew (#5191) ... * fix(lark): tolerate binding token clock skew Clamp binding-token expiry against the database clock while preserving the 15-minute TTL cap. Return the persisted expiry so binding cards reflect the value enforced by Postgres. * docs(lark): correct stale table name in binding token TTL comments Post-#124 the table is channel_binding_token (with the channel_binding_token_ttl_cap CHECK); update the two comments in types.go and binding_token_test.go that still named the pre-generalization lark_binding_token table. --------- Co-authored-by: Bohan-J <bohan.optimism@gmail.com>	2026-07-10 14:47:54 +08:00
Bohan Jiang	756e7e39b3	fix(chat): prune orphaned outbound card messages on chat-session delete (#4810) (#5152) ... The standalone chat-session delete path pruned channel_chat_session_binding but not channel_outbound_card_message. Both are keyed by chat_session_id with no FK (MUL-3515 §4) and no reaper, so deleting a chat session left the card rows as permanent orphans — the same no-FK-orphan class as the #4810 installation fix, which already covers the workspace-delete / runtime-teardown / reclaim paths. Add DeleteChannelOutboundCardMessagesBySession and call it in the same tx as the binding prune; extend the delete-chat-session test to assert both are swept. Follow-up nit from the #5103 review (Elon). MUL-3937 Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-07-09 15:20:49 +08:00
Bohan Jiang	ccacce60a1	fix(channels): auto-reclaim orphaned IM-bot installations + accurate rebind conflict copy (#4810) MUL-3937 (#5103) ... * fix(channels): auto-reclaim orphaned bot installations + accurate rebind conflict copy channel_installation has no FK to workspace/agent (MUL-3515 §4), so deleting a workspace or hard-deleting an agent left the row behind, occupying the (channel_type, app_id) routing slot forever — the bot could never be rebound and the UI had no way to clear it (#4810). The 409 also always blamed "a different Multica workspace" even when the real owner sat in the same workspace. Auto-reclaim on delete: - DeleteWorkspace and the runtime-teardown paths now sweep the workspace's / archived agents' channel installations and every dependent row in-tx. - The shared install path (Feishu + Slack) reclaims a DEAD prior owner — a revoked placeholder or an orphan whose workspace/agent is gone — before the upsert, healing installations stranded before this fix. A live owner (active agent, including an archived one) is left in place, not stolen. Accurate conflict copy: - A rebind refused by a LIVE owner now distinguishes same-workspace / another agent, an archived agent, and a genuinely different workspace, for both Slack (typed sentinels) and Feishu (registration message). MUL-3937 Co-authored-by: multica-agent <github@multica.ai> * fix(channels): reclaim cross-workspace revoked bots + sweep card/dedup/audit (#4810) Address the #5103 review (yyclaw + Steve): - Reclaim: a REVOKED installation in ANY workspace is now dead (except the caller's own row), not just same-workspace. Disconnect never hard-deletes the row and there is no release UI, so a cross-workspace revoked row would pin a bot's app_id slot forever, with the misleading "connected to another workspace" copy resurfacing. A new binder proves control by holding the app credentials, so reclaiming is safe. Live ACTIVE owners (incl. archived) are still refused. - Sweep the two dependent tables the cleanups missed, in all three paths (reclaim / DeleteWorkspace / runtime teardown): channel_outbound_card_message (no reaper, so a permanent orphan otherwise) and channel_inbound_message_dedup (PurgeChannelInboundDedup has no caller). - Audit rows: PURGE on the hard-delete paths instead of detaching them into permanently unattributable NULL rows; keep DETACH on reclaim, where the workspace survives and the row stays useful for triage. - Tests: flip cross-ws revoked to reclaimed + add cross-ws active preserved; extend the reclaim and both delete-path cleanup tests for card/dedup and the audit purge/detach split; assert the channel sweep on the DeleteRuntimeProfile entry point. MUL-3937 Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-07-09 15:07:11 +08:00
Bohan Jiang	33bd8aeaa9	MUL-4134: fix(lark): preserve same-agent bindings when reconnecting a revoked Feishu bot (#4997) ... * fix(lark): allow rebinding a revoked Feishu bot to a different agent When a Feishu/Lark Bot is disconnected from agent A (status → revoked), the row is preserved for audit but still holds the (channel_type, config->>app_id) unique index slot. Binding the same Bot to agent B would fail with: duplicate key value violates unique constraint "idx_channel_installation_type_appid" (SQLSTATE 23505) because UpsertChannelInstallation conflicts on (workspace_id, agent_id, channel_type) — a different agent_id means no conflict match, so it tries INSERT and hits the app_id unique index. Fix: before the upsert, inside the same transaction, hard-delete any revoked installation with the same app_id in the same workspace. The delete is fenced to status=revoked so an active installation can never be silently removed. If no revoked row exists the delete is a no-op (deletes zero rows, returns nil error) and the upsert proceeds normally. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * fix(lark): preserve same-agent bindings when reconnecting a revoked Feishu bot The cleanup added in the previous commit hard-deletes every revoked channel_installation sharing the app_id in the workspace before the upsert — including the row belonging to the agent currently being (re)installed. That regresses the common "disconnect then reconnect the same bot to the same agent" flow: disconnect only flips status to 'revoked' (bindings are preserved), and UpsertChannelInstallation conflicts on (workspace_id, agent_id, channel_type), so before this the same agent's row was reactivated in place — installation_id and every channel_user_binding / channel_chat_session_binding kept. Deleting it first forces an INSERT with a fresh installation_id, orphaning every member's account link (they must re-link) and all chat-session continuity; only the installer is re-bound. Fence the delete with `agent_id <> $agent_id` so it only clears a DIFFERENT agent's revoked row (the genuine app_id-slot blocker). The same agent's revoked row is left for the upsert to reactivate losslessly. Since idx_channel_installation_type_appid is globally unique on (channel_type, app_id), at most one row ever holds a given app_id, so the excluded row is exactly the one the upsert will reuse. Adds DB-backed regression tests: same-agent revoked row preserved, different-agent revoked row deleted, active row never deleted, other workspace fenced, plus end-to-end reactivation semantics (same agent keeps installation_id + bindings; different agent gets a fresh id). Co-authored-by: multica-agent <github@multica.ai> * fix(lark): clean dependent rows when hard-deleting a rebound Feishu installation Addresses review on #4997 (MUL-4134). channel_* has no FK/cascade (MUL-3515 §4), so hard-deleting a different-agent revoked installation left application-owned rows dangling at a removed installation_id: - channel_chat_session_binding: the outbound patcher would resolve a binding, then fail loading the deleted installation — turning a clean no-op into error logs. - channel_binding_token: a still-unexpired bind link (15 min TTL) could be redeemed into the deleted installation, reporting "bound" against a bot that no longer reaches the user. - channel_inbound_audit: dangling installation_id, where migration 124 models the old ON DELETE SET NULL as an app-layer NULL. - channel_user_binding: dead member links (a different agent is a distinct connection; links do not follow and can never be reused). Rework RemoveRevokedInstallationByAppID to resolve the single row holding the app_id and act only when it is revoked, in this workspace, and owned by another agent; then, on the caller's transaction, clear chat-session bindings, pending binding tokens and member links, NULL the audit references, and finally delete the row via the fenced query (defense in depth). Same-agent reconnect and active/other-workspace rows are no-ops. Adds DeleteChannelUserBindingsByInstallation, DeleteChannelBindingTokensByInstallation, and NullChannelInboundAuditInstallationID queries, plus a DB-backed test (TestChannelStore_RebindCleansDependentRows) asserting every dependent is cleaned and the audit row survives detached. Verified the test fails when the cleanup is skipped. Co-authored-by: multica-agent <github@multica.ai> * fix(lark): make the rebind cleanup race-safe with a guarded delete gate Addresses the concurrency must-fix on #4997 (MUL-4134). The prior shape read the candidate installation, checked revoked/workspace/agent in Go, cleaned the dependent rows, then ran the fenced delete. That read-then- clean-then-delete order has a TOCTOU: while B is rebinding the bot to a different agent, A can reconnect to the SAME agent and reactivate the row to 'active' in between. B still wipes A's user/chat/token bindings and NULLs its audit based on the stale "it was revoked" read, then the fenced delete no-ops (status is no longer revoked) — so A's installation survives active but its bindings are gone. Concurrent same-agent data loss, reintroduced. Make the guarded DELETE the atomic gate. DeleteChannelInstallationByAppID becomes DeleteRevokedChannelInstallationByAppID `:one ... RETURNING id`, and RemoveRevokedInstallationByAppID keys all dependent cleanup off the id the delete actually claimed. No separate read. Under READ COMMITTED a concurrent reactivation makes the DELETE re-check status='revoked' against the live row (EvalPlanQual): it claims nothing, returns pgx.ErrNoRows, and no dependents are touched. With no FK the cleanup can follow the claiming delete in the same transaction; any failure rolls the whole thing back. Adds TestChannelStore_RebindGuardedDeleteRaceWithReactivation: two real transactions race on one revoked installation — one reactivates and holds the row lock, the other runs the rebind cleanup and blocks on the guarded delete — asserting the installation and every binding stay intact. Verified this test fails on the old read-then-clean-then-delete shape and passes (also under -race) on the gated version. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: jiangliangyou <jiangliangyou@xiaomi.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> Co-authored-by: J <j@multica.ai>	2026-07-07 15:07:02 +08:00
Bohan Jiang	247e8ed5ab	feat(slack): reuse account link across apps in one Slack workspace (#4786) ... A Slack user who linked their Multica identity to one bot was re-prompted to link again when messaging a second bot (a different Slack app) in the SAME Slack workspace, because bindings are keyed per-installation (installation_id, channel_user_id). On an unbound inbound, the identity resolver now looks up an existing binding for the same (Multica workspace, Slack team, Slack user) via the new FindReusableChannelUserBinding query and, if that user is still a workspace member, materializes a binding for the new installation instead of returning ErrSenderUnbound — so the second bot resolves the user silently. Reuse is fenced to one Multica workspace AND one Slack team, so it never crosses either boundary; legacy installs with no recorded team never reuse. Adds resolver unit tests for every decision path and updates the Slack integration docs (en/zh/ja/ko). MUL-3911 Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-07-01 16:58:23 +08:00
Bohan Jiang	11a3cf206b	feat(slack): bring-your-own-app install + per-installation Socket Mode (MUL-3666) (#4566) ... * feat(slack): single app-level Socket Mode connection routed by team_id (MUL-3666) Reshape the Slack adapter from the stage-3 per-installation Socket Mode model into the multi-tenant B2 connection model: ONE deployment-level Socket Mode connection (app-level xapp- token, env MULTICA_SLACK_APP_TOKEN) receives the Events API stream for every installed workspace and routes each inbound event to its channel_installation by team_id — the existing GetChannelInstallationByAppID routing, unchanged. - AppConnector: the single shared connection (slack/app_connector.go). No leader election — per the design "one (or a few)" connections are fine: each replica opens one, Slack delivers each event to one of them, and the existing (installation, message_id) two-phase dedup guarantees exactly-once processing. Resolves the per-team bot user id (via the same app_id query) to detect/strip @-mentions, since one connection serves many workspaces. - Inbound translation (Events API -> channel.InboundMessage) extracted to slack/inbound.go as free functions parameterized by the per-team bot identity. - channel.go trimmed to the outbound Send-only sender; per-installation config (config.go) no longer carries an app-level token — installs hold only the per-workspace bot token (xoxb-) for outbound, since xapp- can't be OAuth'd. - engine.Supervisor now skips channel types with no registered Factory, so Slack installs (driven by the app-level connector, not per-installation channels) no longer churn the lease/Build loop. - Wiring: router.go builds the connector when MULTICA_SLACK_APP_TOKEN is set; main.go runs it alongside the Supervisor. Feishu untouched; channel_* schema unchanged. Verified: go build ./..., go vet ./..., gofmt, and go test ./internal/integrations/... all pass. Co-authored-by: multica-agent <github@multica.ai> * feat(slack): OAuth self-serve install backend (MUL-3666) Add the in-product OAuth install flow that creates Slack installations, the keystone the B2 connector consumes. - slack.InstallService: Begin (build authorize URL, seal workspace/agent/ initiator into the OAuth state), Complete (verify state, exchange code via oauth.v2.access, upsert channel_type='slack' install with the bot token encrypted at rest, auto-bind the installer's Slack id so their first message is not dropped), plus List/Get/Revoke. State is stateless: sealed with the deployment secretbox + an embedded expiry, no session store. - HTTP handlers (handler/slack.go): member-visible list, admin-only begin + revoke, and the public OAuth callback (recovers context from the sealed state, redirects the browser back to Settings → Integrations with a result flag). - Routes + wiring: workspace-scoped list/begin/revoke mirror the Lark admin/member split; the callback is a public route like GitHub's. Built from MULTICA_SLACK_CLIENT_ID/SECRET (+ redirect derived from MULTICA_PUBLIC_URL, override MULTICA_SLACK_REDIRECT_URL; scopes via MULTICA_SLACK_SCOPES). - Realtime: slack_installation:created / :revoked events. Verified: go build ./..., go vet, gofmt, and go test ./internal/integrations/slack/... all pass (new install_test.go covers state sign/verify/expiry/tamper, authorize URL, code exchange + encrypted upsert + installer bind, and oauth error paths). Co-authored-by: multica-agent <github@multica.ai> * feat(slack): in-product OAuth install UI for web + desktop (MUL-3666) Add the "Connect Slack" self-serve install UI mirroring the Feishu/Lark integration, completing the in-product install half of B2. Slack's OAuth flow is a redirect (not a device-code QR poll), so the UI is simpler than Lark's. - core: SlackInstallation / List / Begin types; api.listSlackInstallations / beginSlackInstall / deleteSlackInstallation; slackKeys + slackInstallationsOptions query; realtime invalidation on slack_installation:* events. - views: slack-tab.tsx (SlackTab settings panel + per-agent SlackAgentBindButton + connected badge + disconnect confirm). Connect calls beginSlackInstall and hands the authorize URL to openExternal (system browser on desktop, new tab on web); Slack bounces to the backend callback which lands the install, and the realtime event refreshes the list. Wired into the Settings → Integrations tab and the agent-detail Integrations tab alongside Lark. - i18n: en + zh-Hans settings.slack.* strings. Verified: pnpm typecheck (full monorepo, 6/6) and pnpm lint (@multica/core, @multica/views — 0 errors) pass. Co-authored-by: multica-agent <github@multica.ai> * feat(slack): outbound Replier + user-binding redeem flow (MUL-3666) Fill the stage-3 Replier=nil tail so non-installer Slack users can onboard and get status feedback — completing B2 end to end. - slack.OutboundReplier (engine.OutboundReplier): on NeedsBinding it mints a single-use binding token and DMs/replies a "link your account" prompt with the redeem URL (wrapped as <url|label> so formatMrkdwn doesn't mangle the base64url token); on AgentOffline/AgentArchived it posts a status notice; on an /issue-created Ingest it confirms the new issue. Plain chat stays silent (the agent's own reply lands via EventChatDone). Reuses the bot-token Send path and reads the installation row from ResolvedInstallation.Platform — no new transport. - slack.BindingTokenService: Mint + transactional RedeemAndBind over the generic channel_binding_token / channel_user_binding queries (channel_type='slack'), mirroring lark.BindingTokenService. 15-min TTL, SHA256-hashed tokens, the three typed failure modes (invalid/expired, already-assigned, not-member). - HTTP: POST /api/slack/binding/redeem (public, session-authed) maps the failures to 410/409/403. NewSlackResolverSet now takes the replier (nil disables it). - Frontend: /slack/bind redeem page (packages/views/slack + apps/web route) + api.redeemSlackBindingToken + en/zh slack_bind copy. Verified: go build ./..., go vet, gofmt, go test ./internal/integrations/... (new replier_test.go covers all outcome branches + the prompt URL), plus full pnpm typecheck (6/6) and pnpm lint (0 errors). Co-authored-by: multica-agent <github@multica.ai> * fix(slack): address review must-fixes — connector leak, team-keyed install, /issue copy (MUL-3666) Three fixes from Niko's review: 1. AppConnector.connectOnce leaked the Socket Mode goroutine/connection on a handler error: it ran sm.RunContext on the long-lived ctx and returned the error without cancelling it, so a transient DB/router error left the old connection alive (consuming events into an unread channel) while Run opened a second one. Each connection now runs under its own cancellable context and a deferred cancel + join tears it down on every exit path before reconnect. 2. Slack re-install collided with the (channel_type, app_id) unique index: connecting the same Slack team to a different agent failed because the upsert conflict key was (workspace_id, agent_id, channel_type). Add a team-keyed UpsertChannelInstallationByAppID (ON CONFLICT on the (channel_type, app_id) index, updating agent_id) and use it for the Slack OAuth install, so re-connecting a workspace moves the bot to the chosen agent instead of erroring. Feishu's per-agent upsert is unchanged. 3. /issue clarified: it is not a registered Slack slash command (no `commands` scope), so Slack never routes one to us. Issue creation runs through the message path — `@bot /issue <title>` in a channel or `/issue <title>` in a DM — which the engine parser handles. Documented in the connector and the user-facing copy (en + zh). Verified: go build ./..., go vet, gofmt, go test ./internal/integrations/..., make sqlc, plus pnpm typecheck (6/6) and pnpm lint (0 errors). Co-authored-by: multica-agent <github@multica.ai> * fix(slack): make OAuth install transactional — agent-move binding consistency + cross-workspace guard (MUL-3666) Address Elon's review: the team-keyed upsert kept the same installation row and only flipped agent_id, but engine session reuse matches purely on (installation_id, channel_chat_id) and each chat_session is permanently tied to the agent it was created under — so after moving a Slack team from Agent A to Agent B, existing DMs/threads kept routing to Agent A; only brand-new channels/threads reached B. Cross-workspace re-install was worse: the SQL also moved workspace_id while the application-layer user/chat-session bindings stayed behind, inheriting the previous workspace's relations. InstallService.Complete now runs one transaction (lookup → upsert → retire → installer-bind), all application-layer per the no-FK rule: - Look up the existing installation by team_id (config->>'app_id'). - Reject a silent cross-workspace ownership change (ErrTeamOwnedByAnotherWorkspace → callback redirects with slack_error=team_in_other_workspace). The owning workspace must disconnect first. - On an agent change within the same workspace, retire the installation's chat-session bindings (new DeleteChannelChatSessionBindingsByInstallation) so the next message creates a fresh session under the new agent. The chat_session rows are preserved for history; user bindings stay valid (same users/workspace). - Installer auto-bind moves into the tx; an already-bound-elsewhere id is a benign skip, a real DB error aborts the whole install. InstallService now takes a TxStarter; the queries seam gains WithTx (dbInstallQueries adapter) so Complete stays unit-testable with a fake tx. Verified: make sqlc, go build ./..., go vet, gofmt, go test ./internal/integrations/... (new tests: agent-move retire, same-agent no-retire, cross-workspace reject, fresh-install no-retire). Co-authored-by: multica-agent <github@multica.ai> * fix(slack): atomic cross-workspace install guard + green up frontend CI (MUL-3666) Two things: address Elon's review and fix the failing frontend CI job. Review (atomic cross-workspace guard): the previous guard was a SELECT before the upsert, which loses the concurrent-OAuth race — two workspaces can both read no rows, one inserts, the other's ON CONFLICT update then silently re-points the team. Move the guard into the upsert itself: ON CONFLICT ... DO UPDATE ... WHERE channel_installation.workspace_id = EXCLUDED.workspace_id, and map the empty RETURNING (pgx.ErrNoRows) to ErrTeamOwnedByAnotherWorkspace. The pre-SELECT now only feeds the agent-change cleanup. Also corrected the error copy: a team stays bound to its first Multica workspace (revoke is soft, keeping the row + unique index), so migration is an operator action, not "disconnect first". CI (frontend vitest, @multica/views#test): - The agent IntegrationsTab now renders the real SlackAgentBindButton, whose connected badge calls useQueryClient — absent from integrations-tab.test.tsx's react-query mock. Hoisted the owner/admin gate above the per-platform sections (one role notice instead of one per platform), made the agents members_note generic (en/zh/ja/ko), and updated the test (mock @multica/core/slack, stub SlackAgentBindButton, assert both platforms). - Added slack-tab.test.tsx covering the real SlackAgentBindButton / SlackTab. - locale parity: added the slack (settings) + slack_bind (common) blocks to ja and ko so every EN key has a translated counterpart. Verified: make sqlc, go build ./..., go vet, gofmt, go test ./internal/integrations/...; pnpm --filter @multica/views test (1478 pass), pnpm typecheck (6/6), pnpm lint (0 errors). Co-authored-by: multica-agent <github@multica.ai> * fix(slack): surface agent-page Slack entry points when Lark is off (MUL-3666) The agent-detail Integrations tab and the inspector's Integrations section only considered Lark, so a Slack-only deployment (Lark disabled) showed neither the Integrations tab nor a Connect-Slack button — the per-agent entry points were unreachable. - agent-overview-pane: gate the Integrations tab on Lark OR Slack configured (new slackInstallationsOptions query), not Lark alone. - agent-detail-inspector: render SlackAgentBindButton alongside LarkAgentBindButton in the Integrations section. - regression test: the Integrations tab appears when only Slack is configured. Verified: pnpm typecheck (6/6), pnpm --filter @multica/views test (1478+ pass), pnpm lint (0 errors). Co-authored-by: multica-agent <github@multica.ai> * feat(slack): BYO-app install backend — paste xoxb+xapp, per-app install keyed by real app id (MUL-3666) Adds the bring-your-own-app install path so multiple agents can each have their own bot identity in the SAME Slack workspace (hosted B2 caps at one agent/workspace). User pastes their app's bot token (xoxb-) + app-level token (xapp-); we validate the bot token via auth.test, parse the real Slack app id from the xapp- token, encrypt both tokens, and persist a per-app installation keyed by that app id (real 'A…' ids never collide with hosted 'T…' team ids in the existing unique index — no schema change). - config.go: add app_token_encrypted (BYO discriminator + per-app socket token) - install.go: extract shared persistInstall (atomic cross-ws guard + agent-move retire) - byo_install.go: RegisterBYO + auth.test + app-id parse - handler + route: POST /api/workspaces/{id}/slack/install/byo (admin-only) - tests: keying, encryption, invalid tokens, auth.test failure, cross-ws, agent move Follow-ups (separate commits): per-app Socket Mode connector that consumes the stored app token; in-product BYO install dialog (video + paste form). Co-authored-by: multica-agent <github@multica.ai> * refactor(slack): drop OAuth, unify on BYO per-installation model (MUL-3666) Per product decision, Slack drops the hosted-app OAuth path entirely and unifies on bring-your-own-app (BYO): every installation carries its OWN app-level token and gets its OWN Socket Mode connection, so multiple agents can each have a distinct bot identity in one Slack workspace. - Remove OAuth install (Begin/Complete/code-exchange/sealed state/OAuthConfig/ default scopes), the OAuth callback + begin handlers + routes, and the MULTICA_SLACK_CLIENT_ID/SECRET/REDIRECT/APP_TOKEN env wiring. - Replace the single deployment-level AppConnector with a per-installation slackChannel (authenticated with its own xapp- token) registered as a channel Factory, so the engine Supervisor drives one Socket Mode connection per installation (exactly like Feishu). inbound/outbound/resolvers reused as-is. - Route inbound by the event's api_app_id (== the installation's real app id), not team_id. - InstallService slims to at-rest encryption + the shared persistInstall + list/get/revoke; install is the BYO paste path only (byo_install.go). - Tests: drop the OAuth tests; slack + handler + engine all green. Follow-up (frontend): replace the OAuth "Connect Slack" button with the BYO paste dialog (the begin endpoint it calls is now gone). Co-authored-by: multica-agent <github@multica.ai> * fix(slack): verify BYO bot + app tokens are from the same app, and the app token is live (MUL-3666) Niko review: RegisterBYO only parsed the app id from the xapp string and auth.test'd the bot token, so pasting app A's bot token with app B's app token would 'connect' but be broken (inbound on B's socket, outbound with A's identity). Now: resolve the bot's owning app id via bots.info (on the bot_id from auth.test) and require it to equal the xapp's app id; and live- validate the app token via apps.connections.open. Reject (no persist) on mismatch or a dead app token. Co-authored-by: multica-agent <github@multica.ai> * feat(slack): in-product BYO install dialog (paste bot + app tokens) (MUL-3666) The OAuth begin endpoint was removed server-side, so the "Connect Slack" button now opens a dialog where the admin pastes the bot token (xoxb-) and app-level token (xapp-) of the Slack app they created, and submits to the BYO install endpoint. Includes an optional setup-video link (URL constant, left empty until the walkthrough is recorded). - core: drop beginSlackInstall / BeginSlackInstallResponse; add registerSlackBYO + RegisterSlackBYORequest. - views: SlackAgentBindButton opens the BYO dialog; refreshed comments and install_supported docs (now means "configured", no OAuth). - i18n: new slack.byo_* keys + refreshed page_description in en/zh-Hans/ja/ko. - tests: dialog submit path; views vitest (1479), typecheck, lint, locale parity all green. Co-authored-by: multica-agent <github@multica.ai> * fix(slack): Elon review — team_id routing guard, per-agent reconnect, users:read hint (MUL-3666) 1. Inbound routing keys on api_app_id (the APP, not the Slack workspace), so additionally require the event's team_id to match the installation's stored team. A distributed BYO app installed into another Slack workspace emits the same app id and would otherwise mis-route to this Multica installation. Extracted installationServesTeam() + unit test. 2. BYO install is now agent-keyed (UpsertChannelInstallation, conflict on workspace_id+agent_id+channel_type): one bot per agent. Disconnect → reconnect a NEW app for the SAME agent now UPDATES that agent's row in place instead of violating the (workspace, agent, channel) unique. A unique violation on the (channel_type, app_id) routing index → ErrTeamOwnedByAnother- Workspace (the app is already connected to another agent/workspace). No chat-session retire is needed: a row's agent_id never changes. 3. UX: bots.info (the same-app check) needs the users:read scope — the connect dialog now lists the required bot scopes including it, and the error text says so. Backend build/vet/gofmt/test + views vitest + typecheck + locale parity green. Co-authored-by: multica-agent <github@multica.ai> * fix(slack): publish slack_installation:created on BYO connect; refresh stale comments (MUL-3666) Niko final review: RegisterSlackBYO wrote the response but never published EventSlackInstallationCreated, so only the installer's own tab refreshed — other open clients (Settings, Agent Integrations, other tabs) did not see the new bot in realtime, inconsistent with the revoke event and Lark. Now publishes it on success via a small publishSlackInstallationCreated helper, with a unit test (Bus.Publish is synchronous). Also refreshed comments that still described the removed hosted-OAuth / single deployment-level AppConnector model (handler SlackInstall field, channel.go / inbound.go / outbound.go / byo_install.go). PR title updated separately to the BYO per-installation Socket Mode model. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-06-29 14:09:34 +08:00
Bohan Jiang	cb6616f530	feat(slack): Socket Mode channel.Channel adapter (MUL-3516) (#4523) ... * feat(slack): Socket Mode channel.Channel adapter (MUL-3516) First slice of the Slack adapter: implements channel.Channel (Type/Connect/Disconnect/Send/Capabilities) over Slack Socket Mode, normalizes inbound events to channel.InboundMessage (DM, channel @mention, thread reply; bot-loop + edit/delete guards), decodes the per-installation config/secret blob, and registers the Factory under TypeSlack. No engine, core, or channel_* schema change. Unit-tested (translation, capabilities, config decode, chunking, Send via httptest). Resolvers + engine wiring + Block Kit binding replier follow. Co-authored-by: multica-agent <github@multica.ai> * fix(slack): address adapter review (MUL-3516) - Propagate InboundHandler errors through dispatchEventsAPI/handleSocketEvent to Connect so an infra failure tears down the connection for Supervisor reconnect/backoff instead of being silently swallowed (ACK still happens first). - Capabilities: declare only CapText | CapThreadReply; drop CapRichCard/CapAttachment/CapMessageEdit until those Send paths are wired. - slackChatType: map mpim (multi-party DM) to group, not p2p, so the 'must address bot' filter applies; only 1:1 im is p2p. - Document the group-addressing decision: explicit @bot mention required in groups; mention-free thread continuation deferred to the session-aware layer. - Tests: handler-error propagation, slackChatType table, mpim-requires-mention, capabilities negative assertions. Co-authored-by: multica-agent <github@multica.ai> * refactor(channel): shared channel-agnostic ChatSession service (MUL-3516) Extract the session/append//issue machinery — currently locked inside the Feishu-pinned lark.chatSessionService — into a shared engine.ChatSession parameterized by channel_type + session titles, so every IM adapter reuses it instead of re-implementing it. Logic is verbatim (find-or-create session+binding with unique-violation race re-read; append+touch+reply-target+in-tx dedup Mark; /issue parse with bare-command previous-message fallback) but channel-neutral: command-parse source is supplied by the adapter (enrichment is platform-specific). Backed by a narrow SessionQueries interface so it is unit-tested with an in-memory fake (no DB). /issue parser moved to engine.ParseIssueCommand. Next: migrate Feishu onto it and wire Slack's ResolverSet, removing the lark duplicate. Co-authored-by: multica-agent <github@multica.ai> * fix(channel): decouple session binding key from outbound target (MUL-3516) Addresses Elon's round-2 review. engine.ChatSession.EnsureSession previously keyed the binding on a raw chat id (EnsureSessionInput.ChatID), so a resolver wiring Slack straight through would collapse every @bot thread in one channel into a single chat_session and overwrite last_thread_id. Make the API un-misusable: - EnsureSessionInput.ChatID -> BindingKey: the explicit session-isolation key (Feishu: chat id; Slack DM: channel id; Slack channel: channel id + thread root), documented so a raw threaded-platform chat id is never passed straight through. - Add EnsureSessionInput.BindingConfig (opaque) persisted on the binding's config column, so the real outbound channel/thread is preserved when BindingKey is composite — outbound routing stays separate from the isolation key. - channel.sql CreateChannelChatSessionBinding now writes config (additive, uses the existing NOT NULL column; lark caller passes '{}', no schema change, no Feishu regression). - Tests: TestEnsureSession_ThreadRootIsolation (two thread roots in one channel -> two sessions; same root reuses) and TestEnsureSession_StoresBindingConfig. No production wiring change yet (per review, the not-yet-wired shared service is an accepted preparatory state); this makes the API correct before Feishu/Slack are migrated onto it. Co-authored-by: multica-agent <github@multica.ai> * feat(slack): Slack ResolverSet with thread-root session isolation (MUL-3516) Wires Slack into the channel-agnostic engine.Router via a ResolverSet built on the generic channel_* queries (installation route by team_id, identity + workspace-membership recheck, two-phase dedup, audit) plus the shared engine.ChatSession. No new query, no schema change. slackSessionRouting is the per-message isolation rule (Elon round-2 / Niko round-3): a DM is one session per channel; a channel/group message is isolated by thread root (key = channel:threadRoot, root = inbound thread_ts or the message ts for a top-level @mention), so two @bot threads in one channel are two sessions. The real channel id rides in BindingConfig for outbound; the reply thread is returned separately. Tests cover DM/channel/thread routing, config, and that distinct thread roots isolate while a same-thread follow-up reuses its key. Not yet wired into router.go (still a preparatory commit, per review); Feishu migration onto the shared service, router/config wiring, and the Slack outbound path follow. Co-authored-by: multica-agent <github@multica.ai> * feat(slack): Markdown->mrkdwn outbound formatting (MUL-3516) Slack renders mrkdwn, not Markdown, so an unconverted agent reply shows literal ** , ## and [text](url). Add formatMrkdwn — a faithful Go port of Hermes Agent's slack format_message (MIT) — and apply it in slackChannel.Send before chunking/posting. Protects fenced+inline code, converted links, and existing Slack entities behind placeholders; converts headers/bold/italic/strike/links; escapes control chars. Unit tests cover each construct plus fenced-code protection and a link nested in bold. Co-authored-by: multica-agent <github@multica.ai> * docs(slack): preserve Hermes MIT notice for ported mrkdwn converter (MUL-3516) Addresses Niko's review. formatMrkdwn is a substantial port of Hermes Agent's slack format_message; MIT requires preserving the copyright + permission notice. Add the full Hermes MIT copyright/permission notice + source URL as a header on mrkdwn.go (no repo-level third-party notice file exists, and the header cannot get separated from the ported code). Also add the suggested Send-layer regression test (TestSend_AppliesMrkdwn) that pins the wiring: slackChannel.Send converts Markdown to mrkdwn before posting. Co-authored-by: multica-agent <github@multica.ai> * refactor(lark): migrate Feishu onto shared engine.ChatSession, drop duplicate (MUL-3516) Completes 'every IM reuses one shared session service' and removes the dual-path the reviewers flagged as temporary. Feishu's ResolverSet now drives the channel-agnostic engine.ChatSession (channel_type=feishu, Lark session titles preserved) instead of the Feishu-specific lark.chatSessionService, which is deleted. Behavior is unchanged: engine.ChatSession is the verbatim port of the old logic and is unit-tested; the new Feishu binder param-mapping (BindingKey=chat id, CommandText=un-enriched CommandBody from Raw) is covered by feishu_resolvers_test.go. - Delete chat_service.go (chatSessionService + helpers) and issue_command.go/_test.go (parser now engine.ParseIssueCommand). Relocate the shared TxStarter interface to tx.go (still used by binding-token + registration services). - chat.go keeps only the AuditLogger seam; remove the now-dead ChatSessionService / EnsureChatSessionParams / AppendUserMessageParams / AppendResult / IssueCommand types. - router.go constructs engine.NewChatSession for Feishu; inbound_enricher_test + doc.go updated. make-test parity: go build ./..., go vet, gofmt, and go test ./internal/integrations/{lark,channel/...,slack} all pass (full Feishu suite green). Co-authored-by: multica-agent <github@multica.ai> * feat(slack): wire Slack adapter + ResolverSet + outbound into router (MUL-3516) Activates the full Slack pipeline, gated by MULTICA_SLACK_SECRET_KEY (the bot/app-token decryption key). When unset the block is skipped, so existing deployments are unaffected and Feishu is untouched. - router.go registers slack.RegisterSlack (Socket Mode connect/send Factory) + channelRouter.Register(TypeSlack, NewSlackResolverSet) (inbound pipeline) + slack.NewOutbound(...).Register(bus) (outbound). - New slack/outbound.go: an EventChatDone subscriber mirroring the Feishu Patcher. It finds the Slack chat binding for the finished session, recovers the real channel from the binding config (the channel_chat_id may be a composite thread-isolation key) + the reply thread from last_thread_id, and posts via slackChannel.Send (reusing formatMrkdwn / chunking / threading). Sessions with no Slack binding are ignored, so it coexists with the Feishu Patcher on the shared bus. - Tests: posts to the bound channel/thread with the real channel id; ignores non-Slack sessions, empty completions, revoked installations, and non-chat events. Slack now shares engine.ChatSession, channel_* tables, IssueService and TaskService with Feishu. Remaining: config-driven installation provisioning (an operator currently creates the channel_type='slack' row; the config block shape — which workspace/agent — is a product decision) and a live end-to-end smoke. go build ./..., go vet, gofmt, and go test ./internal/integrations/{slack,channel/...,lark} all pass. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-06-25 14:29:00 +08:00
Bohan Jiang	3e21e58df0	feat(channel): channel-agnostic engine (Supervisor + Router); Feishu as channel.Channel (MUL-3620) (#4512) ... * feat(channel): add channel-agnostic engine Supervisor (MUL-3620) Stage-1 (MUL-3515) shipped the channel abstraction but nothing drove it. Add the generic engine that does: - channel.InboundHandler + Config.Handler: the single shared inbound entry the engine injects into every adapter (Hermes set_message_handler model). - channel.Channel.Connect now blocks for the connection lifetime (doc), so the supervisor can tie lease renewal to connection liveness. - new package channel/engine: Supervisor, generalized out of lark.Hub. It enumerates active installations across ALL channel types (no hard-coded feishu), fences each behind the WS lease CAS, builds the platform Channel via channel.Registry, drives Connect/Disconnect with backoff+jitter, and restarts on credential rotation. Knows nothing about any platform. channel.Channel is now driven by an engine; integrations/channel has an external consumer. Feishu adapter + boot cutover follow next. Tests: supervisor_test.go covers lease CAS, reclaim, reap-on-revoke, rotation restart + token fencing, backoff on build error, lease-loss teardown, bounded release, shutdown timeout. Race-clean. Co-authored-by: multica-agent <github@multica.ai> * feat(lark): drive Feishu through the channel engine; remove lark.Hub (MUL-3620) Refactor Feishu into the first channel.Channel and cut boot over to the channel-agnostic engine.Supervisor, removing the Feishu-only Hub. - feishuChannel implements channel.Channel: Connect runs the existing WS long-conn connector for one installation; Send posts a text reply via the Lark IM API; Capabilities declares Feishu's feature set. RegisterFeishu wires it to channel.TypeFeishu — adding a platform is now 'register a Factory', no engine edit. - FeishuRuntime extracts the former Hub.handleEvent / scheduleReply: runs the Dispatcher and drives the detached typing indicator + OutcomeReplier off the connector ACK path. main.go drains it on shutdown after the supervisor stops delivering events. - channelInstallationStore (engine.InstallationStore) enumerates active installations across ALL channel types via the new de-hardcoded query ListAllActiveChannelInstallations; the Supervisor routes each row to its registered Factory by channel_type. Generic per-row fingerprint replaces the feishu-specific one. - boot: engine.Supervisor replaces lark.Hub.Run; MULTICA_LARK_HUB_DISABLED keeps its name for runbook compatibility. - delete hub.go / hub_pgx.go / hub_test.go; relocate the connector contract (EventConnector/EventEmitter), uuidString, and the reply-path tests (-> feishu_runtime_test.go) so coverage is preserved. No channel_* schema change. Feishu behaviour unchanged; lark + channel + engine tests green under -race; go build/vet ./... clean. Remaining (follow-up): lift the Dispatcher pipeline into a channel- agnostic engine.Router over channel.InboundMessage + resolver interfaces, so the inbound core stops being Lark-shaped and adding a channel needs zero core edits (validated by Slack, MUL-3516). Co-authored-by: multica-agent <github@multica.ai> * feat(channel): add channel-agnostic engine.Router (inbound pipeline) (MUL-3620) Generalize lark.Dispatcher's inbound pipeline into engine.Router: the single shared channel.InboundHandler the Supervisor injects into every Channel. It routes by ChannelType to a registered ResolverSet and runs the same ordered pipeline for every platform (install route -> two-phase dedup -> group @bot filter -> identity+membership -> ensure session -> append+mark -> /issue -> debounced run), then drives the detached OutboundReplier + typing indicator. Platform specifics live behind resolver interfaces (InstallationResolver, IdentityResolver, Deduper, SessionBinder, Auditor, OutboundReplier, TypingNotifier) + shared services (IssueCreator/TaskEnqueuer/SessionReader). Adding a platform is 'register a ResolverSet', not 'edit the Router'. Outcome / DropReason values match the legacy lark ones 1:1. Additive: lark.Dispatcher untouched and still wired; the feishu ResolverSet, the cutover, and the old-path removal land next. channel.InboundMessage gains ForceFresh (the normalized /fresh affordance). Batcher moved into engine. router_test.go covers the pipeline invariants (routing, dedup finalize states, group filter, identity, membership, ensure/append, /issue, debounce, flush offline, force-fresh, drain) with generic fakes; race-clean. Co-authored-by: multica-agent <github@multica.ai> * feat(lark): cut Feishu over to engine.Router; remove lark.Dispatcher; core no longer Lark-shaped (MUL-3620) Wire the channel-agnostic engine.Router (added in the prior commit) as the shared inbound handler and refactor Feishu into a ResolverSet, completing the generic-engine cutover. The inbound core (engine.Router) now contains zero platform specifics. - Feishu ResolverSet (feishu_resolvers.go): InstallationResolver, IdentityResolver, Deduper, SessionBinder, Auditor, OutboundReplier, TypingNotifier — each backed by the existing ChannelStore / ChatSessionService / OutcomeReplier / typing indicator, translating at the channel.InboundMessage boundary (platform fields read from Raw). origin_type stays 'lark_chat'. - feishuChannel now produces a normalized channel.InboundMessage and hands it to the engine handler via channel.Config.Handler; the old Raw round-trip through lark.Dispatcher is gone. - Remove lark.Dispatcher, FeishuRuntime, and lark's pending_batcher (the engine owns the pipeline + batcher now); their behavioural coverage moved to engine.Router tests. Surviving native types (InboundMessage / Outcome / DispatchResult) relocated to feishu_types.go. elon review nits addressed: - The channel engine (Registry + Router + Supervisor) is now built UNCONDITIONALLY, outside the MULTICA_LARK_SECRET_KEY gate, so a non-Lark deployment runs it; Feishu registers its Factory + ResolverSet only when its key is present. - channel.Config.Raw is now genuinely the platform config JSONB (channel_installation.config): the feishu factory builds a credentials-only Installation from it, and the workspace/agent identity is resolved per message by the Router — no full-db-row marshaling. - feishuChannel gains direct unit tests: factory config decode, Send text + reply-target mapping, Capabilities, inbound normalization + Raw round-trip, msg-type + result mapping. No channel_* schema change. go build/vet ./... clean; channel + engine + lark green under -race. Feishu behaviour preserved (pipeline logic lifted verbatim, only generalized). Co-authored-by: multica-agent <github@multica.ai> * docs(channel): fix stale comments on the channel engine boot (MUL-3620) Address Elon's review nit: three comments still described the pre-cutover behavior. - handler.go: ChannelSupervisor is built UNCONDITIONALLY now, not nil when MULTICA_LARK_SECRET_KEY is unset. - main.go: same — the supervisor always exists; only MULTICA_LARK_HUB_DISABLED parks it. - router.go: with no platform registered the store still lists active rows; Registry.Build returns ErrUnknownType and the supervisor backs off (it does not 'find no installations'). Comment-only; no behavior change. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-06-24 17:01:33 +08:00
Bohan Jiang	ce28d0aa0e	feat(integrations): add platform-agnostic channel foundation (MUL-3515) (#4412) ... * feat(integrations): add platform-agnostic channel foundation Introduce server/internal/integrations/channel — the contract every inbound IM integration implements, so the core never learns a platform's event JSON. Four pieces: - Channel interface (Type/Connect/Disconnect/Send/Capabilities) + Factory + Config (channel_type + opaque JSON blob, maps to channel_installation). - Normalized InboundMessage/OutboundMessage envelopes + Source/MediaRef/ ReplyCtx/MsgType/ChatType. Envelope holds only cross-platform-true fields; platform specifics live in Raw, read only by the adapter. - Capability bitmask: declaration only, no degrade logic in core. - Registry: Type->Factory map, last-writer-wins, concurrency-safe. Pure package (no DB/network/platform deps). Foundation for MUL-3515; the lark cutover + lark_*->channel_* generalization land in follow-up PRs. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * feat(channel): generalize lark_* tables into channel_* (DB layer) Migration 123 creates channel_installation / channel_user_binding / channel_chat_session_binding / channel_inbound_message_dedup / channel_inbound_audit / channel_outbound_card_message / channel_binding_token. Each carries a channel_type discriminator and a JSONB config for platform-specific identifiers/credentials; cross-platform columns stay flat. Existing Feishu rows are backfilled (channel_type= 'feishu', app_secret_encrypted via base64). NO foreign keys / cascades (MUL-3515 §4) — integrity moves to the app layer in the cutover. queries/channel.sql ports the lark query surface to channel_*, JSONB-aware, plus DeleteChannelUserBindingsByWorkspaceMember / DeleteChannelChatSessionBindingBySession for the app-layer cleanup that replaces the removed cascades. lark_* tables/queries are left in place here and removed once the Go cutover lands, so this commit ships green on its own. Verified: sqlc generate, go build ./..., full migrate chain (1..123) on Postgres 17, and a real-data backfill spot-check (base64 round-trip, NULL-strip, functional unique index on (channel_type, app_id)). MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * fix(channel): name app_id query param + multi-IM install key + null-safe binding merge Addresses review on MUL-3515 (PR #4412): - GetChannelInstallationByAppID: explicitly name params and cast app_id to ::text so sqlc emits AppID string. A bare $2 next to `config ->> 'app_id'` was mis-attributed to the JSONB config column, generating Config []byte. - channel_installation uniqueness -> (workspace_id, agent_id, channel_type), with the UpsertChannelInstallation conflict key matched. Lets one agent hold one installation per IM (feishu + slack + ...) instead of a later install clobbering an earlier one. Behaviorally identical in the current feishu-only world; "one agent, at most one IM overall" stays an app-layer rule per MUL-3515 §4, not a DB constraint. - CreateChannelUserBinding merges jsonb_strip_nulls(EXCLUDED.config) so a re-bind carrying {"union_id": null} no longer erases an already-captured union_id, restoring the old COALESCE(EXCLUDED.union_id, ...) semantics. Regenerated with sqlc v1.31.1. Verified on PG17: re-install replaces in place, feishu+slack coexist, null re-bind keeps union_id, real union_id wins. Co-authored-by: multica-agent <github@multica.ai> * feat(lark): channel-backed Feishu store + fix base64 backfill wrapping Cutover step 1 of switching the lark Go code from lark_* onto the channel_* tables (MUL-3515). Introduces the JSONB config boundary the rest of the cutover sits on, and fixes a latent backfill bug surfaced while building it. - migration 123: strip newlines from the app_secret_encrypted base64 backfill. PostgreSQL encode(...,'base64') MIME-wraps at 76 chars, and a secretbox- sealed ~72-byte secret exceeds that. Go's encoding/json decodes a JSON string into []byte with base64.StdEncoding, which rejects embedded newlines, so without the strip every migrated installation would fail to decrypt its app secret once reads move to channel_installation.config. - store.go: flat domain types (Installation / UserBinding / ChatSessionBinding) with field parity to the retired db.Lark* rows, plus the feishu config codec. Row->domain mappers decode the JSONB config; the secret decoder is whitespace-tolerant so legacy MIME-wrapped data still round-trips, while the encoder emits unwrapped base64. Binding config encodes an absent union_id as "{}" so the upsert's jsonb_strip_nulls merge never clobbers a stored union_id. - store_test.go: 72-byte secret round-trip, MIME-wrapped tolerance, optional null-strip, and flat-column preservation. Verified on PG17. Field parity keeps the upcoming ~190 db.LarkInstallation call sites a mechanical rename. No call sites switched yet; behavior unchanged. Co-authored-by: multica-agent <github@multica.ai> * feat(lark): route inbound integration onto channel_* + explicit membership checks Cutover step 2 (MUL-3515): switch the Feishu Go code from the lark_* queries to channel_* via a ChannelStore adapter, and replace the removed member foreign key with explicit application-layer membership checks. No user-visible behavior change. - channel_store.go: ChannelStore embeds *db.Queries and SHADOWS the ~24 lark query methods with channel_*-backed equivalents, keeping the db.Lark* signatures so the dispatcher/hub/services and their ~20k lines of tests stay untouched; the feishu JSONB config is (de)coded by store.go. Adds IsWorkspaceMember and a tx-aware WithTx. Only production wiring swaps *db.Queries for *ChannelStore. - Membership re-check (§4 removed the lark_user_binding -> member FK, so a binding row no longer proves current membership): * the dispatcher inbound identity step verifies membership after the binding lookup; a former member's stale binding is dropped as non_workspace_member + audited and never reaches chat_session (§4.3 safety property). * RedeemAndBind and BindInstallerTx replace the now-dead FK (23503) branch with an explicit IsWorkspaceMember gate, preserving the existing ErrBindingNotWorkspaceMember outcome without burning the token. - router wires the ChannelStore into the patcher, typing indicator, dispatcher, hub, and the union_id/region backfills; constructor-based services wrap *db.Queries internally so their signatures and nil-check tests are unchanged. Verified: go build ./... ; go vet ; gofmt ; go test -race ./internal/integrations/... (full lark suite green unchanged + new membership drop/error tests). Adapter field mappings (secret base64, union_id RMW, chat-id/open-id remaps, dedup, token, card) checked end-to-end against a PG17 channel_* schema. lark_* tables and queries remain (unused at runtime) until the S3 cleanup-hooks and S4 drop-tables/rename commits. Co-authored-by: multica-agent <github@multica.ai> * fix(channel): renumber generalization migration 123 -> 124 main merged 123_issue_stage after this branch forked, so the branch's 123_channel_generalization now collides on the migration number. The runner keys schema_migrations by full version string and would still apply both, but a duplicate number is a merge hazard and convention violation, so move the channel migration to the next free slot (124). issue_stage (ALTER issue ADD COLUMN stage) and the channel generalization touch disjoint tables; verified on PG17 that 123_issue_stage applies cleanly on a DB already carrying 124_channel_generalization, so the two are order-independent. sqlc regenerated (v1.31.1): only the migration-number comment changed. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * feat(channel): prune channel bindings on member removal + chat session delete MUL-3515 §4 dropped every channel_* foreign key, so the old ON DELETE CASCADE that cleared a user's channel_user_binding when they left a workspace, and a chat's channel_chat_session_binding when its chat_session was deleted, no longer fires. Re-establish that integrity in the application layer, inside the existing transactions: revokeAndRemoveMember -> DeleteChannelUserBindingsByWorkspaceMember, DeleteChatSession -> DeleteChannelChatSessionBindingBySession. Adds real-DB tests for both paths, including a scoping check that a remaining member's binding survives the prune. Verified on PG17: both new tests plus the existing revocation tests and the full handler package pass. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * fix(channel): scope Lark/Feishu store reads to channel_type='feishu' The S2 cutover routed the Feishu integration onto channel_*, but the Lark-facing ChannelStore wrappers read installation / chat-session-binding / outbound-card rows across ALL channel_type values. Once a second IM exists, that would let the Lark hub supervise a non-Feishu installation, the Lark install list show it, /lark/installations/{id} revoke another channel's row, and the outbound patcher / typing indicator act on a non-Feishu chat binding or card. Add a channel_type predicate to the six read/list channel queries and pass channelTypeFeishu from every wrapper: GetChannelInstallation, GetChannelInstallationInWorkspace, ListChannelInstallationsByWorkspace, ListActiveChannelInstallations, GetChannelChatSessionBindingBySession, GetChannelOutboundCardByTask. The S3 cleanup deletes (DeleteChannelUserBindingsByWorkspaceMember / DeleteChannelChatSessionBindingBySession) stay all-channel on purpose: a member leaving or a chat_session being deleted should clear every IM's binding. Adds a real-DB test that seeds a Slack installation/binding/card next to the Feishu ones and asserts the Lark wrappers never return them. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * refactor(channel): replace db.Lark* translation layer with lark domain types S2 introduced ChannelStore as a translation layer that read/wrote channel_* but kept the retired db.Lark* struct/param shapes so the dispatcher/hub/services and their ~20k lines of tests did not have to change. This collapses that layer: the store now takes and returns the package's flat domain types (Installation, UserBinding, ChatSessionBinding, InboundMessageDedup, BindingTokenRow, OutboundCardMessage) and the *Params types in params.go, with channel-neutral field names (ChannelUserID / ChannelChatID / ...). All call sites, fakes, and tests move to the domain types. No behavior change: only channel_* is read/written (as before); db.Lark* is now unused, and the lark_* tables + queries/lark.sql are removed in the next commit. Verified on PG17: go build / vet / gofmt clean, go test -race ./internal/integrations/... green (the ~20k-line fake suite), and the lark + handler suites pass. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * refactor(channel): drop lark_* tables and queries (remove old path) The Go cutover (previous commit) moved the lark package entirely onto channel_* and the domain types, leaving the lark_* tables, queries/lark.sql, and the generated db.Lark* models unused. Remove them per the design (§5: replace, do not keep both): migration 125 drops the seven lark_* tables (data already lives in channel_* since migration 124), and queries/lark.sql is deleted + sqlc regenerated, removing the db.Lark* models and lark query methods. The 125 down recreates the authoritative pre-drop schema (bot_union_id, region, per-installation dedup PK, thread-reply columns). Verified on PG17: fresh migrate up ends with lark_* gone + channel_* present; isolated 125 down/up round-trips correctly; go build / vet / gofmt clean; go test -race ./internal/integrations/... and the handler suite pass. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * fix(migrations): remove trailing blank line at EOF of 125 down migration git diff --check flagged a blank line at EOF of 125_drop_lark_tables.down.sql (a pg_dump-generation artifact). Whitespace only; the recreate SQL is unchanged. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * refactor(channel): defer lark_* table drop to a follow-up migration Preflight deploy review: dropping lark_* in the same release that cuts over (old migration 125) is not rollback/rolling-safe — the v0.3.27 release still reads lark_*, so a rolling deploy or a post-deploy code rollback would hit "relation does not exist". Remove the drop and keep the old tables for one release (standard expand/contract): migration 124 already backfilled lark_* -> channel_*, the new code reads/writes only channel_*, and the physical drop moves to a separate cleanup migration once this ships and is observed. The lark_* tables remain in the schema, so sqlc regenerates the (now unused) db.Lark* models; queries/lark.sql stays deleted (the new code uses channel_*). No code path reads lark_* — only the destructive drop is deferred, keeping the design's no-compat-layer / no-dual-write rule while being deploy-safe. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * fix(channel): skip orphaned installations in hub-boot active scan Preflight deploy review: channel_installation dropped the workspace/agent FK (MUL-3515 §4), so unlike lark_installation it does not cascade away when its workspace is deleted or its agent is hard-deleted (e.g. runtime teardown). The hub-boot query then keeps opening a WebSocket for a bot whose owner is gone. JOIN ListActiveChannelInstallations to live workspace + agent so an orphaned installation is never connected, uniformly for every deletion path. The JOIN matches the old ON DELETE CASCADE semantics (row existence, not agent archival), so an archived-but-present agent's installation is still listed; the orphaned row's encrypted secret is thereby never decrypted/used. Tests: a real-DB handler test asserts a deleted-workspace/agent installation and a non-Feishu one are both excluded; the lark scope test's active-list assertion moved there since the JOIN now needs real workspace/agent fixtures. (Physically deleting dormant orphaned channel rows on workspace/agent deletion is a separate app-layer-cleanup follow-up.) MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * docs(channel): document non-rolling cutover constraint for the lark->channel migration Elon deploy review: keeping the lark_* tables (deferred drop) stops old v0.3.27 code from crashing, but is not full expand/contract. Migration 124 is a one-time backfill; afterwards new code runs on channel_* (lease + dedup on channel_*) while pre-cutover code runs on lark_* (lease + dedup on lark_*). If both run concurrently during a rolling deploy, each side claims the same Feishu bot's WS lease on its own table and double-processes inbound events. This release therefore requires a NON-ROLLING cutover (stop the old hub before applying migration 124 + starting new code; rollback is not lossless once new code writes channel_*). Documented where deployers/reviewers see it: migration 124 header gains a ROLLOUT note; the channel_store.go header is corrected (lark_* tables are retained one release for rollback safety, not "gone"; the store still never touches them). Comment-only — no schema/codegen/behavior change. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * feat(lark): add MULTICA_LARK_HUB_DISABLED switch for the channel cutover The lark_*->channel_* cutover needs a way to make the Feishu bot briefly unavailable WITHOUT taking down the whole multica-api process — the Lark hub is a goroutine inside it, not a separate Deployment. MULTICA_LARK_HUB_DISABLED=true parks the hub at startup: the API serves HTTP normally but never claims a WS lease or opens a Feishu connection. Rollout (see migration 124 ROLLOUT note): ship the new release with the flag SET so new pods run API-only while old pods (hub on lark_*) drain during the rolling deploy — the two hubs never overlap. After the old pods are gone and migration 124 has run, flip the flag off; the new hub comes up on channel_*. The old backend does NOT need this switch — its hub stops when k8s terminates the old pods, not via a flag. Nil-ing LarkHub reuses the existing not-configured path so both the startup start and the shutdown join skip it. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * docs(channel): point migration 124 ROLLOUT note at the hub-disable switch Refine the rollout note to use MULTICA_LARK_HUB_DISABLED for a bot-only cutover (new pods serve API with the hub parked while old pods drain; flip the switch off after the migration), instead of the earlier whole-API recreate. Comment-only. MUL-3515 Co-authored-by: multica-agent <github@multica.ai> * docs(channel): fix migration 124 rollout order and document self-host cutover The previous ROLLOUT note shipped the new (channel_*) build before running migration 124, so the channel_*-backed HTTP paths (installation list/install/revoke, chat-session delete, member revoke) would 500 in the window between new-pod boot and the deferred migration. Restate the runbook around two explicit invariants — channel_* must exist before the new build serves those paths, and the old/new hubs must never overlap — and order the steps so channel_* is created first (park old hub -> snapshot -> deploy parked new build -> unpark). Document that default self-host (entrypoint migrate + single-replica Recreate) satisfies both invariants automatically and needs no manual steps; only prd / multi-replica rolling self-host needs the switch procedure. Clarify in main.go that the hub-park switch is generation-agnostic (parks whichever hub the build carries), which is what enables the preparatory release. Refs MUL-3515 Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-06-24 12:46:20 +08:00