Commit Graph
28 Commits
Author SHA1 Message Date
Basuotian 3647ec9a9c MUL-7687 fix(wecom): push an issue notification through the acting agent's bot (#8803)
In a multi-bot organization, every issue notification a member received arrived
through whichever WeCom bot they had bound most recently, because
FindChannelBindingForMember keys on the recipient alone. Three agents' comments
all looked like they came from one of them, and the apparent sender drifted
whenever the member re-bound an account.

An inbox item already carries the actor that raised it. When that actor is an
agent, the WeCom push now resolves the bot as the binding the recipient holds
on the acting agent's active installation
(FindChannelBindingForMemberOnAgentInstallation), keeping the recipient-wide
lookup as the fallback — for an agent this member never linked, and for
member-authored notifications. Delivery is never narrowed, which is the
guarantee #7136 named.

When the preferred bot provably put nothing on the wire, the recipient-wide
binding is tried too. Proving that needed a four-state relay verdict
(relayNotWired / relayAccepted / relayNoHolder / relayUncertain) in place of a
bool, plus a non-fencing DedupeStore.ClaimState read so the publisher can
observe a delivery claim without fencing a worker that is mid-delivery. An
unclaimed frame is fenced under a token no delivery worker ever presents, so a
late or replaying dispatcher cannot add a second card after the fallback ran.
An uncertain send is deliberately not retried.

The acceptance wait runs on the request's own goroutine, so it is paid only on
the agent-attributed attempt — the one with a second route behind it — and is
bounded by the caller's context rather than by the window alone.

Reviewed-by: seacen
Reviewed-by: Eve (MUL-7687)
2026-10-12 11:32:52 +08:00
苗大 3bacc41f84 MUL-7573 fix(lark): clear the Typing reaction without trusting in-process state (#8656)
* fix(lark): persist and reconcile typing cleanup across replicas

- preserve terminal trigger targets and per-input cleanup ownership
- reconcile late Adds and source deletion without delaying replies
- recover interrupted ledger indexes and cover lifecycle boundaries

Refs: #8655

* fix(lark): restrict typing cleanup to the current application

- match both app operator type and the current App ID
- preserve foreign-app, human and unknown-owner reactions
- cover ownership filtering in fast and durable cleanup paths

Refs: #8655

* fix(lark): bound typing recovery and credential retention

- recover taskless indicators after lost debounce timers
- cap workspace ledger admission and redact terminal credentials
- isolate maintenance budgets and test retention and quota boundaries

Refs: #8655

* docs(lark): document typing cleanup recovery and rollout limits

- explain ownership, retention, quota and observable terminal outcomes
- document migration upgrades and safe application rollback

Refs: #8655

* fix(deps): require patched Go 1.26.9 and x/net 0.60

- Require the patched Go standard library to clear reachable vulnerabilities.
- Upgrade x/net to the matching HTTP/2 security release.
- Align local development prerequisites with the minimum Go version.
2026-10-08 20:20:15 -07:00
Bohan Jiangandmultica-agent 1df4e0a8bf MUL-7477 fix(telegram): one reply per turn across replicas, restarts and retries (#8545)
* feat(db): record who owns a channel reply and what the provider accepted

A task's reply reaches a chat through three code paths — the streamed
placeholder, the final answer, the failure notice — and across replicas
through three processes: the daemon's transcript report and its completion
callback are independent HTTP requests, and the event bus is in-process. There
is nowhere for them to agree on whether a reply has already been sent.

channel_reply_delivery is that place: one row per task, carrying the phase the
reply is in, whether a send is out, and which message id it produced.

No foreign keys, and each index builds CONCURRENTLY in its own migration.
Workspace teardown deletes the rows alongside channel_outbound_message.

Co-authored-by: multica-agent <[email protected]>

* fix(telegram): deliver one reply per turn, across replicas and restarts

Users see every reply twice (#8049, #7750). #8117 closed the window where a
chat:done landed while the placeholder's sendMessage was still in flight, but
the reply's owner was an in-process map, so the duplicate simply came through
another door — most often the one #8117 could not reach at all: on the cloud
deployment the text report and the completion callback land on different
replicas, and the replica that cannot see the placeholder posts its own copy
of the same answer.

Every path now claims channel_reply_delivery before touching Telegram:

- The placeholder send is claimed in the database, so exactly one process
  opens a message and every other path edits it — including a process that
  never saw the stream.
- A send whose response was lost is recorded as unknown rather than absent.
  sendMessage takes no caller-supplied idempotency key, so re-sending cannot
  be deduplicated by the provider: delivery stops and the row says why.
- Terminal delivery claims from any phase but settled, so a second completion
  event is dropped while an interrupted delivery resumes from the chunk it
  reached. Chunk progress is written after each part, not after the last.
- Edit failures are classified instead of falling back to a fresh send:
  markup is retried as plain text, an ambiguous failure is retried on the same
  message under a budget, a permanently rejected edit settles the reply rather
  than spinning (which held the session's queue and blocked later answers),
  and only a target Telegram confirms is gone justifies a new message.
- The failure notice goes through the same ownership and the same
  classification, so it replaces the placeholder instead of appearing beside it.
- Cancelled and empty completions close the reply, so a text frame still in
  flight cannot reopen it.
- An automatic retry inherits the placeholder its previous attempt left behind
  instead of answering beside it. Scoped to a parked row, so two ordinary
  turns that happen to say the same thing still get one reply each.

The regression tests are the audit probes written against this bug: 12 of the
14 failed on main, and the two that passed are controls that must keep
passing — a normal streamed reply, and two separate turns with identical text.

Co-authored-by: multica-agent <[email protected]>

* refactor(db): key reply delivery by user turn and guard it with a lease

Review of the first version found the state model, not the implementation, to
be the problem. Three invariants were missing and are now in the schema:

- A placeholder is not progress. Recording the streamed message used to bump
  chunks_sent, which terminal delivery then read as "the first part of the
  final answer is already out" — so the answer was never written. The columns
  are now independent.
- One turn, one owner. owner_token / owner_expires_at are a lease: a path
  proves it holds the turn before calling the provider and proves it still
  holds it when recording the outcome. An atomic UPDATE was never the same
  thing as a single delivery, and two processes could both claim a reply.
- Ownership follows the real retry lineage. The row is keyed by the root of
  the agent_task_queue retry_of_task_id chain instead of "the most recently
  parked reply on this binding", which let an ordinary new request adopt an
  unrelated one and left the previous attempt able to reopen its own.

An expired lease frees a turn whose owner died, but never resets an
outstanding send to "not sent": there is no idempotency key to make that safe.

Co-authored-by: multica-agent <[email protected]>

* fix(telegram): one reply per turn across replicas, restarts and retries

Follow-up to the review of the first attempt, which shipped a broken success
path: its in-memory stand-in for the ownership table disagreed with the SQL
about one column, and the whole suite passed while the final answer never
reached Telegram. The tests no longer model that state machine — every
ownership query in this package now runs for real against Postgres.

Beyond the truncation, the review found seven ways a reply could still arrive
twice, be overwritten, or block the session behind it. All are covered here:

- Terminal delivery is exclusive. Two processes could both take a reply and
  both send it; taking the turn is now a lease, re-proven before every call.
- Chunk sends get the same published-before, recorded-after treatment the
  placeholder had, so an interrupted multi-part answer resumes after the last
  part instead of repeating it, and a lost response ends delivery rather than
  risking a second copy.
- A send left in flight by a process that died no longer blocks its session
  forever: the lease expires, the successor inherits it as unknown, and the
  turn settles instead of waiting on an owner that will never return.
- A streaming edit re-proves the turn immediately before it edits. Guarding
  only the first send left a stalled frame able to overwrite a final answer
  another replica had already published.
- Cancelled and empty completions create the closing record when the turn
  never reached Telegram at all, so a first frame arriving afterwards cannot
  open a placeholder nothing will finish.
- The failure notice honours an in-flight or unknown send instead of posting a
  second bubble about a turn the user may already have been answered about.
- Delivery without ownership is gone. It was written up as an accepted risk;
  it was a default duplicate path. A reply that cannot be owned is reported as
  a failure, on a budget that does not hold up the session.

Both suites — the delivery contract and the review's boundary cases — run by
default, and against real SQL, because that is what the fake hid.

Co-authored-by: multica-agent <[email protected]>

* refactor(db): order reply attempts and index the paths that delete them

Keying reply ownership by user turn stopped a retry from opening a second
answer, but left every attempt of that turn equal: a late frame from an
attempt the chain had already moved past could take the turn back and rewrite
what the user was reading with content from a superseded run. attempt_depth
records how far down the retry chain the holder sits, and the claim only ever
moves forward.

The table is also deleted by installation and by binding — workspace teardown,
a re-pointed installation, a retired chat-session binding — and had an index
on neither, so each of those was a full scan that grows with every turn. Both
are indexed now, and the three teardown paths that clean channel_outbound_
message clean this table alongside it, which the no-foreign-key rule requires.

Co-authored-by: multica-agent <[email protected]>

* fix(telegram): close the remaining duplicate and lost-notice paths

Review of the lease design found five more ways a reply could go wrong, each
fixed with a test that fails without it:

- A 5xx was treated as proof that nothing was posted. Telegram can accept a
  sendMessage and fail on the way back; the reply then went out twice. Only a
  4xx is evidence of a message that never landed.
- A failed retry-lineage lookup fell back to "this task is its own turn",
  which opens a second reply beside the one the previous attempt is holding.
  It is reported now, and delivery waits or stops rather than guessing.
- An attempt the retry chain had moved past could re-acquire its turn and
  overwrite the visible answer with its own stale tail.
- A reply settled by another replica left this process holding the stream and
  its chat-scheduler reference with nothing left to release them. Enough of
  those and the scheduler refuses new chats, so streaming stops for chats with
  nothing wrong with them. The next frame releases them, and an idle sweep
  covers turns where no next frame ever comes.
- The failure notice was posted inline and simply dropped whenever another
  path held the turn at that instant — likely, since that is exactly when a
  text frame is mid-request on another replica — leaving the placeholder as
  the last thing the user saw. It is terminal work now, so it waits for the
  turn like the answer does.

The notice moving to the queue also takes the last Telegram call off the
synchronous event bus.

Co-authored-by: multica-agent <[email protected]>

* fix(db): upgrade installed databases to attempt_depth, and guard the close path

attempt_depth was added by editing migration 500, which every database that
already installed this feature has recorded. Those databases never saw the
change, so every query touching the column failed on exactly the deployments
where the table already existed — and rebuilding from scratch passed, which is
why it went unnoticed. It moves to its own forward migration, written so a
database created by 500 and one upgraded through 504 end up identical, with a
test that runs the upgrade against the old table shape rather than a fresh one.

The close path also skipped the ordering the claim enforces. A cancellation or
empty completion from an attempt the retry chain had moved past could settle
the whole turn, and the retry's answer was then dropped as already delivered —
the user gets nothing. It now follows the same one-way rule, and a close that
belongs to a superseded attempt is a no-op rather than something to wait for.

Co-authored-by: multica-agent <[email protected]>

* fix(telegram): bound the notice to one call per step, and sweep on evidence

Two paths still escaped the ownership model:

The failure notice looped inside a single step — up to two edit attempts, and
a 429 wait taken inline. A step can then outlast the lease by minutes, so the
call after the wait lands on a turn another replica has taken over and may
already have finished. It is now shaped like the answer's delivery: at most
one Telegram call, hand back a retryAt, and let the next step re-prove the
turn first.

The stale-stream sweep treated thirty quiet minutes as abandoned. A run's
silence budget is hours — a task can sit in a tool call or a test run far
longer than any interval worth picking — so the sweep was free to reclaim a
live reply, after which the next frame would rebuild the stream with an empty
accumulator and edit the existing message down to just that fragment. Quiet
now only makes a stream a candidate; what releases it is the delivery row
saying the turn is settled or that a later attempt holds it.

Co-authored-by: multica-agent <[email protected]>

* chore(db): renumber the reply-delivery migrations above main's

main landed 500 and 501 while this branch was in review, so the numeric
prefixes collided on the merge ref and TestMigrationNumericPrefixesAreUnique
failed in CI. These move to 502-506.

Renumbering is safe for a database that already applied the old numbers: every
file here is idempotent — CREATE TABLE IF NOT EXISTS, CREATE INDEX
CONCURRENTLY IF NOT EXISTS, ADD COLUMN IF NOT EXISTS — so re-running them
under the new versions is a no-op that only records the new rows. Verified
against the local database, which had the old numbering applied.

Co-authored-by: multica-agent <[email protected]>

* fix(telegram): keep a provider call inside the lease that authorises it

The lease is only meaningful if a call made under it cannot outlive it. The
shared Bot API client allows 65s — which getUpdates needs for long polling —
against a 30s lease, and the turn was re-proven before waiting for the chat's
scheduler lock rather than after. A slow call could therefore still be talking
to Telegram after another process had taken the turn over and settled it, and
its result would then be fenced off: the reply is lost rather than retried.

Delivery calls now get their own budget, sized so one call plus recording its
outcome fits inside the lease, and every path re-proves the turn immediately
before the call rather than merely at the top of the step. A test asserts the
budget relationship so changing one constant cannot quietly reopen the gap.

Reaching the bad outcome needed a duplicate terminal event, which the task
completion path does not currently produce, so this was assessed as hardening
rather than a live defect. It is cheap enough to make hold by construction.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: multica-agent <[email protected]>
2026-09-20 18:12:39 +08:00
aa684997f1 MUL-7024 fix(wecom): clear the previous bot's rows when an agent swaps bots (#7985)
* fix(wecom): clear the previous bot's rows when an agent swaps bots

Pointing an agent at a different bot reuses the installation row —
UpsertChannelInstallation conflicts on (workspace_id, agent_id,
channel_type) and only rewrites config — so every dependent row keyed on
installation_id crossed over into a bot it did not belong to.

They are not merely stale. A WeCom aibot userid is anonymized per (bot,
user), which is the premise the whole binding flow rests on, so a
carried-over channel_user_binding holds an id from a namespace the new
bot does not share. Outbound.tryDeliverInbox finds that binding by
(workspace, member, channel_type), resolves the sender by its reused
installation_id — now the LIVE NEW bot — and addresses the OLD bot's
userid over it. A p2p chat binding carries the same userid as its
channel_chat_id, and a queued task delivery carries it again.

ClearChannelInstallationBotScopedRows is the reuse-case counterpart to
the cleanup ReclaimDeadChannelInstallationByAppID already does for the
dead-owner case. It runs in the install transaction, before the upsert,
so a swap that fails later leaves the old bot's rows intact rather than
half-cleared.

The installation row itself, its audit trail and the Chats stay: the row
is the same connection under new credentials, an inbound-audit row still
records something an operator can act on, and a Chat is the user's work
product that outlives whichever bot carried it — deleting the binding
detaches the route, the same shape a disconnect leaves behind.

A same-bot secret rotation is deliberately NOT a swap; clearing there
would silently unbind every member. Both directions are tested.

Closes #6547

* fix(wecom): report what a bot swap threw away, and cover every swept table

Review follow-ups on #6547, none of which change what the sweep deletes.

ClearChannelInstallationBotScopedRows becomes :one and returns a count
per swept table. A queued channel_task_delivery is a RUNNING task's
answer: dropping it is right — its address is the old bot's userid,
unreachable from the new connection either way — but processEvent then
finds no row and returns nil, with no counter and no line of its own, so
the install path is the only place that can say where the answer went.
InstallationService gains a nil-safe logger the way
channel_media_reconciler already has one, and logs the three counts.

The comment now says why the sweep is not airtight. The slot lock is the
NEW bot's; the OLD bot's socket stays live until the Supervisor tears it
down, so a message arriving under it between the DELETE and the COMMIT
re-inserts a binding that outlives the sweep. Short and self-healing —
that binding is the old namespace, so the next delivery fails to address
and the user re-binds — and closing it properly needs a second clear
once the old connection is confirmed down, which this transaction cannot
observe.

The test seeded four of the seven bot-scoped tables, so three deletes
were asserted by the query text alone. It now seeds channel_task_delivery,
channel_outbound_message and channel_outbound_card_message too, and
counts the card table by chat_session_id since it carries no
installation_id. With the swap gate forced off every one of the seven
reports surviving rows; with it on, both directions still pass.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015xUxTgkvJSCUpBTJDWZTYF

* fix(wecom): count the queued outbound sends a bot swap drops

Review's two nits from the second round.

The log covered three of the seven swept tables, and the argument for
logging channel_task_delivery — a queued answer disappears, processEvent
finds no row and returns nil with no counter and no line — is the same one
for channel_outbound_message. It is likely zero for WeCom in practice, which
is a reason to count it rather than to argue about it: the query returns a
fourth count and the swap line reports it.

The race-window comment said the stray binding is self-healing without
saying what healing looks like from the other side. It is one message that
gets no answer and no line explaining why, until the user re-binds — the
same silence the counts above just fixed one table over.

The counts had no test. They go nowhere but the log — Upsert returns an
Installation, not a report — so the swap test now captures the line through
the service's own logger and asserts a count per swept table. Dropping the
new field from the log fails it.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Wv2qRKBY6fCBnhVae7Mj58

---------

Co-authored-by: YeJing <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>
2026-09-16 01:01:31 +08:00
091d3ce5de MUL-7209 fix(lark): reply natively and mention the asker in ordinary groups (#8238)
* fix(lark): reply natively and mention the asker in ordinary groups

In an ordinary Feishu group the agent's answer arrived as a standalone
message with a plain-text "@name" the model had written itself, so it
neither attached to the question nor notified the member who asked
(#8234). Two independent gaps:

Native reply. The trigger message id was already there — the engine
records it on every inbound turn and the delivery row freezes it per
task — but threadReplyTarget required a thread id alongside it, so an
ordinary group (which has none) discarded a perfectly usable target and
fell through to the chat-level send. Now the thread id only decides
reply_in_thread; the message id alone is enough to reply. p2p is left on
the chat-level send: a 1:1 has no competing questions for a quote to
disambiguate.

Native mention. The sender's open_id is parsed on inbound but never
persisted, so the outbound patcher only had a Multica UUID. It now
resolves the open_id back from the task's own initiator_user_id through
a new installation-scoped channel_user_binding lookup, and renders a
real mention — <at user_id=...> for text, <at id=...> for the schema-2.0
card, since the two wire shapes spell it differently. Reading identity
from the task (not from the chat's most recent sender) is what keeps two
members' concurrent runs from cross-mentioning each other.

Identity is never inferred from the "@name" in the agent's prose:
duplicate and guessed names would ping the wrong colleague. With no
open_id — unbound member, missing row, failed lookup — the answer goes
out unmentioned rather than mentioning someone else.

sendWithThreadFallback is renamed sendWithReplyFallback; it now guards
ordinary replies too, and its classified error set (recalled trigger,
invisible message) already covered them.

Co-authored-by: multica-agent <[email protected]>

* fix(lark): freeze the trigger's sender per task instead of re-deriving it

The mention added in the previous commit resolved the open_id at send time
by looking the task's initiator_user_id back up in channel_user_binding.
That is unsound. channel_user_binding is unique on (installation_id,
channel_user_id), NOT on (installation_id, multica_user_id), and the redeem
path plain-INSERTs when a member binds a second Feishu account on the same
installation — the ON CONFLICT only fires for a repeat of the same
channel_user_id. With two rows for one member the member-keyed lookup has no
ordering and no reverse uniqueness to lean on, so it could return either: a
reply to account B carrying account A's mention. That is reachable from the
normal binding flow, not a corrupt-database state, and it defeats the
acceptance criterion the mention exists for.

The sender is per-trigger data, exactly like the message id it accompanies,
so it now travels the same path: channel_chat_session_binding.last_sender_id
is written in the same statement as last_message_id / last_thread_id on every
inbound turn, and CreateChannelTaskDeliveryFromSession freezes it onto the
task's delivery row. The Patcher reads that snapshot and the Multica member
stops being an input to the mention entirely, so the sibling-identity case is
now unrepresentable rather than merely unlikely. Writing all three in one
statement is what guarantees the sender always describes the message the
reply targets.

GetChannelUserBindingByMember and its store wrapper are removed with the
lookup they existed for.

Only the Lark adapter populates SenderChannelID today; other channels leave
it NULL, which reads as "no identity" and sends unmentioned — the same
degradation as pre-migration rows, which are not backfilled.

Tests: the sender rides along in the reply-target write and is NULL when
absent; a DB-backed test proves a delivery row keeps its own sender after a
second member messages the same group; and the Patcher mentions the account
that sent the trigger even when the task's initiator holds another open_id.

Co-authored-by: multica-agent <[email protected]>

* fix(lark): snapshot the reply trigger per context generation, not per session

The previous commit froze the trigger onto the task's delivery row, but it
sampled that trigger from channel_chat_session_binding — one row per
chat_session, holding only the LATEST trigger. The debouncer keys its timers
on (chat_session, context revision), so a run batched in revision 1 is
enqueued after a /clear has opened revision 2 and revision 2's own message has
committed. The delivery row then froze the newer generation's message and
sender: A's answer quoting and @-mentioning B. It needs no unusual
interleaving — only B's append committing inside A's 3s debounce window.

channel_chat_context_generation is keyed (chat_session_id, revision) and
already snapshots initiator_user_id against precisely this hazard;
SetChannelChatContextInitiator's comment spells it out for crash recovery. The
trigger belongs beside it, so the message/thread/sender triple moves there and
CreateChannelTaskDeliveryFromSession takes the context revision and reads that
generation. The snapshot is written under the same `cmd == nil` guard as the
initiator: a message excluded from agent input is not the question being
answered, so it must not become the message the answer quotes.

The binding keeps last_message_id / last_thread_id, which remain a genuine
per-session cursor driving the history-boundary bookkeeping, and loses the
last_sender_id the previous commit added — nothing reads it now. Migration 457
is rewritten rather than superseded, since it has not shipped; it seeds each
session's CURRENT generation from the binding cursor so live sessions keep
replying correctly across the deploy. Older generations are deliberately not
seeded: for those the cursor has already moved on and would be exactly the
wrong value.

This also fixes the same latent mis-attribution for the reply target on every
other channel — Telegram's quote-reply and Slack's thread had it too; only
Lark's ordinary-group reply made it visible.

Tests: two generations are appended with no delivery created in between, then
enqueued, and each delivery keeps its own trigger — the test first asserts the
session cursor holds the OTHER generation's message, so the wrong answer is
demonstrably reachable. Immutability after creation is kept as a separate case.

Co-authored-by: multica-agent <[email protected]>

* fix(lark): drop the migration's trigger seed, leaving pre-migration rows NULL

The seed copied channel_chat_session_binding's cursor into each session's
current generation. It cannot be right: AppendUserMessage advances that cursor
for channel commands too, while the generation trigger is deliberately written
only when cmd == nil. A session whose last pre-deploy message was /issue would
have that command seeded as a generation's reply target — the exact value the
guard exists to keep out.

It also buys almost nothing. The trigger is recorded during AppendUserMessage,
which commits well before the debounced flush that enqueues the task and
creates its delivery, so the first inbound turn after deploy already supplies
a correct value. The only rows a seed could serve are generations enqueued
with no post-deploy append — the recovered-run path — and for exactly those
the cursor has already advanced past the generation, so seeding hands them a
confidently wrong message and sender instead of no answer. The benefit set and
the wrong-value set are nearly the same set.

Leaving them NULL degrades to a chat-level send with no mention, which for
those runs is strictly better than the pre-change behaviour of quoting the
session's newest trigger, i.e. some other member's message. This also matches
451_agent_task_comment_thread's convention: pre-migration rows drain without
rewriting historical data.

Migration 457 is now purely additive DDL.

Co-authored-by: multica-agent <[email protected]>

* fix(lark): keep the topic route on the binding, and never leak a topic reply

Moving the whole message/thread/sender triple to the context generation
conflated two different things. The TRIGGER — which message an answer quotes,
which account it mentions — is per-generation. The ROUTE — which chat or topic
the answer goes to — is per-binding: a thread-isolated session has one binding
per topic, so its thread id names that topic for every generation of it.

Sourcing the route from the generation left pre-migration generations with no
thread. For Slack and Telegram that silently relocated a recovered run's answer
from its thread to the parent channel; for Lark, outboundChatID resolves the
composite "chat:thread" key to the parent group, so a topic conversation would
surface in front of the whole group. That is a visibility change, not the safe
degradation the previous commit claimed.

So channel_thread_id goes back to the binding — where it was before this PR —
and the generation carries only last_message_id and last_sender_id. Slack and
Telegram route correctly again with no adapter change, since both can place a
message in a thread from the thread id alone.

Lark cannot: its only route into a topic is replying to a message inside it.
With a topic route and no trigger message it now declines to send at all
rather than falling through to the parent group. Silence is recoverable — the
member asks again, and the answer is still in Multica — while a leak is not.
This is deliberately narrower than sendWithReplyFallback's chat-level retry,
which fires only when Lark reports the topic itself cannot receive the reply;
there the topic is unusable and delivering beats losing the reply. An ordinary
group keeps replying without a quote or mention, because a chat-level send
lands in the very chat the session belongs to.

Reachable only for generations predating migration 457 that are recovered
after deploy without a new inbound turn.

Co-authored-by: multica-agent <[email protected]>

* fix(lark): keep the reply thread with the trigger, and renumber to 460

The previous commit called the thread "route" and read it from the binding.
That only holds for thread-ISOLATED sessions, where there is one binding per
thread. Slack DMs are the counterexample the repo already encodes:
slackSessionRouting keeps ONE binding for the whole DM channel while replying
into whichever thread the member used. Reading the thread from the binding
there hands a debounced revision-1 run the thread of a later revision — the
same cross-generation hazard this snapshot exists to close for the message and
sender, reintroduced through the one field I moved out.

That is a live regression on ordinary DM traffic, not a legacy-only edge: A
asks inside thread T1, a /clear within the 3s window opens revision 2 at top
level, and revision 1's answer lands wherever revision 2 spoke.

So last_thread_id goes back onto the generation with the message and sender.
The three are one trigger and travel together. The binding keeps its own
cursor for the history-boundary bookkeeping, and route stays what it always
was: chat, type, config, revision.

Lark's fail-closed guard now identifies a topic-isolated binding by its
config, which is what actually marks one — larkSessionRouting writes a config
only for topic sessions — rather than by a thread id that is now trigger data
and legitimately absent for pre-migration generations.

Migration renumbered 457 -> 460: main has since taken 457, 458 and 459.

The route-survives-missing-trigger test is replaced rather than repaired. It
ran on a p2p, non-composite fixture with a hand-inserted thread, so it proved
nothing about isolated bindings and would have locked in the wrong
generalization. Its replacement is the DM shape: T1 in revision 1, top level
in revision 2, both appended before either delivery is created, each keeping
its own thread, with the binding cursor asserted to hold the other one.

Co-authored-by: multica-agent <[email protected]>

* fix(lark): recover the thread for isolated bindings when a generation has none

Putting the thread back on the generation fixed Slack DMs but reopened the
legacy hole on the channels that route by thread. A pre-460 generation carries
no thread, so a recovered run in a Slack channel thread or a Telegram forum
topic answered in the parent conversation: both adapters read
delivery.channel_thread_id directly and neither recovers a thread from the
composite key. Lark was covered by its config guard; these two were not.

CreateChannelTaskDeliveryFromSession now expresses both semantics instead of
one. When the generation recorded a trigger its thread wins, INCLUDING when
that thread is legitimately empty — which is what distinguishes a Slack DM
answered at top level from one answered inside a thread, the distinction the
previous commit restored. Only when there is no trigger at all does it fall
back, and only for a thread-isolated binding, where the thread is a stable
property of the session rather than a moving cursor.

Isolation is detected from the marker every adapter already writes: a binding
whose key is the composite "chat:thread" stores the real chat id in its config,
so a config chat id that differs from channel_chat_id IS the isolation flag
(larkSessionRouting, slackSessionRouting, telegramSessionRouting). A DM or
plain chat stores a config id equal to its key and recovers nothing, because
its cursor only ever names whoever spoke last.

Done in the shared query rather than in three adapters: the delivery row is
where route and trigger are frozen, so fixing it there covers Slack, Telegram
and any future thread-routed channel at once. Slack's historyTarget already
recovers a thread root from the composite key for reads; this is the same idea
for the write, minus the key parsing.

Lark still declines to send in this case regardless — a recovered thread is not
enough when the only way into a topic is replying to a message inside it.

Also corrects the migration's closing note, which still described NULL as
degrading uniformly to a chat-level send.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-09-10 16:45:55 +08:00
7c1d826a98 chore(metrics): remove database-sampled metrics (#7819)
Co-authored-by: Sol-Boy <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-31 17:06:17 +08:00
YYClaw 9e2d9b6d1c MUL-6661 feat(channels): add /new and /clear conversation controls (#7468)
* feat(channels): add new and clear conversation controls

* fix(dingtalk): resolve bot identity before dispatch

* feat(chat): expose channel-created chats consistently

* refactor(channels): snapshot task delivery routes
2026-08-25 18:28:01 +08:00
1cc46b2697 MUL-6516 fix(channels): enforce durable context boundaries for /new (#7362)
* fix(channels): enforce new context boundaries

* fix(channels): address context boundary review findings

* fix(channels): enforce causal new context boundaries

* fix(channels): scope outbound provenance to target

---------

Co-authored-by: Sol-Boy <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-22 11:27:39 +08:00
YYClaw 5e2d349ec4 MUL-6426: refactor(dingtalk): safely retire group routing and enhance discovery (#7241)
Retire mutable DingTalk group routing: inbound messages always target the installation's configured Agent, and observed groups become read-only inventory in dedicated tables with persisted activity counters.

Migrations 382-389 clean stale cross-Agent bindings, create and backfill dingtalk_group_presence and dingtalk_bot_identity, and install a mirror trigger so writes from an older process during a rollback window are not lost. The legacy dingtalk_group_route table and its indexes are retained for expand/contract; a later release drops the trigger and table together.

Closes #7239
2026-08-21 14:26:48 +08:00
b4bfe083be MUL-6407: mint UUIDv7 primary keys for append-heavy tables (#7230)
* perf(db): mint UUIDv7 primary keys for the append-heavy tables

Random v4 primary keys scatter every insert across the whole primary-key
B-tree, so each row dirties a different page. On the tables that grow
continuously — the task queue, messages, comments, activity and audit rows —
that is pure write amplification. UUIDv7 puts a big-endian millisecond
timestamp in the leading 48 bits, so consecutive inserts share a page.

The 12 tables converted here are the append-only hot paths:

  agent_task_queue, activity_log, inbox_item, comment, issue, chat_session,
  chat_message, autopilot_run, webhook_delivery, task_token,
  channel_inbound_audit, sys_cron_executions

Ids come from the new pkg/dbid, which returns a pgtype.UUID ready to assign to
a sqlc params struct. Upsert-dominated tables (task_usage, agent_runtime, ...)
are deliberately left alone: v7 only buys locality for rows that are actually
inserted. Lease and claim tokens keep gen_random_uuid() — they must stay
unpredictable and carry no timestamp.

No migration, and no backfill. Every converted INSERT binds its id as
COALESCE(sqlc.narg('id')::uuid, gen_random_uuid()), so the column default stays
reachable: a caller that passes no id inserts exactly as it does today. Tables
will hold a mix of v4 and v7 ids permanently, which the uuid type and every
parser in the repo already accept.

Verification: - pkg/dbid: version/variant, uniqueness, and a non-decreasing timestamp
    prefix; a shape test asserting every converted query keeps the COALESCE
    fallback; and a source scan asserting every production params literal mints
    an id, so a call site added later cannot silently fall back to v4.
  - internal/handler: round-trips issue, comment, activity_log, inbox_item,
    chat_session, chat_message, agent_task_queue, task_token and
    channel_inbound_audit against Postgres and asserts the stored id is v7,
    plus one case proving an unset id still inserts (as v4).
  - internal/scheduler: covers the one hand-written INSERT, asserting the
    execution id is v7 while lease_token stays v4.
Co-authored-by: multica-agent <[email protected]>

* fix(db): address UUIDv7 review feedback

Co-authored-by: multica-agent <[email protected]>

* fix(db): cover Telegram audit UUIDv7

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: Sol-Boy <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-20 13:42:44 +08:00
mxbao063-baoandmxbao063-bao 8060aa7277 MUL-5947: feat(dingtalk): route groups to different agents (#6660)
Route each DingTalk group discovered by one Stream robot to its own agent, with a revision-fenced cutover so a reassigned group never inherits the previous agent's session, transcript, or pending outbound card.

Co-authored-by: mxbao063-bao <[email protected]>
2026-08-13 14:09:25 +08:00
YYClaw 2a19f9ba2c MUL-5873 fix(channels): improve /new and /issue feedback (#6578)
A bare /new now returns an explicit confirmation and persists the fresh-start intent on channel_chat_session_binding.pending_fresh until the next chat task is successfully queued. A bare /issue returns usage guidance and skips issue creation, agent execution, and media resolution instead of inferring a title from chat history.

Closes #6577
2026-08-10 12:05:58 +08:00
Xichang(Seacen) Zhao 02d06229db fix(wecom): prove control of the bot before the install reclaim destroys somebody’s row (MUL-5891) (#6594)
* fix(wecom): prove control of the bot before the install reclaim destroys somebody's row

idx_channel_installation_type_appid is UNIQUE on
(channel_type, config->>'app_id') with NO workspace in it, so a bot id's
routing slot is global. ReclaimDeadChannelInstallationByAppID then hard-deletes
the row holding that slot along with every channel_user_binding,
channel_chat_session_binding and pending binding token beneath it.

Nothing verified that the caller has anything to do with that bot. Bot ids are
not secret — every member using the bot can see one — so an admin of ANY
workspace could type a known bot id with a junk secret and destroy a revoked
owner's installation and all of its bindings.

The reclaim query's own comment states the premise this restores: "workspace B,
which proves control by holding the same app credentials, rebinds." That
sentence describes a check that did not exist.

Upsert now probes first: one connection, one subscribe, no read loop, no
registration, nothing else in the process learns it happened. A rejection
(ErrCredentialsRejected) is the one answer entitled to blame the admin's
credentials. Everything else is ErrCredentialsUnverifiable and answers 503 —
reporting an unreachable WeCom as a wrong secret sends the admin to rotate one
that was fine, and a rotated WeCom secret cannot be recovered.

A nil probe keeps the old behaviour, so a deployment that cannot reach WeCom
at install time can still install. DingTalk already validates credentials at
install; WeCom was the outlier.

Four tests. Removing the probe fails two of them with the cost stated:
"Upsert reached the write path with an unproven credential — the reclaim would
have destroyed the current holder's installation and its bindings".

* fix(wecom): decide who owns the bot before the probe touches WeCom

Review follow-up on the install control check.

The probe is not a read. It subscribes, and WeCom allows exactly one live
subscriber per bot, so subscribing displaces whoever is connected. Firing it at
the top of Upsert meant a request that was about to be REFUSED still knocked
the rightful owner offline first: their supervisor reconnects, the caller
repeats the request, and an install nobody was permitted to make becomes a
denial of service against a live bot. The active / archived / other-workspace
conflicts were only detected further down, at the unique index.

Upsert now runs the whole sequence in one transaction, in order: take a
transaction-level advisory lock on (channel_type, bot_id), read the current
owner inside it, refuse every live owner other than the caller's own row, and
only then probe, reclaim and write. A refused install reaches WeCom zero times
and leaves the owner's row byte-for-byte where it was. The lock closes the
TOCTOU window a pre-read outside the transaction would leave open: a racing
install parks on the lock instead of reading a stale "nobody owns this".

Two supporting changes:

Only the codes WeCom documents as a refusal of the pair (40001 invalid secret,
40013 invalid corpid) are reported to the admin as a wrong credential.
Everything else non-zero — 45009/45033 throttling, platform failures, codes
added since — fails closed as unverifiable, with the raw code and message
logged. WeCom guarantees nothing about non-zero codes except that they are not
success, and telling an admin to rotate a long-connection secret that was fine
destroys it: it cannot be recovered. Codes only, never errmsg text.

Proof of control no longer has an off switch. NewInstallationService defaults
to the real handshake probe and returns ErrProbeRequired on an explicit nil,
and Upsert refuses to run without one, so no wiring mistake can reopen the
destruction path. Tests inject a fake explicitly.

New sqlc queries: LockChannelInstallationAppIDSlot (the serialization boundary)
and GetChannelInstallationSlotOwnerByAppID (LEFT JOINs so an orphan row
survives the read and stays distinguishable from a live owner).

Rebased onto main.
2026-08-08 11:07:07 +08:00
Xichang(Seacen) Zhao 72304b34cd fix(wecom): throttle binding-token minting to one link per user per minute (MUL-5880) (#6584)
Every message from an unbound user reached the needs_binding outcome and minted a row, so someone typing six lines at a bot they had not linked yet wrote six channel_binding_token rows and received six links.

Mint now looks for a live, unconsumed token minted inside BindingTokenMintInterval (60s) and, finding one, returns Reused with no raw secret — the table only ever held the hash. The caller points the user at the link already in their chat rather than building a "?token=" URL with nothing after it.

The throttle is best-effort: the lookup and the insert are two statements, and losing that race costs one extra row and one extra link, both private to the same user and both on the usual TTL. The window is measured entirely on the database clock.

Co-authored-by: seacen <[email protected]>
2026-08-08 11:06:35 +08:00
leroy-chenandseacen 040bb97b0f feat(wecom): WeCom (企业微信) smart-bot integration via the Channel engine (MUL-5226) (#5833)
Adds the WeCom (企业微信) smart-bot integration built on the unified Channel engine — inbound chat, /issue, inbox notifications, install/revoke UI and an explicit user-binding flow.

Co-authored-by: seacen <[email protected]>

Follow-up: #6547
2026-08-07 11:02:13 +08:00
Xichang(Seacen) ZhaoandClaude Fable 5 671a1a0c21 fix(server): read the chat's channel from its binding instead of guessing (#6330)
A claim reported chat_channel_type by trying the binding lookup once per
channel it knew the name of. The list was Slack alone, then {Slack, Feishu}
after MUL-4899. Any channel added later — WeCom is the first — fell off the
end and claimed as a web chat, so the runtime brief told the agent to deliver
files with `multica attachment upload` into a conversation that cannot carry
an attachment. That is the same failure MUL-4899 fixed, reintroduced by the
shape of the fix.

Every channel writes the same channel_chat_session_binding row and differs
only in channel_type, and UNIQUE (chat_session_id) allows at most one, so the
row already holds the answer. Read it by session id and take channel_type off
what comes back. chat_in_thread stays Slack-only, now keyed on the row's own
channel_type: the two commands it picks between are hardwired to the Slack
history reader, and no other channel has one.

The channel_type-scoped query stays for the outbound senders, which are
per-platform by construction and must not deliver into a foreign channel.

Co-authored-by: Claude Fable 5 <[email protected]>
2026-08-04 14:04:01 +08:00
b06af2ae17 feat(runtime): unbind agents on runtime delete instead of destroying them (#6220)
* feat(runtime): unbind agents on runtime delete instead of destroying them

Deleting a runtime archived its agents and then hard-deleted the rows, so the
agents and every conversation with them disappeared — while the confirmation
dialog said "archive", which a user reasonably reads as recoverable. Retiring a
laptop is an ordinary action; losing the agents configured on it is not an
ordinary consequence.

An agent is now a persistent business object and a runtime is replaceable
execution capacity: deleting a runtime unbinds its agents. `runtime_id IS NULL`
means unbound — orthogonal to archived — and the agent keeps its instructions,
skills, chats, labels, channel installations, autopilots and task history.
service.AgentReadiness already refused an agent with no runtime, so the
scheduling safety gate needed no change.

Two columns become nullable, not one. Without `agent_task_queue.runtime_id`,
deleting the runtime still cascades the task history away (and task_message /
task_usage / task_token with it), so the agents would survive with no record of
anything they did — the same class of loss. A NOT VALID CHECK keeps NULL confined
to history: an active task must always have a runtime, so claim / dispatch /
delivery-CAS paths can never observe one without. It is written against
completed_at rather than a status list so a future non-terminal status fails
closed instead of slipping through.

Two prerequisites this depends on:

- 'deferred' (migration 128) was missing from CancelAgentTasksByRuntimeOrAgent.
  It went unnoticed because the delete used to cascade those rows away; with the
  new CHECK it would abort the delete and make the runtime undeletable.
- The channel-installation / label / chat-pin / invocation-target / draft-restore
  cleanups were scoped to "archived agents on this runtime". Archived user agents
  now survive, so that scope is narrowed to kind='system' — otherwise the fix
  would produce a subtler loss: agent alive, configuration wiped.

Also removes the squad guard that refused (409) when an active squad's leader was
an archived agent on the runtime, plus the archived-squad delete that existed
only to get past squad.leader_id's RESTRICT FK. The leader is no longer deleted,
so nothing needs to be given up to retire a machine. Autopilots are no longer
paused either: their assignee survives, and a rebind restores them without the
owner having to remember to re-enable.

Reason codes: an unbound agent reports agent_runtime_required, not
runtime_offline. The copy for runtime_offline tells users to reconnect a machine;
an unbound agent has no machine to reconnect, and the fix is to bind a runtime.
Chat's bare 409 string gains the same code so the composer can offer that action.

API: agents gain runtime_bound. runtime_id stays a string (empty when unbound) so
installed clients keep parsing and no gated two-release rollout is needed. The
confirmed-delete endpoint is /unbind-agents-and-delete; /archive-agents-and-delete
still routes to it, and the compared expected_active_agent_ids set is unchanged —
widening it would 409 every older client forever.

Co-authored-by: multica-agent <[email protected]>

* fix: make runtime unbinding recoverable

Co-authored-by: multica-agent <[email protected]>

* fix: address runtime unbind review nits

Co-authored-by: multica-agent <[email protected]>

* fix: resolve runtime unbind review blockers

Co-authored-by: multica-agent <[email protected]>

* fix(migrations): renumber runtime unbind after main merge

Co-authored-by: multica-agent <[email protected]>

* test(daemon): avoid late-request lease flake

Co-authored-by: multica-agent <[email protected]>

* test(autopilots): bind validation fixture runtime

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-03 12:39:27 +08:00
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
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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* chore(migrations): renumber to 207/208 after upstream took 203-206

Co-Authored-By: Claude Fable 5 <[email protected]>

* docs(channel): note DiscardMedia self-invocation and the unknown-outcome skip

Co-Authored-By: Claude Fable 5 <[email protected]>

* chore(migrations): renumber to 212/213 after upstream took 207-211

Co-Authored-By: Claude Fable 5 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* chore(migrations): renumber to 213-216 after upstream took 212

Co-Authored-By: Claude Fable 5 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* chore(migrations): renumber to 216-221 after upstream took 213-215

Co-Authored-By: Claude Fable 5 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* chore(channel): gofmt the media-pending append fields

Co-Authored-By: Claude Opus 5 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: Claude Fable 5 <[email protected]>
Co-authored-by: Bohan-J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-27 15:44:39 +08:00
beastandBohan-J 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 <[email protected]>
2026-07-10 14:47:54 +08:00
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 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-09 15:20:49 +08:00
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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-09 15:07:11 +08:00
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 <[email protected]>
Co-authored-by: multica-agent <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: jiangliangyou <[email protected]>
Co-authored-by: Claude <[email protected]>
Co-authored-by: multica-agent <[email protected]>
Co-authored-by: J <[email protected]>
2026-07-07 15:07:02 +08:00
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 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-01 16:58:23 +08:00
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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-29 14:09:34 +08:00
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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-25 14:29:00 +08:00
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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-24 17:01:33 +08:00
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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-24 12:46:20 +08:00