15 Commits

Author SHA1 Message Date
beast
60048172a7 fix(lark): ingest inbound images and videos as chat attachments (MUL-4934) (#5580)
* fix: ingest feishu media as chat attachments

* fix: ingest feishu post embedded media

* fix(lark): make inbound media retries safe

* fix lark media resource limit

* fix(lark): move inbound media off ack path

* fix(channel): make inbound media runs durable

* fix(channel): close enqueue-vs-append race on media deferral

EnqueueChatTask read the session-wide media deadline in one statement and
sealed the input batch in a later one. Under READ COMMITTED a media message
committing between the two got sealed into a task the deadline read had
already decided was 'queued', so the daemon could claim it before its
attachment bound — the agent received the bare placeholder, and the later
media-ready promotion was a no-op against a non-deferred task.

After the seal, re-derive the deferral from the sealed batch itself in the
same transaction (DeferChatTaskForSealedPendingMedia): if any sealed message
still carries an unexpired media marker, flip the task to deferred with
fire_at aligned to the latest marker. The existing post-commit promote fence
already covers the opposite direction (marker cleared mid-transaction).

Adds a deterministic regression test that injects the media append between
the deadline read and the seal via a wrapped pgx.Tx.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): keep committed chat task out of enqueue error path

The post-commit media-ready fence returned its error from EnqueueChatTask
even though the deferred task was already durably committed. The router
flush treats any enqueue error as "no task exists": it clears the typing
indicator and logs an enqueue failure while the run still happens at its
fire_at deadline. Log the fence failure instead — the claim-path deferred
promoter re-queues the task regardless.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): cap global media resolution concurrency

Media jobs were serialized per session but unbounded across sessions: a
burst could open arbitrarily many concurrent 45s Lark downloads, and each
unknown-length upload may buffer up to the 100 MiB resource cap in memory.
Gate resolveAndBindMedia behind a global slot semaphore (default 8,
RouterConfig.MediaConcurrency). Per-session ordering is unchanged; on
shutdown a job cancelled while waiting for a slot proceeds straight to the
bounded DB finalize so marker clearing stays prompt. Also document that the
per-message media budget spans queue/slot waits (it must match the
persisted fire_at) and why timed-out uploads cannot leak unbounded orphans.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(chat): keep channel-sealed user messages on task cancel

Sealing the channel input batch stamps task_id onto channel user
messages, which exposed them to the cancel draft-restore path: an
empty-transcript cancel would DeleteUserChatMessageByTask the sealed
Feishu/Slack messages and detach their attachments. Those messages are
the durable record of what the platform sender wrote — the sender has
no Multica composer to restore a draft into.

Gate the restore-delete on ChatSessionHasChannelBinding in both the
synchronous finalize and the deferred finalize (the latter covers
markers left by an older replica during a rolling deploy); a bound
session now settles as "Stopped." instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): skip the media pipeline for messages without media

Every inbound message on a Media-enabled platform persisted a 45s
media deadline and queued a resolution job, so a plain text message
could wait behind the global media semaphore (its task deferred while
other sessions download 100 MiB videos) and a crash between append and
clear delayed a pure-text run to the full 45s fallback.

Add MediaResolver.HasMedia — a pure in-memory probe the Router calls
on the ACK path — and only persist the deadline / enqueue the job when
the message actually references platform media. The Feishu resolver
decodes the already-received payload and reports standalone image or
video keys and post-embedded img/media spans.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(chat): gate cancel restore on immutable channel provenance

The previous guard keyed the cancel restore-delete off
ChatSessionHasChannelBinding, but a binding only proves routing exists
right now: archiving a session and rebinding an installation both
delete the binding while preserving chat history, so a still-cancellable
sealed task could again restore-delete the original inbound messages.

Persist provenance on the message instead: migration 203 adds
chat_message.channel_ingested, stamped inside the channel append
transaction and never mutated, and both cancel finalize paths now gate
on TaskHasChannelIngestedMessages over the task's sealed batch. The
binding-existence query is removed. Regression tests cover ingest ->
archive/unbind -> cancel for a queued and a started task.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): reclaim media uploads that never gain an attachment row

Deadline expiry dropped already-resolved refs and a BindMedia failure
was log-only, leaving uploaded objects with no attachment row and no
reclaim path — the dedup mark commits with the message before media
runs, so a redelivery is dropped as a duplicate and never re-resolves
(and thus never overwrites) those keys, and workspace/session deletion
only enumerates the attachment table.

Add MediaResolver.DiscardMedia — a best-effort delete by StorageKey —
and call it from both failure paths in resolveAndBindMedia. The Feishu
resolver forwards to the storage backend's Delete. Tests cover a
partial upload discarded at the deadline, discard on bind failure, and
key-level deletion in the resolver.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(server): refresh comments stale after detached media ingestion

Channel tasks now seal a self-owned input batch, media ingestion is no
longer out of scope for the flattener, and MediaRefs are filled by the
detached resolver after append rather than by feishuChannel pre-engine.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(chat): stop keying channel empty-completion silence off chat_input_task_id

Sealing gave channel tasks a self input-owner, which broke
writeChatCompletionOutcome's discriminator: it treated any owned task
as direct, so an empty channel completion wrote the no_response
fallback row and the outbound patcher — which forwards any non-empty
chat:done content verbatim — pushed the English fallback body to
Feishu/Slack, violating the MUL-4351 contract.

Silence is now decided by the immutable channel_ingested provenance of
the task's input batch, looked up by the batch OWNER id
(chat_input_task_id): auto-retry clones inherit the owner while their
sealed messages stay tagged with the parent's id, so keying off the
task's own id would misread a channel retry as direct. The cancel-path
provenance gates switch to the same owner key via chatInputOwnerID.
chat_input_task_id is back to meaning only "input batch owner".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(migrations): renumber to 203/204 after upstream took 202

Upstream main merged 202_runtime_profile_add_qwen while this branch
held 202/203, tripping TestMigrationNumericPrefixesStayUniqueAfterLegacySet
on the CI merge tree. channel_media_pending becomes 203 and
channel_ingested becomes 204; no content changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channels): gate outbound delivery on channel provenance, not owner

Merging main brought #5645 (keep direct chat replies in Multica),
whose outbound gate assumed channel tasks leave chat_input_task_id
NULL. Sealed channel tasks own an input batch too, so on the merge
tree every channel reply and failure notice was classified as direct
and silently dropped — agents stopped replying in Feishu/Slack.

Both outbound gates now call engine.TaskInputIsChannelIngested: a NULL
owner keeps #5645's deliver-by-default for pre-sealing tasks, an owned
batch delivers only when it carries the immutable channel_ingested
stamp (keyed by the owner id, so auto-retry clones inherit the
verdict). Direct replies stay in Multica; sealed channel replies reach
the platform. Tests cover both directions on both platforms.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): discard media orphans on a fresh context, after finalize

DiscardMedia shared finalizeCtx with BindMedia, so a bind that failed
because the finalize deadline expired handed the storage deletes an
already-dead context — the compensation silently no-opped and the
orphans leaked anyway. The deadline path also ran S3 deletes before
the marker clear, eating the same 5s budget the user-facing
bind/promotion needed.

Collect the refs from both failure paths, run bind + promotion on the
finalize budget first, then delete on a fresh discard context. The
bind-failure test now pins that discard receives a live context.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): compensate result-uncertain media uploads and commits

The compensation protocol treated "the call returned an error" as "the
side effect did not happen", which is wrong in both directions across
the result-uncertain windows:

- An upload error can follow a server-side write (lost response,
  deadline mid-write). The attempted key never reached the router, so
  nothing could reclaim it and dedup guarantees no re-resolve. The
  resolver now idempotently deletes the deterministic key on a fresh
  budget right at the failure site.

- A commit error is not a rollback guarantee: a lost ack can report
  failure after Postgres durably committed the attachment rows, and
  the router's discard would then delete objects those rows reference.
  BindMediaRefs now converges the ambiguity on a fresh budget — any of
  the batch's URLs present proves the atomic commit landed (bind
  reports success); none proves the rollback (discard stays safe); a
  failed verification returns ErrMediaBindResultUnknown and the router
  keeps the uploads, preferring a rare orphan over a broken attachment.

Fault-injection coverage: an upload error deletes the attempted key; a
lost-ack commit keeps the bound attachment and reports success; a
verified rollback stays a discardable error; the router keeps uploads
on the unknown-outcome sentinel.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(channel): replace inline media compensation with an intent ledger and reconciler

Inline best-effort compensation cannot answer "did my side effect
happen?" at the moment it needs the answer — the DELETE/PUT reordering
and the empty-read-vs-in-flight-COMMIT gaps were both instances of the
same two-system atomicity problem. Persist the intent instead and let
an asynchronous reconciler settle it:

- channel_media_pending_object (migration 214; claim index 215 as its
  own single-statement CONCURRENTLY migration): a state machine row
  ('pending' -> 'deleting') with lease, attempt, and backoff columns.
- The resolver upserts the row BEFORE each PUT, state-guarded so a key
  the reconciler owns is never resurrected (the resource is skipped).
  ObjectURL is a pure function of configuration, so the row carries the
  attachment URL pre-upload.
- BindMediaRefs deletes the batch's rows INSIDE the attachment-insert
  transaction: commit landed <=> intents gone, atomically, so an
  ambiguous COMMIT never needs adjudication. A key already claimed to
  'deleting' is skipped (placeholder stays).
- Nothing is ever deleted inline. The reconciler — an independent
  worker so storage latency cannot starve other sweepers — claims due
  rows ('pending' past the settle delay, or expired leases) under a
  fresh lease, checks for a durable attachment reference only AFTER the
  claim (race-free: bind can no longer succeed on the key), deletes
  unreferenced objects outside any transaction, and backs off failed
  deletes with attempt-based retry. Crash windows converge for free.
- The settle delay is a fixed constant carrying NO correctness weight;
  invariant tests pin it at >=10x every pipeline budget. Metrics cover
  deletes, referenced clears, delete failures, and ledger backlog.

Removed: MediaResolver.DiscardMedia, ErrMediaBindResultUnknown, the
post-commit verification, and both router discard branches.

Tests: intent-before-upload ordering; upload error leaves the row and
deletes nothing; bind-wins vs reconciler-wins on the same key; lost-ack
and rolled-back commit injections (intent cleared iff the attachment
landed); reconciler three-state settle; expired-lease reclaim; delete
failure backoff and retry; settle invariants.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): never build or sweep the media reconciler without storage

store is nil when S3 is unconfigured AND the local upload dir fails to
initialize, but the reconciler was constructed unconditionally and
main only gates the goroutine on the reconciler pointer — the first
unreferenced ledger row (rows can pre-exist from a boot where storage
worked) would nil-pointer panic a bare goroutine and take down the
process.

Construct the reconciler only when a storage backend exists, and guard
RunOnce defensively: with no deleter it skips the sweep without
claiming, so rows are not stranded in 'deleting' until lease expiry.
Test covers the pre-existing-row + missing-storage boot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(channel): remove the dead pre-resolved MediaRefs ingress path

lark.InboundMessage.MediaRefs and the resolver's early-returns for
pre-populated refs were vestiges of the pre-detached synchronous design
— no producer fills them before the router anymore. Worse, the intent
ledger made the path actively misleading: refs arriving without ledger
rows would be silently skipped at bind (with a log blaming the
reconciler), contradicting the field's "already persisted" contract.

Delete the field, its channelMessageFromLark mapping, and both
early-returns; channel.InboundMessage.MediaRefs is now documented as
what it actually is — ResolveMedia's output channel, always empty on
ingress, attachable only through a claimed ledger intent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): enforce workspace tenancy on every ledger query

The intent-ledger upsert's conflict branch guarded only on state, so a
cross-workspace storage_key collision could rewrite the row's
workspace/message/url ownership; release and delete keyed on
(storage_key, lease_token) alone. The derived key embeds the workspace
UUID so none of this is reachable today — but tenancy must be enforced
by the workspace column in every query, never derived from the key
string (MUL-3515 rule, restated in this PR's review).

The upsert now updates only within the same workspace (a cross-tenant
conflict updates nothing, returns no row, and the resolver skips the
upload — the fail-safe direction), and release/delete take
(workspace_id, storage_key, lease_token). Tests pin that a foreign
workspace can neither steal, release, nor delete a row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(migrations): build the ledger primary key via a concurrent index

storage_key TEXT PRIMARY KEY created its unique index implicitly at
CREATE TABLE, against the repo convention that every migration index —
including a new table's unique index — is built CONCURRENTLY in its
own single-statement migration (the exact three-step pattern
client_usage_daily shipped in 207-209). The table now declares
storage_key NOT NULL, 216 builds the unique index concurrently, and
217 attaches the primary key USING INDEX; the claim index moves to
218. ON CONFLICT (storage_key) still resolves against the constraint,
and the full down/up round-trip is verified.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): bound each reconciler object delete with its own timeout

DeleteObject ran on the worker-lifetime context and the SDK's default
HTTP client has no overall request timeout, so one black-holed
connection would wedge the sequential sweep loop — and with it every
later batch and the backlog gauge — forever; a single-replica
deployment has no other worker to reclaim the lease. Each delete now
gets a 30s timeout (well under the 2min lease), and a timed-out delete
takes the existing release/backoff path. Covered by a blocking-deleter
test with an injectable timeout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): anchor the media deadline to the DB clock and bound queue waits by it

Two deadline gaps from review:

- The persisted marker was an application-clock timestamp compared
  against SQL now() everywhere it is read, so a skewed app node could
  shrink the fallback window and hand the agent a placeholder before
  the resolver's local budget ended. The append transaction now anchors
  a relative budget (MediaPendingSeconds) with now() + make_interval,
  writer and readers sharing one clock; the local resolve budget stays
  monotonic app-side. A DB test pins that the remaining budget measured
  by the DB clock equals the requested one.

- enqueueMedia's waits (per-session order, global slot) only watched
  shutdown, so in a burst an already-expired job kept its goroutine and
  payload until it reached the front. Both waits now also watch the
  message's deadline; on expiry the job skips the resolver entirely and
  runs only the empty finalize (marker clear + promotion), which also
  unblocks the session's later messages. Covered by a queued-expiry
  test that finalizes while the only slot is deterministically held.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): start the local media budget before the append transaction

The DB anchors the durable fallback at insert-time now(), but the
local monotonic budget started only after AppendMessage returned — so
the resolver outlived the fallback by the append/commit latency, a
window where the deferred task is already claimable while the resolver
still runs and the agent reads a placeholder that binds moments later.
Capture the local deadline before calling AppendMessage, restoring the
ordering local-gives-up <= durable-fallback-fires. A slow-append test
pins that the resolver's context deadline is measured from the
pre-append instant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(migrations): renumber to 224-229 after upstream took 216-223

Verified against the merged tree: the numeric-prefix uniqueness test
passes and the full migration set applies cleanly from scratch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): heartbeat the reconciler lease per row

One claim covers up to 50 rows under a single 2-minute lease, but the
batch is processed sequentially and each delete may run its full 30s
timeout — a few stalled deletes could outlive the lease mid-batch,
letting another replica reclaim the tail: duplicate concurrent
deletes, inflated attempt/backoff on rows whose owner was alive, and
skewed metrics.

The lease is now renewed before EACH row's settle work, so it only
ever needs to cover one row's worst case (invariant-tested: lease >=
2x the per-delete timeout). A renewal that matches no row means
another worker reclaimed it after a genuine expiry — the row is
skipped, leaving the new owner's state untouched. Test simulates a
mid-batch reclaim and pins the skip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): dedup post media resources and make local writes atomic

A rich post may reference the same image_key/file_key in several spans.
The object key derives from (message, type, key), so duplicates uploaded
to the SAME key twice: LocalStorage.UploadStream truncated the
destination up front and removed it outright on a copy error, so a
second failing attempt destroyed the object the first success had
produced — leaving an attachment row pointing at nothing. A second
succeeding attempt instead produced two attachment rows for one object.

Collapse duplicate spans by (fetch type, platform key) before the
upload loop, and write local uploads through a temp file renamed into
place so a failed write can only discard its own temp file. Tests cover
a duplicated span uploading once and a failed re-upload leaving the
previous object intact.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): fence late-materializing PUTs with a tombstone schedule

A DELETE cannot be ordered against a PUT the client already abandoned:
the store may materialize the object after the delete completes. The
reconciler cleared the ledger row right after deleting, so such an
object had no row and nothing to reclaim it — which made the settle
delay the de-facto correctness barrier for the PUT/DELETE race, exactly
what the design says it must not be.

The row is now kept as a tombstone ('tombstoned' state, migration 226's
CHECK) and re-deleted on a widening schedule (15m, 1h, 6h, 24h, the
pass index carried in last_error), so a late materialization is
reclaimed by a later pass; only after the schedule is exhausted is the
row dropped. Claim, heartbeat, lease, and tenancy predicates are
unchanged — a tombstone is claimed exactly like any other due row. A
separate gauge reports tombstones so they cannot be mistaken for a
backlog of objects awaiting reclaim, and the header comment now states
precisely what state fences (bind/commit) versus what the schedule
fences (late PUTs).

Tests: the reviewer's interleaving — DELETE completes, the abandoned PUT
materializes right after, and the object is gone by the end of the
schedule — plus a full schedule walk asserting the object is counted
once and the row clears at the end.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): tombstones must re-delete, not re-ask the reference question

A tombstone revisit ran the same reference check as a first settle, so
an attachment carrying the same URL — a re-ingested copy of the object —
sent the row down the "referenced, keep it" branch: the object was kept
and the row cleared, abandoning the re-delete schedule that fences the
ORIGINAL object against an abandoned PUT. A tombstone has already been
judged unreferenced and deleted; it exists only to re-delete whatever
materializes later, so it now goes straight to the delete + schedule
tail (extracted as settleDeletedObject, shared with the first settle).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(channel): keep the tombstone schedule position in its own column

The re-delete pass index was encoded into last_error, which the failure
path also writes: one failed re-delete erased the position and restarted
the walk. A store failing intermittently could therefore keep a tombstone
alive indefinitely — every recovery would resume at pass 1 and the row
would never reach the end of the schedule to be dropped.

tombstone_pass is now its own column (the table is introduced in this PR,
so migration 226 carries it), advanced only by a successful delete, and
the tombstone write clears the now-stale last_error. Test walks the
schedule across a failed re-delete and asserts it resumes rather than
restarts, and that the row still terminates.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(lark): derive media object keys per chat message

The object key was derived from the platform message alone, so a second
ingest of the same Feishu message reused the first ingest's ledger row.
That row can be a tombstone (up to ~31h while the re-delete schedule
runs), and the intent upsert refuses anything that has left 'pending', so
the second ingest skipped the upload and silently produced a placeholder
with no attachment. A re-ingest is reachable: the inbound dedup claim is
reclaimable once 60s stale and the dedup row is only vacuumed after 24h.

Keying on the chat message the object will attach to keeps the two
ingests independent, and nothing leaks: each one's objects are covered by
its own ledger row.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(storage): route both local upload paths through one atomic write

UploadStream wrote through a temp file and renamed into place, but the
buffered Upload path still truncated the destination up front — the
destructive shape the stream path exists to avoid, one caller away from
coming back. Both now share writeAtomic, which also restores the 0644 the
direct write used (CreateTemp makes files 0600).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(storage): keep the local upload chmod best-effort

The rename-into-place rewrite made a failed chmod fail the whole upload.
CreateTemp's 0600 has to be widened to the 0644 the direct write used, but
an upload dir on a mount that ignores chmod (SMB/NFS/FUSE) accepted the
old direct write fine — turning those deployments' uploads into hard
errors would be a regression for a cosmetic property. Log and continue.

Tests pin 0644 on both upload paths, and that a failed buffered upload
leaves no temp litter and no damage to a previous object.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(channel): never re-delete an object an attachment references

The tombstone pass skipped the reference check and deleted unconditionally,
so a durable attachment carrying that URL lost the only object it can read
— the dangling attachment the intent ledger exists to prevent, and the
opposite of the posture every other path here takes ("a reclaimable orphan
beats a broken attachment").

The check now runs on every pass. A positive result on a tombstone is
unreachable by design — keys are per (chat message, resource) and a bind
cannot attach a key that has left 'pending' — so reaching it means an
invariant broke: keep the object, clear the row, log it, and count it on
a dedicated reconciler_tombstone_referenced_total counter. The test's
contract is flipped to assert the referenced object survives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(storage): make the local staging file reclaimable after a crash

os.CreateTemp's random suffix meant a crash between the staging write and
the rename left a file nothing could name: the ledger records only the
final storage key, and DeleteObject removed only the object and its
sidecar. Each leftover can approach the 100 MiB resource cap and they
accumulate without bound.

The staging path is now derived from the object key, so DeleteObject
removes it alongside the object — which makes the media reconciler reclaim
it too, since the intent row is written before the upload. Opening it 0644
directly also drops the chmod the previous commit had to make best-effort.
Both read paths refuse the staging name (keys come from the request URL,
and a half-written body should not be readable); a user-supplied ".tmp"
extension is unaffected, since object keys are generated and never
dot-prefixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(channel): renumber the media migrations after merging main

main took 224 (agent_task_session_rollout_missing), so the ledger group
moves to 225-230 and the cross-references inside the table migration follow.
main's CompleteTask also grew a sessionRolloutMissing parameter; the three
call sites this PR added to chat_input_ownership_test.go pass false.

Verified the way the numbering is meant to be verified: full migration set
applied from scratch on the merged tree, and the whole server suite run
against that database.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(channel): let Postgres compute every reconciler deadline

The reconciler built settle cutoffs, lease expiry, backoff and re-delete
times from the process clock and compared them against the database's
now(). A replica whose clock had drifted would therefore settle rows whose
upload was still in flight (the object is deleted and the bind then refuses
to attach — media silently lost), hand out leases that are born expired
(rows churn between workers, attempt/backoff inflate), or compress the
tombstone schedule that fences a late-materializing PUT.

The four settle queries now take durations and derive their timestamps from
now(), so every replica reads one clock. The parameter types are the guard:
an app-side timestamp can no longer be passed. Test asserts the persisted
lease, backoff and re-delete deadlines all track the database's now().

The generated code also picks up main's new agent_task_queue column in the
two RETURNING task.* queries this PR adds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(lark): drop unrelated gofmt-only churn from this PR

Six files carried whitespace/comment-reformatting with no functional
change, unrelated to the inbound media pipeline. Reverting them to the
base revision keeps the diff focused on the feature (75 -> 69 files):

  server/internal/service/empty_claim_cache.go
  server/internal/integrations/lark/markdown_detect.go
  server/internal/integrations/lark/ws_chunk_assembler.go
  server/internal/integrations/lark/ws_chunk_assembler_test.go
  server/internal/integrations/lark/ws_frame_test.go
  server/internal/integrations/lark/registration_test.go

Verified: `git diff -w` against these files was already empty, so no
behavior is affected. go vet clean; tests covering these files pass.

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-27 15:44:39 +08:00
beast
387e4fee5d fix(channels): keep direct chat replies in Multica (MUL-4988) (#5645)
Classify outbound replies by task origin (chat_input_task_id) before consulting a channel binding, so web/mobile direct-chat completions and failures stay inside Multica even when the session was originally created from Lark or Slack. Channel-created tasks continue to deliver. Closes #5644.
2026-07-20 14:00:54 +08:00
Jiayuan Zhang
ed9adc2bbe feat: improve create issue field controls (#5532)
Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-17 11:11:52 +08:00
Bohan Jiang
ccacce60a1 fix(channels): auto-reclaim orphaned IM-bot installations + accurate rebind conflict copy (#4810) MUL-3937 (#5103)
* fix(channels): auto-reclaim orphaned bot installations + accurate rebind conflict copy

channel_installation has no FK to workspace/agent (MUL-3515 §4), so deleting a
workspace or hard-deleting an agent left the row behind, occupying the
(channel_type, app_id) routing slot forever — the bot could never be rebound and
the UI had no way to clear it (#4810). The 409 also always blamed "a different
Multica workspace" even when the real owner sat in the same workspace.

Auto-reclaim on delete:
- DeleteWorkspace and the runtime-teardown paths now sweep the workspace's /
  archived agents' channel installations and every dependent row in-tx.
- The shared install path (Feishu + Slack) reclaims a DEAD prior owner — a
  revoked placeholder or an orphan whose workspace/agent is gone — before the
  upsert, healing installations stranded before this fix. A live owner (active
  agent, including an archived one) is left in place, not stolen.

Accurate conflict copy:
- A rebind refused by a LIVE owner now distinguishes same-workspace / another
  agent, an archived agent, and a genuinely different workspace, for both Slack
  (typed sentinels) and Feishu (registration message).

MUL-3937

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

* fix(channels): reclaim cross-workspace revoked bots + sweep card/dedup/audit (#4810)

Address the #5103 review (yyclaw + Steve):

- Reclaim: a REVOKED installation in ANY workspace is now dead (except the
  caller's own row), not just same-workspace. Disconnect never hard-deletes the
  row and there is no release UI, so a cross-workspace revoked row would pin a
  bot's app_id slot forever, with the misleading "connected to another
  workspace" copy resurfacing. A new binder proves control by holding the app
  credentials, so reclaiming is safe. Live ACTIVE owners (incl. archived) are
  still refused.
- Sweep the two dependent tables the cleanups missed, in all three paths
  (reclaim / DeleteWorkspace / runtime teardown): channel_outbound_card_message
  (no reaper, so a permanent orphan otherwise) and channel_inbound_message_dedup
  (PurgeChannelInboundDedup has no caller).
- Audit rows: PURGE on the hard-delete paths instead of detaching them into
  permanently unattributable NULL rows; keep DETACH on reclaim, where the
  workspace survives and the row stays useful for triage.
- Tests: flip cross-ws revoked to reclaimed + add cross-ws active preserved;
  extend the reclaim and both delete-path cleanup tests for card/dedup and the
  audit purge/detach split; assert the channel sweep on the DeleteRuntimeProfile
  entry point.

MUL-3937

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

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-09 15:07:11 +08:00
ZeroIce
8797667fac fix(slack): prefer attachment body over fallback (#4866)
Co-authored-by: multica-agent <github@multica.ai>
2026-07-03 12:33:35 +08:00
Bohan Jiang
88377c1b1d fix(slack): recover attachment/blocks text for empty-Text chat messages (MUL-3931) (#4805)
* fix(slack): recover attachment/blocks text for empty-Text chat messages (MUL-3931)

`multica chat thread`/`chat history` dropped any message with an empty
top-level Text via normalizePage(), intended to skip join/system markers
but also discarding alerting/webhook bot roots (Grafana cards, incoming
webhooks) whose body lives in Attachments/Blocks. This regressed the
#4717 fix for its most common trigger: @mentioning the agent under an
alert card.

Add flattenSlackText(): fall back to Attachments[].Fallback, then
title/text/fields, then a best-effort Block Kit flatten (section/header/
context/markdown) before discarding. Derived text is capped so a verbose
card cannot flood the agent context; genuine content-less markers are
still dropped. Both read paths share normalizePage, so one fix covers
chat thread and chat history.

Closes #4803

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

* fix(slack): flatten rich_text blocks in chat history fallback (MUL-3931)

Addresses the review nit on #4805: flattenBlocks() previously handled
section/header/context/markdown but skipped rich_text, so a message
whose only body is a rich_text block (the shape Slack rich-text input
produces) could still be dropped. Walk rich_text sections/lists/quotes/
preformatted runs and concatenate their text and link runs.

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

---------

Co-authored-by: J <agent-j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-01 22:40:21 +08:00
Bohan Jiang
159d9bebb1 feat(slack): route /issue slash command through quick-create (MUL-3908) (#4793)
The Slack `/issue` slash command used to directly create a raw issue: the
typed line became the title verbatim and a `todo` issue was assigned to the
agent to work on immediately. That files a rough, unstructured issue and starts
the agent on it before it is well-formed.

Switch the slash command to the quick-create pipeline instead
(TaskService.EnqueueQuickCreateTask, the same path as the web "quick create"
modal): the invoker's natural-language description is handed to the
installation's agent as a prompt, and the agent authors a well-formed issue
(proper title + structured description) in the background, attributed to the
bound member. Because creation is now asynchronous, the ephemeral reply is an
acknowledgement ("On it…") rather than a created-confirmation with a number;
the agent's completion surfaces to the invoker as a Multica inbox notification
through the shared quick-create completion path.

Installation routing and identity/membership checks are unchanged, so the same
workspace boundary and account-binding rules apply. Scope is the slash command
only — the message-based `@bot /issue` still runs through the shared
cross-platform engine (which also serves Lark) and keeps its direct-create
behavior.

- slash_command.go: swap IssueService.Create for EnqueueQuickCreateTask via a
  narrow quickCreateEnqueuer interface; prompt is the full text (no title/body
  split); drop the now-unused splitIssueText / issueCreatedText / GetWorkspace.
- router.go: wire h.TaskService instead of h.IssueService.
- tests: cover enqueue + ack, multiline prompt pass-through, empty prompt,
  unbound, non-member, inactive, team mismatch, and enqueue-failure.
- docs (4 locales): describe the quick-create behavior.

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-01 17:32:42 +08:00
Bohan Jiang
247e8ed5ab feat(slack): reuse account link across apps in one Slack workspace (#4786)
A Slack user who linked their Multica identity to one bot was re-prompted
to link again when messaging a second bot (a different Slack app) in the
SAME Slack workspace, because bindings are keyed per-installation
(installation_id, channel_user_id).

On an unbound inbound, the identity resolver now looks up an existing
binding for the same (Multica workspace, Slack team, Slack user) via the
new FindReusableChannelUserBinding query and, if that user is still a
workspace member, materializes a binding for the new installation instead
of returning ErrSenderUnbound — so the second bot resolves the user
silently. Reuse is fenced to one Multica workspace AND one Slack team, so
it never crosses either boundary; legacy installs with no recorded team
never reuse.

Adds resolver unit tests for every decision path and updates the Slack
integration docs (en/zh/ja/ko).

MUL-3911

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-01 16:58:23 +08:00
Bohan Jiang
240ec4efd0 feat(slack): native /issue slash command over Socket Mode (MUL-3908) (#4780)
* feat(slack): native /issue slash command over Socket Mode (MUL-3908)

A message beginning with `/issue` is intercepted by the Slack client as a slash command and never delivered to the app, so the message-prefix /issue never worked on Slack (no event, no 👀, no issue).

Register /issue as a real slash command in the app manifest and handle EventTypeSlashCommand over the existing per-installation Socket Mode connection. It is a one-shot issue creation (no chat session / agent run) that reuses the shared IssueService and the same installation-routing + identity/membership checks as the message path, replying privately via the command's response_url (ephemeral) since a slash command has no message to react to.

Docs: register the command in the manifest and describe the slash-command behavior across all four locales.
Co-authored-by: multica-agent <github@multica.ai>

* docs(slack): add commands scope to /issue manifest; fix chat-run wording (MUL-3908)

Review follow-up: the manifest examples registered features.slash_commands but omitted the commands bot scope, so updating + reinstalling could still fail to grant the /issue command. Add - commands to oauth_config.scopes.bot in all four locales and document it in the permissions table.

Also correct the misleading "no agent run" wording in the slash-command header and router comment: a todo issue assigned to the agent still triggers it via maybeEnqueueOnAssign (issue-assignment), like the message /issue — the slash command only skips the chat session / chat run.

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

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-01 14:47:44 +08:00
Bohan Jiang
3a6d3522c8 feat(slack): two-command channel reads — chat history (overview) + chat thread [id] (MUL-3871) (#4762)
Replaces the single scoped `multica chat history --scope` read with two clean
noun-commands so the agent can navigate a channel with many threads (e.g. read
the specific thread a user referred to):

- `multica chat history` — the channel OVERVIEW: recent top-level messages, each
  thread tagged with thread_id + reply_count + latest_reply (it does NOT expand
  thread contents). Backed by GET /api/chat/history + slack.History.ChannelOverview
  (conversations.history).
- `multica chat thread [id]` — read one thread: no id = the thread you're in,
  an id = a specific thread IN THE SAME channel. Backed by GET /api/chat/thread +
  slack.History.Thread (conversations.replies; DM falls back to history).

The channel stays server-pinned to the session; a thread id is only a
within-channel locator, so the security boundary (no cross-channel reads) is
unchanged. `--scope` is removed.

The prompt now teaches both commands and, via a new chat_in_thread signal
(derived from the binding: last_thread_id != last_message_id), tells the agent
which to start with — `chat history` for a top-level @mention, `chat thread` for
an in-thread one.

Tests: slack ChannelOverview/Thread (current/by-id/DM-fallback/no-binding/clamp),
handler both endpoints + auth, prompt top-level vs in-thread guidance.

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-01 12:46:47 +08:00
Bohan Jiang
50a48cef1e feat(slack): unified multica chat history pull for channel backfill (MUL-3871) (#4747)
* feat(slack): add unified `multica chat history` pull for channel backfill (MUL-3871)

Agents @mentioned in a Slack thread/channel only saw the triggering message,
never the prior conversation (GitHub #4717). Instead of force-assembling a
recent-context block on every inbound (the Feishu approach), expose a single
channel-agnostic pull command the agent runs on demand.

- channel: normalized HistoryMessage/HistoryPage/HistoryOptions vocab so the
  agent sees one shape regardless of platform.
- slack.History: resolves session -> binding -> installation -> bot token and
  reads conversations.replies (real thread) or conversations.history (DM /
  top-level channel, capturing sibling messages). thread_ts is recorded on the
  binding config at session creation to pick the right call.
- handler GET /api/chat/history: authorized purely by the task-scoped token
  (stamped X-Task-ID -> the task's own chat session), so an agent can only read
  the conversation it is currently running for.
- multica chat history CLI command (no args; same for every channel).
- buildChatPrompt nudge so the agent discovers the command.

Feishu is intentionally untouched. Adding a platform = implement the reader.

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

* fix(slack): require task-token actor source on chat history endpoint

Niko's review caught a privilege-boundary hole: the endpoint trusted
X-Task-ID, but it is mounted under the general Auth group where a normal
JWT / mul_ PAT request does NOT strip a client-forged X-Task-ID — only the
mat_ task-token branch stamps it. A workspace member who knew a chat task id
could forge the header and read that task's Slack channel/DM/thread history.

Gate on the server-set X-Actor-Source == "task_token" (the Auth middleware
deletes any client-supplied value and re-stamps it only on the mat_ branch),
then trust X-Task-ID. Adds a regression test: a forged X-Task-ID without the
task-token actor source is rejected with 403 and never reaches the reader.

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

* fix(slack): thread-first history for follow-ups, channel for first turn (MUL-3871)

A Slack conversation has two nested histories: the surrounding channel and the
agent's own thread (the bot's first reply opens a thread on the @mention). The
first version picked replies-vs-history from a thread_ts fixed at session
creation, so a session started by a top-level @mention always read CHANNEL
history — even on follow-ups inside the bot's thread, which should read THREAD
history first.

- Add a HistoryScope (auto|thread|channel). The handler resolves auto:
  first turn (no prior bot reply) -> channel; follow-up -> thread. The agent can
  override with --scope channel|thread, and the response reports the scope read.
- The thread root is derived from the binding (last_thread_id / composite-key
  suffix), available for every engaged group session, instead of the
  creation-time thread_ts (now removed from the binding config).
- A DM degrades a thread request to channel history (DMs have no threads).
- Prompt guidance + CLI help updated to explain the policy.

Tests: scope selection (thread/channel/DM-fallback/no-root), root derivation,
and handler auto-resolution (first->channel, follow-up->thread, explicit
override).

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

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-30 16:48:13 +08:00
Bohan Jiang
e5995c423f feat(slack): typing reaction on inbound message (MUL-3874) (#4737)
* feat(slack): add typing reaction on inbound message (MUL-3874)

Mirror the Feishu typing indicator on Slack: react with 👀 on the user's
message when it is ingested, then remove the reaction when the agent's run
finishes (EventChatDone) or fails (EventTaskFailed).

- New slack.TypingIndicatorManager: Add on ingest, Clear on terminal run
  events; state keyed by chat_session_id, bot token re-resolved from the DB on
  clear (never held in memory), all failures logged and swallowed (best-effort).
- Wire via the channel-agnostic engine.TypingNotifier seam (slackTypingNotifier
  in the ResolverSet) — the Router already calls OnIngested off the ACK path.
- Clear subscribes to the event bus directly so a failed run also drops the
  reaction (the outbound replier only handles EventChatDone).
- Skip messages older than 2m so Socket Mode reconnect replays don't restamp.

Requires the installed Slack app to hold the reactions:write scope.

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

* fix(slack): clear typing reaction when no task runs; document reactions:write (MUL-3874)

Addresses review feedback on the typing-indicator PR.

1. Stuck reaction on offline/archived agent. The debounced flush
   (flushChatRun) enqueues no task when the agent has no runtime or is
   archived (or on any enqueue/reload error), so no task lifecycle event is
   ever published and the bus-driven clear never fires — leaving the 👀 (and
   Feishu's Typing) reaction stuck on the user's message. Fix at the shared
   engine seam: add TypingNotifier.OnSettled(ctx, sessionID), which the Router
   calls from the flush on every no-task exit (before any offline/archived
   notice). Both the Slack and Feishu notifiers route it to manager.Clear, so
   the latent Feishu case is fixed too. Adds engine coverage (offline/archived
   clear, success does not) and a Slack OnSettled test.

2. Missing reactions:write scope in docs. reactions.add/remove silently fail
   without the scope, but the BYO app manifest/docs never listed it. Add
   reactions:write to the manifest + scope table and a reinstall note across
   all four locales (en/zh/ja/ko).

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

---------

Co-authored-by: J <agent-j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-30 14:21:08 +08:00
Bohan Jiang
2b940046d7 fix(slack): build the binding link from the web app URL, matching Lark (MUL-3666) (#4703)
The Slack "link your account" prompt built its redeem link from
MULTICA_PUBLIC_URL, but /slack/bind is a web-app page — the link must use the
web app URL, not the backend/API URL. MULTICA_PUBLIC_URL is intentionally the
backend/API public URL (webhooks, daemon server_url, attachments); the Lark
replier already uses appURLFromEnv() (MULTICA_APP_URL ?? FRONTEND_ORIGIN).
Slack was never migrated, so on deployments that set FRONTEND_ORIGIN but not
MULTICA_PUBLIC_URL (e.g. dev) the binding prompt silently failed
("public url not configured") and @-mentions got no response.

Rename slack.OutboundReplierConfig.PublicURL -> AppURL and feed it
appURLFromEnv() in router.go, mirroring Lark. Backend/API-URL uses of
MULTICA_PUBLIC_URL (webhooks, attachments, daemon server_url) are unchanged.

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-29 17:13:41 +08:00
Bohan Jiang
11a3cf206b feat(slack): bring-your-own-app install + per-installation Socket Mode (MUL-3666) (#4566)
* feat(slack): single app-level Socket Mode connection routed by team_id (MUL-3666)

Reshape the Slack adapter from the stage-3 per-installation Socket Mode model
into the multi-tenant B2 connection model: ONE deployment-level Socket Mode
connection (app-level xapp- token, env MULTICA_SLACK_APP_TOKEN) receives the
Events API stream for every installed workspace and routes each inbound event
to its channel_installation by team_id — the existing
GetChannelInstallationByAppID routing, unchanged.

- AppConnector: the single shared connection (slack/app_connector.go). No leader
  election — per the design "one (or a few)" connections are fine: each replica
  opens one, Slack delivers each event to one of them, and the existing
  (installation, message_id) two-phase dedup guarantees exactly-once processing.
  Resolves the per-team bot user id (via the same app_id query) to detect/strip
  @-mentions, since one connection serves many workspaces.
- Inbound translation (Events API -> channel.InboundMessage) extracted to
  slack/inbound.go as free functions parameterized by the per-team bot identity.
- channel.go trimmed to the outbound Send-only sender; per-installation config
  (config.go) no longer carries an app-level token — installs hold only the
  per-workspace bot token (xoxb-) for outbound, since xapp- can't be OAuth'd.
- engine.Supervisor now skips channel types with no registered Factory, so Slack
  installs (driven by the app-level connector, not per-installation channels) no
  longer churn the lease/Build loop.
- Wiring: router.go builds the connector when MULTICA_SLACK_APP_TOKEN is set;
  main.go runs it alongside the Supervisor. Feishu untouched; channel_* schema
  unchanged.

Verified: go build ./..., go vet ./..., gofmt, and
go test ./internal/integrations/... all pass.

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

* feat(slack): OAuth self-serve install backend (MUL-3666)

Add the in-product OAuth install flow that creates Slack installations, the
keystone the B2 connector consumes.

- slack.InstallService: Begin (build authorize URL, seal workspace/agent/
  initiator into the OAuth state), Complete (verify state, exchange code via
  oauth.v2.access, upsert channel_type='slack' install with the bot token
  encrypted at rest, auto-bind the installer's Slack id so their first message
  is not dropped), plus List/Get/Revoke. State is stateless: sealed with the
  deployment secretbox + an embedded expiry, no session store.
- HTTP handlers (handler/slack.go): member-visible list, admin-only begin +
  revoke, and the public OAuth callback (recovers context from the sealed state,
  redirects the browser back to Settings → Integrations with a result flag).
- Routes + wiring: workspace-scoped list/begin/revoke mirror the Lark
  admin/member split; the callback is a public route like GitHub's. Built from
  MULTICA_SLACK_CLIENT_ID/SECRET (+ redirect derived from MULTICA_PUBLIC_URL,
  override MULTICA_SLACK_REDIRECT_URL; scopes via MULTICA_SLACK_SCOPES).
- Realtime: slack_installation:created / :revoked events.

Verified: go build ./..., go vet, gofmt, and go test ./internal/integrations/slack/...
all pass (new install_test.go covers state sign/verify/expiry/tamper, authorize
URL, code exchange + encrypted upsert + installer bind, and oauth error paths).

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

* feat(slack): in-product OAuth install UI for web + desktop (MUL-3666)

Add the "Connect Slack" self-serve install UI mirroring the Feishu/Lark
integration, completing the in-product install half of B2. Slack's OAuth flow
is a redirect (not a device-code QR poll), so the UI is simpler than Lark's.

- core: SlackInstallation / List / Begin types; api.listSlackInstallations /
  beginSlackInstall / deleteSlackInstallation; slackKeys + slackInstallationsOptions
  query; realtime invalidation on slack_installation:* events.
- views: slack-tab.tsx (SlackTab settings panel + per-agent SlackAgentBindButton
  + connected badge + disconnect confirm). Connect calls beginSlackInstall and
  hands the authorize URL to openExternal (system browser on desktop, new tab on
  web); Slack bounces to the backend callback which lands the install, and the
  realtime event refreshes the list. Wired into the Settings → Integrations tab
  and the agent-detail Integrations tab alongside Lark.
- i18n: en + zh-Hans settings.slack.* strings.

Verified: pnpm typecheck (full monorepo, 6/6) and pnpm lint (@multica/core,
@multica/views — 0 errors) pass.

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

* feat(slack): outbound Replier + user-binding redeem flow (MUL-3666)

Fill the stage-3 Replier=nil tail so non-installer Slack users can onboard and
get status feedback — completing B2 end to end.

- slack.OutboundReplier (engine.OutboundReplier): on NeedsBinding it mints a
  single-use binding token and DMs/replies a "link your account" prompt with the
  redeem URL (wrapped as <url|label> so formatMrkdwn doesn't mangle the
  base64url token); on AgentOffline/AgentArchived it posts a status notice; on an
  /issue-created Ingest it confirms the new issue. Plain chat stays silent (the
  agent's own reply lands via EventChatDone). Reuses the bot-token Send path and
  reads the installation row from ResolvedInstallation.Platform — no new transport.
- slack.BindingTokenService: Mint + transactional RedeemAndBind over the generic
  channel_binding_token / channel_user_binding queries (channel_type='slack'),
  mirroring lark.BindingTokenService. 15-min TTL, SHA256-hashed tokens, the
  three typed failure modes (invalid/expired, already-assigned, not-member).
- HTTP: POST /api/slack/binding/redeem (public, session-authed) maps the failures
  to 410/409/403. NewSlackResolverSet now takes the replier (nil disables it).
- Frontend: /slack/bind redeem page (packages/views/slack + apps/web route) +
  api.redeemSlackBindingToken + en/zh slack_bind copy.

Verified: go build ./..., go vet, gofmt, go test ./internal/integrations/...
(new replier_test.go covers all outcome branches + the prompt URL), plus full
pnpm typecheck (6/6) and pnpm lint (0 errors).

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

* fix(slack): address review must-fixes — connector leak, team-keyed install, /issue copy (MUL-3666)

Three fixes from Niko's review:

1. AppConnector.connectOnce leaked the Socket Mode goroutine/connection on a
   handler error: it ran sm.RunContext on the long-lived ctx and returned the
   error without cancelling it, so a transient DB/router error left the old
   connection alive (consuming events into an unread channel) while Run opened a
   second one. Each connection now runs under its own cancellable context and a
   deferred cancel + join tears it down on every exit path before reconnect.

2. Slack re-install collided with the (channel_type, app_id) unique index:
   connecting the same Slack team to a different agent failed because the upsert
   conflict key was (workspace_id, agent_id, channel_type). Add a team-keyed
   UpsertChannelInstallationByAppID (ON CONFLICT on the (channel_type, app_id)
   index, updating agent_id) and use it for the Slack OAuth install, so
   re-connecting a workspace moves the bot to the chosen agent instead of
   erroring. Feishu's per-agent upsert is unchanged.

3. /issue clarified: it is not a registered Slack slash command (no `commands`
   scope), so Slack never routes one to us. Issue creation runs through the
   message path — `@bot /issue <title>` in a channel or `/issue <title>` in a
   DM — which the engine parser handles. Documented in the connector and the
   user-facing copy (en + zh).

Verified: go build ./..., go vet, gofmt, go test ./internal/integrations/...,
make sqlc, plus pnpm typecheck (6/6) and pnpm lint (0 errors).

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

* fix(slack): make OAuth install transactional — agent-move binding consistency + cross-workspace guard (MUL-3666)

Address Elon's review: the team-keyed upsert kept the same installation row and
only flipped agent_id, but engine session reuse matches purely on
(installation_id, channel_chat_id) and each chat_session is permanently tied to
the agent it was created under — so after moving a Slack team from Agent A to
Agent B, existing DMs/threads kept routing to Agent A; only brand-new
channels/threads reached B. Cross-workspace re-install was worse: the SQL also
moved workspace_id while the application-layer user/chat-session bindings stayed
behind, inheriting the previous workspace's relations.

InstallService.Complete now runs one transaction (lookup → upsert → retire →
installer-bind), all application-layer per the no-FK rule:

- Look up the existing installation by team_id (config->>'app_id').
- Reject a silent cross-workspace ownership change (ErrTeamOwnedByAnotherWorkspace
  → callback redirects with slack_error=team_in_other_workspace). The owning
  workspace must disconnect first.
- On an agent change within the same workspace, retire the installation's
  chat-session bindings (new DeleteChannelChatSessionBindingsByInstallation) so
  the next message creates a fresh session under the new agent. The chat_session
  rows are preserved for history; user bindings stay valid (same users/workspace).
- Installer auto-bind moves into the tx; an already-bound-elsewhere id is a
  benign skip, a real DB error aborts the whole install.

InstallService now takes a TxStarter; the queries seam gains WithTx (dbInstallQueries
adapter) so Complete stays unit-testable with a fake tx.

Verified: make sqlc, go build ./..., go vet, gofmt, go test ./internal/integrations/...
(new tests: agent-move retire, same-agent no-retire, cross-workspace reject,
fresh-install no-retire).

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

* fix(slack): atomic cross-workspace install guard + green up frontend CI (MUL-3666)

Two things: address Elon's review and fix the failing frontend CI job.

Review (atomic cross-workspace guard): the previous guard was a SELECT before
the upsert, which loses the concurrent-OAuth race — two workspaces can both read
no rows, one inserts, the other's ON CONFLICT update then silently re-points the
team. Move the guard into the upsert itself: ON CONFLICT ... DO UPDATE ... WHERE
channel_installation.workspace_id = EXCLUDED.workspace_id, and map the empty
RETURNING (pgx.ErrNoRows) to ErrTeamOwnedByAnotherWorkspace. The pre-SELECT now
only feeds the agent-change cleanup. Also corrected the error copy: a team stays
bound to its first Multica workspace (revoke is soft, keeping the row + unique
index), so migration is an operator action, not "disconnect first".

CI (frontend vitest, @multica/views#test):
- The agent IntegrationsTab now renders the real SlackAgentBindButton, whose
  connected badge calls useQueryClient — absent from integrations-tab.test.tsx's
  react-query mock. Hoisted the owner/admin gate above the per-platform sections
  (one role notice instead of one per platform), made the agents members_note
  generic (en/zh/ja/ko), and updated the test (mock @multica/core/slack, stub
  SlackAgentBindButton, assert both platforms).
- Added slack-tab.test.tsx covering the real SlackAgentBindButton / SlackTab.
- locale parity: added the slack (settings) + slack_bind (common) blocks to ja
  and ko so every EN key has a translated counterpart.

Verified: make sqlc, go build ./..., go vet, gofmt, go test ./internal/integrations/...;
pnpm --filter @multica/views test (1478 pass), pnpm typecheck (6/6), pnpm lint (0 errors).

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

* fix(slack): surface agent-page Slack entry points when Lark is off (MUL-3666)

The agent-detail Integrations tab and the inspector's Integrations section
only considered Lark, so a Slack-only deployment (Lark disabled) showed neither
the Integrations tab nor a Connect-Slack button — the per-agent entry points
were unreachable.

- agent-overview-pane: gate the Integrations tab on Lark OR Slack configured
  (new slackInstallationsOptions query), not Lark alone.
- agent-detail-inspector: render SlackAgentBindButton alongside LarkAgentBindButton
  in the Integrations section.
- regression test: the Integrations tab appears when only Slack is configured.

Verified: pnpm typecheck (6/6), pnpm --filter @multica/views test (1478+ pass),
pnpm lint (0 errors).

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

* feat(slack): BYO-app install backend — paste xoxb+xapp, per-app install keyed by real app id (MUL-3666)

Adds the bring-your-own-app install path so multiple agents can each have
their own bot identity in the SAME Slack workspace (hosted B2 caps at one
agent/workspace). User pastes their app's bot token (xoxb-) + app-level
token (xapp-); we validate the bot token via auth.test, parse the real
Slack app id from the xapp- token, encrypt both tokens, and persist a
per-app installation keyed by that app id (real 'A…' ids never collide
with hosted 'T…' team ids in the existing unique index — no schema change).

- config.go: add app_token_encrypted (BYO discriminator + per-app socket token)
- install.go: extract shared persistInstall (atomic cross-ws guard + agent-move retire)
- byo_install.go: RegisterBYO + auth.test + app-id parse
- handler + route: POST /api/workspaces/{id}/slack/install/byo (admin-only)
- tests: keying, encryption, invalid tokens, auth.test failure, cross-ws, agent move

Follow-ups (separate commits): per-app Socket Mode connector that consumes
the stored app token; in-product BYO install dialog (video + paste form).

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

* refactor(slack): drop OAuth, unify on BYO per-installation model (MUL-3666)

Per product decision, Slack drops the hosted-app OAuth path entirely and
unifies on bring-your-own-app (BYO): every installation carries its OWN
app-level token and gets its OWN Socket Mode connection, so multiple agents
can each have a distinct bot identity in one Slack workspace.

- Remove OAuth install (Begin/Complete/code-exchange/sealed state/OAuthConfig/
  default scopes), the OAuth callback + begin handlers + routes, and the
  MULTICA_SLACK_CLIENT_ID/SECRET/REDIRECT/APP_TOKEN env wiring.
- Replace the single deployment-level AppConnector with a per-installation
  slackChannel (authenticated with its own xapp- token) registered as a channel
  Factory, so the engine Supervisor drives one Socket Mode connection per
  installation (exactly like Feishu). inbound/outbound/resolvers reused as-is.
- Route inbound by the event's api_app_id (== the installation's real app id),
  not team_id.
- InstallService slims to at-rest encryption + the shared persistInstall +
  list/get/revoke; install is the BYO paste path only (byo_install.go).
- Tests: drop the OAuth tests; slack + handler + engine all green.

Follow-up (frontend): replace the OAuth "Connect Slack" button with the BYO
paste dialog (the begin endpoint it calls is now gone).

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

* fix(slack): verify BYO bot + app tokens are from the same app, and the app token is live (MUL-3666)

Niko review: RegisterBYO only parsed the app id from the xapp string and
auth.test'd the bot token, so pasting app A's bot token with app B's app
token would 'connect' but be broken (inbound on B's socket, outbound with
A's identity). Now: resolve the bot's owning app id via bots.info (on the
bot_id from auth.test) and require it to equal the xapp's app id; and live-
validate the app token via apps.connections.open. Reject (no persist) on
mismatch or a dead app token.

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

* feat(slack): in-product BYO install dialog (paste bot + app tokens) (MUL-3666)

The OAuth begin endpoint was removed server-side, so the "Connect Slack"
button now opens a dialog where the admin pastes the bot token (xoxb-) and
app-level token (xapp-) of the Slack app they created, and submits to the
BYO install endpoint. Includes an optional setup-video link (URL constant,
left empty until the walkthrough is recorded).

- core: drop beginSlackInstall / BeginSlackInstallResponse; add
  registerSlackBYO + RegisterSlackBYORequest.
- views: SlackAgentBindButton opens the BYO dialog; refreshed comments and
  install_supported docs (now means "configured", no OAuth).
- i18n: new slack.byo_* keys + refreshed page_description in en/zh-Hans/ja/ko.
- tests: dialog submit path; views vitest (1479), typecheck, lint, locale
  parity all green.

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

* fix(slack): Elon review — team_id routing guard, per-agent reconnect, users:read hint (MUL-3666)

1. Inbound routing keys on api_app_id (the APP, not the Slack workspace), so
   additionally require the event's team_id to match the installation's stored
   team. A distributed BYO app installed into another Slack workspace emits the
   same app id and would otherwise mis-route to this Multica installation.
   Extracted installationServesTeam() + unit test.

2. BYO install is now agent-keyed (UpsertChannelInstallation, conflict on
   workspace_id+agent_id+channel_type): one bot per agent. Disconnect →
   reconnect a NEW app for the SAME agent now UPDATES that agent's row in place
   instead of violating the (workspace, agent, channel) unique. A unique
   violation on the (channel_type, app_id) routing index → ErrTeamOwnedByAnother-
   Workspace (the app is already connected to another agent/workspace). No
   chat-session retire is needed: a row's agent_id never changes.

3. UX: bots.info (the same-app check) needs the users:read scope — the connect
   dialog now lists the required bot scopes including it, and the error text
   says so.

Backend build/vet/gofmt/test + views vitest + typecheck + locale parity green.

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

* fix(slack): publish slack_installation:created on BYO connect; refresh stale comments (MUL-3666)

Niko final review: RegisterSlackBYO wrote the response but never published
EventSlackInstallationCreated, so only the installer's own tab refreshed —
other open clients (Settings, Agent Integrations, other tabs) did not see the
new bot in realtime, inconsistent with the revoke event and Lark. Now publishes
it on success via a small publishSlackInstallationCreated helper, with a unit
test (Bus.Publish is synchronous).

Also refreshed comments that still described the removed hosted-OAuth /
single deployment-level AppConnector model (handler SlackInstall field,
channel.go / inbound.go / outbound.go / byo_install.go). PR title updated
separately to the BYO per-installation Socket Mode model.

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

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-29 14:09:34 +08:00
Bohan Jiang
cb6616f530 feat(slack): Socket Mode channel.Channel adapter (MUL-3516) (#4523)
* feat(slack): Socket Mode channel.Channel adapter (MUL-3516)

First slice of the Slack adapter: implements channel.Channel (Type/Connect/Disconnect/Send/Capabilities) over Slack Socket Mode, normalizes inbound events to channel.InboundMessage (DM, channel @mention, thread reply; bot-loop + edit/delete guards), decodes the per-installation config/secret blob, and registers the Factory under TypeSlack. No engine, core, or channel_* schema change. Unit-tested (translation, capabilities, config decode, chunking, Send via httptest). Resolvers + engine wiring + Block Kit binding replier follow.

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

* fix(slack): address adapter review (MUL-3516)

- Propagate InboundHandler errors through dispatchEventsAPI/handleSocketEvent to Connect so an infra failure tears down the connection for Supervisor reconnect/backoff instead of being silently swallowed (ACK still happens first).
- Capabilities: declare only CapText | CapThreadReply; drop CapRichCard/CapAttachment/CapMessageEdit until those Send paths are wired.
- slackChatType: map mpim (multi-party DM) to group, not p2p, so the 'must address bot' filter applies; only 1:1 im is p2p.
- Document the group-addressing decision: explicit @bot mention required in groups; mention-free thread continuation deferred to the session-aware layer.
- Tests: handler-error propagation, slackChatType table, mpim-requires-mention, capabilities negative assertions.

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

* refactor(channel): shared channel-agnostic ChatSession service (MUL-3516)

Extract the session/append//issue machinery — currently locked inside the Feishu-pinned lark.chatSessionService — into a shared engine.ChatSession parameterized by channel_type + session titles, so every IM adapter reuses it instead of re-implementing it. Logic is verbatim (find-or-create session+binding with unique-violation race re-read; append+touch+reply-target+in-tx dedup Mark; /issue parse with bare-command previous-message fallback) but channel-neutral: command-parse source is supplied by the adapter (enrichment is platform-specific). Backed by a narrow SessionQueries interface so it is unit-tested with an in-memory fake (no DB). /issue parser moved to engine.ParseIssueCommand. Next: migrate Feishu onto it and wire Slack's ResolverSet, removing the lark duplicate.

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

* fix(channel): decouple session binding key from outbound target (MUL-3516)

Addresses Elon's round-2 review. engine.ChatSession.EnsureSession previously keyed the binding on a raw chat id (EnsureSessionInput.ChatID), so a resolver wiring Slack straight through would collapse every @bot thread in one channel into a single chat_session and overwrite last_thread_id. Make the API un-misusable:

- EnsureSessionInput.ChatID -> BindingKey: the explicit session-isolation key (Feishu: chat id; Slack DM: channel id; Slack channel: channel id + thread root), documented so a raw threaded-platform chat id is never passed straight through.
- Add EnsureSessionInput.BindingConfig (opaque) persisted on the binding's config column, so the real outbound channel/thread is preserved when BindingKey is composite — outbound routing stays separate from the isolation key.
- channel.sql CreateChannelChatSessionBinding now writes config (additive, uses the existing NOT NULL column; lark caller passes '{}', no schema change, no Feishu regression).
- Tests: TestEnsureSession_ThreadRootIsolation (two thread roots in one channel -> two sessions; same root reuses) and TestEnsureSession_StoresBindingConfig.

No production wiring change yet (per review, the not-yet-wired shared service is an accepted preparatory state); this makes the API correct before Feishu/Slack are migrated onto it.

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

* feat(slack): Slack ResolverSet with thread-root session isolation (MUL-3516)

Wires Slack into the channel-agnostic engine.Router via a ResolverSet built on the generic channel_* queries (installation route by team_id, identity + workspace-membership recheck, two-phase dedup, audit) plus the shared engine.ChatSession. No new query, no schema change.

slackSessionRouting is the per-message isolation rule (Elon round-2 / Niko round-3): a DM is one session per channel; a channel/group message is isolated by thread root (key = channel:threadRoot, root = inbound thread_ts or the message ts for a top-level @mention), so two @bot threads in one channel are two sessions. The real channel id rides in BindingConfig for outbound; the reply thread is returned separately. Tests cover DM/channel/thread routing, config, and that distinct thread roots isolate while a same-thread follow-up reuses its key.

Not yet wired into router.go (still a preparatory commit, per review); Feishu migration onto the shared service, router/config wiring, and the Slack outbound path follow.

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

* feat(slack): Markdown->mrkdwn outbound formatting (MUL-3516)

Slack renders mrkdwn, not Markdown, so an unconverted agent reply shows literal ** , ## and [text](url). Add formatMrkdwn — a faithful Go port of Hermes Agent's slack format_message (MIT) — and apply it in slackChannel.Send before chunking/posting. Protects fenced+inline code, converted links, and existing Slack entities behind placeholders; converts headers/bold/italic/strike/links; escapes control chars. Unit tests cover each construct plus fenced-code protection and a link nested in bold.

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

* docs(slack): preserve Hermes MIT notice for ported mrkdwn converter (MUL-3516)

Addresses Niko's review. formatMrkdwn is a substantial port of Hermes Agent's slack format_message; MIT requires preserving the copyright + permission notice. Add the full Hermes MIT copyright/permission notice + source URL as a header on mrkdwn.go (no repo-level third-party notice file exists, and the header cannot get separated from the ported code). Also add the suggested Send-layer regression test (TestSend_AppliesMrkdwn) that pins the wiring: slackChannel.Send converts Markdown to mrkdwn before posting.

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

* refactor(lark): migrate Feishu onto shared engine.ChatSession, drop duplicate (MUL-3516)

Completes 'every IM reuses one shared session service' and removes the dual-path the reviewers flagged as temporary. Feishu's ResolverSet now drives the channel-agnostic engine.ChatSession (channel_type=feishu, Lark session titles preserved) instead of the Feishu-specific lark.chatSessionService, which is deleted. Behavior is unchanged: engine.ChatSession is the verbatim port of the old logic and is unit-tested; the new Feishu binder param-mapping (BindingKey=chat id, CommandText=un-enriched CommandBody from Raw) is covered by feishu_resolvers_test.go.

- Delete chat_service.go (chatSessionService + helpers) and issue_command.go/_test.go (parser now engine.ParseIssueCommand). Relocate the shared TxStarter interface to tx.go (still used by binding-token + registration services).
- chat.go keeps only the AuditLogger seam; remove the now-dead ChatSessionService / EnsureChatSessionParams / AppendUserMessageParams / AppendResult / IssueCommand types.
- router.go constructs engine.NewChatSession for Feishu; inbound_enricher_test + doc.go updated.

make-test parity: go build ./..., go vet, gofmt, and go test ./internal/integrations/{lark,channel/...,slack} all pass (full Feishu suite green).

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

* feat(slack): wire Slack adapter + ResolverSet + outbound into router (MUL-3516)

Activates the full Slack pipeline, gated by MULTICA_SLACK_SECRET_KEY (the bot/app-token decryption key). When unset the block is skipped, so existing deployments are unaffected and Feishu is untouched.

- router.go registers slack.RegisterSlack (Socket Mode connect/send Factory) + channelRouter.Register(TypeSlack, NewSlackResolverSet) (inbound pipeline) + slack.NewOutbound(...).Register(bus) (outbound).
- New slack/outbound.go: an EventChatDone subscriber mirroring the Feishu Patcher. It finds the Slack chat binding for the finished session, recovers the real channel from the binding config (the channel_chat_id may be a composite thread-isolation key) + the reply thread from last_thread_id, and posts via slackChannel.Send (reusing formatMrkdwn / chunking / threading). Sessions with no Slack binding are ignored, so it coexists with the Feishu Patcher on the shared bus.
- Tests: posts to the bound channel/thread with the real channel id; ignores non-Slack sessions, empty completions, revoked installations, and non-chat events.

Slack now shares engine.ChatSession, channel_* tables, IssueService and TaskService with Feishu. Remaining: config-driven installation provisioning (an operator currently creates the channel_type='slack' row; the config block shape — which workspace/agent — is a product decision) and a live end-to-end smoke. go build ./..., go vet, gofmt, and go test ./internal/integrations/{slack,channel/...,lark} all pass.

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

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-25 14:29:00 +08:00